diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index dd923fada2..605893e698 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -69,6 +69,7 @@ import '@/evaluation.ee/test-definitions.controller.ee'; import '@/evaluation.ee/test-runs.controller.ee'; import '@/workflows/workflow-history.ee/workflow-history.controller.ee'; import '@/workflows/workflows.controller'; +import '@/webhooks/webhooks.controller'; @Service() export class Server extends AbstractServer { diff --git a/packages/cli/src/webhooks/webhook.service.ts b/packages/cli/src/webhooks/webhook.service.ts index feacb6412c..dfc79de68a 100644 --- a/packages/cli/src/webhooks/webhook.service.ts +++ b/packages/cli/src/webhooks/webhook.service.ts @@ -19,7 +19,7 @@ import { WebhookRepository } from '@/databases/repositories/webhook.repository'; import { NodeTypes } from '@/node-types'; import { CacheService } from '@/services/cache/cache.service'; -type Method = NonNullable; +import type { Method } from './webhook.types'; @Service() export class WebhookService { diff --git a/packages/cli/src/webhooks/webhook.types.ts b/packages/cli/src/webhooks/webhook.types.ts index 4d34b00d04..9e0b5c00a0 100644 --- a/packages/cli/src/webhooks/webhook.types.ts +++ b/packages/cli/src/webhooks/webhook.types.ts @@ -35,3 +35,5 @@ export interface IWebhookResponseCallbackData { noWebhookResponse?: boolean; responseCode?: number; } + +export type Method = NonNullable; diff --git a/packages/cli/src/webhooks/webhooks.controller.ts b/packages/cli/src/webhooks/webhooks.controller.ts new file mode 100644 index 0000000000..497aab88a0 --- /dev/null +++ b/packages/cli/src/webhooks/webhooks.controller.ts @@ -0,0 +1,23 @@ +import { Post, RestController } from '@n8n/decorators'; +import { Request } from 'express'; +import get from 'lodash/get'; + +import { WebhookService } from './webhook.service'; +import type { Method } from './webhook.types'; + +@RestController('/webhooks') +export class WebhooksController { + constructor(private readonly webhookService: WebhookService) {} + + @Post('/find') + async findWebhook(req: Request) { + const body = get(req, 'body', {}) as { path: string; method: Method }; + + try { + const webhook = await this.webhookService.findWebhook(body.method, body.path); + return webhook; + } catch (error) { + return null; + } + } +} diff --git a/packages/frontend/editor-ui/src/api/webhooks.ts b/packages/frontend/editor-ui/src/api/webhooks.ts new file mode 100644 index 0000000000..074fbae674 --- /dev/null +++ b/packages/frontend/editor-ui/src/api/webhooks.ts @@ -0,0 +1,17 @@ +import { makeRestApiRequest } from '@/utils/apiUtils'; +import type { IRestApiContext } from '@/Interface'; +import type { IHttpRequestMethods } from 'n8n-workflow'; + +type WebhookData = { + workflowId: string; + webhookPath: string; + method: IHttpRequestMethods; + node: string; +}; + +export const findWebhook = async ( + context: IRestApiContext, + data: { path: string; method: string }, +): Promise => { + return await makeRestApiRequest(context, 'POST', '/webhooks/find', data); +}; diff --git a/packages/frontend/editor-ui/src/components/Modals.vue b/packages/frontend/editor-ui/src/components/Modals.vue index 7d5e82c9a8..eaef0fb35c 100644 --- a/packages/frontend/editor-ui/src/components/Modals.vue +++ b/packages/frontend/editor-ui/src/components/Modals.vue @@ -35,6 +35,7 @@ import { COMMUNITY_PLUS_ENROLLMENT_MODAL, DELETE_FOLDER_MODAL_KEY, MOVE_FOLDER_MODAL_KEY, + WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY, } from '@/constants'; import AboutModal from '@/components/AboutModal.vue'; @@ -71,6 +72,7 @@ import ProjectMoveResourceModal from '@/components/Projects/ProjectMoveResourceM import NewAssistantSessionModal from '@/components/AskAssistant/NewAssistantSessionModal.vue'; import PromptMfaCodeModal from './PromptMfaCodeModal/PromptMfaCodeModal.vue'; import CommunityPlusEnrollmentModal from '@/components/CommunityPlusEnrollmentModal.vue'; +import WorkflowActivationConflictingWebhookModal from '@/components/WorkflowActivationConflictingWebhookModal.vue'; import type { EventBus } from '@n8n/utils/event-bus'; @@ -294,5 +296,11 @@ import type { EventBus } from '@n8n/utils/event-bus'; + + + + diff --git a/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.test.ts b/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.test.ts new file mode 100644 index 0000000000..d4bbfb1063 --- /dev/null +++ b/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.test.ts @@ -0,0 +1,65 @@ +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 { waitFor } from '@testing-library/vue'; + +vi.mock('@/stores/ui.store', () => { + return { + useUIStore: vi.fn(() => ({ + closeModal: vi.fn(), + })), + }; +}); +vi.mock('@/stores/root.store', () => { + return { + useRootStore: vi.fn(() => ({ + webhookUrl: 'http://webhook-base', + urlBaseEditor: 'http://editor-base', + })), + }; +}); + +const renderComponent = createComponentRenderer(WorkflowActivationConflictingWebhookModal, { + global: { + stubs: { + Modal: { + template: + '
', + }, + }, + }, +}); + +describe('WorkflowActivationConflictingWebhookModal', () => { + beforeEach(() => { + createTestingPinia(); + }); + + it('should render modal', async () => { + const props = { + modalName: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY, + data: { + triggerName: 'Trigger in this workflow', + workflowName: 'Test Workflow', + workflowId: '123', + webhookPath: 'webhook-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 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-path')).toHaveTextContent( + 'http://webhook-base/webhook-path', + ); + }); +}); diff --git a/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.vue b/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.vue new file mode 100644 index 0000000000..2af9d749e1 --- /dev/null +++ b/packages/frontend/editor-ui/src/components/WorkflowActivationConflictingWebhookModal.vue @@ -0,0 +1,84 @@ + + + + + diff --git a/packages/frontend/editor-ui/src/components/WorkflowActivator.vue b/packages/frontend/editor-ui/src/components/WorkflowActivator.vue index 4dcaa7cb92..14a6f6f232 100644 --- a/packages/frontend/editor-ui/src/components/WorkflowActivator.vue +++ b/packages/frontend/editor-ui/src/components/WorkflowActivator.vue @@ -7,11 +7,19 @@ import type { VNode } from 'vue'; import { computed, h, watch } from 'vue'; import { useI18n } from '@/composables/useI18n'; import type { PermissionsRecord } from '@/permissions'; -import { EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, PLACEHOLDER_EMPTY_WORKFLOW_ID } from '@/constants'; +import { + WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY, + EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE, + PLACEHOLDER_EMPTY_WORKFLOW_ID, +} from '@/constants'; import WorkflowActivationErrorMessage from './WorkflowActivationErrorMessage.vue'; import { useCredentialsStore } from '@/stores/credentials.store'; import type { INodeUi, IUsedCredential } from '@/Interface'; import { OPEN_AI_API_CREDENTIAL_TYPE } from 'n8n-workflow'; +import { useUIStore } from '@/stores/ui.store'; + +import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers'; +import { useRouter } from 'vue-router'; const props = defineProps<{ workflowActive: boolean; @@ -26,6 +34,11 @@ const emit = defineEmits<{ const { showMessage } = useToast(); const workflowActivate = useWorkflowActivate(); +const uiStore = useUIStore(); + +const router = useRouter(); +const workflowHelpers = useWorkflowHelpers({ router }); + const i18n = useI18n(); const workflowsStore = useWorkflowsStore(); const credentialsStore = useCredentialsStore(); @@ -47,9 +60,12 @@ const isCurrentWorkflow = computed((): boolean => { return workflowsStore.workflowId === props.workflowId; }); +const foundTriggers = computed(() => + getActivatableTriggerNodes(workflowsStore.workflowTriggerNodes), +); + const containsTrigger = computed((): boolean => { - const foundTriggers = getActivatableTriggerNodes(workflowsStore.workflowTriggerNodes); - return foundTriggers.length > 0; + return foundTriggers.value.length > 0; }); const containsOnlyExecuteWorkflowTrigger = computed((): boolean => { @@ -114,10 +130,31 @@ const shouldShowFreeAiCreditsWarning = computed((): boolean => { }); async function activeChanged(newActiveState: boolean) { + if (!isWorkflowActive.value) { + const conflictData = await workflowHelpers.checkConflictingWebhooks(props.workflowId); + + if (conflictData) { + const { trigger, conflict } = conflictData; + const conflictingWorkflow = await workflowsStore.fetchWorkflow(conflict.workflowId); + + uiStore.openModalWithData({ + name: WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY, + data: { + triggerName: trigger.name, + workflowName: conflictingWorkflow.name, + ...conflict, + }, + }); + + return; + } + } + const newState = await workflowActivate.updateWorkflowActivation( props.workflowId, newActiveState, ); + emit('update:workflowActive', { id: props.workflowId, active: newState }); } diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts index ac7d5addab..389ded7e56 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts @@ -1,4 +1,4 @@ -import type { IWorkflowDataUpdate } from '@/Interface'; +import type { IWorkflowData, IWorkflowDataUpdate, IWorkflowDb } from '@/Interface'; import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers'; import router from '@/router'; import { createTestingPinia } from '@pinia/testing'; @@ -6,8 +6,10 @@ import { setActivePinia } from 'pinia'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { useWorkflowsEEStore } from '@/stores/workflows.ee.store'; import { useTagsStore } from '@/stores/tags.store'; +import { useUIStore } from '@/stores/ui.store'; import { createTestWorkflow } from '@/__tests__/mocks'; -import type { AssignmentCollectionValue } from 'n8n-workflow'; +import { WEBHOOK_NODE_TYPE, type AssignmentCollectionValue } from 'n8n-workflow'; +import * as apiWebhooks from '../api/webhooks'; const getDuplicateTestWorkflow = (): IWorkflowDataUpdate => ({ name: 'Duplicate webhook test', @@ -59,12 +61,14 @@ describe('useWorkflowHelpers', () => { let workflowsStore: ReturnType; let workflowsEEStore: ReturnType; let tagsStore: ReturnType; + let uiStore: ReturnType; beforeAll(() => { setActivePinia(createTestingPinia()); workflowsStore = useWorkflowsStore(); workflowsEEStore = useWorkflowsEEStore(); tagsStore = useTagsStore(); + uiStore = useUIStore(); }); afterEach(() => { @@ -370,4 +374,84 @@ describe('useWorkflowHelpers', () => { expect(upsertTagsSpy).toHaveBeenCalledWith([]); }); }); + + describe('checkConflictingWebhooks', () => { + it('should return null if no conflicts', async () => { + const workflowHelpers = useWorkflowHelpers({ router }); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [], + } as unknown as IWorkflowDb); + expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null); + }); + + it('should return conflicting webhook data and workflow id is different', async () => { + const workflowHelpers = useWorkflowHelpers({ router }); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + parameters: { + method: 'GET', + path: 'test-path', + }, + }, + ], + } as unknown as IWorkflowDb); + vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({ + method: 'GET', + webhookPath: 'test-path', + node: 'Webhook 1', + workflowId: '456', + }); + expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({ + conflict: { + method: 'GET', + node: 'Webhook 1', + webhookPath: 'test-path', + workflowId: '456', + }, + trigger: { + parameters: { + method: 'GET', + path: 'test-path', + }, + type: 'n8n-nodes-base.webhook', + }, + }); + }); + + it('should return null if webhook already exist but workflow id is the same', async () => { + const workflowHelpers = useWorkflowHelpers({ router }); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + parameters: { + method: 'GET', + path: 'test-path', + }, + }, + ], + } as unknown as IWorkflowDb); + vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({ + method: 'GET', + webhookPath: 'test-path', + node: 'Webhook 1', + workflowId: '123', + }); + expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null); + }); + + it('should call getWorkflowDataToSave if state is dirty', async () => { + const workflowHelpers = useWorkflowHelpers({ router }); + uiStore.stateIsDirty = true; + vi.spyOn(workflowHelpers, 'getWorkflowDataToSave').mockResolvedValue({ + nodes: [], + } as unknown as IWorkflowData); + expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts index 6976298984..6e592d4c07 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts @@ -26,7 +26,12 @@ import type { NodeParameterValue, Workflow, } from 'n8n-workflow'; -import { NodeConnectionTypes, ExpressionEvaluatorProxy, NodeHelpers } from 'n8n-workflow'; +import { + NodeConnectionTypes, + ExpressionEvaluatorProxy, + NodeHelpers, + WEBHOOK_NODE_TYPE, +} from 'n8n-workflow'; import type { ICredentialsResponse, @@ -71,6 +76,7 @@ import { useProjectsStore } from '@/stores/projects.store'; import { useTagsStore } from '@/stores/tags.store'; import { useWorkflowsEEStore } from '@/stores/workflows.ee.store'; import { useNpsSurveyStore } from '@/stores/npsSurvey.store'; +import { findWebhook } from '../api/webhooks'; export type ResolveParameterOptions = { targetItem?: TargetItem; @@ -851,6 +857,22 @@ export function useWorkflowHelpers(options: { router: ReturnType node.type.startsWith(packageName)); }; + async function checkConflictingWebhooks(workflowId: string) { + let data; + if (uiStore.stateIsDirty) { + data = await getWorkflowDataToSave(); + } else { + data = await workflowsStore.fetchWorkflow(workflowId); + } + + const webhookTriggers = data.nodes.filter( + (node) => node.disabled !== true && node.type === WEBHOOK_NODE_TYPE, + ); + + for (const trigger of webhookTriggers) { + const method = (trigger.parameters.method as string) ?? 'GET'; + + const path = trigger.parameters.path as string; + + const conflict = await findWebhook(rootStore.restApiContext, { + path, + method, + }); + + if (conflict && conflict.workflowId !== workflowId) { + return { trigger, conflict }; + } + } + + return null; + } + return { setDocumentTitle, resolveParameter, @@ -1248,5 +1314,6 @@ export function useWorkflowHelpers(options: { router: ReturnType { customHeading: undefined, }, }, + [WORKFLOW_ACTIVATION_CONFLICTING_WEBHOOK_MODAL_KEY]: { + open: false, + data: { + workflowName: '', + workflowId: '', + webhookPath: '', + node: '', + }, + }, }); const modalStack = ref([]);