Skip to content

Commit

Permalink
fix: Fix error that occurs when dragging a type into an object (#13762)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasEng authored Oct 11, 2024
1 parent 5b272f3 commit f9c59ef
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 42 deletions.
1 change: 1 addition & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,7 @@
"schema_editor.add_reference.prompt": "Oppgi navnet på typen det skal refereres til.",
"schema_editor.add_reference.type_does_not_exist": "Typen {{reference}} eksisterer ikke.",
"schema_editor.add_string": "Legg til tekst",
"schema_editor.add_type": "Legg til type",
"schema_editor.all_of": "Alle i",
"schema_editor.any_of": "Noen av",
"schema_editor.array": "Liste",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
isNodeValidParent,
ObjectKind,
ROOT_POINTER,
SchemaModel,
} from '@altinn/schema-model';
import { useTranslation } from 'react-i18next';
import { StudioButton, StudioDeleteButton, StudioDropdownMenu } from '@studio/components';
Expand Down Expand Up @@ -89,13 +90,13 @@ const AddNodeMenu = ({ schemaPointer }: AddNodeMenuProps) => {
};

const useAddNodeMenuItems = (schemaPointer: string): AddNodeMenuItemProps[] => {
const { setSelectedUniquePointer, schemaModel } = useSchemaEditorAppContext();
const { setSelectedUniquePointer } = useSchemaEditorAppContext();
const addNode = useAddProperty();

const addAndSelectNode = (...params: Parameters<typeof addNode>) => {
const newPointer = addNode(...params);
if (newPointer) {
const newUniquePointer = schemaModel.getUniquePointer(newPointer);
const newUniquePointer = SchemaModel.getUniquePointer(newPointer);
setSelectedUniquePointer(newUniquePointer);
}
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import type { ReferenceNode } from '@altinn/schema-model';
import { extractNameFromPointer, isReference, SchemaModel } from '@altinn/schema-model';
import {
ROOT_POINTER,
extractNameFromPointer,
isReference,
SchemaModel,
} from '@altinn/schema-model';

import {
definitionNodeMock,
combinationNodeMock,
Expand All @@ -12,6 +18,15 @@ import { useAddReference } from './useAddReference';
import type { SavableSchemaModel } from '../../../classes/SavableSchemaModel';
import { ArrayUtils } from '@studio/pure-functions';

// Test data:
const nameOfDefinition = extractNameFromPointer(definitionNodeMock.schemaPointer);
const uniqueRootNodePointer = SchemaModel.getUniquePointer(ROOT_POINTER);
const schemaPointerOfParent = combinationNodeMock.schemaPointer;
const uniquePointerOfParent = SchemaModel.getUniquePointer(
schemaPointerOfParent,
uniqueRootNodePointer,
);

describe('useAddReference', () => {
const setup = () => {
const save = jest.fn();
Expand All @@ -24,14 +39,12 @@ describe('useAddReference', () => {

it('Adds a reference to the given position', () => {
const { add, save } = setup();
const nameOfDefinition = extractNameFromPointer(definitionNodeMock.schemaPointer);
const pointerOfParent = combinationNodeMock.schemaPointer;
const indexInNewParent = 1;
const target: ItemPosition = { parentId: pointerOfParent, index: indexInNewParent };
const target: ItemPosition = { parentId: uniquePointerOfParent, index: indexInNewParent };
add(nameOfDefinition, target);
expect(save).toHaveBeenCalledTimes(1);
const savedModel: SavableSchemaModel = save.mock.lastCall[0];
const childrenOfNewParent = savedModel.getChildNodes(pointerOfParent);
const childrenOfNewParent = savedModel.getChildNodes(schemaPointerOfParent);
const addedChild = childrenOfNewParent[indexInNewParent];
expect(isReference(addedChild)).toBe(true);
const addedReferenceNode = addedChild as ReferenceNode;
Expand All @@ -40,14 +53,12 @@ describe('useAddReference', () => {

it('Adds a reference to the end if the given position is -1', () => {
const { add, save } = setup();
const nameOfDefinition = extractNameFromPointer(definitionNodeMock.schemaPointer);
const pointerOfParent = combinationNodeMock.schemaPointer;
const givenIndex = -1;
const target: ItemPosition = { parentId: pointerOfParent, index: givenIndex };
const target: ItemPosition = { parentId: uniquePointerOfParent, index: givenIndex };
add(nameOfDefinition, target);
expect(save).toHaveBeenCalledTimes(1);
const savedModel: SavableSchemaModel = save.mock.lastCall[0];
const childrenOfNewParent = savedModel.getChildNodes(pointerOfParent);
const childrenOfNewParent = savedModel.getChildNodes(schemaPointerOfParent);
const addedChild = ArrayUtils.last(childrenOfNewParent);
expect(isReference(addedChild)).toBe(true);
const addedReferenceNode = addedChild as ReferenceNode;
Expand All @@ -56,14 +67,12 @@ describe('useAddReference', () => {

it('Adds a reference to the end if the given position is the same as the number of elements', () => {
const { add, save } = setup();
const nameOfDefinition = extractNameFromPointer(definitionNodeMock.schemaPointer);
const pointerOfParent = combinationNodeMock.schemaPointer;
const givenIndex = combinationNodeMock.children.length;
const target: ItemPosition = { parentId: pointerOfParent, index: givenIndex };
const target: ItemPosition = { parentId: uniquePointerOfParent, index: givenIndex };
add(nameOfDefinition, target);
expect(save).toHaveBeenCalledTimes(1);
const savedModel: SavableSchemaModel = save.mock.lastCall[0];
const childrenOfNewParent = savedModel.getChildNodes(pointerOfParent);
const childrenOfNewParent = savedModel.getChildNodes(schemaPointerOfParent);
const addedChild = ArrayUtils.last(childrenOfNewParent);
expect(isReference(addedChild)).toBe(true);
const addedReferenceNode = addedChild as ReferenceNode;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
import type { HandleAdd, ItemPosition } from 'app-shared/types/dndTypes';
import { useCallback } from 'react';
import type { NodePosition } from '@altinn/schema-model';
import { SchemaModel } from '@altinn/schema-model';
import { calculatePositionInFullList } from '../utils';
import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel';
import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext';

export const useAddReference = (): HandleAdd<string> => {
const { setSelectedUniquePointer, schemaModel } = useSchemaEditorAppContext();
const { setSelectedUniquePointer } = useSchemaEditorAppContext();
const savableModel = useSavableSchemaModel();
return useCallback(
(reference: string, position: ItemPosition) => {
const index = calculatePositionInFullList(savableModel, position);
const target: NodePosition = { parentPointer: position.parentId, index };
const schemaPointer = savableModel.getFinalNode(target.parentPointer).schemaPointer;
const parentPointer = savableModel.getSchemaPointerByUniquePointer(position.parentId);
const target: NodePosition = { parentPointer, index };
const { schemaPointer } = savableModel.getFinalNode(target.parentPointer);
const refName = savableModel.generateUniqueChildName(schemaPointer, 'ref');
const ref = savableModel.addReference(refName, reference, target);
const uniquePointer = schemaModel.getUniquePointer(ref.schemaPointer);
const uniquePointer = SchemaModel.getUniquePointer(ref.schemaPointer);
setSelectedUniquePointer(uniquePointer);
},
[savableModel, setSelectedUniquePointer, schemaModel],
[savableModel, setSelectedUniquePointer],
);
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { HandleMove, ItemPosition } from 'app-shared/types/dndTypes';
import { useCallback } from 'react';
import type { NodePosition } from '@altinn/schema-model';
import { extractNameFromPointer, isCombination } from '@altinn/schema-model';
import { SchemaModel, extractNameFromPointer, isCombination } from '@altinn/schema-model';
import { calculatePositionInFullList } from '../utils';
import { useSavableSchemaModel } from '../../../hooks/useSavableSchemaModel';
import { useTranslation } from 'react-i18next';
Expand Down Expand Up @@ -38,7 +38,7 @@ export const useMoveProperty = (): HandleMove => {
} else {
const movedNode = savableModel.moveNode(schemaPointer, target);
if (selectedUniquePointer === uniquePointer) {
const movedUniquePointer = savableModel.getUniquePointer(
const movedUniquePointer = SchemaModel.getUniquePointer(
movedNode.schemaPointer,
position.parentId,
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { useAddProperty } from '../../../../hooks/useAddProperty';
import type { FieldType, ObjectKind } from '@altinn/schema-model';
import { useSavableSchemaModel } from '../../../../hooks/useSavableSchemaModel';
import { SchemaModel } from '@altinn/schema-model';
import { useSchemaEditorAppContext } from '@altinn/schema-editor/hooks/useSchemaEditorAppContext';
import { AddPropertiesMenu } from '../../../AddPropertiesMenu';
import { useTranslation } from 'react-i18next';
Expand All @@ -14,15 +14,14 @@ interface AddPropertyMenuProps {

export const AddPropertyMenu = ({ schemaPointer, uniquePointer }: AddPropertyMenuProps) => {
const { setSelectedUniquePointer } = useSchemaEditorAppContext();
const savableModel = useSavableSchemaModel();
const { t } = useTranslation();

const addProperty = useAddProperty();

const addPropertyAndClose = (kind: ObjectKind, fieldType?: FieldType) => {
const childPointer = addProperty(kind, fieldType, schemaPointer);
if (childPointer) {
const uniqueChildPointer = savableModel.getUniquePointer(childPointer, uniquePointer);
const uniqueChildPointer = SchemaModel.getUniquePointer(childPointer, uniquePointer);
setSelectedUniquePointer(uniqueChildPointer);
}
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import type { ReactElement, ReactNode } from 'react';
import React from 'react';
import type { SchemaModel, UiSchemaNode } from '@altinn/schema-model';
import { extractNameFromPointer, isNodeValidParent, isReference } from '@altinn/schema-model';
import type { UiSchemaNode } from '@altinn/schema-model';
import {
SchemaModel,
extractNameFromPointer,
isNodeValidParent,
isReference,
} from '@altinn/schema-model';
import { DragAndDropTree } from 'app-shared/components/DragAndDropTree';
import { renderSchemaNodeList } from '../renderSchemaNodeList';
import { renderIcon } from './renderIcon';
Expand All @@ -28,7 +33,7 @@ export const SchemaNode = ({
? ''
: extractNameFromPointer(schemaPointer);
const index = savableModel.getIndexOfChildNode(schemaPointer);
const uniquePointer = savableModel.getUniquePointer(schemaPointer, uniqueParentPointer);
const uniquePointer = SchemaModel.getUniquePointer(schemaPointer, uniqueParentPointer);

const title = label || t('schema_editor.tree.combination_child_title', { index });
const labelWrapper = (labelComponent: ReactNode) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import { renderSchemaNodeList } from './renderSchemaNodeList';
import { useTranslation } from 'react-i18next';
import { useSavableSchemaModel } from '../../hooks/useSavableSchemaModel';
import { useSchemaEditorAppContext } from '../../hooks/useSchemaEditorAppContext';
import { SchemaModel } from '@altinn/schema-model/lib/SchemaModel';

export interface SchemaTreeProps {
schemaPointer?: string;
}

export const SchemaTree = ({ schemaPointer }: SchemaTreeProps) => {
const savableModel = useSavableSchemaModel();
const uniquePointer = savableModel.getUniquePointer(schemaPointer);
const uniquePointer = SchemaModel.getUniquePointer(schemaPointer);
const { selectedUniquePointer, setSelectedUniquePointer } = useSchemaEditorAppContext();
const { t } = useTranslation();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React from 'react';
import { StudioButton } from '@studio/components';
import { PlusIcon } from '@studio/icons';
import type { UiSchemaNode } from '@altinn/schema-model';
import { SchemaModel } from '@altinn/schema-model';
import classes from './TypesInspector.module.css';
import { Divider } from 'app-shared/primitives';
import { useTranslation } from 'react-i18next';
Expand All @@ -25,7 +26,7 @@ export const TypesInspector = ({ schemaItems }: TypesInspectorProps) => {

const setSelectedType = (schemaPointer: string) => {
setSelectedTypePointer(schemaPointer);
const uniquePointer = schemaModel.getUniquePointer(schemaPointer);
const uniquePointer = SchemaModel.getUniquePointer(schemaPointer);
setSelectedUniquePointer(uniquePointer);
};

Expand Down Expand Up @@ -58,6 +59,7 @@ export const TypesInspector = ({ schemaItems }: TypesInspectorProps) => {
variant='tertiary'
icon={<PlusIcon height={40} />}
onClick={handleAddDefinition}
title={t('schema_editor.add_type')}
/>
</div>

Expand Down
10 changes: 5 additions & 5 deletions frontend/packages/schema-model/src/lib/SchemaModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ describe('SchemaModel', () => {
describe('getUniquePointer', () => {
it('Returns the unique pointer when called on the root node', () => {
const expectedUniqueRootPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}`;
expect(schemaModel.getUniquePointer(rootNodeMock.schemaPointer)).toEqual(
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}`;

expect(schemaModel.getUniquePointer(referenceToObjectNodeMock.schemaPointer)).toEqual(
expect(SchemaModel.getUniquePointer(referenceToObjectNodeMock.schemaPointer)).toEqual(
expectedUniquePointer,
);
});
Expand All @@ -187,13 +187,13 @@ describe('SchemaModel', () => {
const expectedUniqueGrandchildPointer = `${UNIQUE_POINTER_PREFIX}${ROOT_POINTER}/properties/referenceToParent/properties/child/properties/grandchild`;

expect(
schemaModel.getUniquePointer(
SchemaModel.getUniquePointer(
defNodeWithChildrenChildMock.schemaPointer,
referenceToObjectNodeMock.schemaPointer,
),
).toEqual(expectedUniqueChildPointer);
expect(
schemaModel.getUniquePointer(
SchemaModel.getUniquePointer(
defNodeWithChildrenGrandchildMock.schemaPointer,
expectedUniqueChildPointer,
),
Expand All @@ -203,7 +203,7 @@ describe('SchemaModel', () => {
it('Returns a pointer reflecting the path to a given node in a reference to a combination', () => {
const { schemaPointer } = combinationDefNodeChild1Mock;
const uniquePointerOfParent = referenceToCombinationDefNodeMock.schemaPointer;
const result = schemaModel.getUniquePointer(schemaPointer, uniquePointerOfParent);
const result = SchemaModel.getUniquePointer(schemaPointer, uniquePointerOfParent);
const expectedResult = `${UNIQUE_POINTER_PREFIX}#/properties/referenceToCombinationDef/oneOf/0`;
expect(result).toEqual(expectedResult);
});
Expand Down
8 changes: 4 additions & 4 deletions frontend/packages/schema-model/src/lib/SchemaModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class SchemaModel {
}

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

const parentSchemaPointer = this.getParentSchemaPointerByUniquePointer(pointer);
Expand All @@ -108,15 +108,15 @@ export class SchemaModel {
return this.getNodeByUniquePointer(parentUniquePointer);
}

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

public getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string {
public static getUniquePointer(schemaPointer: string, uniqueParentPointer?: string): string {
if (!uniqueParentPointer || !isDefinitionPointer(schemaPointer))
return `${UNIQUE_POINTER_PREFIX}${schemaPointer}`;
const category = extractCategoryFromPointer(schemaPointer);
const parentPointer = this.removeUniquePointerPrefix(uniqueParentPointer);
const parentPointer = SchemaModel.removeUniquePointerPrefix(uniqueParentPointer);
return `${UNIQUE_POINTER_PREFIX}${parentPointer}/${category}/${extractNameFromPointer(schemaPointer)}`;
}

Expand Down
43 changes: 40 additions & 3 deletions frontend/testing/playwright/pages/DataModelPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Environment } from '../helpers/StudioEnvironment';
import path from 'path';
import { expect } from '@playwright/test';
import { DataTestId } from '../enum/DataTestId';
import { droppableListId, typeItemId } from '@studio/testing/testids';

export class DataModelPage extends BasePage {
constructor(page: Page, environment?: Environment) {
Expand Down Expand Up @@ -38,13 +39,13 @@ export class DataModelPage extends BasePage {

public async clickOnAddPropertyButton(): Promise<void> {
await this.page
.getByRole('button', { name: this.textMock('schema_editor.add') })
.getByRole('button', { name: this.textMock('schema_editor.add_node_of_type') })
.first()
.click();
}

public async clickOnObjectAddPropertyButton(): Promise<void> {
await this.page.getByTitle(this.textMock('schema_editor.add')).click();
await this.page.getByTitle(this.textMock('schema_editor.add_node_of_type')).click();
}

public async clickOnAddObjectPropertyMenuItem(): Promise<void> {
Expand All @@ -58,7 +59,12 @@ export class DataModelPage extends BasePage {
}

public async checkThatTreeItemPropertyExistsOnScreen(name: string): Promise<void> {
await this.page.getByRole('treeitem', { name }).isVisible();
const treeItem = this.getTreeItemProperty(name);
await expect(treeItem).toBeVisible();
}

private getTreeItemProperty(name: string): Locator {
return this.page.getByRole('treeitem', { name });
}

public async clearNameField(): Promise<void> {
Expand Down Expand Up @@ -184,4 +190,35 @@ export class DataModelPage extends BasePage {
private getTypeCombobox(): Locator {
return this.page.getByRole('combobox', { name: this.textMock('schema_editor.type') });
}

public async addType(): Promise<void> {
await this.getAddTypeButton().click();
}

private getAddTypeButton(): Locator {
return this.page.getByRole('button', { name: this.textMock('schema_editor.add_type') });
}

public async verifyThatTypeIsVisible(typeName: string): Promise<void> {
const typeItem = this.getTypeItem(typeName);
await expect(typeItem).toBeVisible();
}

public getTypeItem(name: string): Locator {
const pointer = '#/$defs/' + name;
return this.page.getByTestId(typeItemId(pointer));
}

public getDroppableList(parentName: string): Locator {
return this.page.getByTitle(parentName).getByTestId(droppableListId);
}

public async clickOnBackToDataModelButton(): Promise<void> {
await this.getBackToDataModelButton().click();
}

private getBackToDataModelButton(): Locator {
const name = this.textMock('schema_editor.back_to_data_model');
return this.page.getByRole('button', { name });
}
}
Loading

0 comments on commit f9c59ef

Please sign in to comment.