From ed3ad6d684597f7c4b7419dfa81d476e66f10eba Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Wed, 13 Nov 2024 13:44:44 +0200 Subject: [PATCH] fix(n8n Form Node): Find completion page (#11674) --- .../webhooks/__tests__/waiting-forms.test.ts | 199 ++++++++++++++++++ packages/cli/src/webhooks/waiting-forms.ts | 62 +++--- 2 files changed, 234 insertions(+), 27 deletions(-) create mode 100644 packages/cli/src/webhooks/__tests__/waiting-forms.test.ts diff --git a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts new file mode 100644 index 0000000000..bec6f95d7f --- /dev/null +++ b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts @@ -0,0 +1,199 @@ +import { mock } from 'jest-mock-extended'; +import { FORM_NODE_TYPE, type Workflow } from 'n8n-workflow'; + +import type { ExecutionRepository } from '@/databases/repositories/execution.repository'; +import { WaitingForms } from '@/webhooks/waiting-forms'; + +describe('WaitingForms', () => { + const executionRepository = mock(); + const waitingWebhooks = new WaitingForms(mock(), mock(), executionRepository); + + beforeEach(() => { + jest.restoreAllMocks(); + }); + + describe('findCompletionPage', () => { + it('should return lastNodeExecuted if it is a non-disabled form completion node', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue([]), + nodes: { + Form1: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + }, + }); + + const result = waitingWebhooks.findCompletionPage(workflow, {}, 'Form1'); + expect(result).toBe('Form1'); + }); + + it('should return undefined if lastNodeExecuted is disabled', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue([]), + nodes: { + Form1: { + disabled: true, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + }, + }); + + const result = waitingWebhooks.findCompletionPage(workflow, {}, 'Form1'); + expect(result).toBeUndefined(); + }); + + it('should return undefined if lastNodeExecuted is not a form node', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue([]), + nodes: { + NonForm: { + disabled: undefined, + type: 'other-node-type', + parameters: {}, + }, + }, + }); + + const result = waitingWebhooks.findCompletionPage(workflow, {}, 'NonForm'); + expect(result).toBeUndefined(); + }); + + it('should return undefined if lastNodeExecuted operation is not completion', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue([]), + nodes: { + Form1: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'page', + }, + }, + }, + }); + + const result = waitingWebhooks.findCompletionPage(workflow, {}, 'Form1'); + expect(result).toBeUndefined(); + }); + + it('should find first valid completion form in parent nodes if lastNodeExecuted is not valid', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue(['Form1', 'Form2', 'Form3']), + nodes: { + LastNode: { + disabled: undefined, + type: 'other-node-type', + parameters: {}, + }, + Form1: { + disabled: true, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + Form2: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + Form3: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + }, + }); + + const runData = { + Form2: [], + Form3: [], + }; + + const result = waitingWebhooks.findCompletionPage(workflow, runData, 'LastNode'); + expect(result).toBe('Form3'); + }); + + it('should return undefined if no valid completion form is found in parent nodes', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue(['Form1', 'Form2']), + nodes: { + LastNode: { + disabled: undefined, + type: 'other-node-type', + parameters: {}, + }, + Form1: { + disabled: true, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + Form2: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'submit', + }, + }, + }, + }); + + const result = waitingWebhooks.findCompletionPage(workflow, {}, 'LastNode'); + expect(result).toBeUndefined(); + }); + + it('should skip parent nodes without runData', () => { + const workflow = mock({ + getParentNodes: jest.fn().mockReturnValue(['Form1', 'Form2', 'Form3']), + nodes: { + LastNode: { + disabled: undefined, + type: 'other-node-type', + parameters: {}, + }, + Form1: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + Form2: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + Form3: { + disabled: undefined, + type: FORM_NODE_TYPE, + parameters: { + operation: 'completion', + }, + }, + }, + }); + + const runData = { + Form2: [], + }; + + const result = waitingWebhooks.findCompletionPage(workflow, runData, 'LastNode'); + expect(result).toBe('Form2'); + }); + }); +}); diff --git a/packages/cli/src/webhooks/waiting-forms.ts b/packages/cli/src/webhooks/waiting-forms.ts index 5a491c1fb3..cd2de74ed0 100644 --- a/packages/cli/src/webhooks/waiting-forms.ts +++ b/packages/cli/src/webhooks/waiting-forms.ts @@ -1,5 +1,6 @@ import axios from 'axios'; import type express from 'express'; +import type { IRunData } from 'n8n-workflow'; import { FORM_NODE_TYPE, sleep, Workflow } from 'n8n-workflow'; import { Service } from 'typedi'; @@ -57,6 +58,29 @@ export class WaitingForms extends WaitingWebhooks { } catch (error) {} } + findCompletionPage(workflow: Workflow, runData: IRunData, lastNodeExecuted: string) { + const parentNodes = workflow.getParentNodes(lastNodeExecuted); + const lastNode = workflow.nodes[lastNodeExecuted]; + + if ( + !lastNode.disabled && + lastNode.type === FORM_NODE_TYPE && + lastNode.parameters.operation === 'completion' + ) { + return lastNodeExecuted; + } else { + return parentNodes.reverse().find((nodeName) => { + const node = workflow.nodes[nodeName]; + return ( + !node.disabled && + node.type === FORM_NODE_TYPE && + node.parameters.operation === 'completion' && + runData[nodeName] + ); + }); + } + } + async executeWebhook( req: WaitingWebhookRequest, res: express.Response, @@ -89,35 +113,19 @@ export class WaitingForms extends WaitingWebhooks { throw new ConflictError(`The execution "${executionId}" is running already.`); } - let completionPage; + let lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; + if (execution.finished) { + // find the completion page to render + // if there is no completion page, render the default page const workflow = this.getWorkflow(execution); - const parentNodes = workflow.getParentNodes( - execution.data.resultData.lastNodeExecuted as string, + const completionPage = this.findCompletionPage( + workflow, + execution.data.resultData.runData, + lastNodeExecuted, ); - const lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; - const lastNode = workflow.nodes[lastNodeExecuted]; - - if ( - !lastNode.disabled && - lastNode.type === FORM_NODE_TYPE && - lastNode.parameters.operation === 'completion' - ) { - completionPage = lastNodeExecuted; - } else { - completionPage = Object.keys(workflow.nodes).find((nodeName) => { - const node = workflow.nodes[nodeName]; - return ( - parentNodes.includes(nodeName) && - !node.disabled && - node.type === FORM_NODE_TYPE && - node.parameters.operation === 'completion' - ); - }); - } - if (!completionPage) { res.render('form-trigger-completion', { title: 'Form Submitted', @@ -128,16 +136,16 @@ export class WaitingForms extends WaitingWebhooks { return { noWebhookResponse: true, }; + } else { + lastNodeExecuted = completionPage; } } - const targetNode = completionPage || (execution.data.resultData.lastNodeExecuted as string); - return await this.getWebhookExecutionData({ execution, req, res, - lastNodeExecuted: targetNode, + lastNodeExecuted, executionId, suffix, });