From 4ac9266820bee980c43609d3d398d54e40b35776 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 19 Jun 2024 15:10:30 +0200 Subject: [PATCH] fix(core)!: `$(...).[last,first,all]()` defaulting to the first output instead of the output that connects the nodes (#9760) --- packages/cli/BREAKING-CHANGES.md | 12 +++ packages/workflow/src/WorkflowDataProxy.ts | 38 +++++---- .../workflow/test/WorkflowDataProxy.test.ts | 40 +++++++++ .../multiple_outputs_run.json | 84 +++++++++++++++++++ .../multiple_outputs_workflow.json | 84 +++++++++++++++++++ 5 files changed, 244 insertions(+), 14 deletions(-) create mode 100644 packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_run.json create mode 100644 packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_workflow.json diff --git a/packages/cli/BREAKING-CHANGES.md b/packages/cli/BREAKING-CHANGES.md index f03f1b2623..4623a92d5f 100644 --- a/packages/cli/BREAKING-CHANGES.md +++ b/packages/cli/BREAKING-CHANGES.md @@ -2,6 +2,18 @@ This list shows all the versions which include breaking changes and how to upgrade. +## 1.47.0 + +### What changed? + +Calling `$(...).last()` (or `$(...).first()` or `$(...).all()` respectively) without arguments is returning the the last item (or first or all items) of the output that connects the two nodes. Before it was returning the item/items of the first output of that node. + +### When is action necessary? + +If you are using `$(...).last()` (or `$(...).first()` or `$(...)all()` respectively) without arguments for nodes that have multiple outputs (e.g. `If`, `Switch`, `Compare Datasets`, etc.) and you want it to default to the first output. In that case change it to `$(...).last(0)` (or `first` or `all` respectively). + +This does not affect the Array functions `[].last()`, `[].first()`. + ## 1.40.0 ### What changed? diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 4f29e64670..94225c6533 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -619,18 +619,9 @@ export class WorkflowDataProxy { getDataProxy(): IWorkflowDataProxyData { const that = this; - const getNodeOutput = (nodeName?: string, branchIndex?: number, runIndex?: number) => { - let executionData: INodeExecutionData[]; - - if (nodeName === undefined) { - executionData = that.connectionInputData; - } else { - branchIndex = branchIndex || 0; - runIndex = runIndex === undefined ? -1 : runIndex; - executionData = that.getNodeExecutionData(nodeName, false, branchIndex, runIndex); - } - - return executionData; + const getNodeOutput = (nodeName: string, branchIndex: number, runIndex?: number) => { + runIndex = runIndex === undefined ? -1 : runIndex; + return that.getNodeExecutionData(nodeName, false, branchIndex, runIndex); }; // replacing proxies with the actual data. @@ -1073,6 +1064,12 @@ export class WorkflowDataProxy { if (property === 'first') { ensureNodeExecutionData(); return (branchIndex?: number, runIndex?: number) => { + branchIndex = + branchIndex ?? + // default to the output the active node is connected to + that.workflow.getNodeConnectionIndexes(that.activeNodeName, nodeName) + ?.sourceIndex ?? + 0; const executionData = getNodeOutput(nodeName, branchIndex, runIndex); if (executionData[0]) return executionData[0]; return undefined; @@ -1081,6 +1078,12 @@ export class WorkflowDataProxy { if (property === 'last') { ensureNodeExecutionData(); return (branchIndex?: number, runIndex?: number) => { + branchIndex = + branchIndex ?? + // default to the output the active node is connected to + that.workflow.getNodeConnectionIndexes(that.activeNodeName, nodeName) + ?.sourceIndex ?? + 0; const executionData = getNodeOutput(nodeName, branchIndex, runIndex); if (!executionData.length) return undefined; if (executionData[executionData.length - 1]) { @@ -1091,8 +1094,15 @@ export class WorkflowDataProxy { } if (property === 'all') { ensureNodeExecutionData(); - return (branchIndex?: number, runIndex?: number) => - getNodeOutput(nodeName, branchIndex, runIndex); + return (branchIndex?: number, runIndex?: number) => { + branchIndex = + branchIndex ?? + // default to the output the active node is connected to + that.workflow.getNodeConnectionIndexes(that.activeNodeName, nodeName) + ?.sourceIndex ?? + 0; + return getNodeOutput(nodeName, branchIndex, runIndex); + }; } if (property === 'context') { return that.nodeContextGetter(nodeName); diff --git a/packages/workflow/test/WorkflowDataProxy.test.ts b/packages/workflow/test/WorkflowDataProxy.test.ts index 98f0a3959c..bb01d39730 100644 --- a/packages/workflow/test/WorkflowDataProxy.test.ts +++ b/packages/workflow/test/WorkflowDataProxy.test.ts @@ -53,6 +53,46 @@ const getProxyFromFixture = (workflow: IWorkflowBase, run: IRun | null, activeNo }; describe('WorkflowDataProxy', () => { + describe('$(If))', () => { + const fixture = loadFixture('multiple_outputs'); + const proxy = getProxyFromFixture(fixture.workflow, fixture.run, 'Edit Fields'); + + test('last() should use the output the node is connected to by default', () => { + expect(proxy.$('If').last().json.code).toEqual(2); + }); + + test('last(0) should use the first output', () => { + expect(proxy.$('If').last(0)).toBeUndefined(); + }); + + test('last(1) should use the second output', () => { + expect(proxy.$('If').last(1).json.code).toEqual(2); + }); + + test('first() should use the output the node is connected to by default', () => { + expect(proxy.$('If').first().json.code).toEqual(1); + }); + test('first(0) should use the output the node is connected to by default', () => { + expect(proxy.$('If').first(0)).toBeUndefined(); + }); + test('first(1) should use the output the node is connected to by default', () => { + expect(proxy.$('If').first(1).json.code).toEqual(1); + }); + + test('all() should use the output the node is connected to by default', () => { + expect(proxy.$('If').all()[0].json.code).toEqual(1); + expect(proxy.$('If').all()[1].json.code).toEqual(2); + }); + test('all(0) should use the output the node is connected to by default', () => { + expect(proxy.$('If').all(0)[0]).toBeUndefined(); + expect(proxy.$('If').all(0)[1]).toBeUndefined(); + }); + test('all(1) should use the output the node is connected to by default', () => { + expect(proxy.$('If').all(1)[0].json.code).toEqual(1); + expect(proxy.$('If').all(1)[1].json.code).toEqual(2); + }); + }); + describe('Base', () => { const fixture = loadFixture('base'); const proxy = getProxyFromFixture(fixture.workflow, fixture.run, 'End'); diff --git a/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_run.json b/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_run.json new file mode 100644 index 0000000000..d8510298ee --- /dev/null +++ b/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_run.json @@ -0,0 +1,84 @@ +{ + "data": { + "startData": {}, + "resultData": { + "runData": { + "When clicking ‘Test workflow’": [ + { + "hints": [], + "startTime": 1718369813697, + "executionTime": 0, + "source": [], + "executionStatus": "success", + "data": { + "main": [ + [ + { + "json": { + "name": "First item", + "code": 1 + }, + "pairedItem": { + "item": 0 + } + }, + { + "json": { + "name": "Second item", + "code": 2 + }, + "pairedItem": { + "item": 0 + } + } + ] + ] + } + } + ], + "If": [ + { + "hints": [], + "startTime": 1718369813698, + "executionTime": 1, + "source": [ + { + "previousNode": "When clicking ‘Test workflow’" + } + ], + "executionStatus": "success", + "data": { + "main": [ + [], + [ + { + "json": { + "name": "First item", + "code": 1 + }, + "pairedItem": { + "item": 0 + } + }, + { + "json": { + "name": "Second item", + "code": 2 + }, + "pairedItem": { + "item": 1 + } + } + ] + ] + } + } + ] + } + } + }, + "mode": "manual", + "startedAt": "2024-02-08T15:45:18.848Z", + "stoppedAt": "2024-02-08T15:45:18.862Z", + "status": "running" +} diff --git a/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_workflow.json b/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_workflow.json new file mode 100644 index 0000000000..faeb806db2 --- /dev/null +++ b/packages/workflow/test/fixtures/WorkflowDataProxy/multiple_outputs_workflow.json @@ -0,0 +1,84 @@ +{ + "meta": { + "instanceId": "060d2be233778dc6349e0f3fa8d972652e8dff467638325ffc56812c6b66ef1a" + }, + "nodes": [ + { + "parameters": {}, + "id": "656d6d8d-1af6-4af7-ab53-7c4e495ee51c", + "name": "When clicking ‘Test workflow’", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [-300, 1880] + }, + { + "parameters": { + "conditions": { + "options": { + "caseSensitive": true, + "leftValue": "", + "typeValidation": "strict" + }, + "conditions": [ + { + "id": "1fff886f-3d13-4fbf-b0fb-7e2f845937c0", + "leftValue": "={{ false }}", + "rightValue": "", + "operator": { + "type": "boolean", + "operation": "true", + "singleValue": true + } + } + ], + "combinator": "and" + }, + "options": {} + }, + "id": "204feab7-f5e9-458f-8fdb-4b762b184147", + "name": "If", + "type": "n8n-nodes-base.if", + "typeVersion": 2, + "position": [40, 1880] + }, + { + "parameters": { + "assignments": { + "assignments": [] + }, + "options": {} + }, + "id": "ab122060-f8da-475e-b6c6-d9e486289e1f", + "name": "Edit Fields", + "type": "n8n-nodes-base.set", + "typeVersion": 3.3, + "position": [360, 2080] + } + ], + "connections": { + "When clicking ‘Test workflow’": { + "main": [ + [ + { + "node": "If", + "type": "main", + "index": 0 + } + ] + ] + }, + "If": { + "main": [ + [], + [ + { + "node": "Edit Fields", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": {} +}