Skip to content

Commit

Permalink
fix(forms): reset anonymized fields immediately
Browse files Browse the repository at this point in the history
fixes #2285
  • Loading branch information
sleidig committed Apr 3, 2024
1 parent e77e385 commit 8a6396b
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import { User } from "../../user/user";
import { TEST_USER } from "../../user/demo-user-generator.service";
import { CurrentUserSubject } from "../../session/current-user-subject";
import moment from "moment";
import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service";
import { MockEntityMapperService } from "../../entity/entity-mapper/mock-entity-mapper-service";

describe("EntityFormService", () => {
let service: EntityFormService;
Expand Down Expand Up @@ -306,6 +308,25 @@ describe("EntityFormService", () => {
Entity.schema.delete("test");
});

it("should not save 'null' as value from empty form fields", async () => {
Entity.schema.set("test", { dataType: "string" });

const entity = new Entity();
const form = service.createFormGroup([{ id: "test" }], entity);
form.get("test").reset();
expect(form.get("test").getRawValue()).toEqual(null);

await service.saveChanges(form, entity);

const entityMapper = TestBed.inject(
EntityMapperService,
) as MockEntityMapperService;
const actualSaved = entityMapper.get(entity.getType(), entity.getId());
expect(actualSaved).toEqual(entity);
// service should remove 'null' value, which are the default for empty form fields
expect(actualSaved["test"]).not.toEqual(null);
});

it("should add column definitions from property schema", () => {
class Test extends Child {
@DatabaseField({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,15 @@ export class EntityFormService {
entity: T,
): Promise<T> {
this.checkFormValidity(form);

const updatedEntity = entity.copy() as T;
Object.assign(updatedEntity, form.getRawValue());
updatedEntity.assertValid();
for (const [key, value] of Object.entries(form.getRawValue())) {
if (value !== null) {
updatedEntity[key] = value;
}
}

updatedEntity.assertValid();
this.assertPermissionsToSave(entity, updatedEntity);

return this.entityMapper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ describe("EntityAnonymizeService", () => {
}
}

it("should anonymize and only keep properties marked to be retained", async () => {
it("should anonymize and only keep properties marked to be retained, setting others to 'null'", async () => {
const entity = new AnonymizableEntity();
entity.defaultField = "test";
entity.retainedField = "test";
Expand All @@ -107,13 +107,12 @@ describe("EntityAnonymizeService", () => {

AnonymizableEntity.expectAnonymized(
entity.getId(),
AnonymizableEntity.create({ retainedField: "test" }),
AnonymizableEntity.create({ retainedField: "test", defaultField: null }),
);
});

it("should anonymize and keep empty record without any fields", async () => {
const entity = new AnonymizableEntity();
entity.defaultField = "test";

await service.anonymizeEntity(entity);

Expand All @@ -140,6 +139,7 @@ describe("EntityAnonymizeService", () => {
AnonymizableEntity.create({
inactive: true,
anonymized: true,
defaultField: null,
...entityProperties,
}),
true,
Expand All @@ -154,7 +154,11 @@ describe("EntityAnonymizeService", () => {

AnonymizableEntity.expectAnonymized(
entity.getId(),
AnonymizableEntity.create({ inactive: true, anonymized: true }),
AnonymizableEntity.create({
inactive: true,
anonymized: true,
defaultField: null,
}),
true,
);
});
Expand Down Expand Up @@ -187,7 +191,7 @@ describe("EntityAnonymizeService", () => {

AnonymizableEntity.expectAnonymized(
entity.getId(),
AnonymizableEntity.create({}),
AnonymizableEntity.create({ file: null }),
);
expect(mockFileService.removeFile).toHaveBeenCalled();
});
Expand Down Expand Up @@ -234,6 +238,12 @@ describe("EntityAnonymizeService", () => {
expectedAnonymizedEntity.inactive = true;
expectedAnonymizedEntity.anonymized = true;

for (const [k, v] of Object.entries(actualEntity)) {
if (v === null) {
expectedAnonymizedEntity[k] = null;
}
}

expect(comparableEntityData(actualEntity)).toEqual(
comparableEntityData(expectedAnonymizedEntity),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ export class EntityAnonymizeService extends CascadingEntityAction {
await firstValueFrom(this.fileService.removeFile(entity, key));
}

delete entity[key];
if (entity[key] !== undefined) {
entity[key] = null;
}
}

private async keepEntityUnchanged(e: Entity): Promise<CascadingActionResult> {
Expand Down
19 changes: 19 additions & 0 deletions src/app/core/entity/schema/entity-schema.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,25 @@ describe("EntitySchemaService", () => {
expect(rawData.aString).toEqual("192");
});

it("should keep 'null' as value if explicitly set", () => {
class TestEntity extends Entity {
@DatabaseField() aString: string;
}

const entity = new TestEntity();

const data = {
_id: entity.getId(),
aString: null,
};
service.loadDataIntoEntity(entity, data);

expect(entity.aString).toEqual(null);

const rawData = service.transformEntityToDatabaseFormat(entity);
expect(rawData.aString).toEqual(null);
});

it("should return the directly defined component name for viewing and editing a property", () => {
class Test extends Entity {
@DatabaseField({
Expand Down
14 changes: 12 additions & 2 deletions src/app/core/entity/schema/entity-schema.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class EntitySchemaService {
for (const key of schema.keys()) {
const schemaField: EntitySchemaField = schema.get(key);

if (data[key] === undefined || data[key] === null) {
if (data[key] === undefined) {
continue;
}

Expand Down Expand Up @@ -153,7 +153,7 @@ export class EntitySchemaService {
let value = entity[key];
const schemaField: EntitySchemaField = schema.get(key);

if (value === undefined || value === null) {
if (value === undefined) {
// skip and keep undefined
continue;
}
Expand Down Expand Up @@ -218,6 +218,11 @@ export class EntitySchemaService {
schemaField: EntitySchemaField,
entity?: Entity,
) {
if (value === null) {
// keep 'null' to be able to explicitly mark a value as being reset
return null;
}

return this.getDatatypeOrDefault(
schemaField.dataType,
).transformToDatabaseFormat(value, schemaField, entity);
Expand All @@ -234,6 +239,11 @@ export class EntitySchemaService {
schemaField: EntitySchemaField,
dataObject?: any,
) {
if (value === null) {
// keep 'null' to be able to explicitly mark a value as being reset
return null;
}

return this.getDatatypeOrDefault(
schemaField.dataType,
).transformToObjectFormat(value, schemaField, dataObject);
Expand Down

0 comments on commit 8a6396b

Please sign in to comment.