Skip to content

Commit

Permalink
fix(editor): Fix unique names for node duplication (#6134)
Browse files Browse the repository at this point in the history
* 🐛 Fix unique names for node duplication

* 🐛 Fix i18n references
  • Loading branch information
ivov authored Apr 28, 2023
1 parent 20a72bb commit 71ae6c6
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 102 deletions.
2 changes: 1 addition & 1 deletion packages/editor-ui/src/components/VariablesRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
});
});
16 changes: 15 additions & 1 deletion packages/editor-ui/src/composables/useI18n.ts
Original file line number Diff line number Diff line change
@@ -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 };
}
2 changes: 1 addition & 1 deletion packages/editor-ui/src/composables/useToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
108 changes: 108 additions & 0 deletions packages/editor-ui/src/composables/useUniqueNodeName.ts
Original file line number Diff line number Diff line change
@@ -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 };
}
2 changes: 1 addition & 1 deletion packages/editor-ui/src/composables/useUpgradeLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions packages/editor-ui/src/stores/workflows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 71ae6c6

Please sign in to comment.