Merge pull request #1336 from 4Science/CST-4634-Many-notifications-when-trying-to-create-an-eperson-with-already-existing-email

Added custom validator and mail check during eperson registration
2025-10-07 01:54:15 +00:00 · 2021-10-15 12:22:14 -05:00
parent d778f98b0d bd02bcd0de
commit 66b55bf910
5 changed files with 361 additions and 92 deletions
--- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts
+++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts
@@ -28,6 +28,9 @@ import { createPaginatedList } from '../../../shared/testing/utils.test';
 import { RequestService } from '../../../core/data/request.service';
 import { PaginationService } from '../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../shared/testing/pagination-service.stub';
+import { FormArray, FormControl, FormGroup,Validators, NG_VALIDATORS, NG_ASYNC_VALIDATORS } from '@angular/forms';
+import { ValidateEmailNotTaken } from './validators/email-taken.validator';
+

 describe('EPersonFormComponent', () => {
  let component: EPersonFormComponent;
@@ -99,12 +102,78 @@ describe('EPersonFormComponent', () => {
          }
        });
        return createSuccessfulRemoteDataObject$(ePerson);
+      },
+      getEPersonByEmail(email): Observable<RemoteData<EPerson>> {
+        return createSuccessfulRemoteDataObject$(null);
      }
    };
-    builderService = getMockFormBuilderService();
+    builderService = Object.assign(getMockFormBuilderService(),{
+      createFormGroup(formModel, options = null) {
+        const controls = {};
+        formModel.forEach( model => {
+            model.parent = parent;
+            const controlModel = model;
+            const controlState = { value: controlModel.value, disabled: controlModel.disabled };
+            const controlOptions = this.createAbstractControlOptions(controlModel.validators, controlModel.asyncValidators, controlModel.updateOn);
+            controls[model.id] = new FormControl(controlState, controlOptions);
+        });
+        return new FormGroup(controls, options);
+      },
+      createAbstractControlOptions(validatorsConfig = null, asyncValidatorsConfig = null, updateOn = null) {
+        return {
+            validators: validatorsConfig !== null ? this.getValidators(validatorsConfig) : null,
+        };
+      },
+      getValidators(validatorsConfig) {
+          return this.getValidatorFns(validatorsConfig);
+      },
+      getValidatorFns(validatorsConfig, validatorsToken = this._NG_VALIDATORS) {
+        let validatorFns = [];
+        if (this.isObject(validatorsConfig)) {
+            validatorFns = Object.keys(validatorsConfig).map(validatorConfigKey => {
+                const validatorConfigValue = validatorsConfig[validatorConfigKey];
+                if (this.isValidatorDescriptor(validatorConfigValue)) {
+                    const descriptor = validatorConfigValue;
+                    return this.getValidatorFn(descriptor.name, descriptor.args, validatorsToken);
+                }
+                return this.getValidatorFn(validatorConfigKey, validatorConfigValue, validatorsToken);
+            });
+        }
+        return validatorFns;
+      },
+      getValidatorFn(validatorName, validatorArgs = null, validatorsToken = this._NG_VALIDATORS) {
+        let validatorFn;
+        if (Validators.hasOwnProperty(validatorName)) { // Built-in Angular Validators
+            validatorFn = Validators[validatorName];
+        } else { // Custom Validators
+            if (this._DYNAMIC_VALIDATORS && this._DYNAMIC_VALIDATORS.has(validatorName)) {
+                validatorFn = this._DYNAMIC_VALIDATORS.get(validatorName);
+            } else if (validatorsToken) {
+                validatorFn = validatorsToken.find(validator => validator.name === validatorName);
+            }
+        }
+        if (validatorFn === undefined) { // throw when no validator could be resolved
+            throw new Error(`validator '${validatorName}' is not provided via NG_VALIDATORS, NG_ASYNC_VALIDATORS or DYNAMIC_FORM_VALIDATORS`);
+        }
+        if (validatorArgs !== null) {
+            return validatorFn(validatorArgs);
+        }
+        return validatorFn;
+    },
+      isValidatorDescriptor(value) {
+          if (this.isObject(value)) {
+              return value.hasOwnProperty('name') && value.hasOwnProperty('args');
+          }
+          return false;
+      },
+      isObject(value) {
+        return typeof value === 'object' && value !== null;
+      }
+    });
    authService = new AuthServiceStub();
    authorizationService = jasmine.createSpyObj('authorizationService', {
-      isAuthorized: observableOf(true)
+      isAuthorized: observableOf(true),
+
    });
    groupsDataService = jasmine.createSpyObj('groupsDataService', {
      findAllByHref: createSuccessfulRemoteDataObject$(createPaginatedList([])),
@@ -146,6 +215,131 @@ describe('EPersonFormComponent', () => {
    expect(component).toBeDefined();
  });

+  describe('check form validation', () => {
+    let firstName;
+    let lastName;
+    let email;
+    let canLogIn;
+    let requireCertificate;
+
+    let expected;
+    beforeEach(() => {
+      firstName = 'testName';
+      lastName = 'testLastName';
+      email = 'testEmail@test.com';
+      canLogIn = false;
+      requireCertificate = false;
+
+      expected = Object.assign(new EPerson(), {
+        metadata: {
+          'eperson.firstname': [
+            {
+              value: firstName
+            }
+          ],
+          'eperson.lastname': [
+            {
+              value: lastName
+            },
+          ],
+        },
+        email: email,
+        canLogIn: canLogIn,
+        requireCertificate: requireCertificate,
+      });
+      spyOn(component.submitForm, 'emit');
+      component.canLogIn.value = canLogIn;
+      component.requireCertificate.value = requireCertificate;
+
+      fixture.detectChanges();
+      component.initialisePage();
+      fixture.detectChanges();
+    });
+    describe('firstName, lastName and email should be required', () => {
+      it('form should be invalid because the firstName is required', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.firstName.valid).toBeFalse();
+          expect(component.formGroup.controls.firstName.errors.required).toBeTrue();
+        });
+      }));
+      it('form should be invalid because the lastName is required', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.lastName.valid).toBeFalse();
+          expect(component.formGroup.controls.lastName.errors.required).toBeTrue();
+        });
+      }));
+      it('form should be invalid because the email is required', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.email.valid).toBeFalse();
+          expect(component.formGroup.controls.email.errors.required).toBeTrue();
+        });
+      }));
+    });
+
+    describe('after inserting information firstName,lastName and email not required', () => {
+      beforeEach(() => {
+        component.formGroup.controls.firstName.setValue('test');
+        component.formGroup.controls.lastName.setValue('test');
+        component.formGroup.controls.email.setValue('test@test.com');
+        fixture.detectChanges();
+      });
+      it('firstName should be valid because the firstName is set', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.firstName.valid).toBeTrue();
+          expect(component.formGroup.controls.firstName.errors).toBeNull();
+        });
+      }));
+      it('lastName should be valid because the lastName is set', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.lastName.valid).toBeTrue();
+          expect(component.formGroup.controls.lastName.errors).toBeNull();
+        });
+      }));
+      it('email should be valid because the email is set', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.email.valid).toBeTrue();
+          expect(component.formGroup.controls.email.errors).toBeNull();
+        });
+      }));
+    });
+
+
+    describe('after inserting email wrong should show pattern validation error', () => {
+      beforeEach(() => {
+        component.formGroup.controls.email.setValue('test@test');
+        fixture.detectChanges();
+      });
+      it('email should not be valid because the email pattern', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.email.valid).toBeFalse();
+          expect(component.formGroup.controls.email.errors.pattern).toBeTruthy();
+        });
+      }));
+    });
+
+    describe('after already utilized email', () => {
+      beforeEach(() => {
+        const ePersonServiceWithEperson = Object.assign(ePersonDataServiceStub,{
+          getEPersonByEmail(): Observable<RemoteData<EPerson>> {
+            return createSuccessfulRemoteDataObject$(EPersonMock);
+          }
+        });
+        component.formGroup.controls.email.setValue('test@test.com');
+        component.formGroup.controls.email.setAsyncValidators(ValidateEmailNotTaken.createValidator(ePersonServiceWithEperson));
+        fixture.detectChanges();
+      });
+
+      it('email should not be valid because email is already taken', waitForAsync(() => {
+        fixture.whenStable().then(() => {
+          expect(component.formGroup.controls.email.valid).toBeFalse();
+          expect(component.formGroup.controls.email.errors.emailTaken).toBeTruthy();
+        });
+      }));
+    });
+
+
+
+  });
  describe('when submitting the form', () => {
    let firstName;
    let lastName;
--- a/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts
+++ b/src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts
@@ -1,4 +1,4 @@
-import { Component, EventEmitter, OnDestroy, OnInit, Output } from '@angular/core';
+import { ChangeDetectorRef, Component, EventEmitter, OnDestroy, OnInit, Output } from '@angular/core';
 import { FormGroup } from '@angular/forms';
 import {
  DynamicCheckboxModel,
@@ -8,7 +8,7 @@ import {
 } from '@ng-dynamic-forms/core';
 import { TranslateService } from '@ngx-translate/core';
 import { combineLatest as observableCombineLatest, Observable, of as observableOf, Subscription } from 'rxjs';
-import { switchMap, take } from 'rxjs/operators';
+import { debounceTime, switchMap, take } from 'rxjs/operators';
 import { PaginatedList } from '../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../core/data/remote-data';
 import { EPersonDataService } from '../../../core/eperson/eperson-data.service';
@@ -33,10 +33,11 @@ import { RequestService } from '../../../core/data/request.service';
 import { NoContent } from '../../../core/shared/NoContent.model';
 import { PaginationService } from '../../../core/pagination/pagination.service';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
+import { ValidateEmailNotTaken } from './validators/email-taken.validator';

@Component({
  selector: 'ds-eperson-form',
-  templateUrl: './eperson-form.component.html'
+  templateUrl: './eperson-form.component.html',
 })
 /**
 * A form used for creating and editing EPeople
@@ -161,7 +162,13 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
   */
  isImpersonated = false;

-  constructor(public epersonService: EPersonDataService,
+  /**
+   * Subscription to email field value change
+   */
+  emailValueChangeSubscribe: Subscription;
+
+  constructor(protected changeDetectorRef: ChangeDetectorRef,
+              public epersonService: EPersonDataService,
              public groupsDataService: GroupDataService,
              private formBuilderService: FormBuilderService,
              private translateService: TranslateService,
@@ -187,6 +194,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
   * This method will initialise the page
   */
  initialisePage() {
+
    observableCombineLatest(
      this.translateService.get(`${this.messagePrefix}.firstName`),
      this.translateService.get(`${this.messagePrefix}.lastName`),
@@ -219,9 +227,13 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
        name: 'email',
        validators: {
          required: null,
-          pattern: '^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,4}$'
+          pattern: '^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,4}$',
        },
        required: true,
+        errorMessages: {
+          emailTaken: 'error.validation.emailTaken',
+          pattern: 'error.validation.NotValidEmail'
+        },
        hint: emailHint
      });
      this.canLogIn = new DynamicCheckboxModel(
@@ -260,6 +272,13 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
          canLogIn: eperson != null ? eperson.canLogIn : true,
          requireCertificate: eperson != null ? eperson.requireCertificate : false
        });
+
+        if (eperson === null && !!this.formGroup.controls.email) {
+          this.formGroup.controls.email.setAsyncValidators(ValidateEmailNotTaken.createValidator(this.epersonService));
+          this.emailValueChangeSubscribe = this.email.valueChanges.pipe(debounceTime(300)).subscribe(() => {
+            this.changeDetectorRef.detectChanges();
+          });
+        }
      }));

      const activeEPerson$ = this.epersonService.getActiveEPerson();
@@ -280,7 +299,13 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
      );

      this.canImpersonate$ = activeEPerson$.pipe(
-        switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined))
+        switchMap((eperson) => {
+          if (hasValue(eperson)) {
+            return this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, eperson.self);
+          } else {
+            return observableOf(false);
+          }
+        })
      );
      this.canDelete$ = activeEPerson$.pipe(
        switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(eperson) ? eperson.self : undefined))
@@ -343,10 +368,10 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
      getFirstCompletedRemoteData()
    ).subscribe((rd: RemoteData<EPerson>) => {
      if (rd.hasSucceeded) {
-        this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.created.success', {name: ePersonToCreate.name}));
+        this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.created.success', { name: ePersonToCreate.name }));
        this.submitForm.emit(ePersonToCreate);
      } else {
-        this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.created.failure', {name: ePersonToCreate.name}));
+        this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.created.failure', { name: ePersonToCreate.name }));
        this.cancelForm.emit();
      }
    });
@@ -382,10 +407,10 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    const response = this.epersonService.updateEPerson(editedEperson);
    response.pipe(getFirstCompletedRemoteData()).subscribe((rd: RemoteData<EPerson>) => {
      if (rd.hasSucceeded) {
-        this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.edited.success', {name: editedEperson.name}));
+        this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.edited.success', { name: editedEperson.name }));
        this.submitForm.emit(editedEperson);
      } else {
-        this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.edited.failure', {name: editedEperson.name}));
+        this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.edited.failure', { name: editedEperson.name }));
        this.cancelForm.emit();
      }
    });
@@ -395,28 +420,6 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    }
  }

-  /**
-   * Checks for the given ePerson if there is already an ePerson in the system with that email
-   * and shows notification if this is the case
-   * @param ePerson               ePerson values to check
-   * @param notificationSection   whether in create or edit
-   */
-  private showNotificationIfEmailInUse(ePerson: EPerson, notificationSection: string) {
-    // Relevant message for email in use
-    this.subs.push(this.epersonService.searchByScope('email', ePerson.email, {
-      currentPage: 1,
-      elementsPerPage: 0
-    }).pipe(getFirstSucceededRemoteData(), getRemoteDataPayload())
-      .subscribe((list: PaginatedList<EPerson>) => {
-        if (list.totalElements > 0) {
-          this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.' + notificationSection + '.failure.emailInUse', {
-            name: ePerson.name,
-            email: ePerson.email
-          }));
-        }
-      }));
-  }
-
  /**
   * Event triggered when the user changes page
   * @param event
@@ -428,15 +431,6 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    });
  }

-  /**
-   * Update the list of groups by fetching it from the rest api or cache
-   */
-  private updateGroups(options) {
-    this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => {
-      this.groups = this.groupsDataService.findAllByHref(eperson._links.groups.href, options);
-    }));
-  }
-
  /**
   * Start impersonating the EPerson
   */
@@ -471,7 +465,8 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
              }
              this.cancelForm.emit();
            });
-            }}
+          }
+        }
      });
    });
  }
@@ -491,8 +486,10 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    this.onCancel();
    this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe());
    this.paginationService.clearPagination(this.config.id);
+    if (hasValue(this.emailValueChangeSubscribe)) {
+      this.emailValueChangeSubscribe.unsubscribe();
+    }
  }
-

  /**
   * This method will ensure that the page gets reset and that the cache is cleared
@@ -503,4 +500,35 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    });
    this.initialisePage();
  }
+
+  /**
+   * Checks for the given ePerson if there is already an ePerson in the system with that email
+   * and shows notification if this is the case
+   * @param ePerson               ePerson values to check
+   * @param notificationSection   whether in create or edit
+   */
+  private showNotificationIfEmailInUse(ePerson: EPerson, notificationSection: string) {
+    // Relevant message for email in use
+    this.subs.push(this.epersonService.searchByScope('email', ePerson.email, {
+      currentPage: 1,
+      elementsPerPage: 0
+    }).pipe(getFirstSucceededRemoteData(), getRemoteDataPayload())
+      .subscribe((list: PaginatedList<EPerson>) => {
+        if (list.totalElements > 0) {
+          this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.' + notificationSection + '.failure.emailInUse', {
+            name: ePerson.name,
+            email: ePerson.email
+          }));
+        }
+      }));
+  }
+
+  /**
+   * Update the list of groups by fetching it from the rest api or cache
+   */
+  private updateGroups(options) {
+    this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => {
+      this.groups = this.groupsDataService.findAllByHref(eperson._links.groups.href, options);
+    }));
+  }
 }
--- a/src/app/access-control/epeople-registry/eperson-form/validators/email-taken.validator.ts
+++ b/src/app/access-control/epeople-registry/eperson-form/validators/email-taken.validator.ts
@@ -0,0 +1,25 @@
+import { AbstractControl, ValidationErrors } from '@angular/forms';
+import { Observable } from 'rxjs';
+import { map } from 'rxjs/operators';
+
+import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
+import { getFirstSucceededRemoteData, } from '../../../../core/shared/operators';
+
+export class ValidateEmailNotTaken {
+
+  /**
+   * This method will create the validator with the ePersonDataService requested from component
+   * @param ePersonDataService the service with DI in the component that this validator is being utilized.
+   */
+  static createValidator(ePersonDataService: EPersonDataService) {
+    return (control: AbstractControl): Promise<ValidationErrors | null> | Observable<ValidationErrors | null> => {
+      return ePersonDataService.getEPersonByEmail(control.value)
+        .pipe(
+          getFirstSucceededRemoteData(),
+          map(res => {
+            return !!res.payload ? { emailTaken: true } : null;
+          })
+        );
+    };
+  }
+}
--- a/src/app/app.module.ts
+++ b/src/app/app.module.ts
@@ -7,7 +7,11 @@ import { EffectsModule } from '@ngrx/effects';
 import { RouterStateSerializer, StoreRouterConnectingModule } from '@ngrx/router-store';
 import { MetaReducer, Store, StoreModule, USER_PROVIDED_META_REDUCERS } from '@ngrx/store';
 import { StoreDevtoolsModule } from '@ngrx/store-devtools';
-import { DYNAMIC_MATCHER_PROVIDERS } from '@ng-dynamic-forms/core';
+import {
+  DYNAMIC_ERROR_MESSAGES_MATCHER,
+  DYNAMIC_MATCHER_PROVIDERS,
+  DynamicErrorMessagesMatcher
+} from '@ng-dynamic-forms/core';
 import { TranslateModule } from '@ngx-translate/core';
 import { ScrollToModule } from '@nicky-lenaers/ngx-scroll-to';

@@ -52,6 +56,7 @@ import { IdleModalComponent } from './shared/idle-modal/idle-modal.component';

 import { UUIDService } from './core/shared/uuid.service';
 import { CookieService } from './core/services/cookie.service';
+import { AbstractControl } from '@angular/forms';

 export function getBase() {
  return environment.ui.nameSpace;
@@ -61,6 +66,14 @@ export function getMetaReducers(): MetaReducer<AppState>[] {
  return environment.debug ? [...appMetaReducers, ...debugMetaReducers] : appMetaReducers;
 }

+/**
+ * Condition for displaying error messages on email form field
+ */
+export const ValidateEmailErrorStateMatcher: DynamicErrorMessagesMatcher =
+  (control: AbstractControl, model: any, hasFocus: boolean) => {
+    return (control.touched && !hasFocus) || (control.errors?.emailTaken && hasFocus);
+  };
+
 const IMPORTS = [
  CommonModule,
  SharedModule,
@@ -146,6 +159,10 @@ const PROVIDERS = [
   multi: true,
   deps: [ CookieService, UUIDService ]
  },
+  {
+    provide: DYNAMIC_ERROR_MESSAGES_MATCHER,
+    useValue: ValidateEmailErrorStateMatcher
+  },
  ...DYNAMIC_MATCHER_PROVIDERS,
 ];

--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -1290,6 +1290,11 @@

  "error.validation.filerequired": "The file upload is mandatory",

+  "error.validation.required": "This field is required",
+
+  "error.validation.NotValidEmail": "This E-mail is not a valid email",
+
+  "error.validation.emailTaken": "This E-mail is already taken",


  "file-section.error.header": "Error obtaining files for this item",