Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Return correct trigger count for nodes with multiple webhooks #14300

15 changes: 14 additions & 1 deletion packages/cli/src/active-workflow-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,10 +673,23 @@ export class ActiveWorkflowManager {
const triggerFilter = (nodeType: INodeType) =>
!!nodeType.trigger && !nodeType.description.name.includes('manualTrigger');

// Retrieve unique webhooks as some nodes have multiple webhooks
const workflowWebhooks = WebhookHelpers.getWorkflowWebhooks(
workflow,
additionalData,
undefined,
true,
);

const uniqueWebhooks = workflowWebhooks.reduce<Set<string>>((acc, webhook) => {
acc.add(webhook.node);
return acc;
}, new Set());

return (
workflow.queryNodes(triggerFilter).length +
workflow.getPollNodes().length +
WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true).length
uniqueWebhooks.size
);
}

Expand Down
60 changes: 58 additions & 2 deletions packages/cli/test/integration/active-workflow-manager.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import { Logger } from 'n8n-core';
import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
import { NodeApiError, Workflow } from 'n8n-workflow';
import type { IWebhookData, IWorkflowBase, WorkflowActivateMode } from 'n8n-workflow';
import type {
IWebhookData,
IWorkflowBase,
WorkflowActivateMode,
INodeTypeData,
} from 'n8n-workflow';

import { ActiveExecutions } from '@/active-executions';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import type { WebhookEntity } from '@/databases/entities/webhook-entity';
import { WorkflowRepository } from '@/databases/repositories/workflow.repository';
import { ExecutionService } from '@/executions/execution.service';
import { ExternalHooks } from '@/external-hooks';
import { NodeTypes } from '@/node-types';
Expand Down Expand Up @@ -43,7 +51,18 @@ beforeAll(async () => {

activeWorkflowManager = Container.get(ActiveWorkflowManager);

await utils.initNodeTypes();
const nodes: INodeTypeData = {
'n8n-nodes-base.scheduleTrigger': {
type: new ScheduleTrigger(),
sourcePath: '',
},
'n8n-nodes-base.formTrigger': {
type: new FormTrigger(),
sourcePath: '',
},
};

await utils.initNodeTypes(nodes);

const owner = await createOwner();
createActiveWorkflow = async () => await createWorkflow({ active: true }, owner);
Expand Down Expand Up @@ -136,6 +155,43 @@ describe('add()', () => {
},
);
});

test('should count workflow triggers correctly when node has multiple webhooks', async () => {
const workflowRepositoryInstance = Container.get(WorkflowRepository);
const updateWorkflowTriggerCountSpy = jest.spyOn(
workflowRepositoryInstance,
'updateWorkflowTriggerCount',
);
await activeWorkflowManager.init();

// Mock all of the webhooks
const mockWebhooks: IWebhookData[] = [
mock<IWebhookData>({ node: 'Form Trigger', httpMethod: 'GET', path: '/webhook-path' }),
mock<IWebhookData>({ node: 'Form Trigger', httpMethod: 'POST', path: '/webhook-path' }),
];
webhookService.getNodeWebhooks.mockReturnValue(mockWebhooks);
webhookService.createWebhook.mockReturnValue(
mock<WebhookEntity>({ webhookPath: '/webhook-path' }),
);

// Create a workflow which has a form trigger
const dbWorkflow = await createWorkflow({
nodes: [
{
id: 'uuid-1',
parameters: { path: 'test-webhook-path', options: {} },
name: 'Form Trigger',
type: 'n8n-nodes-base.formTrigger',
typeVersion: 1,
position: [500, 300],
},
],
});

await activeWorkflowManager.add(dbWorkflow.id, 'activate');

expect(updateWorkflowTriggerCountSpy).toHaveBeenCalledWith(dbWorkflow.id, 1);
});
});

describe('removeAll()', () => {
Expand Down
13 changes: 10 additions & 3 deletions packages/cli/test/integration/shared/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { Ftp } from 'n8n-nodes-base/credentials/Ftp.credentials';
import { GithubApi } from 'n8n-nodes-base/credentials/GithubApi.credentials';
import { Cron } from 'n8n-nodes-base/nodes/Cron/Cron.node';
import { FormTrigger } from 'n8n-nodes-base/nodes/Form/FormTrigger.node';
import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node';
import { Set } from 'n8n-nodes-base/nodes/Set/Set.node';
import { Start } from 'n8n-nodes-base/nodes/Start/Start.node';
Expand Down Expand Up @@ -67,9 +68,8 @@ export async function initCredentialsTypes(): Promise<void> {
/**
* Initialize node types.
*/
export async function initNodeTypes() {
ScheduleTrigger.prototype.trigger = async () => ({});
const nodes: INodeTypeData = {
export async function initNodeTypes(customNodes?: INodeTypeData) {
const defaultNodes: INodeTypeData = {
'n8n-nodes-base.start': {
type: new Start(),
sourcePath: '',
Expand All @@ -86,7 +86,14 @@ export async function initNodeTypes() {
type: new ScheduleTrigger(),
sourcePath: '',
},
'n8n-nodes-base.formTrigger': {
type: new FormTrigger(),
sourcePath: '',
},
};

ScheduleTrigger.prototype.trigger = async () => ({});
const nodes = customNodes ?? defaultNodes;
const loader = mock<DirectoryLoader>();
loader.getNode.mockImplementation((nodeType) => {
const node = nodes[`n8n-nodes-base.${nodeType}`];
Expand Down
8 changes: 0 additions & 8 deletions packages/nodes-base/test/nodes/TriggerHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,6 @@ function getNodeVersion(Trigger: new () => VersionedNodeType, version?: number)
return instance.nodeVersions[version ?? instance.currentVersion];
}

export async function testVersionedTriggerNode(
Trigger: new () => VersionedNodeType,
version?: number,
options: TestTriggerNodeOptions = {},
) {
return await testTriggerNode(getNodeVersion(Trigger, version), options);
}

Comment on lines -49 to -56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find 👍🏾

export async function testTriggerNode(
Trigger: (new () => INodeType) | INodeType,
options: TestTriggerNodeOptions = {},
Expand Down
Loading