feat(n8n Form Trigger Node): Improvements (#7571)

Github issue / Community forum post (link here to close automatically): --------- Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <[email protected]> Co-authored-by: Giulio Andreini <[email protected]>
n8n-io · Dec 13, 2023 · 953a58f · 953a58f
1 parent 26f0d57
commit 953a58f
Show file tree

Hide file tree

Showing 37 changed files with 1,152 additions and 485 deletions.
diff --git a/cypress/e2e/16-form-trigger-node.cy.ts b/cypress/e2e/16-form-trigger-node.cy.ts
@@ -1,7 +1,5 @@
 import { WorkflowPage, NDV } from '../pages';
-import { v4 as uuid } from 'uuid';
-import { getPopper, getVisiblePopper, getVisibleSelect } from '../utils';
-import { META_KEY } from '../constants';
+import { getVisibleSelect } from '../utils';
 
 const workflowPage = new WorkflowPage();
 const ndv = new NDV();
@@ -76,12 +74,25 @@ describe('n8n Form Trigger', () => {
 		)
 			.find('input')
 			.type('Option 2');
-		//add optionall submitted message
-		cy.get('.param-options > .button').click();
-		cy.get('.indent > .parameter-item')
-			.find('input')
+
+		//add optional submitted message
+		cy.get('.param-options').click();
+		cy.contains('span', 'Text to Show')
+			.should('exist')
+			.parent()
+			.parent()
+			.next()
+			.children()
+			.children()
+			.children()
+			.children()
+			.children()
+			.children()
+			.children()
+			.first()
 			.clear()
 			.type('Your test form was successfully submitted');
+
 		ndv.getters.backToCanvas().click();
 		workflowPage.getters.nodeIssuesByName('n8n Form Trigger').should('not.exist');
 	});

diff --git a/packages/cli/src/AbstractServer.ts b/packages/cli/src/AbstractServer.ts
@@ -14,6 +14,7 @@ import { ExternalHooks } from '@/ExternalHooks';
 import { send, sendErrorResponse } from '@/ResponseHelper';
 import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares';
 import { TestWebhooks } from '@/TestWebhooks';
+import { WaitingForms } from '@/WaitingForms';
 import { WaitingWebhooks } from '@/WaitingWebhooks';
 import { webhookRequestHandler } from '@/WebhookHelpers';
 import { generateHostInstanceId } from './databases/utils/generators';
@@ -39,6 +40,12 @@ export abstract class AbstractServer {
 
 	protected restEndpoint: string;
 
+	protected endpointForm: string;
+
+	protected endpointFormTest: string;
+
+	protected endpointFormWaiting: string;
+
 	protected endpointWebhook: string;
 
 	protected endpointWebhookTest: string;
@@ -63,6 +70,11 @@ export abstract class AbstractServer {
 		this.sslCert = config.getEnv('ssl_cert');
 
 		this.restEndpoint = config.getEnv('endpoints.rest');
+
+		this.endpointForm = config.getEnv('endpoints.form');
+		this.endpointFormTest = config.getEnv('endpoints.formTest');
+		this.endpointFormWaiting = config.getEnv('endpoints.formWaiting');
+
 		this.endpointWebhook = config.getEnv('endpoints.webhook');
 		this.endpointWebhookTest = config.getEnv('endpoints.webhookTest');
 		this.endpointWebhookWaiting = config.getEnv('endpoints.webhookWaiting');
@@ -165,10 +177,21 @@ export abstract class AbstractServer {
 
 		// Setup webhook handlers before bodyParser, to let the Webhook node handle binary data in requests
 		if (this.webhooksEnabled) {
+			const activeWorkflowRunner = Container.get(ActiveWorkflowRunner);
+
+			// Register a handler for active forms
+			this.app.all(`/${this.endpointForm}/:path(*)`, webhookRequestHandler(activeWorkflowRunner));
+
 			// Register a handler for active webhooks
 			this.app.all(
 				`/${this.endpointWebhook}/:path(*)`,
-				webhookRequestHandler(Container.get(ActiveWorkflowRunner)),
+				webhookRequestHandler(activeWorkflowRunner),
+			);
+
+			// Register a handler for waiting forms
+			this.app.all(
+				`/${this.endpointFormWaiting}/:path/:suffix?`,
+				webhookRequestHandler(Container.get(WaitingForms)),
 			);
 
 			// Register a handler for waiting webhooks
@@ -181,7 +204,8 @@ export abstract class AbstractServer {
 		if (this.testWebhooksEnabled) {
 			const testWebhooks = Container.get(TestWebhooks);
 
-			// Register a handler for test webhooks
+			// Register a handler
+			this.app.all(`/${this.endpointFormTest}/:path(*)`, webhookRequestHandler(testWebhooks));
 			this.app.all(`/${this.endpointWebhookTest}/:path(*)`, webhookRequestHandler(testWebhooks));
 
 			// Removes a test webhook

diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts
@@ -2,7 +2,11 @@
 import type { Request, Response } from 'express';
 import { parse, stringify } from 'flatted';
 import picocolors from 'picocolors';
-import { ErrorReporterProxy as ErrorReporter, NodeApiError } from 'n8n-workflow';
+import {
+	ErrorReporterProxy as ErrorReporter,
+	FORM_TRIGGER_PATH_IDENTIFIER,
+	NodeApiError,
+} from 'n8n-workflow';
 import { Readable } from 'node:stream';
 import type {
 	IExecutionDb,
@@ -67,6 +71,20 @@ export function sendErrorResponse(res: Response, error: Error) {
 			console.error(picocolors.red(error.httpStatusCode), error.message);
 		}
 
+		//render custom 404 page for form triggers
+		const { originalUrl } = res.req;
+		if (error.errorCode === 404 && originalUrl) {
+			const basePath = originalUrl.split('/')[1];
+			const isLegacyFormTrigger = originalUrl.includes(FORM_TRIGGER_PATH_IDENTIFIER);
+			const isFormTrigger = basePath.includes('form');
+
+			if (isFormTrigger || isLegacyFormTrigger) {
+				const isTestWebhook = basePath.includes('test');
+				res.status(404);
+				return res.render('form-trigger-404', { isTestWebhook });
+			}
+		}
+
 		httpStatusCode = error.httpStatusCode;
 
 		if (error.errorCode) {

diff --git a/packages/cli/src/WaitingForms.ts b/packages/cli/src/WaitingForms.ts
@@ -0,0 +1,19 @@
+import { Service } from 'typedi';
+
+import type { IExecutionResponse } from '@/Interfaces';
+import { WaitingWebhooks } from '@/WaitingWebhooks';
+
+@Service()
+export class WaitingForms extends WaitingWebhooks {
+	protected override includeForms = true;
+
+	protected override logReceivedWebhook(method: string, executionId: string) {
+		this.logger.debug(`Received waiting-form "${method}" for execution "${executionId}"`);
+	}
+
+	protected disableNode(execution: IExecutionResponse, method?: string) {
+		if (method === 'POST') {
+			execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
+		}
+	}
+}
diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts
@@ -5,6 +5,7 @@ import type express from 'express';
 import * as WebhookHelpers from '@/WebhookHelpers';
 import { NodeTypes } from '@/NodeTypes';
 import type {
+	IExecutionResponse,
 	IResponseCallbackData,
 	IWebhookManager,
 	IWorkflowDb,
@@ -19,21 +20,32 @@ import { NotFoundError } from './errors/response-errors/not-found.error';
 
 @Service()
 export class WaitingWebhooks implements IWebhookManager {
+	protected includeForms = false;
+
 	constructor(
-		private readonly logger: Logger,
+		protected readonly logger: Logger,
 		private readonly nodeTypes: NodeTypes,
 		private readonly executionRepository: ExecutionRepository,
 		private readonly ownershipService: OwnershipService,
 	) {}
 
 	// TODO: implement `getWebhookMethods` for CORS support
 
+	protected logReceivedWebhook(method: string, executionId: string) {
+		this.logger.debug(`Received waiting-webhook "${method}" for execution "${executionId}"`);
+	}
+
+	protected disableNode(execution: IExecutionResponse, _method?: string) {
+		execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
+	}
+
 	async executeWebhook(
 		req: WaitingWebhookRequest,
 		res: express.Response,
 	): Promise<IResponseCallbackData> {
 		const { path: executionId, suffix } = req.params;
-		this.logger.debug(`Received waiting-webhook "${req.method}" for execution "${executionId}"`);
+
+		this.logReceivedWebhook(req.method, executionId);
 
 		// Reset request parameters
 		req.params = {} as WaitingWebhookRequest['params'];
@@ -55,7 +67,7 @@ export class WaitingWebhooks implements IWebhookManager {
 
 		// Set the node as disabled so that the data does not get executed again as it would result
 		// in starting the wait all over again
-		execution.data.executionData!.nodeExecutionStack[0].node.disabled = true;
+		this.disableNode(execution, req.method);
 
 		// Remove waitTill information else the execution would stop
 		execution.data.waitTill = undefined;
@@ -97,7 +109,8 @@ export class WaitingWebhooks implements IWebhookManager {
 			(webhook) =>
 				webhook.httpMethod === req.method &&
 				webhook.path === (suffix ?? '') &&
-				webhook.webhookDescription.restartWebhook === true,
+				webhook.webhookDescription.restartWebhook === true &&
+				(webhook.webhookDescription.isForm || false) === this.includeForms,
 		);
 
 		if (webhookData === undefined) {

diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts
@@ -37,7 +37,6 @@ import {
 	BINARY_ENCODING,
 	createDeferredPromise,
 	ErrorReporterProxy as ErrorReporter,
-	FORM_TRIGGER_PATH_IDENTIFIER,
 	NodeHelpers,
 } from 'n8n-workflow';
 
@@ -133,16 +132,7 @@ export const webhookRequestHandler =
 		try {
 			response = await webhookManager.executeWebhook(req, res);
 		} catch (error) {
-			if (
-				error.errorCode === 404 &&
-				(error.message as string).includes(FORM_TRIGGER_PATH_IDENTIFIER)
-			) {
-				const isTestWebhook = req.originalUrl.includes('webhook-test');
-				res.status(404);
-				return res.render('form-trigger-404', { isTestWebhook });
-			} else {
-				return ResponseHelper.sendErrorResponse(res, error as Error);
-			}
+			return ResponseHelper.sendErrorResponse(res, error as Error);
 		}
 
 		// Don't respond, if already responded
@@ -560,10 +550,27 @@ export async function executeWebhook(
 					} else {
 						// TODO: This probably needs some more changes depending on the options on the
 						//       Webhook Response node
+						const headers = response.headers;
+						let responseCode = response.statusCode;
+						let data = response.body as IDataObject;
+
+						// for formTrigger node redirection has to be handled by sending redirectURL in response body
+						if (
+							nodeType.description.name === 'formTrigger' &&
+							headers.location &&
+							String(responseCode).startsWith('3')
+						) {
+							responseCode = 200;
+							data = {
+								redirectURL: headers.location,
+							};
+							headers.location = undefined;
+						}
+
 						responseCallback(null, {
-							data: response.body as IDataObject,
-							headers: response.headers,
-							responseCode: response.statusCode,
+							data,
+							headers,
+							responseCode,
 						});
 					}
 

diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts
@@ -963,9 +963,11 @@ export async function getBase(
 ): Promise<IWorkflowExecuteAdditionalData> {
 	const urlBaseWebhook = WebhookHelpers.getWebhookBaseUrl();
 
+	const formWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.formWaiting');
+
 	const webhookBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhook');
-	const webhookWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookWaiting');
 	const webhookTestBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookTest');
+	const webhookWaitingBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookWaiting');
 
 	const variables = await WorkflowHelpers.getVariables();
 
@@ -974,6 +976,7 @@ export async function getBase(
 		executeWorkflow,
 		restApiUrl: urlBaseWebhook + config.getEnv('endpoints.rest'),
 		instanceBaseUrl: urlBaseWebhook,
+		formWaitingBaseUrl,
 		webhookBaseUrl,
 		webhookWaitingBaseUrl,
 		webhookTestBaseUrl,

diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts
@@ -668,6 +668,24 @@ export const schema = {
 			env: 'N8N_ENDPOINT_REST',
 			doc: 'Path for rest endpoint',
 		},
+		form: {
+			format: String,
+			default: 'form',
+			env: 'N8N_ENDPOINT_FORM',
+			doc: 'Path for form endpoint',
+		},
+		formTest: {
+			format: String,
+			default: 'form-test',
+			env: 'N8N_ENDPOINT_FORM_TEST',
+			doc: 'Path for test form endpoint',
+		},
+		formWaiting: {
+			format: String,
+			default: 'form-waiting',
+			env: 'N8N_ENDPOINT_FORM_WAIT',
+			doc: 'Path for waiting form endpoint',
+		},
 		webhook: {
 			format: String,
 			default: 'webhook',

diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts
@@ -81,6 +81,9 @@ export class FrontendService {
 		}
 
 		this.settings = {
+			endpointForm: config.getEnv('endpoints.form'),
+			endpointFormTest: config.getEnv('endpoints.formTest'),
+			endpointFormWaiting: config.getEnv('endpoints.formWaiting'),
 			endpointWebhook: config.getEnv('endpoints.webhook'),
 			endpointWebhookTest: config.getEnv('endpoints.webhookTest'),
 			saveDataErrorExecution: config.getEnv('executions.saveDataOnError'),