fix(editor): Fix unique names for node duplication (#6134)

* 🐛 Fix unique names for node duplication

* 🐛 Fix i18n references

packages/editor-ui/src/components/VariablesRow.vue

@@ -7,7 +7,7 @@ import { EnterpriseEditionFeature } from '@/constants';
 import { useSettingsStore, useUsersStore } from '@/stores';
 import { getVariablesPermissions } from '@/permissions';
 
-const i18n = useI18n();
+const { i18n } = useI18n();
 const copyToClipboard = useCopyToClipboard();
 const { showMessage } = useToast();
 const settingsStore = useSettingsStore();

packages/editor-ui/src/composables/__tests__/useUniqueNodeName.test.ts

@@ -0,0 +1,79 @@
+import { setActivePinia } from 'pinia';
+import { createTestingPinia } from '@pinia/testing';
+import { useUniqueNodeName } from '@/composables/useUniqueNodeName';
+import { useNodeTypesStore } from '@/stores/nodeTypes';
+import { useWorkflowsStore } from '@/stores/workflows';
+
+describe('useUniqueNodeName', () => {
+	beforeAll(() => {
+		setActivePinia(createTestingPinia());
+	});
+
+	afterEach(() => {
+		vi.clearAllMocks();
+	});
+
+	test('should return a unique node name for an alphabetic node name', () => {
+		const workflowsStore = useWorkflowsStore();
+
+		const mockCanvasNames = new Set(['Hello']);
+
+		vi.spyOn(workflowsStore, 'canvasNames', 'get').mockReturnValue(mockCanvasNames);
+
+		const { uniqueNodeName } = useUniqueNodeName();
+
+		expect(uniqueNodeName('Hello')).toBe('Hello1');
+
+		mockCanvasNames.add('Hello1');
+
+		expect(uniqueNodeName('Hello')).toBe('Hello2');
+	});
+
+	test('should return a unique node name for a numeric node name', () => {
+		const workflowsStore = useWorkflowsStore();
+
+		const mockCanvasNames = new Set(['123']);
+
+		vi.spyOn(workflowsStore, 'canvasNames', 'get').mockReturnValue(mockCanvasNames);
+
+		const { uniqueNodeName } = useUniqueNodeName();
+
+		expect(uniqueNodeName('123')).toBe('123-1');
+
+		mockCanvasNames.add('123-1');
+
+		expect(uniqueNodeName('123')).toBe('123-2');
+	});
+
+	test('should return a unique node name for a number-suffixed node name', () => {
+		const workflowsStore = useWorkflowsStore();
+		const nodeTypesStore = useNodeTypesStore();
+
+		const mockCanvasNames = new Set(['S3']);
+
+		vi.spyOn(workflowsStore, 'canvasNames', 'get').mockReturnValue(mockCanvasNames);
+		vi.spyOn(nodeTypesStore, 'allNodeTypes', 'get').mockReturnValue([
+			{
+				displayName: 'S3',
+				name: 'S3',
+				description: '',
+				version: 1,
+				inputs: [''],
+				outputs: [''],
+				group: [''],
+				properties: [],
+				defaults: {
+					name: 'S3',
+				},
+			},
+		]);
+
+		const { uniqueNodeName } = useUniqueNodeName();
+
+		expect(uniqueNodeName('S3')).toBe('S31');
+
+		mockCanvasNames.add('S31');
+
+		expect(uniqueNodeName('S3')).toBe('S32');
+	});
+});

packages/editor-ui/src/composables/useI18n.ts

@@ -1,5 +1,19 @@
 import { i18n } from '@/plugins/i18n';
+import { useRootStore } from '@/stores/n8nRootStore';
 
 export function useI18n() {
-	return i18n;
+	const isEnglishLocale = useRootStore().defaultLocale === 'en';
+
+	function localizeNodeName(nodeName: string, type: string) {
+		if (isEnglishLocale) return nodeName;
+
+		const nodeTypeName = i18n.shortNodeType(type);
+
+		return i18n.headerText({
+			key: `headers.${nodeTypeName}.displayName`,
+			fallback: nodeName,
+		});
+	}
+
+	return { i18n, localizeNodeName };
 }

packages/editor-ui/src/composables/useToast.ts

@@ -18,7 +18,7 @@ export function useToast() {
 	const telemetry = useTelemetry();
 	const workflowsStore = useWorkflowsStore();
 	const externalHooks = useExternalHooks();
-	const i18n = useI18n();
+	const { i18n } = useI18n();
 
 	function showMessage(
 		messageData: Omit<ElNotificationOptions, 'message'> & { message?: string },

packages/editor-ui/src/composables/useUniqueNodeName.ts

@@ -0,0 +1,108 @@
+import { useWorkflowsStore } from '@/stores/workflows';
+import { useNodeTypesStore } from '@/stores/nodeTypes';
+
+export function useUniqueNodeName() {
+	/**
+	 * All in-store node name defaults ending with a number, e.g.
+	 * `AWS S3`, `Magento 2`, `MSG91`, `S3`, `SIGNL4`, `sms77`
+	 */
+	function numberSuffixedNames() {
+		return useNodeTypesStore().allNodeTypes.reduce<string[]>((acc, nodeType) => {
+			if (typeof nodeType.defaults.name !== 'string') {
+				throw new Error('Expected node name default to be a string');
+			}
+
+			if (/\d$/.test(nodeType.defaults.name)) acc.push(nodeType.defaults.name);
+
+			return acc;
+		}, []);
+	}
+
+	/**
+	 * Create a unique node name from an original name, based on the names of
+	 * all nodes on canvas and any extra names that cannot be used.
+	 */
+	function uniqueNodeName(originalName: string, extraNames: string[] = []) {
+		const { canvasNames } = useWorkflowsStore();
+
+		const isUnique = !canvasNames.has(originalName) && !extraNames.includes(originalName);
+
+		if (isUnique) return originalName;
+
+		const nsn = numberSuffixedNames().find((nsn) => originalName.startsWith(nsn));
+
+		// edge case, number suffix as part of name: S3 -> S31 -> S32
+
+		if (nsn) {
+			let unique = '';
+			let index = 1;
+
+			const remainder = originalName.split(nsn).pop();
+
+			const lastChar = remainder?.[remainder.length - 1];
+
+			if (lastChar && Number.isInteger(Number(lastChar))) {
+				index = parseInt(lastChar, 10);
+				originalName = originalName.slice(0, -1);
+			}
+
+			unique = originalName;
+
+			while (canvasNames.has(unique) || extraNames.includes(unique)) {
+				unique = originalName + index++;
+			}
+
+			return unique;
+		}
+
+		// edge case, all-number name: 123 -> 123-1 -> 123-2
+
+		if (/^\d+-?\d*$/.test(originalName)) {
+			let unique = '';
+			let index = 1;
+
+			const match = originalName.match(/(?<base>\d+)-?(?<suffix>\d*)/);
+
+			if (!match?.groups) {
+				throw new Error('Failed to find match for unique name');
+			}
+
+			if (match?.groups?.suffix !== '') {
+				index = parseInt(match.groups.suffix, 10);
+			}
+
+			unique = match.groups.base;
+
+			while (canvasNames.has(unique) || extraNames.includes(unique)) {
+				unique = match.groups.base + '-' + index++;
+			}
+
+			return unique;
+		}
+
+		// normal case: A -> A1 -> A2
+
+		let unique = '';
+		let index = 1;
+
+		const match = originalName.match(/(?<base>.*\D+)(?<suffix>\d*)/);
+
+		if (!match?.groups) {
+			throw new Error('Failed to find match for unique name');
+		}
+
+		if (match?.groups?.suffix !== '') {
+			index = parseInt(match.groups.suffix, 10);
+		}
+
+		unique = match.groups.base;
+
+		while (canvasNames.has(unique) || extraNames.includes(unique)) {
+			unique = match.groups.base + index++;
+		}
+
+		return unique;
+	}
+
+	return { uniqueNodeName };
+}

packages/editor-ui/src/composables/useUpgradeLink.ts

@@ -6,7 +6,7 @@ import { computed } from 'vue';
 export function useUpgradeLink(queryParams = { default: '', desktop: '' }) {
 	const uiStore = useUIStore();
 	const usageStore = useUsageStore();
-	const i18n = useI18n();
+	const { i18n } = useI18n();
 
 	const upgradeLinkUrl = computed(() => {
 		const linkUrlTranslationKey = uiStore.contextBasedTranslationKeys.upgradeLinkUrl as BaseTextKey;

packages/editor-ui/src/stores/workflows.ts

@@ -204,6 +204,12 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
 		allNodes(): INodeUi[] {
 			return this.workflow.nodes;
 		},
+		/**
+		 * Names of all nodes currently on canvas.
+		 */
+		canvasNames(): Set<string> {
+			return new Set(this.allNodes.map((n) => n.name));
+		},
 		nodesByName(): { [name: string]: INodeUi } {
 			return this.workflow.nodes.reduce((accu: { [name: string]: INodeUi }, node) => {
 				accu[node.name] = node;

packages/editor-ui/src/views/NodeView.vue

@@ -210,6 +210,8 @@ import useGlobalLinkActions from '@/composables/useGlobalLinkActions';
 import useCanvasMouseSelect from '@/composables/useCanvasMouseSelect';
 import { showMessage } from '@/mixins/showMessage';
 import { useTitleChange } from '@/composables/useTitleChange';
+import { useUniqueNodeName } from '@/composables/useUniqueNodeName';
+import { useI18n } from '@/composables/useI18n';
 
 import { workflowHelpers } from '@/mixins/workflowHelpers';
 import { workflowRun } from '@/mixins/workflowRun';
@@ -341,6 +343,8 @@ export default mixins(
 			...useCanvasMouseSelect(),
 			...useGlobalLinkActions(),
 			...useTitleChange(),
+			...useUniqueNodeName(),
+			...useI18n(),
 		};
 	},
 	errorCaptured: (err, vm, info) => {
@@ -477,12 +481,6 @@ export default mixins(
 		currentUser(): IUser | null {
 			return this.usersStore.currentUser;
 		},
-		defaultLocale(): string {
-			return this.rootStore.defaultLocale;
-		},
-		isEnglishLocale(): boolean {
-			return this.defaultLocale === 'en';
-		},
 		activeNode(): INodeUi | null {
 			return this.ndvStore.activeNode;
 		},
@@ -678,78 +676,6 @@ export default mixins(
 			this.workflowsStore.workflowExecutionData = null;
 			this.updateNodesExecutionIssues();
 		},
-		translateName(type: string, originalName: string) {
-			return this.$locale.headerText({
-				key: `headers.${this.$locale.shortNodeType(type)}.displayName`,
-				fallback: originalName,
-			});
-		},
-		getUniqueNodeName({
-			originalName,
-			additionalUsedNames = [],
-			type = '',
-		}: {
-			originalName: string;
-			additionalUsedNames?: string[];
-			type?: string;
-		}) {
-			const allNodeNamesOnCanvas = this.workflowsStore.allNodes.map((n: INodeUi) => n.name);
-			originalName = this.isEnglishLocale ? originalName : this.translateName(type, originalName);
-
-			if (
-				!allNodeNamesOnCanvas.includes(originalName) &&
-				!additionalUsedNames.includes(originalName)
-			) {
-				return originalName; // already unique
-			}
-
-			let natives: string[] = this.nativelyNumberSuffixedDefaults;
-			natives = this.isEnglishLocale
-				? natives
-				: natives.map((name) => {
-						const type = name.toLowerCase().replace('_', '');
-						return this.translateName(type, name);
-				  });
-
-			const found = natives.find((n) => originalName.startsWith(n));
-
-			let ignore, baseName, nameIndex, uniqueName;
-			let index = 1;
-
-			if (found) {
-				// name natively ends with number
-				nameIndex = originalName.split(found).pop();
-				if (nameIndex) {
-					index = parseInt(nameIndex, 10);
-				}
-				baseName = uniqueName = found;
-			} else {
-				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;
-				}
-			}
-
-			while (
-				allNodeNamesOnCanvas.includes(uniqueName) ||
-				additionalUsedNames.includes(uniqueName)
-			) {
-				uniqueName = baseName + index++;
-			}
-
-			return uniqueName;
-		},
 		async onSaveKeyboardShortcut(e: KeyboardEvent) {
 			let saved = await this.saveCurrentWorkflow();
 			if (saved) await this.settingsStore.fetchPromptsData();
@@ -1931,11 +1857,9 @@ export default mixins(
 				newNodeData.position = NodeViewUtils.getNewNodePosition(this.nodes, position);
 			}
 
-			// Check if node-name is unique else find one that is
-			newNodeData.name = this.getUniqueNodeName({
-				originalName: newNodeData.name,
-				type: newNodeData.type,
-			});
+			const localizedName = this.localizeNodeName(newNodeData.name, newNodeData.type);
+
+			newNodeData.name = this.uniqueNodeName(localizedName);
 
 			if (nodeTypeData.webhooks && nodeTypeData.webhooks.length) {
 				newNodeData.webhookId = uuid();
@@ -2732,11 +2656,9 @@ export default mixins(
 				const newNodeData = deepCopy(this.getNodeDataToSave(node));
 				newNodeData.id = uuid();
 
-				// Check if node-name is unique else find one that is
-				newNodeData.name = this.getUniqueNodeName({
-					originalName: newNodeData.name,
-					type: newNodeData.type,
-				});
+				const localizedName = this.localizeNodeName(newNodeData.name, newNodeData.type);
+
+				newNodeData.name = this.uniqueNodeName(localizedName);
 
 				newNodeData.position = NodeViewUtils.getNewNodePosition(
 					this.nodes,
@@ -3116,10 +3038,7 @@ export default mixins(
 				this.renamingActive = true;
 			}
 
-			// Check if node-name is unique else find one that is
-			newName = this.getUniqueNodeName({
-				originalName: newName,
-			});
+			newName = this.uniqueNodeName(newName);
 
 			// Rename the node and update the connections
 			const workflow = this.getCurrentWorkflow(true);
@@ -3386,11 +3305,10 @@ export default mixins(
 				}
 
 				oldName = node.name;
-				newName = this.getUniqueNodeName({
-					originalName: node.name,
-					additionalUsedNames: newNodeNames,
-					type: node.type,
-				});
+
+				const localized = this.localizeNodeName(node.name, node.type);
+
+				newName = this.uniqueNodeName(localized, newNodeNames);
 
 				newNodeNames.push(newName);
 				nodeNameTable[oldName] = newName;

packages/editor-ui/src/views/VariablesView.vue

@@ -20,7 +20,7 @@ const environmentsStore = useEnvironmentsStore();
 const usersStore = useUsersStore();
 const uiStore = useUIStore();
 const telemetry = useTelemetry();
-const i18n = useI18n();
+const { i18n } = useI18n();
 const message = useMessage();
 
 const layoutRef = ref<InstanceType<typeof ResourcesListLayout> | null>(null);