diff --git a/libs/designer-v2/src/lib/core/utils/__test__/graph.spec.ts b/libs/designer-v2/src/lib/core/utils/__test__/graph.spec.ts index 2cfb92a4376..71568e85a10 100644 --- a/libs/designer-v2/src/lib/core/utils/__test__/graph.spec.ts +++ b/libs/designer-v2/src/lib/core/utils/__test__/graph.spec.ts @@ -1,6 +1,15 @@ -import { createWorkflowEdge, createWorkflowNode, isRootNode, getAllNodesInsideNode, getUpstreamNodeIds, isTriggerNode } from '../graph'; +import { + createWorkflowEdge, + createWorkflowNode, + isRootNode, + getAllNodesInsideNode, + getUpstreamNodeIds, + isTriggerNode, + isOperationNameValid, +} from '../graph'; import { WORKFLOW_NODE_TYPES } from '@microsoft/logic-apps-shared'; import { describe, vi, beforeEach, afterEach, beforeAll, afterAll, it, test, expect } from 'vitest'; +import { getTestIntl } from '../../../__test__/intl-test-helper'; describe('Graph Utilities', () => { const graph = { id: 'root', @@ -127,6 +136,20 @@ describe('Graph Utilities', () => { }); }); + describe('isOperationNameValid', () => { + it('should return true as there is no node with that name', () => { + expect(isOperationNameValid('Compose', 'Compose_12', 'Compose_', false, nodesMetadata, {}, getTestIntl()).isValid).toBeTruthy(); + }); + + it('should return true as there is no other node with that name, even if the selected node previous and new name are equal', () => { + expect(isOperationNameValid('Compose', 'Compose_13', 'Compose_13', false, nodesMetadata, {}, getTestIntl()).isValid).toBeTruthy(); + }); + + it('should return false as there is already a node with that name', () => { + expect(isOperationNameValid('Compose', 'Compose_5', 'Compose_', false, nodesMetadata, {}, getTestIntl()).isValid).toBeFalsy(); + }); + }); + describe('getAllNodesInsideNode', () => { it('should return all children for a workflow graph node which are operations', () => { expect(getAllNodesInsideNode('Scope_2', graph, operationMap)).toEqual(['Compose_7', 'Compose_8', 'Compose_9']); diff --git a/libs/designer-v2/src/lib/core/utils/graph.ts b/libs/designer-v2/src/lib/core/utils/graph.ts index fe0fb629883..91817b07a99 100644 --- a/libs/designer-v2/src/lib/core/utils/graph.ts +++ b/libs/designer-v2/src/lib/core/utils/graph.ts @@ -217,12 +217,14 @@ export const getFirstParentOfType = ( export const isOperationNameValid = ( nodeId: string, newName: string, + oldName: string, isTrigger: boolean, nodesMetadata: NodesMetadata, idReplacements: Record, intl: IntlShape ): { isValid: boolean; message: string } => { const name = transformOperationTitle(newName); + const previousName = transformOperationTitle(oldName); const subgraphType = getRecordEntry(nodesMetadata, nodeId)?.subgraphType; const messages = { @@ -255,12 +257,17 @@ export const isOperationNameValid = ( } } - // Check for name uniqueness. - const existingNames = Object.keys(nodesMetadata).map((id) => getRecordEntry(idReplacements, id) ?? id); - const isDuplicateName = existingNames.some((nodeName) => equals(nodeName, name)); - if (isDuplicateName) { - return { isValid: false, message: messages.DEFAULT }; + // Check for name uniqueness only when previous and new names of the selected node are not equal + const previousAndNewNamesNotSame = !equals(previousName, name); + + if (previousAndNewNamesNotSame) { + const existingNames = Object.keys(nodesMetadata).map((id) => getRecordEntry(idReplacements, id) ?? id); + const isDuplicateName = existingNames.some((nodeName) => equals(nodeName, name)); + if (isDuplicateName) { + return { isValid: false, message: messages.DEFAULT }; + } } + return { isValid: true, message: '' }; }; diff --git a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx index 1fbd78a45a2..3656f25d1b1 100644 --- a/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx +++ b/libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx @@ -154,7 +154,15 @@ export const NodeDetailsPanel = (props: CommonPanelProps): JSX.Element => { ); const onTitleChange = (originalId: string, newId: string): { valid: boolean; oldValue?: string; message: string } => { - const validation = isOperationNameValid(originalId, newId, isTrigger, nodesMetadata, idReplacements, intl); + const validation = isOperationNameValid( + originalId, + newId, + selectedNodeData!.displayName, + isTrigger, + nodesMetadata, + idReplacements, + intl + ); return { valid: validation.isValid, oldValue: validation.isValid ? newId : originalId, message: validation.message }; }; @@ -284,5 +292,5 @@ export const NodeDetailsPanel = (props: CommonPanelProps): JSX.Element => { }; // TODO: 12798935 Analytics (event logging) -// eslint-disable-next-line @typescript-eslint/no-empty-function + const handleTrackEvent = (_data: PageActionTelemetryData): void => {}; diff --git a/libs/designer/src/lib/core/utils/__test__/graph.spec.ts b/libs/designer/src/lib/core/utils/__test__/graph.spec.ts index 2cfb92a4376..71568e85a10 100644 --- a/libs/designer/src/lib/core/utils/__test__/graph.spec.ts +++ b/libs/designer/src/lib/core/utils/__test__/graph.spec.ts @@ -1,6 +1,15 @@ -import { createWorkflowEdge, createWorkflowNode, isRootNode, getAllNodesInsideNode, getUpstreamNodeIds, isTriggerNode } from '../graph'; +import { + createWorkflowEdge, + createWorkflowNode, + isRootNode, + getAllNodesInsideNode, + getUpstreamNodeIds, + isTriggerNode, + isOperationNameValid, +} from '../graph'; import { WORKFLOW_NODE_TYPES } from '@microsoft/logic-apps-shared'; import { describe, vi, beforeEach, afterEach, beforeAll, afterAll, it, test, expect } from 'vitest'; +import { getTestIntl } from '../../../__test__/intl-test-helper'; describe('Graph Utilities', () => { const graph = { id: 'root', @@ -127,6 +136,20 @@ describe('Graph Utilities', () => { }); }); + describe('isOperationNameValid', () => { + it('should return true as there is no node with that name', () => { + expect(isOperationNameValid('Compose', 'Compose_12', 'Compose_', false, nodesMetadata, {}, getTestIntl()).isValid).toBeTruthy(); + }); + + it('should return true as there is no other node with that name, even if the selected node previous and new name are equal', () => { + expect(isOperationNameValid('Compose', 'Compose_13', 'Compose_13', false, nodesMetadata, {}, getTestIntl()).isValid).toBeTruthy(); + }); + + it('should return false as there is already a node with that name', () => { + expect(isOperationNameValid('Compose', 'Compose_5', 'Compose_', false, nodesMetadata, {}, getTestIntl()).isValid).toBeFalsy(); + }); + }); + describe('getAllNodesInsideNode', () => { it('should return all children for a workflow graph node which are operations', () => { expect(getAllNodesInsideNode('Scope_2', graph, operationMap)).toEqual(['Compose_7', 'Compose_8', 'Compose_9']); diff --git a/libs/designer/src/lib/core/utils/graph.ts b/libs/designer/src/lib/core/utils/graph.ts index cd727ec7b3a..f7ddafd19eb 100644 --- a/libs/designer/src/lib/core/utils/graph.ts +++ b/libs/designer/src/lib/core/utils/graph.ts @@ -210,12 +210,14 @@ export const getFirstParentOfType = ( export const isOperationNameValid = ( nodeId: string, newName: string, + oldName: string, isTrigger: boolean, nodesMetadata: NodesMetadata, idReplacements: Record, intl: IntlShape ): { isValid: boolean; message: string } => { const name = transformOperationTitle(newName); + const previousName = transformOperationTitle(oldName); const subgraphType = getRecordEntry(nodesMetadata, nodeId)?.subgraphType; const messages = { @@ -248,12 +250,17 @@ export const isOperationNameValid = ( } } - // Check for name uniqueness. - const existingNames = Object.keys(nodesMetadata).map((id) => getRecordEntry(idReplacements, id) ?? id); - const isDuplicateName = existingNames.some((nodeName) => equals(nodeName, name)); - if (isDuplicateName) { - return { isValid: false, message: messages.DEFAULT }; + // Check for name uniqueness only when previous and new names of the selected node are not equal + const previousAndNewNamesNotSame = !equals(previousName, name); + + if (previousAndNewNamesNotSame) { + const existingNames = Object.keys(nodesMetadata).map((id) => getRecordEntry(idReplacements, id) ?? id); + const isDuplicateName = existingNames.some((nodeName) => equals(nodeName, name)); + if (isDuplicateName) { + return { isValid: false, message: messages.DEFAULT }; + } } + return { isValid: true, message: '' }; }; diff --git a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx index 00038304d2a..834f06840b3 100644 --- a/libs/designer/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx +++ b/libs/designer/src/lib/ui/panel/nodeDetailsPanel/nodeDetailsPanel.tsx @@ -153,7 +153,15 @@ export const NodeDetailsPanel = (props: CommonPanelProps): JSX.Element => { ); const onTitleChange = (originalId: string, newId: string): { valid: boolean; oldValue?: string; message: string } => { - const validation = isOperationNameValid(originalId, newId, isTrigger, nodesMetadata, idReplacements, intl); + const validation = isOperationNameValid( + originalId, + newId, + selectedNodeData!.displayName, + isTrigger, + nodesMetadata, + idReplacements, + intl + ); return { valid: validation.isValid, oldValue: validation.isValid ? newId : originalId, message: validation.message }; }; @@ -283,5 +291,5 @@ export const NodeDetailsPanel = (props: CommonPanelProps): JSX.Element => { }; // TODO: 12798935 Analytics (event logging) -// eslint-disable-next-line @typescript-eslint/no-empty-function + const handleTrackEvent = (_data: PageActionTelemetryData): void => {};