117287: Removed method calls returning observables from the EPerson form

src/app/access-control/epeople-registry/epeople-registry.component.html

@@ -66,7 +66,7 @@
               </thead>
               <tbody>
               <tr *ngFor="let epersonDto of (ePeopleDto$ | async)?.page"
-                  [ngClass]="{'table-primary' : isActive(epersonDto.eperson) | async}">
+                  [ngClass]="{'table-primary' : (activeEPerson$ | async) === epersonDto.eperson}">
                 <td>{{epersonDto.eperson.id}}</td>
                 <td>{{ dsoNameService.getName(epersonDto.eperson) }}</td>
                 <td>{{epersonDto.eperson.email}}</td>

src/app/access-control/epeople-registry/epeople-registry.component.ts

@@ -45,6 +45,8 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
    */
   ePeopleDto$: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject<PaginatedList<EpersonDtoModel>>({} as any);
 
+  activeEPerson$: Observable<EPerson>;
+
   /**
    * An observable for the pageInfo, needed to pass to the pagination component
    */
@@ -121,6 +123,7 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
         this.isEPersonFormShown = true;
       }
     }));
+    this.activeEPerson$ = this.epersonService.getActiveEPerson();
     this.subs.push(this.ePeople$.pipe(
       switchMap((epeople: PaginatedList<EPerson>) => {
         if (epeople.pageInfo.totalElements > 0) {
@@ -188,29 +191,12 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
     );
   }
 
-  /**
-   * Checks whether the given EPerson is active (being edited)
-   * @param eperson
-   */
-  isActive(eperson: EPerson): Observable<boolean> {
-    return this.getActiveEPerson().pipe(
-      map((activeEPerson) => eperson === activeEPerson)
-    );
-  }
-
-  /**
-   * Gets the active eperson (being edited)
-   */
-  getActiveEPerson(): Observable<EPerson> {
-    return this.epersonService.getActiveEPerson();
-  }
-
   /**
    * Start editing the selected EPerson
    * @param ePerson
    */
   toggleEditEPerson(ePerson: EPerson) {
-    this.getActiveEPerson().pipe(take(1)).subscribe((activeEPerson: EPerson) => {
+    this.activeEPerson$.pipe(take(1)).subscribe((activeEPerson: EPerson) => {
       if (ePerson === activeEPerson) {
         this.epersonService.cancelEditEPerson();
         this.isEPersonFormShown = false;

src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html

@@ -1,4 +1,4 @@
-<div *ngIf="epersonService.getActiveEPerson() | async; then editheader; else createHeader"></div>
+<div *ngIf="activeEPerson$ | async; then editheader; else createHeader"></div>
 
 <ng-template #createHeader>
   <h4>{{messagePrefix + '.create' | translate}}</h4>
@@ -39,7 +39,7 @@
 
 <ds-themed-loading [showMessage]="false" *ngIf="!formGroup"></ds-themed-loading>
 
-<div *ngIf="epersonService.getActiveEPerson() | async">
+<div *ngIf="activeEPerson$ | async">
   <h5>{{messagePrefix + '.groupsEPersonIsMemberOf' | translate}}</h5>
 
   <ds-themed-loading [showMessage]="false" *ngIf="!(groups | async)"></ds-themed-loading>

src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts

@@ -1,11 +1,11 @@
 import { Observable, of as observableOf } from 'rxjs';
 import { CommonModule } from '@angular/common';
-import { NO_ERRORS_SCHEMA } from '@angular/core';
+import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { UntypedFormControl, UntypedFormGroup, FormsModule, ReactiveFormsModule, Validators } from '@angular/forms';
 import { BrowserModule, By } from '@angular/platform-browser';
 import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
-import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
+import { TranslateModule } from '@ngx-translate/core';
 import { buildPaginatedList, PaginatedList } from '../../../core/data/paginated-list.model';
 import { RemoteData } from '../../../core/data/remote-data';
 import { EPersonDataService } from '../../../core/eperson/eperson-data.service';
@@ -19,7 +19,6 @@ import { EPersonMock, EPersonMock2 } from '../../../shared/testing/eperson.mock'
 import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
 import { getMockFormBuilderService } from '../../../shared/mocks/form-builder-service.mock';
 import { NotificationsServiceStub } from '../../../shared/testing/notifications-service.stub';
-import { TranslateLoaderMock } from '../../../shared/mocks/translate-loader.mock';
 import { AuthService } from '../../../core/auth/auth.service';
 import { AuthServiceStub } from '../../../shared/testing/auth-service.stub';
 import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service';
@@ -31,6 +30,7 @@ import { PaginationServiceStub } from '../../../shared/testing/pagination-servic
 import { FindListOptions } from '../../../core/data/find-list-options.model';
 import { ValidateEmailNotTaken } from './validators/email-taken.validator';
 import { EpersonRegistrationService } from '../../../core/data/eperson-registration.service';
+import { RouterTestingModule } from '@angular/router/testing';
 
 describe('EPersonFormComponent', () => {
   let component: EPersonFormComponent;
@@ -53,9 +53,6 @@ describe('EPersonFormComponent', () => {
     ePersonDataServiceStub = {
       activeEPerson: null,
       allEpeople: mockEPeople,
-      getEPeople(): Observable<RemoteData<PaginatedList<EPerson>>> {
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(null, this.allEpeople));
-      },
       getActiveEPerson(): Observable<EPerson> {
         return observableOf(this.activeEPerson);
       },
@@ -184,12 +181,8 @@ describe('EPersonFormComponent', () => {
     paginationService = new PaginationServiceStub();
     TestBed.configureTestingModule({
       imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
-        TranslateModule.forRoot({
+        RouterTestingModule,
-          loader: {
+        TranslateModule.forRoot(),
-            provide: TranslateLoader,
-            useClass: TranslateLoaderMock
-          }
-        }),
       ],
       declarations: [EPersonFormComponent],
       providers: [
@@ -204,7 +197,7 @@ describe('EPersonFormComponent', () => {
         { provide: EpersonRegistrationService, useValue: epersonRegistrationService },
         EPeopleRegistryComponent
       ],
-      schemas: [NO_ERRORS_SCHEMA]
+      schemas: [CUSTOM_ELEMENTS_SCHEMA],
     }).compileComponents();
   }));
 
@@ -223,37 +216,13 @@ describe('EPersonFormComponent', () => {
   });
 
   describe('check form validation', () => {
-    let firstName;
+    let canLogIn: boolean;
-    let lastName;
+    let requireCertificate: boolean;
-    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;
@@ -343,15 +312,13 @@ describe('EPersonFormComponent', () => {
         });
       }));
     });
-
-
-
   });
+
   describe('when submitting the form', () => {
     let firstName;
     let lastName;
     let email;
-    let canLogIn;
+    let canLogIn: boolean;
     let requireCertificate;
 
     let expected;
@@ -380,6 +347,7 @@ describe('EPersonFormComponent', () => {
         requireCertificate: requireCertificate,
       });
       spyOn(component.submitForm, 'emit');
+      component.ngOnInit();
       component.firstName.value = firstName;
       component.lastName.value = lastName;
       component.email.value = email;
@@ -421,9 +389,17 @@ describe('EPersonFormComponent', () => {
           email: email,
           canLogIn: canLogIn,
           requireCertificate: requireCertificate,
-          _links: undefined
+          _links: {
+            groups: {
+              href: '',
+            },
+            self: {
+              href: '',
+            },
+          },
         });
         spyOn(ePersonDataServiceStub, 'getActiveEPerson').and.returnValue(observableOf(expectedWithId));
+        component.ngOnInit();
         component.onSubmit();
         fixture.detectChanges();
       });
@@ -473,22 +449,19 @@ describe('EPersonFormComponent', () => {
   });
 
   describe('delete', () => {
-
-    let ePersonId;
     let eperson: EPerson;
     let modalService;
 
     beforeEach(() => {
       spyOn(authService, 'impersonate').and.callThrough();
-      ePersonId = 'testEPersonId';
       eperson = EPersonMock;
       component.epersonInitial = eperson;
       component.canDelete$ = observableOf(true);
       spyOn(component.epersonService, 'getActiveEPerson').and.returnValue(observableOf(eperson));
       modalService = (component as any).modalService;
       spyOn(modalService, 'open').and.returnValue(Object.assign({ componentInstance: Object.assign({ response: observableOf(true) }) }));
+      component.ngOnInit();
       fixture.detectChanges();
-
     });
 
     it('the delete button should be active if the eperson can be deleted', () => {

src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts

@@ -137,6 +137,11 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    */
   canImpersonate$: Observable<boolean>;
 
+  /**
+   * The current {@link EPerson}
+   */
+  activeEPerson$: Observable<EPerson>;
+
   /**
    * List of subscriptions
    */
@@ -195,7 +200,11 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
     private epersonRegistrationService: EpersonRegistrationService,
     public dsoNameService: DSONameService,
   ) {
-    this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => {
+  }
+
+  ngOnInit() {
+    this.activeEPerson$ = this.epersonService.getActiveEPerson();
+    this.subs.push(this.activeEPerson$.subscribe((eperson: EPerson) => {
       this.epersonInitial = eperson;
       if (hasValue(eperson)) {
         this.isImpersonated = this.authService.isImpersonatingUser(eperson.id);
@@ -203,9 +212,6 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
         this.submitLabel = 'form.submit';
       }
     }));
-  }
-
-  ngOnInit() {
     this.initialisePage();
   }
 
@@ -213,124 +219,112 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    * This method will initialise the page
    */
   initialisePage() {
-
+    this.firstName = new DynamicInputModel({
-    observableCombineLatest([
+      id: 'firstName',
-      this.translateService.get(`${this.messagePrefix}.firstName`),
+      label: this.translateService.instant(`${this.messagePrefix}.firstName`),
-      this.translateService.get(`${this.messagePrefix}.lastName`),
+      name: 'firstName',
-      this.translateService.get(`${this.messagePrefix}.email`),
+      validators: {
-      this.translateService.get(`${this.messagePrefix}.canLogIn`),
+        required: null,
-      this.translateService.get(`${this.messagePrefix}.requireCertificate`),
+      },
-      this.translateService.get(`${this.messagePrefix}.emailHint`),
+      required: true,
-    ]).subscribe(([firstName, lastName, email, canLogIn, requireCertificate, emailHint]) => {
-      this.firstName = new DynamicInputModel({
-        id: 'firstName',
-        label: firstName,
-        name: 'firstName',
-        validators: {
-          required: null,
-        },
-        required: true,
-      });
-      this.lastName = new DynamicInputModel({
-        id: 'lastName',
-        label: lastName,
-        name: 'lastName',
-        validators: {
-          required: null,
-        },
-        required: true,
-      });
-      this.email = new DynamicInputModel({
-        id: 'email',
-        label: email,
-        name: 'email',
-        validators: {
-          required: null,
-          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(
-        {
-          id: 'canLogIn',
-          label: canLogIn,
-          name: 'canLogIn',
-          value: (this.epersonInitial != null ? this.epersonInitial.canLogIn : true)
-        });
-      this.requireCertificate = new DynamicCheckboxModel(
-        {
-          id: 'requireCertificate',
-          label: requireCertificate,
-          name: 'requireCertificate',
-          value: (this.epersonInitial != null ? this.epersonInitial.requireCertificate : false)
-        });
-      this.formModel = [
-        this.firstName,
-        this.lastName,
-        this.email,
-        this.canLogIn,
-        this.requireCertificate,
-      ];
-      this.formGroup = this.formBuilderService.createFormGroup(this.formModel);
-      this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => {
-        if (eperson != null) {
-          this.groups = this.groupsDataService.findListByHref(eperson._links.groups.href, {
-            currentPage: 1,
-            elementsPerPage: this.config.pageSize
-          });
-        }
-        this.formGroup.patchValue({
-          firstName: eperson != null ? eperson.firstMetadataValue('eperson.firstname') : '',
-          lastName: eperson != null ? eperson.firstMetadataValue('eperson.lastname') : '',
-          email: eperson != null ? eperson.email : '',
-          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();
-
-      this.groups = activeEPerson$.pipe(
-        switchMap((eperson) => {
-          return observableCombineLatest([observableOf(eperson), this.paginationService.getFindListOptions(this.config.id, {
-            currentPage: 1,
-            elementsPerPage: this.config.pageSize
-          })]);
-        }),
-        switchMap(([eperson, findListOptions]) => {
-          if (eperson != null) {
-            return this.groupsDataService.findListByHref(eperson._links.groups.href, findListOptions, true, true, followLink('object'));
-          }
-          return observableOf(undefined);
-        })
-      );
-
-      this.canImpersonate$ = activeEPerson$.pipe(
-        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))
-      );
-      this.canReset$ = observableOf(true);
     });
+    this.lastName = new DynamicInputModel({
+      id: 'lastName',
+      label: this.translateService.instant(`${this.messagePrefix}.lastName`),
+      name: 'lastName',
+      validators: {
+        required: null,
+      },
+      required: true,
+    });
+    this.email = new DynamicInputModel({
+      id: 'email',
+      label: this.translateService.instant(`${this.messagePrefix}.email`),
+      name: 'email',
+      validators: {
+        required: null,
+        pattern: '^[a-z0-9._%+-]+@[a-z0-9.-]+\\.[a-z]{2,4}$',
+      },
+      required: true,
+      errorMessages: {
+        emailTaken: 'error.validation.emailTaken',
+        pattern: 'error.validation.NotValidEmail'
+      },
+      hint: this.translateService.instant(`${this.messagePrefix}.emailHint`),
+    });
+    this.canLogIn = new DynamicCheckboxModel(
+      {
+        id: 'canLogIn',
+        label: this.translateService.instant(`${this.messagePrefix}.canLogIn`),
+        name: 'canLogIn',
+        value: (this.epersonInitial != null ? this.epersonInitial.canLogIn : true)
+      });
+    this.requireCertificate = new DynamicCheckboxModel(
+      {
+        id: 'requireCertificate',
+        label: this.translateService.instant(`${this.messagePrefix}.requireCertificate`),
+        name: 'requireCertificate',
+        value: (this.epersonInitial != null ? this.epersonInitial.requireCertificate : false)
+      });
+    this.formModel = [
+      this.firstName,
+      this.lastName,
+      this.email,
+      this.canLogIn,
+      this.requireCertificate,
+    ];
+    this.formGroup = this.formBuilderService.createFormGroup(this.formModel);
+    this.subs.push(this.activeEPerson$.subscribe((eperson: EPerson) => {
+      if (eperson != null) {
+        this.groups = this.groupsDataService.findListByHref(eperson._links.groups.href, {
+          currentPage: 1,
+          elementsPerPage: this.config.pageSize
+        });
+      }
+      this.formGroup.patchValue({
+        firstName: eperson != null ? eperson.firstMetadataValue('eperson.firstname') : '',
+        lastName: eperson != null ? eperson.firstMetadataValue('eperson.lastname') : '',
+        email: eperson != null ? eperson.email : '',
+        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();
+        });
+      }
+    }));
+
+    this.groups = this.activeEPerson$.pipe(
+      switchMap((eperson) => {
+        return observableCombineLatest([observableOf(eperson), this.paginationService.getFindListOptions(this.config.id, {
+          currentPage: 1,
+          elementsPerPage: this.config.pageSize
+        })]);
+      }),
+      switchMap(([eperson, findListOptions]) => {
+        if (eperson != null) {
+          return this.groupsDataService.findListByHref(eperson._links.groups.href, findListOptions, true, true, followLink('object'));
+        }
+        return observableOf(undefined);
+      })
+    );
+
+    this.canImpersonate$ = this.activeEPerson$.pipe(
+      switchMap((eperson) => {
+        if (hasValue(eperson)) {
+          return this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, eperson.self);
+        } else {
+          return observableOf(false);
+        }
+      })
+    );
+    this.canDelete$ = this.activeEPerson$.pipe(
+      switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(eperson) ? eperson.self : undefined))
+    );
+    this.canReset$ = observableOf(true);
   }
 
   /**
@@ -348,7 +342,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    * Emit the updated/created eperson using the EventEmitter submitForm
    */
   onSubmit() {
-    this.epersonService.getActiveEPerson().pipe(take(1)).subscribe(
+    this.activeEPerson$.pipe(take(1)).subscribe(
       (ePerson: EPerson) => {
         const values = {
           metadata: {
@@ -464,7 +458,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    * It'll either show a success or error message depending on whether the delete was successful or not.
    */
   delete(): void {
-    this.epersonService.getActiveEPerson().pipe(
+    this.activeEPerson$.pipe(
       take(1),
       switchMap((eperson: EPerson) => {
         const modalRef = this.modalService.open(ConfirmationModalComponent);
@@ -545,7 +539,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
    * This method will ensure that the page gets reset and that the cache is cleared
    */
   reset() {
-    this.epersonService.getActiveEPerson().pipe(take(1)).subscribe((eperson: EPerson) => {
+    this.activeEPerson$.pipe(take(1)).subscribe((eperson: EPerson) => {
       this.requestService.removeByHrefSubstring(eperson.self);
     });
     this.initialisePage();
@@ -577,7 +571,7 @@ 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.subs.push(this.activeEPerson$.subscribe((eperson: EPerson) => {
       this.groups = this.groupsDataService.findListByHref(eperson._links.groups.href, options);
     }));
   }