Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Update webhook paths when duplicating workflow #9516

Merged
merged 11 commits into from
May 29, 2024
Merged

fix(editor): Update webhook paths when duplicating workflow #9516

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a785426
fix(editor): Update webhook paths when duplicating workflow
MiloradFilipovic May 27, 2024
dd10c9d
⏪ Reverting testing code
MiloradFilipovic May 27, 2024
a6dae64
✅ Adding e2e tests
MiloradFilipovic May 27, 2024
46973c8
✅ Added initial version of unit test
MiloradFilipovic May 27, 2024
36ae8c3
✔️ Finished initial unit tests
MiloradFilipovic May 28, 2024
717c8f4
⚡ Updating wait node webhook suffix when duplicating
MiloradFilipovic May 28, 2024
6de2dde
🔨 Extracting webhook suffix logic to separate function, removing unus…
MiloradFilipovic May 28, 2024
5edab17
🔥 Removing webhook suffix logic
MiloradFilipovic May 29, 2024
fa9a982
✔️ Creating new test workflow for each test
MiloradFilipovic May 29, 2024
9639b8f
Merge branch 'master' into ADO-2149-update-webhook-path-when-duplicating
MiloradFilipovic May 29, 2024
7022211
👌 Updating comments in tests
MiloradFilipovic May 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 19 additions & 40 deletions packages/editor-ui/src/composables/useWorkflowHelpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ import type { IWorkflowDataUpdate } from '@/Interface';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import router from '@/router';
import { createTestingPinia } from '@pinia/testing';
import type { INode } from 'n8n-workflow';
import { setActivePinia } from 'pinia';

const TEST_WEBHOOK_SUFFIX = '/test';

const duplicateTestWorkflow: IWorkflowDataUpdate = {
const getDuplicateTestWorkflow = (): IWorkflowDataUpdate => ({
name: 'Duplicate webhook test',
active: false,
nodes: [
Expand Down Expand Up @@ -39,7 +36,7 @@ const duplicateTestWorkflow: IWorkflowDataUpdate = {
parameters: {
resume: 'webhook',
options: {
webhookSuffix: TEST_WEBHOOK_SUFFIX,
webhookSuffix: '/test',
},
},
id: '979d8443-51b1-48e2-b239-acf399b66509',
Expand All @@ -51,23 +48,7 @@ const duplicateTestWorkflow: IWorkflowDataUpdate = {
},
],
connections: {},
};

/**
* Extract webhook suffixes from nodes that have them
* @param nodes List of nodes
* @returns List of webhook suffixes found in the nodes
*/
const extractWebhookSuffixes = (nodes: INode[]) => {
return nodes
.map((node) => node.parameters.options)
.filter(
(options): options is { webhookSuffix: string } =>
options !== null && typeof options === 'object' && 'webhookSuffix' in options,
)
.map((options) => options.webhookSuffix)
.filter((suffix) => suffix);
};
});

vi.mock('@/stores/workflows.store', () => ({
useWorkflowsStore: vi.fn(() => ({
Expand Down Expand Up @@ -96,49 +77,47 @@ describe('useWorkflowHelpers', () => {
});

it('should respect `resetWebhookUrls: false` when duplicating workflows', async () => {
if (!duplicateTestWorkflow.nodes) {
const workflow = getDuplicateTestWorkflow();
if (!workflow.nodes) {
throw new Error('Missing nodes in test workflow');
}
const { saveAsNewWorkflow } = useWorkflowHelpers({ router });
const webHookIdsPreSave = duplicateTestWorkflow.nodes.map((node) => node.webhookId);
const pathsPreSave = duplicateTestWorkflow.nodes.map((node) => node.parameters.path);
const webHookIdsPreSave = workflow.nodes.map((node) => node.webhookId);
const pathsPreSave = workflow.nodes.map((node) => node.parameters.path);

await saveAsNewWorkflow({
name: duplicateTestWorkflow.name,
name: workflow.name,
resetWebhookUrls: false,
data: duplicateTestWorkflow,
data: workflow,
});

const webHookIdsPostSave = duplicateTestWorkflow.nodes.map((node) => node.webhookId);
const pathsPostSave = duplicateTestWorkflow.nodes.map((node) => node.parameters.path);
const webhookSuffixPostSave = extractWebhookSuffixes(duplicateTestWorkflow.nodes);
const webHookIdsPostSave = workflow.nodes.map((node) => node.webhookId);
const pathsPostSave = workflow.nodes.map((node) => node.parameters.path);
// Expect webhookIds, paths and suffix to be the same as in the original workflow
expect(webHookIdsPreSave).toEqual(webHookIdsPostSave);
expect(pathsPreSave).toEqual(pathsPostSave);
expect(webhookSuffixPostSave).toEqual([TEST_WEBHOOK_SUFFIX]);
});

it('should respect `resetWebhookUrls: true` when duplicating workflows', async () => {
if (!duplicateTestWorkflow.nodes) {
const workflow = getDuplicateTestWorkflow();
if (!workflow.nodes) {
throw new Error('Missing nodes in test workflow');
}
const { saveAsNewWorkflow } = useWorkflowHelpers({ router });
const webHookIdsPreSave = duplicateTestWorkflow.nodes.map((node) => node.webhookId);
const pathsPreSave = duplicateTestWorkflow.nodes.map((node) => node.parameters.path);
const webHookIdsPreSave = workflow.nodes.map((node) => node.webhookId);
const pathsPreSave = workflow.nodes.map((node) => node.parameters.path);

await saveAsNewWorkflow({
name: duplicateTestWorkflow.name,
name: workflow.name,
resetWebhookUrls: true,
data: duplicateTestWorkflow,
data: workflow,
});

const webHookIdsPostSave = duplicateTestWorkflow.nodes.map((node) => node.webhookId);
const pathsPostSave = duplicateTestWorkflow.nodes.map((node) => node.parameters.path);
const webhookSuffixPostSave = extractWebhookSuffixes(duplicateTestWorkflow.nodes);
const webHookIdsPostSave = workflow.nodes.map((node) => node.webhookId);
const pathsPostSave = workflow.nodes.map((node) => node.parameters.path);
// Now, expect webhookIds, paths and suffix to be different
MiloradFilipovic marked this conversation as resolved.
Show resolved Hide resolved
expect(webHookIdsPreSave).not.toEqual(webHookIdsPostSave);
expect(pathsPreSave).not.toEqual(pathsPostSave);
expect(webhookSuffixPostSave).toEqual([`${TEST_WEBHOOK_SUFFIX}-copy`]);
});
});
});
8 changes: 0 additions & 8 deletions packages/editor-ui/src/composables/useWorkflowHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,14 +1009,6 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
node.parameters.path = newId;
changedNodes[node.name] = node.webhookId;
}
// Also update the webhookSuffix if it exists in wait nodes
if (
node.parameters.options &&
typeof node.parameters.options === 'object' &&
'webhookSuffix' in node.parameters.options
) {
node.parameters.options.webhookSuffix = `${node.parameters.options.webhookSuffix}-copy`;
}
return node;
});
}
Expand Down
Loading