fix(n8n Form Trigger Node): Do not rerun trigger when it has run data (#10687)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions

This commit is contained in:
Michael Kret 2024-09-05 20:09:38 +03:00 committed by GitHub
parent 37a808896e
commit 3adbcab27d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 228 additions and 57 deletions

View file

@ -6,7 +6,6 @@ import type {
IWorkflowDb, IWorkflowDb,
} from '@/Interface'; } from '@/Interface';
import type { import type {
IDataObject,
IRunData, IRunData,
IRunExecutionData, IRunExecutionData,
ITaskData, ITaskData,
@ -14,24 +13,20 @@ import type {
Workflow, Workflow,
StartNodeData, StartNodeData,
IRun, IRun,
INode,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { NodeConnectionType } from 'n8n-workflow'; import { NodeConnectionType } from 'n8n-workflow';
import { useToast } from '@/composables/useToast'; import { useToast } from '@/composables/useToast';
import { useNodeHelpers } from '@/composables/useNodeHelpers'; import { useNodeHelpers } from '@/composables/useNodeHelpers';
import { import { CHAT_TRIGGER_NODE_TYPE, WORKFLOW_LM_CHAT_MODAL_KEY } from '@/constants';
CHAT_TRIGGER_NODE_TYPE,
FORM_TRIGGER_NODE_TYPE,
WAIT_NODE_TYPE,
WORKFLOW_LM_CHAT_MODAL_KEY,
} from '@/constants';
import { useTitleChange } from '@/composables/useTitleChange'; import { useTitleChange } from '@/composables/useTitleChange';
import { useRootStore } from '@/stores/root.store'; import { useRootStore } from '@/stores/root.store';
import { useUIStore } from '@/stores/ui.store'; import { useUIStore } from '@/stores/ui.store';
import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { useWorkflowsStore } from '@/stores/workflows.store'; import { useWorkflowsStore } from '@/stores/workflows.store';
import { openPopUpWindow } from '@/utils/executionUtils'; import { displayForm } from '@/utils/executionUtils';
import { useExternalHooks } from '@/composables/useExternalHooks'; import { useExternalHooks } from '@/composables/useExternalHooks';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers'; import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import type { useRouter } from 'vue-router'; import type { useRouter } from 'vue-router';
@ -261,36 +256,18 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType<typeof u
const runWorkflowApiResponse = await runWorkflowApi(startRunData); const runWorkflowApiResponse = await runWorkflowApi(startRunData);
const pinData = workflowData.pinData ?? {}; const pinData = workflowData.pinData ?? {};
for (const node of workflowData.nodes) { const getTestUrl = (() => {
if (pinData[node.name]) continue; return (node: INode) => {
if (![FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(node.type)) {
continue;
}
if (
options.destinationNode &&
options.destinationNode !== node.name &&
!directParentNodes.includes(node.name)
) {
continue;
}
if (node.name === options.destinationNode || !node.disabled) {
let testUrl = '';
if (node.type === FORM_TRIGGER_NODE_TYPE) {
const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion); const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion);
if (nodeType?.webhooks?.length) { if (nodeType?.webhooks?.length) {
testUrl = workflowHelpers.getWebhookUrl(nodeType.webhooks[0], node, 'test'); return workflowHelpers.getWebhookUrl(nodeType.webhooks[0], node, 'test');
}
} }
return '';
};
})();
if ( const shouldShowForm = (() => {
node.type === WAIT_NODE_TYPE && return (node: INode) => {
node.parameters.resume === 'form' &&
runWorkflowApiResponse.executionId
) {
const workflowTriggerNodes = workflow const workflowTriggerNodes = workflow
.getTriggerNodes() .getTriggerNodes()
.map((triggerNode) => triggerNode.name); .map((triggerNode) => triggerNode.name);
@ -301,18 +278,22 @@ export function useRunWorkflow(useRunWorkflowOpts: { router: ReturnType<typeof u
workflowTriggerNodes.some((triggerNode) => workflowTriggerNodes.some((triggerNode) =>
workflowsStore.isNodeInOutgoingNodeConnections(triggerNode, node.name), workflowsStore.isNodeInOutgoingNodeConnections(triggerNode, node.name),
); );
return showForm;
};
})();
if (!showForm) continue; displayForm({
nodes: workflowData.nodes,
const { webhookSuffix } = (node.parameters.options ?? {}) as IDataObject; runData: workflowsStore.getWorkflowExecution?.data?.resultData?.runData,
const suffix = destinationNode: options.destinationNode,
webhookSuffix && typeof webhookSuffix !== 'object' ? `/${webhookSuffix}` : ''; pinData,
testUrl = `${rootStore.formWaitingUrl}/${runWorkflowApiResponse.executionId}${suffix}`; directParentNodes,
} formWaitingUrl: rootStore.formWaitingUrl,
executionId: runWorkflowApiResponse.executionId,
if (testUrl && options.source !== 'RunData.ManualChatMessage') openPopUpWindow(testUrl); source: options.source,
} getTestUrl,
} shouldShowForm,
});
await useExternalHooks().run('workflowRun.runWorkflow', { await useExternalHooks().run('workflowRun.runWorkflow', {
nodeName: options.destinationNode, nodeName: options.destinationNode,

View file

@ -0,0 +1,128 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { displayForm, openPopUpWindow } from '../executionUtils';
import type { INode, IRunData, IPinData } from 'n8n-workflow';
const FORM_TRIGGER_NODE_TYPE = 'formTrigger';
const WAIT_NODE_TYPE = 'waitNode';
vi.mock('../executionUtils', async () => {
const actual = await vi.importActual('../executionUtils');
return {
...actual,
openPopUpWindow: vi.fn(),
};
});
describe('displayForm', () => {
const getTestUrlMock = vi.fn();
const shouldShowFormMock = vi.fn();
beforeEach(() => {
vi.clearAllMocks();
});
it('should not call openPopUpWindow if node has already run or is pinned', () => {
const nodes: INode[] = [
{
id: '1',
name: 'Node1',
typeVersion: 1,
type: FORM_TRIGGER_NODE_TYPE,
position: [0, 0],
parameters: {},
},
{
id: '2',
name: 'Node2',
typeVersion: 1,
type: WAIT_NODE_TYPE,
position: [0, 0],
parameters: {},
},
];
const runData: IRunData = { Node1: [] };
const pinData: IPinData = { Node2: [{ json: { data: {} } }] };
displayForm({
nodes,
runData,
pinData,
destinationNode: undefined,
directParentNodes: [],
formWaitingUrl: 'http://example.com',
executionId: undefined,
source: undefined,
getTestUrl: getTestUrlMock,
shouldShowForm: shouldShowFormMock,
});
expect(openPopUpWindow).not.toHaveBeenCalled();
});
it('should skip nodes if destinationNode does not match and node is not a directParentNode', () => {
const nodes: INode[] = [
{
id: '1',
name: 'Node1',
typeVersion: 1,
type: FORM_TRIGGER_NODE_TYPE,
position: [0, 0],
parameters: {},
},
{
id: '2',
name: 'Node2',
typeVersion: 1,
type: WAIT_NODE_TYPE,
position: [0, 0],
parameters: {},
},
];
displayForm({
nodes,
runData: undefined,
pinData: {},
destinationNode: 'Node3',
directParentNodes: ['Node4'],
formWaitingUrl: 'http://example.com',
executionId: '12345',
source: undefined,
getTestUrl: getTestUrlMock,
shouldShowForm: shouldShowFormMock,
});
expect(openPopUpWindow).not.toHaveBeenCalled();
});
it('should not open pop-up if source is "RunData.ManualChatMessage"', () => {
const nodes: INode[] = [
{
id: '1',
name: 'Node1',
typeVersion: 1,
type: FORM_TRIGGER_NODE_TYPE,
position: [0, 0],
parameters: {},
},
];
getTestUrlMock.mockReturnValue('http://test-url.com');
displayForm({
nodes,
runData: undefined,
pinData: {},
destinationNode: undefined,
directParentNodes: [],
formWaitingUrl: 'http://example.com',
executionId: undefined,
source: 'RunData.ManualChatMessage',
getTestUrl: getTestUrlMock,
shouldShowForm: shouldShowFormMock,
});
expect(openPopUpWindow).not.toHaveBeenCalled();
});
});

View file

@ -1,6 +1,7 @@
import type { ExecutionStatus, IDataObject } from 'n8n-workflow'; import type { ExecutionStatus, IDataObject, INode, IPinData, IRunData } from 'n8n-workflow';
import type { ExecutionFilterType, ExecutionsQueryFilter } from '@/Interface'; import type { ExecutionFilterType, ExecutionsQueryFilter } from '@/Interface';
import { isEmpty } from '@/utils/typesUtils'; import { isEmpty } from '@/utils/typesUtils';
import { FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE } from '../constants';
export function getDefaultExecutionFilters(): ExecutionFilterType { export function getDefaultExecutionFilters(): ExecutionFilterType {
return { return {
@ -86,3 +87,64 @@ export const openPopUpWindow = (
window.open(url, '_blank', features); window.open(url, '_blank', features);
} }
}; };
export function displayForm({
nodes,
runData,
pinData,
destinationNode,
directParentNodes,
formWaitingUrl,
executionId,
source,
getTestUrl,
shouldShowForm,
}: {
nodes: INode[];
runData: IRunData | undefined;
pinData: IPinData;
destinationNode: string | undefined;
directParentNodes: string[];
formWaitingUrl: string;
executionId: string | undefined;
source: string | undefined;
getTestUrl: (node: INode) => string;
shouldShowForm: (node: INode) => boolean;
}) {
for (const node of nodes) {
const hasNodeRun = runData && runData?.hasOwnProperty(node.name);
if (hasNodeRun || pinData[node.name]) continue;
if (![FORM_TRIGGER_NODE_TYPE, WAIT_NODE_TYPE].includes(node.type)) {
continue;
}
if (
destinationNode &&
destinationNode !== node.name &&
!directParentNodes.includes(node.name)
) {
continue;
}
if (node.name === destinationNode || !node.disabled) {
let testUrl = '';
if (node.type === FORM_TRIGGER_NODE_TYPE) {
testUrl = getTestUrl(node);
}
if (node.type === WAIT_NODE_TYPE && node.parameters.resume === 'form' && executionId) {
if (!shouldShowForm(node)) continue;
const { webhookSuffix } = (node.parameters.options ?? {}) as IDataObject;
const suffix =
webhookSuffix && typeof webhookSuffix !== 'object' ? `/${webhookSuffix}` : '';
testUrl = `${formWaitingUrl}/${executionId}${suffix}`;
}
if (testUrl && source !== 'RunData.ManualChatMessage') openPopUpWindow(testUrl);
}
}
}