From 21e014d0765e1f65f61a85e84dadb16343177445 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Tue, 17 Sep 2024 10:24:01 +0200 Subject: [PATCH 01/11] better separate uniquePointer and SchemaPointer --- .../packages/schema-model/src/lib/SchemaModel.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 99577ee6ef8..399057dfd75 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -85,13 +85,14 @@ export class SchemaModel { } public getSchemaPointerByUniquePointer(uniquePointer: string): string { - if (this.hasNode(uniquePointer)) return uniquePointer; + const pointer = uniquePointer.replace('uniquePointer-', ''); + if (this.hasNode(pointer)) return pointer; - const parentNodePointer = this.getParentSchemaPointerByUniquePointer(uniquePointer); + const parentNodePointer = this.getParentSchemaPointerByUniquePointer(pointer); return makePointerFromArray([ parentNodePointer, Keyword.Properties, - extractNameFromPointer(uniquePointer), + extractNameFromPointer(pointer), ]); } @@ -103,14 +104,15 @@ export class SchemaModel { } private getParentPropertyNodeByUniquePointer(uniquePointer: string): UiSchemaNode { - const parentUniquePointer = uniquePointer.split('/').slice(0, -2).join('/'); + const parentUniquePointer = uniquePointer.split('/').slice(1, -3).join('/'); return this.getNodeByUniquePointer(parentUniquePointer); } public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string { - if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer)) return schemaPointer; + if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer)) + return `uniquePointer-${schemaPointer}`; - return `${uniqueParentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; + return `uniquePointer-${uniqueParentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; } public hasNode(schemaPointer: string): boolean { From 63c6206b6a01c4023b78946d7e5c7207e1a13d67 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Tue, 17 Sep 2024 14:23:27 +0200 Subject: [PATCH 02/11] keep focus on selected node that is dragged and moved in schemaTree --- .../SchemaEditor/hooks/useMoveProperty.ts | 12 +++++------- .../packages/schema-model/src/lib/SchemaModel.ts | 16 +++++++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts index 83c62a086e9..d3e8706699f 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts @@ -11,9 +11,6 @@ export const useMoveProperty = (): HandleMove => { const savableModel = useSavableSchemaModel(); const { selectedUniquePointer, setSelectedUniquePointer } = useSchemaEditorAppContext(); const { t } = useTranslation(); - const selectedSchemaPointer = selectedUniquePointer - ? savableModel.getSchemaPointerByUniquePointer(selectedUniquePointer) - : null; const areThereCollidingNames = useCallback( (schemaPointer: string, schemaParentPointer: string): boolean => { const currentParent = savableModel.getParentNode(schemaPointer); @@ -39,12 +36,13 @@ export const useMoveProperty = (): HandleMove => { const parent = extractNameFromPointer(schemaParentPointer); alert(t('schema_editor.move_node_same_name_error', { name, parent })); } else { - const movedNode = savableModel.moveNode(schemaPointer, target); - if (selectedSchemaPointer === schemaPointer) { - setSelectedUniquePointer(movedNode.schemaPointer); + if (selectedUniquePointer === uniquePointer) { + const movedNode = savableModel.moveNode(schemaPointer, target); + const movedUniquePointer = savableModel.getUniquePointer(movedNode.schemaPointer); + setSelectedUniquePointer(movedUniquePointer); } } }, - [savableModel, t, areThereCollidingNames, selectedSchemaPointer, setSelectedUniquePointer], + [savableModel, t, areThereCollidingNames, selectedUniquePointer, setSelectedUniquePointer], ); }; diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 399057dfd75..35118ee3bb2 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -85,12 +85,12 @@ export class SchemaModel { } public getSchemaPointerByUniquePointer(uniquePointer: string): string { - const pointer = uniquePointer.replace('uniquePointer-', ''); + const pointer = this.removeUniquePointerPrefix(uniquePointer); if (this.hasNode(pointer)) return pointer; - const parentNodePointer = this.getParentSchemaPointerByUniquePointer(pointer); + const parentSchemaPointer = this.getParentSchemaPointerByUniquePointer(pointer); return makePointerFromArray([ - parentNodePointer, + parentSchemaPointer, Keyword.Properties, extractNameFromPointer(pointer), ]); @@ -104,15 +104,20 @@ export class SchemaModel { } private getParentPropertyNodeByUniquePointer(uniquePointer: string): UiSchemaNode { - const parentUniquePointer = uniquePointer.split('/').slice(1, -3).join('/'); + const parentUniquePointer = uniquePointer.split('/').slice(0, -2).join('/'); return this.getNodeByUniquePointer(parentUniquePointer); } + private removeUniquePointerPrefix(uniquePointer: string): string { + return uniquePointer.replace('uniquePointer-', ''); + } + public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string { if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer)) return `uniquePointer-${schemaPointer}`; - return `uniquePointer-${uniqueParentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; + const parentPointer = this.removeUniquePointerPrefix(uniqueParentPointer); + return `uniquePointer-${parentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; } public hasNode(schemaPointer: string): boolean { @@ -290,6 +295,7 @@ export class SchemaModel { public moveNode(schemaPointer: string, target: NodePosition): UiSchemaNode { const currentParentPointer = this.getParentNode(schemaPointer).schemaPointer; const finalParent = this.getFinalNode(target.parentPointer); + console.log('finalParent', finalParent); const movedNode = isCombination(finalParent) ? this.moveNodeToCombination(schemaPointer, finalParent, target.index) : this.moveNodeToObject(schemaPointer, finalParent, target.index); From 7751a5e91996380b5bf73e14d38e0e403d010a5c Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Tue, 17 Sep 2024 14:53:54 +0200 Subject: [PATCH 03/11] fix a bug (also in main) - if adding a reference that doesn't exist, error will occur in log --- .../SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx b/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx index eb05966ba98..820d503c082 100644 --- a/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx +++ b/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx @@ -31,8 +31,10 @@ export const AddPropertyMenu = ({ schemaPointer, uniquePointer }: AddPropertyMen const addPropertyAndClose = (kind: ObjectKind, fieldType?: FieldType) => { const childPointer = addProperty(kind, fieldType, schemaPointer); - setSelectedUniquePointer(savableModel.getUniquePointer(childPointer, uniquePointer)); - closeDropdown(); + if (childPointer) { + setSelectedUniquePointer(savableModel.getUniquePointer(childPointer, uniquePointer)); + closeDropdown(); + } }; const closeDropdown = () => setIsAddDropdownOpen(false); From 5286eeabaaa6ff7fec451574da53778b0654b510 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Tue, 17 Sep 2024 16:17:10 +0200 Subject: [PATCH 04/11] redo change in useMoveProperty - doesn't move if not selected --- .../src/components/SchemaEditor/hooks/useMoveProperty.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts index d3e8706699f..f9217140b47 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts @@ -36,8 +36,8 @@ export const useMoveProperty = (): HandleMove => { const parent = extractNameFromPointer(schemaParentPointer); alert(t('schema_editor.move_node_same_name_error', { name, parent })); } else { + const movedNode = savableModel.moveNode(schemaPointer, target); if (selectedUniquePointer === uniquePointer) { - const movedNode = savableModel.moveNode(schemaPointer, target); const movedUniquePointer = savableModel.getUniquePointer(movedNode.schemaPointer); setSelectedUniquePointer(movedUniquePointer); } From 9b968cd0104daae5d4809374b1954531554d1021 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Wed, 18 Sep 2024 09:14:13 +0200 Subject: [PATCH 05/11] cleanup - set uniquePointer instead of schemaPointer --- .../src/components/NodePanel/HeadingRow/HeadingRow.tsx | 7 +++++-- .../src/components/SchemaEditor/hooks/useAddReference.ts | 7 ++++--- .../SchemaNode/ActionButtons/AddPropertyMenu.tsx | 3 ++- .../src/components/TypesInspector/TypesInspector.tsx | 3 ++- frontend/packages/schema-model/src/lib/SchemaModel.ts | 1 - 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.tsx b/frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.tsx index c24b5736b06..1cdfe5ba660 100644 --- a/frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.tsx +++ b/frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.tsx @@ -89,12 +89,15 @@ const AddNodeMenu = ({ schemaPointer }: AddNodeMenuProps) => { }; const useAddNodeMenuItems = (schemaPointer: string): AddNodeMenuItemProps[] => { - const { setSelectedUniquePointer } = useSchemaEditorAppContext(); + const { setSelectedUniquePointer, schemaModel } = useSchemaEditorAppContext(); const addNode = useAddProperty(); const addAndSelectNode = (...params: Parameters) => { const newPointer = addNode(...params); - if (newPointer) setSelectedUniquePointer(newPointer); + if (newPointer) { + const newUniquePointer = schemaModel.getUniquePointer(newPointer); + setSelectedUniquePointer(newUniquePointer); + } }; return [ diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useAddReference.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useAddReference.ts index 80e7dc719ba..14d8d6817a5 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useAddReference.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useAddReference.ts @@ -6,7 +6,7 @@ import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel'; import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext'; export const useAddReference = (): HandleAdd => { - const { setSelectedUniquePointer } = useSchemaEditorAppContext(); + const { setSelectedUniquePointer, schemaModel } = useSchemaEditorAppContext(); const savableModel = useSavableSchemaModel(); return useCallback( (reference: string, position: ItemPosition) => { @@ -15,8 +15,9 @@ export const useAddReference = (): HandleAdd => { const schemaPointer = savableModel.getFinalNode(target.parentPointer).schemaPointer; const refName = savableModel.generateUniqueChildName(schemaPointer, 'ref'); const ref = savableModel.addReference(refName, reference, target); - setSelectedUniquePointer(ref.schemaPointer); + const uniquePointer = schemaModel.getUniquePointer(ref.schemaPointer); + setSelectedUniquePointer(uniquePointer); }, - [savableModel, setSelectedUniquePointer], + [savableModel, setSelectedUniquePointer, schemaModel], ); }; diff --git a/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx b/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx index 820d503c082..ea829d602dc 100644 --- a/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx +++ b/frontend/packages/schema-editor/src/components/SchemaTree/SchemaNode/ActionButtons/AddPropertyMenu.tsx @@ -32,7 +32,8 @@ export const AddPropertyMenu = ({ schemaPointer, uniquePointer }: AddPropertyMen const addPropertyAndClose = (kind: ObjectKind, fieldType?: FieldType) => { const childPointer = addProperty(kind, fieldType, schemaPointer); if (childPointer) { - setSelectedUniquePointer(savableModel.getUniquePointer(childPointer, uniquePointer)); + const uniqueChildPointer = savableModel.getUniquePointer(childPointer, uniquePointer); + setSelectedUniquePointer(uniqueChildPointer); closeDropdown(); } }; diff --git a/frontend/packages/schema-editor/src/components/TypesInspector/TypesInspector.tsx b/frontend/packages/schema-editor/src/components/TypesInspector/TypesInspector.tsx index aa9d34f0414..cd653d53d5d 100644 --- a/frontend/packages/schema-editor/src/components/TypesInspector/TypesInspector.tsx +++ b/frontend/packages/schema-editor/src/components/TypesInspector/TypesInspector.tsx @@ -25,7 +25,8 @@ export const TypesInspector = ({ schemaItems }: TypesInspectorProps) => { const setSelectedType = (schemaPointer: string) => { setSelectedTypePointer(schemaPointer); - setSelectedUniquePointer(schemaPointer); + const uniquePointer = schemaModel.getUniquePointer(schemaPointer); + setSelectedUniquePointer(uniquePointer); }; const handleAddDefinition = (e: MouseEvent) => { diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 35118ee3bb2..93246c3cce2 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -295,7 +295,6 @@ export class SchemaModel { public moveNode(schemaPointer: string, target: NodePosition): UiSchemaNode { const currentParentPointer = this.getParentNode(schemaPointer).schemaPointer; const finalParent = this.getFinalNode(target.parentPointer); - console.log('finalParent', finalParent); const movedNode = isCombination(finalParent) ? this.moveNodeToCombination(schemaPointer, finalParent, target.index) : this.moveNodeToObject(schemaPointer, finalParent, target.index); From 336d26c2be252d7881586a4e04b21e10cf79f835 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Wed, 18 Sep 2024 09:15:27 +0200 Subject: [PATCH 06/11] update tests --- .../hooks/useMoveProperty.test.ts | 32 +++++----- .../schema-model/src/lib/SchemaModel.test.ts | 62 ++++++++++--------- 2 files changed, 50 insertions(+), 44 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts index d743a8e2b2b..28cb05306f3 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts @@ -154,37 +154,41 @@ describe('useMoveProperty', () => { expect(save).toHaveBeenCalledTimes(0); }); - it('Updates the selected node pointer if moving a node that is selected into an object', () => { - const setSelectedNodePointerMock = jest.fn(); + it('Updates the selected unique node pointer if moving a node that is selected into an object', () => { + const setSelectedUniquePointerMock = jest.fn(); + const uniquePointerPrefix = 'uniquePointer'; const { move, save } = setup({ - selectedUniquePointer: fieldNode1Mock.schemaPointer, - setSelectedUniquePointer: setSelectedNodePointerMock, + selectedUniquePointer: `${uniquePointerPrefix}-${fieldNode1Mock.schemaPointer}`, + setSelectedUniquePointer: setSelectedUniquePointerMock, }); - const pointerOfNodeToMove = fieldNode1Mock.schemaPointer; + const pointerOfNodeToMove = `${uniquePointerPrefix}-${fieldNode1Mock.schemaPointer}`; const index = rootNodeMock.children.length; const target: ItemPosition = { parentId: ROOT_POINTER, index }; move(pointerOfNodeToMove, target); - expect(setSelectedNodePointerMock).toHaveBeenCalledTimes(1); + expect(setSelectedUniquePointerMock).toHaveBeenCalledTimes(1); const savedModel: SavableSchemaModel = save.mock.lastCall[0]; const rootChildren = savedModel.getRootChildren(); const addedRootChild = rootChildren[index]; - expect(setSelectedNodePointerMock).toHaveBeenCalledWith(addedRootChild.schemaPointer); + expect(setSelectedUniquePointerMock).toHaveBeenCalledWith( + `${uniquePointerPrefix}-${addedRootChild.schemaPointer}`, + ); }); - it('Updates the selected node pointer if moving a node that is selected into a combination node', () => { - const setSelectedNodePointerMock = jest.fn(); + it('Updates the selected unique node pointer if moving a node that is selected into a combination node', () => { + const setSelectedUniquePointerMock = jest.fn(); + const uniquePointerPrefix = 'uniquePointer'; const { move } = setup({ selectedUniquePointer: toggableNodeMock.schemaPointer, - setSelectedUniquePointer: setSelectedNodePointerMock, + setSelectedUniquePointer: setSelectedUniquePointerMock, }); const pointerOfNodeToMove = toggableNodeMock.schemaPointer; - const pointerOfNewParent = combinationNodeMock.schemaPointer; + const pointerOfNewParent = `${uniquePointerPrefix}-${combinationNodeMock.schemaPointer}`; const indexInNewParent = 0; const target: ItemPosition = { parentId: pointerOfNewParent, index: indexInNewParent }; move(pointerOfNodeToMove, target); - expect(setSelectedNodePointerMock).toHaveBeenCalledTimes(1); - expect(setSelectedNodePointerMock).toHaveBeenCalledWith( - `${combinationNodeMock.schemaPointer}/anyOf/${indexInNewParent}`, + expect(setSelectedUniquePointerMock).toHaveBeenCalledTimes(1); + expect(setSelectedUniquePointerMock).toHaveBeenCalledWith( + `${pointerOfNewParent}/anyOf/${indexInNewParent}`, ); }); }); diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts index e6eb4c4bcd8..2c6f8fce9a3 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts @@ -113,27 +113,26 @@ describe('SchemaModel', () => { }); describe('getNodeByUniquePointer', () => { - it('Returns the node when pointer it is the root pointer', () => { - expect(schemaModel.getNodeByUniquePointer(rootNodeMock.schemaPointer)).toEqual(rootNodeMock); + const uniquePointerPrefix = 'uniquePointer'; + it('Returns the node when unique pointer is the root', () => { + const uniqueRootPointer = `${uniquePointerPrefix}-${rootNodeMock.schemaPointer}`; + expect(schemaModel.getNodeByUniquePointer(uniqueRootPointer)).toEqual(rootNodeMock); }); - it('Returns the node when pointer is a property node', () => { - expect(schemaModel.getNodeByUniquePointer(parentNodeMock.schemaPointer)).toEqual( - parentNodeMock, - ); - expect(schemaModel.getNodeByUniquePointer(defNodeMock.schemaPointer)).toEqual(defNodeMock); - expect(schemaModel.getNodeByUniquePointer(allOfNodeMock.schemaPointer)).toEqual( - allOfNodeMock, - ); - expect(schemaModel.getNodeByUniquePointer(stringNodeMock.schemaPointer)).toEqual( - stringNodeMock, - ); + it('Returns the node when unique pointer is a property node', () => { + const uniqueParentPointer = `${uniquePointerPrefix}-${parentNodeMock.schemaPointer}`; + expect(schemaModel.getNodeByUniquePointer(uniqueParentPointer)).toEqual(parentNodeMock); + const uniqueDefPointer = `${uniquePointerPrefix}-${defNodeMock.schemaPointer}`; + expect(schemaModel.getNodeByUniquePointer(uniqueDefPointer)).toEqual(defNodeMock); + const uniqueAllOfPointer = `${uniquePointerPrefix}-${allOfNodeMock.schemaPointer}`; + expect(schemaModel.getNodeByUniquePointer(uniqueAllOfPointer)).toEqual(allOfNodeMock); + const uniqueStringPointer = `${uniquePointerPrefix}-${stringNodeMock.schemaPointer}`; + expect(schemaModel.getNodeByUniquePointer(uniqueStringPointer)).toEqual(stringNodeMock); }); it('Returns the node reflecting the path to a given unique pointer in a reference', () => { - const uniqueChildPointer = '#/properties/referenceToParent/properties/child'; - const uniqueGrandchildPointer = - '#/properties/referenceToParent/properties/child/properties/grandchild'; + const uniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; + const uniqueGrandchildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; expect(schemaModel.getNodeByUniquePointer(uniqueChildPointer)).toEqual( defNodeWithChildrenChildMock, @@ -145,9 +144,9 @@ describe('SchemaModel', () => { }); describe('getSchemaPointerByUniquePointer', () => { - const uniqueGrandChildPointer = - '#/properties/referenceToParent/properties/child/properties/grandchild'; - const uniqueChildPointer = '#/properties/referenceToParent/properties/child'; + const uniquePointerPrefix = 'uniquePointer'; + const uniqueGrandChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; + const uniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; it('Returns the schema pointer for a given unique pointer', () => { expect(schemaModel.getSchemaPointerByUniquePointer(uniqueChildPointer)).toEqual( @@ -160,35 +159,38 @@ describe('SchemaModel', () => { }); describe('getUniquePointer', () => { - it('Returns the pointer as is when called on the root node', () => { + const uniquePointerPrefix = 'uniquePointer'; + it('Returns the unique pointer when called on the root node', () => { + const expectedUniqueRootPointer = `${uniquePointerPrefix}-#`; expect(schemaModel.getUniquePointer(rootNodeMock.schemaPointer)).toEqual( - rootNodeMock.schemaPointer, + expectedUniqueRootPointer, ); }); - it('Returns the pointer as is when called on a property node', () => { + it('Returns the unique pointer when called on a property node', () => { + const expectedUniquePointer = `${uniquePointerPrefix}-${referenceToObjectNodeMock.schemaPointer}`; + expect(schemaModel.getUniquePointer(referenceToObjectNodeMock.schemaPointer)).toEqual( - referenceToObjectNodeMock.schemaPointer, + expectedUniquePointer, ); }); - it('Returns a pointer reflecting the path to a given node in a reference', () => { - const expectedChildPointer = '#/properties/referenceToParent/properties/child'; - const expectedGrandchildPointer = - '#/properties/referenceToParent/properties/child/properties/grandchild'; + it('Returns a unique pointer reflecting the path to a given node in a reference', () => { + const expectedUniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; + const expectedUniqueGrandchildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; expect( schemaModel.getUniquePointer( defNodeWithChildrenChildMock.schemaPointer, referenceToObjectNodeMock.schemaPointer, ), - ).toEqual(expectedChildPointer); + ).toEqual(expectedUniqueChildPointer); expect( schemaModel.getUniquePointer( defNodeWithChildrenGrandchildMock.schemaPointer, - expectedChildPointer, + expectedUniqueChildPointer, ), - ).toEqual(expectedGrandchildPointer); + ).toEqual(expectedUniqueGrandchildPointer); }); }); From c728881d0c673bbe36d5756b1a3740a35c06ec1f Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Fri, 20 Sep 2024 12:46:20 +0200 Subject: [PATCH 07/11] add a constant of unique pointer prefix --- .../hooks/useMoveProperty.test.ts | 11 +++---- frontend/packages/schema-model/src/index.ts | 2 +- .../schema-model/src/lib/SchemaModel.test.ts | 31 +++++++++---------- .../schema-model/src/lib/SchemaModel.ts | 8 ++--- .../schema-model/src/lib/constants.ts | 1 + 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts index 28cb05306f3..722e46d9400 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts @@ -7,6 +7,7 @@ import { ROOT_POINTER, SchemaModel, validateTestUiSchema, + UNIQUE_POINTER_PREFIX, } from '@altinn/schema-model'; import { combinationNodeMock, @@ -156,12 +157,11 @@ describe('useMoveProperty', () => { it('Updates the selected unique node pointer if moving a node that is selected into an object', () => { const setSelectedUniquePointerMock = jest.fn(); - const uniquePointerPrefix = 'uniquePointer'; const { move, save } = setup({ - selectedUniquePointer: `${uniquePointerPrefix}-${fieldNode1Mock.schemaPointer}`, + selectedUniquePointer: `${UNIQUE_POINTER_PREFIX}-${fieldNode1Mock.schemaPointer}`, setSelectedUniquePointer: setSelectedUniquePointerMock, }); - const pointerOfNodeToMove = `${uniquePointerPrefix}-${fieldNode1Mock.schemaPointer}`; + const pointerOfNodeToMove = `${UNIQUE_POINTER_PREFIX}-${fieldNode1Mock.schemaPointer}`; const index = rootNodeMock.children.length; const target: ItemPosition = { parentId: ROOT_POINTER, index }; move(pointerOfNodeToMove, target); @@ -170,19 +170,18 @@ describe('useMoveProperty', () => { const rootChildren = savedModel.getRootChildren(); const addedRootChild = rootChildren[index]; expect(setSelectedUniquePointerMock).toHaveBeenCalledWith( - `${uniquePointerPrefix}-${addedRootChild.schemaPointer}`, + `${UNIQUE_POINTER_PREFIX}-${addedRootChild.schemaPointer}`, ); }); it('Updates the selected unique node pointer if moving a node that is selected into a combination node', () => { const setSelectedUniquePointerMock = jest.fn(); - const uniquePointerPrefix = 'uniquePointer'; const { move } = setup({ selectedUniquePointer: toggableNodeMock.schemaPointer, setSelectedUniquePointer: setSelectedUniquePointerMock, }); const pointerOfNodeToMove = toggableNodeMock.schemaPointer; - const pointerOfNewParent = `${uniquePointerPrefix}-${combinationNodeMock.schemaPointer}`; + const pointerOfNewParent = `${UNIQUE_POINTER_PREFIX}-${combinationNodeMock.schemaPointer}`; const indexInNewParent = 0; const target: ItemPosition = { parentId: pointerOfNewParent, index: indexInNewParent }; move(pointerOfNodeToMove, target); diff --git a/frontend/packages/schema-model/src/index.ts b/frontend/packages/schema-model/src/index.ts index bb51af0a0f4..67b3c24154e 100644 --- a/frontend/packages/schema-model/src/index.ts +++ b/frontend/packages/schema-model/src/index.ts @@ -1,6 +1,6 @@ export { buildJsonSchema } from './lib/build-json-schema'; export { buildUiSchema } from './lib/build-ui-schema'; -export { ROOT_POINTER } from './lib/constants'; +export { ROOT_POINTER, UNIQUE_POINTER_PREFIX } from './lib/constants'; export type { UiSchemaNode, UiSchemaNodes } from './types'; export type { FieldNode } from './types/FieldNode'; export type { CombinationNode } from './types/CombinationNode'; diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts index 2c6f8fce9a3..7cebe2d32db 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts @@ -37,7 +37,7 @@ import type { FieldNode } from '../types/FieldNode'; import type { ReferenceNode } from '../types/ReferenceNode'; import { extractNameFromPointer } from './pointerUtils'; import { isArray, isDefinition } from './utils'; -import { ROOT_POINTER } from './constants'; +import { ROOT_POINTER, UNIQUE_POINTER_PREFIX } from './constants'; import type { CombinationNode } from '../types/CombinationNode'; import { ArrayUtils } from '@studio/pure-functions'; @@ -113,26 +113,25 @@ describe('SchemaModel', () => { }); describe('getNodeByUniquePointer', () => { - const uniquePointerPrefix = 'uniquePointer'; it('Returns the node when unique pointer is the root', () => { - const uniqueRootPointer = `${uniquePointerPrefix}-${rootNodeMock.schemaPointer}`; + const uniqueRootPointer = `${UNIQUE_POINTER_PREFIX}-${rootNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueRootPointer)).toEqual(rootNodeMock); }); it('Returns the node when unique pointer is a property node', () => { - const uniqueParentPointer = `${uniquePointerPrefix}-${parentNodeMock.schemaPointer}`; + const uniqueParentPointer = `${UNIQUE_POINTER_PREFIX}-${parentNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueParentPointer)).toEqual(parentNodeMock); - const uniqueDefPointer = `${uniquePointerPrefix}-${defNodeMock.schemaPointer}`; + const uniqueDefPointer = `${UNIQUE_POINTER_PREFIX}-${defNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueDefPointer)).toEqual(defNodeMock); - const uniqueAllOfPointer = `${uniquePointerPrefix}-${allOfNodeMock.schemaPointer}`; + const uniqueAllOfPointer = `${UNIQUE_POINTER_PREFIX}-${allOfNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueAllOfPointer)).toEqual(allOfNodeMock); - const uniqueStringPointer = `${uniquePointerPrefix}-${stringNodeMock.schemaPointer}`; + const uniqueStringPointer = `${UNIQUE_POINTER_PREFIX}-${stringNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueStringPointer)).toEqual(stringNodeMock); }); it('Returns the node reflecting the path to a given unique pointer in a reference', () => { - const uniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; - const uniqueGrandchildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; + const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; + const uniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; expect(schemaModel.getNodeByUniquePointer(uniqueChildPointer)).toEqual( defNodeWithChildrenChildMock, @@ -144,9 +143,8 @@ describe('SchemaModel', () => { }); describe('getSchemaPointerByUniquePointer', () => { - const uniquePointerPrefix = 'uniquePointer'; - const uniqueGrandChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; - const uniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; + const uniqueGrandChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; + const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; it('Returns the schema pointer for a given unique pointer', () => { expect(schemaModel.getSchemaPointerByUniquePointer(uniqueChildPointer)).toEqual( @@ -159,16 +157,15 @@ describe('SchemaModel', () => { }); describe('getUniquePointer', () => { - const uniquePointerPrefix = 'uniquePointer'; it('Returns the unique pointer when called on the root node', () => { - const expectedUniqueRootPointer = `${uniquePointerPrefix}-#`; + const expectedUniqueRootPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}`; expect(schemaModel.getUniquePointer(rootNodeMock.schemaPointer)).toEqual( expectedUniqueRootPointer, ); }); it('Returns the unique pointer when called on a property node', () => { - const expectedUniquePointer = `${uniquePointerPrefix}-${referenceToObjectNodeMock.schemaPointer}`; + const expectedUniquePointer = `${UNIQUE_POINTER_PREFIX}-${referenceToObjectNodeMock.schemaPointer}`; expect(schemaModel.getUniquePointer(referenceToObjectNodeMock.schemaPointer)).toEqual( expectedUniquePointer, @@ -176,8 +173,8 @@ describe('SchemaModel', () => { }); it('Returns a unique pointer reflecting the path to a given node in a reference', () => { - const expectedUniqueChildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child`; - const expectedUniqueGrandchildPointer = `${uniquePointerPrefix}-#/properties/referenceToParent/properties/child/properties/grandchild`; + const expectedUniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; + const expectedUniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; expect( schemaModel.getUniquePointer( diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 93246c3cce2..5852f6c5a6e 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -24,7 +24,7 @@ import { moveArrayItem, replaceItemsByValue, } from 'app-shared/utils/arrayUtils'; -import { ROOT_POINTER } from './constants'; +import { ROOT_POINTER, UNIQUE_POINTER_PREFIX } from './constants'; import type { ReferenceNode } from '../types/ReferenceNode'; import { ObjectUtils, ArrayUtils } from '@studio/pure-functions'; import { replaceStart } from 'app-shared/utils/stringUtils'; @@ -109,15 +109,15 @@ export class SchemaModel { } private removeUniquePointerPrefix(uniquePointer: string): string { - return uniquePointer.replace('uniquePointer-', ''); + return uniquePointer.replace(UNIQUE_POINTER_PREFIX, ''); } public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string { if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer)) - return `uniquePointer-${schemaPointer}`; + return `${UNIQUE_POINTER_PREFIX}${schemaPointer}`; const parentPointer = this.removeUniquePointerPrefix(uniqueParentPointer); - return `uniquePointer-${parentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; + return `${UNIQUE_POINTER_PREFIX}${parentPointer}/properties/${extractNameFromPointer(schemaPointer)}`; } public hasNode(schemaPointer: string): boolean { diff --git a/frontend/packages/schema-model/src/lib/constants.ts b/frontend/packages/schema-model/src/lib/constants.ts index 825c373d711..ace09930068 100644 --- a/frontend/packages/schema-model/src/lib/constants.ts +++ b/frontend/packages/schema-model/src/lib/constants.ts @@ -1,2 +1,3 @@ export const META_SCHEMA_ID = 'https://json-schema.org/draft/2020-12/schema'; export const ROOT_POINTER = '#'; +export const UNIQUE_POINTER_PREFIX = 'uniquePointer-'; From 758d3bb3530b68d285cc1f70d840313688c4c06e Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Fri, 20 Sep 2024 12:58:10 +0200 Subject: [PATCH 08/11] fix tests --- .../hooks/useMoveProperty.test.ts | 8 +++--- .../schema-model/src/lib/SchemaModel.test.ts | 26 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts index 722e46d9400..57a33194ed7 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts @@ -158,10 +158,10 @@ describe('useMoveProperty', () => { it('Updates the selected unique node pointer if moving a node that is selected into an object', () => { const setSelectedUniquePointerMock = jest.fn(); const { move, save } = setup({ - selectedUniquePointer: `${UNIQUE_POINTER_PREFIX}-${fieldNode1Mock.schemaPointer}`, + selectedUniquePointer: `${UNIQUE_POINTER_PREFIX}${fieldNode1Mock.schemaPointer}`, setSelectedUniquePointer: setSelectedUniquePointerMock, }); - const pointerOfNodeToMove = `${UNIQUE_POINTER_PREFIX}-${fieldNode1Mock.schemaPointer}`; + const pointerOfNodeToMove = `${UNIQUE_POINTER_PREFIX}${fieldNode1Mock.schemaPointer}`; const index = rootNodeMock.children.length; const target: ItemPosition = { parentId: ROOT_POINTER, index }; move(pointerOfNodeToMove, target); @@ -170,7 +170,7 @@ describe('useMoveProperty', () => { const rootChildren = savedModel.getRootChildren(); const addedRootChild = rootChildren[index]; expect(setSelectedUniquePointerMock).toHaveBeenCalledWith( - `${UNIQUE_POINTER_PREFIX}-${addedRootChild.schemaPointer}`, + `${UNIQUE_POINTER_PREFIX}${addedRootChild.schemaPointer}`, ); }); @@ -181,7 +181,7 @@ describe('useMoveProperty', () => { setSelectedUniquePointer: setSelectedUniquePointerMock, }); const pointerOfNodeToMove = toggableNodeMock.schemaPointer; - const pointerOfNewParent = `${UNIQUE_POINTER_PREFIX}-${combinationNodeMock.schemaPointer}`; + const pointerOfNewParent = `${UNIQUE_POINTER_PREFIX}${combinationNodeMock.schemaPointer}`; const indexInNewParent = 0; const target: ItemPosition = { parentId: pointerOfNewParent, index: indexInNewParent }; move(pointerOfNodeToMove, target); diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts index 7cebe2d32db..59436ce7198 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts @@ -114,24 +114,24 @@ describe('SchemaModel', () => { describe('getNodeByUniquePointer', () => { it('Returns the node when unique pointer is the root', () => { - const uniqueRootPointer = `${UNIQUE_POINTER_PREFIX}-${rootNodeMock.schemaPointer}`; + const uniqueRootPointer = `${UNIQUE_POINTER_PREFIX}${rootNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueRootPointer)).toEqual(rootNodeMock); }); it('Returns the node when unique pointer is a property node', () => { - const uniqueParentPointer = `${UNIQUE_POINTER_PREFIX}-${parentNodeMock.schemaPointer}`; + const uniqueParentPointer = `${UNIQUE_POINTER_PREFIX}${parentNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueParentPointer)).toEqual(parentNodeMock); - const uniqueDefPointer = `${UNIQUE_POINTER_PREFIX}-${defNodeMock.schemaPointer}`; + const uniqueDefPointer = `${UNIQUE_POINTER_PREFIX}${defNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueDefPointer)).toEqual(defNodeMock); - const uniqueAllOfPointer = `${UNIQUE_POINTER_PREFIX}-${allOfNodeMock.schemaPointer}`; + const uniqueAllOfPointer = `${UNIQUE_POINTER_PREFIX}${allOfNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueAllOfPointer)).toEqual(allOfNodeMock); - const uniqueStringPointer = `${UNIQUE_POINTER_PREFIX}-${stringNodeMock.schemaPointer}`; + const uniqueStringPointer = `${UNIQUE_POINTER_PREFIX}${stringNodeMock.schemaPointer}`; expect(schemaModel.getNodeByUniquePointer(uniqueStringPointer)).toEqual(stringNodeMock); }); it('Returns the node reflecting the path to a given unique pointer in a reference', () => { - const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; - const uniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; + const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child`; + const uniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; expect(schemaModel.getNodeByUniquePointer(uniqueChildPointer)).toEqual( defNodeWithChildrenChildMock, @@ -143,8 +143,8 @@ describe('SchemaModel', () => { }); describe('getSchemaPointerByUniquePointer', () => { - const uniqueGrandChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; - const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; + const uniqueGrandChildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; + const uniqueChildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child`; it('Returns the schema pointer for a given unique pointer', () => { expect(schemaModel.getSchemaPointerByUniquePointer(uniqueChildPointer)).toEqual( @@ -158,14 +158,14 @@ describe('SchemaModel', () => { describe('getUniquePointer', () => { it('Returns the unique pointer when called on the root node', () => { - const expectedUniqueRootPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}`; + const expectedUniqueRootPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}`; expect(schemaModel.getUniquePointer(rootNodeMock.schemaPointer)).toEqual( expectedUniqueRootPointer, ); }); it('Returns the unique pointer when called on a property node', () => { - const expectedUniquePointer = `${UNIQUE_POINTER_PREFIX}-${referenceToObjectNodeMock.schemaPointer}`; + const expectedUniquePointer = `${UNIQUE_POINTER_PREFIX}${referenceToObjectNodeMock.schemaPointer}`; expect(schemaModel.getUniquePointer(referenceToObjectNodeMock.schemaPointer)).toEqual( expectedUniquePointer, @@ -173,8 +173,8 @@ describe('SchemaModel', () => { }); it('Returns a unique pointer reflecting the path to a given node in a reference', () => { - const expectedUniqueChildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child`; - const expectedUniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}-${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; + const expectedUniqueChildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child`; + const expectedUniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`; expect( schemaModel.getUniquePointer( From 24955806916900e54ddf4ab0363f5a5d84285d0e Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Fri, 20 Sep 2024 13:19:36 +0200 Subject: [PATCH 09/11] utilize StringUtils --- frontend/packages/schema-model/src/lib/SchemaModel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 5852f6c5a6e..238444ff929 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -26,7 +26,7 @@ import { } from 'app-shared/utils/arrayUtils'; import { ROOT_POINTER, UNIQUE_POINTER_PREFIX } from './constants'; import type { ReferenceNode } from '../types/ReferenceNode'; -import { ObjectUtils, ArrayUtils } from '@studio/pure-functions'; +import { ObjectUtils, ArrayUtils, StringUtils } from '@studio/pure-functions'; import { replaceStart } from 'app-shared/utils/stringUtils'; import { createDefinitionPointer, @@ -109,7 +109,7 @@ export class SchemaModel { } private removeUniquePointerPrefix(uniquePointer: string): string { - return uniquePointer.replace(UNIQUE_POINTER_PREFIX, ''); + return StringUtils.removeStart(uniquePointer, UNIQUE_POINTER_PREFIX); } public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string { From 2e20dd9442283f2ff9c09d11065364c0d5fb9282 Mon Sep 17 00:00:00 2001 From: lassopicasso Date: Wed, 25 Sep 2024 15:26:30 +0200 Subject: [PATCH 10/11] fix unit tests --- frontend/packages/schema-model/src/lib/SchemaModel.test.ts | 2 +- frontend/packages/schema-model/src/lib/SchemaModel.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts index 78acbbbb4d5..566e8271a1f 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.test.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.test.ts @@ -204,7 +204,7 @@ describe('SchemaModel', () => { const { schemaPointer } = combinationDefNodeChild1Mock; const uniquePointerOfParent = referenceToCombinationDefNodeMock.schemaPointer; const result = schemaModel.getUniquePointer(schemaPointer, uniquePointerOfParent); - const expectedResult = '#/properties/referenceToCombinationDef/oneOf/0'; + const expectedResult = `${UNIQUE_POINTER_PREFIX}#/properties/referenceToCombinationDef/oneOf/0`; expect(result).toEqual(expectedResult); }); }); diff --git a/frontend/packages/schema-model/src/lib/SchemaModel.ts b/frontend/packages/schema-model/src/lib/SchemaModel.ts index 456945956e3..1255b3d132e 100644 --- a/frontend/packages/schema-model/src/lib/SchemaModel.ts +++ b/frontend/packages/schema-model/src/lib/SchemaModel.ts @@ -117,7 +117,7 @@ export class SchemaModel { return `${UNIQUE_POINTER_PREFIX}${schemaPointer}`; const category = extractCategoryFromPointer(schemaPointer); const parentPointer = this.removeUniquePointerPrefix(uniqueParentPointer); - return `${UNIQUE_POINTER_PREFIX}${parentPointer}${category}/${extractNameFromPointer(schemaPointer)}`; + return `${UNIQUE_POINTER_PREFIX}${parentPointer}/${category}/${extractNameFromPointer(schemaPointer)}`; } public hasNode(schemaPointer: string): boolean { From 37eb2fc8670c954f7539e0cf12f241f31bdde27e Mon Sep 17 00:00:00 2001 From: Tomas Date: Fri, 27 Sep 2024 08:51:56 +0200 Subject: [PATCH 11/11] Provide parent pointer to unique pointer calculation --- .../hooks/useMoveProperty.test.ts | 53 ++++++++++++++++--- .../SchemaEditor/hooks/useMoveProperty.ts | 5 +- .../schema-editor/test/mocks/uiSchemaMock.ts | 9 ++++ 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts index 57a33194ed7..4a0501a5384 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.test.ts @@ -8,13 +8,16 @@ import { SchemaModel, validateTestUiSchema, UNIQUE_POINTER_PREFIX, + Keyword, } from '@altinn/schema-model'; import { + childOfReferredNodeMock, combinationNodeMock, fieldNode1Mock, nodeWithSameNameAsObjectChildMock, objectChildMock, objectNodeMock, + referenceNodeMock, rootNodeMock, toggableNodeMock, uiSchemaNodesMock, @@ -22,6 +25,8 @@ import { import type { SavableSchemaModel } from '../../../classes/SavableSchemaModel'; import { ArrayUtils } from '@studio/pure-functions'; +const uniquePointerOfRoot = UNIQUE_POINTER_PREFIX + ROOT_POINTER; + describe('useMoveProperty', () => { const setup = (schemaEditorAppContextProps?: Partial) => { const save = jest.fn(); @@ -33,7 +38,7 @@ describe('useMoveProperty', () => { }; const { result } = renderHookWithProviders({ appContextProps })(useMoveProperty); const move: HandleMove = result.current; - return { move, save }; + return { move, save, schemaModel }; }; it('Moves a property to the given position', () => { @@ -57,7 +62,7 @@ describe('useMoveProperty', () => { const { move, save } = setup(); const pointerOfNodeToMove = fieldNode1Mock.schemaPointer; const indexInNewParent = 1; - const target: ItemPosition = { parentId: ROOT_POINTER, index: indexInNewParent }; + const target: ItemPosition = { parentId: uniquePointerOfRoot, index: indexInNewParent }; move(pointerOfNodeToMove, target); expect(save).toHaveBeenCalledTimes(1); const savedModel: SavableSchemaModel = save.mock.lastCall[0]; @@ -72,7 +77,7 @@ describe('useMoveProperty', () => { it('Moves a property to the given position when it is on the root and the target index is 0', () => { const { move, save } = setup(); const pointerOfNodeToMove = fieldNode1Mock.schemaPointer; - const target: ItemPosition = { parentId: ROOT_POINTER, index: 0 }; + const target: ItemPosition = { parentId: uniquePointerOfRoot, index: 0 }; move(pointerOfNodeToMove, target); expect(save).toHaveBeenCalledTimes(1); const savedModel: SavableSchemaModel = save.mock.lastCall[0]; @@ -88,7 +93,7 @@ describe('useMoveProperty', () => { const { move, save } = setup(); const pointerOfNodeToMove = fieldNode1Mock.schemaPointer; const index = rootNodeMock.children.length; - const target: ItemPosition = { parentId: ROOT_POINTER, index }; + const target: ItemPosition = { parentId: uniquePointerOfRoot, index }; move(pointerOfNodeToMove, target); expect(save).toHaveBeenCalledTimes(1); const savedModel: SavableSchemaModel = save.mock.lastCall[0]; @@ -104,7 +109,7 @@ describe('useMoveProperty', () => { const { move, save } = setup(); const pointerOfNodeToMove = objectNodeMock.schemaPointer; const index = -1; - const target: ItemPosition = { parentId: ROOT_POINTER, index }; + const target: ItemPosition = { parentId: uniquePointerOfRoot, index }; move(pointerOfNodeToMove, target); expect(save).toHaveBeenCalledTimes(1); const savedModel: SavableSchemaModel = save.mock.lastCall[0]; @@ -176,11 +181,11 @@ describe('useMoveProperty', () => { it('Updates the selected unique node pointer if moving a node that is selected into a combination node', () => { const setSelectedUniquePointerMock = jest.fn(); + const pointerOfNodeToMove = UNIQUE_POINTER_PREFIX + toggableNodeMock.schemaPointer; const { move } = setup({ - selectedUniquePointer: toggableNodeMock.schemaPointer, + selectedUniquePointer: pointerOfNodeToMove, setSelectedUniquePointer: setSelectedUniquePointerMock, }); - const pointerOfNodeToMove = toggableNodeMock.schemaPointer; const pointerOfNewParent = `${UNIQUE_POINTER_PREFIX}${combinationNodeMock.schemaPointer}`; const indexInNewParent = 0; const target: ItemPosition = { parentId: pointerOfNewParent, index: indexInNewParent }; @@ -190,4 +195,38 @@ describe('useMoveProperty', () => { `${pointerOfNewParent}/anyOf/${indexInNewParent}`, ); }); + + it('Updates the selected unique node pointer when moving a node that is selected out of a referenced object', () => { + const setSelectedUniquePointerMock = jest.fn(); + const schemaPointerOfNodeToMove = childOfReferredNodeMock.schemaPointer; + const nameOfNodeToMove = extractNameFromPointer(schemaPointerOfNodeToMove); + const uniquePointerOfParent = UNIQUE_POINTER_PREFIX + referenceNodeMock.schemaPointer; + const uniquePointerOfNodeToMove = `${uniquePointerOfParent}/${Keyword.Properties}/${nameOfNodeToMove}`; + const expectedFinalUniquePointer = `${uniquePointerOfRoot}/${Keyword.Properties}/${nameOfNodeToMove}`; + const { move } = setup({ + selectedUniquePointer: uniquePointerOfNodeToMove, + setSelectedUniquePointer: setSelectedUniquePointerMock, + }); + const target: ItemPosition = { parentId: uniquePointerOfRoot, index: 0 }; + move(uniquePointerOfNodeToMove, target); + expect(setSelectedUniquePointerMock).toHaveBeenCalledTimes(1); + expect(setSelectedUniquePointerMock).toHaveBeenCalledWith(expectedFinalUniquePointer); + }); + + it('Updates the selected unique node pointer when moving a node that is selected into a referenced object', () => { + const setSelectedUniquePointerMock = jest.fn(); + const schemaPointerOfNodeToMove = objectNodeMock.schemaPointer; + const nameOfNodeToMove = extractNameFromPointer(schemaPointerOfNodeToMove); + const uniquePointerOfNodeToMove = `${uniquePointerOfRoot}/${Keyword.Properties}/${nameOfNodeToMove}`; + const uniquePointerOfTargetParent = UNIQUE_POINTER_PREFIX + referenceNodeMock.schemaPointer; + const expectedFinalUniquePointer = `${uniquePointerOfTargetParent}/${Keyword.Properties}/${nameOfNodeToMove}`; + const { move } = setup({ + selectedUniquePointer: uniquePointerOfNodeToMove, + setSelectedUniquePointer: setSelectedUniquePointerMock, + }); + const target: ItemPosition = { parentId: uniquePointerOfTargetParent, index: 0 }; + move(uniquePointerOfNodeToMove, target); + expect(setSelectedUniquePointerMock).toHaveBeenCalledTimes(1); + expect(setSelectedUniquePointerMock).toHaveBeenCalledWith(expectedFinalUniquePointer); + }); }); diff --git a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts index f9217140b47..3afbd23b4ae 100644 --- a/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts +++ b/frontend/packages/schema-editor/src/components/SchemaEditor/hooks/useMoveProperty.ts @@ -38,7 +38,10 @@ export const useMoveProperty = (): HandleMove => { } else { const movedNode = savableModel.moveNode(schemaPointer, target); if (selectedUniquePointer === uniquePointer) { - const movedUniquePointer = savableModel.getUniquePointer(movedNode.schemaPointer); + const movedUniquePointer = savableModel.getUniquePointer( + movedNode.schemaPointer, + position.parentId, + ); setSelectedUniquePointer(movedUniquePointer); } } diff --git a/frontend/packages/schema-editor/test/mocks/uiSchemaMock.ts b/frontend/packages/schema-editor/test/mocks/uiSchemaMock.ts index 0fa819fc421..ace53c3a21e 100644 --- a/frontend/packages/schema-editor/test/mocks/uiSchemaMock.ts +++ b/frontend/packages/schema-editor/test/mocks/uiSchemaMock.ts @@ -19,6 +19,7 @@ const stringDefinitionNodePointer = '#/$defs/def2'; const nodeWithSameNameAsObjectChildPointer = '#/properties/someNode'; const referenceNodePointer = '#/properties/referenceNode'; const referredNodePointer = '#/$defs/referredNode'; +const childOfReferredNodePointer = '#/$defs/referredNode/properties/childOfReferredNode'; export const nodeMockBase: FieldNode = { objectKind: ObjectKind.Field, @@ -43,7 +44,14 @@ export const referenceNodeMock: ReferenceNode = { export const referredNodeMock: FieldNode = { ...nodeMockBase, + fieldType: FieldType.Object, schemaPointer: referredNodePointer, + children: [childOfReferredNodePointer], +}; + +export const childOfReferredNodeMock: FieldNode = { + ...nodeMockBase, + schemaPointer: childOfReferredNodePointer, }; export const rootNodeMock: FieldNode = { @@ -137,6 +145,7 @@ export const uiSchemaNodesMock: UiSchemaNodes = [ fieldNode2Mock, referenceNodeMock, referredNodeMock, + childOfReferredNodeMock, nodeWithCustomPropsMock, toggableNodeMock, objectNodeMock,