feat: Don't allow multiple active workflows with same form path (#16722)

This commit is contained in:
Michael Kret
2025-06-27 13:04:21 +03:00
committed by GitHub
parent 3fb8bbb59f
commit 98b821bbd8
6 changed files with 160 additions and 11 deletions

View File

@@ -4,6 +4,7 @@ import WorkflowActivationConflictingWebhookModal from '@/components/WorkflowActi
import { 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';
vi.mock('@/stores/ui.store', () => {
return {
@@ -37,12 +38,13 @@ describe('WorkflowActivationConflictingWebhookModal', () => {
createTestingPinia();
});
it('should render modal', async () => {
it('should render webhook conflict modal', async () => {
const props = {
modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
data: {
triggerName: 'Trigger in this workflow',
workflowName: 'Test Workflow',
triggerType: WEBHOOK_NODE_TYPE,
workflowId: '123',
webhookPath: 'webhook-path',
method: 'GET',
@@ -62,4 +64,31 @@ describe('WorkflowActivationConflictingWebhookModal', () => {
'http://webhook-base/webhook-path',
);
});
it('should render form conflict modal', async () => {
const props = {
modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
data: {
triggerName: 'Trigger in this workflow',
workflowName: 'Test Form',
triggerType: FORM_TRIGGER_NODE_TYPE,
workflowId: '123',
webhookPath: 'form-path',
method: 'GET',
node: 'Node in workflow',
},
};
const wrapper = renderComponent({ props });
await waitFor(() => {
expect(wrapper.queryByTestId('conflicting-webhook-callout')).toBeInTheDocument();
});
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-path')).toHaveTextContent(
'http://webhook-base/form-path',
);
});
});

View File

@@ -6,6 +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';
const modalBus = createEventBus();
const uiStore = useUIStore();
@@ -14,6 +15,7 @@ const rootStore = useRootStore();
const props = defineProps<{
data: {
workflowName: string;
triggerType: string;
workflowId: string;
webhookPath: string;
node: string;
@@ -26,6 +28,11 @@ const webhookUrl = computed(() => {
return rootStore.webhookUrl;
});
const webhookType = computed(() => {
if (data.triggerType === FORM_TRIGGER_NODE_TYPE) return 'form';
return 'webhook';
});
const workflowUrl = computed(() => {
return rootStore.urlBaseEditor + 'workflow/' + data.workflowId;
});
@@ -39,14 +46,14 @@ const onClick = async () => {
<Modal
width="540px"
:name="WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY"
title="Conflicting Webhook Path"
:title="`Conflicting ${webhookType === 'form' ? 'Form' : 'Webhook'} Path`"
:event-bus="modalBus"
:center="true"
>
<template #content>
<n8n-callout theme="danger" data-test-id="conflicting-webhook-callout">
A webhook trigger '{{ data.node }}' in the workflow '{{ data.workflowName }}' uses a
conflicting URL path, so this workflow cannot be activated
A {{ webhookType }} trigger '{{ 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>

View File

@@ -143,7 +143,7 @@ async function activeChanged(newActiveState: boolean) {
uiStore.openModalWithData({
name: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY,
data: {
triggerName: trigger.name,
triggerType: trigger.type,
workflowName: conflictingWorkflow.name,
...conflict,
},

View File

@@ -362,6 +362,92 @@ describe('useWorkflowHelpers', () => {
} as unknown as WorkflowData);
expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
});
it('should return null if no conflicts with FORM_TRIGGER_NODE_TYPE', async () => {
const workflowHelpers = useWorkflowHelpers();
uiStore.stateIsDirty = false;
vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
nodes: [
{
type: 'n8n-nodes-base.formTrigger',
parameters: {
options: {
path: 'test-path',
},
},
webhookId: '123',
},
],
} as unknown as IWorkflowDb);
vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue(null);
expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null);
});
it('should return conflicting webhook data and workflow id is different with FORM_TRIGGER_NODE_TYPE', async () => {
const workflowHelpers = useWorkflowHelpers();
uiStore.stateIsDirty = false;
vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
nodes: [
{
type: 'n8n-nodes-base.formTrigger',
parameters: {
options: {
path: 'test-path',
},
},
webhookId: '123',
},
],
} as unknown as IWorkflowDb);
vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
method: 'GET',
webhookPath: 'test-path',
node: 'Form Trigger 1',
workflowId: '456',
});
expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({
conflict: {
method: 'GET',
node: 'Form Trigger 1',
webhookPath: 'test-path',
workflowId: '456',
},
trigger: {
parameters: {
options: {
path: 'test-path',
},
},
type: 'n8n-nodes-base.formTrigger',
webhookId: '123',
},
});
});
it('should return null if webhook already exist but workflow id is the same with FORM_TRIGGER_NODE_TYPE', async () => {
const workflowHelpers = useWorkflowHelpers();
uiStore.stateIsDirty = false;
vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({
nodes: [
{
type: 'n8n-nodes-base.formTrigger',
parameters: {
options: {
path: 'test-path',
},
},
webhookId: '123',
},
],
} as unknown as IWorkflowDb);
vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({
method: 'GET',
webhookPath: 'test-path',
node: 'Form Trigger 1',
workflowId: '123',
});
expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null);
});
});
describe('executeData', () => {

View File

@@ -21,7 +21,12 @@ import type {
NodeParameterValue,
Workflow,
} from 'n8n-workflow';
import { NodeConnectionTypes, NodeHelpers, WEBHOOK_NODE_TYPE } from 'n8n-workflow';
import {
FORM_TRIGGER_NODE_TYPE,
NodeConnectionTypes,
NodeHelpers,
WEBHOOK_NODE_TYPE,
} from 'n8n-workflow';
import type {
ICredentialsResponse,
@@ -909,6 +914,26 @@ export function useWorkflowHelpers() {
return workflow.nodes.some((node) => node.type.startsWith(packageName));
};
function getMethod(trigger: INode) {
if (trigger.type === WEBHOOK_NODE_TYPE) {
return (trigger.parameters.method as string) ?? 'GET';
}
return 'GET';
}
function getWebhookPath(trigger: INode) {
if (trigger.type === WEBHOOK_NODE_TYPE) {
return trigger.parameters.path as string;
}
if (trigger.type === FORM_TRIGGER_NODE_TYPE) {
return (
(((trigger.parameters.options as { path: string }) || {}).path as string) ??
trigger.webhookId
);
}
return '';
}
async function checkConflictingWebhooks(workflowId: string) {
let data;
if (uiStore.stateIsDirty) {
@@ -917,14 +942,15 @@ export function useWorkflowHelpers() {
data = await workflowsStore.fetchWorkflow(workflowId);
}
const webhookTriggers = data.nodes.filter(
(node) => node.disabled !== true && node.type === WEBHOOK_NODE_TYPE,
const triggers = data.nodes.filter(
(node) =>
node.disabled !== true && [WEBHOOK_NODE_TYPE, FORM_TRIGGER_NODE_TYPE].includes(node.type),
);
for (const trigger of webhookTriggers) {
const method = (trigger.parameters.method as string) ?? 'GET';
for (const trigger of triggers) {
const method = getMethod(trigger);
const path = trigger.parameters.path as string;
const path = getWebhookPath(trigger);
const conflict = await findWebhook(rootStore.restApiContext, {
path,

View File

@@ -184,6 +184,7 @@ export const useUIStore = defineStore(STORES.UI, () => {
[WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY]: {
open: false,
data: {
triggerType: '',
workflowName: '',
workflowId: '',
webhookPath: '',