From 4fdf469190e390fbe8d489328884f42e0102a8e1 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 10 Mar 2025 12:45:07 +0200 Subject: [PATCH] fix(n8n Form Node): Completion page response mode, do not error on execution running (no-changelog) (#13566) --- .../webhooks/__tests__/waiting-forms.test.ts | 24 +++++- packages/cli/src/webhooks/waiting-forms.ts | 25 +++++- packages/cli/src/webhooks/webhook-helpers.ts | 32 +++++++- .../cli/templates/form-trigger.handlebars | 68 ++++++++++++++++ .../src/components/ParameterInputList.test.ts | 78 +++++++++++++++++++ .../src/components/ParameterInputList.vue | 19 +++++ packages/workflow/src/Constants.ts | 2 + packages/workflow/src/Interfaces.ts | 2 +- 8 files changed, 243 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts index 0ae8d5f7d87..5d3696b7a4d 100644 --- a/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts +++ b/packages/cli/src/webhooks/__tests__/waiting-forms.test.ts @@ -1,6 +1,6 @@ import type express from 'express'; 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 { WaitingForms } from '@/webhooks/waiting-forms'; @@ -220,5 +220,27 @@ describe('WaitingForms', () => { expect(execution.data.isTestWebhook).toBe(true); }); + + it('should return status of execution if suffix is WAITING_FORMS_EXECUTION_STATUS', async () => { + const execution = mock({ + status: 'success', + }); + executionRepository.findSingleExecution.mockResolvedValue(execution); + + const res = mock(); + + 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); + }); }); }); diff --git a/packages/cli/src/webhooks/waiting-forms.ts b/packages/cli/src/webhooks/waiting-forms.ts index 2c8a65afe0d..69dbe3dfbef 100644 --- a/packages/cli/src/webhooks/waiting-forms.ts +++ b/packages/cli/src/webhooks/waiting-forms.ts @@ -1,7 +1,12 @@ import { Service } from '@n8n/di'; import type express from 'express'; 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 { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -74,6 +79,22 @@ export class WaitingForms extends WaitingWebhooks { 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) { throw new NotFoundError(`The execution "${executionId}" does not exist.`); } @@ -85,7 +106,7 @@ export class WaitingForms extends WaitingWebhooks { } if (execution.status === 'running') { - throw new ConflictError(`The execution "${executionId}" is running already.`); + return { noWebhookResponse: true }; } let lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 90ec615e247..a41a778e46c 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -108,13 +108,33 @@ export function autoDetectResponseMode( workflow: Workflow, 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') { return undefined; } + if ( - [FORM_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) && - method === 'POST' + workflowStartNode.type === FORM_NODE_TYPE && + workflowStartNode.parameters.operation === 'completion' ) { + return 'onReceived'; + } + if ([FORM_NODE_TYPE, WAIT_NODE_TYPE].includes(workflowStartNode.type) && method === 'POST') { const connectedNodes = workflow.getChildNodes(workflowStartNode.name); for (const nodeName of connectedNodes) { @@ -244,7 +264,7 @@ export async function executeWebhook( '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 // the default that people know as early as possible (probably already testing phase) // that something does not resolve properly. @@ -583,6 +603,12 @@ export async function executeWebhook( responsePromise, ); + if (responseMode === 'formPage' && !didSendResponse) { + res.send({ formWaitingUrl: `${additionalData.formWaitingBaseUrl}/${executionId}` }); + process.nextTick(() => res.end()); + didSendResponse = true; + } + Container.get(Logger).debug( `Started execution of workflow "${workflow.name}" from webhook with execution ID ${executionId}`, { executionId }, diff --git a/packages/cli/templates/form-trigger.handlebars b/packages/cli/templates/form-trigger.handlebars index 4b85be7ab63..84deafd8c12 100644 --- a/packages/cli/templates/form-trigger.handlebars +++ b/packages/cli/templates/form-trigger.handlebars @@ -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) => { const valid = []; e.preventDefault(); @@ -813,6 +863,11 @@ document.querySelector('#submit-btn').disabled = true; document.querySelector('#submit-btn').style.cursor = 'not-allowed'; document.querySelector('#submit-btn span').style.display = 'inline-block'; + + if (window.location.href.includes('form-waiting')) { + intervalId = setTimeout(checkExecutionStatus, interval); + } + fetch('', { method: 'POST', body: formData, @@ -828,6 +883,12 @@ json = JSON.parse(text); } catch (e) {} + if(json?.formWaitingUrl) { + formWaitingUrl = json.formWaitingUrl; + intervalId = setTimeout(checkExecutionStatus, interval); + return; + } + if (json?.redirectURL) { const url = json.redirectURL.includes("://") ? json.redirectURL : "https://" + json.redirectURL; window.location.replace(url); @@ -845,6 +906,13 @@ document.body.innerHTML = text; 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) { diff --git a/packages/frontend/editor-ui/src/components/ParameterInputList.test.ts b/packages/frontend/editor-ui/src/components/ParameterInputList.test.ts index aa5f6a941f1..b3f1386d343 100644 --- a/packages/frontend/editor-ui/src/components/ParameterInputList.test.ts +++ b/packages/frontend/editor-ui/src/components/ParameterInputList.test.ts @@ -3,6 +3,8 @@ import ParameterInputList from './ParameterInputList.vue'; import { createTestingPinia } from '@pinia/testing'; import { mockedStore } from '@/__tests__/utils'; import { useNDVStore } from '@/stores/ndv.store'; +import * as workflowHelpers from '@/composables/useWorkflowHelpers'; + import { TEST_NODE_NO_ISSUES, TEST_PARAMETERS, @@ -11,6 +13,15 @@ import { FIXED_COLLECTION_PARAMETERS, TEST_ISSUE, } 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 () => { const actual = await vi.importActual('vue-router'); @@ -98,4 +109,71 @@ describe('ParameterInputList', () => { ).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(); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/components/ParameterInputList.vue b/packages/frontend/editor-ui/src/components/ParameterInputList.vue index bed01160dde..740e4de47cb 100644 --- a/packages/frontend/editor-ui/src/components/ParameterInputList.vue +++ b/packages/frontend/editor-ui/src/components/ParameterInputList.vue @@ -28,6 +28,7 @@ import { } from '@/constants'; import { useNDVStore } from '@/stores/ndv.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; + import { getMainAuthField, getNodeAuthFields, @@ -114,6 +115,11 @@ const filteredParameters = computedWithControl( if (activeNode && activeNode.type === FORM_TRIGGER_NODE_TYPE) { return updateFormTriggerParameters(parameters, activeNode.name); } + + if (activeNode && activeNode.type === FORM_NODE_TYPE) { + return updateFormParameters(parameters, activeNode.name); + } + if ( activeNode && activeNode.type === WAIT_NODE_TYPE && @@ -267,6 +273,19 @@ function updateWaitParameters(parameters: INodeProperties[], nodeName: string) { 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) { emit('parameterBlur', parameterName); } diff --git a/packages/workflow/src/Constants.ts b/packages/workflow/src/Constants.ts index 0dec1b58383..db399abf1e0 100644 --- a/packages/workflow/src/Constants.ts +++ b/packages/workflow/src/Constants.ts @@ -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 PROJECT_ROOT = '0'; + +export const WAITING_FORMS_EXECUTION_STATUS = 'n8n-execution-status'; diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 9ef38c45326..31706d8ae61 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2047,7 +2047,7 @@ export interface IWebhookResponseData { } export type WebhookResponseData = 'allEntries' | 'firstEntryJson' | 'firstEntryBinary' | 'noData'; -export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode'; +export type WebhookResponseMode = 'onReceived' | 'lastNode' | 'responseNode' | 'formPage'; export interface INodeTypes { getByName(nodeType: string): INodeType | IVersionedNodeType;