fix(n8n Form Node): Find completion page (#11674)

n8n-io · Nov 13, 2024 · ed3ad6d · ed3ad6d
1 parent 9bd79fc
commit ed3ad6d
Show file tree

Hide file tree

Showing 2 changed files with 234 additions and 27 deletions.
diff --git a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts 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<ExecutionRepository>();
+	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<Workflow>({
+				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<Workflow>({
+				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<Workflow>({
+				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<Workflow>({
+				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<Workflow>({
+				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<Workflow>({
+				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<Workflow>({
+				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
@@ -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,
 		});