From 4bbf7cb749192420ee7014f3f51fd0e12f6f3b24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20G=C3=B3mez=20Morales?= Date: Thu, 28 Aug 2025 09:39:32 +0200 Subject: [PATCH] fix(editor): Prevent execution data from leaking into workflow diffs UI (#18605) Co-authored-by: Csaba Tuncsik --- .../workflow-diff/WorkflowDiffModal.test.ts | 46 +------ .../workflow-diff/WorkflowDiffModal.vue | 5 +- .../features/workflow-diff/useWorkflowDiff.ts | 8 +- .../editor-ui/src/utils/workflowUtils.test.ts | 124 ++++++++++++++++++ .../editor-ui/src/utils/workflowUtils.ts | 27 ++++ 5 files changed, 166 insertions(+), 44 deletions(-) create mode 100644 packages/frontend/editor-ui/src/utils/workflowUtils.test.ts create mode 100644 packages/frontend/editor-ui/src/utils/workflowUtils.ts diff --git a/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.test.ts b/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.test.ts index b635a48a66..81958e8815 100644 --- a/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.test.ts +++ b/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.test.ts @@ -27,9 +27,15 @@ vi.mock('vue-router', () => ({ vi.mock('@/features/workflow-diff/useViewportSync', () => ({ useProvideViewportSync: () => ({ - selectedDetailId: vi.fn(), + selectedDetailId: ref(undefined), onNodeClick: vi.fn(), }), + useInjectViewportSync: () => ({ + triggerViewportChange: vi.fn(), + onViewportChange: vi.fn(), + selectedDetailId: ref(undefined), + triggerNodeClick: vi.fn(), + }), })); vi.mock('@/features/workflow-diff/useWorkflowDiff', () => ({ @@ -89,27 +95,6 @@ const renderModal = createComponentRenderer(WorkflowDiffModal, { `, }, - SyncedWorkflowCanvas: { - template: '
', - }, - WorkflowDiffAside: { - template: '
', - }, - Node: { - template: '
', - }, - HighlightedEdge: { - template: '
', - }, - NodeDiff: { - template: '
', - }, - DiffBadge: { - template: '', - }, - NodeIcon: { - template: '', - }, }, }, }); @@ -137,7 +122,6 @@ describe('WorkflowDiffModal', () => { it('should mount successfully', async () => { const { container } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -154,7 +138,6 @@ describe('WorkflowDiffModal', () => { it('should initialize with correct props', () => { const { container } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -173,7 +156,6 @@ describe('WorkflowDiffModal', () => { it('should display changes button', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -190,7 +172,6 @@ describe('WorkflowDiffModal', () => { it('should open changes dropdown when clicking Changes button', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -212,7 +193,6 @@ describe('WorkflowDiffModal', () => { it('should render workflow panels', () => { const { container } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -229,7 +209,6 @@ describe('WorkflowDiffModal', () => { it('should render navigation buttons', () => { const { container } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -258,7 +237,6 @@ describe('WorkflowDiffModal', () => { workflowsStore.fetchWorkflow.mockResolvedValue(localWorkflow); const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -273,7 +251,6 @@ describe('WorkflowDiffModal', () => { it('should render back button', () => { const { container } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -289,7 +266,6 @@ describe('WorkflowDiffModal', () => { it('should handle different workflow directions', () => { const pullComponent = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -300,7 +276,6 @@ describe('WorkflowDiffModal', () => { }); const pushComponent = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -317,7 +292,6 @@ describe('WorkflowDiffModal', () => { it('should show empty state when no changes exist in tabs', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -347,7 +321,6 @@ describe('WorkflowDiffModal', () => { it('should show empty state for connectors tab when no connector changes', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -376,7 +349,6 @@ describe('WorkflowDiffModal', () => { it('should show empty state for settings tab when no settings changes', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -412,7 +384,6 @@ describe('WorkflowDiffModal', () => { workflowsStore.fetchWorkflow.mockRejectedValue(new Error('Workflow not found')); const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -433,7 +404,6 @@ describe('WorkflowDiffModal', () => { workflowsStore.fetchWorkflow.mockResolvedValue(mockWorkflow); const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -451,7 +421,6 @@ describe('WorkflowDiffModal', () => { it('should handle push direction without crashing', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, @@ -468,7 +437,6 @@ describe('WorkflowDiffModal', () => { it('should handle pull direction without crashing', async () => { const { getByText } = renderModal({ - pinia: createTestingPinia(), props: { data: { eventBus, diff --git a/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.vue b/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.vue index e4b2f96556..167eaabe2c 100644 --- a/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.vue +++ b/packages/frontend/editor-ui/src/features/workflow-diff/WorkflowDiffModal.vue @@ -13,6 +13,7 @@ import type { IWorkflowDb } from '@/Interface'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useSourceControlStore } from '@/stores/sourceControl.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; +import { removeWorkflowExecutionData } from '@/utils/workflowUtils'; import { N8nButton, N8nHeading, N8nIconButton, N8nRadioButtons, N8nText } from '@n8n/design-system'; import type { BaseTextKey } from '@n8n/i18n'; import { useI18n } from '@n8n/i18n'; @@ -86,8 +87,8 @@ const sourceWorkFlow = computed(() => (props.data.direction === 'push' ? remote const targetWorkFlow = computed(() => (props.data.direction === 'push' ? local : remote)); const { source, target, nodesDiff, connectionsDiff } = useWorkflowDiff( - computed(() => sourceWorkFlow.value.state.value?.workflow), - computed(() => targetWorkFlow.value.state.value?.workflow), + computed(() => removeWorkflowExecutionData(sourceWorkFlow.value.state.value?.workflow)), + computed(() => removeWorkflowExecutionData(targetWorkFlow.value.state.value?.workflow)), ); type SettingsChange = { diff --git a/packages/frontend/editor-ui/src/features/workflow-diff/useWorkflowDiff.ts b/packages/frontend/editor-ui/src/features/workflow-diff/useWorkflowDiff.ts index 2250af4816..09f8fdb80d 100644 --- a/packages/frontend/editor-ui/src/features/workflow-diff/useWorkflowDiff.ts +++ b/packages/frontend/editor-ui/src/features/workflow-diff/useWorkflowDiff.ts @@ -1,6 +1,6 @@ import _pick from 'lodash-es/pick'; import _isEqual from 'lodash-es/isEqual'; -import type { CanvasConnection } from '@/types'; +import type { CanvasConnection, CanvasNode } from '@/types'; import type { INodeUi, IWorkflowDb } from '@/Interface'; import type { MaybeRefOrGetter, Ref, ComputedRef } from 'vue'; import { useWorkflowsStore } from '@/stores/workflows.store'; @@ -134,15 +134,17 @@ function createWorkflowDiff( return { workflow: workflowRef, - nodes: nodes.value.map((node) => { + nodes: nodes.value.map((node: CanvasNode) => { node.draggable = false; node.selectable = false; node.focusable = false; return node; }), - connections: connections.value.map((connection) => { + connections: connections.value.map((connection: CanvasConnection) => { connection.selectable = false; connection.focusable = false; + // Remove execution data from connection labels in diff context + connection.label = ''; return connection; }), }; diff --git a/packages/frontend/editor-ui/src/utils/workflowUtils.test.ts b/packages/frontend/editor-ui/src/utils/workflowUtils.test.ts new file mode 100644 index 0000000000..387fc47ef1 --- /dev/null +++ b/packages/frontend/editor-ui/src/utils/workflowUtils.test.ts @@ -0,0 +1,124 @@ +import { removeWorkflowExecutionData } from './workflowUtils'; +import type { IWorkflowDb } from '@/Interface'; +import type { INodeIssues } from 'n8n-workflow'; + +describe('workflowUtils', () => { + describe('removeWorkflowExecutionData', () => { + it('should return undefined if workflow is undefined', () => { + expect(removeWorkflowExecutionData(undefined)).toBeUndefined(); + }); + + it('should remove execution-related data from nodes and workflow-level pinData', () => { + const mockWorkflow: IWorkflowDb = { + id: 'test-workflow', + name: 'Test Workflow', + active: false, + isArchived: false, + createdAt: '2023-01-01T00:00:00Z', + updatedAt: '2023-01-01T00:00:00Z', + nodes: [ + { + id: 'node1', + name: 'Test Node', + type: 'test-type', + typeVersion: 1, + position: [100, 100], + parameters: {}, + // Execution-related data that should be removed + issues: {} as INodeIssues, + pinData: { someData: 'test' }, + }, + { + id: 'node2', + name: 'Clean Node', + type: 'another-type', + typeVersion: 1, + position: [200, 200], + parameters: {}, + // No execution data + }, + ], + connections: {}, + // Workflow-level execution data that should be removed + pinData: { node1: [{ json: { data: 'execution-result' } }] }, + versionId: '1.0', + }; + + const result = removeWorkflowExecutionData(mockWorkflow); + + expect(result).toBeDefined(); + expect(result!.nodes).toHaveLength(2); + + // First node should have execution data removed + expect(result!.nodes[0]).toEqual({ + id: 'node1', + name: 'Test Node', + type: 'test-type', + typeVersion: 1, + position: [100, 100], + parameters: {}, + }); + + // Second node should remain unchanged (no execution data to remove) + expect(result!.nodes[1]).toEqual({ + id: 'node2', + name: 'Clean Node', + type: 'another-type', + typeVersion: 1, + position: [200, 200], + parameters: {}, + }); + + // Workflow-level pinData should be removed + expect(result!.pinData).toBeUndefined(); + + // Workflow metadata should be preserved + expect(result!.id).toBe('test-workflow'); + expect(result!.name).toBe('Test Workflow'); + expect(result!.connections).toEqual({}); + }); + + it('should preserve all other node properties', () => { + const mockWorkflow: IWorkflowDb = { + id: 'test-workflow', + name: 'Test Workflow', + active: false, + isArchived: false, + createdAt: '2023-01-01T00:00:00Z', + updatedAt: '2023-01-01T00:00:00Z', + nodes: [ + { + id: 'node1', + name: 'Complex Node', + type: 'complex-type', + typeVersion: 2, + position: [150, 250], + parameters: { param1: 'value1', param2: { nested: true } }, + color: '#ff0000', + notes: 'Some notes', + disabled: true, + // Execution data to be removed + issues: {} as INodeIssues, + pinData: { result: [{ json: { test: 'data' } }] }, + }, + ], + connections: {}, + versionId: '2.0', + }; + + const result = removeWorkflowExecutionData(mockWorkflow); + + expect(result!.nodes[0]).toEqual({ + id: 'node1', + name: 'Complex Node', + type: 'complex-type', + typeVersion: 2, + position: [150, 250], + parameters: { param1: 'value1', param2: { nested: true } }, + color: '#ff0000', + notes: 'Some notes', + disabled: true, + }); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/utils/workflowUtils.ts b/packages/frontend/editor-ui/src/utils/workflowUtils.ts new file mode 100644 index 0000000000..c16ed14654 --- /dev/null +++ b/packages/frontend/editor-ui/src/utils/workflowUtils.ts @@ -0,0 +1,27 @@ +import type { IWorkflowDb, INodeUi } from '@/Interface'; + +/** + * Removes execution data from workflow nodes and workflow-level execution data + * to ensure clean comparisons in diffs. This prevents execution status, run data, + * pinned data, and other runtime information from appearing in workflow difference + * comparisons. + */ +export function removeWorkflowExecutionData( + workflow: IWorkflowDb | undefined, +): IWorkflowDb | undefined { + if (!workflow) return workflow; + + // Remove workflow-level execution data and clean up nodes + const { pinData, ...cleanWorkflow } = workflow; + + const sanitizedWorkflow: IWorkflowDb = { + ...cleanWorkflow, + nodes: workflow.nodes.map((node: INodeUi) => { + // Create a clean copy without execution-related data + const { issues, pinData, ...cleanNode } = node; + return cleanNode; + }), + }; + + return sanitizedWorkflow; +}