mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-19 16:55:02 +02:00
fix(Form Trigger Node): Honor the final submission response setting
This commit is contained in:
parent
fc0142edfa
commit
d15873fcbb
117
packages/cli/src/webhooks/__tests__/form-response-mode.test.ts
Normal file
117
packages/cli/src/webhooks/__tests__/form-response-mode.test.ts
Normal file
|
|
@ -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<INodeTypes>();
|
||||
nodeTypes.getByNameAndVersion.mockReturnValue(
|
||||
mock<INodeType>({
|
||||
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<ITaskData>()] } },
|
||||
});
|
||||
const context = new WebhookExecutionContext(workflow, form, mock<IWebhookData>(), '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();
|
||||
});
|
||||
});
|
||||
46
packages/cli/src/webhooks/form-response-mode.ts
Normal file
46
packages/cli/src/webhooks/form-response-mode.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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<WebhookResponseMode>(
|
||||
'responseMode',
|
||||
undefined,
|
||||
|
|
|
|||
|
|
@ -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 <b>model</b>",
|
||||
"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",
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
'<button :data-test-id="parameter.name" :disabled="isReadOnly">{{ value }}</button>',
|
||||
'<button :data-test-id="parameter.name" :disabled="isReadOnly">{{ value }} {{ parameter.options[0].description }}</button>',
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -328,6 +328,7 @@ export class Form extends Node {
|
|||
name: 'default',
|
||||
httpMethod: 'POST',
|
||||
responseMode: 'responseNode',
|
||||
responseData: 'noData',
|
||||
path: '',
|
||||
restartWebhook: true,
|
||||
isFullPath: true,
|
||||
|
|
|
|||
|
|
@ -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<IWorkflowBase> {
|
||||
function formWorkflow(
|
||||
webhookId: string,
|
||||
{
|
||||
withNextPage = true,
|
||||
responseMode = 'onReceived',
|
||||
delayAfterLastPage = false,
|
||||
withEnding = false,
|
||||
}: {
|
||||
withNextPage?: boolean;
|
||||
responseMode?: 'onReceived' | 'lastNode';
|
||||
delayAfterLastPage?: boolean;
|
||||
withEnding?: boolean;
|
||||
} = {},
|
||||
): Partial<IWorkflowBase> {
|
||||
return {
|
||||
name: `Form submission feedback ${webhookId}`,
|
||||
nodes: [
|
||||
|
|
@ -16,7 +29,7 @@ function formWorkflow(webhookId: string, withNextPage = true): Partial<IWorkflow
|
|||
webhookId,
|
||||
parameters: {
|
||||
formTitle: 'First page',
|
||||
responseMode: 'onReceived',
|
||||
responseMode,
|
||||
formFields: { values: [{ fieldLabel: 'Name', requiredField: true }] },
|
||||
options: {
|
||||
respondWithOptions: {
|
||||
|
|
@ -49,12 +62,50 @@ function formWorkflow(webhookId: string, withNextPage = true): Partial<IWorkflow
|
|||
},
|
||||
]
|
||||
: []),
|
||||
...(delayAfterLastPage
|
||||
? [
|
||||
{
|
||||
id: 'after-submission',
|
||||
name: 'After submission',
|
||||
type: 'n8n-nodes-base.wait',
|
||||
typeVersion: 1.1,
|
||||
position: [672, 0] as [number, number],
|
||||
parameters: { resume: 'timeInterval', amount: 8, unit: 'seconds' },
|
||||
},
|
||||
]
|
||||
: []),
|
||||
...(withEnding
|
||||
? [
|
||||
{
|
||||
id: 'ending',
|
||||
name: 'Ending',
|
||||
type: 'n8n-nodes-base.form',
|
||||
typeVersion: 2.5,
|
||||
position: [896, 0] as [number, number],
|
||||
parameters: { operation: 'completion', completionTitle: 'Custom ending' },
|
||||
},
|
||||
]
|
||||
: []),
|
||||
],
|
||||
connections: {
|
||||
'Form Trigger': { main: [[{ node: 'Processing', type: 'main', index: 0 }]] },
|
||||
...(withNextPage
|
||||
? { Processing: { main: [[{ node: 'Next page', type: 'main' as const, index: 0 }]] } }
|
||||
: {}),
|
||||
...(delayAfterLastPage
|
||||
? {
|
||||
'Next page': {
|
||||
main: [[{ node: 'After submission', type: 'main' as const, index: 0 }]],
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
...(withEnding
|
||||
? {
|
||||
'After submission': {
|
||||
main: [[{ node: 'Ending', type: 'main' as const, index: 0 }]],
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
@ -88,6 +139,9 @@ test.describe(
|
|||
{ makeUnique: false },
|
||||
);
|
||||
workflowId = created.workflowId;
|
||||
await expect
|
||||
.poll(async () => (await api.webhooks.trigger(`/form/${webhookId}`)).status())
|
||||
.toBe(200);
|
||||
|
||||
let acceptSubmission = () => {};
|
||||
const submissionGate = new Promise<void>((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();
|
||||
});
|
||||
},
|
||||
);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user