Skip to content

Commit

Permalink
fix(editor): Do not overwrite the webhookId in the new canvas (#11562)
Browse files Browse the repository at this point in the history
  • Loading branch information
netroy authored Nov 5, 2024
1 parent 463d101 commit dfd785b
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 49 deletions.
47 changes: 1 addition & 46 deletions packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {
IVersionNotificationSettings,
} from '@n8n/api-types';
import type { Scope } from '@n8n/permissions';
import type { IMenuItem, NodeCreatorTag } from 'n8n-design-system';
import type { NodeCreatorTag } from 'n8n-design-system';
import type {
GenericValue,
IConnections,
Expand Down Expand Up @@ -888,51 +888,6 @@ export interface RootState {
export interface NodeMetadataMap {
[nodeName: string]: INodeMetadata;
}
export interface IRootState {
activeExecutions: IExecutionsCurrentSummaryExtended[];
activeWorkflows: string[];
activeActions: string[];
activeCredentialType: string | null;
baseUrl: string;
defaultLocale: string;
endpointForm: string;
endpointFormTest: string;
endpointFormWaiting: string;
endpointWebhook: string;
endpointWebhookTest: string;
endpointWebhookWaiting: string;
executionId: string | null;
executingNode: string[];
executionWaitingForWebhook: boolean;
pushConnectionActive: boolean;
saveDataErrorExecution: string;
saveDataSuccessExecution: string;
saveManualExecutions: boolean;
timezone: string;
stateIsDirty: boolean;
executionTimeout: number;
maxExecutionTimeout: number;
versionCli: string;
oauthCallbackUrls: object;
n8nMetadata: object;
workflowExecutionData: IExecutionResponse | null;
workflowExecutionPairedItemMappings: { [itemId: string]: Set<string> };
lastSelectedNode: string | null;
lastSelectedNodeOutputIndex: number | null;
nodeViewOffsetPosition: XYPosition;
nodeViewMoveInProgress: boolean;
selectedNodes: INodeUi[];
pushRef: string;
urlBaseEditor: string;
urlBaseWebhook: string;
workflow: IWorkflowDb;
workflowsById: IWorkflowsMap;
sidebarMenuItems: IMenuItem[];
instanceId: string;
nodeMetadata: NodeMetadataMap;
subworkflowExecutionError: Error | null;
binaryDataMode: string;
}

export interface CommunityPackageMap {
[name: string]: PublicInstalledPackage;
Expand Down
65 changes: 63 additions & 2 deletions packages/editor-ui/src/composables/useCanvasOperations.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { setActivePinia } from 'pinia';
import type { IConnection, Workflow } from 'n8n-workflow';
import type {
IConnection,
INodeTypeDescription,
IWebhookDescription,
Workflow,
} from 'n8n-workflow';
import { NodeConnectionType, NodeHelpers } from 'n8n-workflow';
import { useCanvasOperations } from '@/composables/useCanvasOperations';
import type { CanvasNode } from '@/types';
Expand All @@ -23,7 +28,13 @@ import { useCredentialsStore } from '@/stores/credentials.store';
import { waitFor } from '@testing-library/vue';
import { createTestingPinia } from '@pinia/testing';
import { mockedStore } from '@/__tests__/utils';
import { SET_NODE_TYPE, STICKY_NODE_TYPE, STORES } from '@/constants';
import {
FORM_TRIGGER_NODE_TYPE,
SET_NODE_TYPE,
STICKY_NODE_TYPE,
STORES,
WEBHOOK_NODE_TYPE,
} from '@/constants';
import type { Connection } from '@vue-flow/core';
import { useClipboard } from '@/composables/useClipboard';
import { createCanvasConnectionHandleString } from '@/utils/canvasUtilsV2';
Expand Down Expand Up @@ -1889,6 +1900,56 @@ describe('useCanvasOperations', () => {
expect(vi.mocked(useClipboard().copy).mock.calls).toMatchSnapshot();
});
});

describe('resolveNodeWebhook', () => {
const nodeTypeDescription = mock<INodeTypeDescription>({
webhooks: [mock<IWebhookDescription>()],
});

it("should set webhookId if it doesn't already exist", () => {
const node = mock<INodeUi>({ webhookId: undefined });

const { resolveNodeWebhook } = useCanvasOperations({ router });
resolveNodeWebhook(node, nodeTypeDescription);

expect(node.webhookId).toBeDefined();
});

it('should not set webhookId if it already exists', () => {
const node = mock<INodeUi>({ webhookId: 'random-id' });

const { resolveNodeWebhook } = useCanvasOperations({ router });
resolveNodeWebhook(node, nodeTypeDescription);

expect(node.webhookId).toBe('random-id');
});

it("should not set webhookId if node description doesn't define any webhooks", () => {
const node = mock<INodeUi>({ webhookId: undefined });

const { resolveNodeWebhook } = useCanvasOperations({ router });
resolveNodeWebhook(node, mock<INodeTypeDescription>({ webhooks: [] }));

expect(node.webhookId).toBeUndefined();
});

test.each([WEBHOOK_NODE_TYPE, FORM_TRIGGER_NODE_TYPE])(
'should update the webhook path, if the node type is %s, and the path parameter is empty',
(nodeType) => {
const node = mock<INodeUi>({
webhookId: 'random-id',
type: nodeType,
parameters: { path: '' },
});

const { resolveNodeWebhook } = useCanvasOperations({ router });
resolveNodeWebhook(node, nodeTypeDescription);

expect(node.webhookId).toBe('random-id');
expect(node.parameters.path).toBe('random-id');
},
);
});
});

function buildImportNodes() {
Expand Down
3 changes: 2 additions & 1 deletion packages/editor-ui/src/composables/useCanvasOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1080,7 +1080,7 @@ export function useCanvasOperations({ router }: { router: ReturnType<typeof useR
}

function resolveNodeWebhook(node: INodeUi, nodeTypeDescription: INodeTypeDescription) {
if (nodeTypeDescription.webhooks?.length) {
if (nodeTypeDescription.webhooks?.length && !node.webhookId) {
node.webhookId = uuid();
}

Expand Down Expand Up @@ -1932,5 +1932,6 @@ export function useCanvasOperations({ router }: { router: ReturnType<typeof useR
fetchWorkflowDataFromUrl,
resetWorkspace,
initializeWorkspace,
resolveNodeWebhook,
};
}

0 comments on commit dfd785b

Please sign in to comment.