fix(editor): Enhance error handling and toast notifications in WorkflowDiffModal (#20812)

packages/frontend/editor-ui/src/features/sourceControl.ee/components/SourceControlPushModal.test.ts

@@ -146,14 +146,15 @@ describe('SourceControlPushModal', () => {
 		// Setup store with default mock to prevent automatic data loading
 		pinia = createTestingPinia();
 		sourceControlStore = mockedStore(useSourceControlStore);
-		sourceControlStore.getAggregatedStatus = vi.fn().mockResolvedValue([]);
-		sourceControlStore.pushWorkfolder = vi.fn().mockResolvedValue([]);
+		sourceControlStore.getAggregatedStatus.mockResolvedValue([]);
 
 		settingsStore = mockedStore(useSettingsStore);
 		settingsStore.settings.enterprise = defaultSettings.enterprise;
 	});
 
-	it('mounts', () => {
+	it('mounts', async () => {
+		sourceControlStore.getAggregatedStatus.mockResolvedValue([]);
+
 		const { getByText } = renderModal({
 			pinia,
 			props: {
@@ -163,7 +164,10 @@ describe('SourceControlPushModal', () => {
 				},
 			},
 		});
-		expect(getByText('Commit and push changes')).toBeInTheDocument();
+
+		await waitFor(() => {
+			expect(getByText('Commit and push changes')).toBeInTheDocument();
+		});
 	});
 
 	it('should toggle checkboxes', async () => {
@@ -190,7 +194,10 @@ describe('SourceControlPushModal', () => {
 			},
 		];
 
-		const { getByTestId, getAllByTestId } = renderModal({
+		sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+		const { getByTestId, getAllByTestId, getByText } = renderModal({
+			pinia,
 			props: {
 				data: {
 					eventBus,
@@ -199,8 +206,17 @@ describe('SourceControlPushModal', () => {
 			},
 		});
 
+		// Wait for modal content to be visible (v-if="!isLoading" check passes)
+		await waitFor(() => {
+			expect(getByText('Commit and push changes')).toBeInTheDocument();
+		});
+
+		await waitFor(() => {
+			const files = getAllByTestId('source-control-push-modal-file-checkbox');
+			expect(files).toHaveLength(2);
+		});
+
 		const files = getAllByTestId('source-control-push-modal-file-checkbox');
-		expect(files).toHaveLength(2);
 
 		await userEvent.click(files[0]);
 		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
@@ -299,7 +315,9 @@ describe('SourceControlPushModal', () => {
 			},
 		];
 
-		const { getByTestId, getByRole } = renderModal({
+		sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+		const { getByTestId, getByRole, getByText } = renderModal({
 			pinia,
 			props: {
 				data: {
@@ -309,9 +327,18 @@ describe('SourceControlPushModal', () => {
 			},
 		});
 
+		// Wait for modal content to be visible
+		await waitFor(() => {
+			expect(getByText('Commit and push changes')).toBeInTheDocument();
+		});
+
+		await waitFor(() => {
+			const submitButton = getByTestId('source-control-push-modal-submit');
+			expect(submitButton).toBeDisabled();
+		});
+
 		const submitButton = getByTestId('source-control-push-modal-submit');
 		const commitMessage = 'commit message';
-		expect(submitButton).toBeDisabled();
 
 		expect(getByRole('alert').textContent).toContain(
 			[
@@ -360,10 +387,12 @@ describe('SourceControlPushModal', () => {
 			},
 		];
 
+		sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
 		mockRoute.name = VIEWS.WORKFLOW;
 		mockRoute.params = { name: 'gTbbBkkYTnNyX1jD' };
 
-		const { getByTestId, getAllByTestId } = renderModal({
+		const { getByTestId, getAllByTestId, getByText } = renderModal({
 			pinia,
 			props: {
 				data: {
@@ -373,8 +402,17 @@ describe('SourceControlPushModal', () => {
 			},
 		});
 
+		// Wait for modal content to be visible
+		await waitFor(() => {
+			expect(getByText('Commit and push changes')).toBeInTheDocument();
+		});
+
+		await waitFor(() => {
+			const files = getAllByTestId('source-control-push-modal-file-checkbox');
+			expect(files).toHaveLength(2);
+		});
+
 		const files = getAllByTestId('source-control-push-modal-file-checkbox');
-		expect(files).toHaveLength(2);
 
 		// The current workflow should be auto-selected now that we fixed the regression
 		expect(within(files[0]).getByRole('checkbox')).toBeChecked();
@@ -419,7 +457,10 @@ describe('SourceControlPushModal', () => {
 			},
 		];
 
-		const { getAllByTestId } = renderModal({
+		sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+		const { getAllByTestId, getByText } = renderModal({
+			pinia,
 			props: {
 				data: {
 					eventBus,
@@ -428,8 +469,15 @@ describe('SourceControlPushModal', () => {
 			},
 		});
 
-		const workflows = getAllByTestId('source-control-push-modal-file-checkbox');
-		expect(workflows).toHaveLength(2);
+		// Wait for modal content to be visible
+		await waitFor(() => {
+			expect(getByText('Commit and push changes')).toBeInTheDocument();
+		});
+
+		await waitFor(() => {
+			const workflows = getAllByTestId('source-control-push-modal-file-checkbox');
+			expect(workflows).toHaveLength(2);
+		});
 
 		const tab = getAllByTestId('source-control-push-modal-tab').filter(({ textContent }) =>
 			textContent?.includes('Credentials'),
@@ -467,7 +515,10 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			const { getByTestId, getAllByTestId } = renderModal({
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+			const { getByTestId, getAllByTestId, getByText } = renderModal({
+				pinia,
 				props: {
 					data: {
 						eventBus,
@@ -476,7 +527,14 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
-			expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			});
 
 			await userEvent.type(getByTestId('source-control-push-search'), '1');
 			await waitFor(() => {
@@ -511,7 +569,10 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			const { getByTestId, getAllByTestId } = renderModal({
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+			const { getByTestId, getAllByTestId, getByText } = renderModal({
+				pinia,
 				props: {
 					data: {
 						eventBus,
@@ -520,7 +581,14 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
-			expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			});
 
 			await userEvent.click(getByTestId('source-control-filter-dropdown'));
 
@@ -588,7 +656,10 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			const { getByTestId, getAllByTestId } = renderModal({
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+			const { getByTestId, getAllByTestId, getByText } = renderModal({
+				pinia,
 				props: {
 					data: {
 						eventBus,
@@ -597,13 +668,27 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				const tab = getAllByTestId('source-control-push-modal-tab').filter(({ textContent }) =>
+					textContent?.includes(name),
+				);
+				expect(tab.length).toBeGreaterThan(0);
+			});
+
 			const tab = getAllByTestId('source-control-push-modal-tab').filter(({ textContent }) =>
 				textContent?.includes(name),
 			);
 
 			await userEvent.click(tab[0]);
 
-			expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			await waitFor(() => {
+				expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(2);
+			});
 
 			await userEvent.click(getByTestId('source-control-filter-dropdown'));
 
@@ -635,7 +720,10 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			const { getByTestId, getAllByTestId, queryAllByTestId } = renderModal({
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+			const { getByTestId, getAllByTestId, queryAllByTestId, getByText } = renderModal({
+				pinia,
 				props: {
 					data: {
 						eventBus,
@@ -644,7 +732,14 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
-			expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(1);
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(1);
+			});
 
 			await userEvent.click(getByTestId('source-control-filter-dropdown'));
 
@@ -689,9 +784,9 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			// Use the existing store instance from beforeEach
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
 
-			const { getByTestId } = renderModal({
+			const { getByTestId, getByText } = renderModal({
 				pinia,
 				props: {
 					data: {
@@ -701,6 +796,15 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getByTestId('source-control-push-modal-commit')).toBeInTheDocument();
+			});
+
 			const commitInput = getByTestId('source-control-push-modal-commit');
 			const commitMessage = 'Test commit message';
 
@@ -732,9 +836,9 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			// Use the existing store instance from beforeEach
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
 
-			const { getByTestId } = renderModal({
+			const { getByTestId, getByText } = renderModal({
 				pinia,
 				props: {
 					data: {
@@ -744,6 +848,15 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getByTestId('source-control-push-modal-commit')).toBeInTheDocument();
+			});
+
 			const commitInput = getByTestId('source-control-push-modal-commit');
 
 			expect(getByTestId('source-control-push-modal-submit')).toBeDisabled();
@@ -767,12 +880,12 @@ describe('SourceControlPushModal', () => {
 				},
 			];
 
-			// Use the existing store instance from beforeEach
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
 
 			mockRoute.name = 'SOME_OTHER_VIEW';
 			mockRoute.params = { name: 'differentId' };
 
-			const { getByTestId, getAllByTestId } = renderModal({
+			const { getByTestId, getAllByTestId, getByText } = renderModal({
 				pinia,
 				props: {
 					data: {
@@ -782,11 +895,20 @@ describe('SourceControlPushModal', () => {
 				},
 			});
 
+			// Wait for modal content to be visible
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				const files = getAllByTestId('source-control-push-modal-file-checkbox');
+				expect(files).toHaveLength(1);
+			});
+
 			const commitInput = getByTestId('source-control-push-modal-commit');
 			const commitMessage = 'Test commit message';
 
 			const files = getAllByTestId('source-control-push-modal-file-checkbox');
-			expect(files).toHaveLength(1);
 			expect(within(files[0]).getByRole('checkbox')).not.toBeChecked();
 
 			await userEvent.type(commitInput, commitMessage);

packages/frontend/editor-ui/src/features/sourceControl.ee/components/SourceControlPushModal.vue

@@ -659,10 +659,8 @@ watchEffect(() => {
 
 // Load data when modal opens
 onMounted(async () => {
-	// Only load fresh data if we don't have any initial data
-	if (!props.data.status || props.data.status.length === 0) {
-		await loadSourceControlStatus();
-	}
+	// Always load fresh data to ensure workflow names are populated
+	await loadSourceControlStatus();
 });
 </script>
 

packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.test.ts

@@ -17,12 +17,26 @@ const mockRoute = reactive({
 	name: '',
 	params: {},
 	fullPath: '',
+	query: {},
 });
 
+const mockRouterBack = vi.fn();
+const mockRouterReplace = vi.fn();
+const mockShowError = vi.fn();
+
+vi.mock('@/composables/useToast', () => ({
+	useToast: vi.fn(() => ({
+		showError: mockShowError,
+	})),
+}));
+
 vi.mock('vue-router', () => ({
 	useRoute: () => mockRoute,
 	RouterLink: vi.fn(),
-	useRouter: vi.fn(),
+	useRouter: () => ({
+		back: mockRouterBack,
+		replace: mockRouterReplace,
+	}),
 }));
 
 vi.mock('@/features/workflow-diff/useViewportSync', () => ({
@@ -106,6 +120,17 @@ describe('WorkflowDiffModal', () => {
 
 	beforeEach(() => {
 		vi.clearAllMocks();
+		mockShowError.mockClear();
+		mockRouterBack.mockClear();
+		mockRouterReplace.mockClear();
+
+		// Mock window.history.length to simulate having navigation history
+		Object.defineProperty(window.history, 'length', {
+			writable: true,
+			configurable: true,
+			value: 2,
+		});
+
 		createTestingPinia();
 		nodeTypesStore = mockedStore(useNodeTypesStore);
 		sourceControlStore = mockedStore(useSourceControlStore);
@@ -451,4 +476,219 @@ describe('WorkflowDiffModal', () => {
 			});
 		});
 	});
+
+	describe('Error Handling - Remote Workflow', () => {
+		it('should show toast error and close modal when remote workflow fails to load', async () => {
+			sourceControlStore.getRemoteWorkflow.mockRejectedValue(
+				new Error('Remote API error') as never,
+			);
+
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(mockShowError).toHaveBeenCalledTimes(1);
+				expect(mockRouterBack).toHaveBeenCalledTimes(1);
+			});
+		});
+
+		it('should continue loading local workflow when remote fails', async () => {
+			sourceControlStore.getRemoteWorkflow.mockRejectedValue(
+				new Error('Remote API error') as never,
+			);
+
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(workflowsStore.fetchWorkflow).toHaveBeenCalledWith('test-workflow-id');
+			});
+		});
+	});
+
+	describe('Error Handling - Local Workflow', () => {
+		it('should show toast error and close modal when local workflow fails to load', async () => {
+			workflowsStore.fetchWorkflow.mockRejectedValue(new Error('Local API error') as never);
+
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(mockShowError).toHaveBeenCalledTimes(1);
+				expect(mockRouterBack).toHaveBeenCalledTimes(1);
+			});
+		});
+
+		it('should continue loading remote workflow when local fails', async () => {
+			workflowsStore.fetchWorkflow.mockRejectedValue(new Error('Local API error') as never);
+
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(sourceControlStore.getRemoteWorkflow).toHaveBeenCalledWith('test-workflow-id');
+			});
+		});
+	});
+
+	describe('Error Handling - Both Workflows', () => {
+		it('should show both toast errors but close modal only once when both workflows fail', async () => {
+			sourceControlStore.getRemoteWorkflow.mockRejectedValue(
+				new Error('Remote API error') as never,
+			);
+			workflowsStore.fetchWorkflow.mockRejectedValue(new Error('Local API error') as never);
+
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(mockShowError).toHaveBeenCalledTimes(2);
+				expect(mockRouterBack).toHaveBeenCalledTimes(1);
+			});
+		});
+	});
+
+	describe('Successful Loading', () => {
+		it('should load both remote and local workflows successfully', async () => {
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(sourceControlStore.getRemoteWorkflow).toHaveBeenCalledWith('test-workflow-id');
+				expect(workflowsStore.fetchWorkflow).toHaveBeenCalledWith('test-workflow-id');
+			});
+		});
+
+		it('should load node types before fetching workflows', async () => {
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(nodeTypesStore.loadNodeTypesIfNotLoaded).toHaveBeenCalled();
+			});
+		});
+
+		it('should not show toast errors on successful load', async () => {
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(sourceControlStore.getRemoteWorkflow).toHaveBeenCalled();
+				expect(workflowsStore.fetchWorkflow).toHaveBeenCalled();
+			});
+
+			expect(mockShowError).not.toHaveBeenCalled();
+		});
+	});
+
+	describe('handleBeforeClose behavior', () => {
+		it('should call router.back when history length > 1', async () => {
+			Object.defineProperty(window.history, 'length', {
+				writable: true,
+				configurable: true,
+				value: 2,
+			});
+
+			const { container } = renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			const backButton = container.querySelector('[data-icon="arrow-left"]');
+			await userEvent.click(backButton!);
+
+			expect(mockRouterBack).toHaveBeenCalledTimes(1);
+			expect(mockRouterReplace).not.toHaveBeenCalled();
+		});
+
+		it('should call router.replace when history length = 1', async () => {
+			Object.defineProperty(window.history, 'length', {
+				writable: true,
+				configurable: true,
+				value: 1,
+			});
+
+			mockRoute.query = { diff: 'workflow-id', direction: 'push', other: 'param' };
+
+			const { container } = renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						direction: 'push',
+					},
+				},
+			});
+
+			const backButton = container.querySelector('[data-icon="arrow-left"]');
+			await userEvent.click(backButton!);
+
+			expect(mockRouterBack).not.toHaveBeenCalled();
+			expect(mockRouterReplace).toHaveBeenCalledTimes(1);
+			expect(mockRouterReplace).toHaveBeenCalledWith({
+				query: { other: 'param' },
+			});
+		});
+	});
 });

packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.vue

@@ -3,6 +3,7 @@ import Node from '@/features/canvas/components/elements/nodes/CanvasNode.vue';
 import Modal from '@/components/Modal.vue';
 import NodeIcon from '@/components/NodeIcon.vue';
 import { useTelemetry } from '@/composables/useTelemetry';
+import { useToast } from '@/composables/useToast';
 import { STICKY_NODE_TYPE, WORKFLOW_DIFF_MODAL_KEY } from '@/constants';
 import DiffBadge from '@/features/workflow-diff/DiffBadge.vue';
 import NodeDiff from '@/features/workflow-diff/NodeDiff.vue';
@@ -41,6 +42,7 @@ const props = defineProps<{
 const { selectedDetailId, onNodeClick, syncIsEnabled } = useProvideViewportSync();
 
 const telemetry = useTelemetry();
+const toast = useToast();
 const $style = useCssModule();
 const nodeTypesStore = useNodeTypesStore();
 const sourceControlStore = useSourceControlStore();
@@ -56,13 +58,17 @@ const manualAsyncConfiguration = {
 	immediate: false,
 } as const;
 
+const isClosed = ref(false);
+
 const remote = useAsyncState<{ workflow?: IWorkflowDb; remote: boolean } | undefined, [], false>(
 	async () => {
 		try {
 			const { workflowId } = props.data;
 			const { content: workflow } = await sourceControlStore.getRemoteWorkflow(workflowId);
 			return { workflow, remote: true };
-		} catch {
+		} catch (error) {
+			toast.showError(error, i18n.baseText('generic.error'));
+			handleBeforeClose();
 			return { workflow: undefined, remote: true };
 		}
 	},
@@ -76,7 +82,9 @@ const local = useAsyncState<{ workflow?: IWorkflowDb; remote: boolean } | undefi
 			const { workflowId } = props.data;
 			const workflow = await workflowsStore.fetchWorkflow(workflowId);
 			return { workflow, remote: false };
-		} catch {
+		} catch (error) {
+			toast.showError(error, i18n.baseText('generic.error'));
+			handleBeforeClose();
 			return { workflow: undefined, remote: false };
 		}
 	},
@@ -84,12 +92,6 @@ const local = useAsyncState<{ workflow?: IWorkflowDb; remote: boolean } | undefi
 	manualAsyncConfiguration,
 );
 
-useAsyncState(async () => {
-	await Promise.all([nodeTypesStore.loadNodeTypesIfNotLoaded()]);
-	await Promise.all([remote.execute(), local.execute()]);
-	return true;
-}, false);
-
 const sourceWorkFlow = computed(() => (props.data.direction === 'push' ? remote : local));
 
 const targetWorkFlow = computed(() => (props.data.direction === 'push' ? local : remote));
@@ -301,6 +303,9 @@ const nodeDiffs = computed(() => {
 });
 
 function handleBeforeClose() {
+	if (isClosed.value) return;
+	isClosed.value = true;
+
 	selectedDetailId.value = undefined;
 
 	// Check if we have history to go back to avoid empty navigation issues
@@ -325,8 +330,11 @@ function handleEscapeKey(event: KeyboardEvent) {
 	}
 }
 
-onMounted(() => {
+onMounted(async () => {
 	document.addEventListener('keydown', handleEscapeKey, true);
+	await nodeTypesStore.loadNodeTypesIfNotLoaded();
+	void remote.execute();
+	void local.execute();
 });
 
 onUnmounted(() => {
@@ -348,7 +356,7 @@ const isSourceWorkflowNew = computed(() => {
 	return !sourceExists && targetExists;
 });
 
-onNodeClick((nodeId) => {
+onNodeClick((nodeId: string) => {
 	const node = nodesDiff.value.get(nodeId);
 	if (!node) {
 		return;