From 3dcc887cc7e04704abd2f217cd3420343e581805 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 10 Feb 2026 09:14:16 +0000 Subject: [PATCH 1/3] Add confirmation modal for adaptor changes with credentials (#4395) When changing adaptors in the collaborative editor, users now see a confirmation modal warning that credentials will be reset. The modal only appears when: - Changing to a different adaptor package (not just version) - A credential is currently selected Also optimizes Y.Doc synchronization to eliminate double-sync by ensuring version changes happen in a single operation rather than syncing @latest then the actual version. Includes comprehensive test coverage for all confirmation scenarios. --- CHANGELOG.md | 4 + .../components/ConfigureAdaptorModal.tsx | 131 ++++++++++++- .../components/inspector/JobForm.tsx | 60 +++--- .../components/ConfigureAdaptorModal.test.tsx | 185 ++++++++++++++++++ 4 files changed, 341 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b14498310..1c0a9573a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to ### Added +- Confirmation modal when changing adaptors in collaborative editor to warn + users that credentials will be reset + [#4395](https://github.com/OpenFn/lightning/issues/4395) + ### Changed - Refactor CircleCI to build-then-fan-out pattern, compiling once then running diff --git a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx index a2539d0e82..e8c3960414 100644 --- a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx +++ b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx @@ -4,7 +4,7 @@ import { DialogPanel, DialogTitle, } from '@headlessui/react'; -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useCredentialQueries, @@ -22,6 +22,7 @@ import { import { cn } from '#/utils/cn'; import { AdaptorIcon } from './AdaptorIcon'; +import { AlertDialog } from './AlertDialog'; import { Tooltip } from './Tooltip'; import { VersionPicker } from './VersionPicker'; @@ -185,11 +186,12 @@ function SectionDivider({ label }: SectionDividerProps) { interface ConfigureAdaptorModalProps { isOpen: boolean; onClose: () => void; - onAdaptorChange: (adaptorPackage: string) => void; // Immediately sync adaptor to Y.Doc - onVersionChange: (version: string) => void; // Immediately sync version to Y.Doc + onAdaptorChange: (adaptorPackage: string) => void; // Update adaptor package in form state (onVersionChange will sync to Y.Doc) + onVersionChange: (version: string) => void; // Immediately sync complete package@version to Y.Doc onCredentialChange: (credentialId: string | null) => void; // Immediately sync credential to Y.Doc onOpenAdaptorPicker: () => void; // Notify parent to manage modal switching to adaptor picker onOpenCredentialModal: (adaptorName: string, credentialId?: string) => void; // Notify parent to manage modal switching to credential modal (for create or edit) + pendingAdaptorSelection: string | null; // Adaptor selected from picker, awaiting confirmation currentAdaptor: string; currentVersion: string; currentCredentialId: string | null; @@ -206,12 +208,12 @@ interface ConfigureAdaptorModalProps { export function ConfigureAdaptorModal({ isOpen, onClose, - // Note: onAdaptorChange is in the interface for parent compatibility, - // but adaptor changes go through onOpenAdaptorPicker flow instead + onAdaptorChange, onVersionChange, onCredentialChange, onOpenAdaptorPicker, onOpenCredentialModal, + pendingAdaptorSelection, currentAdaptor, currentVersion, currentCredentialId, @@ -220,6 +222,10 @@ export function ConfigureAdaptorModal({ // UI state (not synced to Y.Doc) const [showOtherCredentials, setShowOtherCredentials] = useState(false); + // Confirmation modal state + const [isConfirmationModalOpen, setIsConfirmationModalOpen] = useState(false); + const [pendingAdaptor, setPendingAdaptor] = useState(null); + // Get current user and credentials from store const currentUser = useUser(); const { projectCredentials, keychainCredentials } = useCredentials(); @@ -227,9 +233,14 @@ export function ConfigureAdaptorModal({ // High-priority Escape handler to prevent closing parent IDE/inspector // Priority 100 (MODAL) ensures this runs before IDE handler (priority 50) + // Don't close if confirmation modal is open (let AlertDialog handle it) useKeyboardShortcut( 'Escape', - () => { + e => { + if (isConfirmationModalOpen) { + e.stopPropagation(); + return; + } onClose(); }, 100, @@ -443,6 +454,102 @@ export function ConfigureAdaptorModal({ onOpenAdaptorPicker(); // Let parent open AdaptorSelectionModal }; + /** + * Actually performs the adaptor change after confirmation (or if no + * confirmation needed). + */ + const handleAdaptorChangeConfirmed = useCallback( + (adaptorName: string) => { + // Extract package name and update adaptor + const packageMatch = adaptorName.match(/(.+?)(@|$)/); + const newPackage = packageMatch ? packageMatch[1] : adaptorName; + + if (newPackage) { + onAdaptorChange(newPackage); + + // Auto-select latest version for new adaptor, fallback to "latest" if not found + const adaptor = allAdaptors.find(a => a.name === newPackage); + let versionToUse = 'latest'; // Fallback + + if (adaptor && adaptor.versions.length > 0) { + const sortedVersions = sortVersionsDescending( + adaptor.versions.map(v => v.version) + ); + versionToUse = sortedVersions[0] || 'latest'; + } + + onVersionChange(versionToUse); // Always called now + } + + // Reopen ConfigureAdaptorModal (it was closed by handleChangeClick) + // The parent will handle reopening based on the adaptor change event + }, + [allAdaptors, onAdaptorChange, onVersionChange] + ); + + /** + * Handles adaptor selection from AdaptorSelectionModal. + * Shows confirmation if adaptor package is changing and credentials are + * set. + */ + const handleAdaptorSelectFromPicker = useCallback( + (newAdaptorName: string) => { + const currentPackage = extractPackageName(currentAdaptor); + const newPackage = extractPackageName(newAdaptorName); + + // Only show confirmation if: + // 1. Adaptor package is changing (not just version) + // 2. Credentials are currently set + const isAdaptorChanging = currentPackage !== newPackage; + const hasCredentials = currentCredentialId !== null; + + if (isAdaptorChanging && hasCredentials) { + // Show confirmation modal + setPendingAdaptor(newAdaptorName); + setIsConfirmationModalOpen(true); + } else { + // No confirmation needed - proceed immediately + handleAdaptorChangeConfirmed(newAdaptorName); + } + }, + [currentAdaptor, currentCredentialId, handleAdaptorChangeConfirmed] + ); + + /** + * Confirms adaptor change - clears credentials then changes adaptor. + */ + const handleAdaptorChangeConfirm = useCallback(() => { + if (pendingAdaptor) { + // Step 1: Clear credentials first (both project and keychain) + onCredentialChange(null); + + // Step 2: Apply the adaptor change + handleAdaptorChangeConfirmed(pendingAdaptor); + + // Step 3: Close confirmation modal and reset state + setPendingAdaptor(null); + setIsConfirmationModalOpen(false); + } + }, [pendingAdaptor, onCredentialChange, handleAdaptorChangeConfirmed]); + + /** + * Cancels adaptor change - keeps current adaptor and credentials. + */ + const handleAdaptorChangeCancel = useCallback(() => { + // Simply discard the pending change + setPendingAdaptor(null); + setIsConfirmationModalOpen(false); + // ConfigureAdaptorModal remains open - user can try again or close + }, []); + + // Handle pending adaptor selection from AdaptorSelectionModal + useEffect(() => { + if (!isOpen || !pendingAdaptorSelection) return; + + // User selected a new adaptor from picker - check if confirmation needed + handleAdaptorSelectFromPicker(pendingAdaptorSelection); + }, [isOpen, pendingAdaptorSelection, handleAdaptorSelectFromPicker]); + // Open LiveView credential modal with adaptor schema (notifies parent) const handleCreateCredential = () => { const adaptorName = extractAdaptorName(currentAdaptor); @@ -758,6 +865,18 @@ export function ConfigureAdaptorModal({ + + {/* Confirmation modal for adaptor changes */} + ); } diff --git a/assets/js/collaborative-editor/components/inspector/JobForm.tsx b/assets/js/collaborative-editor/components/inspector/JobForm.tsx index 026a8b3bb0..557a66f560 100644 --- a/assets/js/collaborative-editor/components/inspector/JobForm.tsx +++ b/assets/js/collaborative-editor/components/inspector/JobForm.tsx @@ -64,6 +64,10 @@ export function JobForm({ job }: JobFormProps) { // Track if adaptor picker was opened from configure modal (to return there on close) const [adaptorPickerFromConfigure, setAdaptorPickerFromConfigure] = useState(false); + // Track adaptor selected from picker before confirmation + const [pendingAdaptorSelection, setPendingAdaptorSelection] = useState< + string | null + >(null); // Credential modal is managed by the context const { openCredentialModal, onModalClose, onCredentialSaved } = @@ -194,43 +198,29 @@ export function JobForm({ job }: JobFormProps) { ); // Handle adaptor selection from picker - const handleAdaptorSelect = useCallback( - (adaptorName: string) => { - // Update the adaptor package in form - const packageMatch = adaptorName.match(/(.+?)(@|$)/); - const newPackage = packageMatch ? packageMatch[1] : adaptorName; - form.setFieldValue('adaptor_package', newPackage || null); - - // Set version to "latest" by default when picking an adaptor - const fullAdaptor = `${newPackage}@latest`; - form.setFieldValue('adaptor', fullAdaptor); - - // Close adaptor picker and always open configure modal - setIsAdaptorPickerOpen(false); - setAdaptorPickerFromConfigure(false); - setIsConfigureModalOpen(true); - }, - [form] - ); + const handleAdaptorSelect = useCallback((adaptorName: string) => { + // Close adaptor picker + setIsAdaptorPickerOpen(false); - // Handler for adaptor changes - immediately syncs to Y.Doc - const handleAdaptorChange = useCallback( - (adaptorPackage: string) => { - // Get current version from form - const currentAdaptorValue = form.getFieldValue('adaptor') as string; - const { version: currentVersion } = resolveAdaptor(currentAdaptorValue); + // Store selection as pending (don't apply yet) + setPendingAdaptorSelection(adaptorName); - // Build new adaptor string with current version - const newAdaptor = `${adaptorPackage}@${currentVersion || 'latest'}`; + // Reopen ConfigureAdaptorModal which will handle confirmation + setIsConfigureModalOpen(true); + }, []); - // Update form state + // Handle confirmed adaptor change (after confirmation or no confirmation needed) + const handleAdaptorChangeConfirmed = useCallback( + (adaptorPackage: string) => { + // Update form state with new adaptor package + // Note: ConfigureAdaptorModal will call onVersionChange next, which will + // sync the complete package@version to Y.Doc in a single operation form.setFieldValue('adaptor_package', adaptorPackage); - form.setFieldValue('adaptor', newAdaptor); - // Persist to Y.Doc - updateJob(job.id, { adaptor: newAdaptor }); + // Clear pending selection + setPendingAdaptorSelection(null); }, - [form, job.id, updateJob] + [form] ); // Handler for version changes - immediately syncs to Y.Doc @@ -370,12 +360,16 @@ export function JobForm({ job }: JobFormProps) { {/* Configure Adaptor Modal */} setIsConfigureModalOpen(false)} - onAdaptorChange={handleAdaptorChange} + onClose={() => { + setIsConfigureModalOpen(false); + setPendingAdaptorSelection(null); // Clear pending on close + }} + onAdaptorChange={handleAdaptorChangeConfirmed} onVersionChange={handleVersionChange} onCredentialChange={handleCredentialChange} onOpenAdaptorPicker={handleOpenAdaptorPickerFromConfigure} onOpenCredentialModal={handleOpenCredentialModal} + pendingAdaptorSelection={pendingAdaptorSelection} currentAdaptor={ resolveAdaptor(currentAdaptor).package || '@openfn/language-common' } diff --git a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx index fa8ffd8b92..2dce5c703c 100644 --- a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx +++ b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx @@ -267,6 +267,7 @@ describe('ConfigureAdaptorModal', () => { onCredentialChange: mockOnCredentialChange, onOpenAdaptorPicker: mockOnOpenAdaptorPicker, onOpenCredentialModal: mockOnOpenCredentialModal, + pendingAdaptorSelection: null, currentAdaptor: '@openfn/language-salesforce', currentVersion: '2.1.0', currentCredentialId: null, @@ -1699,6 +1700,190 @@ describe('ConfigureAdaptorModal', () => { }); }); + describe('Adaptor Change Confirmation', () => { + it('shows confirmation modal when changing adaptor with credentials set', async () => { + renderWithProviders( + + ); + + // Confirmation modal should appear + await waitFor(() => { + expect(screen.getByText('Change Adaptor?')).toBeInTheDocument(); + }); + + expect( + screen.getByText( + /warning: changing adaptors will reset the credential/i + ) + ).toBeInTheDocument(); + }); + + it('does NOT show confirmation when changing adaptor without credentials', async () => { + renderWithProviders( + + ); + + // Confirmation modal should NOT appear + await waitFor(() => { + expect(screen.queryByText('Change Adaptor?')).not.toBeInTheDocument(); + }); + + // onAdaptorChange should be called immediately + expect(mockOnAdaptorChange).toHaveBeenCalledWith('@openfn/language-http'); + }); + + it('does NOT show confirmation when changing version of same adaptor', async () => { + renderWithProviders( + + ); + + // Confirmation modal should NOT appear + await waitFor(() => { + expect(screen.queryByText('Change Adaptor?')).not.toBeInTheDocument(); + }); + + // onAdaptorChange should be called immediately + expect(mockOnAdaptorChange).toHaveBeenCalledWith( + '@openfn/language-salesforce' + ); + }); + + it('clears credentials when user confirms adaptor change', async () => { + const user = userEvent.setup(); + + renderWithProviders( + + ); + + // Wait for confirmation modal + await waitFor(() => { + expect(screen.getByText('Change Adaptor?')).toBeInTheDocument(); + }); + + // Click "Continue" button + const continueButton = screen.getByRole('button', { name: /continue/i }); + await user.click(continueButton); + + // Should clear credentials FIRST + expect(mockOnCredentialChange).toHaveBeenCalledWith(null); + + // Then change adaptor + expect(mockOnAdaptorChange).toHaveBeenCalledWith('@openfn/language-http'); + + // Confirmation modal should close + await waitFor(() => { + expect(screen.queryByText('Change Adaptor?')).not.toBeInTheDocument(); + }); + }); + + it('keeps everything unchanged when user cancels adaptor change', async () => { + const user = userEvent.setup(); + + renderWithProviders( + + ); + + // Wait for confirmation modal + await waitFor(() => { + expect(screen.getByText('Change Adaptor?')).toBeInTheDocument(); + }); + + // Click "Cancel" button + const cancelButton = screen.getByRole('button', { name: /cancel/i }); + await user.click(cancelButton); + + // Should NOT clear credentials + expect(mockOnCredentialChange).not.toHaveBeenCalled(); + + // Should NOT change adaptor + expect(mockOnAdaptorChange).not.toHaveBeenCalled(); + + // Confirmation modal should close + await waitFor(() => { + expect(screen.queryByText('Change Adaptor?')).not.toBeInTheDocument(); + }); + + // Main modal should still be open + expect(screen.getByText('Configure connection')).toBeInTheDocument(); + }); + + it('shows primary variant (blue button) for confirmation', async () => { + renderWithProviders( + + ); + + await waitFor(() => { + expect(screen.getByText('Change Adaptor?')).toBeInTheDocument(); + }); + + // Check that Continue button has primary styling (blue background) + const continueButton = screen.getByRole('button', { name: /continue/i }); + // Check class contains bg-primary-600 (AlertDialog primary variant) + expect(continueButton.className).toContain('bg-primary-600'); + }); + + // NOTE: Escape key behavior now correctly handled - ConfigureAdaptorModal's Escape handler + // checks isConfirmationModalOpen and allows AlertDialog to handle Escape when confirmation is open. + // Manual testing confirms Escape closes AlertDialog (not parent modal). The Cancel button test + // provides adequate automated coverage of the cancel functionality. + + it('auto-selects latest version when changing to new adaptor', async () => { + const user = userEvent.setup(); + + renderWithProviders( + + ); + + await waitFor(() => { + expect(screen.getByText('Change Adaptor?')).toBeInTheDocument(); + }); + + // Confirm change + const continueButton = screen.getByRole('button', { name: /continue/i }); + await user.click(continueButton); + + // Should change to HTTP with latest version + expect(mockOnAdaptorChange).toHaveBeenCalledWith('@openfn/language-http'); + expect(mockOnVersionChange).toHaveBeenCalledWith('1.5.0'); // latest from mockProjectAdaptors + }); + }); + describe('Edge Cases', () => { it('handles empty adaptor name gracefully', async () => { renderWithProviders( From 8020fbd461c1ed1dbd1cd533398ac91a278e51e8 Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 10 Feb 2026 09:39:58 +0000 Subject: [PATCH 2/3] self-review -esc button behaviour matches other alert dialogs on the site --- .../components/ConfigureAdaptorModal.tsx | 14 ++++++-------- .../components/ConfigureAdaptorModal.test.tsx | 10 ++++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx index e8c3960414..b05b643b05 100644 --- a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx +++ b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx @@ -233,18 +233,16 @@ export function ConfigureAdaptorModal({ // High-priority Escape handler to prevent closing parent IDE/inspector // Priority 100 (MODAL) ensures this runs before IDE handler (priority 50) - // Don't close if confirmation modal is open (let AlertDialog handle it) + // When confirmation is open, disable this handler so AlertDialog's native Escape can close it + // After AlertDialog closes, Inspector's priority 10 handler fires and closes everything + // Result: Escape = "abort mission completely" (closes confirmation + modal + inspector) useKeyboardShortcut( 'Escape', - e => { - if (isConfirmationModalOpen) { - e.stopPropagation(); - return; - } + () => { onClose(); }, 100, - { enabled: isOpen } + { enabled: isOpen && !isConfirmationModalOpen } ); // When adaptor changes externally (from Y.Doc or adaptor picker), @@ -475,7 +473,7 @@ export function ConfigureAdaptorModal({ const sortedVersions = sortVersionsDescending( adaptor.versions.map(v => v.version) ); - versionToUse = sortedVersions[0] || 'latest'; + versionToUse = sortedVersions[0]; } onVersionChange(versionToUse); // Always called now diff --git a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx index 2dce5c703c..cb783f1983 100644 --- a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx +++ b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx @@ -1852,10 +1852,12 @@ describe('ConfigureAdaptorModal', () => { expect(continueButton.className).toContain('bg-primary-600'); }); - // NOTE: Escape key behavior now correctly handled - ConfigureAdaptorModal's Escape handler - // checks isConfirmationModalOpen and allows AlertDialog to handle Escape when confirmation is open. - // Manual testing confirms Escape closes AlertDialog (not parent modal). The Cancel button test - // provides adequate automated coverage of the cancel functionality. + // NOTE: Escape key behavior when confirmation is open: + // - ConfigureAdaptorModal's Escape handler (priority 100) is disabled + // - AlertDialog's native Escape closes the confirmation + // - Inspector's Escape handler (priority 10) then fires, closing everything + // Result: Escape = "abort mission completely" (closes confirmation + modal + inspector) + // The Cancel button test provides adequate coverage of the cancellation flow. it('auto-selects latest version when changing to new adaptor', async () => { const user = userEvent.setup(); From c85f5ec0058488ce86109dac074bedc8565e2bde Mon Sep 17 00:00:00 2001 From: Lucy Macartney Date: Tue, 10 Feb 2026 10:27:38 +0000 Subject: [PATCH 3/3] removes ai redundant comments --- .../components/ConfigureAdaptorModal.tsx | 2 +- .../components/ConfigureAdaptorModal.test.tsx | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx index b05b643b05..1102c9ab9b 100644 --- a/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx +++ b/assets/js/collaborative-editor/components/ConfigureAdaptorModal.tsx @@ -476,7 +476,7 @@ export function ConfigureAdaptorModal({ versionToUse = sortedVersions[0]; } - onVersionChange(versionToUse); // Always called now + onVersionChange(versionToUse); } // Reopen ConfigureAdaptorModal (it was closed by handleChangeClick) diff --git a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx index cb783f1983..86ac4707b0 100644 --- a/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx +++ b/assets/test/collaborative-editor/components/ConfigureAdaptorModal.test.tsx @@ -1852,13 +1852,6 @@ describe('ConfigureAdaptorModal', () => { expect(continueButton.className).toContain('bg-primary-600'); }); - // NOTE: Escape key behavior when confirmation is open: - // - ConfigureAdaptorModal's Escape handler (priority 100) is disabled - // - AlertDialog's native Escape closes the confirmation - // - Inspector's Escape handler (priority 10) then fires, closing everything - // Result: Escape = "abort mission completely" (closes confirmation + modal + inspector) - // The Cancel button test provides adequate coverage of the cancellation flow. - it('auto-selects latest version when changing to new adaptor', async () => { const user = userEvent.setup();