fix(n8n Form Node): Completion page response mode, do not error on execution running (no-changelog) (#13566)

This commit is contained in:
Michael Kret
2025-03-10 12:45:07 +02:00
committed by GitHub
parent 8cbb188598
commit 4fdf469190
8 changed files with 243 additions and 7 deletions

View File

@@ -1,6 +1,6 @@
import type express from 'express'; import type express from 'express';
import { mock } from 'jest-mock-extended'; import { mock } from 'jest-mock-extended';
import { FORM_NODE_TYPE, type Workflow } from 'n8n-workflow'; import { FORM_NODE_TYPE, WAITING_FORMS_EXECUTION_STATUS, type Workflow } from 'n8n-workflow';
import type { ExecutionRepository } from '@/databases/repositories/execution.repository'; import type { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { WaitingForms } from '@/webhooks/waiting-forms'; import { WaitingForms } from '@/webhooks/waiting-forms';
@@ -220,5 +220,27 @@ describe('WaitingForms', () => {
expect(execution.data.isTestWebhook).toBe(true); expect(execution.data.isTestWebhook).toBe(true);
}); });
it('should return status of execution if suffix is WAITING_FORMS_EXECUTION_STATUS', async () => {
const execution = mock<IExecutionResponse>({
status: 'success',
});
executionRepository.findSingleExecution.mockResolvedValue(execution);
const res = mock<express.Response>();
const result = await waitingForms.executeWebhook(
{
params: {
path: '123',
suffix: WAITING_FORMS_EXECUTION_STATUS,
},
} as WaitingWebhookRequest,
res,
);
expect(result).toEqual({ noWebhookResponse: true });
expect(res.send).toHaveBeenCalledWith(execution.status);
});
}); });
}); });

View File

@@ -1,7 +1,12 @@
import { Service } from '@n8n/di'; import { Service } from '@n8n/di';
import type express from 'express'; import type express from 'express';
import type { IRunData } from 'n8n-workflow'; import type { IRunData } from 'n8n-workflow';
import { FORM_NODE_TYPE, Workflow } from 'n8n-workflow'; import {
FORM_NODE_TYPE,
WAIT_NODE_TYPE,
WAITING_FORMS_EXECUTION_STATUS,
Workflow,
} from 'n8n-workflow';
import { ConflictError } from '@/errors/response-errors/conflict.error'; import { ConflictError } from '@/errors/response-errors/conflict.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error';
@@ -74,6 +79,22 @@ export class WaitingForms extends WaitingWebhooks {
const execution = await this.getExecution(executionId); const execution = await this.getExecution(executionId);
if (suffix === WAITING_FORMS_EXECUTION_STATUS) {
let status: string = execution?.status ?? 'null';
const { node } = execution?.data.executionData?.nodeExecutionStack[0] ?? {};
if (node && status === 'waiting') {
if (node.type === FORM_NODE_TYPE) {
status = 'form-waiting';
}
if (node.type === WAIT_NODE_TYPE && node.parameters.resume === 'form') {
status = 'form-waiting';
}
}
res.send(status);
return { noWebhookResponse: true };
}
if (!execution) { if (!execution) {
throw new NotFoundError(`The execution "${executionId}" does not exist.`); throw new NotFoundError(`The execution "${executionId}" does not exist.`);
} }
@@ -85,7 +106,7 @@ export class WaitingForms extends WaitingWebhooks {
} }
if (execution.status === 'running') { if (execution.status === 'running') {
throw new ConflictError(`The execution "${executionId}" is running already.`); return { noWebhookResponse: true };
} }
let lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; let lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string;

View File

@@ -108,13 +108,33 @@ export function autoDetectResponseMode(
workflow: Workflow, workflow: Workflow,
method: string, method: string,
) { ) {
if (workflowStartNode.type === FORM_TRIGGER_NODE_TYPE && method === 'POST') {
const connectedNodes = workflow.getChildNodes(workflowStartNode.name);
for (const nodeName of connectedNodes) {
const node = workflow.nodes[nodeName];
if (node.type === WAIT_NODE_TYPE && node.parameters.resume !== 'form') {
continue;
}
if ([FORM_NODE_TYPE, WAIT_NODE_TYPE].includes(node.type) && !node.disabled) {
return 'formPage';
}
}
}
if (workflowStartNode.type === WAIT_NODE_TYPE && workflowStartNode.parameters.resume !== 'form') { if (workflowStartNode.type === WAIT_NODE_TYPE && workflowStartNode.parameters.resume !== 'form') {
return undefined; return undefined;
} }
if ( if (
[FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) && workflowStartNode.type === FORM_NODE_TYPE &&
method === 'POST' workflowStartNode.parameters.operation === 'completion'
) { ) {
return 'onReceived';
}
if ([FORM_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) && method === 'POST') {
const connectedNodes = workflow.getChildNodes(workflowStartNode.name); const connectedNodes = workflow.getChildNodes(workflowStartNode.name);
for (const nodeName of connectedNodes) { for (const nodeName of connectedNodes) {
@@ -244,7 +264,7 @@ export async function executeWebhook(
'firstEntryJson', 'firstEntryJson',
); );
if (!['onReceived', 'lastNode', 'responseNode'].includes(responseMode)) { if (!['onReceived', 'lastNode', 'responseNode', 'formPage'].includes(responseMode)) {
// If the mode is not known we error. Is probably best like that instead of using // If the mode is not known we error. Is probably best like that instead of using
// the default that people know as early as possible (probably already testing phase) // the default that people know as early as possible (probably already testing phase)
// that something does not resolve properly. // that something does not resolve properly.
@@ -583,6 +603,12 @@ export async function executeWebhook(
responsePromise, responsePromise,
); );
if (responseMode === 'formPage' && !didSendResponse) {
res.send({ formWaitingUrl: `${additionalData.formWaitingBaseUrl}/${executionId}` });
process.nextTick(() => res.end());
didSendResponse = true;
}
Container.get(Logger).debug( Container.get(Logger).debug(
`Started execution of workflow "${workflow.name}" from webhook with execution ID ${executionId}`, `Started execution of workflow "${workflow.name}" from webhook with execution ID ${executionId}`,
{ executionId }, { executionId },

View File

@@ -769,6 +769,56 @@
}); });
}); });
let interval = 1000;
let timeoutId;
let formWaitingUrl;
const checkExecutionStatus = async () => {
if (!interval) return;
try {
const response = await fetch(`${formWaitingUrl ?? window.location.href}/n8n-execution-status`);
const text = (await response.text()).trim();
if (text === "form-waiting") {
window.location.replace(formWaitingUrl ?? window.location.href);
return;
}
if (text === "success") {
form.style.display = 'none';
document.querySelector('#submitted-form').style.display = 'block';
clearTimeout(timeoutId);
return;
}
if (text === "null") {
form.style.display = 'none';
document.querySelector('#submitted-form').style.display = 'block';
document.querySelector('#submitted-header').textContent = 'Could not get execution status';
document.querySelector('#submitted-content').textContent =
'Make sure "Save successful production executions" is enabled in your workflow settings';
clearTimeout(timeoutId);
return;
}
if(["canceled", "crashed", "error" ].includes(text)) {
form.style.display = 'none';
document.querySelector('#submitted-form').style.display = 'block';
document.querySelector('#submitted-header').textContent = 'Problem submitting response';
document.querySelector('#submitted-content').textContent =
'Please try again or contact support if the problem persists';
clearTimeout(timeoutId);
return;
}
interval = Math.round(interval * 1.1);
timeoutId = setTimeout(checkExecutionStatus, interval);
} catch (error) {
console.error("Error fetching data:", error);
}
};
form.addEventListener('submit', (e) => { form.addEventListener('submit', (e) => {
const valid = []; const valid = [];
e.preventDefault(); e.preventDefault();
@@ -813,6 +863,11 @@
document.querySelector('#submit-btn').disabled = true; document.querySelector('#submit-btn').disabled = true;
document.querySelector('#submit-btn').style.cursor = 'not-allowed'; document.querySelector('#submit-btn').style.cursor = 'not-allowed';
document.querySelector('#submit-btn span').style.display = 'inline-block'; document.querySelector('#submit-btn span').style.display = 'inline-block';
if (window.location.href.includes('form-waiting')) {
intervalId = setTimeout(checkExecutionStatus, interval);
}
fetch('', { fetch('', {
method: 'POST', method: 'POST',
body: formData, body: formData,
@@ -828,6 +883,12 @@
json = JSON.parse(text); json = JSON.parse(text);
} catch (e) {} } catch (e) {}
if(json?.formWaitingUrl) {
formWaitingUrl = json.formWaitingUrl;
intervalId = setTimeout(checkExecutionStatus, interval);
return;
}
if (json?.redirectURL) { if (json?.redirectURL) {
const url = json.redirectURL.includes("://") ? json.redirectURL : "https://" + json.redirectURL; const url = json.redirectURL.includes("://") ? json.redirectURL : "https://" + json.redirectURL;
window.location.replace(url); window.location.replace(url);
@@ -845,6 +906,13 @@
document.body.innerHTML = text; document.body.innerHTML = text;
return; return;
} }
if (text === '') {
// this is empty cleanup response from responsePromise
// no need to keep checking execution status
clearTimeout(timeoutId);
interval = 0;
}
} }
if (response.status === 200) { if (response.status === 200) {

View File

@@ -3,6 +3,8 @@ import ParameterInputList from './ParameterInputList.vue';
import { createTestingPinia } from '@pinia/testing'; import { createTestingPinia } from '@pinia/testing';
import { mockedStore } from '@/__tests__/utils'; import { mockedStore } from '@/__tests__/utils';
import { useNDVStore } from '@/stores/ndv.store'; import { useNDVStore } from '@/stores/ndv.store';
import * as workflowHelpers from '@/composables/useWorkflowHelpers';
import { import {
TEST_NODE_NO_ISSUES, TEST_NODE_NO_ISSUES,
TEST_PARAMETERS, TEST_PARAMETERS,
@@ -11,6 +13,15 @@ import {
FIXED_COLLECTION_PARAMETERS, FIXED_COLLECTION_PARAMETERS,
TEST_ISSUE, TEST_ISSUE,
} from './ParameterInputList.test.constants'; } from './ParameterInputList.test.constants';
import { FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE } from 'n8n-workflow';
import type { INodeUi } from '../Interface';
import type { MockInstance } from 'vitest';
vi.mock('@/composables/useWorkflowHelpers', () => ({
useWorkflowHelpers: vi.fn().mockReturnValue({
getCurrentWorkflow: vi.fn(),
}),
}));
vi.mock('vue-router', async () => { vi.mock('vue-router', async () => {
const actual = await vi.importActual('vue-router'); const actual = await vi.importActual('vue-router');
@@ -98,4 +109,71 @@ describe('ParameterInputList', () => {
).toBeInTheDocument(); ).toBeInTheDocument();
expect(getByText(TEST_ISSUE)).toBeInTheDocument(); expect(getByText(TEST_ISSUE)).toBeInTheDocument();
}); });
describe('updateFormParameters', () => {
const workflowHelpersMock: MockInstance = vi.spyOn(workflowHelpers, 'useWorkflowHelpers');
const formParameters = [
{
displayName: 'TRIGGER NOTICE',
name: 'triggerNotice',
type: 'notice',
default: '',
},
];
afterAll(() => {
workflowHelpersMock.mockRestore();
});
it('should show triggerNotice if Form Trigger not connected', () => {
ndvStore.activeNode = { name: 'From', type: FORM_NODE_TYPE, parameters: {} } as INodeUi;
workflowHelpersMock.mockReturnValue({
getCurrentWorkflow: vi.fn(() => {
return {
getParentNodes: vi.fn(() => []),
nodes: {},
};
}),
});
const { getByText } = renderComponent({
props: {
parameters: formParameters,
nodeValues: {},
},
});
expect(getByText('TRIGGER NOTICE')).toBeInTheDocument();
});
it('should not show triggerNotice if Form Trigger is connected', () => {
ndvStore.activeNode = { name: 'From', type: FORM_NODE_TYPE, parameters: {} } as INodeUi;
workflowHelpersMock.mockReturnValue({
getCurrentWorkflow: vi.fn(() => {
return {
getParentNodes: vi.fn(() => ['Form Trigger']),
nodes: {
'Form Trigger': {
type: FORM_TRIGGER_NODE_TYPE,
parameters: {},
},
},
};
}),
});
const { queryByText } = renderComponent({
props: {
parameters: formParameters,
nodeValues: {},
},
});
const el = queryByText('TRIGGER NOTICE');
expect(el).not.toBeInTheDocument();
});
});
}); });

View File

@@ -28,6 +28,7 @@ import {
} from '@/constants'; } from '@/constants';
import { useNDVStore } from '@/stores/ndv.store'; import { useNDVStore } from '@/stores/ndv.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { import {
getMainAuthField, getMainAuthField,
getNodeAuthFields, getNodeAuthFields,
@@ -114,6 +115,11 @@ const filteredParameters = computedWithControl(
if (activeNode && activeNode.type === FORM_TRIGGER_NODE_TYPE) { if (activeNode && activeNode.type === FORM_TRIGGER_NODE_TYPE) {
return updateFormTriggerParameters(parameters, activeNode.name); return updateFormTriggerParameters(parameters, activeNode.name);
} }
if (activeNode && activeNode.type === FORM_NODE_TYPE) {
return updateFormParameters(parameters, activeNode.name);
}
if ( if (
activeNode && activeNode &&
activeNode.type === WAIT_NODE_TYPE && activeNode.type === WAIT_NODE_TYPE &&
@@ -267,6 +273,19 @@ function updateWaitParameters(parameters: INodeProperties[], nodeName: string) {
return parameters; return parameters;
} }
function updateFormParameters(parameters: INodeProperties[], nodeName: string) {
const workflow = workflowHelpers.getCurrentWorkflow();
const parentNodes = workflow.getParentNodes(nodeName);
const formTriggerName = parentNodes.find(
(node) => workflow.nodes[node].type === FORM_TRIGGER_NODE_TYPE,
);
if (formTriggerName) return parameters.filter((parameter) => parameter.name !== 'triggerNotice');
return parameters;
}
function onParameterBlur(parameterName: string) { function onParameterBlur(parameterName: string) {
emit('parameterBlur', parameterName); emit('parameterBlur', parameterName);
} }

View File

@@ -103,3 +103,5 @@ export const FREE_AI_CREDITS_USED_ALL_CREDITS_ERROR_CODE = 400;
export const FROM_AI_AUTO_GENERATED_MARKER = '/*n8n-auto-generated-fromAI-override*/'; export const FROM_AI_AUTO_GENERATED_MARKER = '/*n8n-auto-generated-fromAI-override*/';
export const PROJECT_ROOT = '0'; export const PROJECT_ROOT = '0';
export const WAITING_FORMS_EXECUTION_STATUS = 'n8n-execution-status';

View File

@@ -2047,7 +2047,7 @@ export interface IWebhookResponseData {
} }
export type WebhookResponseData = 'allEntries' | 'firstEntryJson' | 'firstEntryBinary' | 'noData'; export type WebhookResponseData = 'allEntries' | 'firstEntryJson' | 'firstEntryBinary' | 'noData';
export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode'; export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode' | 'formPage';
export interface INodeTypes { export interface INodeTypes {
getByName(nodeType: string): INodeType | IVersionedNodeType; getByName(nodeType: string): INodeType | IVersionedNodeType;