diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index cae0921634..66978e4b33 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -346,7 +346,7 @@ describe('NDV', () => { }); }); - it('should flag issues as soon as params are set', () => { + it('webhook shoul fallback to webhookId if path is empty', () => { workflowPage.actions.addInitialNodeToCanvas('Webhook'); workflowPage.getters.canvasNodes().first().dblclick(); @@ -356,13 +356,31 @@ describe('NDV', () => { ndv.getters.parameterInput('path').clear(); - ndv.getters.nodeExecuteButton().should('be.disabled'); - ndv.getters.triggerPanelExecuteButton().should('not.exist'); + const urrWithUUID = + /https?:\/\/[^\/]+\/webhook-test\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i; + + cy.contains('Webhook URLs') + .parent() + .invoke('text') + .then((text) => { + const match = text.match(urrWithUUID); + expect(match, 'Should contain dynamic URL with UUID').to.not.be.null; + }); + ndv.actions.close(); - workflowPage.getters.nodeIssuesByName('Webhook').should('exist'); workflowPage.getters.canvasNodes().first().dblclick(); - ndv.getters.parameterInput('path').type('t'); + ndv.getters.parameterInput('path').type('test-path'); + + const urlWithCustomPath = /https?:\/\/[^\/]+\/webhook-test\/test-path/i; + + cy.contains('Webhook URLs') + .parent() + .invoke('text') + .then((text) => { + const match = text.match(urlWithCustomPath); + expect(match, 'Should contain URL with custom path').to.not.be.null; + }); ndv.getters.nodeExecuteButton().should('not.be.disabled'); ndv.getters.triggerPanelExecuteButton().should('exist'); diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts index 9e2cd4b90f..2a22f4d701 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts @@ -448,6 +448,56 @@ describe('useWorkflowHelpers', () => { }); expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null); }); + + it('should return trigger.parameters.path when both trigger.parameters.path and trigger.webhookId are strings', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + parameters: { + method: 'GET', + path: 'test-path', + }, + webhookId: 'test-webhook-id', + }, + ], + } as unknown as IWorkflowDb); + const findWebhookSpy = vi.spyOn(apiWebhooks, 'findWebhook'); + findWebhookSpy.mockResolvedValue(null); + expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null); + expect(findWebhookSpy).toBeCalledWith(expect.anything(), { + method: 'GET', + path: 'test-path', + }); + findWebhookSpy.mockRestore(); + }); + + it('should return trigger.webhookId when trigger.parameters.path is an empty string', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + parameters: { + method: 'GET', + path: '', + }, + webhookId: 'test-webhook-id', + }, + ], + } as unknown as IWorkflowDb); + const findWebhookSpy = vi.spyOn(apiWebhooks, 'findWebhook'); + findWebhookSpy.mockResolvedValue(null); + expect(await workflowHelpers.checkConflictingWebhooks('12345')).toEqual(null); + expect(findWebhookSpy).toBeCalledWith(expect.anything(), { + method: 'GET', + path: 'test-webhook-id', + }); + findWebhookSpy.mockRestore(); + }); }); describe('executeData', () => { diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts index 1e41d42e99..2a2c307723 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts @@ -922,7 +922,7 @@ export function useWorkflowHelpers() { function getWebhookPath(trigger: INode) { if (trigger.type === WEBHOOK_NODE_TYPE) { - return trigger.parameters.path as string; + return (trigger.parameters.path as string) || (trigger.webhookId as string); } if (trigger.type === FORM_TRIGGER_NODE_TYPE) { return ((trigger.parameters.options as { path: string }) || {}).path ?? trigger.webhookId; diff --git a/packages/nodes-base/nodes/Webhook/Webhook.node.ts b/packages/nodes-base/nodes/Webhook/Webhook.node.ts index f14ff4ecaa..102d5e7685 100644 --- a/packages/nodes-base/nodes/Webhook/Webhook.node.ts +++ b/packages/nodes-base/nodes/Webhook/Webhook.node.ts @@ -133,7 +133,6 @@ export class Webhook extends Node { type: 'string', default: '', placeholder: 'webhook', - required: true, description: "The path to listen to, dynamic values could be specified by using ':', e.g. 'your-path/:dynamic-value'. If dynamic values are set 'webhookId' would be prepended to path.", }, diff --git a/packages/workflow/src/node-helpers.ts b/packages/workflow/src/node-helpers.ts index 0fc0f59cf3..7852a9d8ce 100644 --- a/packages/workflow/src/node-helpers.ts +++ b/packages/workflow/src/node-helpers.ts @@ -956,17 +956,22 @@ export function getNodeWebhookPath( path: string, isFullPath?: boolean, restartWebhook?: boolean, -): string { +) { let webhookPath = ''; + if (restartWebhook === true) { return path; } + if (node.webhookId === undefined) { - webhookPath = `${workflowId}/${encodeURIComponent(node.name.toLowerCase())}/${path}`; + const nodeName = encodeURIComponent(node.name.toLowerCase()); + + webhookPath = `${workflowId}/${nodeName}/${path}`; } else { if (isFullPath === true) { - return path; + return path || node.webhookId; } + webhookPath = `${node.webhookId}/${path}`; } return webhookPath; diff --git a/packages/workflow/test/node-helpers.test.ts b/packages/workflow/test/node-helpers.test.ts index d1d3f46949..b9e893d626 100644 --- a/packages/workflow/test/node-helpers.test.ts +++ b/packages/workflow/test/node-helpers.test.ts @@ -21,8 +21,10 @@ import { isDefaultNodeName, makeNodeName, isTool, + getNodeWebhookPath, } from '../src/node-helpers'; import type { Workflow } from '../src/workflow'; +import { mock } from 'vitest-mock-extended'; describe('NodeHelpers', () => { describe('getNodeParameters', () => { @@ -4382,6 +4384,7 @@ describe('NodeHelpers', () => { test(testData.description, () => { // If this test has a custom mock return value, configure it if (testData.mockReturnValue) { + // eslint-disable-next-line @typescript-eslint/unbound-method vi.mocked(workflowMock.expression.getSimpleParameterValue).mockReturnValueOnce( testData.mockReturnValue, ); @@ -5611,4 +5614,48 @@ describe('NodeHelpers', () => { expect(result).toBe(false); }); }); + describe('getNodeWebhookPath', () => { + const mockWorkflowId = 'workflow-123'; + const mockPath = 'test-path'; + + it('should return path when restartWebhook is true', () => { + const node = mock({ name: 'TestNode' }); + + const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, true); + + expect(result).toBe(mockPath); + }); + + it('should return path when node has webhookId and isFullPath is true', () => { + const node = mock({ name: 'TestNode', webhookId: 'webhook-456' }); + + const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, true, false); + + expect(result).toBe(mockPath); + }); + + it('should return webhookId when node has webhookId, isFullPath is true, and path is empty', () => { + const node = mock({ name: 'TestNode', webhookId: 'webhook-456' }); + + const result = getNodeWebhookPath(mockWorkflowId, node, '', true, false); + + expect(result).toBe('webhook-456'); + }); + + it('should return webhookId/path when node has webhookId and isFullPath is false', () => { + const node = mock({ name: 'TestNode', webhookId: 'webhook-456' }); + + const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, false); + + expect(result).toBe('webhook-456/test-path'); + }); + + it('should return workflowId/nodename/path when node has no webhookId', () => { + const node = mock({ name: 'TestNode', webhookId: undefined }); + + const result = getNodeWebhookPath(mockWorkflowId, node, mockPath, false, false); + + expect(result).toBe('workflow-123/testnode/test-path'); + }); + }); });