From d15873fcbbcf1575dc2ac96beaf52741a67dbca2 Mon Sep 17 00:00:00 2001 From: Arvin Ansari Date: Wed, 30 Sep 2026 14:10:50 +0200 Subject: [PATCH] fix(Form Trigger Node): Honor the final submission response setting --- .../__tests__/form-response-mode.test.ts | 117 +++++++++++++++ .../cli/src/webhooks/form-response-mode.ts | 46 ++++++ packages/cli/src/webhooks/webhook-helpers.ts | 12 +- .../frontend/@n8n/i18n/src/locales/en.json | 3 +- .../components/ParameterInputList.test.ts | 19 ++- .../components/ParameterInputList.vue | 18 ++- packages/nodes-base/nodes/Form/Form.node.ts | 1 + .../nodes/form-submission-feedback.spec.ts | 135 +++++++++++++++++- 8 files changed, 336 insertions(+), 15 deletions(-) create mode 100644 packages/cli/src/webhooks/__tests__/form-response-mode.test.ts create mode 100644 packages/cli/src/webhooks/form-response-mode.ts diff --git a/packages/cli/src/webhooks/__tests__/form-response-mode.test.ts b/packages/cli/src/webhooks/__tests__/form-response-mode.test.ts new file mode 100644 index 00000000000..e2b8c01e704 --- /dev/null +++ b/packages/cli/src/webhooks/__tests__/form-response-mode.test.ts @@ -0,0 +1,117 @@ +import { + FORM_NODE_TYPE, + FORM_TRIGGER_NODE_TYPE, + Expression, + createRunExecutionData, + type INode, + type INodeType, + type INodeTypes, + type ITaskData, + type IWebhookData, + Workflow, +} from 'n8n-workflow'; +import { mock } from 'vitest-mock-extended'; + +import { getFormTriggerResponseMode } from '../form-response-mode'; +import { WebhookExecutionContext } from '../webhook-execution-context'; + +describe('getFormTriggerResponseMode', () => { + beforeAll(async () => { + await Expression.initExpressionEngine({ + engine: 'vm', + bridgeTimeout: 1000, + bridgeMemoryLimit: 128, + poolSize: 1, + maxCodeCacheSize: 10, + }); + }); + + afterAll(async () => { + await Expression.disposeExpressionEngine(); + }); + + function setup(responseMode?: string) { + const unusedTrigger: INode = { + id: 'unused', + name: 'Unused trigger', + type: FORM_TRIGGER_NODE_TYPE, + typeVersion: 2.5, + position: [0, 0], + parameters: { responseMode: 'lastNode' }, + }; + const trigger: INode = { + id: 'trigger', + name: 'Form Trigger', + type: FORM_TRIGGER_NODE_TYPE, + typeVersion: 2.5, + position: [0, 200], + parameters: responseMode === undefined ? {} : { responseMode }, + }; + const form: INode = { + id: 'form', + name: 'Form', + type: FORM_NODE_TYPE, + typeVersion: 2.5, + position: [200, 0], + parameters: {}, + }; + const nodeTypes = mock(); + nodeTypes.getByNameAndVersion.mockReturnValue( + mock({ + description: { + properties: [ + { + name: 'responseMode', + displayName: 'Respond When', + type: 'string', + noDataExpression: false, + default: 'onReceived', + }, + ], + }, + }), + ); + const workflow = new Workflow({ + nodes: [unusedTrigger, trigger, form], + connections: { + 'Unused trigger': { main: [[{ node: 'Form', type: 'main', index: 0 }]] }, + 'Form Trigger': { main: [[{ node: 'Form', type: 'main', index: 0 }]] }, + }, + active: false, + nodeTypes, + }); + const runExecutionData = createRunExecutionData({ + resultData: { runData: { 'Form Trigger': [mock()] } }, + }); + const context = new WebhookExecutionContext(workflow, form, mock(), 'manual', {}); + return { context, runExecutionData, trigger }; + } + + it.each([ + ['onReceived', 'onReceived'], + ['lastNode', 'lastNode'], + ['responseNode', 'responseNode'], + [undefined, 'onReceived'], + ['={{ "onReceived" }}', 'onReceived'], + ])('uses %s from the trigger that ran', async (savedMode, expectedMode) => { + const { context, runExecutionData } = setup(savedMode); + await context.workflow.expression.acquireIsolate(); + try { + expect(getFormTriggerResponseMode(context, runExecutionData)).toBe(expectedMode); + } finally { + await context.workflow.expression.releaseIsolate(); + } + }); + + it('keeps the existing mode if no ancestor form trigger ran', () => { + const { context } = setup('onReceived'); + expect(getFormTriggerResponseMode(context, createRunExecutionData())).toBeUndefined(); + expect(getFormTriggerResponseMode(context, undefined)).toBeUndefined(); + }); + + it('ignores disabled triggers', () => { + const { context, runExecutionData, trigger } = setup('onReceived'); + trigger.disabled = true; + expect(getFormTriggerResponseMode(context, runExecutionData)).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/webhooks/form-response-mode.ts b/packages/cli/src/webhooks/form-response-mode.ts new file mode 100644 index 00000000000..9961aa70bde --- /dev/null +++ b/packages/cli/src/webhooks/form-response-mode.ts @@ -0,0 +1,46 @@ +import { + FORM_TRIGGER_NODE_TYPE, + type IRunExecutionData, + type WebhookResponseMode, +} from 'n8n-workflow'; + +import type { WebhookExecutionContext } from './webhook-execution-context'; + +/** Use the trigger that ran when multiple form triggers lead to the same page. */ +export function getFormTriggerResponseMode( + context: WebhookExecutionContext, + runExecutionData: IRunExecutionData | undefined, +): WebhookResponseMode | undefined { + if (!runExecutionData) return undefined; + + const { workflow, workflowStartNode, executionMode, additionalKeys } = context; + const trigger = workflow + .getParentNodes(workflowStartNode.name) + .map((name) => workflow.nodes[name]) + .find( + (node) => + node.type === FORM_TRIGGER_NODE_TYPE && + !node.disabled && + !!runExecutionData.resultData.runData[node.name]?.length, + ); + if (!trigger) return undefined; + + const savedMode = trigger.parameters.responseMode ?? 'onReceived'; + if (typeof savedMode !== 'string') return undefined; + + const responseMode = workflow.expression.getSimpleParameterValue( + trigger, + savedMode, + executionMode, + additionalKeys, + ); + + if ( + responseMode === 'onReceived' || + responseMode === 'lastNode' || + responseMode === 'responseNode' + ) { + return responseMode; + } + return undefined; +} diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index fc5d66e36a8..1cb3f3b6599 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -96,6 +96,7 @@ import { WorkflowRunner } from '@/workflow-runner'; import { toResponseExpectation } from './engine-v2-response-expectation'; import { EngineV2Webhooks } from './engine-v2-webhooks'; +import { getFormTriggerResponseMode } from './form-response-mode'; import { applySandboxCSP, WebhookResponseHeaders, @@ -990,7 +991,7 @@ export async function executeWebhook( responsePropertyName, responseContentType, responseBinaryPropertyName, - } = evaluateResponseOptions(context, req); + } = evaluateResponseOptions(context, req, runExecutionData); if (!SUPPORTED_RESPONSE_MODES.has(responseMode)) { // If the mode is not known we error. Is probably best like that instead of using @@ -1538,12 +1539,19 @@ export async function executeWebhook( /** * Evaluates the response mode, code and data for a webhook node */ -function evaluateResponseOptions(context: WebhookExecutionContext, req: WebhookRequest) { +function evaluateResponseOptions( + context: WebhookExecutionContext, + req: WebhookRequest, + runExecutionData: IRunExecutionData | undefined, +) { const { workflow, workflowStartNode } = context; //check if response mode should be set automatically, e.g. multipage form const responseMode = autoDetectResponseMode(workflowStartNode, workflow, req.method) ?? + (workflowStartNode.type === FORM_NODE_TYPE && req.method === 'POST' + ? getFormTriggerResponseMode(context, runExecutionData) + : undefined) ?? context.evaluateSimpleWebhookDescriptionExpression( 'responseMode', undefined, diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index cf608e536d5..595d2aeaea6 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -2766,7 +2766,8 @@ "parameterInputList.callout.dismiss.confirm.confirmButtonText": "Confirm", "parameterInputList.callout.dismiss.confirm.cancelButtonText": "Cancel", "parameterInputList.autoRequiresChatTriggerDescription": "Available when connected to a Chat Trigger node.", - "parameterInputList.multiPageFormResponseNotice": "This setting applies to single-page forms. Your workflow has more form pages. Each submission continues to the next page when it is ready.", + "parameterInputList.multiPageFormResponseNotice": "This setting controls the confirmation after the last form page is submitted. Earlier pages continue to the next page when it is ready.", + "parameterInputList.multiPageFormSubmissionDescription": "When all forms in this workflow have been submitted", "parameterOverride.overridePanelText": "Defined automatically by the model", "parameterOverride.applyOverrideButtonTooltip": "Let the model define this parameter", "parameterOverride.descriptionTooltip": "Explain to the LLM how it should generate this value, a good, specific description would allow LLMs to produce expected results much more often", diff --git a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.test.ts b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.test.ts index 60cbb2015a0..4fa6b15e1dc 100644 --- a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.test.ts +++ b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.test.ts @@ -1040,7 +1040,11 @@ describe('ParameterInputList', () => { name: 'responseMode', type: 'options', options: [ - { name: 'Form Is Submitted', value: 'onReceived' }, + { + name: 'Form Is Submitted', + value: 'onReceived', + description: 'As soon as this node receives the form submission', + }, { name: 'Workflow Finishes', value: 'lastNode' }, ], default: 'onReceived', @@ -1228,7 +1232,7 @@ describe('ParameterInputList', () => { ParameterInputFull: { props: ['parameter', 'isReadOnly', 'value'], template: - '', + '', }, }, }, @@ -1237,18 +1241,23 @@ describe('ParameterInputList', () => { const control = getByTestId('responseMode'); expect(control).toHaveTextContent(responseMode); + expect(control).toBeEnabled(); if (multiPage) { - expect(control).toBeDisabled(); + expect(control).toHaveTextContent( + 'parameterInputList.multiPageFormSubmissionDescription', + ); expect(queryByText('parameterInputList.multiPageFormResponseNotice')).toBeInTheDocument(); lastNode.disabled = true; - await waitFor(() => expect(control).toBeEnabled()); + await waitFor(() => + expect(control).toHaveTextContent('As soon as this node receives the form submission'), + ); expect( queryByText('parameterInputList.multiPageFormResponseNotice'), ).not.toBeInTheDocument(); expect(control).toHaveTextContent(responseMode); } else { - expect(control).toBeEnabled(); + expect(control).toHaveTextContent('As soon as this node receives the form submission'); expect( queryByText('parameterInputList.multiPageFormResponseNotice'), ).not.toBeInTheDocument(); diff --git a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.vue b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.vue index 4bfd60988ad..a110371b6c1 100644 --- a/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.vue +++ b/packages/frontend/editor-ui/src/features/ndv/parameters/components/ParameterInputList.vue @@ -262,9 +262,7 @@ throttledWatch( filteredParameters.map(async (parameter) => { const parameterPath = getPath(parameter.name); const isMultipleValues = multipleValues(parameter); - const isDisabled = - (disabledMap[parameterPath] ?? false) || - (hasFollowingFormPage.value && parameter.name === 'responseMode'); + const isDisabled = disabledMap[parameterPath] ?? false; const showOptions = shouldShowOptions(parameter); const dependentParametersValues = await getDependentParametersValues(parameter); const issues = getParameterIssues(parameter); @@ -380,7 +378,19 @@ function updateFormTriggerParameters(parameters: INodeProperties[]) { for (const parameter of parameters) { if (parameter.name === 'responseMode') { - triggerParameters.push(parameter); + triggerParameters.push({ + ...parameter, + options: parameter.options?.map((option) => + 'value' in option && option.value === 'onReceived' + ? { + ...option, + description: i18n.baseText( + 'parameterInputList.multiPageFormSubmissionDescription', + ), + } + : option, + ), + }); triggerParameters.push({ displayName: i18n.baseText('parameterInputList.multiPageFormResponseNotice'), name: 'formResponseModeNotice', diff --git a/packages/nodes-base/nodes/Form/Form.node.ts b/packages/nodes-base/nodes/Form/Form.node.ts index 3995b1a1a57..c98daf9599b 100644 --- a/packages/nodes-base/nodes/Form/Form.node.ts +++ b/packages/nodes-base/nodes/Form/Form.node.ts @@ -328,6 +328,7 @@ export class Form extends Node { name: 'default', httpMethod: 'POST', responseMode: 'responseNode', + responseData: 'noData', path: '', restartWebhook: true, isFullPath: true, diff --git a/packages/testing/playwright/tests/e2e/nodes/form-submission-feedback.spec.ts b/packages/testing/playwright/tests/e2e/nodes/form-submission-feedback.spec.ts index 7ef019cc79b..1d31fe786c8 100644 --- a/packages/testing/playwright/tests/e2e/nodes/form-submission-feedback.spec.ts +++ b/packages/testing/playwright/tests/e2e/nodes/form-submission-feedback.spec.ts @@ -3,7 +3,20 @@ import type { IWorkflowBase } from 'n8n-workflow'; import { test, expect } from '../../../fixtures/base'; import { PublicFormPage } from '../../../pages/PublicFormPage'; -function formWorkflow(webhookId: string, withNextPage = true): Partial { +function formWorkflow( + webhookId: string, + { + withNextPage = true, + responseMode = 'onReceived', + delayAfterLastPage = false, + withEnding = false, + }: { + withNextPage?: boolean; + responseMode?: 'onReceived' | 'lastNode'; + delayAfterLastPage?: boolean; + withEnding?: boolean; + } = {}, +): Partial { return { name: `Form submission feedback ${webhookId}`, nodes: [ @@ -16,7 +29,7 @@ function formWorkflow(webhookId: string, withNextPage = true): Partial (await api.webhooks.trigger(`/form/${webhookId}`)).status()) + .toBe(200); let acceptSubmission = () => {}; const submissionGate = new Promise((resolve) => { @@ -150,6 +204,9 @@ test.describe( { makeUnique: false }, ); workflowId = created.workflowId; + await expect + .poll(async () => (await api.webhooks.trigger(`/form/${webhookId}`)).status()) + .toBe(200); await context.route(`**/form/${webhookId}`, async (route) => { if (route.request().method() === 'POST') { @@ -182,12 +239,15 @@ test.describe( const webhookId = crypto.randomUUID(); const created = await api.workflows.importWorkflowFromDefinition( { - ...formWorkflow(webhookId, false), + ...formWorkflow(webhookId, { withNextPage: false }), active: true, }, { makeUnique: false }, ); workflowId = created.workflowId; + await expect + .poll(async () => (await api.webhooks.trigger(`/form/${webhookId}`)).status()) + .toBe(200); const form = await PublicFormPage.fromNewTab(context, `${baseURL}/form/${webhookId}`); await form.fillField('Name', 'Alex'); @@ -196,5 +256,74 @@ test.describe( await expect(form.waitingCard).toBeHidden(); await form.close(); }); + + for (const responseMode of ['onReceived', 'lastNode'] as const) { + test(`confirms the last page at the selected time with ${responseMode}`, async ({ + api, + context, + baseURL, + }) => { + const webhookId = crypto.randomUUID(); + const created = await api.workflows.importWorkflowFromDefinition( + { + ...formWorkflow(webhookId, { responseMode, delayAfterLastPage: true }), + active: true, + }, + { makeUnique: false }, + ); + workflowId = created.workflowId; + await expect + .poll(async () => (await api.webhooks.trigger(`/form/${webhookId}`)).status()) + .toBe(200); + const form = await PublicFormPage.fromNewTab(context, `${baseURL}/form/${webhookId}`); + await form.fillField('Name', 'Alex'); + await form.submit(); + await form.fillField('City', 'Madrid'); + const [execution] = await api.workflows.getExecutions(workflowId); + await form.submit(); + await expect + .poll(async () => (await api.workflows.getExecution(execution.id)).status) + .toBe('running'); + + await expect(form.submittedCard).toBeVisible({ + visible: responseMode === 'onReceived', + timeout: 3000, + }); + await expect(form.submitSpinner).toBeVisible({ visible: responseMode === 'lastNode' }); + expect((await api.workflows.getExecution(execution.id)).status).toBe('running'); + await expect(form.waitingCard).toBeHidden(); + expect((await api.workflows.waitForExecutionById(execution.id)).status).toBe('success'); + await form.expectText('Your response has been recorded'); + await form.close(); + }); + } + + test('opens an explicit Form Ending after the final submission', async ({ + api, + context, + baseURL, + }) => { + const webhookId = crypto.randomUUID(); + const created = await api.workflows.importWorkflowFromDefinition( + { + ...formWorkflow(webhookId, { delayAfterLastPage: true, withEnding: true }), + active: true, + }, + { makeUnique: false }, + ); + workflowId = created.workflowId; + await expect + .poll(async () => (await api.webhooks.trigger(`/form/${webhookId}`)).status()) + .toBe(200); + const form = await PublicFormPage.fromNewTab(context, `${baseURL}/form/${webhookId}`); + await form.fillField('Name', 'Alex'); + await form.submit(); + await form.fillField('City', 'Madrid'); + await form.submit(); + await expect(form.waitingCard).toBeVisible(); + await expect(form.submittedCard).toBeHidden(); + await form.expectText('Custom ending', { timeout: 15000 }); + await form.close(); + }); }, );