Skip to content

Commit

Permalink
fix(core): Run full manual execution when a trigger is executed even …
Browse files Browse the repository at this point in the history
…if run data exists (#13194)
  • Loading branch information
despairblue authored Feb 14, 2025
1 parent 8e37088 commit 66acb1b
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { mock } from 'jest-mock-extended';
import type { INode, IWorkflowBase, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import type { INode, IWorkflowBase, INodeType, IWorkflowExecuteAdditionalData } from 'n8n-workflow';

import type { User } from '@/databases/entities/user';
import type { NodeTypes } from '@/node-types';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import type { WorkflowRunner } from '@/workflow-runner';
import { WorkflowExecutionService } from '@/workflows/workflow-execution.service';
Expand Down Expand Up @@ -52,13 +53,14 @@ const hackerNewsNode: INode = {
};

describe('WorkflowExecutionService', () => {
const nodeTypes = mock<NodeTypes>();
const workflowRunner = mock<WorkflowRunner>();
const workflowExecutionService = new WorkflowExecutionService(
mock(),
mock(),
mock(),
mock(),
mock(),
nodeTypes,
mock(),
workflowRunner,
mock(),
Expand Down Expand Up @@ -86,7 +88,10 @@ describe('WorkflowExecutionService', () => {
const executionId = 'fake-execution-id';
const userId = 'user-id';
const user = mock<User>({ id: userId });
const runPayload = mock<WorkflowRequest.ManualRunPayload>({ startNodes: [] });
const runPayload = mock<WorkflowRequest.ManualRunPayload>({
startNodes: [],
destinationNode: undefined,
});

workflowRunner.run.mockResolvedValue(executionId);

Expand All @@ -108,6 +113,41 @@ describe('WorkflowExecutionService', () => {
expect(result).toEqual({ executionId });
});

test('removes runData if the destination node is a trigger', async () => {
const executionId = 'fake-execution-id';
const userId = 'user-id';
const user = mock<User>({ id: userId });
const node = mock<INode>();
const runPayload = mock<WorkflowRequest.ManualRunPayload>({
workflowData: { nodes: [node] },
startNodes: [],
destinationNode: node.name,
});

jest
.spyOn(nodeTypes, 'getByNameAndVersion')
.mockReturnValueOnce(mock<INodeType>({ description: { group: ['trigger'] } }));

workflowRunner.run.mockResolvedValue(executionId);

const result = await workflowExecutionService.executeManually(runPayload, user);

expect(workflowRunner.run).toHaveBeenCalledWith({
destinationNode: runPayload.destinationNode,
executionMode: 'manual',
runData: undefined,
pinData: runPayload.workflowData.pinData,
pushRef: undefined,
workflowData: runPayload.workflowData,
userId,
partialExecutionVersion: 1,
startNodes: runPayload.startNodes,
dirtyNodeNames: runPayload.dirtyNodeNames,
triggerToStartFrom: runPayload.triggerToStartFrom,
});
expect(result).toEqual({ executionId });
});

[
{
name: 'trigger',
Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/workflows/workflow-execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ export class WorkflowExecutionService {
return await this.workflowRunner.run(runData, true, undefined, undefined, responsePromise);
}

private isDestinationNodeATrigger(destinationNode: string, workflow: IWorkflowBase) {
const node = workflow.nodes.find((n) => n.name === destinationNode);

if (node === undefined) {
return false;
}

const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);

return nodeType.description.group.includes('trigger');
}

async executeManually(
{
workflowData,
Expand All @@ -107,6 +119,23 @@ export class WorkflowExecutionService {
pinData,
);

// TODO: Reverse the order of events, first find out if the execution is
// partial or full, if it's partial create the execution and run, if it's
// full get the data first and only then create the execution.
//
// If the destination node is a trigger, then per definition this
// is not a partial execution and thus we can ignore the run data.
// If we don't do this we'll end up creating an execution, calling the
// partial execution flow, finding out that we don't have run data to
// create the execution stack and have to cancel the execution, come back
// here and either create the runData (e.g. scheduler trigger) or wait for
// a webhook or event.
if (destinationNode) {
if (this.isDestinationNodeATrigger(destinationNode, workflowData)) {
runData = undefined;
}
}

// if we have a trigger to start from and it's not the pinned trigger
// ignore the pinned trigger
if (pinnedTrigger && triggerToStartFrom && pinnedTrigger.name !== triggerToStartFrom.name) {
Expand Down Expand Up @@ -178,7 +207,7 @@ export class WorkflowExecutionService {
},
resultData: {
pinData,
runData,
runData: runData ?? {},
},
manualData: {
userId: data.userId,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/workflows/workflow.request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export declare namespace WorkflowRequest {

type ManualRunPayload = {
workflowData: IWorkflowBase;
runData: IRunData;
runData?: IRunData;
startNodes?: StartNodeData[];
destinationNode?: string;
dirtyNodeNames?: string[];
Expand Down

0 comments on commit 66acb1b

Please # to comment.