diff --git a/src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts b/src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts index 0ccea0ad79..c0c9152686 100644 --- a/src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts +++ b/src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts @@ -37,13 +37,13 @@ import { EntityDatatype } from "../../../basic-datatypes/entity/entity.datatype" import { EntityArrayDatatype } from "../../../basic-datatypes/entity-array/entity-array.datatype"; import { ConfigurableEnumService } from "../../../basic-datatypes/configurable-enum/configurable-enum.service"; import { EntityRegistry } from "../../../entity/database-entity.decorator"; -import { uniqueIdValidator } from "../../../common-components/entity-form/unique-id-validator"; import { AdminEntityService } from "../../admin-entity.service"; import { ConfigureEnumPopupComponent } from "../../../basic-datatypes/configurable-enum/configure-enum-popup/configure-enum-popup.component"; import { ConfigurableEnum } from "../../../basic-datatypes/configurable-enum/configurable-enum"; import { generateIdFromLabel } from "../../../../utils/generate-id-from-label/generate-id-from-label"; import { merge } from "rxjs"; import { filter } from "rxjs/operators"; +import { uniqueIdValidator } from "app/core/common-components/entity-form/unique-id-validator/unique-id-validator"; /** * Allows configuration of the schema of a single Entity field, like its dataType and labels. @@ -115,10 +115,12 @@ export class AdminEntityFieldComponent implements OnChanges { } private initSettings() { - this.fieldIdForm = this.fb.control(this.fieldId, [ - Validators.required, - uniqueIdValidator(Array.from(this.entityType.schema.keys())), - ]); + this.fieldIdForm = this.fb.control(this.fieldId, { + validators: [Validators.required], + asyncValidators: [ + uniqueIdValidator(Array.from(this.entityType.schema.keys())), + ], + }); this.additionalForm = this.fb.control(this.entitySchemaField.additional); this.schemaFieldsForm = this.fb.group({ diff --git a/src/app/core/basic-datatypes/string/edit-text/edit-text.component.html b/src/app/core/basic-datatypes/string/edit-text/edit-text.component.html index 2c096759b0..8dffdd07d0 100644 --- a/src/app/core/basic-datatypes/string/edit-text/edit-text.component.html +++ b/src/app/core/basic-datatypes/string/edit-text/edit-text.component.html @@ -1,7 +1,7 @@ {{ label }} - + diff --git a/src/app/core/common-components/entity-field-edit/entity-field-edit.component.html b/src/app/core/common-components/entity-field-edit/entity-field-edit.component.html index a6c3d3feae..5402a005bf 100644 --- a/src/app/core/common-components/entity-field-edit/entity-field-edit.component.html +++ b/src/app/core/common-components/entity-field-edit/entity-field-edit.component.html @@ -5,7 +5,7 @@ component: _field.editComponent, config: { formFieldConfig: _field, - formControl: form.get(_field.id), + formControl: form?.get(_field.id), entity: entity } }" diff --git a/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.spec.ts b/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.spec.ts index cbdda3fdbc..ff5be7cbe0 100644 --- a/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.spec.ts +++ b/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.spec.ts @@ -5,13 +5,29 @@ import { patternWithMessage, } from "./dynamic-validators.service"; import { FormValidatorConfig } from "./form-validator-config"; -import { UntypedFormControl, ValidatorFn } from "@angular/forms"; +import { + AsyncValidatorFn, + UntypedFormControl, + ValidatorFn, +} from "@angular/forms"; +import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; +import { User } from "../../../user/user"; describe("DynamicValidatorsService", () => { let service: DynamicValidatorsService; + let mockedEntityMapper: jasmine.SpyObj; + beforeEach(() => { - TestBed.configureTestingModule({}); + mockedEntityMapper = jasmine.createSpyObj("EntityMapperService", [ + "loadType", + ]); + + TestBed.configureTestingModule({ + providers: [ + { provide: EntityMapperService, useValue: mockedEntityMapper }, + ], + }); service = TestBed.inject(DynamicValidatorsService); }); @@ -19,21 +35,26 @@ describe("DynamicValidatorsService", () => { expect(service).toBeTruthy(); }); - function testValidator( - validator: ValidatorFn, + async function testValidator( + validator: ValidatorFn | AsyncValidatorFn, successState: any, failureState: any, ) { - const results = [successState, failureState].map((state) => { - const mockControl = new UntypedFormControl(state); - return validator(mockControl); - }); - expect(results[0]) + function dummyFormControl(state) { + const control = new UntypedFormControl(state); + control.markAsDirty(); + return control; + } + + const resultSuccess = await validator(dummyFormControl(successState)); + expect(resultSuccess) .withContext("Expected validator not to have errors") .toBeNull(); - expect(results[1]) + + const resultFailure = await validator(dummyFormControl(failureState)); + expect(resultFailure) .withContext("Expected validator to have errors") - .not.toBeNull(); + .toEqual(jasmine.any(Object)); } it("should load validators from the config", () => { @@ -41,7 +62,7 @@ describe("DynamicValidatorsService", () => { min: 9, pattern: "[a-z]*", }; - const validators = service.buildValidators(config); + const validators = service.buildValidators(config).validators; expect(validators).toHaveSize(2); testValidator(validators[0], 10, 8); testValidator(validators[1], "ab", "1"); @@ -55,7 +76,7 @@ describe("DynamicValidatorsService", () => { validEmail: true, pattern: "foo", }; - const validators = service.buildValidators(config); + const validators = service.buildValidators(config).validators; [ [10, 8], [8, 11], @@ -73,7 +94,7 @@ describe("DynamicValidatorsService", () => { message: "M", pattern: "[a-z]", }, - }); + }).validators; expect(validators).toHaveSize(1); const invalidForm = new UntypedFormControl("09"); const validationErrors = validators[0](invalidForm); @@ -83,6 +104,16 @@ describe("DynamicValidatorsService", () => { }), ); }); + + it("should build uniqueId async validator", async () => { + const config: FormValidatorConfig = { + uniqueId: "User", + }; + mockedEntityMapper.loadType.and.resolveTo([new User("existing id")]); + + const validators = service.buildValidators(config).asyncValidators; + await testValidator(validators[0], "new id", "existing id"); + }); }); describe("patternWithMessage", () => { diff --git a/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.ts b/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.ts index 9b949b73b6..0a770733fc 100644 --- a/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.ts +++ b/src/app/core/common-components/entity-form/dynamic-form-validators/dynamic-validators.service.ts @@ -1,9 +1,16 @@ import { Injectable } from "@angular/core"; import { DynamicValidator, FormValidatorConfig } from "./form-validator-config"; -import { AbstractControl, ValidatorFn, Validators } from "@angular/forms"; +import { + AbstractControl, + FormControl, + FormControlOptions, + ValidationErrors, + ValidatorFn, + Validators, +} from "@angular/forms"; import { LoggingService } from "../../../logging/logging.service"; - -type ValidatorFactory = (value: any, name: string) => ValidatorFn; +import { uniqueIdValidator } from "../unique-id-validator/unique-id-validator"; +import { EntityMapperService } from "../../../entity/entity-mapper/entity-mapper.service"; /** * creates a pattern validator that also carries a predefined @@ -47,23 +54,45 @@ export class DynamicValidatorsService { * given a value that serves as basis for the validation. * @private */ - private static validators: { - [key in DynamicValidator]: ValidatorFactory | null; - } = { - min: (value) => Validators.min(value as number), - max: (value) => Validators.max(value as number), - pattern: (value) => { - if (typeof value === "object") { - return patternWithMessage(value.pattern, value.message); - } else { - return Validators.pattern(value as string); + private getValidator( + key: DynamicValidator, + value: any, + ): + | { async?: false; fn: ValidatorFn } + | { + async: true; + fn: AsyncPromiseValidatorFn; } - }, - validEmail: (value) => (value ? Validators.email : null), - required: (value) => (value ? Validators.required : null), - }; + | null { + switch (key) { + case "min": + return { fn: Validators.min(value as number) }; + case "max": + return { fn: Validators.max(value as number) }; + case "pattern": + if (typeof value === "object") { + return { fn: patternWithMessage(value.pattern, value.message) }; + } else { + return { fn: Validators.pattern(value as string) }; + } + case "validEmail": + return value ? { fn: Validators.email } : null; + case "uniqueId": + return value ? this.buildUniqueIdValidator(value) : null; + case "required": + return value ? { fn: Validators.required } : null; + default: + this.loggingService.warn( + `Trying to generate validator ${key} but it does not exist`, + ); + return null; + } + } - constructor(private loggingService: LoggingService) {} + constructor( + private loggingService: LoggingService, + private entityMapper: EntityMapperService, + ) {} /** * Builds all validator functions that are part of the configuration object. @@ -76,24 +105,58 @@ export class DynamicValidatorsService { * [ Validators.required, Validators.max(5) ] * @see ValidatorFn */ - public buildValidators(config: FormValidatorConfig): ValidatorFn[] { - const validators: ValidatorFn[] = []; + public buildValidators(config: FormValidatorConfig): FormControlOptions { + const formControlOptions = { + validators: [], + asyncValidators: [], + }; + for (const key of Object.keys(config)) { - const factory = DynamicValidatorsService.validators[key]; - if (!factory) { - this.loggingService.warn( - `Trying to generate validator ${key} but it does not exist`, - ); - continue; - } - const validatorFn = factory(config[key], key); - if (validatorFn !== null) { - validators.push(validatorFn); + const validatorFn = this.getValidator( + key as DynamicValidator, + config[key], + ); + + if (validatorFn?.async) { + const validatorFnWithReadableErrors = (control) => + validatorFn + .fn(control) + .then((res) => this.addHumanReadableError(key, res)); + formControlOptions.asyncValidators.push(validatorFnWithReadableErrors); + } else if (validatorFn) { + const validatorFnWithReadableErrors = (control: FormControl) => + this.addHumanReadableError(key, validatorFn.fn(control)); + formControlOptions.validators.push(validatorFnWithReadableErrors); } - // A validator function of `null` is a legal case. For example - // { required : false } produces a `null` validator function + + // A validator function of `null` is a legal case, for which no validator function is added. + // For example `{ required : false }` produces a `null` validator function + } + + if (formControlOptions.asyncValidators.length > 0) { + (formControlOptions as FormControlOptions).updateOn = "blur"; + } + + return formControlOptions; + } + + private addHumanReadableError( + validatorType: string, + validationResult: ValidationErrors | null, + ): ValidationErrors { + if (!validationResult) { + return validationResult; } - return validators; + + validationResult[validatorType] = { + ...validationResult[validatorType], + errorMessage: this.descriptionForValidator( + validatorType, + validationResult[validatorType], + ), + }; + + return validationResult; } /** @@ -103,7 +166,7 @@ export class DynamicValidatorsService { * @param validator The validator to get the description for * @param validationValue The value associated with the validator */ - public descriptionForValidator( + private descriptionForValidator( validator: DynamicValidator | string, validationValue: any, ): string { @@ -126,6 +189,8 @@ export class DynamicValidatorsService { return $localize`Please enter a valid date`; case "isNumber": return $localize`Please enter a valid number`; + case "uniqueId": + return validationValue; default: this.loggingService.error( `No description defined for validator "${validator}": ${JSON.stringify( @@ -135,4 +200,23 @@ export class DynamicValidatorsService { throw $localize`Invalid input`; } } + + private buildUniqueIdValidator(value: string): { + async: true; + fn: AsyncPromiseValidatorFn; + } { + return { + fn: uniqueIdValidator(() => + this.entityMapper + .loadType(value) + // TODO: extend this to allow checking for any configurable property (e.g. Child.name rather than only id) + .then((entities) => entities.map((entity) => entity.getId(false))), + ), + async: true, + }; + } } + +export type AsyncPromiseValidatorFn = ( + control: FormControl, +) => Promise; diff --git a/src/app/core/common-components/entity-form/dynamic-form-validators/form-validator-config.ts b/src/app/core/common-components/entity-form/dynamic-form-validators/form-validator-config.ts index 5e05f974a5..edb1f843ee 100644 --- a/src/app/core/common-components/entity-form/dynamic-form-validators/form-validator-config.ts +++ b/src/app/core/common-components/entity-form/dynamic-form-validators/form-validator-config.ts @@ -11,6 +11,8 @@ export type DynamicValidator = | "required" /** type: boolean */ | "validEmail" + /** type: string = EntityType; check against existing ids of the entity type */ + | "uniqueId" /** type: string or regex */ | "pattern"; diff --git a/src/app/core/common-components/entity-form/entity-form.service.ts b/src/app/core/common-components/entity-form/entity-form.service.ts index 85526a3975..29e744aa17 100644 --- a/src/app/core/common-components/entity-form/entity-form.service.ts +++ b/src/app/core/common-components/entity-form/entity-form.service.ts @@ -1,5 +1,11 @@ import { Injectable } from "@angular/core"; -import { FormBuilder, FormGroup, ɵElement } from "@angular/forms"; +import { + FormBuilder, + FormControl, + FormControlOptions, + FormGroup, + ɵElement, +} from "@angular/forms"; import { ColumnConfig, FormFieldConfig, toFormFieldConfig } from "./FormConfig"; import { Entity, EntityConstructor } from "../../entity/model/entity"; import { EntityMapperService } from "../../entity/entity-mapper/entity-mapper.service"; @@ -157,7 +163,7 @@ export class EntityFormService { * @private */ private addFormControlConfig( - formConfig: Object, + formConfig: { [key: string]: FormControl }, fieldConfig: ColumnConfig, entity: Entity, forTable: boolean, @@ -176,14 +182,16 @@ export class EntityFormService { ) { value = this.getDefaultValue(field); } - formConfig[field.id] = [value]; + const controlOptions: FormControlOptions = { nonNullable: true }; if (field.validators) { const validators = this.dynamicValidator.buildValidators( field.validators, ); - formConfig[field.id].push(validators); + Object.assign(controlOptions, validators); } + + formConfig[field.id] = new FormControl(value, controlOptions); } private getDefaultValue(schema: EntitySchemaField) { diff --git a/src/app/core/common-components/entity-form/unique-id-validator.ts b/src/app/core/common-components/entity-form/unique-id-validator.ts deleted file mode 100644 index 5a992bb85d..0000000000 --- a/src/app/core/common-components/entity-form/unique-id-validator.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { AbstractControl, ValidationErrors, ValidatorFn } from "@angular/forms"; - -export function uniqueIdValidator(existingIds: string[]): ValidatorFn { - return (control: AbstractControl): ValidationErrors | null => { - const value = control.value; - - if (existingIds.some((id) => id === value)) { - return { - uniqueId: $localize`:form field validation error:id already in use`, - }; - } - - return null; - }; -} diff --git a/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.spec.ts b/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.spec.ts new file mode 100644 index 0000000000..693801a199 --- /dev/null +++ b/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.spec.ts @@ -0,0 +1,41 @@ +import { waitForAsync } from "@angular/core/testing"; +import { uniqueIdValidator } from "./unique-id-validator"; +import { FormControl, ValidatorFn } from "@angular/forms"; + +describe("UniqueIdValidator", () => { + let validator: ValidatorFn; + + let demoIds; + let formControl: FormControl; + + beforeEach(waitForAsync(() => { + demoIds = ["id1", "id2", "id3"]; + formControl = new FormControl(); + validator = uniqueIdValidator(demoIds); + })); + + it("should validate new id", async () => { + formControl.setValue("new id"); + formControl.markAsDirty(); + const validationResult = await validator(formControl); + + expect(validationResult).toBeNull(); + }); + + it("should disallow already existing id", async () => { + formControl.setValue(demoIds[1]); + formControl.markAsDirty(); + const validationResult = await validator(formControl); + + expect(validationResult).toEqual({ uniqueId: jasmine.any(String) }); + }); + + it("should allow to keep unchanged value (to not refuse saving an existing entity with unchanged id)", async () => { + formControl = new FormControl(demoIds[1], { nonNullable: true }); + formControl.markAsDirty(); + const validationResult = await validator(formControl); + + expect(formControl.pristine).toBeFalse(); + expect(validationResult).toBeNull(); + }); +}); diff --git a/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.ts b/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.ts new file mode 100644 index 0000000000..90958b852c --- /dev/null +++ b/src/app/core/common-components/entity-form/unique-id-validator/unique-id-validator.ts @@ -0,0 +1,25 @@ +import { FormControl, ValidationErrors } from "@angular/forms"; +import { AsyncPromiseValidatorFn } from "../dynamic-form-validators/dynamic-validators.service"; + +export function uniqueIdValidator( + existingIds: string[] | (() => Promise), +): AsyncPromiseValidatorFn { + return async (control: FormControl): Promise => { + if (control.value === control.defaultValue) { + return null; + } + + const existingValues = Array.isArray(existingIds) + ? existingIds + : await existingIds(); + const value = control.value; + + if (existingValues.some((id) => id === value)) { + return { + uniqueId: $localize`:form field validation error:id already in use`, + }; + } + + return null; + }; +} diff --git a/src/app/core/common-components/error-hint/error-hint.component.html b/src/app/core/common-components/error-hint/error-hint.component.html index aeda1c0cea..3bd4b1147a 100644 --- a/src/app/core/common-components/error-hint/error-hint.component.html +++ b/src/app/core/common-components/error-hint/error-hint.component.html @@ -1,4 +1,4 @@ -
- {{ validatorService.descriptionForValidator(err.key, err.value) }} +
+ {{ err.value["errorMessage"] }}
diff --git a/src/app/core/common-components/error-hint/error-hint.component.ts b/src/app/core/common-components/error-hint/error-hint.component.ts index d1c7007a97..3612566ea0 100644 --- a/src/app/core/common-components/error-hint/error-hint.component.ts +++ b/src/app/core/common-components/error-hint/error-hint.component.ts @@ -1,6 +1,5 @@ import { Component, Input } from "@angular/core"; import { UntypedFormControl } from "@angular/forms"; -import { DynamicValidatorsService } from "../entity-form/dynamic-form-validators/dynamic-validators.service"; import { KeyValuePipe, NgForOf } from "@angular/common"; @Component({ @@ -12,6 +11,4 @@ import { KeyValuePipe, NgForOf } from "@angular/common"; }) export class ErrorHintComponent { @Input() form: UntypedFormControl; - - constructor(public validatorService: DynamicValidatorsService) {} } diff --git a/src/app/core/form-dialog/row-details/row-details.component.ts b/src/app/core/form-dialog/row-details/row-details.component.ts index dcebe47d03..2c47908c6e 100644 --- a/src/app/core/form-dialog/row-details/row-details.component.ts +++ b/src/app/core/form-dialog/row-details/row-details.component.ts @@ -65,6 +65,10 @@ export class RowDetailsComponent { @Inject(MAT_DIALOG_DATA) public data: DetailsComponentData, private formService: EntityFormService, ) { + this.init(data); + } + + private init(data: DetailsComponentData) { this.form = this.formService.createFormGroup(data.columns, data.entity); this.enableSaveWithoutChangesIfNew(data.entity); diff --git a/src/app/core/user/user.ts b/src/app/core/user/user.ts index 4c390db113..83d5e4538d 100644 --- a/src/app/core/user/user.ts +++ b/src/app/core/user/user.ts @@ -37,7 +37,7 @@ export class User extends Entity { /** username used for login and identification */ @DatabaseField({ label: $localize`:Label of username:Username`, - validators: { required: true }, + validators: { required: true, uniqueId: "User" }, }) set name(value: string) { if (this._name && value !== this._name) {