From a2fba4a6f3d2d41a8c987a5c3b0b869a48053436 Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 4 Feb 2025 12:37:17 +0100 Subject: [PATCH 01/52] feat(editor): indicate dirty nodes with yellow borders/connectors on canvas --- .../canvas/elements/edges/CanvasEdge.vue | 2 + .../render-types/CanvasHandleMainOutput.vue | 13 +++++- .../render-types/parts/CanvasHandlePlus.vue | 8 +++- .../nodes/render-types/CanvasNodeDefault.vue | 5 +++ .../parts/CanvasNodeStatusIcons.vue | 18 ++++++++ .../src/composables/useCanvasMapping.ts | 41 +++++++++++++------ .../editor-ui/src/stores/workflows.store.ts | 41 +++++++++++++++++++ packages/editor-ui/src/types/canvas.ts | 3 +- 8 files changed, 116 insertions(+), 15 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue index 6072bdd82a538..efc11a886f02d 100644 --- a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue +++ b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue @@ -67,6 +67,8 @@ const edgeColor = computed(() => { return 'var(--node-type-supplemental-color)'; } else if (props.selected) { return 'var(--color-background-dark)'; + } else if (status.value === 'warning') { + return 'var(--color-warning)'; } else { return 'var(--color-foreground-xdark)'; } diff --git a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue index 89c83b4887c54..3792c875700f4 100644 --- a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue +++ b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue @@ -41,7 +41,13 @@ const runDataLabel = computed(() => const isHandlePlusVisible = computed(() => !isConnecting.value || isHovered.value); -const plusType = computed(() => (runDataTotal.value > 0 ? 'success' : 'default')); +const plusType = computed(() => + renderOptions.value.staleness !== undefined + ? 'warning' + : runDataTotal.value > 0 + ? 'success' + : 'default', +); const plusLineSize = computed( () => @@ -60,6 +66,7 @@ const outputLabelClasses = computed(() => ({ const runDataLabelClasses = computed(() => ({ [$style.label]: true, [$style.runDataLabel]: true, + [$style.stale]: renderOptions.value.staleness !== undefined, })); function onMouseEnter() { @@ -137,6 +144,10 @@ function onClickAdd() { transform: translate(-50%, -150%); font-size: var(--font-size-xs); color: var(--color-success); + + &.stale { + color: var(--color-warning); + } } diff --git a/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue b/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue index 880a9a04017b2..c18f41e3cbe49 100644 --- a/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue +++ b/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue @@ -7,7 +7,7 @@ const props = withDefaults( handleClasses?: string; plusSize?: number; lineSize?: number; - type?: 'success' | 'secondary' | 'default'; + type?: 'success' | 'warning' | 'secondary' | 'default'; }>(), { position: 'right', @@ -163,6 +163,12 @@ function onClick(event: MouseEvent) { } } + &.warning { + .line { + stroke: var(--color-warning); + } + } + .plus { &:hover { cursor: pointer; diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue index 2481831249e9c..e15a41bb0ac3f 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue @@ -59,6 +59,7 @@ const classes = computed(() => { [$style.configurable]: renderOptions.value.configurable, [$style.configuration]: renderOptions.value.configuration, [$style.trigger]: renderOptions.value.trigger, + [$style.stale]: renderOptions.value.staleness !== undefined, }; }); @@ -268,6 +269,10 @@ function openContextMenu(event: MouseEvent) { &.waiting { border-color: var(--color-canvas-node-waiting-border-color, var(--color-secondary)); } + + &.stale { + border-color: var(--color-warning); + } } .description { diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index aa80598baa06b..6d5d4e425fd9d 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -4,6 +4,7 @@ import TitledList from '@/components/TitledList.vue'; import { useNodeHelpers } from '@/composables/useNodeHelpers'; import { useCanvasNode } from '@/composables/useCanvasNode'; import { useI18n } from '@/composables/useI18n'; +import { CanvasNodeRenderType } from '@/types'; const nodeHelpers = useNodeHelpers(); const i18n = useI18n(); @@ -18,9 +19,15 @@ const { hasRunData, runDataIterations, isDisabled, + render, } = useCanvasNode(); const hideNodeIssues = computed(() => false); // @TODO Implement this +const isStale = computed( + () => + render.value.type === CanvasNodeRenderType.Default && + render.value.options.staleness === 'stale', +); From 1c893d4024ce3b436cf7f4172318bd8972bc0176 Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 7 Feb 2025 13:33:41 +0100 Subject: [PATCH 12/52] fix: if new partial execution is not enabled, nothing should change --- packages/editor-ui/src/components/OutputPanel.vue | 14 +++++++++++--- packages/editor-ui/src/stores/workflows.store.ts | 10 ++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index 71211821f3d76..ac13a67c950a9 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -197,9 +197,17 @@ const runsCount = computed(() => { return 0; }); -const staleData = computed( - () => node.value && workflowsStore.dirtinessByName[node.value.name] === 'dirty', -); +const staleData = computed(() => { + if (!node.value) { + return false; + } + const updatedAt = workflowsStore.getParametersLastUpdate(node.value.name); + if (!updatedAt || !runTaskData.value) { + return false; + } + const runAt = runTaskData.value.startTime; + return updatedAt > runAt; +}); const outputPanelEditMode = computed(() => { return ndvStore.outputPanelEditMode; diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 5cded69daf544..6f848342d677d 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -275,6 +275,11 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { const getPastChatMessages = computed(() => Array.from(new Set(chatMessages.value))); const dirtinessByName = computed(() => { + // Do not highlight dirtiness if new partial execution is not enabled + if (settingsStore.partialExecutionVersion === 1) { + return {}; + } + const dirtiness: Record = {}; const visitedByName: Record = {}; @@ -305,10 +310,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { if (lastUpdate && runAt && lastUpdate > runAt) { dirtiness[nodeName] = 'dirty'; - // If new partial execution is enabled, dirty node - if (settingsStore.partialExecutionVersion === 2) { - markDownstreamStaleRecursively(nodeName); - } + markDownstreamStaleRecursively(nodeName); } } From 73137585f0a664e45fb098376a33a369a8868d74 Mon Sep 17 00:00:00 2001 From: autologie Date: Mon, 10 Feb 2025 09:45:03 +0100 Subject: [PATCH 13/52] fix: nodes without runData should remain gray --- packages/editor-ui/src/stores/workflows.store.test.ts | 2 -- packages/editor-ui/src/stores/workflows.store.ts | 11 +++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.test.ts b/packages/editor-ui/src/stores/workflows.store.test.ts index 6c2ccb4c8993b..07e52ca209733 100644 --- a/packages/editor-ui/src/stores/workflows.store.test.ts +++ b/packages/editor-ui/src/stores/workflows.store.test.ts @@ -781,7 +781,6 @@ describe('useWorkflowsStore', () => { expect(workflowsStore.dirtinessByName).toEqual({ node1: 'dirty', node2: 'upstream-dirty', - node3: 'upstream-dirty', }); }); @@ -797,7 +796,6 @@ describe('useWorkflowsStore', () => { expect(workflowsStore.dirtinessByName).toEqual({ node1: 'dirty', node2: 'upstream-dirty', - node3: 'upstream-dirty', }); }); diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 6f848342d677d..ad2a9cdf4e79b 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -282,6 +282,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { const dirtiness: Record = {}; const visitedByName: Record = {}; + const runDataByNode = workflowExecutionData.value?.data?.resultData.runData ?? {}; function markDownstreamStaleRecursively(nodeName: string): void { if (visitedByName[nodeName]) { @@ -293,7 +294,11 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { for (const inputConnections of Object.values(outgoingConnectionsByNodeName(nodeName))) { for (const connections of inputConnections) { for (const { node } of connections ?? []) { - dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; + const hasRunData = (runDataByNode[node] ?? []).length > 0; + + if (hasRunData) { + dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; + } markDownstreamStaleRecursively(node); } @@ -301,9 +306,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } } - for (const [nodeName, taskData] of Object.entries( - workflowExecutionData.value?.data?.resultData.runData ?? {}, - )) { + for (const [nodeName, taskData] of Object.entries(runDataByNode)) { const lastUpdate = getParametersLastUpdate(nodeName); const runAt = taskData[0]?.startTime; From 4bac7e1840b8968eb8a63a52c5b0f397a15aa66d Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 11 Feb 2025 08:48:02 +0100 Subject: [PATCH 14/52] refactor: use Set for tracking visited nodes --- packages/editor-ui/src/stores/workflows.store.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index ad2a9cdf4e79b..e3ecac9b762ea 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -281,15 +281,15 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } const dirtiness: Record = {}; - const visitedByName: Record = {}; + const visitedNodes: Set = new Set(); const runDataByNode = workflowExecutionData.value?.data?.resultData.runData ?? {}; function markDownstreamStaleRecursively(nodeName: string): void { - if (visitedByName[nodeName]) { + if (visitedNodes.has(nodeName)) { return; // prevent infinite recursion } - visitedByName[nodeName] = true; + visitedNodes.add(nodeName); for (const inputConnections of Object.values(outgoingConnectionsByNodeName(nodeName))) { for (const connections of inputConnections) { From 3407147743d1d630f7cd561af2a57c9cbf8f4caa Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 11 Feb 2025 09:14:39 +0100 Subject: [PATCH 15/52] fix: node injection/removal and toggling enabled/disabled should change downstream dirtiness --- packages/editor-ui/src/Interface.ts | 6 +-- packages/editor-ui/src/components/RunData.vue | 6 +-- .../src/composables/useNodeHelpers.ts | 7 ++- .../src/composables/useRunWorkflow.test.ts | 1 + .../src/composables/useRunWorkflow.ts | 27 +++--------- .../editor-ui/src/stores/workflows.store.ts | 44 ++++++++++++++++--- packages/editor-ui/src/types/canvas.ts | 2 +- 7 files changed, 54 insertions(+), 39 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index fc4ebbce15e80..08fa0b24c06fa 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -139,10 +139,7 @@ export interface IUpdateInformation>; } export type XYPosition = [number, number]; @@ -849,6 +846,7 @@ export interface ITemplatesNode extends IVersionNode { export interface INodeMetadata { parametersLastUpdatedAt?: number; + incomingConnectionsLastUpdatedAt?: number; pristine: boolean; } diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index bf7175e68defb..ab97a0fb61fd5 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -1254,12 +1254,12 @@ function onRunIndexChange(run: number) { function enableNode() { if (node.value) { - const updateInformation = { + const updateInformation: INodeUpdatePropertiesInformation = { name: node.value.name, properties: { disabled: !node.value.disabled, - } as IDataObject, - } as INodeUpdatePropertiesInformation; + }, + }; workflowsStore.updateNodeProperties(updateInformation); } diff --git a/packages/editor-ui/src/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index 461f550212eba..5d5947ae442a1 100644 --- a/packages/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/editor-ui/src/composables/useNodeHelpers.ts @@ -25,7 +25,6 @@ import type { ITaskDataConnections, IRunData, IBinaryKeyData, - IDataObject, INode, INodePropertyOptions, INodeCredentialsDetails, @@ -676,12 +675,12 @@ export function useNodeHelpers() { } // Toggle disabled flag - const updateInformation = { + const updateInformation: INodeUpdatePropertiesInformation = { name: node.name, properties: { disabled: newDisabledState, - } as IDataObject, - } as INodeUpdatePropertiesInformation; + }, + }; telemetry.track('User set node enabled status', { node_type: node.type, diff --git a/packages/editor-ui/src/composables/useRunWorkflow.test.ts b/packages/editor-ui/src/composables/useRunWorkflow.test.ts index 6b8505c5fe284..83858b6cd7477 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.test.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.test.ts @@ -345,6 +345,7 @@ describe('useRunWorkflow({ router })', () => { }, ], }; + vi.mocked(workflowsStore).dirtinessByName = { [executeName]: 'dirty' }; vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue({ name: 'Test Workflow', getParentNodes: () => [parentName], diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index cdcec5f5a432d..3ffc77d8a0c13 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -38,25 +38,6 @@ import { useExecutionsStore } from '@/stores/executions.store'; import { useSettingsStore } from '@/stores/settings.store'; import { usePushConnectionStore } from '@/stores/pushConnection.store'; -const getDirtyNodeNames = ( - runData: IRunData, - getParametersLastUpdate: (nodeName: string) => number | undefined, -): string[] | undefined => { - const dirtyNodeNames = Object.entries(runData).reduce((acc, [nodeName, tasks]) => { - if (!tasks.length) return acc; - - const updatedAt = getParametersLastUpdate(nodeName) ?? 0; - - if (updatedAt > tasks[0].startTime) { - acc.push(nodeName); - } - - return acc; - }, []); - - return dirtyNodeNames.length ? dirtyNodeNames : undefined; -}; - export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType }) { const nodeHelpers = useNodeHelpers(); const workflowHelpers = useWorkflowHelpers({ router: useRunWorkflowOpts.router }); @@ -288,10 +269,12 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType + dirtiness === 'incoming-connections-changed' || dirtiness === 'dirty' ? [nodeName] : [], ); + + startRunData.dirtyNodeNames = nodeNames.length > 0 ? nodeNames : undefined; } // Init the execution data to represent the start of the execution diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index e3ecac9b762ea..a49746f78cd91 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -307,12 +307,24 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } for (const [nodeName, taskData] of Object.entries(runDataByNode)) { - const lastUpdate = getParametersLastUpdate(nodeName); const runAt = taskData[0]?.startTime; - if (lastUpdate && runAt && lastUpdate > runAt) { + if (!runAt) { + continue; + } + + const parametersLastUpdate = getParametersLastUpdate(nodeName); + + if (parametersLastUpdate && parametersLastUpdate > runAt) { dirtiness[nodeName] = 'dirty'; + markDownstreamStaleRecursively(nodeName); + continue; + } + + const incomingConnectionsLastUpdate = getIncomingConnectionsLastUpdate(nodeName); + if (incomingConnectionsLastUpdate && incomingConnectionsLastUpdate > runAt) { + dirtiness[nodeName] = 'incoming-connections-changed'; markDownstreamStaleRecursively(nodeName); } } @@ -375,6 +387,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return nodeMetadata.value[nodeName]?.parametersLastUpdatedAt; } + function getIncomingConnectionsLastUpdate(nodeName: string): number | undefined { + return nodeMetadata.value[nodeName]?.incomingConnectionsLastUpdatedAt; + } + function isNodePristine(nodeName: string): boolean { return nodeMetadata.value[nodeName] === undefined || nodeMetadata.value[nodeName].pristine; } @@ -995,6 +1011,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { connections.splice(parseInt(index, 10), 1); } } + + nodeMetadata.value[destinationData.node].incomingConnectionsLastUpdatedAt = Date.now(); } function removeAllConnections(data: { setStateDirty: boolean }): void { @@ -1222,9 +1240,24 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { for (const key of Object.keys(updateInformation.properties)) { uiStore.stateIsDirty = true; - updateNodeAtIndex(nodeIndex, { - [key]: updateInformation.properties[key], - }); + const typedKey = key as keyof INodeUpdatePropertiesInformation['properties']; + const property = updateInformation.properties[typedKey]; + + updateNodeAtIndex(nodeIndex, { [key]: property }); + } + + if (updateInformation.properties.disabled !== undefined) { + const outgoingConnections = Object.values( + outgoingConnectionsByNodeName(updateInformation.name), + ); + + for (const nodeConnections of outgoingConnections) { + for (const connections of nodeConnections) { + for (const { node } of connections ?? []) { + nodeMetadata.value[node].incomingConnectionsLastUpdatedAt = Date.now(); + } + } + } } } } @@ -1711,6 +1744,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { getNodeById, getNodesByIds, getParametersLastUpdate, + getIncomingConnectionsLastUpdate, isNodePristine, isNodeExecuting, getExecutionDataById, diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts index 8cba5d92a3cfb..77ff691a38f28 100644 --- a/packages/editor-ui/src/types/canvas.ts +++ b/packages/editor-ui/src/types/canvas.ts @@ -47,7 +47,7 @@ export const enum CanvasNodeRenderType { export type CanvasNodeDefaultRenderLabelSize = 'small' | 'medium' | 'large'; -export type CanvasNodeDirtiness = 'dirty' | 'upstream-dirty'; +export type CanvasNodeDirtiness = 'dirty' | 'upstream-dirty' | 'incoming-connections-changed'; export type CanvasNodeDefaultRender = { type: CanvasNodeRenderType.Default; From 466baee170ddbfeae7073b45f9f5e04ed60f81d1 Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 11 Feb 2025 16:25:34 +0100 Subject: [PATCH 16/52] refactor: use history as the place to store data for calculating dirtiness --- packages/editor-ui/src/Interface.ts | 1 - .../editor-ui/src/components/NodeSettings.vue | 2 +- .../src/composables/useCanvasMapping.ts | 8 +- .../src/composables/useCanvasOperations.ts | 40 ++++--- .../src/composables/useHistoryHelper.ts | 14 ++- .../src/composables/useNodeHelpers.ts | 13 ++- .../src/composables/useRunWorkflow.ts | 101 +++++++++++++++++- packages/editor-ui/src/models/history.ts | 80 ++++++++------ packages/editor-ui/src/stores/canvas.store.ts | 2 +- .../editor-ui/src/stores/workflows.store.ts | 81 -------------- packages/editor-ui/src/views/NodeView.vue | 23 ++-- 11 files changed, 213 insertions(+), 152 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 08fa0b24c06fa..2868a458b0e3c 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -846,7 +846,6 @@ export interface ITemplatesNode extends IVersionNode { export interface INodeMetadata { parametersLastUpdatedAt?: number; - incomingConnectionsLastUpdatedAt?: number; pristine: boolean; } diff --git a/packages/editor-ui/src/components/NodeSettings.vue b/packages/editor-ui/src/components/NodeSettings.vue index 49cb19ca71ae2..43ee007edb37a 100644 --- a/packages/editor-ui/src/components/NodeSettings.vue +++ b/packages/editor-ui/src/components/NodeSettings.vue @@ -765,7 +765,7 @@ const credentialSelected = (updateInformation: INodeUpdatePropertiesInformation) const nameChanged = (name: string) => { if (node.value) { - historyStore.pushCommandToUndo(new RenameNodeCommand(node.value.name, name)); + historyStore.pushCommandToUndo(new RenameNodeCommand(node.value.name, name, Date.now())); } valueChanged({ value: name, diff --git a/packages/editor-ui/src/composables/useCanvasMapping.ts b/packages/editor-ui/src/composables/useCanvasMapping.ts index c3ccd70d9d924..7af9183f68fd6 100644 --- a/packages/editor-ui/src/composables/useCanvasMapping.ts +++ b/packages/editor-ui/src/composables/useCanvasMapping.ts @@ -49,6 +49,8 @@ import { sanitizeHtml } from '@/utils/htmlUtils'; import { MarkerType } from '@vue-flow/core'; import { useNodeHelpers } from './useNodeHelpers'; import { getTriggerNodeServiceName } from '@/utils/nodeTypesUtils'; +import { useRunWorkflow } from './useRunWorkflow'; +import { useRouter } from 'vue-router'; export function useCanvasMapping({ nodes, @@ -63,6 +65,8 @@ export function useCanvasMapping({ const workflowsStore = useWorkflowsStore(); const nodeTypesStore = useNodeTypesStore(); const nodeHelpers = useNodeHelpers(); + const router = useRouter(); + const { dirtinessByName } = useRunWorkflow({ router }); function createStickyNoteRenderType(node: INodeUi): CanvasNodeStickyNoteRender { return { @@ -97,7 +101,7 @@ export function useCanvasMapping({ labelSize: nodeOutputLabelSizeById.value[node.id], }, tooltip: nodeTooltipById.value[node.id], - dirtiness: workflowsStore.dirtinessByName[node.name], + dirtiness: dirtinessByName.value[node.name], }, }; } @@ -594,7 +598,7 @@ export function useCanvasMapping({ const sourceNodeName = connection.data?.source.node; - if (sourceNodeName && workflowsStore.dirtinessByName[sourceNodeName] !== undefined) { + if (sourceNodeName && dirtinessByName.value[sourceNodeName] !== undefined) { return 'warning'; } diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index e47c93baf453b..40c3fcbe0244a 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -193,7 +193,9 @@ export function useCanvasOperations({ router }: { router: ReturnType= 0; i--) { await commands[i].revert(); - reverseCommands.push(commands[i].getReverseCommand()); + reverseCommands.push(commands[i].getReverseCommand(timestamp)); } historyStore.pushUndoableToRedo(new BulkCommand(reverseCommands)); await nextTick(); @@ -46,7 +49,7 @@ export function useHistoryHelper(activeRoute: RouteLocationNormalizedLoaded) { } if (command instanceof Command) { await command.revert(); - historyStore.pushUndoableToRedo(command.getReverseCommand()); + historyStore.pushUndoableToRedo(command.getReverseCommand(timestamp)); uiStore.stateIsDirty = true; } trackCommand(command, 'undo'); @@ -61,13 +64,16 @@ export function useHistoryHelper(activeRoute: RouteLocationNormalizedLoaded) { if (!command) { return; } + + const timestamp = Date.now(); + if (command instanceof BulkCommand) { historyStore.bulkInProgress = true; const commands = command.commands; const reverseCommands = []; for (let i = commands.length - 1; i >= 0; i--) { await commands[i].revert(); - reverseCommands.push(commands[i].getReverseCommand()); + reverseCommands.push(commands[i].getReverseCommand(timestamp)); } historyStore.pushBulkCommandToUndo(new BulkCommand(reverseCommands), false); await nextTick(); @@ -75,7 +81,7 @@ export function useHistoryHelper(activeRoute: RouteLocationNormalizedLoaded) { } if (command instanceof Command) { await command.revert(); - historyStore.pushCommandToUndo(command.getReverseCommand(), false); + historyStore.pushCommandToUndo(command.getReverseCommand(timestamp), false); uiStore.stateIsDirty = true; } trackCommand(command, 'redo'); diff --git a/packages/editor-ui/src/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index 5d5947ae442a1..26fcc61662572 100644 --- a/packages/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/editor-ui/src/composables/useNodeHelpers.ts @@ -695,7 +695,12 @@ export function useNodeHelpers() { updateNodesInputIssues(); if (trackHistory) { historyStore.pushCommandToUndo( - new EnableNodeToggleCommand(node.name, node.disabled === true, newDisabledState), + new EnableNodeToggleCommand( + node.name, + node.disabled === true, + newDisabledState, + Date.now(), + ), ); } } @@ -907,7 +912,7 @@ export function useNodeHelpers() { type: NodeConnectionType.Main, }, ]; - const removeCommand = new RemoveConnectionCommand(connectionData); + const removeCommand = new RemoveConnectionCommand(connectionData, Date.now()); historyStore.pushCommandToUndo(removeCommand); } } @@ -1133,7 +1138,7 @@ export function useNodeHelpers() { if (removeVisualConnection) { deleteJSPlumbConnection(info.connection, trackHistory); } else if (trackHistory) { - historyStore.pushCommandToUndo(new RemoveConnectionCommand(connectionInfo)); + historyStore.pushCommandToUndo(new RemoveConnectionCommand(connectionInfo, Date.now())); } workflowsStore.removeConnection({ connection: connectionInfo }); } @@ -1234,7 +1239,7 @@ export function useNodeHelpers() { matchCredentials(newNode); workflowsStore.addNode(newNode); if (trackHistory) { - historyStore.pushCommandToUndo(new AddNodeCommand(newNode)); + historyStore.pushCommandToUndo(new AddNodeCommand(newNode, Date.now())); } }); diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index 3ffc77d8a0c13..51c7dcb2b0e18 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -37,6 +37,15 @@ import { get } from 'lodash-es'; import { useExecutionsStore } from '@/stores/executions.store'; import { useSettingsStore } from '@/stores/settings.store'; import { usePushConnectionStore } from '@/stores/pushConnection.store'; +import { computed } from 'vue'; +import type { CanvasNodeDirtiness } from '@/types'; +import { + BulkCommand, + EnableNodeToggleCommand, + RemoveConnectionCommand, + type Undoable, +} from '@/models/history'; +import { useHistoryStore } from '@/stores/history.store'; export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType }) { const nodeHelpers = useNodeHelpers(); @@ -49,6 +58,91 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType { + // Do not highlight dirtiness if new partial execution is not enabled + if (settingsStore.partialExecutionVersion === 1) { + return {}; + } + + const dirtiness: Record = {}; + const visitedNodes: Set = new Set(); + const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; + + function markDownstreamStaleRecursively(nodeName: string): void { + if (visitedNodes.has(nodeName)) { + return; // prevent infinite recursion + } + + visitedNodes.add(nodeName); + + for (const inputConnections of Object.values( + workflowsStore.outgoingConnectionsByNodeName(nodeName), + )) { + for (const connections of inputConnections) { + for (const { node } of connections ?? []) { + const hasRunData = (runDataByNode[node] ?? []).length > 0; + + if (hasRunData) { + dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; + } + + markDownstreamStaleRecursively(node); + } + } + } + } + + function shouldMarkDirty(command: Undoable, nodeName: string): boolean { + if (command instanceof BulkCommand) { + return command.commands.some((c) => shouldMarkDirty(c, nodeName)); + } + + if (command instanceof RemoveConnectionCommand) { + return command.connectionData[1]?.node === nodeName; + } + + if (command instanceof EnableNodeToggleCommand) { + debugger; + return command.nodeName === nodeName; + } + + return false; + } + + for (const [nodeName, taskData] of Object.entries(runDataByNode)) { + const runAt = taskData[0]?.startTime; + + if (!runAt) { + continue; + } + + const parametersLastUpdate = workflowsStore.getParametersLastUpdate(nodeName); + + if (parametersLastUpdate && parametersLastUpdate > runAt) { + dirtiness[nodeName] = 'dirty'; + markDownstreamStaleRecursively(nodeName); + continue; + } + + const incomingConnectionsLastUpdate = Math.max( + 0, + ...historyStore.undoStack.flatMap((command) => + shouldMarkDirty(command, nodeName) ? [command.getTimestamp()] : [], + ), + ); + + if (incomingConnectionsLastUpdate > runAt) { + dirtiness[nodeName] = 'incoming-connections-changed'; + markDownstreamStaleRecursively(nodeName); + } + } + + return dirtiness; + }); + // Starts to execute a workflow on server async function runWorkflowApi(runData: IStartRunData): Promise { if (!pushConnectionStore.isConnected) { @@ -245,7 +339,6 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType - dirtiness === 'incoming-connections-changed' || dirtiness === 'dirty' ? [nodeName] : [], + const nodeNames = Object.entries(dirtinessByName).flatMap(([nodeName, dirtiness]) => + dirtiness === 'incoming-connections-changed' || dirtiness === 'dirty' ? [nodeName] : [], ); startRunData.dirtyNodeNames = nodeNames.length > 0 ? nodeNames : undefined; @@ -451,6 +543,7 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType; + + getTimestamp(): number { + return this.timestamp; + } } export class BulkCommand extends Undoable { @@ -43,6 +52,10 @@ export class BulkCommand extends Undoable { super(); this.commands = commands; } + + getTimestamp(): number { + return Math.max(0, ...this.commands.map((command) => command.timestamp)); + } } export class MoveNodeCommand extends Command { @@ -52,15 +65,20 @@ export class MoveNodeCommand extends Command { newPosition: XYPosition; - constructor(nodeName: string, oldPosition: XYPosition, newPosition: XYPosition) { - super(COMMANDS.MOVE_NODE); + constructor( + nodeName: string, + oldPosition: XYPosition, + newPosition: XYPosition, + timestamp: number, + ) { + super(COMMANDS.MOVE_NODE, timestamp); this.nodeName = nodeName; this.newPosition = newPosition; this.oldPosition = oldPosition; } - getReverseCommand(): Command { - return new MoveNodeCommand(this.nodeName, this.newPosition, this.oldPosition); + getReverseCommand(timestamp: number): Command { + return new MoveNodeCommand(this.nodeName, this.newPosition, this.oldPosition, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -88,13 +106,13 @@ export class MoveNodeCommand extends Command { export class AddNodeCommand extends Command { node: INodeUi; - constructor(node: INodeUi) { - super(COMMANDS.ADD_NODE); + constructor(node: INodeUi, timestamp: number) { + super(COMMANDS.ADD_NODE, timestamp); this.node = node; } - getReverseCommand(): Command { - return new RemoveNodeCommand(this.node); + getReverseCommand(timestamp: number): Command { + return new RemoveNodeCommand(this.node, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -112,13 +130,13 @@ export class AddNodeCommand extends Command { export class RemoveNodeCommand extends Command { node: INodeUi; - constructor(node: INodeUi) { - super(COMMANDS.REMOVE_NODE); + constructor(node: INodeUi, timestamp: number) { + super(COMMANDS.REMOVE_NODE, timestamp); this.node = node; } - getReverseCommand(): Command { - return new AddNodeCommand(this.node); + getReverseCommand(timestamp: number): Command { + return new AddNodeCommand(this.node, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -136,13 +154,13 @@ export class RemoveNodeCommand extends Command { export class AddConnectionCommand extends Command { connectionData: [IConnection, IConnection]; - constructor(connectionData: [IConnection, IConnection]) { - super(COMMANDS.ADD_CONNECTION); + constructor(connectionData: [IConnection, IConnection], timestamp: number) { + super(COMMANDS.ADD_CONNECTION, timestamp); this.connectionData = connectionData; } - getReverseCommand(): Command { - return new RemoveConnectionCommand(this.connectionData); + getReverseCommand(timestamp: number): Command { + return new RemoveConnectionCommand(this.connectionData, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -166,13 +184,13 @@ export class AddConnectionCommand extends Command { export class RemoveConnectionCommand extends Command { connectionData: [IConnection, IConnection]; - constructor(connectionData: [IConnection, IConnection]) { - super(COMMANDS.REMOVE_CONNECTION); + constructor(connectionData: [IConnection, IConnection], timestamp: number) { + super(COMMANDS.REMOVE_CONNECTION, timestamp); this.connectionData = connectionData; } - getReverseCommand(): Command { - return new AddConnectionCommand(this.connectionData); + getReverseCommand(timestamp: number): Command { + return new AddConnectionCommand(this.connectionData, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -202,15 +220,15 @@ export class EnableNodeToggleCommand extends Command { newState: boolean; - constructor(nodeName: string, oldState: boolean, newState: boolean) { - super(COMMANDS.ENABLE_NODE_TOGGLE); + constructor(nodeName: string, oldState: boolean, newState: boolean, timestamp: number) { + super(COMMANDS.ENABLE_NODE_TOGGLE, timestamp); this.nodeName = nodeName; this.newState = newState; this.oldState = oldState; } - getReverseCommand(): Command { - return new EnableNodeToggleCommand(this.nodeName, this.newState, this.oldState); + getReverseCommand(timestamp: number): Command { + return new EnableNodeToggleCommand(this.nodeName, this.newState, this.oldState, timestamp); } isEqualTo(anotherCommand: Command): boolean { @@ -235,14 +253,14 @@ export class RenameNodeCommand extends Command { newName: string; - constructor(currentName: string, newName: string) { - super(COMMANDS.RENAME_NODE); + constructor(currentName: string, newName: string, timestamp: number) { + super(COMMANDS.RENAME_NODE, timestamp); this.currentName = currentName; this.newName = newName; } - getReverseCommand(): Command { - return new RenameNodeCommand(this.newName, this.currentName); + getReverseCommand(timestamp: number): Command { + return new RenameNodeCommand(this.newName, this.currentName, timestamp); } isEqualTo(anotherCommand: Command): boolean { diff --git a/packages/editor-ui/src/stores/canvas.store.ts b/packages/editor-ui/src/stores/canvas.store.ts index b96699f29704e..7bd9df9311e10 100644 --- a/packages/editor-ui/src/stores/canvas.store.ts +++ b/packages/editor-ui/src/stores/canvas.store.ts @@ -294,7 +294,7 @@ export const useCanvasStore = defineStore('canvas', () => { const oldPosition = node.position; if (oldPosition[0] !== newNodePosition[0] || oldPosition[1] !== newNodePosition[1]) { historyStore.pushCommandToUndo( - new MoveNodeCommand(node.name, oldPosition, newNodePosition), + new MoveNodeCommand(node.name, oldPosition, newNodePosition, Date.now()), ); workflowStore.updateNodeProperties(updateInformation); } diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index a49746f78cd91..5be37e74e8181 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -89,7 +89,6 @@ import { useNodeHelpers } from '@/composables/useNodeHelpers'; import { useUsersStore } from '@/stores/users.store'; import { updateCurrentUserSettings } from '@/api/users'; import { useExecutingNode } from '@/composables/useExecutingNode'; -import { type CanvasNodeDirtiness } from '@/types'; const defaults: Omit & { settings: NonNullable } = { name: '', @@ -274,64 +273,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { const getPastChatMessages = computed(() => Array.from(new Set(chatMessages.value))); - const dirtinessByName = computed(() => { - // Do not highlight dirtiness if new partial execution is not enabled - if (settingsStore.partialExecutionVersion === 1) { - return {}; - } - - const dirtiness: Record = {}; - const visitedNodes: Set = new Set(); - const runDataByNode = workflowExecutionData.value?.data?.resultData.runData ?? {}; - - function markDownstreamStaleRecursively(nodeName: string): void { - if (visitedNodes.has(nodeName)) { - return; // prevent infinite recursion - } - - visitedNodes.add(nodeName); - - for (const inputConnections of Object.values(outgoingConnectionsByNodeName(nodeName))) { - for (const connections of inputConnections) { - for (const { node } of connections ?? []) { - const hasRunData = (runDataByNode[node] ?? []).length > 0; - - if (hasRunData) { - dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; - } - - markDownstreamStaleRecursively(node); - } - } - } - } - - for (const [nodeName, taskData] of Object.entries(runDataByNode)) { - const runAt = taskData[0]?.startTime; - - if (!runAt) { - continue; - } - - const parametersLastUpdate = getParametersLastUpdate(nodeName); - - if (parametersLastUpdate && parametersLastUpdate > runAt) { - dirtiness[nodeName] = 'dirty'; - markDownstreamStaleRecursively(nodeName); - continue; - } - - const incomingConnectionsLastUpdate = getIncomingConnectionsLastUpdate(nodeName); - - if (incomingConnectionsLastUpdate && incomingConnectionsLastUpdate > runAt) { - dirtiness[nodeName] = 'incoming-connections-changed'; - markDownstreamStaleRecursively(nodeName); - } - } - - return dirtiness; - }); - function getWorkflowResultDataByNodeName(nodeName: string): ITaskData[] | null { if (getWorkflowRunData.value === null) { return null; @@ -387,10 +328,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return nodeMetadata.value[nodeName]?.parametersLastUpdatedAt; } - function getIncomingConnectionsLastUpdate(nodeName: string): number | undefined { - return nodeMetadata.value[nodeName]?.incomingConnectionsLastUpdatedAt; - } - function isNodePristine(nodeName: string): boolean { return nodeMetadata.value[nodeName] === undefined || nodeMetadata.value[nodeName].pristine; } @@ -1011,8 +948,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { connections.splice(parseInt(index, 10), 1); } } - - nodeMetadata.value[destinationData.node].incomingConnectionsLastUpdatedAt = Date.now(); } function removeAllConnections(data: { setStateDirty: boolean }): void { @@ -1245,20 +1180,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { updateNodeAtIndex(nodeIndex, { [key]: property }); } - - if (updateInformation.properties.disabled !== undefined) { - const outgoingConnections = Object.values( - outgoingConnectionsByNodeName(updateInformation.name), - ); - - for (const nodeConnections of outgoingConnections) { - for (const connections of nodeConnections) { - for (const { node } of connections ?? []) { - nodeMetadata.value[node].incomingConnectionsLastUpdatedAt = Date.now(); - } - } - } - } } } @@ -1733,7 +1654,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { getPastChatMessages, isChatPanelOpen: computed(() => isChatPanelOpen.value), isLogsPanelOpen: computed(() => isLogsPanelOpen.value), - dirtinessByName, setPanelOpen, outgoingConnectionsByNodeName, incomingConnectionsByNodeName, @@ -1744,7 +1664,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { getNodeById, getNodesByIds, getParametersLastUpdate, - getIncomingConnectionsLastUpdate, isNodePristine, isNodeExecuting, getExecutionDataById, diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 6d2db0753b7a8..5cdf517c32257 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -1700,7 +1700,12 @@ export default defineComponent({ oldPosition[1] !== updateInformation.properties.position[1] ) { this.historyStore.pushCommandToUndo( - new MoveNodeCommand(nodeName, oldPosition, updateInformation.properties.position), + new MoveNodeCommand( + nodeName, + oldPosition, + updateInformation.properties.position, + Date.now(), + ), recordHistory, ); } @@ -2914,7 +2919,9 @@ export default defineComponent({ if (!this.isLoading) { this.uiStore.stateIsDirty = true; if (!this.suspendRecordingDetachedConnections) { - this.historyStore.pushCommandToUndo(new AddConnectionCommand(connectionData)); + this.historyStore.pushCommandToUndo( + new AddConnectionCommand(connectionData, Date.now()), + ); } // When we add multiple nodes, this event could be fired hundreds of times for large workflows. // And because the updateNodesInputIssues() method is quite expensive, we only call it if not in insert mode @@ -3103,7 +3110,9 @@ export default defineComponent({ .overrideTargetEndpoint as Endpoint | null; if (connectionInfo) { - this.historyStore.pushCommandToUndo(new RemoveConnectionCommand(connectionInfo)); + this.historyStore.pushCommandToUndo( + new RemoveConnectionCommand(connectionInfo, Date.now()), + ); } this.connectTwoNodes( sourceNodeName, @@ -3122,7 +3131,7 @@ export default defineComponent({ ) { // Ff connection being detached by user, save this in history // but skip if it's detached as a side effect of bulk undo/redo or node rename process - const removeCommand = new RemoveConnectionCommand(connectionInfo); + const removeCommand = new RemoveConnectionCommand(connectionInfo, Date.now()); this.historyStore.pushCommandToUndo(removeCommand); } @@ -3719,7 +3728,7 @@ export default defineComponent({ // Remove node from selected index if found in it this.uiStore.removeNodeFromSelection(node); if (trackHistory) { - this.historyStore.pushCommandToUndo(new RemoveNodeCommand(node)); + this.historyStore.pushCommandToUndo(new RemoveNodeCommand(node, Date.now())); } }); // allow other events to finish like drag stop @@ -3818,7 +3827,9 @@ export default defineComponent({ workflow.renameNode(currentName, newName); if (trackHistory) { - this.historyStore.pushCommandToUndo(new RenameNodeCommand(currentName, newName)); + this.historyStore.pushCommandToUndo( + new RenameNodeCommand(currentName, newName, Date.now()), + ); } // Update also last selected node and execution data From 0211987bb30b96c9f1f36d38fa2c9b2ceabe2879 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 09:50:17 +0100 Subject: [PATCH 17/52] remove duplicate store --- packages/editor-ui/src/composables/useRunWorkflow.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index 65af734ab2e5d..47a8591410bfe 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -64,7 +64,6 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType { // Do not highlight dirtiness if new partial execution is not enabled @@ -117,8 +116,9 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType Date: Wed, 12 Feb 2025 14:16:43 +0100 Subject: [PATCH 18/52] cover all connection related dirtiness checks --- .../src/composables/useRunWorkflow.ts | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index 47a8591410bfe..aa949f6756b6b 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -42,9 +42,11 @@ import { usePushConnectionStore } from '@/stores/pushConnection.store'; import { computed } from 'vue'; import type { CanvasNodeDirtiness } from '@/types'; import { + AddConnectionCommand, + AddNodeCommand, BulkCommand, EnableNodeToggleCommand, - RemoveConnectionCommand, + RemoveNodeCommand, type Undoable, } from '@/models/history'; import { useHistoryStore } from '@/stores/history.store'; @@ -104,21 +106,27 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType shouldMarkDirty(c, nodeName)); } - if (command instanceof RemoveConnectionCommand) { + if (command instanceof AddConnectionCommand) { return command.connectionData[1]?.node === nodeName; } + if (command instanceof RemoveNodeCommand || command instanceof AddNodeCommand) { + return Object.values(workflowsStore.incomingConnectionsByNodeName(nodeName)).some((c) => + c.some((cc) => cc?.some((ccc) => ccc.node === command.node.name)), + ); + } + if (command instanceof EnableNodeToggleCommand) { - debugger; - return command.nodeName === nodeName; + return Object.values(workflowsStore.incomingConnectionsByNodeName(nodeName)).some((c) => + c.some((cc) => cc?.some((ccc) => ccc.node === command.nodeName)), + ); } return false; } - for (const node of workflowsStore.allNodes) { - const nodeName = node.name; - const runAt = runDataByNode[nodeName]?.[0]?.startTime; + for (const [nodeName, taskData] of Object.entries(runDataByNode)) { + const runAt = taskData?.[0]?.startTime; if (!runAt) { continue; @@ -367,7 +375,7 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType + const nodeNames = Object.entries(dirtinessByName.value).flatMap(([nodeName, dirtiness]) => dirtiness === 'incoming-connections-changed' || dirtiness === 'dirty' ? [nodeName] : [], ); From 2846863892d5d650eb6ba951fc660b630966ed3f Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 15:05:06 +0100 Subject: [PATCH 19/52] feat: updating pinned data makes downstream dirty --- packages/editor-ui/src/Interface.ts | 1 + .../parts/CanvasNodeStatusIcons.test.ts | 5 +++- .../parts/CanvasNodeStatusIcons.vue | 2 +- .../src/composables/useRunWorkflow.test.ts | 2 +- .../src/composables/useRunWorkflow.ts | 24 +++++++++++++------ .../src/stores/workflows.store.test.ts | 6 ++--- .../editor-ui/src/stores/workflows.store.ts | 21 +++++++++++++--- packages/editor-ui/src/types/canvas.ts | 6 ++++- 8 files changed, 50 insertions(+), 17 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 8bf613b2b222a..2b164742fbd1a 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -847,6 +847,7 @@ export interface ITemplatesNode extends IVersionNode { export interface INodeMetadata { parametersLastUpdatedAt?: number; + pinnedDataUpdatedAt?: number; pristine: boolean; } diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.test.ts b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.test.ts index 6dc81530fce51..8bdd32386d7d0 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.test.ts +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.test.ts @@ -59,7 +59,10 @@ describe('CanvasNodeStatusIcons', () => { provide: createCanvasNodeProvide({ data: { runData: { outputMap: {}, iterations: 15, visible: true }, - render: { type: CanvasNodeRenderType.Default, options: { dirtiness: 'dirty' } }, + render: { + type: CanvasNodeRenderType.Default, + options: { dirtiness: 'parameters-updated' }, + }, }, }), }, diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index 7cc7e29ecbb44..fc9c25308ddc4 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -27,7 +27,7 @@ const hideNodeIssues = computed(() => false); // @TODO Implement this const isStale = computed( () => render.value.type === CanvasNodeRenderType.Default && - render.value.options.dirtiness === 'dirty', + render.value.options.dirtiness === 'parameters-updated', ); diff --git a/packages/editor-ui/src/composables/useRunWorkflow.test.ts b/packages/editor-ui/src/composables/useRunWorkflow.test.ts index 608bdfb285c00..e27fe41a596bf 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.test.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.test.ts @@ -346,7 +346,7 @@ describe('useRunWorkflow({ router })', () => { }, ], }; - vi.mocked(workflowsStore).dirtinessByName = { [executeName]: 'dirty' }; + vi.mocked(workflowsStore).dirtinessByName = { [executeName]: 'parameters-updated' }; vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue({ name: 'Test Workflow', getParentNodes: () => [parentName], diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index aa949f6756b6b..007726fc7718f 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -125,17 +125,18 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType runAt) { - dirtiness[nodeName] = 'dirty'; + if (parametersLastUpdate > runAt) { + dirtiness[nodeName] = 'parameters-updated'; markDownstreamStaleRecursively(nodeName); continue; } @@ -148,8 +149,17 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType runAt) { - dirtiness[nodeName] = 'incoming-connections-changed'; + dirtiness[nodeName] = 'incoming-connections-updated'; markDownstreamStaleRecursively(nodeName); + continue; + } + + const pinnedDataUpdatedAt = workflowsStore.getPinnedDataLastUpdate(nodeName) ?? 0; + + if (pinnedDataUpdatedAt > runAt) { + dirtiness[nodeName] = 'pinned-data-updated'; + markDownstreamStaleRecursively(nodeName); + continue; } } @@ -376,7 +386,7 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType - dirtiness === 'incoming-connections-changed' || dirtiness === 'dirty' ? [nodeName] : [], + dirtiness ? [nodeName] : [], ); startRunData.dirtyNodeNames = nodeNames.length > 0 ? nodeNames : undefined; diff --git a/packages/editor-ui/src/stores/workflows.store.test.ts b/packages/editor-ui/src/stores/workflows.store.test.ts index 07e52ca209733..c1e3cdaf0eea9 100644 --- a/packages/editor-ui/src/stores/workflows.store.test.ts +++ b/packages/editor-ui/src/stores/workflows.store.test.ts @@ -767,7 +767,7 @@ describe('useWorkflowsStore', () => { it('should mark nodes with run data older than the last update time as dirty', () => { workflowsStore.workflow = { connections: {} as IConnections } as IWorkflowDb; - expect(workflowsStore.dirtinessByName).toEqual({ node1: 'dirty' }); + expect(workflowsStore.dirtinessByName).toEqual({ node1: 'parameters-updated' }); }); it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { @@ -779,7 +779,7 @@ describe('useWorkflowsStore', () => { } as IWorkflowDb; expect(workflowsStore.dirtinessByName).toEqual({ - node1: 'dirty', + node1: 'parameters-updated', node2: 'upstream-dirty', }); }); @@ -794,7 +794,7 @@ describe('useWorkflowsStore', () => { } as IWorkflowDb; expect(workflowsStore.dirtinessByName).toEqual({ - node1: 'dirty', + node1: 'parameters-updated', node2: 'upstream-dirty', }); }); diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 5be37e74e8181..afbb9a2d327d3 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -328,6 +328,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return nodeMetadata.value[nodeName]?.parametersLastUpdatedAt; } + function getPinnedDataLastUpdate(nodeName: string): number | undefined { + return nodeMetadata.value[nodeName]?.pinnedDataUpdatedAt; + } + function isNodePristine(nodeName: string): boolean { return nodeMetadata.value[nodeName] === undefined || nodeMetadata.value[nodeName].pristine; } @@ -789,6 +793,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } function pinData(payload: { node: INodeUi; data: INodeExecutionData[] }): void { + const nodeName = payload.node.name; + if (!workflow.value.pinData) { workflow.value = { ...workflow.value, pinData: {} }; } @@ -797,6 +803,11 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { payload.data = [payload.data]; } + if ((workflow.value.pinData?.[nodeName] ?? []).length > 0) { + // Updating existing pinned data + nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); + } + const storedPinData = payload.data.map((item) => isJsonKeyObject(item) ? { json: item.json } : { json: item }, ); @@ -805,7 +816,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { ...workflow.value, pinData: { ...workflow.value.pinData, - [payload.node.name]: storedPinData, + [nodeName]: storedPinData, }, }; @@ -816,21 +827,24 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } function unpinData(payload: { node: INodeUi }): void { + const nodeName = payload.node.name; + if (!workflow.value.pinData) { workflow.value = { ...workflow.value, pinData: {} }; } - const { [payload.node.name]: _, ...pinData } = workflow.value.pinData as IPinData; + const { [nodeName]: _, ...pinData } = workflow.value.pinData as IPinData; workflow.value = { ...workflow.value, pinData, }; + nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); uiStore.stateIsDirty = true; updateCachedWorkflow(); dataPinningEventBus.emit('unpin-data', { - nodeNames: [payload.node.name], + nodeNames: [nodeName], }); } @@ -1664,6 +1678,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { getNodeById, getNodesByIds, getParametersLastUpdate, + getPinnedDataLastUpdate, isNodePristine, isNodeExecuting, getExecutionDataById, diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts index 77ff691a38f28..e1c4ec40d8b43 100644 --- a/packages/editor-ui/src/types/canvas.ts +++ b/packages/editor-ui/src/types/canvas.ts @@ -47,7 +47,11 @@ export const enum CanvasNodeRenderType { export type CanvasNodeDefaultRenderLabelSize = 'small' | 'medium' | 'large'; -export type CanvasNodeDirtiness = 'dirty' | 'upstream-dirty' | 'incoming-connections-changed'; +export type CanvasNodeDirtiness = + | 'parameters-updated' + | 'incoming-connections-updated' + | 'pinned-data-updated' + | 'upstream-dirty'; export type CanvasNodeDefaultRender = { type: CanvasNodeRenderType.Default; From 0110c868e08da31ffa7b1c02f087db33bbc4a047 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 15:08:21 +0100 Subject: [PATCH 20/52] revert unrelated changes --- packages/editor-ui/src/Interface.ts | 5 ++++- packages/editor-ui/src/components/RunData.vue | 6 +++--- packages/editor-ui/src/composables/useNodeHelpers.ts | 5 +++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 2b164742fbd1a..08d6dcf20d8bc 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -139,7 +139,10 @@ export interface IUpdateInformation>; + properties: { + position: XYPosition; + [key: string]: IDataObject | XYPosition; + }; } export type XYPosition = [number, number]; diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index ab97a0fb61fd5..bf7175e68defb 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -1254,12 +1254,12 @@ function onRunIndexChange(run: number) { function enableNode() { if (node.value) { - const updateInformation: INodeUpdatePropertiesInformation = { + const updateInformation = { name: node.value.name, properties: { disabled: !node.value.disabled, - }, - }; + } as IDataObject, + } as INodeUpdatePropertiesInformation; workflowsStore.updateNodeProperties(updateInformation); } diff --git a/packages/editor-ui/src/composables/useNodeHelpers.ts b/packages/editor-ui/src/composables/useNodeHelpers.ts index 26fcc61662572..eee7f040dee02 100644 --- a/packages/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/editor-ui/src/composables/useNodeHelpers.ts @@ -25,6 +25,7 @@ import type { ITaskDataConnections, IRunData, IBinaryKeyData, + IDataObject, INode, INodePropertyOptions, INodeCredentialsDetails, @@ -679,8 +680,8 @@ export function useNodeHelpers() { name: node.name, properties: { disabled: newDisabledState, - }, - }; + } as IDataObject, + } as INodeUpdatePropertiesInformation; telemetry.track('User set node enabled status', { node_type: node.type, From 209a1bb89c7034f9b620d6039452df8f7fa786a1 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 16:16:21 +0100 Subject: [PATCH 21/52] fix failing tests --- .../src/composables/useRunWorkflow.test.ts | 107 +++++++++++++++++- .../src/stores/workflows.store.test.ts | 84 +------------- 2 files changed, 107 insertions(+), 84 deletions(-) diff --git a/packages/editor-ui/src/composables/useRunWorkflow.test.ts b/packages/editor-ui/src/composables/useRunWorkflow.test.ts index e27fe41a596bf..91c949c36b6fe 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.test.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.test.ts @@ -9,6 +9,8 @@ import { type Workflow, type IExecuteData, type ITaskData, + NodeConnectionType, + type INodeConnections, } from 'n8n-workflow'; import { useRunWorkflow } from '@/composables/useRunWorkflow'; @@ -21,9 +23,12 @@ import { useI18n } from '@/composables/useI18n'; import { captor, mock } from 'vitest-mock-extended'; import { useSettingsStore } from '@/stores/settings.store'; import { usePushConnectionStore } from '@/stores/pushConnection.store'; +import { type FrontendSettings } from '@n8n/api-types'; +import { createTestNode } from '@/__tests__/mocks'; vi.mock('@/stores/workflows.store', () => ({ useWorkflowsStore: vi.fn().mockReturnValue({ + allNodes: [], runWorkflow: vi.fn(), subWorkflowExecutionError: null, getWorkflowRunData: null, @@ -37,6 +42,8 @@ vi.mock('@/stores/workflows.store', () => ({ nodeIssuesExit: vi.fn(), checkIfNodeHasChatParent: vi.fn(), getParametersLastUpdate: vi.fn(), + getPinnedDataLastUpdate: vi.fn(), + outgoingConnectionsByNodeName: vi.fn(), }), })); @@ -326,6 +333,18 @@ describe('useRunWorkflow({ router })', () => { const composable = useRunWorkflow({ router }); const parentName = 'When clicking'; const executeName = 'Code'; + vi.mocked(workflowsStore).allNodes = [ + createTestNode({ name: parentName }), + createTestNode({ name: executeName }), + ]; + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( + (nodeName) => + ({ + [parentName]: { + main: [[{ node: executeName, type: NodeConnectionType.Main, index: 0 }]], + }, + })[nodeName] ?? ({} as INodeConnections), + ); vi.mocked(workflowsStore).getWorkflowRunData = { [parentName]: [ { @@ -346,7 +365,6 @@ describe('useRunWorkflow({ router })', () => { }, ], }; - vi.mocked(workflowsStore).dirtinessByName = { [executeName]: 'parameters-updated' }; vi.mocked(workflowHelpers).getCurrentWorkflow.mockReturnValue({ name: 'Test Workflow', getParentNodes: () => [parentName], @@ -650,4 +668,91 @@ describe('useRunWorkflow({ router })', () => { }); }); }); + + describe('dirtinessByName', () => { + beforeEach(() => { + // Enable new partial execution + settingsStore.settings = { + partialExecution: { version: 2, enforce: true }, + } as FrontendSettings; + + vi.mocked(workflowsStore).getWorkflowRunData = { + node1: [ + { + startTime: +new Date('2025-01-01'), // ran before parameter update + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ], + node2: [ + { + startTime: +new Date('2025-01-03'), // ran after parameter update + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ], + node3: [], // never ran before + }; + + vi.mocked(workflowsStore).allNodes = [ + createTestNode({ name: 'node1' }), + createTestNode({ name: 'node2' }), + createTestNode({ name: 'node3' }), + ]; + + vi.mocked(workflowsStore).getParametersLastUpdate.mockImplementation( + (nodeName) => + ({ + node1: +new Date('2025-01-02'), + node2: +new Date('2025-01-02'), + node3: +new Date('2025-01-02'), + })[nodeName], + ); + vi.mocked(workflowsStore).getPinnedDataLastUpdate.mockReturnValue(undefined); + }); + + it('should mark nodes with run data older than the last update time as dirty', () => { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation(() => ({})); + + const { dirtinessByName } = useRunWorkflow({ router }); + + expect(dirtinessByName.value).toEqual({ node1: 'parameters-updated' }); + }); + + it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( + (nodeName) => + ({ + node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, + node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, + })[nodeName] ?? ({} as INodeConnections), + ); + + const { dirtinessByName } = useRunWorkflow({ router }); + + expect(dirtinessByName.value).toEqual({ + node1: 'parameters-updated', + node2: 'upstream-dirty', + }); + }); + + it('should return even if the connections forms a loop', () => { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( + (nodeName) => + ({ + node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, + node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, + })[nodeName] ?? ({} as INodeConnections), + ); + + const { dirtinessByName } = useRunWorkflow({ router }); + + expect(dirtinessByName.value).toEqual({ + node1: 'parameters-updated', + node2: 'upstream-dirty', + }); + }); + }); }); diff --git a/packages/editor-ui/src/stores/workflows.store.test.ts b/packages/editor-ui/src/stores/workflows.store.test.ts index c1e3cdaf0eea9..d60cd8f127a27 100644 --- a/packages/editor-ui/src/stores/workflows.store.test.ts +++ b/packages/editor-ui/src/stores/workflows.store.test.ts @@ -12,15 +12,7 @@ import type { IExecutionResponse, INodeUi, IWorkflowDb, IWorkflowSettings } from import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { SEND_AND_WAIT_OPERATION } from 'n8n-workflow'; -import type { - IPinData, - ExecutionSummary, - IConnection, - INodeExecutionData, - IRunExecutionData, - IRunData, - IConnections, -} from 'n8n-workflow'; +import type { IPinData, ExecutionSummary, IConnection, INodeExecutionData } from 'n8n-workflow'; import { stringSizeInBytes } from '@/utils/typesUtils'; import { dataPinningEventBus } from '@/event-bus'; import { useUIStore } from '@/stores/ui.store'; @@ -729,80 +721,6 @@ describe('useWorkflowsStore', () => { ); }, ); - - describe('dirtinessByName', () => { - beforeEach(() => { - // Enable new partial execution - settingsStore.settings = { - partialExecution: { version: 2, enforce: true }, - } as FrontendSettings; - - workflowsStore.workflowExecutionData = createExecutionData({ - node1: [ - { - startTime: +new Date('2025-01-01'), // ran before parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node2: [ - { - startTime: +new Date('2025-01-03'), // ran after parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node3: [], // never ran before - }); - - workflowsStore.nodeMetadata = { - node1: { parametersLastUpdatedAt: +new Date('2025-01-02'), pristine: true }, - node2: { parametersLastUpdatedAt: +new Date('2025-01-02'), pristine: true }, - node3: { parametersLastUpdatedAt: +new Date('2025-01-02'), pristine: true }, - }; - }); - - it('should mark nodes with run data older than the last update time as dirty', () => { - workflowsStore.workflow = { connections: {} as IConnections } as IWorkflowDb; - - expect(workflowsStore.dirtinessByName).toEqual({ node1: 'parameters-updated' }); - }); - - it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { - workflowsStore.workflow = { - connections: { - node1: { main: [[{ node: 'node2', type: 'main', index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: 'main', index: 0 }]] }, - } as IConnections, - } as IWorkflowDb; - - expect(workflowsStore.dirtinessByName).toEqual({ - node1: 'parameters-updated', - node2: 'upstream-dirty', - }); - }); - - it('should return even if the connections forms a loop', () => { - workflowsStore.workflow = { - connections: { - node1: { main: [[{ node: 'node2', type: 'main', index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: 'main', index: 0 }]] }, - node3: { main: [[{ node: 'node1', type: 'main', index: 0 }]] }, - } as IConnections, - } as IWorkflowDb; - - expect(workflowsStore.dirtinessByName).toEqual({ - node1: 'parameters-updated', - node2: 'upstream-dirty', - }); - }); - - function createExecutionData(runData: IRunData) { - return { data: { resultData: { runData } } as IRunExecutionData } as IExecutionResponse; - } - }); }); function getMockEditFieldsNode() { From 37ec0dd71ff9481d2048b381ef4342d194d0c65a Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 16:25:16 +0100 Subject: [PATCH 22/52] fix: reflect dirtiness to strikethrough line --- .../parts/CanvasNodeDisabledStrikeThrough.vue | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeDisabledStrikeThrough.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeDisabledStrikeThrough.vue index d292e8f951ea3..89684afd2f555 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeDisabledStrikeThrough.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeDisabledStrikeThrough.vue @@ -1,15 +1,19 @@ @@ -31,4 +35,8 @@ const classes = computed(() => { .success { border-color: var(--color-success-light); } + +.warning { + border-color: var(--color-warning-tint-1); +} From 8be673327618c0c62b7e13fa8d32f3a34af2465d Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 12 Feb 2025 17:11:59 +0100 Subject: [PATCH 23/52] fix compile error --- .../editor-ui/src/composables/useCanvasOperations.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/composables/useCanvasOperations.test.ts b/packages/editor-ui/src/composables/useCanvasOperations.test.ts index bc6f94301b1c9..3f32c107f4f94 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.test.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.test.ts @@ -706,7 +706,9 @@ describe('useCanvasOperations', () => { expect(workflowsStore.removeNodeById).toHaveBeenCalledWith(id); expect(workflowsStore.removeNodeExecutionDataById).toHaveBeenCalledWith(id); - expect(historyStore.pushCommandToUndo).toHaveBeenCalledWith(new RemoveNodeCommand(node)); + expect(historyStore.pushCommandToUndo).toHaveBeenCalledWith( + new RemoveNodeCommand(node, Date.now()), + ); }); it('should delete node without tracking history', () => { From ef16e1b3c1876a5d514854e415812f8c1d360be9 Mon Sep 17 00:00:00 2001 From: autologie Date: Thu, 13 Feb 2025 15:00:03 +0100 Subject: [PATCH 24/52] refactor --- .../src/composables/useCanvasMapping.ts | 6 +- .../src/composables/useNodeDirtiness.test.ts | 204 ++++++++++++++++++ .../src/composables/useNodeDirtiness.ts | 172 +++++++++++++++ .../src/composables/useRunWorkflow.test.ts | 88 -------- .../src/composables/useRunWorkflow.ts | 160 +++----------- 5 files changed, 405 insertions(+), 225 deletions(-) create mode 100644 packages/editor-ui/src/composables/useNodeDirtiness.test.ts create mode 100644 packages/editor-ui/src/composables/useNodeDirtiness.ts diff --git a/packages/editor-ui/src/composables/useCanvasMapping.ts b/packages/editor-ui/src/composables/useCanvasMapping.ts index ed1b9a36862fc..13b7052459b86 100644 --- a/packages/editor-ui/src/composables/useCanvasMapping.ts +++ b/packages/editor-ui/src/composables/useCanvasMapping.ts @@ -49,8 +49,7 @@ import { sanitizeHtml } from '@/utils/htmlUtils'; import { MarkerType } from '@vue-flow/core'; import { useNodeHelpers } from './useNodeHelpers'; import { getTriggerNodeServiceName } from '@/utils/nodeTypesUtils'; -import { useRunWorkflow } from './useRunWorkflow'; -import { useRouter } from 'vue-router'; +import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; export function useCanvasMapping({ nodes, @@ -65,8 +64,7 @@ export function useCanvasMapping({ const workflowsStore = useWorkflowsStore(); const nodeTypesStore = useNodeTypesStore(); const nodeHelpers = useNodeHelpers(); - const router = useRouter(); - const { dirtinessByName } = useRunWorkflow({ router }); + const { dirtinessByName } = useNodeDirtiness(); function createStickyNoteRenderType(node: INodeUi): CanvasNodeStickyNoteRender { return { diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts new file mode 100644 index 0000000000000..c5d28c0a6e894 --- /dev/null +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -0,0 +1,204 @@ +import { createTestNode } from '@/__tests__/mocks'; +import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useWorkflowsStore } from '@/stores/workflows.store'; +import { type FrontendSettings } from '@n8n/api-types'; +import { createTestingPinia } from '@pinia/testing'; +import { type INodeConnections, NodeConnectionType } from 'n8n-workflow'; +import { setActivePinia } from 'pinia'; +import type router from 'vue-router'; + +vi.mock('@/stores/workflows.store', () => ({ + useWorkflowsStore: vi.fn().mockReturnValue({ + allNodes: [], + runWorkflow: vi.fn(), + subWorkflowExecutionError: null, + getWorkflowRunData: null, + setWorkflowExecutionData: vi.fn(), + activeExecutionId: null, + nodesIssuesExist: false, + executionWaitingForWebhook: false, + getCurrentWorkflow: vi.fn().mockReturnValue({ id: '123' }), + getNodeByName: vi.fn(), + getExecution: vi.fn(), + nodeIssuesExit: vi.fn(), + checkIfNodeHasChatParent: vi.fn(), + getParametersLastUpdate: vi.fn(), + getPinnedDataLastUpdate: vi.fn(), + outgoingConnectionsByNodeName: vi.fn(), + }), +})); + +vi.mock('@/stores/pushConnection.store', () => ({ + usePushConnectionStore: vi.fn().mockReturnValue({ + isConnected: true, + }), +})); + +vi.mock('@/composables/useTelemetry', () => ({ + useTelemetry: vi.fn().mockReturnValue({ track: vi.fn() }), +})); + +vi.mock('@/composables/useI18n', () => ({ + useI18n: vi.fn().mockReturnValue({ baseText: vi.fn().mockImplementation((key) => key) }), +})); + +vi.mock('@/composables/useExternalHooks', () => ({ + useExternalHooks: vi.fn().mockReturnValue({ + run: vi.fn(), + }), +})); + +vi.mock('@/composables/useToast', () => ({ + useToast: vi.fn().mockReturnValue({ + clearAllStickyNotifications: vi.fn(), + showMessage: vi.fn(), + showError: vi.fn(), + }), +})); + +vi.mock('@/composables/useWorkflowHelpers', () => ({ + useWorkflowHelpers: vi.fn().mockReturnValue({ + getCurrentWorkflow: vi.fn(), + saveCurrentWorkflow: vi.fn(), + getWorkflowDataToSave: vi.fn(), + setDocumentTitle: vi.fn(), + executeData: vi.fn(), + getNodeTypes: vi.fn().mockReturnValue([]), + }), +})); + +vi.mock('@/composables/useNodeHelpers', () => ({ + useNodeHelpers: vi.fn().mockReturnValue({ + updateNodesExecutionIssues: vi.fn(), + }), +})); + +vi.mock('vue-router', async (importOriginal) => { + const { RouterLink } = await importOriginal(); + return { + RouterLink, + useRouter: vi.fn().mockReturnValue({ + push: vi.fn(), + }), + useRoute: vi.fn(), + }; +}); + +describe(useNodeDirtiness, () => { + let workflowsStore: ReturnType; + let settingsStore: ReturnType; + + beforeAll(() => { + const pinia = createTestingPinia({ stubActions: false }); + + setActivePinia(pinia); + + workflowsStore = useWorkflowsStore(); + settingsStore = useSettingsStore(); + }); + + describe.only('dirtinessByName', () => { + beforeEach(() => { + // Enable new partial execution + settingsStore.settings = { + partialExecution: { version: 2, enforce: true }, + } as FrontendSettings; + + vi.mocked(workflowsStore).getWorkflowRunData = { + node1: [ + { + startTime: +new Date('2025-01-01'), // ran before parameter update + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ], + node2: [ + { + startTime: +new Date('2025-01-03'), // ran after parameter update + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ], + node3: [], // never ran before + }; + + vi.mocked(workflowsStore).allNodes = [ + createTestNode({ name: 'node1' }), + createTestNode({ name: 'node2' }), + createTestNode({ name: 'node3' }), + ]; + + vi.mocked(workflowsStore).getParametersLastUpdate.mockImplementation( + (nodeName) => + ({ + node1: +new Date('2025-01-02'), + node2: +new Date('2025-01-02'), + node3: +new Date('2025-01-02'), + })[nodeName], + ); + vi.mocked(workflowsStore).getPinnedDataLastUpdate.mockReturnValue(undefined); + }); + + it('should mark nodes with run data older than the last update time as dirty', () => { + function calculateDirtiness(workflow: string) { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation(() => ({})); + const { dirtinessByName } = useNodeDirtiness(); + + return dirtinessByName.value; + } + + expect( + calculateDirtiness(` + A* -> B + B -> C + C + `), + ).toMatchInlineSnapshot(` + { + "node1": "parameters-updated", + } + `); + }); + + it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( + (nodeName) => + ({ + node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, + node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, + })[nodeName] ?? ({} as INodeConnections), + ); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toMatchInlineSnapshot(` + { + "node1": "parameters-updated", + "node2": "upstream-dirty", + } + `); + }); + + it('should return even if the connections forms a loop', () => { + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( + (nodeName) => + ({ + node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, + node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, + })[nodeName] ?? ({} as INodeConnections), + ); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toMatchInlineSnapshot(` + { + "node1": "parameters-updated", + "node2": "upstream-dirty", + } + `); + }); + }); +}); diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts new file mode 100644 index 0000000000000..5c7ad4e0d1556 --- /dev/null +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -0,0 +1,172 @@ +import { + AddConnectionCommand, + AddNodeCommand, + BulkCommand, + EnableNodeToggleCommand, + RemoveNodeCommand, + type Undoable, +} from '@/models/history'; +import { useHistoryStore } from '@/stores/history.store'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useWorkflowsStore } from '@/stores/workflows.store'; +import { type CanvasNodeDirtiness } from '@/types'; +import { type ITaskData, type INodeConnections, NodeConnectionType } from 'n8n-workflow'; +import { computed } from 'vue'; + +function markDownstreamDirtyRecursively( + nodeName: string, + dirtiness: Record, + visitedNodes: Set, + runDataByNode: Record, + getOutgoingConnections: (nodeName: string) => INodeConnections, +): void { + if (visitedNodes.has(nodeName)) { + return; // prevent infinite recursion + } + + visitedNodes.add(nodeName); + + for (const [type, inputConnections] of Object.entries(getOutgoingConnections(nodeName))) { + if ((type as NodeConnectionType) !== NodeConnectionType.Main) { + continue; + } + + for (const connections of inputConnections) { + for (const { node } of connections ?? []) { + const hasRunData = (runDataByNode[node] ?? []).length > 0; + + if (!hasRunData || dirtiness[node] !== undefined) { + continue; + } + + dirtiness[node] = 'upstream-dirty'; + + markDownstreamDirtyRecursively( + node, + dirtiness, + visitedNodes, + runDataByNode, + getOutgoingConnections, + ); + } + } + } +} + +/** + * Does the command make the given node dirty? + */ +function shouldMarkDirty( + command: Undoable, + nodeName: string, + nodeLastRanAt: number, + getIncomingConnections: (nodeName: string) => INodeConnections, +): boolean { + if (nodeLastRanAt > command.getTimestamp()) { + return false; + } + + if (command instanceof BulkCommand) { + return command.commands.some((cmd) => + shouldMarkDirty(cmd, nodeName, nodeLastRanAt, getIncomingConnections), + ); + } + + if (command instanceof AddConnectionCommand) { + return command.connectionData[1]?.node === nodeName; + } + + if ( + command instanceof RemoveNodeCommand || + command instanceof AddNodeCommand || + command instanceof EnableNodeToggleCommand + ) { + const commandTargetNodeName = + command instanceof RemoveNodeCommand || command instanceof AddNodeCommand + ? command.node.name + : command.nodeName; + + return Object.entries(getIncomingConnections(nodeName)).some(([type, nodeInputConnections]) => { + switch (type as NodeConnectionType) { + case NodeConnectionType.Main: + return nodeInputConnections.some((connections) => + connections?.some((connection) => connection.node === commandTargetNodeName), + ); + default: + return false; + } + }); + } + + return false; +} + +/** + * Determines the subgraph that is affected by the change made after the last execution + */ +export function useNodeDirtiness() { + const historyStore = useHistoryStore(); + const workflowsStore = useWorkflowsStore(); + const settingsStore = useSettingsStore(); + + const dirtinessByName = computed(() => { + // Do not highlight dirtiness if new partial execution is not enabled + if (settingsStore.partialExecutionVersion === 1) { + return {}; + } + + const dirtiness: Record = {}; + const visitedNodes: Set = new Set(); + const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; + + function markDownstreamDirty(nodeName: string) { + markDownstreamDirtyRecursively( + nodeName, + dirtiness, + visitedNodes, + runDataByNode, + workflowsStore.outgoingConnectionsByNodeName, + ); + } + + for (const node of workflowsStore.allNodes) { + const nodeName = node.name; + const runAt = runDataByNode[nodeName]?.[0]?.startTime ?? 0; + + if (!runAt) { + continue; + } + + const parametersLastUpdate = workflowsStore.getParametersLastUpdate(nodeName) ?? 0; + + if (parametersLastUpdate > runAt) { + dirtiness[nodeName] = 'parameters-updated'; + markDownstreamDirty(nodeName); + continue; + } + + if ( + runAt && + historyStore.undoStack.some((command) => + shouldMarkDirty(command, nodeName, runAt, workflowsStore.incomingConnectionsByNodeName), + ) + ) { + dirtiness[nodeName] = 'incoming-connections-updated'; + markDownstreamDirty(nodeName); + continue; + } + + const pinnedDataUpdatedAt = workflowsStore.getPinnedDataLastUpdate(nodeName) ?? 0; + + if (pinnedDataUpdatedAt > runAt) { + dirtiness[nodeName] = 'pinned-data-updated'; + markDownstreamDirty(nodeName); + continue; + } + } + + return dirtiness; + }); + + return { dirtinessByName }; +} diff --git a/packages/editor-ui/src/composables/useRunWorkflow.test.ts b/packages/editor-ui/src/composables/useRunWorkflow.test.ts index 91c949c36b6fe..f02ea12c3cac2 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.test.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.test.ts @@ -23,7 +23,6 @@ import { useI18n } from '@/composables/useI18n'; import { captor, mock } from 'vitest-mock-extended'; import { useSettingsStore } from '@/stores/settings.store'; import { usePushConnectionStore } from '@/stores/pushConnection.store'; -import { type FrontendSettings } from '@n8n/api-types'; import { createTestNode } from '@/__tests__/mocks'; vi.mock('@/stores/workflows.store', () => ({ @@ -668,91 +667,4 @@ describe('useRunWorkflow({ router })', () => { }); }); }); - - describe('dirtinessByName', () => { - beforeEach(() => { - // Enable new partial execution - settingsStore.settings = { - partialExecution: { version: 2, enforce: true }, - } as FrontendSettings; - - vi.mocked(workflowsStore).getWorkflowRunData = { - node1: [ - { - startTime: +new Date('2025-01-01'), // ran before parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node2: [ - { - startTime: +new Date('2025-01-03'), // ran after parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node3: [], // never ran before - }; - - vi.mocked(workflowsStore).allNodes = [ - createTestNode({ name: 'node1' }), - createTestNode({ name: 'node2' }), - createTestNode({ name: 'node3' }), - ]; - - vi.mocked(workflowsStore).getParametersLastUpdate.mockImplementation( - (nodeName) => - ({ - node1: +new Date('2025-01-02'), - node2: +new Date('2025-01-02'), - node3: +new Date('2025-01-02'), - })[nodeName], - ); - vi.mocked(workflowsStore).getPinnedDataLastUpdate.mockReturnValue(undefined); - }); - - it('should mark nodes with run data older than the last update time as dirty', () => { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation(() => ({})); - - const { dirtinessByName } = useRunWorkflow({ router }); - - expect(dirtinessByName.value).toEqual({ node1: 'parameters-updated' }); - }); - - it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( - (nodeName) => - ({ - node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, - })[nodeName] ?? ({} as INodeConnections), - ); - - const { dirtinessByName } = useRunWorkflow({ router }); - - expect(dirtinessByName.value).toEqual({ - node1: 'parameters-updated', - node2: 'upstream-dirty', - }); - }); - - it('should return even if the connections forms a loop', () => { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( - (nodeName) => - ({ - node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, - })[nodeName] ?? ({} as INodeConnections), - ); - - const { dirtinessByName } = useRunWorkflow({ router }); - - expect(dirtinessByName.value).toEqual({ - node1: 'parameters-updated', - node2: 'upstream-dirty', - }); - }); - }); }); diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index 007726fc7718f..67173e316bcdb 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -39,17 +39,7 @@ import { useExecutionsStore } from '@/stores/executions.store'; import { useTelemetry } from './useTelemetry'; import { useSettingsStore } from '@/stores/settings.store'; import { usePushConnectionStore } from '@/stores/pushConnection.store'; -import { computed } from 'vue'; -import type { CanvasNodeDirtiness } from '@/types'; -import { - AddConnectionCommand, - AddNodeCommand, - BulkCommand, - EnableNodeToggleCommand, - RemoveNodeCommand, - type Undoable, -} from '@/models/history'; -import { useHistoryStore } from '@/stores/history.store'; +import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType }) { const nodeHelpers = useNodeHelpers(); @@ -65,106 +55,7 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType { - // Do not highlight dirtiness if new partial execution is not enabled - if (settingsStore.partialExecutionVersion === 1) { - return {}; - } - - const dirtiness: Record = {}; - const visitedNodes: Set = new Set(); - const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; - - function markDownstreamStaleRecursively(nodeName: string): void { - if (visitedNodes.has(nodeName)) { - return; // prevent infinite recursion - } - - visitedNodes.add(nodeName); - - for (const inputConnections of Object.values( - workflowsStore.outgoingConnectionsByNodeName(nodeName), - )) { - for (const connections of inputConnections) { - for (const { node } of connections ?? []) { - const hasRunData = (runDataByNode[node] ?? []).length > 0; - - if (hasRunData) { - dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; - } - - markDownstreamStaleRecursively(node); - } - } - } - } - - function shouldMarkDirty(command: Undoable, nodeName: string): boolean { - if (command instanceof BulkCommand) { - return command.commands.some((c) => shouldMarkDirty(c, nodeName)); - } - - if (command instanceof AddConnectionCommand) { - return command.connectionData[1]?.node === nodeName; - } - - if (command instanceof RemoveNodeCommand || command instanceof AddNodeCommand) { - return Object.values(workflowsStore.incomingConnectionsByNodeName(nodeName)).some((c) => - c.some((cc) => cc?.some((ccc) => ccc.node === command.node.name)), - ); - } - - if (command instanceof EnableNodeToggleCommand) { - return Object.values(workflowsStore.incomingConnectionsByNodeName(nodeName)).some((c) => - c.some((cc) => cc?.some((ccc) => ccc.node === command.nodeName)), - ); - } - - return false; - } - - for (const node of workflowsStore.allNodes) { - const nodeName = node.name; - const runAt = runDataByNode[nodeName]?.[0]?.startTime ?? 0; - - if (!runAt) { - continue; - } - - const parametersLastUpdate = workflowsStore.getParametersLastUpdate(nodeName) ?? 0; - - if (parametersLastUpdate > runAt) { - dirtiness[nodeName] = 'parameters-updated'; - markDownstreamStaleRecursively(nodeName); - continue; - } - - const incomingConnectionsLastUpdate = Math.max( - 0, - ...historyStore.undoStack.flatMap((command) => - shouldMarkDirty(command, nodeName) ? [command.getTimestamp()] : [], - ), - ); - - if (incomingConnectionsLastUpdate > runAt) { - dirtiness[nodeName] = 'incoming-connections-updated'; - markDownstreamStaleRecursively(nodeName); - continue; - } - - const pinnedDataUpdatedAt = workflowsStore.getPinnedDataLastUpdate(nodeName) ?? 0; - - if (pinnedDataUpdatedAt > runAt) { - dirtiness[nodeName] = 'pinned-data-updated'; - markDownstreamStaleRecursively(nodeName); - continue; - } - } - - return dirtiness; - }); + const { dirtinessByName } = useNodeDirtiness(); // Starts to execute a workflow on server async function runWorkflowApi(runData: IStartRunData): Promise { @@ -322,24 +213,31 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType { - // Find for each start node the source data - let sourceData = get(runData, [name, 0, 'source', 0], null); - if (sourceData === null) { - const parentNodes = workflow.getParentNodes(name, NodeConnectionType.Main, 1); - const executeData = workflowHelpers.executeData( - parentNodes, - name, - NodeConnectionType.Main, - 0, - ); - sourceData = get(executeData, ['source', NodeConnectionType.Main, 0], null); - } - return { - name, - sourceData, - }; - }); + // partial executions must have a destination node + const isPartialExecution = options.destinationNode !== undefined; + const version = settingsStore.partialExecutionVersion; + debugger; + const startNodes: StartNodeData[] = + isPartialExecution && version === 2 + ? [] + : startNodeNames.map((name) => { + // Find for each start node the source data + let sourceData = get(runData, [name, 0, 'source', 0], null); + if (sourceData === null) { + const parentNodes = workflow.getParentNodes(name, NodeConnectionType.Main, 1); + const executeData = workflowHelpers.executeData( + parentNodes, + name, + NodeConnectionType.Main, + 0, + ); + sourceData = get(executeData, ['source', NodeConnectionType.Main, 0], null); + } + return { + name, + sourceData, + }; + }); const singleWebhookTrigger = triggers.find((node) => SINGLE_WEBHOOK_TRIGGERS.includes(node.type), @@ -360,9 +258,6 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType Date: Thu, 13 Feb 2025 17:40:50 +0100 Subject: [PATCH 25/52] add unit tests --- packages/editor-ui/src/Interface.ts | 5 + .../parts/CanvasNodeStatusIcons.vue | 4 +- .../src/composables/useNodeDirtiness.test.ts | 340 +++++++++--------- .../src/composables/useNodeDirtiness.ts | 28 +- 4 files changed, 185 insertions(+), 192 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 08d6dcf20d8bc..e869bb13cd748 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -850,6 +850,11 @@ export interface ITemplatesNode extends IVersionNode { export interface INodeMetadata { parametersLastUpdatedAt?: number; + /** + * UNIX timestamp of either when existing pinned data is modified or removed. + * + * Note that pinning data for the first time isn't supposed to set this field. + */ pinnedDataUpdatedAt?: number; pristine: boolean; } diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index fc9c25308ddc4..d77c7198e54c3 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -24,7 +24,7 @@ const { } = useCanvasNode(); const hideNodeIssues = computed(() => false); // @TODO Implement this -const isStale = computed( +const isParameterChanged = computed( () => render.value.type === CanvasNodeRenderType.Default && render.value.options.dirtiness === 'parameters-updated', @@ -75,7 +75,7 @@ const isStale = computed(
diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index c5d28c0a6e894..099170788f90a 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -1,204 +1,190 @@ -import { createTestNode } from '@/__tests__/mocks'; +/* eslint-disable n8n-local-rules/no-unneeded-backticks */ +import { createTestNode, createTestWorkflow } from '@/__tests__/mocks'; +import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; +import { useNodeHelpers } from '@/composables/useNodeHelpers'; import { useSettingsStore } from '@/stores/settings.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { type FrontendSettings } from '@n8n/api-types'; -import { createTestingPinia } from '@pinia/testing'; -import { type INodeConnections, NodeConnectionType } from 'n8n-workflow'; -import { setActivePinia } from 'pinia'; -import type router from 'vue-router'; - -vi.mock('@/stores/workflows.store', () => ({ - useWorkflowsStore: vi.fn().mockReturnValue({ - allNodes: [], - runWorkflow: vi.fn(), - subWorkflowExecutionError: null, - getWorkflowRunData: null, - setWorkflowExecutionData: vi.fn(), - activeExecutionId: null, - nodesIssuesExist: false, - executionWaitingForWebhook: false, - getCurrentWorkflow: vi.fn().mockReturnValue({ id: '123' }), - getNodeByName: vi.fn(), - getExecution: vi.fn(), - nodeIssuesExit: vi.fn(), - checkIfNodeHasChatParent: vi.fn(), - getParametersLastUpdate: vi.fn(), - getPinnedDataLastUpdate: vi.fn(), - outgoingConnectionsByNodeName: vi.fn(), - }), -})); - -vi.mock('@/stores/pushConnection.store', () => ({ - usePushConnectionStore: vi.fn().mockReturnValue({ - isConnected: true, - }), -})); - -vi.mock('@/composables/useTelemetry', () => ({ - useTelemetry: vi.fn().mockReturnValue({ track: vi.fn() }), -})); - -vi.mock('@/composables/useI18n', () => ({ - useI18n: vi.fn().mockReturnValue({ baseText: vi.fn().mockImplementation((key) => key) }), -})); - -vi.mock('@/composables/useExternalHooks', () => ({ - useExternalHooks: vi.fn().mockReturnValue({ - run: vi.fn(), - }), -})); - -vi.mock('@/composables/useToast', () => ({ - useToast: vi.fn().mockReturnValue({ - clearAllStickyNotifications: vi.fn(), - showMessage: vi.fn(), - showError: vi.fn(), - }), -})); - -vi.mock('@/composables/useWorkflowHelpers', () => ({ - useWorkflowHelpers: vi.fn().mockReturnValue({ - getCurrentWorkflow: vi.fn(), - saveCurrentWorkflow: vi.fn(), - getWorkflowDataToSave: vi.fn(), - setDocumentTitle: vi.fn(), - executeData: vi.fn(), - getNodeTypes: vi.fn().mockReturnValue([]), - }), -})); - -vi.mock('@/composables/useNodeHelpers', () => ({ - useNodeHelpers: vi.fn().mockReturnValue({ - updateNodesExecutionIssues: vi.fn(), - }), -})); - -vi.mock('vue-router', async (importOriginal) => { - const { RouterLink } = await importOriginal(); - return { - RouterLink, - useRouter: vi.fn().mockReturnValue({ - push: vi.fn(), - }), - useRoute: vi.fn(), - }; -}); +import { uniq } from 'lodash-es'; +import { type IConnections, type IRunData, NodeConnectionType } from 'n8n-workflow'; +import { createPinia, setActivePinia } from 'pinia'; +import { type RouteLocationNormalizedLoaded } from 'vue-router'; describe(useNodeDirtiness, () => { let workflowsStore: ReturnType; let settingsStore: ReturnType; - beforeAll(() => { - const pinia = createTestingPinia({ stubActions: false }); - - setActivePinia(pinia); - + beforeEach(() => { + vi.useFakeTimers(); + setActivePinia(createPinia()); workflowsStore = useWorkflowsStore(); settingsStore = useSettingsStore(); - }); - - describe.only('dirtinessByName', () => { - beforeEach(() => { - // Enable new partial execution - settingsStore.settings = { - partialExecution: { version: 2, enforce: true }, - } as FrontendSettings; - - vi.mocked(workflowsStore).getWorkflowRunData = { - node1: [ - { - startTime: +new Date('2025-01-01'), // ran before parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node2: [ - { - startTime: +new Date('2025-01-03'), // ran after parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ], - node3: [], // never ran before - }; - - vi.mocked(workflowsStore).allNodes = [ - createTestNode({ name: 'node1' }), - createTestNode({ name: 'node2' }), - createTestNode({ name: 'node3' }), - ]; - - vi.mocked(workflowsStore).getParametersLastUpdate.mockImplementation( - (nodeName) => - ({ - node1: +new Date('2025-01-02'), - node2: +new Date('2025-01-02'), - node3: +new Date('2025-01-02'), - })[nodeName], - ); - vi.mocked(workflowsStore).getPinnedDataLastUpdate.mockReturnValue(undefined); - }); - - it('should mark nodes with run data older than the last update time as dirty', () => { - function calculateDirtiness(workflow: string) { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation(() => ({})); - const { dirtinessByName } = useNodeDirtiness(); - return dirtinessByName.value; - } + // Enable new partial execution + settingsStore.settings = { + partialExecution: { version: 2, enforce: true }, + } as FrontendSettings; + }); - expect( - calculateDirtiness(` - A* -> B - B -> C - C - `), - ).toMatchInlineSnapshot(` + it('should mark nodes with run data older than the last update time as dirty', async () => { + expect( + await calculateDirtiness({ + workflow: ` + a✅ + b✅ + c✅ + `, + action: () => workflowsStore.setNodeParameters({ name: 'a', value: 1 }), + }), + ).toMatchInlineSnapshot(` { - "node1": "parameters-updated", + "a": "parameters-updated", } `); - }); - - it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', () => { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( - (nodeName) => - ({ - node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, - })[nodeName] ?? ({} as INodeConnections), - ); + }); - const { dirtinessByName } = useNodeDirtiness(); + it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', async () => { + expect( + await calculateDirtiness({ + workflow: ` + a✅ -> b✅ + b -> c✅ + c -> d + `, + action: () => workflowsStore.setNodeParameters({ name: 'b', value: 1 }), + }), + ).toMatchInlineSnapshot(` + { + "b": "parameters-updated", + "c": "upstream-dirty", + } + `); + }); - expect(dirtinessByName.value).toMatchInlineSnapshot(` + it('should return even if the connections forms a loop', async () => { + expect( + await calculateDirtiness({ + workflow: ` + a✅ -> b✅ + b -> c✅ + c -> d + d -> e✅ + e -> b + `, + action: () => workflowsStore.setNodeParameters({ name: 'a', value: 1 }), + }), + ).toMatchInlineSnapshot(` { - "node1": "parameters-updated", - "node2": "upstream-dirty", + "a": "parameters-updated", + "b": "upstream-dirty", + "c": "upstream-dirty", + "e": "upstream-dirty", } `); - }); + }); + + it('should mark downstream nodes of a disabled node dirty', async () => { + expect( + await calculateDirtiness({ + workflow: ` + a✅ -> b✅ + b -> c✅ + `, + action: () => + useNodeHelpers().disableNodes([workflowsStore.nodesByName.b], { trackHistory: true }), + }), + ).toMatchInlineSnapshot(` + { + "c": "incoming-connections-updated", + } + `); + }); - it('should return even if the connections forms a loop', () => { - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( - (nodeName) => - ({ - node1: { main: [[{ node: 'node2', type: NodeConnectionType.Main, index: 0 }]] }, - node2: { main: [[{ node: 'node3', type: NodeConnectionType.Main, index: 0 }]] }, - })[nodeName] ?? ({} as INodeConnections), + it('should restore original dirtiness after undoing a command', async () => { + expect( + await calculateDirtiness({ + workflow: ` + a✅ -> b✅ + b -> c✅ + `, + action: async () => { + useNodeHelpers().disableNodes([workflowsStore.nodesByName.b], { trackHistory: true }); + await useHistoryHelper({} as RouteLocationNormalizedLoaded).undo(); + }, + }), + ).toMatchInlineSnapshot(`{}`); + }); + + async function calculateDirtiness({ + workflow, + action, + }: { workflow: string; action: () => Promise | void }) { + const parsedConnections = workflow + .split('\n') + .filter((line) => line.trim() !== '') + .map((line) => + line.split('->').flatMap((node) => { + const [name, second] = node.trim().split('✅'); + + return name ? [{ name, hasData: second !== undefined }] : []; + }), ); + const nodes = uniq(parsedConnections?.flat()).map(({ name }) => createTestNode({ name })); + const connections = parsedConnections?.reduce((conn, [from, to]) => { + if (!to) { + return conn; + } - const { dirtinessByName } = useNodeDirtiness(); + const conns = conn[from.name]?.[NodeConnectionType.Main]?.[0] ?? []; - expect(dirtinessByName.value).toMatchInlineSnapshot(` - { - "node1": "parameters-updated", - "node2": "upstream-dirty", - } - `); + conn[from.name] = { + ...conn[from.name], + [NodeConnectionType.Main]: [ + [...conns, { node: to.name, type: NodeConnectionType.Main, index: conns.length }], + ...(conn[from.name]?.Main?.slice(1) ?? []), + ], + }; + return conn; + }, {}); + const wf = createTestWorkflow({ nodes, connections }); + + workflowsStore.setNodes(wf.nodes); + workflowsStore.setConnections(wf.connections); + + workflowsStore.setWorkflowExecutionData({ + id: wf.id, + finished: true, + mode: 'manual', + status: 'success', + workflowData: wf, + startedAt: new Date(0), + createdAt: new Date(0), + data: { + resultData: { + runData: nodes.reduce((acc, node) => { + if (parsedConnections.some((c) => c.some((n) => n.name === node.name && n.hasData))) { + acc[node.name] = [ + { + startTime: +new Date('2025-01-01'), // ran before parameter update + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ]; + } + + return acc; + }, {}), + }, + }, }); - }); + + vi.setSystemTime(new Date('2025-01-02')); + await action(); + + const { dirtinessByName } = useNodeDirtiness(); + + return dirtinessByName.value; + } }); diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 5c7ad4e0d1556..64f040dd50964 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -35,12 +35,10 @@ function markDownstreamDirtyRecursively( for (const { node } of connections ?? []) { const hasRunData = (runDataByNode[node] ?? []).length > 0; - if (!hasRunData || dirtiness[node] !== undefined) { - continue; + if (hasRunData) { + dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; } - dirtiness[node] = 'upstream-dirty'; - markDownstreamDirtyRecursively( node, dirtiness, @@ -56,7 +54,7 @@ function markDownstreamDirtyRecursively( /** * Does the command make the given node dirty? */ -function shouldMarkDirty( +function shouldCommandMarkDirty( command: Undoable, nodeName: string, nodeLastRanAt: number, @@ -68,7 +66,7 @@ function shouldMarkDirty( if (command instanceof BulkCommand) { return command.commands.some((cmd) => - shouldMarkDirty(cmd, nodeName, nodeLastRanAt, getIncomingConnections), + shouldCommandMarkDirty(cmd, nodeName, nodeLastRanAt, getIncomingConnections), ); } @@ -102,7 +100,7 @@ function shouldMarkDirty( } /** - * Determines the subgraph that is affected by the change made after the last execution + * Determines the subgraph that is affected by changes made after the last (partial) execution */ export function useNodeDirtiness() { const historyStore = useHistoryStore(); @@ -129,6 +127,15 @@ export function useNodeDirtiness() { ); } + function shouldMarkDirty(command: Undoable, nodeName: string, nodeLastRanAt: number) { + return shouldCommandMarkDirty( + command, + nodeName, + nodeLastRanAt, + workflowsStore.incomingConnectionsByNodeName, + ); + } + for (const node of workflowsStore.allNodes) { const nodeName = node.name; const runAt = runDataByNode[nodeName]?.[0]?.startTime ?? 0; @@ -145,12 +152,7 @@ export function useNodeDirtiness() { continue; } - if ( - runAt && - historyStore.undoStack.some((command) => - shouldMarkDirty(command, nodeName, runAt, workflowsStore.incomingConnectionsByNodeName), - ) - ) { + if (historyStore.undoStack.some((command) => shouldMarkDirty(command, nodeName, runAt))) { dirtiness[nodeName] = 'incoming-connections-updated'; markDownstreamDirty(nodeName); continue; From 37c604aef6fa4288ce2a1eda2cda0df6d834ca4c Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 14 Feb 2025 14:28:46 +0100 Subject: [PATCH 26/52] add more tests --- .../src/composables/useNodeDirtiness.test.ts | 433 ++++++++++++------ .../src/composables/useRunWorkflow.ts | 1 - 2 files changed, 297 insertions(+), 137 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 099170788f90a..ff7d54b8fa44a 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -1,190 +1,351 @@ /* eslint-disable n8n-local-rules/no-unneeded-backticks */ -import { createTestNode, createTestWorkflow } from '@/__tests__/mocks'; +import { createTestNode, createTestWorkflow, defaultNodeDescriptions } from '@/__tests__/mocks'; +import { createComponentRenderer } from '@/__tests__/render'; +import { useCanvasOperations } from '@/composables/useCanvasOperations'; import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; -import { useNodeHelpers } from '@/composables/useNodeHelpers'; +import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useSettingsStore } from '@/stores/settings.store'; +import { useUIStore } from '@/stores/ui.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { type FrontendSettings } from '@n8n/api-types'; -import { uniq } from 'lodash-es'; -import { type IConnections, type IRunData, NodeConnectionType } from 'n8n-workflow'; -import { createPinia, setActivePinia } from 'pinia'; -import { type RouteLocationNormalizedLoaded } from 'vue-router'; +import { createTestingPinia } from '@pinia/testing'; +import { uniqBy } from 'lodash-es'; +import { NodeConnectionType, type IConnections, type IRunData } from 'n8n-workflow'; +import { defineComponent } from 'vue'; +import { + createRouter, + createWebHistory, + useRouter, + type RouteLocationNormalizedLoaded, +} from 'vue-router'; describe(useNodeDirtiness, () => { let workflowsStore: ReturnType; let settingsStore: ReturnType; + let historyHelper: ReturnType; + let canvasOperations: ReturnType; + let uiStore: ReturnType; beforeEach(() => { vi.useFakeTimers(); - setActivePinia(createPinia()); - workflowsStore = useWorkflowsStore(); - settingsStore = useSettingsStore(); - - // Enable new partial execution - settingsStore.settings = { - partialExecution: { version: 2, enforce: true }, - } as FrontendSettings; + + const TestComponent = defineComponent({ + setup() { + workflowsStore = useWorkflowsStore(); + settingsStore = useSettingsStore(); + historyHelper = useHistoryHelper({} as RouteLocationNormalizedLoaded); + canvasOperations = useCanvasOperations({ router: useRouter() }); + uiStore = useUIStore(); + + // Enable new partial execution + settingsStore.settings = { + partialExecution: { version: 2, enforce: true }, + } as FrontendSettings; + }, + template: '
', + }); + + createComponentRenderer(TestComponent, { + global: { + plugins: [ + createTestingPinia({ stubActions: false, fakeApp: true }), + createRouter({ + history: createWebHistory(), + routes: [{ path: '/', component: TestComponent }], + }), + ], + }, + })(); }); - it('should mark nodes with run data older than the last update time as dirty', async () => { - expect( - await calculateDirtiness({ - workflow: ` - a✅ - b✅ - c✅ - `, - action: () => workflowsStore.setNodeParameters({ name: 'a', value: 1 }), - }), - ).toMatchInlineSnapshot(` - { - "a": "parameters-updated", - } - `); + it('should be an empty object if no change has been made to the workflow', () => { + setupTestWorkflow('a✅, b✅, c✅'); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({}); }); - it('should mark nodes with a dirty node somewhere in its upstream as upstream-dirty', async () => { - expect( - await calculateDirtiness({ - workflow: ` - a✅ -> b✅ - b -> c✅ - c -> d - `, - action: () => workflowsStore.setNodeParameters({ name: 'b', value: 1 }), - }), - ).toMatchInlineSnapshot(` - { - "b": "parameters-updated", - "c": "upstream-dirty", - } - `); + it('should return even if the connections forms a loop', () => { + setupTestWorkflow('a✅ -> b✅ -> c -> d✅ -> b'); + + canvasOperations.setNodeParameters('b', { foo: 1 }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + b: 'parameters-updated', + d: 'upstream-dirty', + }); }); - it('should return even if the connections forms a loop', async () => { - expect( - await calculateDirtiness({ - workflow: ` - a✅ -> b✅ - b -> c✅ - c -> d - d -> e✅ - e -> b - `, - action: () => workflowsStore.setNodeParameters({ name: 'a', value: 1 }), - }), - ).toMatchInlineSnapshot(` - { - "a": "parameters-updated", - "b": "upstream-dirty", - "c": "upstream-dirty", - "e": "upstream-dirty", - } - `); + describe('injecting a node', () => { + it("should mark a node as 'incoming-connections-updated' if a new node is injected as its parent", async () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a✅ -> b✅'); + + uiStore.lastInteractedWithNodeConnection = { + source: 'a', + target: 'b', + }; + uiStore.lastInteractedWithNodeId = 'a'; + uiStore.lastInteractedWithNodeHandle = 'outputs/main/0'; + + await canvasOperations.addNodes([createTestNode({ name: 'c' })], { trackHistory: true }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + b: 'incoming-connections-updated', + }); + }); }); - it('should mark downstream nodes of a disabled node dirty', async () => { - expect( - await calculateDirtiness({ - workflow: ` - a✅ -> b✅ - b -> c✅ - `, - action: () => - useNodeHelpers().disableNodes([workflowsStore.nodesByName.b], { trackHistory: true }), - }), - ).toMatchInlineSnapshot(` - { - "c": "incoming-connections-updated", - } - `); + describe('deleting a node', () => { + it("should mark a node as 'incoming-connections-updated' if parent node is deleted", async () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a✅ -> b✅ -> c✅'); + + canvasOperations.deleteNodes(['b'], { trackHistory: true }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + c: 'incoming-connections-updated', + }); + }); }); - it('should restore original dirtiness after undoing a command', async () => { - expect( - await calculateDirtiness({ - workflow: ` - a✅ -> b✅ - b -> c✅ - `, - action: async () => { - useNodeHelpers().disableNodes([workflowsStore.nodesByName.b], { trackHistory: true }); - await useHistoryHelper({} as RouteLocationNormalizedLoaded).undo(); - }, - }), - ).toMatchInlineSnapshot(`{}`); + describe('updating node parameters', () => { + it("should mark a node as 'parameters-updated' if its parameter has changed", () => { + setupTestWorkflow('a✅, b✅, c✅'); + + canvasOperations.setNodeParameters('b', { foo: 1 }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + b: 'parameters-updated', + }); + }); + + it("should mark all downstream nodes with data as 'upstream-dirty' if a node upstream got an updated parameter", () => { + setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅ -> e -> f✅'); + + canvasOperations.setNodeParameters('b', { foo: 1 }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + b: 'parameters-updated', + c: 'upstream-dirty', + d: 'upstream-dirty', + f: 'upstream-dirty', + }); + }); + }); + + describe('adding a connection', () => { + it("should mark a node as 'incoming-connections-updated' if a new incoming connection is added", () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a✅ -> b✅ -> c✅'); + + canvasOperations.createConnection({ source: 'a', target: 'c' }, { trackHistory: true }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + c: 'incoming-connections-updated', + }); + }); }); - async function calculateDirtiness({ - workflow, - action, - }: { workflow: string; action: () => Promise | void }) { - const parsedConnections = workflow - .split('\n') + describe('enabling/disabling nodes', () => { + it('should mark downstream nodes dirty if the node is set to disabled', () => { + setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅'); + + canvasOperations.toggleNodesDisabled(['b'], { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + c: 'incoming-connections-updated', + d: 'upstream-dirty', + }); + }); + + it('should mark downstream nodes dirty if the node is set to enabled', () => { + setupTestWorkflow('a✅ -> b🚫 -> c✅ -> d✅'); + + canvasOperations.toggleNodesDisabled(['b'], { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + c: 'incoming-connections-updated', + d: 'upstream-dirty', + }); + }); + + it('should restore original dirtiness after undoing a command', async () => { + setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅'); + + canvasOperations.toggleNodesDisabled(['b'], { + trackHistory: true, + }); + await historyHelper.undo(); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); + }); + }); + + describe('pinned data', () => { + it('should not change dirtiness when data is pinned', async () => { + setupTestWorkflow('a✅ -> b✅ -> c✅'); + + canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); + }); + + it('should update dirtiness when pinned data is removed from a node with run data', async () => { + setupTestWorkflow('a✅ -> b✅📌 -> c✅'); + + canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'pinned-data-updated', + c: 'upstream-dirty', + }); + }); + + // TODO: is this a real scenario? + it.todo( + "should update dirtiness when pinned data is removed from a node which hasn't run", + async () => { + setupTestWorkflow('a✅ -> b📌 -> c✅'); + + canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'pinned-data-updated', + c: 'upstream-dirty', + }); + }, + ); + + it('should update dirtiness when an existing pinned data is updated', async () => { + setupTestWorkflow('a✅ -> b✅📌 -> c✅'); + + workflowsStore.pinData({ node: workflowsStore.nodesByName.b, data: [{ json: {} }] }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'pinned-data-updated', + c: 'upstream-dirty', + }); + }); + }); + + function setupTestWorkflow(diagram: string) { + interface NodeSpec { + name: string; + disabled: boolean; + hasData: boolean; + isPinned: boolean; + } + + const lines = diagram + .split(/\n|,/) .filter((line) => line.trim() !== '') .map((line) => - line.split('->').flatMap((node) => { - const [name, second] = node.trim().split('✅'); + line.split('->').flatMap((node) => { + const [name, ...attributes] = node.trim(); - return name ? [{ name, hasData: second !== undefined }] : []; + return name + ? [ + { + id: name, + name, + hasData: attributes.includes('✅'), + disabled: attributes.includes('🚫'), + isPinned: attributes.includes('📌'), + }, + ] + : []; }), ); - const nodes = uniq(parsedConnections?.flat()).map(({ name }) => createTestNode({ name })); - const connections = parsedConnections?.reduce((conn, [from, to]) => { - if (!to) { - return conn; - } + const nodes = uniqBy(lines?.flat() ?? [], ({ name }) => name).map(createTestNode); + const connections = lines?.reduce((conn, nodesInLine) => { + for (let i = 0; i < nodesInLine.length - 1; i++) { + const from = nodesInLine[i]; + const to = nodesInLine[i + 1]; - const conns = conn[from.name]?.[NodeConnectionType.Main]?.[0] ?? []; + const conns = conn[from.name]?.[NodeConnectionType.Main]?.[0] ?? []; - conn[from.name] = { - ...conn[from.name], - [NodeConnectionType.Main]: [ - [...conns, { node: to.name, type: NodeConnectionType.Main, index: conns.length }], - ...(conn[from.name]?.Main?.slice(1) ?? []), - ], - }; + conn[from.name] = { + ...conn[from.name], + [NodeConnectionType.Main]: [ + [...conns, { node: to.name, type: NodeConnectionType.Main, index: conns.length }], + ...(conn[from.name]?.Main?.slice(1) ?? []), + ], + }; + } return conn; }, {}); - const wf = createTestWorkflow({ nodes, connections }); + const workflow = createTestWorkflow({ nodes, connections }); - workflowsStore.setNodes(wf.nodes); - workflowsStore.setConnections(wf.connections); + workflowsStore.setNodes(workflow.nodes); + workflowsStore.setConnections(workflow.connections); + + for (const node of lines.flat()) { + if (node.isPinned) { + workflowsStore.pinData({ + node: workflowsStore.nodesByName[node.name], + data: [{ json: {} }], + }); + } + } workflowsStore.setWorkflowExecutionData({ - id: wf.id, + id: workflow.id, finished: true, mode: 'manual', status: 'success', - workflowData: wf, + workflowData: workflow, startedAt: new Date(0), createdAt: new Date(0), data: { resultData: { - runData: nodes.reduce((acc, node) => { - if (parsedConnections.some((c) => c.some((n) => n.name === node.name && n.hasData))) { - acc[node.name] = [ - { - startTime: +new Date('2025-01-01'), // ran before parameter update - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ]; + runData: workflow.nodes.reduce((acc, node) => { + if (!lines.some((c) => c.some((n) => n.name === node.name && n.hasData))) { + return acc; } + acc[node.name] = [ + { + startTime: +new Date('2025-01-01'), + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ]; + return acc; }, {}), }, }, }); - vi.setSystemTime(new Date('2025-01-02')); - await action(); - - const { dirtinessByName } = useNodeDirtiness(); - - return dirtinessByName.value; + vi.setSystemTime(new Date('2025-01-02')); // after execution } }); diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index 67173e316bcdb..d922834aba781 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -216,7 +216,6 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType Date: Fri, 14 Feb 2025 17:08:48 +0100 Subject: [PATCH 27/52] make dirty check work for sub nodes --- .../src/composables/useNodeDirtiness.test.ts | 159 ++++++++++-------- .../src/composables/useNodeDirtiness.ts | 18 +- 2 files changed, 99 insertions(+), 78 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index ff7d54b8fa44a..444b53af5c5d8 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -4,13 +4,13 @@ import { createComponentRenderer } from '@/__tests__/render'; import { useCanvasOperations } from '@/composables/useCanvasOperations'; import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; +import { type INodeUi } from '@/Interface'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useSettingsStore } from '@/stores/settings.store'; import { useUIStore } from '@/stores/ui.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { type FrontendSettings } from '@n8n/api-types'; import { createTestingPinia } from '@pinia/testing'; -import { uniqBy } from 'lodash-es'; import { NodeConnectionType, type IConnections, type IRunData } from 'n8n-workflow'; import { defineComponent } from 'vue'; import { @@ -132,6 +132,16 @@ describe(useNodeDirtiness, () => { }); }); + it("should not update dirtiness if the node hasn't run yet", () => { + setupTestWorkflow('a✅, b, c✅'); + + canvasOperations.setNodeParameters('b', { foo: 1 }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({}); + }); + it("should mark all downstream nodes with data as 'upstream-dirty' if a node upstream got an updated parameter", () => { setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅ -> e -> f✅'); @@ -256,64 +266,88 @@ describe(useNodeDirtiness, () => { }); }); + describe('sub-nodes', () => { + it('should mark its root node as dirty when parameters of a sub node has changed', () => { + setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b, e🧠 -> f🧠 -> b'); + + canvasOperations.setNodeParameters('e', { foo: 1 }); + + const { dirtinessByName } = useNodeDirtiness(); + + expect(dirtinessByName.value).toEqual({ + b: 'upstream-dirty', + c: 'upstream-dirty', + e: 'parameters-updated', + }); + }); + }); + function setupTestWorkflow(diagram: string) { - interface NodeSpec { - name: string; - disabled: boolean; - hasData: boolean; - isPinned: boolean; + const nodeNamesWithPinnedData = new Set(); + const nodes: Record = {}; + const connections: IConnections = {}; + const runData: IRunData = {}; + + for (const subGraph of diagram.split(/\n|,/).filter((line) => line.trim() !== '')) { + const elements = subGraph.split(/(->)/).map((s) => s.trim()); + + elements.forEach((element, i, arr) => { + if (element === '->') { + const from = arr[i - 1].slice(0, 1); + const to = arr[i + 1].slice(0, 1); + const type = arr[i - 1].includes('🧠') + ? NodeConnectionType.AiAgent + : NodeConnectionType.Main; + + const conns = connections[from]?.[type]?.[0] ?? []; + + connections[from] = { + ...connections[from], + [type]: [ + [...conns, { node: to, type, index: conns.length }], + ...(connections[from]?.Main?.slice(1) ?? []), + ], + }; + return; + } + + const [name, ...attributes] = element.trim(); + + nodes[name] = + nodes[name] ?? + createTestNode({ + id: name, + name, + disabled: attributes.includes('🚫'), + }); + + if (attributes.includes('✅')) { + runData[name] = [ + { + startTime: +new Date('2025-01-01'), + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ]; + } + + if (attributes.includes('📌')) { + nodeNamesWithPinnedData.add(name); + } + }); } - const lines = diagram - .split(/\n|,/) - .filter((line) => line.trim() !== '') - .map((line) => - line.split('->').flatMap((node) => { - const [name, ...attributes] = node.trim(); - - return name - ? [ - { - id: name, - name, - hasData: attributes.includes('✅'), - disabled: attributes.includes('🚫'), - isPinned: attributes.includes('📌'), - }, - ] - : []; - }), - ); - const nodes = uniqBy(lines?.flat() ?? [], ({ name }) => name).map(createTestNode); - const connections = lines?.reduce((conn, nodesInLine) => { - for (let i = 0; i < nodesInLine.length - 1; i++) { - const from = nodesInLine[i]; - const to = nodesInLine[i + 1]; - - const conns = conn[from.name]?.[NodeConnectionType.Main]?.[0] ?? []; - - conn[from.name] = { - ...conn[from.name], - [NodeConnectionType.Main]: [ - [...conns, { node: to.name, type: NodeConnectionType.Main, index: conns.length }], - ...(conn[from.name]?.Main?.slice(1) ?? []), - ], - }; - } - return conn; - }, {}); - const workflow = createTestWorkflow({ nodes, connections }); + const workflow = createTestWorkflow({ nodes: Object.values(nodes), connections }); workflowsStore.setNodes(workflow.nodes); workflowsStore.setConnections(workflow.connections); - for (const node of lines.flat()) { - if (node.isPinned) { - workflowsStore.pinData({ - node: workflowsStore.nodesByName[node.name], - data: [{ json: {} }], - }); - } + for (const name of nodeNamesWithPinnedData) { + workflowsStore.pinData({ + node: workflowsStore.nodesByName[name], + data: [{ json: {} }], + }); } workflowsStore.setWorkflowExecutionData({ @@ -324,26 +358,7 @@ describe(useNodeDirtiness, () => { workflowData: workflow, startedAt: new Date(0), createdAt: new Date(0), - data: { - resultData: { - runData: workflow.nodes.reduce((acc, node) => { - if (!lines.some((c) => c.some((n) => n.name === node.name && n.hasData))) { - return acc; - } - - acc[node.name] = [ - { - startTime: +new Date('2025-01-01'), - executionTime: 0, - executionStatus: 'success', - source: [], - }, - ]; - - return acc; - }, {}), - }, - }, + data: { resultData: { runData } }, }); vi.setSystemTime(new Date('2025-01-02')); // after execution diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 64f040dd50964..eec3a551c3ddf 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -26,11 +26,7 @@ function markDownstreamDirtyRecursively( visitedNodes.add(nodeName); - for (const [type, inputConnections] of Object.entries(getOutgoingConnections(nodeName))) { - if ((type as NodeConnectionType) !== NodeConnectionType.Main) { - continue; - } - + for (const inputConnections of Object.values(getOutgoingConnections(nodeName))) { for (const connections of inputConnections) { for (const { node } of connections ?? []) { const hasRunData = (runDataByNode[node] ?? []).length > 0; @@ -136,9 +132,19 @@ export function useNodeDirtiness() { ); } + function getRunAt(nodeName: string): number { + return Math.max( + ...Object.entries(workflowsStore.outgoingConnectionsByNodeName(nodeName)) + .filter(([type]) => (type as NodeConnectionType) !== NodeConnectionType.Main) + .flatMap(([, conn]) => conn.flat()) + .map((conn) => (conn ? getRunAt(conn.node) : 0)), + runDataByNode[nodeName]?.[0]?.startTime ?? 0, + ); + } + for (const node of workflowsStore.allNodes) { const nodeName = node.name; - const runAt = runDataByNode[nodeName]?.[0]?.startTime ?? 0; + const runAt = getRunAt(nodeName); if (!runAt) { continue; From 8b1c7d8f1ef0b92fc58bd62a82f14f3974a7597e Mon Sep 17 00:00:00 2001 From: autologie Date: Mon, 17 Feb 2025 14:46:53 +0100 Subject: [PATCH 28/52] refactor test --- .../src/composables/useNodeDirtiness.test.ts | 53 ++++++------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 444b53af5c5d8..b575fcc75c737 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -62,22 +62,18 @@ describe(useNodeDirtiness, () => { it('should be an empty object if no change has been made to the workflow', () => { setupTestWorkflow('a✅, b✅, c✅'); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({}); + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); }); it('should return even if the connections forms a loop', () => { setupTestWorkflow('a✅ -> b✅ -> c -> d✅ -> b'); - canvasOperations.setNodeParameters('b', { foo: 1 }); - - const { dirtinessByName } = useNodeDirtiness(); + expect(() => { + canvasOperations.setNodeParameters('b', { foo: 1 }); - expect(dirtinessByName.value).toEqual({ - b: 'parameters-updated', - d: 'upstream-dirty', - }); + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + useNodeDirtiness().dirtinessByName.value; + }).not.toThrow(); }); describe('injecting a node', () => { @@ -95,9 +91,7 @@ describe(useNodeDirtiness, () => { await canvasOperations.addNodes([createTestNode({ name: 'c' })], { trackHistory: true }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'incoming-connections-updated', }); }); @@ -111,9 +105,7 @@ describe(useNodeDirtiness, () => { canvasOperations.deleteNodes(['b'], { trackHistory: true }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ c: 'incoming-connections-updated', }); }); @@ -125,9 +117,7 @@ describe(useNodeDirtiness, () => { canvasOperations.setNodeParameters('b', { foo: 1 }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'parameters-updated', }); }); @@ -137,9 +127,7 @@ describe(useNodeDirtiness, () => { canvasOperations.setNodeParameters('b', { foo: 1 }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({}); + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); }); it("should mark all downstream nodes with data as 'upstream-dirty' if a node upstream got an updated parameter", () => { @@ -147,9 +135,7 @@ describe(useNodeDirtiness, () => { canvasOperations.setNodeParameters('b', { foo: 1 }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'parameters-updated', c: 'upstream-dirty', d: 'upstream-dirty', @@ -166,9 +152,7 @@ describe(useNodeDirtiness, () => { canvasOperations.createConnection({ source: 'a', target: 'c' }, { trackHistory: true }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ c: 'incoming-connections-updated', }); }); @@ -272,9 +256,7 @@ describe(useNodeDirtiness, () => { canvasOperations.setNodeParameters('e', { foo: 1 }); - const { dirtinessByName } = useNodeDirtiness(); - - expect(dirtinessByName.value).toEqual({ + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'upstream-dirty', c: 'upstream-dirty', e: 'parameters-updated', @@ -298,15 +280,12 @@ describe(useNodeDirtiness, () => { const type = arr[i - 1].includes('🧠') ? NodeConnectionType.AiAgent : NodeConnectionType.Main; - - const conns = connections[from]?.[type]?.[0] ?? []; + const conns = connections[from]?.[type] ?? []; + const conn = conns[0] ?? []; connections[from] = { ...connections[from], - [type]: [ - [...conns, { node: to, type, index: conns.length }], - ...(connections[from]?.Main?.slice(1) ?? []), - ], + [type]: [[...conn, { node: to, type, index: conn.length }], ...conns.slice(1)], }; return; } From 0d518c92dc2e5bf1887e2299f9c50132cab318f1 Mon Sep 17 00:00:00 2001 From: autologie Date: Mon, 17 Feb 2025 14:48:10 +0100 Subject: [PATCH 29/52] fix: output should not become dirty just by opening NDV --- .../components/ResourceMapper/ResourceMapper.vue | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue b/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue index f385842683fbf..f731d155e8ef1 100644 --- a/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue +++ b/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue @@ -29,6 +29,7 @@ import { useNDVStore } from '@/stores/ndv.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { useDocumentVisibility } from '@/composables/useDocumentVisibility'; import { N8nButton, N8nCallout, N8nNotice } from 'n8n-design-system'; +import { isEqual } from 'lodash-es'; type Props = { parameter: INodeProperties; @@ -354,11 +355,14 @@ async function loadAndSetFieldsToMap(): Promise { } return field; }); - state.paramValue = { - ...state.paramValue, - schema: newSchema, - }; - emitValueChanged(); + + if (!isEqual(newSchema, state.paramValue.schema)) { + state.paramValue = { + ...state.paramValue, + schema: newSchema, + }; + emitValueChanged(); + } } } From f23e2930d8223d801c466271f463de9702fedd81 Mon Sep 17 00:00:00 2001 From: autologie Date: Mon, 17 Feb 2025 14:52:27 +0100 Subject: [PATCH 30/52] fix: should not crash if metadata for node does not exist --- packages/editor-ui/src/stores/workflows.store.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index afbb9a2d327d3..bb46fc77bac19 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -803,7 +803,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { payload.data = [payload.data]; } - if ((workflow.value.pinData?.[nodeName] ?? []).length > 0) { + if ((workflow.value.pinData?.[nodeName] ?? []).length > 0 && nodeMetadata.value[nodeName]) { // Updating existing pinned data nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); } @@ -838,7 +838,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { ...workflow.value, pinData, }; - nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); + + if (nodeMetadata.value[nodeName]) { + nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); + } uiStore.stateIsDirty = true; updateCachedWorkflow(); From b05dc6d5fb7eb36be2c0eb74576d1b7190c94dd0 Mon Sep 17 00:00:00 2001 From: autologie Date: Mon, 17 Feb 2025 16:10:55 +0100 Subject: [PATCH 31/52] fix: warning icon is missing in stale output panel of an AI model node --- packages/editor-ui/src/components/OutputPanel.vue | 11 ++++++++++- packages/editor-ui/src/stores/settings.store.ts | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index ac13a67c950a9..1dfb68f340fda 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -21,6 +21,8 @@ import { useTelemetry } from '@/composables/useTelemetry'; import { useI18n } from '@/composables/useI18n'; import { waitingNodeTooltip } from '@/utils/executionUtils'; import { N8nRadioButtons, N8nText } from 'n8n-design-system'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; // Types @@ -75,6 +77,8 @@ const uiStore = useUIStore(); const telemetry = useTelemetry(); const i18n = useI18n(); const { activeNode } = storeToRefs(ndvStore); +const settings = useSettingsStore(); +const { dirtinessByName } = useNodeDirtiness(); // Composables @@ -201,6 +205,11 @@ const staleData = computed(() => { if (!node.value) { return false; } + + if (settings.partialExecutionVersion === 2) { + return dirtinessByName.value[node.value.name] === 'parameters-updated'; + } + const updatedAt = workflowsStore.getParametersLastUpdate(node.value.name); if (!updatedAt || !runTaskData.value) { return false; @@ -352,7 +361,7 @@ const activatePane = () => { {{ i18n.baseText(outputPanelEditMode.enabled ? 'ndv.output.edit' : 'ndv.output') }} { const isCloudDeployment = computed(() => settings.value.deployment?.type === 'cloud'); - const partialExecutionVersion = computed(() => { + const partialExecutionVersion = computed<1 | 2>(() => { const defaultVersion = settings.value.partialExecution?.version ?? 1; const enforceVersion = settings.value.partialExecution?.enforce ?? false; // -1 means we pick the defaultVersion @@ -118,7 +118,7 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { return 1; } - return version; + return version as 1 | 2; }); const isAiCreditsEnabled = computed(() => settings.value.aiCredits?.enabled); From 13c8d1e4349ade873bf86840b9c68fed4c9512d1 Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 18 Feb 2025 10:54:14 +0100 Subject: [PATCH 32/52] try different logic for highlighting dirty nodes --- .../editor-ui/src/composables/useNodeDirtiness.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index eec3a551c3ddf..9df09470a8d74 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -34,14 +34,6 @@ function markDownstreamDirtyRecursively( if (hasRunData) { dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; } - - markDownstreamDirtyRecursively( - node, - dirtiness, - visitedNodes, - runDataByNode, - getOutgoingConnections, - ); } } } @@ -154,13 +146,11 @@ export function useNodeDirtiness() { if (parametersLastUpdate > runAt) { dirtiness[nodeName] = 'parameters-updated'; - markDownstreamDirty(nodeName); continue; } if (historyStore.undoStack.some((command) => shouldMarkDirty(command, nodeName, runAt))) { dirtiness[nodeName] = 'incoming-connections-updated'; - markDownstreamDirty(nodeName); continue; } @@ -168,7 +158,6 @@ export function useNodeDirtiness() { if (pinnedDataUpdatedAt > runAt) { dirtiness[nodeName] = 'pinned-data-updated'; - markDownstreamDirty(nodeName); continue; } } From d8f14c31169339266e16e1aa363918451ecce4ba Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 18 Feb 2025 15:26:28 +0100 Subject: [PATCH 33/52] fix: send startNodes for now --- .../src/composables/useRunWorkflow.ts | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/packages/editor-ui/src/composables/useRunWorkflow.ts b/packages/editor-ui/src/composables/useRunWorkflow.ts index d922834aba781..9b069aac11374 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.ts @@ -216,27 +216,26 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType { - // Find for each start node the source data - let sourceData = get(runData, [name, 0, 'source', 0], null); - if (sourceData === null) { - const parentNodes = workflow.getParentNodes(name, NodeConnectionType.Main, 1); - const executeData = workflowHelpers.executeData( - parentNodes, - name, - NodeConnectionType.Main, - 0, - ); - sourceData = get(executeData, ['source', NodeConnectionType.Main, 0], null); - } - return { - name, - sourceData, - }; - }); + + // TODO: this will be redundant once we cleanup the partial execution v1 + const startNodes: StartNodeData[] = startNodeNames.map((name) => { + // Find for each start node the source data + let sourceData = get(runData, [name, 0, 'source', 0], null); + if (sourceData === null) { + const parentNodes = workflow.getParentNodes(name, NodeConnectionType.Main, 1); + const executeData = workflowHelpers.executeData( + parentNodes, + name, + NodeConnectionType.Main, + 0, + ); + sourceData = get(executeData, ['source', NodeConnectionType.Main, 0], null); + } + return { + name, + sourceData, + }; + }); const singleWebhookTrigger = triggers.find((node) => SINGLE_WEBHOOK_TRIGGERS.includes(node.type), From 90e666ea1291977a0246c62bf389e3fc7589dd85 Mon Sep 17 00:00:00 2001 From: autologie Date: Tue, 18 Feb 2025 15:27:12 +0100 Subject: [PATCH 34/52] change how dirtiness is shown --- .../parts/CanvasNodeStatusIcons.vue | 10 ++--- .../src/composables/useNodeDirtiness.test.ts | 20 ---------- .../src/composables/useNodeDirtiness.ts | 39 +------------------ packages/editor-ui/src/plugins/icons/index.ts | 2 + packages/editor-ui/src/types/canvas.ts | 3 +- 5 files changed, 8 insertions(+), 66 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index d77c7198e54c3..c272b2ce4ce00 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -24,10 +24,8 @@ const { } = useCanvasNode(); const hideNodeIssues = computed(() => false); // @TODO Implement this -const isParameterChanged = computed( - () => - render.value.type === CanvasNodeRenderType.Default && - render.value.options.dirtiness === 'parameters-updated', +const dirtiness = computed(() => + render.value.type === CanvasNodeRenderType.Default ? render.value.options.dirtiness : undefined, ); @@ -75,7 +73,7 @@ const isParameterChanged = computed(
@@ -83,7 +81,7 @@ const isParameterChanged = computed( - +
{ expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); }); - - it("should mark all downstream nodes with data as 'upstream-dirty' if a node upstream got an updated parameter", () => { - setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅ -> e -> f✅'); - - canvasOperations.setNodeParameters('b', { foo: 1 }); - - expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - b: 'parameters-updated', - c: 'upstream-dirty', - d: 'upstream-dirty', - f: 'upstream-dirty', - }); - }); }); describe('adding a connection', () => { @@ -168,7 +155,6 @@ describe(useNodeDirtiness, () => { expect(useNodeDirtiness().dirtinessByName.value).toEqual({ c: 'incoming-connections-updated', - d: 'upstream-dirty', }); }); @@ -181,7 +167,6 @@ describe(useNodeDirtiness, () => { expect(useNodeDirtiness().dirtinessByName.value).toEqual({ c: 'incoming-connections-updated', - d: 'upstream-dirty', }); }); @@ -217,7 +202,6 @@ describe(useNodeDirtiness, () => { expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'pinned-data-updated', - c: 'upstream-dirty', }); }); @@ -233,7 +217,6 @@ describe(useNodeDirtiness, () => { expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'pinned-data-updated', - c: 'upstream-dirty', }); }, ); @@ -245,7 +228,6 @@ describe(useNodeDirtiness, () => { expect(useNodeDirtiness().dirtinessByName.value).toEqual({ b: 'pinned-data-updated', - c: 'upstream-dirty', }); }); }); @@ -257,8 +239,6 @@ describe(useNodeDirtiness, () => { canvasOperations.setNodeParameters('e', { foo: 1 }); expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - b: 'upstream-dirty', - c: 'upstream-dirty', e: 'parameters-updated', }); }); diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 9df09470a8d74..04fe3bbe07923 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -10,35 +10,9 @@ import { useHistoryStore } from '@/stores/history.store'; import { useSettingsStore } from '@/stores/settings.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { type CanvasNodeDirtiness } from '@/types'; -import { type ITaskData, type INodeConnections, NodeConnectionType } from 'n8n-workflow'; +import { type INodeConnections, NodeConnectionType } from 'n8n-workflow'; import { computed } from 'vue'; -function markDownstreamDirtyRecursively( - nodeName: string, - dirtiness: Record, - visitedNodes: Set, - runDataByNode: Record, - getOutgoingConnections: (nodeName: string) => INodeConnections, -): void { - if (visitedNodes.has(nodeName)) { - return; // prevent infinite recursion - } - - visitedNodes.add(nodeName); - - for (const inputConnections of Object.values(getOutgoingConnections(nodeName))) { - for (const connections of inputConnections) { - for (const { node } of connections ?? []) { - const hasRunData = (runDataByNode[node] ?? []).length > 0; - - if (hasRunData) { - dirtiness[node] = dirtiness[node] ?? 'upstream-dirty'; - } - } - } - } -} - /** * Does the command make the given node dirty? */ @@ -102,19 +76,8 @@ export function useNodeDirtiness() { } const dirtiness: Record = {}; - const visitedNodes: Set = new Set(); const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; - function markDownstreamDirty(nodeName: string) { - markDownstreamDirtyRecursively( - nodeName, - dirtiness, - visitedNodes, - runDataByNode, - workflowsStore.outgoingConnectionsByNodeName, - ); - } - function shouldMarkDirty(command: Undoable, nodeName: string, nodeLastRanAt: number) { return shouldCommandMarkDirty( command, diff --git a/packages/editor-ui/src/plugins/icons/index.ts b/packages/editor-ui/src/plugins/icons/index.ts index 18e8113fca4ec..b662d3df03c04 100644 --- a/packages/editor-ui/src/plugins/icons/index.ts +++ b/packages/editor-ui/src/plugins/icons/index.ts @@ -21,6 +21,7 @@ import { faBoxOpen, faBug, faBrain, + faCircle, faCalculator, faCalendar, faChartBar, @@ -198,6 +199,7 @@ export const FontAwesomePlugin: Plugin = { addIcon(faBrain); addIcon(faCalculator); addIcon(faCalendar); + addIcon(faCircle); addIcon(faChartBar); addIcon(faCheck); addIcon(faCheckCircle); diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts index e1c4ec40d8b43..da4f77b2e8e15 100644 --- a/packages/editor-ui/src/types/canvas.ts +++ b/packages/editor-ui/src/types/canvas.ts @@ -50,8 +50,7 @@ export type CanvasNodeDefaultRenderLabelSize = 'small' | 'medium' | 'large'; export type CanvasNodeDirtiness = | 'parameters-updated' | 'incoming-connections-updated' - | 'pinned-data-updated' - | 'upstream-dirty'; + | 'pinned-data-updated'; export type CanvasNodeDefaultRender = { type: CanvasNodeRenderType.Default; From 92bee66531e4147623e41b9641ef0a99d13603b5 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 10:42:19 +0100 Subject: [PATCH 35/52] revert the condition to show icon in NDV --- packages/editor-ui/src/components/OutputPanel.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index 1dfb68f340fda..b89d94244a559 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -361,7 +361,7 @@ const activatePane = () => { {{ i18n.baseText(outputPanelEditMode.enabled ? 'ndv.output.edit' : 'ndv.output') }} Date: Wed, 19 Feb 2025 10:43:02 +0100 Subject: [PATCH 36/52] update tooltip text for dirty nodes --- .../nodes/render-types/parts/CanvasNodeStatusIcons.vue | 4 +++- packages/editor-ui/src/plugins/i18n/locales/en.json | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index c272b2ce4ce00..a171f725ff064 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -79,7 +79,9 @@ const dirtiness = computed(() => > diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json index 202fde80235f1..133532f2b75b0 100644 --- a/packages/editor-ui/src/plugins/i18n/locales/en.json +++ b/packages/editor-ui/src/plugins/i18n/locales/en.json @@ -1062,7 +1062,8 @@ "node.delete": "Delete", "node.add": "Add", "node.issues": "Issues", - "node.dirty": "Output data might be stale, since node parameters have changed", + "node.dirty": "Node configuration changed. Output data may change when this node is run again", + "node.subjectToChange": "Because of changes in the workflow, output data may change when this node is run again", "node.nodeIsExecuting": "Node is executing", "node.nodeIsWaitingTill": "Node is waiting until {date} {time}", "node.theNodeIsWaitingIndefinitelyForAnIncomingWebhookCall": "The node is waiting for an incoming webhook call (indefinitely)", From e4c67d292429426c5646ac8b142a8325e6cb0ae0 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 13:26:22 +0100 Subject: [PATCH 37/52] fix: handle sub nodes properly --- .../src/composables/useNodeDirtiness.test.ts | 33 +++--- .../src/composables/useNodeDirtiness.ts | 105 +++++++++++------- packages/editor-ui/src/types/canvas.ts | 3 +- 3 files changed, 78 insertions(+), 63 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 84430db2bea7c..1eb8e2fc69196 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -205,22 +205,6 @@ describe(useNodeDirtiness, () => { }); }); - // TODO: is this a real scenario? - it.todo( - "should update dirtiness when pinned data is removed from a node which hasn't run", - async () => { - setupTestWorkflow('a✅ -> b📌 -> c✅'); - - canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { - trackHistory: true, - }); - - expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - b: 'pinned-data-updated', - }); - }, - ); - it('should update dirtiness when an existing pinned data is updated', async () => { setupTestWorkflow('a✅ -> b✅📌 -> c✅'); @@ -233,17 +217,28 @@ describe(useNodeDirtiness, () => { }); describe('sub-nodes', () => { - it('should mark its root node as dirty when parameters of a sub node has changed', () => { - setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b, e🧠 -> f🧠 -> b'); + it('should mark its parent nodes with run data as dirty when parameters of a sub node has changed', () => { + setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b, e🧠 -> f✅🧠 -> b'); canvasOperations.setNodeParameters('e', { foo: 1 }); expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - e: 'parameters-updated', + // 'e' itself is not marked as parameters-updated, because it has no run data. + f: 'upstream-dirty', + b: 'upstream-dirty', }); }); }); + /** + * Setup test data in the workflow store using given diagram. + * + * [Symbols] + * - ✅: Node with run data + * - 🚫: Disabled node + * - 📌: Node with pinned data + * - 🧠: A sub node + */ function setupTestWorkflow(diagram: string) { const nodeNamesWithPinnedData = new Set(); const nodes: Record = {}; diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 04fe3bbe07923..6aa7f3d90b292 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -19,16 +19,11 @@ import { computed } from 'vue'; function shouldCommandMarkDirty( command: Undoable, nodeName: string, - nodeLastRanAt: number, getIncomingConnections: (nodeName: string) => INodeConnections, ): boolean { - if (nodeLastRanAt > command.getTimestamp()) { - return false; - } - if (command instanceof BulkCommand) { return command.commands.some((cmd) => - shouldCommandMarkDirty(cmd, nodeName, nodeLastRanAt, getIncomingConnections), + shouldCommandMarkDirty(cmd, nodeName, getIncomingConnections), ); } @@ -46,16 +41,10 @@ function shouldCommandMarkDirty( ? command.node.name : command.nodeName; - return Object.entries(getIncomingConnections(nodeName)).some(([type, nodeInputConnections]) => { - switch (type as NodeConnectionType) { - case NodeConnectionType.Main: - return nodeInputConnections.some((connections) => - connections?.some((connection) => connection.node === commandTargetNodeName), - ); - default: - return false; - } - }); + return Object.values(getIncomingConnections(nodeName)) + .flat() + .flat() + .some((connection) => connection?.node === commandTargetNodeName); } return false; @@ -69,6 +58,54 @@ export function useNodeDirtiness() { const workflowsStore = useWorkflowsStore(); const settingsStore = useSettingsStore(); + function getParentSubNodes(nodeName: string) { + return Object.entries(workflowsStore.incomingConnectionsByNodeName(nodeName)) + .filter(([type]) => (type as NodeConnectionType) !== NodeConnectionType.Main) + .flatMap(([, typeConnections]) => typeConnections.flat().filter((conn) => conn !== null)); + } + + function getParameterUpdateType( + nodeName: string, + after: number, + ): CanvasNodeDirtiness | undefined { + if ((workflowsStore.getParametersLastUpdate(nodeName) ?? 0) > after) { + return 'parameters-updated'; + } + + for (const connection of getParentSubNodes(nodeName)) { + if (getParameterUpdateType(connection.node, after) !== undefined) { + return 'upstream-dirty'; + } + } + + return undefined; + } + + function getConnectionUpdateType( + nodeName: string, + after: number, + ): CanvasNodeDirtiness | undefined { + for (let i = historyStore.undoStack.length - 1; i >= 0; i--) { + const command = historyStore.undoStack[i]; + + if (command.getTimestamp() < after) { + break; + } + + if (shouldCommandMarkDirty(command, nodeName, workflowsStore.incomingConnectionsByNodeName)) { + return 'incoming-connections-updated'; + } + } + + for (const connection of getParentSubNodes(nodeName)) { + if (getConnectionUpdateType(connection.node, after) !== undefined) { + return 'upstream-dirty'; + } + } + + return undefined; + } + const dirtinessByName = computed(() => { // Do not highlight dirtiness if new partial execution is not enabled if (settingsStore.partialExecutionVersion === 1) { @@ -78,42 +115,24 @@ export function useNodeDirtiness() { const dirtiness: Record = {}; const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; - function shouldMarkDirty(command: Undoable, nodeName: string, nodeLastRanAt: number) { - return shouldCommandMarkDirty( - command, - nodeName, - nodeLastRanAt, - workflowsStore.incomingConnectionsByNodeName, - ); - } - - function getRunAt(nodeName: string): number { - return Math.max( - ...Object.entries(workflowsStore.outgoingConnectionsByNodeName(nodeName)) - .filter(([type]) => (type as NodeConnectionType) !== NodeConnectionType.Main) - .flatMap(([, conn]) => conn.flat()) - .map((conn) => (conn ? getRunAt(conn.node) : 0)), - runDataByNode[nodeName]?.[0]?.startTime ?? 0, - ); - } - - for (const node of workflowsStore.allNodes) { - const nodeName = node.name; - const runAt = getRunAt(nodeName); + for (const [nodeName, runData] of Object.entries(runDataByNode)) { + const runAt = runData[0].startTime ?? 0; if (!runAt) { continue; } - const parametersLastUpdate = workflowsStore.getParametersLastUpdate(nodeName) ?? 0; + const parameterUpdate = getParameterUpdateType(nodeName, runAt); - if (parametersLastUpdate > runAt) { - dirtiness[nodeName] = 'parameters-updated'; + if (parameterUpdate) { + dirtiness[nodeName] = parameterUpdate; continue; } - if (historyStore.undoStack.some((command) => shouldMarkDirty(command, nodeName, runAt))) { - dirtiness[nodeName] = 'incoming-connections-updated'; + const connectionUpdate = getConnectionUpdateType(nodeName, runAt); + + if (connectionUpdate) { + dirtiness[nodeName] = connectionUpdate; continue; } diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts index da4f77b2e8e15..e1c4ec40d8b43 100644 --- a/packages/editor-ui/src/types/canvas.ts +++ b/packages/editor-ui/src/types/canvas.ts @@ -50,7 +50,8 @@ export type CanvasNodeDefaultRenderLabelSize = 'small' | 'medium' | 'large'; export type CanvasNodeDirtiness = | 'parameters-updated' | 'incoming-connections-updated' - | 'pinned-data-updated'; + | 'pinned-data-updated' + | 'upstream-dirty'; export type CanvasNodeDefaultRender = { type: CanvasNodeRenderType.Default; From 90f5944c0df9987cde88c184bb3b8536ca7b2989 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 13:43:08 +0100 Subject: [PATCH 38/52] change edge label color --- .../src/components/canvas/elements/edges/CanvasEdge.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue index 7600d0fb2cee9..390f36643a852 100644 --- a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue +++ b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue @@ -89,7 +89,7 @@ const edgeClasses = computed(() => ({ const edgeLabelStyle = computed(() => ({ transform: `translate(0, ${isConnectorStraight.value ? '-100%' : '0%'})`, - color: edgeColor.value, + color: 'var(--color-text-base)', })); const isConnectorStraight = computed(() => renderData.value.isConnectorStraight); From 0ecbe75c413c6279cff3329d6e1a9d8dc5e3b4ec Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 13:43:31 +0100 Subject: [PATCH 39/52] do not change dirtiness when node is enabled --- packages/editor-ui/src/composables/useNodeDirtiness.test.ts | 6 ++---- packages/editor-ui/src/composables/useNodeDirtiness.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 1eb8e2fc69196..f856adba0cde9 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -158,16 +158,14 @@ describe(useNodeDirtiness, () => { }); }); - it('should mark downstream nodes dirty if the node is set to enabled', () => { + it('should not mark anything dirty if a disabled node is set to enabled', () => { setupTestWorkflow('a✅ -> b🚫 -> c✅ -> d✅'); canvasOperations.toggleNodesDisabled(['b'], { trackHistory: true, }); - expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - c: 'incoming-connections-updated', - }); + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); }); it('should restore original dirtiness after undoing a command', async () => { diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 6aa7f3d90b292..8d17946ed8ec8 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -34,7 +34,7 @@ function shouldCommandMarkDirty( if ( command instanceof RemoveNodeCommand || command instanceof AddNodeCommand || - command instanceof EnableNodeToggleCommand + (command instanceof EnableNodeToggleCommand && command.newState) // truthy newState value means node is getting disabled ) { const commandTargetNodeName = command instanceof RemoveNodeCommand || command instanceof AddNodeCommand From 1c632f6054a6b7539caf28f905d162d09d4acb4e Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 14:26:00 +0100 Subject: [PATCH 40/52] make immediate downstream node dirty when pinned data changed --- packages/editor-ui/src/Interface.ts | 8 +-- .../src/composables/useNodeDirtiness.test.ts | 58 ++++++++++++++++--- .../src/composables/useNodeDirtiness.ts | 22 ++++++- .../editor-ui/src/stores/workflows.store.ts | 11 +++- 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index e869bb13cd748..e9ca925365a97 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -850,12 +850,8 @@ export interface ITemplatesNode extends IVersionNode { export interface INodeMetadata { parametersLastUpdatedAt?: number; - /** - * UNIX timestamp of either when existing pinned data is modified or removed. - * - * Note that pinning data for the first time isn't supposed to set this field. - */ - pinnedDataUpdatedAt?: number; + pinnedDataLastUpdatedAt?: number; + pinnedDataLastRemovedAt?: number; pristine: boolean; } diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index f856adba0cde9..57b225cd6df42 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -27,6 +27,9 @@ describe(useNodeDirtiness, () => { let canvasOperations: ReturnType; let uiStore: ReturnType; + const NODE_RUN_AT = new Date('2025-01-01T00:00:01'); + const WORKFLOW_UPDATED_AT = new Date('2025-01-01T00:00:10'); + beforeEach(() => { vi.useFakeTimers(); @@ -122,6 +125,42 @@ describe(useNodeDirtiness, () => { }); }); + it('should clear dirtiness if a dirty node gets new run data', () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a✅ -> b✅ -> c✅'); + + canvasOperations.setNodeParameters('b', { foo: 1 }); + + const runAt = new Date(+WORKFLOW_UPDATED_AT + 1000); + + workflowsStore.setWorkflowExecutionData({ + id: workflowsStore.workflow.id, + finished: true, + mode: 'manual', + status: 'success', + workflowData: workflowsStore.workflow, + startedAt: runAt, + createdAt: runAt, + data: { + resultData: { + runData: { + b: [ + { + startTime: +runAt, + executionTime: 0, + executionStatus: 'success', + source: [], + }, + ], + }, + }, + }, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); + }); + it("should not update dirtiness if the node hasn't run yet", () => { setupTestWorkflow('a✅, b, c✅'); @@ -192,7 +231,7 @@ describe(useNodeDirtiness, () => { }); it('should update dirtiness when pinned data is removed from a node with run data', async () => { - setupTestWorkflow('a✅ -> b✅📌 -> c✅'); + setupTestWorkflow('a✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { trackHistory: true, @@ -203,13 +242,15 @@ describe(useNodeDirtiness, () => { }); }); - it('should update dirtiness when an existing pinned data is updated', async () => { - setupTestWorkflow('a✅ -> b✅📌 -> c✅'); + it('should update dirtiness when an existing pinned data of an incoming node is updated', async () => { + setupTestWorkflow('a✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); workflowsStore.pinData({ node: workflowsStore.nodesByName.b, data: [{ json: {} }] }); expect(useNodeDirtiness().dirtinessByName.value).toEqual({ - b: 'pinned-data-updated', + // 'd' is not marked as pinned-data-updated because it has no run data. + c: 'pinned-data-updated', + e: 'pinned-data-updated', }); }); }); @@ -276,7 +317,7 @@ describe(useNodeDirtiness, () => { if (attributes.includes('✅')) { runData[name] = [ { - startTime: +new Date('2025-01-01'), + startTime: +NODE_RUN_AT, executionTime: 0, executionStatus: 'success', source: [], @@ -308,11 +349,12 @@ describe(useNodeDirtiness, () => { mode: 'manual', status: 'success', workflowData: workflow, - startedAt: new Date(0), - createdAt: new Date(0), + startedAt: NODE_RUN_AT, + createdAt: NODE_RUN_AT, data: { resultData: { runData } }, }); - vi.setSystemTime(new Date('2025-01-02')); // after execution + // prepare for making changes to the workflow + vi.setSystemTime(WORKFLOW_UPDATED_AT); } }); diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 8d17946ed8ec8..b289bdf08bb8c 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -136,9 +136,27 @@ export function useNodeDirtiness() { continue; } - const pinnedDataUpdatedAt = workflowsStore.getPinnedDataLastUpdate(nodeName) ?? 0; + const hasInputPinnedDataChanged = Object.values( + workflowsStore.incomingConnectionsByNodeName(nodeName), + ) + .flat() + .flat() + .filter((connection) => connection !== null) + .some((connection) => { + const pinnedDataLastUpdatedAt = + workflowsStore.getPinnedDataLastUpdate(connection.node) ?? 0; + + return pinnedDataLastUpdatedAt > runAt; + }); + + if (hasInputPinnedDataChanged) { + dirtiness[nodeName] = 'pinned-data-updated'; + continue; + } + + const pinnedDataLastRemovedAt = workflowsStore.getPinnedDataLastRemovedAt(nodeName) ?? 0; - if (pinnedDataUpdatedAt > runAt) { + if (pinnedDataLastRemovedAt > runAt) { dirtiness[nodeName] = 'pinned-data-updated'; continue; } diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 507bccd51ae0b..eae79df9c987f 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -332,7 +332,11 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } function getPinnedDataLastUpdate(nodeName: string): number | undefined { - return nodeMetadata.value[nodeName]?.pinnedDataUpdatedAt; + return nodeMetadata.value[nodeName]?.pinnedDataLastUpdatedAt; + } + + function getPinnedDataLastRemovedAt(nodeName: string): number | undefined { + return nodeMetadata.value[nodeName]?.pinnedDataLastRemovedAt; } function isNodePristine(nodeName: string): boolean { @@ -833,7 +837,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { if ((workflow.value.pinData?.[nodeName] ?? []).length > 0 && nodeMetadata.value[nodeName]) { // Updating existing pinned data - nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); + nodeMetadata.value[nodeName].pinnedDataLastUpdatedAt = Date.now(); } const storedPinData = payload.data.map((item) => @@ -868,7 +872,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { }; if (nodeMetadata.value[nodeName]) { - nodeMetadata.value[nodeName].pinnedDataUpdatedAt = Date.now(); + nodeMetadata.value[nodeName].pinnedDataLastRemovedAt = Date.now(); } uiStore.stateIsDirty = true; @@ -1710,6 +1714,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { getNodesByIds, getParametersLastUpdate, getPinnedDataLastUpdate, + getPinnedDataLastRemovedAt, isNodePristine, isNodeExecuting, getExecutionDataById, From e321529988a6262d489260a7c7af21de00a507f8 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 16:01:54 +0100 Subject: [PATCH 41/52] fix label color --- .../handles/render-types/CanvasHandleMainOutput.vue | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue index 9cc7cebf248c4..f05eb74d775ae 100644 --- a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue +++ b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue @@ -66,7 +66,6 @@ const outputLabelClasses = computed(() => ({ const runDataLabelClasses = computed(() => ({ [$style.label]: true, [$style.runDataLabel]: true, - [$style.dirty]: renderOptions.value.dirtiness !== undefined, })); function onMouseEnter() { @@ -143,11 +142,7 @@ function onClickAdd() { left: 50%; transform: translate(-50%, -150%); font-size: var(--font-size-xs); - color: var(--color-success); - - &.dirty { - color: var(--color-warning); - } + color: var(--color-text-base); } From cb84732efe9d72aa112bada5fb353842eb54e507 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 16:05:19 +0100 Subject: [PATCH 42/52] update dirtiness check for enabling sub node and removing a node --- .../src/composables/useNodeDirtiness.test.ts | 38 +++++++++++++- .../src/composables/useNodeDirtiness.ts | 52 +++++++++++++------ 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 57b225cd6df42..060d635566247 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -101,17 +101,29 @@ describe(useNodeDirtiness, () => { }); describe('deleting a node', () => { - it("should mark a node as 'incoming-connections-updated' if parent node is deleted", async () => { + it("should mark a node as 'incoming-connections-updated' if a parent node is replaced by removing a node", async () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); setupTestWorkflow('a✅ -> b✅ -> c✅'); - canvasOperations.deleteNodes(['b'], { trackHistory: true }); + canvasOperations.deleteNodes(['b'], { trackHistory: true }); // 'a' becomes new parent of 'c' expect(useNodeDirtiness().dirtinessByName.value).toEqual({ c: 'incoming-connections-updated', }); }); + + it("should mark a node as 'incoming-connections-updated' if a parent node get removed", async () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a✅ -> b✅ -> c✅'); + + canvasOperations.deleteNodes(['a'], { trackHistory: true }); // 'b' has no parent node anymore + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'incoming-connections-updated', + }); + }); }); describe('updating node parameters', () => { @@ -267,6 +279,28 @@ describe(useNodeDirtiness, () => { b: 'upstream-dirty', }); }); + + it('should change dirtiness if a disabled sub node is set to enabled', () => { + setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠🚫 -> b'); + + canvasOperations.toggleNodesDisabled(['d'], { + trackHistory: true, + }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'incoming-connections-updated', + }); + }); + + it('should change dirtiness if a sub node is removed', () => { + setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b'); + + canvasOperations.deleteNodes(['d'], { trackHistory: true }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + b: 'incoming-connections-updated', + }); + }); }); /** diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index b289bdf08bb8c..6ce07843307c3 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -3,7 +3,7 @@ import { AddNodeCommand, BulkCommand, EnableNodeToggleCommand, - RemoveNodeCommand, + RemoveConnectionCommand, type Undoable, } from '@/models/history'; import { useHistoryStore } from '@/stores/history.store'; @@ -20,10 +20,11 @@ function shouldCommandMarkDirty( command: Undoable, nodeName: string, getIncomingConnections: (nodeName: string) => INodeConnections, + getOutgoingConnectors: (nodeName: string) => INodeConnections, ): boolean { if (command instanceof BulkCommand) { return command.commands.some((cmd) => - shouldCommandMarkDirty(cmd, nodeName, getIncomingConnections), + shouldCommandMarkDirty(cmd, nodeName, getIncomingConnections, getOutgoingConnectors), ); } @@ -31,20 +32,30 @@ function shouldCommandMarkDirty( return command.connectionData[1]?.node === nodeName; } - if ( - command instanceof RemoveNodeCommand || - command instanceof AddNodeCommand || - (command instanceof EnableNodeToggleCommand && command.newState) // truthy newState value means node is getting disabled - ) { - const commandTargetNodeName = - command instanceof RemoveNodeCommand || command instanceof AddNodeCommand - ? command.node.name - : command.nodeName; - - return Object.values(getIncomingConnections(nodeName)) - .flat() - .flat() - .some((connection) => connection?.node === commandTargetNodeName); + const incomingNodes = Object.values(getIncomingConnections(nodeName)) + .flat() + .flat() + .filter((connection) => connection !== null) + .map((connection) => connection.node); + + if (command instanceof RemoveConnectionCommand) { + const [from, to] = command.connectionData; + + return to.node === nodeName && !incomingNodes.includes(from.node); + } + + if (command instanceof AddNodeCommand) { + return incomingNodes.includes(command.node.name); + } + + if (command instanceof EnableNodeToggleCommand) { + return ( + incomingNodes.includes(command.nodeName) && + (command.newState || + Object.keys(getOutgoingConnectors(command.nodeName)).some( + (type) => type !== NodeConnectionType.Main, + )) + ); } return false; @@ -92,7 +103,14 @@ export function useNodeDirtiness() { break; } - if (shouldCommandMarkDirty(command, nodeName, workflowsStore.incomingConnectionsByNodeName)) { + if ( + shouldCommandMarkDirty( + command, + nodeName, + workflowsStore.incomingConnectionsByNodeName, + workflowsStore.outgoingConnectionsByNodeName, + ) + ) { return 'incoming-connections-updated'; } } From ec256704ee85fad583bcb36677bc8c79344a5fb1 Mon Sep 17 00:00:00 2001 From: autologie Date: Wed, 19 Feb 2025 20:46:08 +0100 Subject: [PATCH 43/52] fix failing unit tests --- .../src/composables/useNodeDirtiness.ts | 2 +- .../src/composables/useRunWorkflow.test.ts | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index 6ce07843307c3..ed01cd77ece27 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -134,7 +134,7 @@ export function useNodeDirtiness() { const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; for (const [nodeName, runData] of Object.entries(runDataByNode)) { - const runAt = runData[0].startTime ?? 0; + const runAt = runData[0]?.startTime ?? 0; if (!runAt) { continue; diff --git a/packages/editor-ui/src/composables/useRunWorkflow.test.ts b/packages/editor-ui/src/composables/useRunWorkflow.test.ts index f02ea12c3cac2..34990271a7585 100644 --- a/packages/editor-ui/src/composables/useRunWorkflow.test.ts +++ b/packages/editor-ui/src/composables/useRunWorkflow.test.ts @@ -42,6 +42,8 @@ vi.mock('@/stores/workflows.store', () => ({ checkIfNodeHasChatParent: vi.fn(), getParametersLastUpdate: vi.fn(), getPinnedDataLastUpdate: vi.fn(), + getPinnedDataLastRemovedAt: vi.fn(), + incomingConnectionsByNodeName: vi.fn(), outgoingConnectionsByNodeName: vi.fn(), }), })); @@ -336,13 +338,15 @@ describe('useRunWorkflow({ router })', () => { createTestNode({ name: parentName }), createTestNode({ name: executeName }), ]; - vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation( - (nodeName) => - ({ - [parentName]: { - main: [[{ node: executeName, type: NodeConnectionType.Main, index: 0 }]], - }, - })[nodeName] ?? ({} as INodeConnections), + vi.mocked(workflowsStore).outgoingConnectionsByNodeName.mockImplementation((nodeName) => + nodeName === parentName + ? { main: [[{ node: executeName, type: NodeConnectionType.Main, index: 0 }]] } + : ({} as INodeConnections), + ); + vi.mocked(workflowsStore).incomingConnectionsByNodeName.mockImplementation((nodeName) => + nodeName === executeName + ? { main: [[{ node: parentName, type: NodeConnectionType.Main, index: 0 }]] } + : ({} as INodeConnections), ); vi.mocked(workflowsStore).getWorkflowRunData = { [parentName]: [ From 1a77ef4c83d651a6fcb4ed24de16e6f603aa782d Mon Sep 17 00:00:00 2001 From: autologie Date: Thu, 20 Feb 2025 07:41:52 +0100 Subject: [PATCH 44/52] add e2e test case --- cypress/composables/ndv.ts | 4 + cypress/e2e/40-manual-partial-execution.cy.ts | 62 ++++++++++++++++ .../Test_workflow_partial_execution_v2.json | 74 +++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cypress/fixtures/Test_workflow_partial_execution_v2.json diff --git a/cypress/composables/ndv.ts b/cypress/composables/ndv.ts index c70e0b20e8894..36ae10669b44b 100644 --- a/cypress/composables/ndv.ts +++ b/cypress/composables/ndv.ts @@ -206,6 +206,10 @@ export function clickWorkflowCardContent(workflowName: string) { getWorkflowCardContent(workflowName).click(); } +export function clickAssignmentCollectionAdd() { + cy.getByTestId('assignment-collection-drop-area').click(); +} + export function assertNodeOutputHintExists() { getNodeOutputHint().should('exist'); } diff --git a/cypress/e2e/40-manual-partial-execution.cy.ts b/cypress/e2e/40-manual-partial-execution.cy.ts index 2eb129475f1f4..1ddcc7908842f 100644 --- a/cypress/e2e/40-manual-partial-execution.cy.ts +++ b/cypress/e2e/40-manual-partial-execution.cy.ts @@ -1,3 +1,16 @@ +import { + clickAssignmentCollectionAdd, + clickGetBackToCanvas, + getNodeRunInfoStale, + getOutputTbodyCell, +} from '../composables/ndv'; +import { + clickExecuteWorkflowButton, + getNodeByName, + getZoomToFitButton, + navigateToNewWorkflowPage, + openNode, +} from '../composables/workflow'; import { NDV, WorkflowPage } from '../pages'; const canvas = new WorkflowPage(); @@ -26,4 +39,53 @@ describe('Manual partial execution', () => { ndv.getters.nodeRunTooltipIndicator().should('exist'); ndv.getters.outputRunSelector().should('not.exist'); // single run }); + + describe('partial execution v2', () => { + beforeEach(() => { + cy.window().then((win) => { + win.localStorage.setItem('PartialExecution.version', '2'); + }); + navigateToNewWorkflowPage(); + }); + + it('should execute from the first dirty node up to the current node', () => { + cy.createFixtureWorkflow('Test_workflow_partial_execution_v2.json'); + + getZoomToFitButton().click(); + + // First, execute the while workflow + clickExecuteWorkflowButton(); + + getNodeByName('A').findChildByTestId('canvas-node-status-success').should('be.visible'); + getNodeByName('B').findChildByTestId('canvas-node-status-success').should('be.visible'); + getNodeByName('C').findChildByTestId('canvas-node-status-success').should('be.visible'); + openNode('A'); + getOutputTbodyCell(1, 0).invoke('text').as('before', { type: 'static' }); + clickGetBackToCanvas(); + + // Change parameter of the node in the middle + openNode('B'); + clickAssignmentCollectionAdd(); + getNodeRunInfoStale().should('be.visible'); + clickGetBackToCanvas(); + + getNodeByName('A').findChildByTestId('canvas-node-status-success').should('be.visible'); + getNodeByName('B').findChildByTestId('canvas-node-status-warning').should('be.visible'); + getNodeByName('C').findChildByTestId('canvas-node-status-success').should('be.visible'); + + // Partial execution + getNodeByName('C').findChildByTestId('execute-node-button').click(); + + getNodeByName('A').findChildByTestId('canvas-node-status-success').should('be.visible'); + getNodeByName('B').findChildByTestId('canvas-node-status-success').should('be.visible'); + getNodeByName('C').findChildByTestId('canvas-node-status-success').should('be.visible'); + openNode('A'); + getOutputTbodyCell(1, 0).invoke('text').as('after', { type: 'static' }); + + // Assert that 'A' ran only once by comparing its output + cy.get('@before').then((before) => + cy.get('@after').then((after) => expect(before).to.equal(after)), + ); + }); + }); }); diff --git a/cypress/fixtures/Test_workflow_partial_execution_v2.json b/cypress/fixtures/Test_workflow_partial_execution_v2.json new file mode 100644 index 0000000000000..c3c8ecc7ae5ab --- /dev/null +++ b/cypress/fixtures/Test_workflow_partial_execution_v2.json @@ -0,0 +1,74 @@ +{ + "nodes": [ + { + "parameters": { + "rule": { + "interval": [{}] + } + }, + "type": "n8n-nodes-base.scheduleTrigger", + "typeVersion": 1.2, + "position": [0, 0], + "id": "dcc1c5e1-c6c1-45f8-80d5-65c88d66d56e", + "name": "A" + }, + { + "parameters": { + "assignments": { + "assignments": [ + { + "id": "3d8f0810-84f0-41ce-a81b-0e7f04fd88cb", + "name": "", + "value": "", + "type": "string" + } + ] + }, + "options": {} + }, + "type": "n8n-nodes-base.set", + "typeVersion": 3.4, + "position": [220, 0], + "id": "097ffa30-d37b-4de6-bd5c-ccd945f31df1", + "name": "B" + }, + { + "parameters": { + "options": {} + }, + "type": "n8n-nodes-base.set", + "typeVersion": 3.4, + "position": [440, 0], + "id": "dc44e635-916f-4f76-a745-1add5762f730", + "name": "C" + } + ], + "connections": { + "A": { + "main": [ + [ + { + "node": "B", + "type": "main", + "index": 0 + } + ] + ] + }, + "B": { + "main": [ + [ + { + "node": "C", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": {}, + "meta": { + "instanceId": "b0d9447cff9c96796e4ac4f00fcd899b03cfac3ab3d4f748ae686d34881eae0c" + } +} From b32383375d092aa37f1b3390ee419f42dd712218 Mon Sep 17 00:00:00 2001 From: autologie Date: Thu, 20 Feb 2025 07:43:58 +0100 Subject: [PATCH 45/52] fix typo --- cypress/e2e/40-manual-partial-execution.cy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/40-manual-partial-execution.cy.ts b/cypress/e2e/40-manual-partial-execution.cy.ts index 1ddcc7908842f..f3b0ec0c95cfd 100644 --- a/cypress/e2e/40-manual-partial-execution.cy.ts +++ b/cypress/e2e/40-manual-partial-execution.cy.ts @@ -53,7 +53,7 @@ describe('Manual partial execution', () => { getZoomToFitButton().click(); - // First, execute the while workflow + // First, execute the whole workflow clickExecuteWorkflowButton(); getNodeByName('A').findChildByTestId('canvas-node-status-success').should('be.visible'); From 33bd650ee15be0a7a1446b59c4c47b2b09c06d25 Mon Sep 17 00:00:00 2001 From: autologie Date: Thu, 20 Feb 2025 12:26:52 +0100 Subject: [PATCH 46/52] replace dirty node icon --- .../render-types/parts/CanvasNodeStatusIcons.vue | 2 +- packages/editor-ui/src/plugins/icons/custom.ts | 12 ++++++++++++ packages/editor-ui/src/plugins/icons/index.ts | 5 ++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index a171f725ff064..1ed3a14439da0 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -83,7 +83,7 @@ const dirtiness = computed(() => i18n.baseText(dirtiness === 'parameters-updated' ? 'node.dirty' : 'node.subjectToChange') }} - +
Date: Thu, 20 Feb 2025 14:55:59 +0100 Subject: [PATCH 47/52] fix: warning icon is missing in OutputPanel when there are multiple runData --- packages/editor-ui/src/components/OutputPanel.vue | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/OutputPanel.vue b/packages/editor-ui/src/components/OutputPanel.vue index b89d94244a559..62ec5518b8d49 100644 --- a/packages/editor-ui/src/components/OutputPanel.vue +++ b/packages/editor-ui/src/components/OutputPanel.vue @@ -361,7 +361,11 @@ const activatePane = () => { {{ i18n.baseText(outputPanelEditMode.enabled ? 'ndv.output.edit' : 'ndv.output') }} Date: Thu, 20 Feb 2025 16:59:59 +0100 Subject: [PATCH 48/52] feat: mark start node of the loop dirty --- .../src/composables/useNodeDirtiness.test.ts | 65 ++++++----- .../src/composables/useNodeDirtiness.ts | 106 +++++++++++++++++- 2 files changed, 138 insertions(+), 33 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index 060d635566247..d51ee7aac2958 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -4,6 +4,7 @@ import { createComponentRenderer } from '@/__tests__/render'; import { useCanvasOperations } from '@/composables/useCanvasOperations'; import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { useNodeDirtiness } from '@/composables/useNodeDirtiness'; +import { MANUAL_TRIGGER_NODE_TYPE, SET_NODE_TYPE } from '@/constants'; import { type INodeUi } from '@/Interface'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useSettingsStore } from '@/stores/settings.store'; @@ -21,6 +22,7 @@ import { } from 'vue-router'; describe(useNodeDirtiness, () => { + let nodeTypeStore: ReturnType; let workflowsStore: ReturnType; let settingsStore: ReturnType; let historyHelper: ReturnType; @@ -35,12 +37,15 @@ describe(useNodeDirtiness, () => { const TestComponent = defineComponent({ setup() { + nodeTypeStore = useNodeTypesStore(); workflowsStore = useWorkflowsStore(); settingsStore = useSettingsStore(); historyHelper = useHistoryHelper({} as RouteLocationNormalizedLoaded); canvasOperations = useCanvasOperations({ router: useRouter() }); uiStore = useUIStore(); + nodeTypeStore.setNodeTypes(defaultNodeDescriptions); + // Enable new partial execution settingsStore.settings = { partialExecution: { version: 2, enforce: true }, @@ -63,27 +68,16 @@ describe(useNodeDirtiness, () => { }); it('should be an empty object if no change has been made to the workflow', () => { - setupTestWorkflow('a✅, b✅, c✅'); + setupTestWorkflow('a🚨✅, b✅, c✅'); expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); }); - it('should return even if the connections forms a loop', () => { - setupTestWorkflow('a✅ -> b✅ -> c -> d✅ -> b'); - - expect(() => { - canvasOperations.setNodeParameters('b', { foo: 1 }); - - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - useNodeDirtiness().dirtinessByName.value; - }).not.toThrow(); - }); - describe('injecting a node', () => { it("should mark a node as 'incoming-connections-updated' if a new node is injected as its parent", async () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); - setupTestWorkflow('a✅ -> b✅'); + setupTestWorkflow('a🚨✅ -> b✅'); uiStore.lastInteractedWithNodeConnection = { source: 'a', @@ -104,7 +98,7 @@ describe(useNodeDirtiness, () => { it("should mark a node as 'incoming-connections-updated' if a parent node is replaced by removing a node", async () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); - setupTestWorkflow('a✅ -> b✅ -> c✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); canvasOperations.deleteNodes(['b'], { trackHistory: true }); // 'a' becomes new parent of 'c' @@ -116,7 +110,7 @@ describe(useNodeDirtiness, () => { it("should mark a node as 'incoming-connections-updated' if a parent node get removed", async () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); - setupTestWorkflow('a✅ -> b✅ -> c✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); canvasOperations.deleteNodes(['a'], { trackHistory: true }); // 'b' has no parent node anymore @@ -128,7 +122,7 @@ describe(useNodeDirtiness, () => { describe('updating node parameters', () => { it("should mark a node as 'parameters-updated' if its parameter has changed", () => { - setupTestWorkflow('a✅, b✅, c✅'); + setupTestWorkflow('a🚨✅, b✅, c✅'); canvasOperations.setNodeParameters('b', { foo: 1 }); @@ -140,7 +134,7 @@ describe(useNodeDirtiness, () => { it('should clear dirtiness if a dirty node gets new run data', () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); - setupTestWorkflow('a✅ -> b✅ -> c✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); canvasOperations.setNodeParameters('b', { foo: 1 }); @@ -174,7 +168,7 @@ describe(useNodeDirtiness, () => { }); it("should not update dirtiness if the node hasn't run yet", () => { - setupTestWorkflow('a✅, b, c✅'); + setupTestWorkflow('a🚨✅, b, c✅'); canvasOperations.setNodeParameters('b', { foo: 1 }); @@ -186,7 +180,7 @@ describe(useNodeDirtiness, () => { it("should mark a node as 'incoming-connections-updated' if a new incoming connection is added", () => { useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); - setupTestWorkflow('a✅ -> b✅ -> c✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); canvasOperations.createConnection({ source: 'a', target: 'c' }, { trackHistory: true }); @@ -198,7 +192,7 @@ describe(useNodeDirtiness, () => { describe('enabling/disabling nodes', () => { it('should mark downstream nodes dirty if the node is set to disabled', () => { - setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅ -> d✅'); canvasOperations.toggleNodesDisabled(['b'], { trackHistory: true, @@ -210,7 +204,7 @@ describe(useNodeDirtiness, () => { }); it('should not mark anything dirty if a disabled node is set to enabled', () => { - setupTestWorkflow('a✅ -> b🚫 -> c✅ -> d✅'); + setupTestWorkflow('a🚨✅ -> b🚫 -> c✅ -> d✅'); canvasOperations.toggleNodesDisabled(['b'], { trackHistory: true, @@ -220,7 +214,7 @@ describe(useNodeDirtiness, () => { }); it('should restore original dirtiness after undoing a command', async () => { - setupTestWorkflow('a✅ -> b✅ -> c✅ -> d✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅ -> d✅'); canvasOperations.toggleNodesDisabled(['b'], { trackHistory: true, @@ -233,7 +227,7 @@ describe(useNodeDirtiness, () => { describe('pinned data', () => { it('should not change dirtiness when data is pinned', async () => { - setupTestWorkflow('a✅ -> b✅ -> c✅'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { trackHistory: true, @@ -243,7 +237,7 @@ describe(useNodeDirtiness, () => { }); it('should update dirtiness when pinned data is removed from a node with run data', async () => { - setupTestWorkflow('a✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); + setupTestWorkflow('a🚨✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); canvasOperations.toggleNodesPinned(['b'], 'pin-icon-click', { trackHistory: true, @@ -255,7 +249,7 @@ describe(useNodeDirtiness, () => { }); it('should update dirtiness when an existing pinned data of an incoming node is updated', async () => { - setupTestWorkflow('a✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); + setupTestWorkflow('a🚨✅ -> b✅📌 -> c✅, b -> d, b -> e✅ -> f✅'); workflowsStore.pinData({ node: workflowsStore.nodesByName.b, data: [{ json: {} }] }); @@ -269,7 +263,7 @@ describe(useNodeDirtiness, () => { describe('sub-nodes', () => { it('should mark its parent nodes with run data as dirty when parameters of a sub node has changed', () => { - setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b, e🧠 -> f✅🧠 -> b'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅, d🧠 -> b, e🧠 -> f✅🧠 -> b'); canvasOperations.setNodeParameters('e', { foo: 1 }); @@ -281,7 +275,7 @@ describe(useNodeDirtiness, () => { }); it('should change dirtiness if a disabled sub node is set to enabled', () => { - setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠🚫 -> b'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅, d🧠🚫 -> b'); canvasOperations.toggleNodesDisabled(['d'], { trackHistory: true, @@ -293,7 +287,7 @@ describe(useNodeDirtiness, () => { }); it('should change dirtiness if a sub node is removed', () => { - setupTestWorkflow('a✅ -> b✅ -> c✅, d🧠 -> b'); + setupTestWorkflow('a🚨✅ -> b✅ -> c✅, d🧠 -> b'); canvasOperations.deleteNodes(['d'], { trackHistory: true }); @@ -303,10 +297,24 @@ describe(useNodeDirtiness, () => { }); }); + describe('workflow with a loop', () => { + it('should change the dirtiness of the first node in the loop when one of nodes in the loop becomes dirty', () => { + setupTestWorkflow('a🚨✅ -> b✅ -> c✅ -> d✅ -> e✅ -> f✅ -> c✅'); + + canvasOperations.setNodeParameters('e', { foo: 1 }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({ + c: 'upstream-dirty', + e: 'parameters-updated', + }); + }); + }); + /** * Setup test data in the workflow store using given diagram. * * [Symbols] + * - 🚨: Trigger node * - ✅: Node with run data * - 🚫: Disabled node * - 📌: Node with pinned data @@ -346,6 +354,7 @@ describe(useNodeDirtiness, () => { id: name, name, disabled: attributes.includes('🚫'), + type: attributes.includes('🚨') ? MANUAL_TRIGGER_NODE_TYPE : SET_NODE_TYPE, }); if (attributes.includes('✅')) { diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index ed01cd77ece27..b17710faccf64 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -53,7 +53,7 @@ function shouldCommandMarkDirty( incomingNodes.includes(command.nodeName) && (command.newState || Object.keys(getOutgoingConnectors(command.nodeName)).some( - (type) => type !== NodeConnectionType.Main, + (type) => (type as NodeConnectionType) !== NodeConnectionType.Main, )) ); } @@ -61,6 +61,41 @@ function shouldCommandMarkDirty( return false; } +/** + * If given node is part of a loop, returns the set of nodes that forms the loop, otherwise returns undefined. + */ +function findLoop( + nodeName: string, + visited: Set, + getIncomingConnections: (nodeName: string) => INodeConnections, +): Set | undefined { + if (visited.has(nodeName)) { + return visited; + } + + const visitedCopy = new Set(visited); + + visitedCopy.add(nodeName); + + for (const [type, typeConnections] of Object.entries(getIncomingConnections(nodeName))) { + if ((type as NodeConnectionType) !== NodeConnectionType.Main) { + continue; + } + + for (const connections of typeConnections) { + for (const { node } of connections ?? []) { + const loop = findLoop(node, visitedCopy, getIncomingConnections); + + if (loop) { + return loop; + } + } + } + } + + return undefined; +} + /** * Determines the subgraph that is affected by changes made after the last (partial) execution */ @@ -124,6 +159,48 @@ export function useNodeDirtiness() { return undefined; } + /** + * Depth of node is defined as the minimum distance (number of connections) from the trigger node + */ + const depthByName = computed(() => { + const depth: Record = {}; + + function setDepthRecursively(nodeName: string, current: number, visited: Set) { + if (visited.has(nodeName)) { + return; + } + + const myVisited = new Set(visited); + + myVisited.add(nodeName); + + for (const [type, typeConnections] of Object.entries( + workflowsStore.outgoingConnectionsByNodeName(nodeName), + )) { + if ((type as NodeConnectionType) !== NodeConnectionType.Main) { + continue; + } + + for (const connections of typeConnections) { + for (const { node } of connections ?? []) { + if (!depth[node] || depth[node] > current) { + depth[node] = current; + } + + setDepthRecursively(node, current + 1, myVisited); + } + } + } + } + + for (const trigger of workflowsStore.workflowTriggerNodes) { + depth[trigger.name] = 0; + setDepthRecursively(trigger.name, 1, new Set()); + } + + return depth; + }); + const dirtinessByName = computed(() => { // Do not highlight dirtiness if new partial execution is not enabled if (settingsStore.partialExecutionVersion === 1) { @@ -133,6 +210,25 @@ export function useNodeDirtiness() { const dirtiness: Record = {}; const runDataByNode = workflowsStore.getWorkflowRunData ?? {}; + function setDirtiness(nodeName: string, value: CanvasNodeDirtiness) { + dirtiness[nodeName] = dirtiness[nodeName] ?? value; + + const loop = findLoop(nodeName, new Set(), workflowsStore.incomingConnectionsByNodeName); + + if (!loop) { + return; + } + + const loopEntryNodeName = [...loop].sort( + (a, b) => (depthByName.value[a] ?? 0) - (depthByName.value[b] ?? 0), + )?.[0]; + + if (loopEntryNodeName) { + // If a node in a loop becomes dirty, the first node in the loop should also be dirty + dirtiness[loopEntryNodeName] = dirtiness[loopEntryNodeName] ?? 'upstream-dirty'; + } + } + for (const [nodeName, runData] of Object.entries(runDataByNode)) { const runAt = runData[0]?.startTime ?? 0; @@ -143,14 +239,14 @@ export function useNodeDirtiness() { const parameterUpdate = getParameterUpdateType(nodeName, runAt); if (parameterUpdate) { - dirtiness[nodeName] = parameterUpdate; + setDirtiness(nodeName, parameterUpdate); continue; } const connectionUpdate = getConnectionUpdateType(nodeName, runAt); if (connectionUpdate) { - dirtiness[nodeName] = connectionUpdate; + setDirtiness(nodeName, connectionUpdate); continue; } @@ -168,14 +264,14 @@ export function useNodeDirtiness() { }); if (hasInputPinnedDataChanged) { - dirtiness[nodeName] = 'pinned-data-updated'; + setDirtiness(nodeName, 'pinned-data-updated'); continue; } const pinnedDataLastRemovedAt = workflowsStore.getPinnedDataLastRemovedAt(nodeName) ?? 0; if (pinnedDataLastRemovedAt > runAt) { - dirtiness[nodeName] = 'pinned-data-updated'; + setDirtiness(nodeName, 'pinned-data-updated'); continue; } } From 357dc1626b24cdfc269b58ef6a6f94e566e17c08 Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 21 Feb 2025 16:21:58 +0100 Subject: [PATCH 49/52] change the warning icon used in OutputPanel --- .../src/components/N8nInfoTip/InfoTip.vue | 18 ++++++++++++++++-- packages/editor-ui/src/components/RunInfo.vue | 13 +++++++------ packages/editor-ui/src/plugins/icons/custom.ts | 2 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/design-system/src/components/N8nInfoTip/InfoTip.vue b/packages/design-system/src/components/N8nInfoTip/InfoTip.vue index 04ba27ae5f77b..7daf381fb8f99 100644 --- a/packages/design-system/src/components/N8nInfoTip/InfoTip.vue +++ b/packages/design-system/src/components/N8nInfoTip/InfoTip.vue @@ -7,7 +7,7 @@ import type { IconColor } from 'n8n-design-system/types/icon'; import N8nIcon from '../N8nIcon'; import N8nTooltip from '../N8nTooltip'; -const THEME = ['info', 'info-light', 'warning', 'danger', 'success'] as const; +const THEME = ['info', 'info-light', 'warning', 'warning-light', 'danger', 'success'] as const; const TYPE = ['note', 'tooltip'] as const; const ICON_MAP = { @@ -15,9 +15,23 @@ const ICON_MAP = { // eslint-disable-next-line @typescript-eslint/naming-convention 'info-light': 'info-circle', warning: 'exclamation-triangle', + // eslint-disable-next-line @typescript-eslint/naming-convention + 'warning-light': 'triangle', // NOTE: This requires a custom icon danger: 'exclamation-triangle', success: 'check-circle', } as const; + +const COLOR_MAP: Record = { + info: 'text-base', + // eslint-disable-next-line @typescript-eslint/naming-convention + 'info-light': 'text-base', + warning: 'warning', + // eslint-disable-next-line @typescript-eslint/naming-convention + 'warning-light': 'warning', + danger: 'danger', + success: 'success', +}; + type IconMap = typeof ICON_MAP; interface InfoTipProps { @@ -38,7 +52,7 @@ const props = withDefaults(defineProps(), { const iconData = computed<{ icon: IconMap[keyof IconMap]; color: IconColor }>(() => { return { icon: ICON_MAP[props.theme], - color: props.theme === 'info' || props.theme === 'info-light' ? 'text-base' : props.theme, + color: COLOR_MAP[props.theme], } as const; }); diff --git a/packages/editor-ui/src/components/RunInfo.vue b/packages/editor-ui/src/components/RunInfo.vue index f4e68657e5cf8..0f9329be367e7 100644 --- a/packages/editor-ui/src/components/RunInfo.vue +++ b/packages/editor-ui/src/components/RunInfo.vue @@ -3,6 +3,7 @@ import type { ITaskData } from 'n8n-workflow'; import { convertToDisplayDateComponents } from '@/utils/formatters/dateFormatter'; import { computed } from 'vue'; import { useI18n } from '@/composables/useI18n'; +import { N8nInfoTip } from 'n8n-design-system'; const i18n = useI18n(); @@ -33,9 +34,9 @@ const runMetadata = computed(() => { diff --git a/packages/editor-ui/src/plugins/icons/custom.ts b/packages/editor-ui/src/plugins/icons/custom.ts index 86201ffecdf0d..e503f4a342461 100644 --- a/packages/editor-ui/src/plugins/icons/custom.ts +++ b/packages/editor-ui/src/plugins/icons/custom.ts @@ -57,6 +57,6 @@ export const faTriangle: IconDefinition = { 512, [], '', - 'M193.646 92C221.359 44 290.641 44 318.354 92L456.918 332C484.631 380 449.99 440 394.564 440H117.436C62.0104 440 27.3694 380 55.0822 332L193.646 92ZM131.292 360H380.708L256 144L131.292 360Z', + 'M214.433 56C232.908 23.9999 279.096 24.0001 297.571 56L477.704 368C496.18 400 473.085 440 436.135 440H75.8685C38.918 440 15.8241 400 34.2993 368L214.433 56ZM256.002 144L131.294 360H380.709L256.002 144Z', ], }; From 89ced4a2cffcd9ad251a2064470cf7afd1e8105e Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 21 Feb 2025 16:43:46 +0100 Subject: [PATCH 50/52] show run count next to the warning icon --- .../render-types/parts/CanvasNodeStatusIcons.vue | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue index 1ed3a14439da0..2edbdc68870a7 100644 --- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue +++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/parts/CanvasNodeStatusIcons.vue @@ -72,18 +72,17 @@ const dirtiness = computed(() => >
-
+
- +
+ + {{ runDataIterations }} +
display: flex; align-items: center; gap: var(--spacing-5xs); + font-weight: 600; } .runData { - font-weight: 600; color: var(--color-success); } From dd5e6544d13e40dc02767c110b255d6e33a03c6d Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 21 Feb 2025 16:45:00 +0100 Subject: [PATCH 51/52] don't turn dirty node's outgoing edge yellow --- .../canvas/elements/edges/CanvasEdge.vue | 2 - .../render-types/CanvasHandleMainOutput.vue | 8 +--- .../render-types/parts/CanvasHandlePlus.vue | 8 +--- .../src/composables/useCanvasMapping.ts | 40 ++++++------------- packages/editor-ui/src/types/canvas.ts | 2 +- 5 files changed, 15 insertions(+), 45 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue index 390f36643a852..591abf02e0b70 100644 --- a/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue +++ b/packages/editor-ui/src/components/canvas/elements/edges/CanvasEdge.vue @@ -67,8 +67,6 @@ const edgeColor = computed(() => { return 'var(--node-type-supplemental-color)'; } else if (props.selected) { return 'var(--color-background-dark)'; - } else if (status.value === 'warning') { - return 'var(--color-warning)'; } else { return 'var(--color-foreground-xdark)'; } diff --git a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue index f05eb74d775ae..457a869c6ebfb 100644 --- a/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue +++ b/packages/editor-ui/src/components/canvas/elements/handles/render-types/CanvasHandleMainOutput.vue @@ -41,13 +41,7 @@ const runDataLabel = computed(() => const isHandlePlusVisible = computed(() => !isConnecting.value || isHovered.value); -const plusType = computed(() => - renderOptions.value.dirtiness !== undefined - ? 'warning' - : runDataTotal.value > 0 - ? 'success' - : 'default', -); +const plusType = computed(() => (runDataTotal.value > 0 ? 'success' : 'default')); const plusLineSize = computed( () => diff --git a/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue b/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue index 0f705f72d32c5..4d28d57417719 100644 --- a/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue +++ b/packages/editor-ui/src/components/canvas/elements/handles/render-types/parts/CanvasHandlePlus.vue @@ -7,7 +7,7 @@ const props = withDefaults( handleClasses?: string; plusSize?: number; lineSize?: number; - type?: 'success' | 'warning' | 'secondary' | 'default'; + type?: 'success' | 'secondary' | 'default'; }>(), { position: 'right', @@ -163,12 +163,6 @@ function onClick(event: MouseEvent) { } } - &.warning { - .line { - stroke: var(--color-warning); - } - } - .plus { &:hover { cursor: pointer; diff --git a/packages/editor-ui/src/composables/useCanvasMapping.ts b/packages/editor-ui/src/composables/useCanvasMapping.ts index 13b7052459b86..b9d43de809ce9 100644 --- a/packages/editor-ui/src/composables/useCanvasMapping.ts +++ b/packages/editor-ui/src/composables/useCanvasMapping.ts @@ -583,36 +583,20 @@ export function useCanvasMapping({ const runDataTotal = nodeExecutionRunDataOutputMapById.value[connection.source]?.[type]?.[index]?.total ?? 0; - function getStatus(): CanvasConnectionData['status'] | undefined { - if (nodeExecutionRunningById.value[connection.source]) { - return 'running'; - } - - if ( - nodePinnedDataById.value[connection.source] && - nodeExecutionRunDataById.value[connection.source] - ) { - return 'pinned'; - } - - if (nodeHasIssuesById.value[connection.source]) { - return 'error'; - } - - const sourceNodeName = connection.data?.source.node; - - if (sourceNodeName && dirtinessByName.value[sourceNodeName] !== undefined) { - return 'warning'; - } - - if (runDataTotal > 0) { - return 'success'; - } - - return undefined; + let status: CanvasConnectionData['status']; + if (nodeExecutionRunningById.value[connection.source]) { + status = 'running'; + } else if ( + nodePinnedDataById.value[connection.source] && + nodeExecutionRunDataById.value[connection.source] + ) { + status = 'pinned'; + } else if (nodeHasIssuesById.value[connection.source]) { + status = 'error'; + } else if (runDataTotal > 0) { + status = 'success'; } - const status = getStatus(); const maxConnections = [ ...nodeInputsById.value[connection.source], ...nodeInputsById.value[connection.target], diff --git a/packages/editor-ui/src/types/canvas.ts b/packages/editor-ui/src/types/canvas.ts index e1c4ec40d8b43..004016b571538 100644 --- a/packages/editor-ui/src/types/canvas.ts +++ b/packages/editor-ui/src/types/canvas.ts @@ -124,7 +124,7 @@ export type CanvasNode = Node; export interface CanvasConnectionData { source: CanvasConnectionPort; target: CanvasConnectionPort; - status?: 'success' | 'error' | 'warning' | 'pinned' | 'running'; + status?: 'success' | 'error' | 'pinned' | 'running'; maxConnections?: number; } From a8258f643623a877adb6ebea07143fc0c4a6498c Mon Sep 17 00:00:00 2001 From: autologie Date: Fri, 21 Feb 2025 17:34:02 +0100 Subject: [PATCH 52/52] fix: removing connection should not make downstream node dirty --- .../src/composables/useNodeDirtiness.test.ts | 12 ++++++++ .../src/composables/useNodeDirtiness.ts | 30 ++++++++++++++----- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts index d51ee7aac2958..d1c1e68956c29 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.test.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.test.ts @@ -190,6 +190,18 @@ describe(useNodeDirtiness, () => { }); }); + describe('removing a connection', () => { + it('should not change dirtiness', () => { + useNodeTypesStore().setNodeTypes(defaultNodeDescriptions); + + setupTestWorkflow('a🚨✅ -> b✅ -> c✅'); + + canvasOperations.deleteConnection({ source: 'a', target: 'b' }, { trackHistory: true }); + + expect(useNodeDirtiness().dirtinessByName.value).toEqual({}); + }); + }); + describe('enabling/disabling nodes', () => { it('should mark downstream nodes dirty if the node is set to disabled', () => { setupTestWorkflow('a🚨✅ -> b✅ -> c✅ -> d✅'); diff --git a/packages/editor-ui/src/composables/useNodeDirtiness.ts b/packages/editor-ui/src/composables/useNodeDirtiness.ts index b17710faccf64..8dc2556d882a3 100644 --- a/packages/editor-ui/src/composables/useNodeDirtiness.ts +++ b/packages/editor-ui/src/composables/useNodeDirtiness.ts @@ -4,6 +4,7 @@ import { BulkCommand, EnableNodeToggleCommand, RemoveConnectionCommand, + RemoveNodeCommand, type Undoable, } from '@/models/history'; import { useHistoryStore } from '@/stores/history.store'; @@ -19,12 +20,19 @@ import { computed } from 'vue'; function shouldCommandMarkDirty( command: Undoable, nodeName: string, + siblingCommands: Undoable[], getIncomingConnections: (nodeName: string) => INodeConnections, getOutgoingConnectors: (nodeName: string) => INodeConnections, ): boolean { if (command instanceof BulkCommand) { return command.commands.some((cmd) => - shouldCommandMarkDirty(cmd, nodeName, getIncomingConnections, getOutgoingConnectors), + shouldCommandMarkDirty( + cmd, + nodeName, + command.commands, + getIncomingConnections, + getOutgoingConnectors, + ), ); } @@ -32,18 +40,25 @@ function shouldCommandMarkDirty( return command.connectionData[1]?.node === nodeName; } + if (command instanceof RemoveConnectionCommand) { + const [from, to] = command.connectionData; + + if (to.node !== nodeName) { + return false; + } + + // the connection was removed along with its source node + return siblingCommands.some( + (sibling) => sibling instanceof RemoveNodeCommand && sibling.node.name === from.node, + ); + } + const incomingNodes = Object.values(getIncomingConnections(nodeName)) .flat() .flat() .filter((connection) => connection !== null) .map((connection) => connection.node); - if (command instanceof RemoveConnectionCommand) { - const [from, to] = command.connectionData; - - return to.node === nodeName && !incomingNodes.includes(from.node); - } - if (command instanceof AddNodeCommand) { return incomingNodes.includes(command.node.name); } @@ -142,6 +157,7 @@ export function useNodeDirtiness() { shouldCommandMarkDirty( command, nodeName, + [], workflowsStore.incomingConnectionsByNodeName, workflowsStore.outgoingConnectionsByNodeName, )