fix(core): Workflow diff preview errors out in push dialog for workflow that's newly created (#22861)

packages/@n8n/api-types/src/index.ts

@@ -68,6 +68,8 @@ export type {
 } from './schemas/project.schema';
 
 export {
+	isSourceControlledFileStatus,
+	type SourceControlledFileStatus,
 	type SourceControlledFile,
 	SOURCE_CONTROL_FILE_LOCATION,
 	SOURCE_CONTROL_FILE_STATUS,

packages/@n8n/api-types/src/schemas/source-controlled-file.schema.ts

@@ -24,6 +24,12 @@ const FileStatusSchema = z.enum([
 ]);
 export const SOURCE_CONTROL_FILE_STATUS = FileStatusSchema.Values;
 
+export type SourceControlledFileStatus = z.infer<typeof FileStatusSchema>;
+
+export function isSourceControlledFileStatus(value: unknown): value is SourceControlledFileStatus {
+	return FileStatusSchema.safeParse(value).success;
+}
+
 const FileLocationSchema = z.enum(['local', 'remote']);
 export const SOURCE_CONTROL_FILE_LOCATION = FileLocationSchema.Values;
 

packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.test.ts

@@ -85,6 +85,68 @@ describe('useWorkflowDiffRouting', () => {
 			});
 		});
 
+		it('should parse valid workflowStatus query parameter when diff is present', async () => {
+			mockRoute.value.query = {
+				diff: 'workflow-123',
+				direction: 'push',
+				workflowStatus: 'modified',
+			};
+
+			useWorkflowDiffRouting();
+			await nextTick();
+
+			expect(mockUiStore.openModalWithData).toHaveBeenCalledWith({
+				name: WORKFLOW_DIFF_MODAL_KEY,
+				data: {
+					eventBus: mockEventBus,
+					workflowId: 'workflow-123',
+					workflowStatus: 'modified',
+					direction: 'push',
+				},
+			});
+		});
+
+		it('should ignore workflowStatus when diff is not present', async () => {
+			mockRoute.value.query = {
+				direction: 'push',
+				workflowStatus: 'modified',
+			};
+
+			useWorkflowDiffRouting();
+			await nextTick();
+
+			expect(mockUiStore.openModalWithData).toHaveBeenCalledWith({
+				name: SOURCE_CONTROL_PUSH_MODAL_KEY,
+				data: { eventBus: mockEventBus },
+			});
+
+			expect(mockUiStore.openModalWithData).not.toHaveBeenCalledWith(
+				expect.objectContaining({
+					name: WORKFLOW_DIFF_MODAL_KEY,
+				}),
+			);
+		});
+
+		it('should ignore invalid workflowStatus values', async () => {
+			mockRoute.value.query = {
+				diff: 'workflow-123',
+				direction: 'push',
+				workflowStatus: 'invalid-status',
+			};
+
+			useWorkflowDiffRouting();
+			await nextTick();
+
+			expect(mockUiStore.openModalWithData).toHaveBeenCalledWith({
+				name: WORKFLOW_DIFF_MODAL_KEY,
+				data: {
+					eventBus: mockEventBus,
+					workflowId: 'workflow-123',
+					direction: 'push',
+				},
+			});
+		});
+
 		it('should ignore invalid diff query parameter types', async () => {
 			mockRoute.value.query = {
 				diff: ['array-value'],
@@ -173,6 +235,27 @@ describe('useWorkflowDiffRouting', () => {
 			});
 		});
 
+		it('should include workflowStatus in diff modal data when provided', async () => {
+			mockRoute.value.query = {
+				diff: 'workflow-456',
+				direction: 'pull',
+				workflowStatus: 'created',
+			};
+
+			useWorkflowDiffRouting();
+			await nextTick();
+
+			expect(mockUiStore.openModalWithData).toHaveBeenCalledWith({
+				name: WORKFLOW_DIFF_MODAL_KEY,
+				data: {
+					eventBus: mockEventBus,
+					workflowId: 'workflow-456',
+					workflowStatus: 'created',
+					direction: 'pull',
+				},
+			});
+		});
+
 		it('should not open diff modal when diff is present but direction is missing', async () => {
 			mockRoute.value.query = {
 				diff: 'workflow-456',

packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.ts

@@ -1,6 +1,7 @@
 import { watch } from 'vue';
 import { useRoute } from 'vue-router';
 import { createEventBus } from '@n8n/utils/event-bus';
+import { isSourceControlledFileStatus, type SourceControlledFileStatus } from '@n8n/api-types';
 import { useUIStore } from '@/app/stores/ui.store';
 import { WORKFLOW_DIFF_MODAL_KEY } from '@/app/constants';
 import {
@@ -48,6 +49,7 @@ export function useWorkflowDiffRouting() {
 
 	const handleDiffModal = (
 		diffWorkflowId: string | undefined,
+		diffWorkflowStatus: SourceControlledFileStatus | undefined,
 		direction: Direction | undefined,
 	) => {
 		const shouldOpen = diffWorkflowId && direction;
@@ -59,6 +61,7 @@ export function useWorkflowDiffRouting() {
 				data: {
 					eventBus: workflowDiffEventBus,
 					workflowId: diffWorkflowId,
+					workflowStatus: diffWorkflowStatus,
 					direction,
 				},
 			});
@@ -107,6 +110,10 @@ export function useWorkflowDiffRouting() {
 
 	const handleRouteChange = () => {
 		const diffWorkflowId = typeof route.query.diff === 'string' ? route.query.diff : undefined;
+		const diffWorkflowStatus =
+			diffWorkflowId && isSourceControlledFileStatus(route.query.workflowStatus)
+				? route.query.workflowStatus
+				: undefined;
 		const direction =
 			typeof route.query.direction === 'string' &&
 			(route.query.direction === 'push' || route.query.direction === 'pull')
@@ -118,7 +125,7 @@ export function useWorkflowDiffRouting() {
 				? route.query.sourceControl
 				: undefined;
 
-		handleDiffModal(diffWorkflowId, direction);
+		handleDiffModal(diffWorkflowId, diffWorkflowStatus, direction);
 		handleSourceControlModals(sourceControl, diffWorkflowId, direction);
 	};
 

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

@@ -22,16 +22,20 @@ const mockRoute = reactive({
 	name: '',
 	params: {},
 	fullPath: '',
+	query: {},
 });
 
+const mockRouterInstance = {
+	back: vi.fn(),
+	push: vi.fn(),
+	replace: vi.fn(),
+	go: vi.fn(),
+	currentRoute: { value: mockRoute },
+};
+
 vi.mock('vue-router', () => ({
 	useRoute: () => mockRoute,
-	useRouter: () => ({
-		back: vi.fn(),
-		push: vi.fn(),
-		replace: vi.fn(),
-		go: vi.fn(),
-	}),
+	useRouter: () => mockRouterInstance,
 	RouterLink: {
 		template: '<a><slot></slot></a>',
 		props: ['to', 'target'],
@@ -859,6 +863,59 @@ describe('SourceControlPushModal', () => {
 		});
 	});
 
+	describe('workflow diff button', () => {
+		beforeEach(() => {
+			settingsStore.settings.enterprise.workflowDiffs = true;
+			vi.clearAllMocks();
+		});
+
+		it('should set workflowStatus url param when diff button is clicked for created workflow', async () => {
+			const status: SourceControlledFile[] = [
+				{
+					id: 'workflow-2',
+					name: 'New workflow',
+					type: 'workflow',
+					status: 'created',
+					location: 'local',
+					conflict: false,
+					file: '/home/user/.n8n/git/workflows/workflow-2.json',
+					updatedAt: '2024-09-20T10:31:40.000Z',
+				},
+			];
+
+			sourceControlStore.getAggregatedStatus.mockResolvedValue(status);
+
+			const { getByTestId, getByText, getAllByTestId } = renderModal({
+				pinia,
+				props: {
+					data: {
+						eventBus,
+						status,
+					},
+				},
+			});
+
+			await waitFor(() => {
+				expect(getByText('Commit and push changes')).toBeInTheDocument();
+			});
+
+			await waitFor(() => {
+				expect(getAllByTestId('source-control-push-modal-file-checkbox')).toHaveLength(1);
+			});
+
+			const compareButton = getByTestId('source-control-workflow-diff-button');
+			await userEvent.click(compareButton);
+
+			expect(mockRouterInstance.push).toHaveBeenCalledWith({
+				query: expect.objectContaining({
+					diff: 'workflow-2',
+					workflowStatus: 'created',
+					direction: 'push',
+				}),
+			});
+		});
+	});
+
 	describe('Enter key behavior', () => {
 		it('should trigger commit and push when Enter is pressed and submit is enabled', async () => {
 			const status: SourceControlledFile[] = [

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

@@ -16,7 +16,7 @@ import type {
 } from '@/features/collaboration/projects/projects.types';
 import { ResourceType } from '@/features/collaboration/projects/projects.utils';
 import { getPushPriorityByStatus, getStatusText, getStatusTheme } from '../sourceControl.utils';
-import type { SourceControlledFile } from '@n8n/api-types';
+import type { SourceControlledFile, SourceControlledFileStatus } from '@n8n/api-types';
 import {
 	ROLE,
 	SOURCE_CONTROL_FILE_LOCATION,
@@ -134,8 +134,6 @@ const projectsForFilters = computed(() => {
 const concatenateWithAnd = (messages: string[]) =>
 	new Intl.ListFormat(i18n.locale, { style: 'long', type: 'conjunction' }).format(messages);
 
-type SourceControlledFileStatus = SourceControlledFile['status'];
-
 type SourceControlledFileWithProject = SourceControlledFile & { project?: ProjectListItem };
 
 type Changes = {
@@ -665,7 +663,7 @@ function castProject(project: ProjectListItem): WorkflowResource {
 	return resource;
 }
 
-function openDiffModal(id: string) {
+function openDiffModal(id: string, workflowStatus: SourceControlledFileStatus) {
 	telemetry.track('User clicks compare workflows', {
 		workflow_id: id,
 		context: 'source_control_push',
@@ -676,6 +674,7 @@ function openDiffModal(id: string) {
 		query: {
 			...route.query,
 			diff: id,
+			workflowStatus,
 			direction: 'push',
 		},
 	});
@@ -929,9 +928,10 @@ onMounted(async () => {
 														placement="top"
 													>
 														<N8nIconButton
+															data-test-id="source-control-workflow-diff-button"
 															icon="file-diff"
 															type="secondary"
-															@click="openDiffModal(file.id)"
+															@click="openDiffModal(file.id, file.status)"
 														/>
 													</N8nTooltip>
 												</template>

packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.test.ts

@@ -151,6 +151,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -167,6 +168,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -185,6 +187,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -201,6 +204,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -222,6 +226,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -238,6 +243,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -266,6 +272,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -280,6 +287,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -295,6 +303,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'pull',
 				},
 			},
@@ -305,6 +314,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -321,6 +331,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -350,6 +361,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -378,6 +390,7 @@ describe('WorkflowDiffModal', () => {
 				data: {
 					eventBus,
 					workflowId: 'test-workflow-id',
+					workflowStatus: 'modified',
 					direction: 'push',
 				},
 			},
@@ -413,6 +426,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'new-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'pull',
 					},
 				},
@@ -433,6 +447,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'missing-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -450,6 +465,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -466,6 +482,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'pull',
 					},
 				},
@@ -488,6 +505,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -509,6 +527,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -529,6 +548,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -548,6 +568,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -571,6 +592,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -590,6 +612,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -607,6 +630,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -623,6 +647,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -650,6 +675,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -676,6 +702,7 @@ describe('WorkflowDiffModal', () => {
 					data: {
 						eventBus,
 						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
 						direction: 'push',
 					},
 				},
@@ -691,4 +718,42 @@ describe('WorkflowDiffModal', () => {
 			});
 		});
 	});
+
+	describe('remote workflow loading', () => {
+		it('should not call getRemoteWorkflow when direction is push and workflowStatus is created', async () => {
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						workflowStatus: 'created',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(workflowsStore.fetchWorkflow).toHaveBeenCalledWith('test-workflow-id');
+			});
+
+			expect(sourceControlStore.getRemoteWorkflow).not.toHaveBeenCalled();
+		});
+
+		it('should call getRemoteWorkflow when direction is push and workflowStatus is not created', async () => {
+			renderModal({
+				props: {
+					data: {
+						eventBus,
+						workflowId: 'test-workflow-id',
+						workflowStatus: 'modified',
+						direction: 'push',
+					},
+				},
+			});
+
+			await vi.waitFor(() => {
+				expect(sourceControlStore.getRemoteWorkflow).toHaveBeenCalledWith('test-workflow-id');
+			});
+		});
+	});
 });

packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.vue

@@ -11,6 +11,7 @@ import SyncedWorkflowCanvas from '@/features/workflows/workflowDiff/SyncedWorkfl
 import { useProvideViewportSync } from '@/features/workflows/workflowDiff/useViewportSync';
 import { useWorkflowDiff } from '@/features/workflows/workflowDiff/useWorkflowDiff';
 import type { IWorkflowDb } from '@/Interface';
+import type { SourceControlledFileStatus } from '@n8n/api-types';
 import { useNodeTypesStore } from '@/app/stores/nodeTypes.store';
 import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
 import { useWorkflowsStore } from '@/app/stores/workflows.store';
@@ -36,7 +37,12 @@ import {
 	N8nText,
 } from '@n8n/design-system';
 const props = defineProps<{
-	data: { eventBus: EventBus; workflowId: string; direction: 'push' | 'pull' };
+	data: {
+		eventBus: EventBus;
+		workflowId: string;
+		direction: 'push' | 'pull';
+		workflowStatus?: SourceControlledFileStatus;
+	};
 }>();
 
 const { selectedDetailId, onNodeClick, syncIsEnabled } = useProvideViewportSync();
@@ -62,6 +68,11 @@ const isClosed = ref(false);
 
 const remote = useAsyncState<{ workflow?: IWorkflowDb; remote: boolean } | undefined, [], false>(
 	async () => {
+		if (props.data.direction === 'push' && props.data.workflowStatus === 'created') {
+			// a new workflow that has yet to be pushed cannot be on remote.
+			return { workflow: undefined, remote: true };
+		}
+
 		try {
 			const { workflowId } = props.data;
 			const { content: workflow } = await sourceControlStore.getRemoteWorkflow(workflowId);