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.

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
1 change: 1 addition & 0 deletions cypress/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const AI_TOOL_CODE_NODE_NAME = 'Custom Code Tool';
export const AI_TOOL_WIKIPEDIA_NODE_NAME = 'Wikipedia';
export const AI_LANGUAGE_MODEL_OPENAI_CHAT_MODEL_NODE_NAME = 'OpenAI Chat Model';
export const AI_OUTPUT_PARSER_AUTO_FIXING_NODE_NAME = 'Auto-fixing Output Parser';
export const WEBHOOK_NODE_NAME = 'Webhook';

export const META_KEY = Cypress.platform === 'darwin' ? '{meta}' : '{ctrl}';

Expand Down
1 change: 1 addition & 0 deletions cypress/e2e/7-workflow-actions.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
EDIT_FIELDS_SET_NODE_NAME,
INSTANCE_MEMBERS,
INSTANCE_OWNER,
WEBHOOK_NODE_NAME,
} from '../constants';
import { WorkflowPage as WorkflowPageClass } from '../pages/workflow';
import { WorkflowsPage as WorkflowsPageClass } from '../pages/workflows';
Expand Down
10 changes: 5 additions & 5 deletions packages/editor-ui/src/components/DuplicateWorkflowDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,6 @@ export default defineComponent({
prevTagIds: currentTagIds,
};
},
async mounted() {
this.name = await this.workflowsStore.getDuplicateCurrentWorkflowName(this.data.name);
await this.$nextTick();
this.focusOnNameInput();
},
computed: {
...mapStores(useCredentialsStore, useUsersStore, useSettingsStore, useWorkflowsStore),
},
Expand All @@ -104,6 +99,11 @@ export default defineComponent({
}
},
},
async mounted() {
this.name = await this.workflowsStore.getDuplicateCurrentWorkflowName(this.data.name);
await this.$nextTick();
this.focusOnNameInput();
},
methods: {
focusOnSelect() {
this.dropdownBus.emit('focus');
Expand Down
123 changes: 123 additions & 0 deletions packages/editor-ui/src/composables/useWorkflowHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import type { IWorkflowDataUpdate } from '@/Interface';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import router from '@/router';
import { createTestingPinia } from '@pinia/testing';
import { setActivePinia } from 'pinia';

const getDuplicateTestWorkflow = (): IWorkflowDataUpdate => ({
name: 'Duplicate webhook test',
active: false,
nodes: [
{
parameters: {
path: '5340ae49-2c96-4492-9073-7744d2e52b8a',
options: {},
},
id: 'c1e1b6e7-df13-41b1-95f6-42903b85e438',
name: 'Webhook',
type: 'n8n-nodes-base.webhook',
typeVersion: 2,
position: [680, 20],
webhookId: '5340ae49-2c96-4492-9073-7744d2e52b8a',
},
{
parameters: {
path: 'aa5150d8-1d7d-4247-88d8-44c96fe3a37b',
options: {},
},
id: 'aa5150d8-1d7d-4247-88d8-44c96fe3a37b',
name: 'Webhook 2',
type: 'n8n-nodes-base.webhook',
typeVersion: 2,
position: [700, 40],
webhookId: 'aa5150d8-1d7d-4247-88d8-44c96fe3a37b',
},
{
parameters: {
resume: 'webhook',
options: {
webhookSuffix: '/test',
},
},
id: '979d8443-51b1-48e2-b239-acf399b66509',
name: 'Wait',
type: 'n8n-nodes-base.wait',
typeVersion: 1.1,
position: [900, 20],
webhookId: '5340ae49-2c96-4492-9073-7744d2e52b8a',
},
],
connections: {},
});

vi.mock('@/stores/workflows.store', () => ({
useWorkflowsStore: vi.fn(() => ({
workflowsById: {},
createNewWorkflow: vi.fn(() => {}),
addWorkflow: vi.fn(() => {}),
setActive: vi.fn(() => {}),
setWorkflowId: vi.fn(() => {}),
setWorkflowVersionId: vi.fn(() => {}),
setWorkflowName: vi.fn(() => {}),
setWorkflowSettings: vi.fn(() => {}),
setNodeValue: vi.fn(() => {}),
setWorkflowTagIds: vi.fn(() => {}),
getCurrentWorkflow: vi.fn(() => ({})),
})),
}));

describe('useWorkflowHelpers', () => {
describe('saveAsNewWorkflow', () => {
beforeAll(() => {
setActivePinia(createTestingPinia());
});

afterEach(() => {
vi.clearAllMocks();
});

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

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

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

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

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

const webHookIdsPostSave = workflow.nodes.map((node) => node.webhookId);
const pathsPostSave = workflow.nodes.map((node) => node.parameters.path);
// Now, expect webhookIds and paths to be different
expect(webHookIdsPreSave).not.toEqual(webHookIdsPostSave);
expect(pathsPreSave).not.toEqual(pathsPostSave);
});
});
});
4 changes: 3 additions & 1 deletion packages/editor-ui/src/composables/useWorkflowHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,7 +1004,9 @@ export function useWorkflowHelpers(options: { router: ReturnType<typeof useRoute
if (resetWebhookUrls) {
workflowDataRequest.nodes = workflowDataRequest.nodes!.map((node) => {
if (node.webhookId) {
node.webhookId = uuid();
const newId = uuid();
netroy marked this conversation as resolved.
Show resolved Hide resolved
node.webhookId = newId;
node.parameters.path = newId;
changedNodes[node.name] = node.webhookId;
}
return node;
Expand Down
Loading