diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index ffd29775a75ae..6a7b0c46aab38 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/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, diff --git a/packages/@n8n/api-types/src/schemas/source-controlled-file.schema.ts b/packages/@n8n/api-types/src/schemas/source-controlled-file.schema.ts index a3c6c94fe9943..0ac2e39125635 100644 --- a/packages/@n8n/api-types/src/schemas/source-controlled-file.schema.ts +++ b/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; + +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; diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.test.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.test.ts index 38a9d64cb6e97..0bf90b24eab14 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.test.ts +++ b/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', diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.ts index e9c0c6c6a7adb..f046482f7fc93 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowDiffRouting.ts +++ b/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); }; diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts index c62c19dcabd86..9a6abf4083ffe 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.test.ts +++ b/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: '', 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[] = [ diff --git a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue b/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue index 02409b98ffa1d..7d72d1b71d817 100644 --- a/packages/frontend/editor-ui/src/features/integrations/sourceControl.ee/components/SourceControlPushModal.vue +++ b/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" > diff --git a/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.test.ts b/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.test.ts index 8b6d07c8288d2..a28598f2bdde5 100644 --- a/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.test.ts +++ b/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'); + }); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.vue b/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.vue index 124185cea59fe..0f85a414b5a19 100644 --- a/packages/frontend/editor-ui/src/features/workflows/workflowDiff/WorkflowDiffModal.vue +++ b/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);