fix: Extend deduplication check to all webhook-based triggers and chat trigger (#18044)

This commit is contained in:
Michael Kret
2025-08-07 13:46:17 +03:00
committed by GitHub
parent 99c2f3715e
commit 847a5d822f
4 changed files with 185 additions and 16 deletions

View File

@@ -1,10 +1,13 @@
import { createTestingPinia } from '@pinia/testing';
import { createComponentRenderer } from '@/__tests__/render';
import WorkflowActivationConflictingWebhookModal from '@/components/WorkflowActivationConflictingWebhookModal.vue';
import { WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY } from '@/constants';
import {
SLACK_TRIGGER_NODE_TYPE,
WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
} from '@/constants';
import { waitFor } from '@testing-library/vue';
import { FORM_TRIGGER_NODE_TYPE, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
import { CHAT_TRIGGER_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
vi.mock('@/stores/ui.store', () => {
return {
@@ -60,6 +63,9 @@ describe('WorkflowActivationConflictingWebhookModal', () => {
expect(wrapper.getByTestId('conflicting-webhook-callout')).toHaveTextContent(
"A webhook trigger 'Node in workflow' in the workflow 'Test Workflow' uses a conflicting URL path, so this workflow cannot be activated",
);
expect(wrapper.getByTestId('conflicting-webhook-suggestion')).toHaveTextContent(
'and activate this one, or adjust the following URL path in either workflow:',
);
expect(wrapper.getByTestId('conflicting-webhook-path')).toHaveTextContent(
'http://webhook-base/webhook-path',
);
@@ -87,8 +93,71 @@ describe('WorkflowActivationConflictingWebhookModal', () => {
expect(wrapper.getByTestId('conflicting-webhook-callout')).toHaveTextContent(
"A form trigger 'Node in workflow' in the workflow 'Test Form' uses a conflicting URL path, so this workflow cannot be activated",
);
expect(wrapper.getByTestId('conflicting-webhook-suggestion')).toHaveTextContent(
'and activate this one, or adjust the following URL path in either workflow:',
);
expect(wrapper.getByTestId('conflicting-webhook-path')).toHaveTextContent(
'http://webhook-base/form-path',
);
});
it('should render chat conflict modal', async () => {
const props = {
modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
data: {
triggerName: 'Chat in this workflow',
workflowName: 'Test Chat',
triggerType: CHAT_TRIGGER_NODE_TYPE,
workflowId: '123',
webhookPath: '123/chat',
method: 'POST',
node: 'Chat trigger',
},
};
const wrapper = renderComponent({ props });
await waitFor(() => {
expect(wrapper.queryByTestId('conflicting-webhook-callout')).toBeInTheDocument();
});
expect(wrapper.getByTestId('conflicting-webhook-callout')).toHaveTextContent(
"A chat trigger 'Chat trigger' in the workflow 'Test Chat' uses a conflicting URL path, so this workflow cannot be activated",
);
expect(wrapper.getByTestId('conflicting-webhook-suggestion')).toHaveTextContent(
'and activate this one, or insert a new Chat Trigger node in either workflow:',
);
expect(wrapper.getByTestId('conflicting-webhook-path')).toHaveTextContent(
'http://webhook-base/123/chat',
);
});
it('should render trigger conflict modal', async () => {
const props = {
modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
data: {
triggerName: 'Slack in this workflow',
workflowName: 'Test Trigger',
triggerType: SLACK_TRIGGER_NODE_TYPE,
workflowId: '123',
webhookPath: '123/webhook',
method: 'POST',
node: 'Slack trigger',
},
};
const wrapper = renderComponent({ props });
await waitFor(() => {
expect(wrapper.queryByTestId('conflicting-webhook-callout')).toBeInTheDocument();
});
expect(wrapper.getByTestId('conflicting-webhook-callout')).toHaveTextContent(
"A trigger 'Slack trigger' in the workflow 'Test Trigger' uses a conflicting URL path, so this workflow cannot be activated",
);
expect(wrapper.getByTestId('conflicting-webhook-suggestion')).toHaveTextContent(
'and activate this one, or insert a new trigger node of the same type in either workflow:',
);
expect(wrapper.getByTestId('conflicting-webhook-path')).toHaveTextContent(
'http://webhook-base/123/webhook',
);
});
});

View File

@@ -6,7 +6,7 @@ import { useUIStore } from '@/stores/ui.store';
import { useRootStore } from '@n8n/stores/useRootStore';
import { computed } from 'vue';
import { FORM_TRIGGER_NODE_TYPE } from 'n8n-workflow';
import { CHAT_TRIGGER_NODE_TYPE, FORM_TRIGGER_NODE_TYPE, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
const modalBus = createEventBus();
const uiStore = useUIStore();
@@ -28,9 +28,33 @@ const webhookUrl = computed(() => {
return rootStore.webhookUrl;
});
const webhookType = computed(() => {
if (data.triggerType === FORM_TRIGGER_NODE_TYPE) return 'form';
return 'webhook';
const webhookTypeUi = computed((): { title: string; callout: string; suggestion: string } => {
const suggestionBase = 'and activate this one, or ';
if (data.triggerType === FORM_TRIGGER_NODE_TYPE)
return {
title: 'Form',
callout: 'form trigger',
suggestion: suggestionBase + 'adjust the following URL path in either workflow:',
};
if (data.triggerType === CHAT_TRIGGER_NODE_TYPE)
return {
title: 'Chat',
callout: 'chat trigger',
suggestion: suggestionBase + 'insert a new Chat Trigger node in either workflow:',
};
if (data.triggerType === WEBHOOK_NODE_TYPE)
return {
title: 'Webhook',
callout: 'webhook trigger',
suggestion: suggestionBase + 'adjust the following URL path in either workflow:',
};
return {
title: 'Trigger',
callout: 'trigger',
suggestion: suggestionBase + 'insert a new trigger node of the same type in either workflow:',
};
});
const workflowUrl = computed(() => {
@@ -46,21 +70,21 @@ const onClick = async () => {
<Modal
width="540px"
:name="WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY"
:title="`Conflicting ${webhookType === 'form' ? 'Form' : 'Webhook'} Path`"
:title="`Conflicting ${webhookTypeUi.title} Path`"
:event-bus="modalBus"
:center="true"
>
<template #content>
<n8n-callout theme="danger" data-test-id="conflicting-webhook-callout">
A {{ webhookType }} trigger '{{ data.node }}' in the workflow '{{ data.workflowName }}' uses
a conflicting URL path, so this workflow cannot be activated
A {{ webhookTypeUi.callout }} '{{ data.node }}' in the workflow '{{ data.workflowName }}'
uses a conflicting URL path, so this workflow cannot be activated
</n8n-callout>
<div :class="$style.container">
<div>
<n8n-text color="text-base"> You can deactivate </n8n-text>
<n8n-link :to="workflowUrl" :underline="true"> '{{ data.workflowName }}' </n8n-link>
<n8n-text color="text-base">
and activate this one, or adjust the following URL path in either workflow:
<n8n-link :to="workflowUrl" :underline="true"> {{ data.workflowName }} </n8n-link>
<n8n-text color="text-base" data-test-id="conflicting-webhook-suggestion">
{{ webhookTypeUi.suggestion }}
</n8n-text>
</div>
</div>

View File

@@ -14,10 +14,11 @@ import {
createTestWorkflowExecutionResponse,
createTestWorkflowObject,
} from '@/__tests__/mocks';
import { NodeConnectionTypes, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
import { CHAT_TRIGGER_NODE_TYPE, NodeConnectionTypes, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
import type { AssignmentCollectionValue, IConnections } from 'n8n-workflow';
import * as apiWebhooks from '@n8n/rest-api-client/api/webhooks';
import { mockedStore } from '@/__tests__/utils';
import { SLACK_TRIGGER_NODE_TYPE } from '../constants';
describe('useWorkflowHelpers', () => {
let workflowsStore: ReturnType<typeof mockedStore<typeof useWorkflowsStore>>;
@@ -308,6 +309,7 @@ describe('useWorkflowHelpers', () => {
nodes: [
{
type: WEBHOOK_NODE_TYPE,
webhookId: '1',
parameters: {
method: 'GET',
path: 'test-path',
@@ -334,6 +336,73 @@ describe('useWorkflowHelpers', () => {
path: 'test-path',
},
type: 'n8n-nodes-base.webhook',
webhookId: '1',
},
});
});
it('should return conflicting webhook data and workflow id is different in trigger', async () => {
const workflowHelpers = useWorkflowHelpers();
uiStore.stateIsDirty = false;
vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
nodes: [
{
type: SLACK_TRIGGER_NODE_TYPE,
webhookId: '1',
parameters: {},
},
],
} as unknown as IWorkflowDb);
vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
method: 'POST',
webhookPath: '1/webhook',
node: 'Webhook 1',
workflowId: '456',
});
expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({
conflict: {
method: 'POST',
node: 'Webhook 1',
webhookPath: '1/webhook',
workflowId: '456',
},
trigger: {
parameters: {},
type: SLACK_TRIGGER_NODE_TYPE,
webhookId: '1',
},
});
});
it('should return conflicting webhook data and workflow id is different in chat', async () => {
const workflowHelpers = useWorkflowHelpers();
uiStore.stateIsDirty = false;
vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
nodes: [
{
type: CHAT_TRIGGER_NODE_TYPE,
webhookId: '1',
parameters: {},
},
],
} as unknown as IWorkflowDb);
vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
method: 'POST',
webhookPath: '1/chat',
node: 'Webhook 1',
workflowId: '456',
});
expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({
conflict: {
method: 'POST',
node: 'Webhook 1',
webhookPath: '1/chat',
workflowId: '456',
},
trigger: {
parameters: {},
type: CHAT_TRIGGER_NODE_TYPE,
webhookId: '1',
},
});
});
@@ -345,6 +414,7 @@ describe('useWorkflowHelpers', () => {
nodes: [
{
type: WEBHOOK_NODE_TYPE,
webhookId: '1',
parameters: {
method: 'GET',
path: 'test-path',

View File

@@ -24,6 +24,7 @@ import type {
Workflow,
} from 'n8n-workflow';
import {
CHAT_TRIGGER_NODE_TYPE,
FORM_TRIGGER_NODE_TYPE,
NodeConnectionTypes,
NodeHelpers,
@@ -1020,7 +1021,7 @@ export function useWorkflowHelpers() {
if (trigger.type === WEBHOOK_NODE_TYPE) {
return (trigger.parameters.method as string) ?? 'GET';
}
return 'GET';
return 'POST';
}
function getWebhookPath(trigger: INode) {
@@ -1030,7 +1031,10 @@ export function useWorkflowHelpers() {
if (trigger.type === FORM_TRIGGER_NODE_TYPE) {
return ((trigger.parameters.options as { path: string }) || {}).path ?? trigger.webhookId;
}
return '';
if (trigger.type === CHAT_TRIGGER_NODE_TYPE) {
return `${trigger.webhookId}/chat`;
}
return `${trigger.webhookId}/webhook`;
}
async function checkConflictingWebhooks(workflowId: string) {
@@ -1043,7 +1047,9 @@ export function useWorkflowHelpers() {
const triggers = data.nodes.filter(
(node) =>
node.disabled !== true && [WEBHOOK_NODE_TYPE, FORM_TRIGGER_NODE_TYPE].includes(node.type),
node.disabled !== true &&
node.webhookId &&
(node.type.toLowerCase().includes('trigger') || node.type === WEBHOOK_NODE_TYPE),
);
for (const trigger of triggers) {