Skip to content

Commit

Permalink
fix(DataModel): Enables the deletion of a child node from a definitio…
Browse files Browse the repository at this point in the history
…n that is currently in use (#12235)

* fix(DataModel): should be possible to delete fields from definition in use
Co-authored-by: WilliamThorenfeldt <133344438+WilliamThorenfeldt@users.noreply.github.com>

---------

Co-authored-by: Øvrelid <david.ovrelid@digdir.no>
Co-authored-by: WilliamThorenfeldt <133344438+WilliamThorenfeldt@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 2, 2024
1 parent f3a7d5d commit c2113ca
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 19 deletions.
1 change: 1 addition & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@
"schema_editor.custom_props_help": "Dette er egenskaper som ikke støttes av Altinn Studio, men som er satt inn på datamodellen ved hjelp av andre verktøy. Her kan man slette egenskepene eller endre deres verdier, men det er ikke mulig å legge til nye. Vær oppmerksom på at egenskaper som slettes fra denne listen ikke kan legges til igjen ved hjelp av Altinn Studio.",
"schema_editor.custom_props_unknown_format": "Ukjent format",
"schema_editor.custom_props_unknown_format_help": "Denne egenskapen har et format som ikke kan forhåndsvises eller redigeres i Altinn Studio.",
"schema_editor.datamodel_definition_field_deletion_text": "Feltet du vil slette tilhører en aktiv type og vil påvirke alle tilknyttede områder. Er du sikker på slettingen?",
"schema_editor.datamodel_field_deletion_confirm": "Ja, slett feltet",
"schema_editor.datamodel_field_deletion_info": "Forsikre deg om at feltet ikke er knyttet til noen oppsett eller regler før du sletter det.",
"schema_editor.datamodel_field_deletion_text": "Er du sikker på at du vil slette dette feltet?",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ const useDeleteNode = (pointer: string, savableModel: SavableSchemaModel) => {
const { setSelectedNodePointer } = useSchemaEditorAppContext();

return useCallback(() => {
if (confirm(t('schema_editor.datamodel_field_deletion_text'))) {
const confirmMessage = savableModel.areDefinitionParentsInUse(pointer)
? t('schema_editor.datamodel_definition_field_deletion_text')
: t('schema_editor.datamodel_field_deletion_text');
if (confirm(confirmMessage)) {
setSelectedNodePointer(null);
savableModel.deleteNode(pointer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import {
definitionNodeMock,
objectNodeMock,
referenceNodeMock,
referredNodeMock,
uiSchemaNodesMock,
} from '../../../../test/mocks/uiSchemaMock';
Expand Down Expand Up @@ -101,6 +102,17 @@ describe('SchemaNode', () => {
expect(deleteButton).toBeEnabled();
});

it('Enables the deletion of a child node from a definition that is currently in use', async () => {
const { pointer } = referenceNodeMock;
const save = jest.fn();
render({ pointer, save });

const deleteButton = screen.getByRole('button', { name: textMock('general.delete') });
jest.spyOn(window, 'confirm').mockImplementation(() => true);
await act(() => user.click(deleteButton));
expect(save).toHaveBeenCalledTimes(1);
});

it('Saves the model correctly when a node is deleted', async () => {
const { pointer } = objectNodeMock;
const schemaModel = setupSchemaModel();
Expand Down
27 changes: 15 additions & 12 deletions frontend/packages/schema-model/src/lib/SchemaModel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
combinationNodeWithMultipleChildrenMock,
defNodeMock,
defNodeWithChildrenChildMock,
defNodeWithChildrenGrandchildMock,
defNodeWithChildrenMock,
enumNodeMock,
nodeWithSameNameAsStringNodeMock,
Expand Down Expand Up @@ -221,6 +220,18 @@ describe('SchemaModel', () => {
});
});

describe('areDefinitionParentsInUse', () => {
it('Returns false if definition parent not in use', () => {
const result = schemaModel.areDefinitionParentsInUse(unusedDefinitionMock.pointer);
expect(result).toBeFalsy();
});

it('Returns true if definition parent is in use', () => {
const result = schemaModel.areDefinitionParentsInUse(defNodeWithChildrenChildMock.pointer);
expect(result).toBeTruthy();
});
});

describe('getFinalNode', () => {
it('Returns the node itself when it is not a reference', () => {
const result = schemaModel.getFinalNode(parentNodeMock.pointer);
Expand Down Expand Up @@ -696,18 +707,10 @@ describe('SchemaModel', () => {
expect(model.asArray()).toEqual(schemaModel.asArray());
});

it('Throws an error and keeps the model unchanged if trying to delete a definition node that is in use', () => {
it('Should not throw an error if trying to delete a child node of a definition in use', () => {
const model = schemaModel.deepClone();
expect(() => model.deleteNode(defNodeMock.pointer)).toThrowError();
expect(() => model.deleteNode(defNodeWithChildrenMock.pointer)).toThrowError();
expect(model.asArray()).toEqual(schemaModel.asArray());
});

it('Throws an error and keeps the model unchanged if trying to delete a child node of a definition in use', () => {
const model = schemaModel.deepClone();
expect(() => model.deleteNode(defNodeWithChildrenChildMock.pointer)).toThrowError();
expect(() => model.deleteNode(defNodeWithChildrenGrandchildMock.pointer)).toThrowError();
expect(model.asArray()).toEqual(schemaModel.asArray());
expect(() => model.deleteNode(defNodeWithChildrenChildMock.pointer)).not.toThrowError();
expect(model.asArray()).not.toEqual(schemaModel.asArray());
});
});

Expand Down
20 changes: 14 additions & 6 deletions frontend/packages/schema-model/src/lib/SchemaModel.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import type { NodePosition, UiSchemaNode, UiSchemaNodes } from '../types';
import { CombinationKind, FieldType } from '../types';
import {
CombinationKind,
FieldType,
type NodePosition,
type UiSchemaNode,
type UiSchemaNodes,
} from '../types';
import type { FieldNode } from '../types/FieldNode';
import type { CombinationNode } from '../types/CombinationNode';
import type { NodeMap } from '../types/NodeMap';
Expand Down Expand Up @@ -417,6 +422,7 @@ export class SchemaModel {
referringNodes.push(node);
}
}

return referringNodes;
}

Expand All @@ -432,9 +438,10 @@ export class SchemaModel {
}

public deleteNode(pointer: string): SchemaModel {
if (pointer === ROOT_POINTER) throw new Error('It is not possible to delete the root node.');
if (this.isDefinitionInUse(pointer))
throw new Error('Cannot delete a definition that is in use.');
if (pointer === ROOT_POINTER) {
throw new Error('It is not possible to delete the root node.');
}

return this.deleteNodeWithChildrenRecursively(pointer);
}

Expand All @@ -448,6 +455,7 @@ export class SchemaModel {
private isDefinitionInUse(pointer: string): boolean {
const node = this.getNode(pointer);
if (!isDefinition(node)) return false;

return this.hasReferringNodes(pointer) || this.areDefinitionParentsInUse(pointer);
}

Expand All @@ -456,7 +464,7 @@ export class SchemaModel {
return !!referringNodes.length;
}

private areDefinitionParentsInUse(pointer: string): boolean {
public areDefinitionParentsInUse(pointer: string): boolean {
const parent = this.getParentNode(pointer);
return this.isDefinitionInUse(parent.pointer);
}
Expand Down

0 comments on commit c2113ca

Please sign in to comment.