Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Separate schemaPointer and uniquePointer better #13538

Merged
merged 19 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
21e014d
better separate uniquePointer and SchemaPointer
lassopicasso Sep 17, 2024
63c6206
keep focus on selected node that is dragged and moved in schemaTree
lassopicasso Sep 17, 2024
7751a5e
fix a bug (also in main) - if adding a reference that doesn't exist, …
lassopicasso Sep 17, 2024
5286eea
redo change in useMoveProperty - doesn't move if not selected
lassopicasso Sep 17, 2024
9b968cd
cleanup - set uniquePointer instead of schemaPointer
lassopicasso Sep 18, 2024
336d26c
update tests
lassopicasso Sep 18, 2024
6bfa994
Merge branch 'main' into separate-unique-and-schema-pointer-more
lassopicasso Sep 18, 2024
c728881
add a constant of unique pointer prefix
lassopicasso Sep 20, 2024
758d3bb
fix tests
lassopicasso Sep 20, 2024
fc5b0f6
Merge branch 'main' into separate-unique-and-schema-pointer-more
lassopicasso Sep 20, 2024
2495580
utilize StringUtils
lassopicasso Sep 20, 2024
a9db192
Merge branch 'main' into separate-unique-and-schema-pointer-more
lassopicasso Sep 25, 2024
e601ec7
Merge branch 'main' into separate-unique-and-schema-pointer-more
lassopicasso Sep 25, 2024
2e20dd9
fix unit tests
lassopicasso Sep 25, 2024
b240351
Merge remote-tracking branch 'origin/main' into separate-unique-and-s…
TomasEng Sep 26, 2024
37eb2fc
Provide parent pointer to unique pointer calculation
TomasEng Sep 27, 2024
b2abffe
Merge branch 'main' into separate-unique-and-schema-pointer-more
TomasEng Sep 27, 2024
95b1936
Merge branch 'main' into separate-unique-and-schema-pointer-more
TomasEng Sep 27, 2024
673142a
Merge branch 'main' into separate-unique-and-schema-pointer-more
TomasEng Sep 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof addNode>) => {
const newPointer = addNode(...params);
if (newPointer) setSelectedUniquePointer(newPointer);
if (newPointer) {
const newUniquePointer = schemaModel.getUniquePointer(newPointer);
setSelectedUniquePointer(newUniquePointer);
}
};

return [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel';
import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext';

export const useAddReference = (): HandleAdd<string> => {
const { setSelectedUniquePointer } = useSchemaEditorAppContext();
const { setSelectedUniquePointer, schemaModel } = useSchemaEditorAppContext();
const savableModel = useSavableSchemaModel();
return useCallback(
(reference: string, position: ItemPosition) => {
Expand All @@ -15,8 +15,9 @@ export const useAddReference = (): HandleAdd<string> => {
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],
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
ROOT_POINTER,
SchemaModel,
validateTestUiSchema,
UNIQUE_POINTER_PREFIX,
} from '@altinn/schema-model';
import {
combinationNodeMock,
Expand Down Expand Up @@ -154,37 +155,39 @@ 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 { move, save } = setup({
selectedUniquePointer: fieldNode1Mock.schemaPointer,
setSelectedUniquePointer: setSelectedNodePointerMock,
selectedUniquePointer: `${UNIQUE_POINTER_PREFIX}${fieldNode1Mock.schemaPointer}`,
setSelectedUniquePointer: setSelectedUniquePointerMock,
});
const pointerOfNodeToMove = fieldNode1Mock.schemaPointer;
const pointerOfNodeToMove = `${UNIQUE_POINTER_PREFIX}${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(
`${UNIQUE_POINTER_PREFIX}${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 { move } = setup({
selectedUniquePointer: toggableNodeMock.schemaPointer,
setSelectedUniquePointer: setSelectedNodePointerMock,
setSelectedUniquePointer: setSelectedUniquePointerMock,
});
const pointerOfNodeToMove = toggableNodeMock.schemaPointer;
const pointerOfNewParent = combinationNodeMock.schemaPointer;
const pointerOfNewParent = `${UNIQUE_POINTER_PREFIX}${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}`,
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -40,11 +37,12 @@ export const useMoveProperty = (): HandleMove => {
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 movedUniquePointer = savableModel.getUniquePointer(movedNode.schemaPointer);
setSelectedUniquePointer(movedUniquePointer);
}
}
},
[savableModel, t, areThereCollidingNames, selectedSchemaPointer, setSelectedUniquePointer],
[savableModel, t, areThereCollidingNames, selectedUniquePointer, setSelectedUniquePointer],
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ 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) {
const uniqueChildPointer = savableModel.getUniquePointer(childPointer, uniquePointer);
setSelectedUniquePointer(uniqueChildPointer);
closeDropdown();
}
};

const closeDropdown = () => setIsAddDropdownOpen(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/packages/schema-model/src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
61 changes: 30 additions & 31 deletions frontend/packages/schema-model/src/lib/SchemaModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -113,27 +113,25 @@ describe('SchemaModel', () => {
});

describe('getNodeByUniquePointer', () => {
it('Returns the node when pointer it is the root pointer', () => {
expect(schemaModel.getNodeByUniquePointer(rootNodeMock.schemaPointer)).toEqual(rootNodeMock);
it('Returns the node when unique pointer is the root', () => {
const uniqueRootPointer = `${UNIQUE_POINTER_PREFIX}${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 = `${UNIQUE_POINTER_PREFIX}${parentNodeMock.schemaPointer}`;
expect(schemaModel.getNodeByUniquePointer(uniqueParentPointer)).toEqual(parentNodeMock);
const uniqueDefPointer = `${UNIQUE_POINTER_PREFIX}${defNodeMock.schemaPointer}`;
expect(schemaModel.getNodeByUniquePointer(uniqueDefPointer)).toEqual(defNodeMock);
const uniqueAllOfPointer = `${UNIQUE_POINTER_PREFIX}${allOfNodeMock.schemaPointer}`;
expect(schemaModel.getNodeByUniquePointer(uniqueAllOfPointer)).toEqual(allOfNodeMock);
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 = '#/properties/referenceToParent/properties/child';
const uniqueGrandchildPointer =
'#/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,
Expand All @@ -145,9 +143,8 @@ describe('SchemaModel', () => {
});

describe('getSchemaPointerByUniquePointer', () => {
const uniqueGrandChildPointer =
'#/properties/referenceToParent/properties/child/properties/grandchild';
const uniqueChildPointer = '#/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(
Expand All @@ -160,35 +157,37 @@ describe('SchemaModel', () => {
});

describe('getUniquePointer', () => {
it('Returns the pointer as is when called on the root node', () => {
it('Returns the unique pointer when called on the root node', () => {
const expectedUniqueRootPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}`;
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 = `${UNIQUE_POINTER_PREFIX}${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 = `${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(
defNodeWithChildrenChildMock.schemaPointer,
referenceToObjectNodeMock.schemaPointer,
),
).toEqual(expectedChildPointer);
).toEqual(expectedUniqueChildPointer);
expect(
schemaModel.getUniquePointer(
defNodeWithChildrenGrandchildMock.schemaPointer,
expectedChildPointer,
expectedUniqueChildPointer,
),
).toEqual(expectedGrandchildPointer);
).toEqual(expectedUniqueGrandchildPointer);
});
});

Expand Down
23 changes: 15 additions & 8 deletions frontend/packages/schema-model/src/lib/SchemaModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ 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 { ObjectUtils, ArrayUtils, StringUtils } from '@studio/pure-functions';
import { replaceStart } from 'app-shared/utils/stringUtils';
import {
createDefinitionPointer,
Expand Down Expand Up @@ -85,13 +85,14 @@ export class SchemaModel {
}

public getSchemaPointerByUniquePointer(uniquePointer: string): string {
if (this.hasNode(uniquePointer)) return uniquePointer;
const pointer = this.removeUniquePointerPrefix(uniquePointer);
if (this.hasNode(pointer)) return pointer;

const parentNodePointer = this.getParentSchemaPointerByUniquePointer(uniquePointer);
const parentSchemaPointer = this.getParentSchemaPointerByUniquePointer(pointer);
return makePointerFromArray([
parentNodePointer,
parentSchemaPointer,
Keyword.Properties,
extractNameFromPointer(uniquePointer),
extractNameFromPointer(pointer),
]);
}

Expand All @@ -107,10 +108,16 @@ export class SchemaModel {
return this.getNodeByUniquePointer(parentUniquePointer);
}

private removeUniquePointerPrefix(uniquePointer: string): string {
return StringUtils.removeStart(uniquePointer, UNIQUE_POINTER_PREFIX);
}

public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string {
if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer)) return schemaPointer;
if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer))
return `${UNIQUE_POINTER_PREFIX}${schemaPointer}`;

return `${uniqueParentPointer}/properties/${extractNameFromPointer(schemaPointer)}`;
const parentPointer = this.removeUniquePointerPrefix(uniqueParentPointer);
return `${UNIQUE_POINTER_PREFIX}${parentPointer}/properties/${extractNameFromPointer(schemaPointer)}`;
}

public hasNode(schemaPointer: string): boolean {
Expand Down
1 change: 1 addition & 0 deletions frontend/packages/schema-model/src/lib/constants.ts
Original file line number Diff line number Diff line change
@@ -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-';