From da41d31bc0e19667a7fef7fac4008c7cb1c6c470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Thu, 30 May 2024 15:25:14 +0200 Subject: [PATCH] fix(editor): Fix empty node name handling (#9548) Co-authored-by: oleg --- cypress/e2e/12-canvas.cy.ts | 11 +++ cypress/e2e/7-workflow-actions.cy.ts | 13 ++++ ...kflow-actions_import_nodes_empty_name.json | 69 +++++++++++++++++++ packages/editor-ui/src/views/NodeView.vue | 16 ++++- 4 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 cypress/fixtures/Test_workflow-actions_import_nodes_empty_name.json diff --git a/cypress/e2e/12-canvas.cy.ts b/cypress/e2e/12-canvas.cy.ts index c1e06c107d636..13fef5b10ccce 100644 --- a/cypress/e2e/12-canvas.cy.ts +++ b/cypress/e2e/12-canvas.cy.ts @@ -364,6 +364,17 @@ describe('Canvas Node Manipulation and Navigation', () => { WorkflowPage.getters.canvasNodeByName(RENAME_NODE_NAME2).should('exist'); }); + it('should not allow empty strings for node names', () => { + WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(CODE_NODE_NAME); + WorkflowPage.getters.canvasNodes().last().click(); + cy.get('body').trigger('keydown', { key: 'F2' }); + cy.get('.rename-prompt').should('be.visible'); + cy.get('body').type('{backspace}'); + cy.get('body').type('{enter}'); + cy.get('.rename-prompt').should('contain', 'Invalid Name'); + }); + it('should duplicate nodes (context menu or shortcut)', () => { WorkflowPage.actions.addNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); WorkflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); diff --git a/cypress/e2e/7-workflow-actions.cy.ts b/cypress/e2e/7-workflow-actions.cy.ts index 75f543ae62a82..bbb781c78e7c3 100644 --- a/cypress/e2e/7-workflow-actions.cy.ts +++ b/cypress/e2e/7-workflow-actions.cy.ts @@ -147,6 +147,19 @@ describe('Workflow Actions', () => { }); }); + it('should allow importing nodes without names', () => { + cy.fixture('Test_workflow-actions_import_nodes_empty_name.json').then((data) => { + cy.get('body').paste(JSON.stringify(data)); + WorkflowPage.actions.zoomToFit(); + WorkflowPage.getters.canvasNodes().should('have.length', 3); + WorkflowPage.getters.nodeConnections().should('have.length', 2); + // Check if all nodes have names + WorkflowPage.getters.canvasNodes().each((node) => { + cy.wrap(node).should('have.attr', 'data-name'); + }); + }); + }); + it('should update workflow settings', () => { cy.visit(WorkflowPages.url); WorkflowPages.getters.workflowCards().then((cards) => { diff --git a/cypress/fixtures/Test_workflow-actions_import_nodes_empty_name.json b/cypress/fixtures/Test_workflow-actions_import_nodes_empty_name.json new file mode 100644 index 0000000000000..e07e32119f36c --- /dev/null +++ b/cypress/fixtures/Test_workflow-actions_import_nodes_empty_name.json @@ -0,0 +1,69 @@ +{ + "meta": { + "instanceId": "4a31be0d29cfa6246ba62b359030d712af57b98c5dfe6a7ee8beee0a46c5b5a4" + }, + "nodes": [ + { + "parameters": { + "operation": "get" + }, + "id": "5b084875-bd5e-4731-9591-18d2c8996945", + "name": "", + "type": "n8n-nodes-base.gmail", + "typeVersion": 2.1, + "position": [ + 900, + 460 + ] + }, + { + "parameters": {}, + "id": "449ab540-d9d7-480d-b131-05e9989a69cd", + "name": "When clicking \"Test workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 460, + 460 + ] + }, + { + "parameters": { + "operation": "get" + }, + "id": "3a791321-6f0c-4f92-91e5-20e1be0d4964", + "name": "Gmail", + "type": "n8n-nodes-base.gmail", + "typeVersion": 2.1, + "position": [ + 680, + 460 + ] + } + ], + "connections": { + "When clicking \"Test workflow\"": { + "main": [ + [ + { + "node": "Gmail", + "type": "main", + "index": 0 + } + ] + ] + }, + "Gmail": { + "main": [ + [ + { + "node": "Gmail1", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": {} +} diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index cac6463f87999..0b8d4a8e91c01 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -2174,7 +2174,15 @@ export default defineComponent({ try { const nodeIdMap: { [prev: string]: string } = {}; if (workflowData.nodes) { + const nodeNames = workflowData.nodes.map((node) => node.name); workflowData.nodes.forEach((node: INode) => { + // Provide a new name for nodes that don't have one + if (!node.name) { + const nodeType = this.nodeTypesStore.getNodeType(node.type); + const newName = this.uniqueNodeName(nodeType?.displayName ?? node.type, nodeNames); + node.name = newName; + nodeNames.push(newName); + } //generate new webhookId if workflow already contains a node with the same webhookId if (node.webhookId && UPDATE_WEBHOOK_ID_NODE_TYPES.includes(node.type)) { const isDuplicate = Object.values( @@ -4137,6 +4145,12 @@ export default defineComponent({ cancelButtonText: this.$locale.baseText('nodeView.prompt.cancel'), inputErrorMessage: this.$locale.baseText('nodeView.prompt.invalidName'), inputValue: currentName, + inputValidator: (value: string) => { + if (!value.trim()) { + return this.$locale.baseText('nodeView.prompt.invalidName'); + } + return true; + }, }, ); @@ -4402,7 +4416,7 @@ export default defineComponent({ async addNodesToWorkflow(data: IWorkflowDataUpdate): Promise { // Because nodes with the same name maybe already exist, it could // be needed that they have to be renamed. Also could it be possible - // that nodes are not allowd to be created because they have a create + // that nodes are not allowed to be created because they have a create // limit set. So we would then link the new nodes with the already existing ones. // In this object all that nodes get saved in the format: // old-name -> new-name