From 953a58f18bfdd36fa8b526ca6213631aacab49cb Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Wed, 13 Dec 2023 17:00:51 +0200 Subject: [PATCH] feat(n8n Form Trigger Node): Improvements (#7571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Github issue / Community forum post (link here to close automatically): --------- Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ Co-authored-by: Giulio Andreini --- cypress/e2e/16-form-trigger-node.cy.ts | 25 +- packages/cli/src/AbstractServer.ts | 28 +- packages/cli/src/ResponseHelper.ts | 20 +- packages/cli/src/WaitingForms.ts | 19 + packages/cli/src/WaitingWebhooks.ts | 21 +- packages/cli/src/WebhookHelpers.ts | 35 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 5 +- packages/cli/src/config/schema.ts | 18 + packages/cli/src/services/frontend.service.ts | 3 + .../cli/templates/form-trigger.handlebars | 47 ++- packages/cli/test/unit/webhooks.test.ts | 5 +- packages/core/src/NodeExecuteFunctions.ts | 2 + packages/editor-ui/src/Interface.ts | 6 + .../__tests__/server/endpoints/settings.ts | 3 + packages/editor-ui/src/__tests__/utils.ts | 3 + .../completions/execution.completions.ts | 6 +- .../editor-ui/src/components/NodeWebhooks.vue | 2 +- .../src/components/VariableSelector.vue | 1 + .../editor-ui/src/mixins/workflowHelpers.ts | 13 +- packages/editor-ui/src/mixins/workflowRun.ts | 64 ++- packages/editor-ui/src/plugins/i18n/index.ts | 1 + .../src/plugins/i18n/locales/en.json | 1 + .../editor-ui/src/stores/n8nRoot.store.ts | 24 ++ .../editor-ui/src/stores/settings.store.ts | 3 + .../editor-ui/src/stores/workflows.store.ts | 12 + packages/editor-ui/src/views/NodeView.vue | 6 +- .../nodes-base/nodes/Form/FormTrigger.node.ts | 306 +------------- .../nodes/Form/common.descriptions.ts | 252 ++++++++++++ packages/nodes-base/nodes/Form/interfaces.ts | 2 + packages/nodes-base/nodes/Form/utils.ts | 92 ++++- .../nodes/Form/v1/FormTriggerV1.node.ts | 98 +++++ .../nodes/Form/v2/FormTriggerV2.node.ts | 102 +++++ .../RespondToWebhook/RespondToWebhook.node.ts | 25 +- packages/nodes-base/nodes/Wait/Wait.node.ts | 378 +++++++++++------- .../nodes-base/nodes/Webhook/description.ts | 2 +- packages/workflow/src/Interfaces.ts | 9 +- packages/workflow/src/TypeValidation.ts | 20 + 37 files changed, 1163 insertions(+), 496 deletions(-) create mode 100644 packages/cli/src/WaitingForms.ts create mode 100644 packages/nodes-base/nodes/Form/common.descriptions.ts create mode 100644 packages/nodes-base/nodes/Form/v1/FormTriggerV1.node.ts create mode 100644 packages/nodes-base/nodes/Form/v2/FormTriggerV2.node.ts diff --git a/cypress/e2e/16-form-trigger-node.cy.ts b/cypress/e2e/16-form-trigger-node.cy.ts index 27198001a8..1ec2abc640 100644 --- 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 index 66ce5855f0..e97e4ce0a6 100644 --- 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 index 4684aac2a1..5213e3964e 100644 --- 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 new file mode 100644 index 0000000000..0625acd7e4 --- /dev/null +++ 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 index 7368c29369..f91ffe1267 100644 --- 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,8 +20,10 @@ 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, @@ -28,12 +31,21 @@ export class WaitingWebhooks implements IWebhookManager { // 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 { 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 index 2f9441c573..81cb48907c 100644 --- 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 index f49fed11b4..4358be529e 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -963,9 +963,11 @@ export async function getBase( ): Promise { 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 index 0fb2b29dfa..9afb4a7eeb 100644 --- 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 index 9f9297a83a..fe7d6141ba 100644 --- 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'), diff --git a/packages/cli/templates/form-trigger.handlebars b/packages/cli/templates/form-trigger.handlebars index 15ec4a10a4..f89e3c0a7d 100644 --- a/packages/cli/templates/form-trigger.handlebars +++ b/packages/cli/templates/form-trigger.handlebars @@ -385,6 +385,10 @@ + {{#if redirectUrl}} + + {{/if}} +