From c81d47cb1a7b0e287896344d542ac9dafaf98fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 29 Nov 2021 12:20:10 +0100 Subject: [PATCH 1/3] :bug: Fix unique node name creation --- packages/editor-ui/src/store.ts | 15 ++++++ packages/editor-ui/src/views/NodeView.vue | 25 ++++++++-- packages/editor-ui/src/views/canvasHelpers.ts | 49 +++++++++++++------ 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/packages/editor-ui/src/store.ts b/packages/editor-ui/src/store.ts index 09b7014025..b880037ca2 100644 --- a/packages/editor-ui/src/store.ts +++ b/packages/editor-ui/src/store.ts @@ -808,6 +808,21 @@ export const store = new Vuex.Store({ allNodeTypes: (state): INodeTypeDescription[] => { return state.nodeTypes; }, + + /** + * Getter for node default names ending with a number: `'S3'`, `'Magento 2'`, etc. + */ + nativelyNumberSuffixedDefaults: (_, getters): string[] => { + const { allNodeTypes } = getters as { + allNodeTypes: Array; + }; + + return allNodeTypes.reduce((acc, cur) => { + if (/\d$/.test(cur.defaults.name)) acc.push(cur.defaults.name); + return acc; + }, []); + }, + nodeType: (state, getters) => (nodeType: string, typeVersion?: number): INodeTypeDescription | null => { const foundType = state.nodeTypes.find(typeData => { return typeData.name === nodeType && typeData.version === (typeVersion || typeData.defaultVersion || DEFAULT_NODETYPE_VERSION); diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 239c3246cc..9f3a59e98a 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -1180,7 +1180,11 @@ export default mixins( } // Check if node-name is unique else find one that is - newNodeData.name = CanvasHelpers.getUniqueNodeName(this.$store.getters.allNodes, newNodeData.name); + newNodeData.name = CanvasHelpers.getUniqueNodeName({ + nodes: this.$store.getters.allNodes, + originalName: newNodeData.name, + nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + }); if (nodeTypeData.webhooks && nodeTypeData.webhooks.length) { newNodeData.webhookId = uuidv4(); @@ -1753,7 +1757,11 @@ export default mixins( const newNodeData = JSON.parse(JSON.stringify(this.getNodeDataToSave(node))); // Check if node-name is unique else find one that is - newNodeData.name = CanvasHelpers.getUniqueNodeName(this.$store.getters.allNodes, newNodeData.name); + newNodeData.name = CanvasHelpers.getUniqueNodeName({ + nodes: this.$store.getters.allNodes, + originalName: newNodeData.name, + nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + }); newNodeData.position = CanvasHelpers.getNewNodePosition( this.nodes, @@ -1993,7 +2001,11 @@ export default mixins( return; } // Check if node-name is unique else find one that is - newName = CanvasHelpers.getUniqueNodeName(this.$store.getters.allNodes, newName); + newName = CanvasHelpers.getUniqueNodeName({ + nodes: this.$store.getters.allNodes, + originalName: newName, + nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + }); // Rename the node and update the connections const workflow = this.getWorkflow(undefined, undefined, true); @@ -2202,7 +2214,12 @@ export default mixins( } oldName = node.name; - newName = CanvasHelpers.getUniqueNodeName(this.$store.getters.allNodes, node.name, newNodeNames); + newName = CanvasHelpers.getUniqueNodeName({ + nodes: this.$store.getters.allNodes, + originalName: node.name, + additionalUsedNames: newNodeNames, + nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + }); newNodeNames.push(newName); nodeNameTable[oldName] = newName; diff --git a/packages/editor-ui/src/views/canvasHelpers.ts b/packages/editor-ui/src/views/canvasHelpers.ts index 3ea65dbcfe..742be9969c 100644 --- a/packages/editor-ui/src/views/canvasHelpers.ts +++ b/packages/editor-ui/src/views/canvasHelpers.ts @@ -609,9 +609,19 @@ export const getZoomToFit = (nodes: INodeUi[]): {offset: XYPosition, zoomLevel: }; }; -export const getUniqueNodeName = (nodes: INodeUi[], originalName: string, additinalUsedNames?: string[]) => { +export const getUniqueNodeName = ({ + nodes, + originalName, + additionalUsedNames, + nativelyNumberSuffixed, +} : { + nodes: INodeUi[], + originalName: string, + additionalUsedNames?: string[], + nativelyNumberSuffixed: string[], +}) => { // Check if node-name is unique else find one that is - additinalUsedNames = additinalUsedNames || []; + additionalUsedNames = additionalUsedNames || []; // Get all the names of the current nodes const nodeNames = nodes.map((node: INodeUi) => { @@ -619,31 +629,40 @@ export const getUniqueNodeName = (nodes: INodeUi[], originalName: string, additi }); // Check first if the current name is already unique - if (!nodeNames.includes(originalName) && !additinalUsedNames.includes(originalName)) { + if (!nodeNames.includes(originalName) && !additionalUsedNames.includes(originalName)) { return originalName; } - const nameMatch = originalName.match(/(.*\D+)(\d*)/); + const found = nativelyNumberSuffixed.find((n) => originalName.startsWith(n)); + let ignore, baseName, nameIndex, uniqueName; let index = 1; - if (nameMatch === null) { - // Name is only a number - index = parseInt(originalName, 10); - baseName = ''; - uniqueName = baseName + index; + if (found) { + nameIndex = originalName.split(found).pop(); + if (nameIndex) index = parseInt(nameIndex, 10); + baseName = uniqueName = found; } else { - // Name is string or string/number combination - [ignore, baseName, nameIndex] = nameMatch; - if (nameIndex !== '') { - index = parseInt(nameIndex, 10); + const nameMatch = originalName.match(/(.*\D+)(\d*)/); + + if (nameMatch === null) { + // Name is only a number + index = parseInt(originalName, 10); + baseName = ''; + uniqueName = baseName + index; + } else { + // Name is string or string/number combination + [ignore, baseName, nameIndex] = nameMatch; + if (nameIndex !== '') { + index = parseInt(nameIndex, 10); + } + uniqueName = baseName; } - uniqueName = baseName; } while ( nodeNames.includes(uniqueName) || - additinalUsedNames.includes(uniqueName) + additionalUsedNames.includes(uniqueName) ) { uniqueName = baseName + (index++); } From 845b5e4d9b253cf9de148580b6b1ed7264ecdb2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 1 Dec 2021 09:44:56 +0100 Subject: [PATCH 2/3] :zap: Add braces to condition --- packages/editor-ui/src/views/canvasHelpers.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/views/canvasHelpers.ts b/packages/editor-ui/src/views/canvasHelpers.ts index 742be9969c..492b0671c6 100644 --- a/packages/editor-ui/src/views/canvasHelpers.ts +++ b/packages/editor-ui/src/views/canvasHelpers.ts @@ -640,7 +640,9 @@ export const getUniqueNodeName = ({ if (found) { nameIndex = originalName.split(found).pop(); - if (nameIndex) index = parseInt(nameIndex, 10); + if (nameIndex) { + index = parseInt(nameIndex, 10); + } baseName = uniqueName = found; } else { const nameMatch = originalName.match(/(.*\D+)(\d*)/); From 513ddbb383e1eb186bccf990f46a653f9d710944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 1 Dec 2021 10:00:03 +0100 Subject: [PATCH 3/3] :zap: Use mapGetters to simplify calls --- packages/editor-ui/src/views/NodeView.vue | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 9f3a59e98a..e93d3baa83 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -243,6 +243,7 @@ export default mixins( ...mapGetters('ui', [ 'sidebarMenuCollapsed', ]), + ...mapGetters(['nativelyNumberSuffixedDefaults']), activeNode (): INodeUi | null { return this.$store.getters.activeNode; }, @@ -1183,7 +1184,7 @@ export default mixins( newNodeData.name = CanvasHelpers.getUniqueNodeName({ nodes: this.$store.getters.allNodes, originalName: newNodeData.name, - nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + nativelyNumberSuffixed: this.nativelyNumberSuffixedDefaults, }); if (nodeTypeData.webhooks && nodeTypeData.webhooks.length) { @@ -1760,7 +1761,7 @@ export default mixins( newNodeData.name = CanvasHelpers.getUniqueNodeName({ nodes: this.$store.getters.allNodes, originalName: newNodeData.name, - nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + nativelyNumberSuffixed: this.nativelyNumberSuffixedDefaults, }); newNodeData.position = CanvasHelpers.getNewNodePosition( @@ -2004,7 +2005,7 @@ export default mixins( newName = CanvasHelpers.getUniqueNodeName({ nodes: this.$store.getters.allNodes, originalName: newName, - nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + nativelyNumberSuffixed: this.nativelyNumberSuffixedDefaults, }); // Rename the node and update the connections @@ -2218,7 +2219,7 @@ export default mixins( nodes: this.$store.getters.allNodes, originalName: node.name, additionalUsedNames: newNodeNames, - nativelyNumberSuffixed: this.$store.getters.nativelyNumberSuffixedDefaults, + nativelyNumberSuffixed: this.nativelyNumberSuffixedDefaults, }); newNodeNames.push(newName);