Also remove unnecessary EpersonDtoModel from extending ReviewersListComponent. Remove "memberOfGroup" from EpersonDtoModel as it is no longer used

src/app/access-control/group-registry/group-form/members-list/members-list.component.ts

@@ -29,8 +29,8 @@ import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
  */
 enum SubKey {
   ActiveGroup,
-  MembersDTO,
-  SearchResultsDTO,
+  Members,
+  SearchResults,
 }
 
 /**
@@ -166,8 +166,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @private
    */
   retrieveMembers(page: number): void {
-    this.unsubFrom(SubKey.MembersDTO);
-    this.subs.set(SubKey.MembersDTO,
+    this.unsubFrom(SubKey.Members);
+    this.subs.set(SubKey.Members,
       this.paginationService.getCurrentPagination(this.config.id, this.config).pipe(
         switchMap((currentPagination) => {
           return this.ePersonDataService.findListByHref(this.groupBeingEdited._links.epersons.href, {
@@ -239,8 +239,8 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param data  Contains scope and query param
    */
   search(data: any) {
-    this.unsubFrom(SubKey.SearchResultsDTO);
-    this.subs.set(SubKey.SearchResultsDTO,
+    this.unsubFrom(SubKey.SearchResults);
+    this.subs.set(SubKey.SearchResults,
       this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
         switchMap((paginationOptions) => {
 

src/app/core/eperson/models/eperson-dto.model.ts

@@ -13,9 +13,4 @@ export class EpersonDtoModel {
      * Whether or not the linked EPerson is able to be deleted
      */
     public ableToDelete: boolean;
-    /**
-     * Whether or not this EPerson is member of group on page it is being used on
-     */
-    public memberOfGroup: boolean;
-
 }

src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.spec.ts

@@ -17,7 +17,7 @@ import { Group } from '../../../../core/eperson/models/group.model';
 import { PageInfo } from '../../../../core/shared/page-info.model';
 import { FormBuilderService } from '../../../../shared/form/builder/form-builder.service';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
-import { GroupMock, GroupMock2 } from '../../../../shared/testing/group-mock';
+import { GroupMock } from '../../../../shared/testing/group-mock';
 import { ReviewersListComponent } from './reviewers-list.component';
 import { EPersonMock, EPersonMock2 } from '../../../../shared/testing/eperson.mock';
 import {
@@ -31,8 +31,10 @@ import { NotificationsServiceStub } from '../../../../shared/testing/notificatio
 import { RouterMock } from '../../../../shared/mocks/router.mock';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 
+// NOTE: Because ReviewersListComponent extends MembersListComponent, the below tests ONLY validate
+// features which are *unique* to ReviewersListComponent. All other features are tested in the
+// members-list.component.spec.ts file.
 describe('ReviewersListComponent', () => {
   let component: ReviewersListComponent;
   let fixture: ComponentFixture<ReviewersListComponent>;
@@ -40,31 +42,27 @@ describe('ReviewersListComponent', () => {
   let builderService: FormBuilderService;
   let ePersonDataServiceStub: any;
   let groupsDataServiceStub: any;
-  let activeGroup;
-  let allEPersons;
-  let allGroups;
-  let epersonMembers;
-  let subgroupMembers;
+  let activeGroup: Group;
+  let epersonMembers: EPerson[];
+  let epersonNonMembers: EPerson[];
   let paginationService;
-  let ePersonDtoModel1: EpersonDtoModel;
-  let ePersonDtoModel2: EpersonDtoModel;
 
   beforeEach(waitForAsync(() => {
     activeGroup = GroupMock;
     epersonMembers = [EPersonMock2];
-    subgroupMembers = [GroupMock2];
-    allEPersons = [EPersonMock, EPersonMock2];
-    allGroups = [GroupMock, GroupMock2];
+    epersonNonMembers = [EPersonMock];
     ePersonDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
+      epersonNonMembers: epersonNonMembers,
+      // This method is used to get all the current members
       findListByHref(_href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
       },
+      // This method is used to search across *non-members*
       searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
         if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allEPersons));
+          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), epersonNonMembers));
         }
         return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
       },
@@ -81,22 +79,22 @@ describe('ReviewersListComponent', () => {
     groupsDataServiceStub = {
       activeGroup: activeGroup,
       epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
-      allGroups: allGroups,
+      epersonNonMembers: epersonNonMembers,
       getActiveGroup(): Observable<Group> {
         return observableOf(activeGroup);
       },
       getEPersonMembers() {
         return this.epersonMembers;
       },
-      searchGroups(query: string): Observable<RemoteData<PaginatedList<Group>>> {
-        if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), this.allGroups));
+      addMemberToGroup(parentGroup, epersonToAdd: EPerson): Observable<RestResponse> {
+        // Add eperson to list of members
+        this.epersonMembers = [...this.epersonMembers, epersonToAdd];
+        // Remove eperson from list of non-members
+        this.epersonNonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToAdd.id) {
+            this.epersonNonMembers.splice(index, 1);
           }
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-      },
-      addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
-        this.epersonMembers = [...this.epersonMembers, eperson];
+        });
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
       clearGroupsRequests() {
@@ -109,21 +107,20 @@ describe('ReviewersListComponent', () => {
         return '/access-control/groups/' + group.id;
       },
       deleteMemberFromGroup(parentGroup, epersonToDelete: EPerson): Observable<RestResponse> {
-        this.epersonMembers = this.epersonMembers.find((eperson: EPerson) => {
-          if (eperson.id !== epersonToDelete.id) {
-            return eperson;
+        // Remove eperson from list of members
+        this.epersonMembers.forEach( (eperson: EPerson, index: number) => {
+          if (eperson.id === epersonToDelete.id) {
+            this.epersonMembers.splice(index, 1);
           }
         });
-        if (this.epersonMembers === undefined) {
-          this.epersonMembers = [];
-        }
+        // Add eperson to list of non-members
+        this.epersonNonMembers = [...this.epersonNonMembers, epersonToDelete];
         return observableOf(new RestResponse(true, 200, 'Success'));
       },
+      // Used to find the currently active group
       findById(id: string) {
-        for (const group of allGroups) {
-          if (group.id === id) {
-            return createSuccessfulRemoteDataObject$(group);
-          }
+        if (activeGroup.id === id) {
+          return createSuccessfulRemoteDataObject$(activeGroup);
         }
         return createNoContentRemoteDataObject$();
       },
@@ -135,7 +132,7 @@ describe('ReviewersListComponent', () => {
     translateService = getMockTranslateService();
 
     paginationService = new PaginationServiceStub();
-    TestBed.configureTestingModule({
+    return TestBed.configureTestingModule({
       imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
         TranslateModule.forRoot({
           loader: {
@@ -169,12 +166,6 @@ describe('ReviewersListComponent', () => {
     fixture.debugElement.nativeElement.remove();
   }));
 
-  beforeEach(() => {
-    ePersonDtoModel1 = new EpersonDtoModel();
-    ePersonDtoModel1.eperson = EPersonMock;
-    ePersonDtoModel2 = new EpersonDtoModel();
-    ePersonDtoModel2.eperson = EPersonMock2;
-  });
 
   describe('when no group is selected', () => {
     beforeEach(() => {
@@ -218,34 +209,32 @@ describe('ReviewersListComponent', () => {
   it('should replace the value when a new member is added when multipleReviewers is false', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
     component.multipleReviewers = false;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.addMemberToGroup(ePersonDtoModel2);
+    component.addMemberToGroup(EPersonMock2);
 
-    expect(component.selectedReviewers).toEqual([ePersonDtoModel2]);
-    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([ePersonDtoModel2.eperson]);
+    expect(component.selectedReviewers).toEqual([EPersonMock2]);
+    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock2]);
   });
 
   it('should add the value when a new member is added when multipleReviewers is true', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
     component.multipleReviewers = true;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.addMemberToGroup(ePersonDtoModel2);
+    component.addMemberToGroup(EPersonMock2);
 
-    expect(component.selectedReviewers).toEqual([ePersonDtoModel1, ePersonDtoModel2]);
-    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([ePersonDtoModel1.eperson, ePersonDtoModel2.eperson]);
+    expect(component.selectedReviewers).toEqual([EPersonMock, EPersonMock2]);
+    expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([EPersonMock, EPersonMock2]);
   });
 
   it('should delete the member when present', () => {
     spyOn(component.selectedReviewersUpdated, 'emit');
-    ePersonDtoModel1.memberOfGroup = true;
-    component.selectedReviewers = [ePersonDtoModel1];
+    component.selectedReviewers = [EPersonMock];
 
-    component.deleteMemberFromGroup(ePersonDtoModel1);
+    component.deleteMemberFromGroup(EPersonMock);
 
     expect(component.selectedReviewers).toEqual([]);
-    expect(ePersonDtoModel1.memberOfGroup).toBeFalse();
     expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([]);
   });
 

src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/reviewers-list/reviewers-list.component.ts

@@ -8,10 +8,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { EPerson } from '../../../../core/eperson/models/eperson.model';
-import { Observable, of as observableOf } from 'rxjs';
-import { hasValue } from '../../../../shared/empty.util';
 import { PaginatedList } from '../../../../core/data/paginated-list.model';
 import {
   MembersListComponent,
@@ -24,8 +21,8 @@ import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
  */
 enum SubKey {
   ActiveGroup,
-  MembersDTO,
-  SearchResultsDTO,
+  Members,
+  SearchResults,
 }
 
 /**
@@ -50,7 +47,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
   @Output()
   selectedReviewersUpdated: EventEmitter<EPerson[]> = new EventEmitter();
 
-  selectedReviewers: EpersonDtoModel[] = [];
+  selectedReviewers: EPerson[] = [];
 
   constructor(
     protected groupService: GroupDataService,
@@ -100,54 +97,40 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
   retrieveMembers(page: number): void {
     this.config.currentPage = page;
     if (this.groupId === null) {
-      this.unsubFrom(SubKey.MembersDTO);
-      const paginatedListOfDTOs: PaginatedList<EpersonDtoModel> = new PaginatedList();
-      paginatedListOfDTOs.page = this.selectedReviewers;
-      this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs);
+      this.unsubFrom(SubKey.Members);
+      const paginatedListOfEPersons: PaginatedList<EPerson> = new PaginatedList();
+      paginatedListOfEPersons.page = this.selectedReviewers;
+      this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
     } else {
       super.retrieveMembers(page);
     }
   }
 
   /**
-   * Checks whether the given {@link possibleMember} is part of the {@link selectedReviewers}.
+   * Removes the {@link eperson} from the {@link selectedReviewers}
    *
-   * @param possibleMember The {@link EPerson} that needs to be checked
+   * @param eperson The {@link EPerson} to remove
    */
-  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
-    return observableOf(hasValue(this.selectedReviewers.find((reviewer: EpersonDtoModel) => reviewer.eperson.id === possibleMember.id)));
-  }
-
-  /**
-   * Removes the {@link ePerson} from the {@link selectedReviewers}
-   *
-   * @param ePerson The {@link EpersonDtoModel} containg the {@link EPerson} to remove
-   */
-  deleteMemberFromGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = false;
-    const index = this.selectedReviewers.indexOf(ePerson);
+  deleteMemberFromGroup(eperson: EPerson) {
+    const index = this.selectedReviewers.indexOf(eperson);
     if (index !== -1) {
       this.selectedReviewers.splice(index, 1);
     }
-    this.selectedReviewersUpdated.emit(this.selectedReviewers.map((ePersonDtoModel: EpersonDtoModel) => ePersonDtoModel.eperson));
+    this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }
 
   /**
-   * Adds the {@link ePerson} to the {@link selectedReviewers} (or replaces it when {@link multipleReviewers} is
+   * Adds the {@link eperson} to the {@link selectedReviewers} (or replaces it when {@link multipleReviewers} is
    * `false`). Afterwards it will emit the list.
    *
-   * @param ePerson The {@link EPerson} to add to the list
+   * @param eperson The {@link EPerson} to add to the list
    */
-  addMemberToGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = true;
+  addMemberToGroup(eperson: EPerson) {
     if (!this.multipleReviewers) {
-      for (const selectedReviewer of this.selectedReviewers) {
-        selectedReviewer.memberOfGroup = false;
-      }
       this.selectedReviewers = [];
     }
-    this.selectedReviewers.push(ePerson);
-    this.selectedReviewersUpdated.emit(this.selectedReviewers.map((epersonDtoModel: EpersonDtoModel) => epersonDtoModel.eperson));
+    this.selectedReviewers.push(eperson);
+    this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }
 
 }