98376: Replaced EPersonListComponent with MembersListComponent & display error for empty reviewers list

src/app/access-control/access-control.module.ts

@@ -10,7 +10,6 @@ import { MembersListComponent } from './group-registry/group-form/members-list/m
 import { SubgroupsListComponent } from './group-registry/group-form/subgroup-list/subgroups-list.component';
 import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
 import { FormModule } from '../shared/form/form.module';
-import { EPersonListComponent } from './group-registry/group-form/eperson-list/eperson-list.component';
 
 @NgModule({
   imports: [
@@ -21,7 +20,7 @@ import { EPersonListComponent } from './group-registry/group-form/eperson-list/e
     FormModule,
   ],
   exports: [
-    EPersonListComponent,
+    MembersListComponent,
   ],
   declarations: [
     EPeopleRegistryComponent,
@@ -30,7 +29,6 @@ import { EPersonListComponent } from './group-registry/group-form/eperson-list/e
     GroupFormComponent,
     SubgroupsListComponent,
     MembersListComponent,
-    EPersonListComponent,
   ],
 })
 /**

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

@@ -1,146 +0,0 @@
-<ng-container>
-  <h3 class="border-bottom pb-2">{{messagePrefix + '.head' | translate}}</h3>
-
-  <h4 id="search" class="border-bottom pb-2">{{messagePrefix + '.search.head' | translate}}
-
-  </h4>
-  <form [formGroup]="searchForm" (ngSubmit)="search(searchForm.value)" class="d-flex justify-content-between">
-    <div>
-      <select name="scope" id="scope" formControlName="scope" class="form-control" aria-label="Search scope">
-        <option value="metadata">{{messagePrefix + '.search.scope.metadata' | translate}}</option>
-        <option value="email">{{messagePrefix + '.search.scope.email' | translate}}</option>
-      </select>
-    </div>
-    <div class="flex-grow-1 mr-3 ml-3">
-      <div class="form-group input-group">
-        <input type="text" name="query" id="query" formControlName="query"
-               class="form-control" aria-label="Search input">
-        <span class="input-group-append">
-            <button type="submit" class="search-button btn btn-primary">
-              <i class="fas fa-search"></i> {{ messagePrefix + '.search.button' | translate }}</button>
-        </span>
-      </div>
-    </div>
-    <div>
-      <button (click)="clearFormAndResetResult();"
-              class="btn btn-secondary">{{messagePrefix + '.button.see-all' | translate}}</button>
-    </div>
-  </form>
-
-  <ds-pagination *ngIf="(ePeopleSearchDtos | async)?.totalElements > 0"
-                 [paginationOptions]="configSearch"
-                 [pageInfoState]="(ePeopleSearchDtos | async)"
-                 [collectionSize]="(ePeopleSearchDtos | async)?.totalElements"
-                 [hideGear]="true"
-                 [hidePagerWhenSinglePage]="true">
-
-    <div class="table-responsive">
-      <table id="epersonsSearch" class="table table-striped table-hover table-bordered">
-        <thead>
-        <tr>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.identity' | translate}}</th>
-          <th class="align-middle">{{messagePrefix + '.table.edit' | translate}}</th>
-        </tr>
-        </thead>
-        <tbody>
-        <tr *ngFor="let ePerson of (ePeopleSearchDtos | async)?.page">
-          <td class="align-middle">{{ePerson.eperson.id}}</td>
-          <td class="align-middle"><a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
-                 [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">{{ePerson.eperson.name}}</a></td>
-          <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
-          </td>
-          <td class="align-middle">
-            <div class="btn-group edit-field">
-              <button *ngIf="ePerson.memberOfGroup"
-                      (click)="deleteMemberFromGroup(ePerson)"
-                      [disabled]="actionConfig.remove.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: {name: ePerson.eperson.name} }}">
-                <i [ngClass]="actionConfig.remove.icon"></i>
-              </button>
-
-              <button *ngIf="!ePerson.memberOfGroup"
-                      (click)="addMemberToGroup(ePerson)"
-                      [disabled]="actionConfig.add.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: {name: ePerson.eperson.name} }}">
-                <i [ngClass]="actionConfig.add.icon"></i>
-              </button>
-            </div>
-          </td>
-        </tr>
-        </tbody>
-      </table>
-    </div>
-
-  </ds-pagination>
-
-  <div *ngIf="(ePeopleSearchDtos | async)?.totalElements == 0 && searchDone"
-       class="alert alert-info w-100 mb-2"
-       role="alert">
-    {{messagePrefix + '.no-items' | translate}}
-  </div>
-
-  <h4>{{messagePrefix + '.headMembers' | translate}}</h4>
-
-  <ds-pagination *ngIf="(ePeopleMembersOfGroupDtos | async)?.totalElements > 0"
-                 [paginationOptions]="config"
-                 [pageInfoState]="(ePeopleMembersOfGroupDtos | async)"
-                 [collectionSize]="(ePeopleMembersOfGroupDtos | async)?.totalElements"
-                 [hideGear]="true"
-                 [hidePagerWhenSinglePage]="true">
-
-    <div class="table-responsive">
-      <table id="ePeopleMembersOfGroup" class="table table-striped table-hover table-bordered">
-        <thead>
-        <tr>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.id' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.name' | translate}}</th>
-          <th scope="col" class="align-middle">{{messagePrefix + '.table.identity' | translate}}</th>
-          <th class="align-middle">{{messagePrefix + '.table.edit' | translate}}</th>
-        </tr>
-        </thead>
-        <tbody>
-        <tr *ngFor="let ePerson of (ePeopleMembersOfGroupDtos | async)?.page">
-          <td class="align-middle">{{ePerson.eperson.id}}</td>
-          <td class="align-middle"><a (click)="ePersonDataService.startEditingNewEPerson(ePerson.eperson)"
-                 [routerLink]="[ePersonDataService.getEPeoplePageRouterLink()]">{{ePerson.eperson.name}}</a></td>
-          <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ ePerson.eperson.email ? ePerson.eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ ePerson.eperson.netid ? ePerson.eperson.netid : '-' }}
-          </td>
-          <td class="align-middle">
-            <div class="btn-group edit-field">
-              <button *ngIf="ePerson.memberOfGroup"
-                      (click)="deleteMemberFromGroup(ePerson)"
-                      [disabled]="actionConfig.remove.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: {name: ePerson.eperson.name} }}">
-                <i [ngClass]="actionConfig.remove.icon"></i>
-              </button>
-              <button *ngIf="!ePerson.memberOfGroup"
-                      (click)="addMemberToGroup(ePerson)"
-                      [disabled]="actionConfig.add.disabled"
-                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: {name: ePerson.eperson.name} }}">
-                <i [ngClass]="actionConfig.add.icon"></i>
-              </button>
-            </div>
-          </td>
-        </tr>
-        </tbody>
-      </table>
-    </div>
-
-  </ds-pagination>
-
-  <div *ngIf="(ePeopleMembersOfGroupDtos | async) == undefined || (ePeopleMembersOfGroupDtos | async)?.totalElements == 0" class="alert alert-info w-100 mb-2"
-       role="alert">
-    {{messagePrefix + '.no-members-yet' | translate}}
-  </div>
-
-</ng-container>

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

@@ -1,247 +0,0 @@
-import { CommonModule } from '@angular/common';
-import { NO_ERRORS_SCHEMA } from '@angular/core';
-import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
-import { FormsModule, ReactiveFormsModule } from '@angular/forms';
-import { BrowserModule, By } from '@angular/platform-browser';
-import { Router } from '@angular/router';
-import { NgbModule } from '@ng-bootstrap/ng-bootstrap';
-import { TranslateLoader, TranslateModule, TranslateService } from '@ngx-translate/core';
-import { Observable, of as observableOf } from 'rxjs';
-import { RestResponse } from '../../../../core/cache/response.models';
-import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
-import { RemoteData } from '../../../../core/data/remote-data';
-import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
-import { GroupDataService } from '../../../../core/eperson/group-data.service';
-import { EPerson } from '../../../../core/eperson/models/eperson.model';
-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 { EPersonListComponent } from './eperson-list.component';
-import { EPersonMock, EPersonMock2 } from '../../../../shared/testing/eperson.mock';
-import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils';
-import { getMockTranslateService } from '../../../../shared/mocks/translate.service.mock';
-import { getMockFormBuilderService } from '../../../../shared/mocks/form-builder-service.mock';
-import { TranslateLoaderMock } from '../../../../shared/testing/translate-loader.mock';
-import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service.stub';
-import { RouterMock } from '../../../../shared/mocks/router.mock';
-import { PaginationService } from '../../../../core/pagination/pagination.service';
-import { PaginationServiceStub } from '../../../../shared/testing/pagination-service.stub';
-
-describe('EPersonListComponent', () => {
-  let component: EPersonListComponent;
-  let fixture: ComponentFixture<EPersonListComponent>;
-  let translateService: TranslateService;
-  let builderService: FormBuilderService;
-  let ePersonDataServiceStub: any;
-  let groupsDataServiceStub: any;
-  let activeGroup;
-  let allEPersons;
-  let allGroups;
-  let epersonMembers;
-  let subgroupMembers;
-  let paginationService;
-
-  beforeEach(waitForAsync(() => {
-    activeGroup = GroupMock;
-    epersonMembers = [EPersonMock2];
-    subgroupMembers = [GroupMock2];
-    allEPersons = [EPersonMock, EPersonMock2];
-    allGroups = [GroupMock, GroupMock2];
-    ePersonDataServiceStub = {
-      activeGroup: activeGroup,
-      epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
-      findAllByHref(href: string): Observable<RemoteData<PaginatedList<EPerson>>> {
-        return createSuccessfulRemoteDataObject$(buildPaginatedList<EPerson>(new PageInfo(), groupsDataServiceStub.getEPersonMembers()));
-      },
-      searchByScope(scope: string, query: string): Observable<RemoteData<PaginatedList<EPerson>>> {
-        if (query === '') {
-          return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), allEPersons));
-        }
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-      },
-      clearEPersonRequests() {
-        // empty
-      },
-      clearLinkRequests() {
-        // empty
-      },
-      getEPeoplePageRouterLink(): string {
-        return '/access-control/epeople';
-      }
-    };
-    groupsDataServiceStub = {
-      activeGroup: activeGroup,
-      epersonMembers: epersonMembers,
-      subgroupMembers: subgroupMembers,
-      allGroups: allGroups,
-      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));
-        }
-        return createSuccessfulRemoteDataObject$(buildPaginatedList(new PageInfo(), []));
-      },
-      addMemberToGroup(parentGroup, eperson: EPerson): Observable<RestResponse> {
-        this.epersonMembers = [...this.epersonMembers, eperson];
-        return observableOf(new RestResponse(true, 200, 'Success'));
-      },
-      clearGroupsRequests() {
-        // empty
-      },
-      clearGroupLinkRequests() {
-        // empty
-      },
-      getGroupEditPageRouterLink(group: Group): string {
-        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;
-          }
-        });
-        if (this.epersonMembers === undefined) {
-          this.epersonMembers = [];
-        }
-        return observableOf(new RestResponse(true, 200, 'Success'));
-      }
-    };
-    builderService = getMockFormBuilderService();
-    translateService = getMockTranslateService();
-
-    paginationService = new PaginationServiceStub();
-    TestBed.configureTestingModule({
-      imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
-        TranslateModule.forRoot({
-          loader: {
-            provide: TranslateLoader,
-            useClass: TranslateLoaderMock
-          }
-        }),
-      ],
-      declarations: [EPersonListComponent],
-      providers: [EPersonListComponent,
-        { provide: EPersonDataService, useValue: ePersonDataServiceStub },
-        { provide: GroupDataService, useValue: groupsDataServiceStub },
-        { provide: NotificationsService, useValue: new NotificationsServiceStub() },
-        { provide: FormBuilderService, useValue: builderService },
-        { provide: Router, useValue: new RouterMock() },
-        { provide: PaginationService, useValue: paginationService },
-      ],
-      schemas: [NO_ERRORS_SCHEMA]
-    }).compileComponents();
-  }));
-
-  beforeEach(() => {
-    fixture = TestBed.createComponent(EPersonListComponent);
-    component = fixture.componentInstance;
-    fixture.detectChanges();
-  });
-  afterEach(fakeAsync(() => {
-    fixture.destroy();
-    flush();
-    component = null;
-    fixture.debugElement.nativeElement.remove();
-  }));
-
-  it('should create EpeopleListComponent', inject([EPersonListComponent], (comp: EPersonListComponent) => {
-    expect(comp).toBeDefined();
-  }));
-
-  it('should show list of eperson members of current active group', () => {
-    const epersonIdsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tr td:first-child'));
-    expect(epersonIdsFound.length).toEqual(1);
-    epersonMembers.map((eperson: EPerson) => {
-      expect(epersonIdsFound.find((foundEl) => {
-        return (foundEl.nativeElement.textContent.trim() === eperson.uuid);
-      })).toBeTruthy();
-    });
-  });
-
-  describe('search', () => {
-    describe('when searching without query', () => {
-      let epersonsFound;
-      beforeEach(fakeAsync(() => {
-        component.search({ scope: 'metadata', query: '' });
-        tick();
-        fixture.detectChanges();
-        epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-      }));
-
-      it('should display all epersons', () => {
-        expect(epersonsFound.length).toEqual(2);
-      });
-
-      describe('if eperson is already a eperson', () => {
-        it('should have delete button, else it should have add button', () => {
-          activeGroup.epersons.map((eperson: EPerson) => {
-            epersonsFound.map((foundEPersonRowElement) => {
-              if (foundEPersonRowElement.debugElement !== undefined) {
-                const epersonId = foundEPersonRowElement.debugElement.query(By.css('td:first-child'));
-                const addButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-plus'));
-                const deleteButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt'));
-                if (epersonId.nativeElement.textContent === eperson.id) {
-                  expect(addButton).toBeUndefined();
-                  expect(deleteButton).toBeDefined();
-                } else {
-                  expect(deleteButton).toBeUndefined();
-                  expect(addButton).toBeDefined();
-                }
-              }
-            });
-          });
-        });
-      });
-
-      describe('if first add button is pressed', () => {
-        beforeEach(fakeAsync(() => {
-          const addButton = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus'));
-          addButton.nativeElement.click();
-          tick();
-          fixture.detectChanges();
-        }));
-        it('all groups in search member of selected group', () => {
-          epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-          expect(epersonsFound.length).toEqual(2);
-          epersonsFound.map((foundEPersonRowElement) => {
-            if (foundEPersonRowElement.debugElement !== undefined) {
-              const addButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-plus'));
-              const deleteButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt'));
-              expect(addButton).toBeUndefined();
-              expect(deleteButton).toBeDefined();
-            }
-          });
-        });
-      });
-
-      describe('if first delete button is pressed', () => {
-        beforeEach(fakeAsync(() => {
-          const addButton = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-trash-alt'));
-          addButton.nativeElement.click();
-          tick();
-          fixture.detectChanges();
-        }));
-        it('first eperson in search delete button, because now member', () => {
-          epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-          epersonsFound.map((foundEPersonRowElement) => {
-            if (foundEPersonRowElement.debugElement !== undefined) {
-              const addButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-plus'));
-              const deleteButton = foundEPersonRowElement.debugElement.query(By.css('td:last-child .fa-trash-alt'));
-              expect(deleteButton).toBeUndefined();
-              expect(addButton).toBeDefined();
-            }
-          });
-        });
-      });
-    });
-  });
-
-});

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

@@ -1,358 +0,0 @@
-import { Component, OnDestroy, OnInit, Input } from '@angular/core';
-import { FormBuilder } from '@angular/forms';
-import { Router } from '@angular/router';
-import { TranslateService } from '@ngx-translate/core';
-import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
-import { GroupDataService } from '../../../../core/eperson/group-data.service';
-import { NotificationsService } from '../../../../shared/notifications/notifications.service';
-import { PaginationService } from '../../../../core/pagination/pagination.service';
-import { Group } from '../../../../core/eperson/models/group.model';
-import {
-  getAllCompletedRemoteData,
-  getFirstSucceededRemoteData,
-  getRemoteDataPayload,
-  getFirstCompletedRemoteData
-} from '../../../../core/shared/operators';
-import {
-  BehaviorSubject,
-  Subscription,
-  combineLatest as observableCombineLatest,
-  Observable,
-  ObservedValueOf,
-  of as observableOf
-} from 'rxjs';
-import { PaginatedList, buildPaginatedList } from '../../../../core/data/paginated-list.model';
-import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
-import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
-import { switchMap, map, take, mergeMap } from 'rxjs/operators';
-import { RemoteData } from '../../../../core/data/remote-data';
-import { EPerson } from '../../../../core/eperson/models/eperson.model';
-
-/**
- * Keys to keep track of specific subscriptions
- */
-enum SubKey {
-  ActiveGroup,
-  MembersDTO,
-  SearchResultsDTO,
-}
-
-export interface EPersonActionConfig {
-  css?: string;
-  disabled: boolean;
-  icon: string;
-}
-
-export interface EPersonListActionConfig {
-  add: EPersonActionConfig;
-  remove: EPersonActionConfig;
-}
-
-@Component({
-  selector: 'ds-eperson-list',
-  templateUrl: './eperson-list.component.html'
-})
-export class EPersonListComponent implements OnInit, OnDestroy {
-
-  @Input()
-  messagePrefix: string;
-
-  @Input()
-  actionConfig: EPersonListActionConfig = {
-    add: {
-      css: 'btn-outline-primary',
-      disabled: false,
-      icon: 'fas fa-plus fa-fw',
-    },
-    remove: {
-      css: 'btn-outline-danger',
-      disabled: false,
-      icon: 'fas fa-trash-alt fa-fw'
-    },
-  };
-
-  /**
-   * EPeople being displayed in search result, initially all members, after search result of search
-   */
-  ePeopleSearchDtos: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject<PaginatedList<EpersonDtoModel>>(undefined);
-  /**
-   * List of EPeople members of currently active group being edited
-   */
-  ePeopleMembersOfGroupDtos: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject<PaginatedList<EpersonDtoModel>>(undefined);
-
-  /**
-   * Pagination config used to display the list of EPeople that are result of EPeople search
-   */
-  configSearch: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), {
-    id: 'sml',
-    pageSize: 5,
-    currentPage: 1
-  });
-  /**
-   * Pagination config used to display the list of EPerson Membes of active group being edited
-   */
-  config: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), {
-    id: 'ml',
-    pageSize: 5,
-    currentPage: 1
-  });
-
-  /**
-   * Map of active subscriptions
-   */
-  subs: Map<SubKey, Subscription> = new Map();
-
-  // The search form
-  searchForm;
-
-  // Current search in edit group - epeople search form
-  currentSearchQuery: string;
-  currentSearchScope: string;
-
-  // Whether or not user has done a EPeople search yet
-  searchDone: boolean;
-
-  // current active group being edited
-  groupBeingEdited: Group;
-
-  constructor(
-    protected groupDataService: GroupDataService,
-    public ePersonDataService: EPersonDataService,
-    protected translateService: TranslateService,
-    protected notificationsService: NotificationsService,
-    protected formBuilder: FormBuilder,
-    protected paginationService: PaginationService,
-    private router: Router
-  ) {
-    this.currentSearchQuery = '';
-    this.currentSearchScope = 'metadata';
-  }
-
-  ngOnInit(): void {
-    this.searchForm = this.formBuilder.group(({
-      scope: 'metadata',
-      query: '',
-    }));
-    this.subs.set(SubKey.ActiveGroup, this.groupDataService.getActiveGroup().subscribe((activeGroup: Group) => {
-      if (activeGroup != null) {
-        this.groupBeingEdited = activeGroup;
-        this.retrieveMembers(this.config.currentPage);
-      }
-    }));
-  }
-
-  /**
-   * Retrieve the EPersons that are members of the group
-   *
-   * @param page the number of the page to retrieve
-   * @private
-   */
-  retrieveMembers(page: number): void {
-    this.unsubFrom(SubKey.MembersDTO);
-    this.subs.set(SubKey.MembersDTO,
-      this.paginationService.getCurrentPagination(this.config.id, this.config).pipe(
-        switchMap((currentPagination) => {
-                  return this.ePersonDataService.findAllByHref(this.groupBeingEdited._links.epersons.href, {
-              currentPage: currentPagination.currentPage,
-              elementsPerPage: currentPagination.pageSize
-            }
-          );
-        }),
-        getAllCompletedRemoteData(),
-        map((rd: RemoteData<any>) => {
-          if (rd.hasFailed) {
-            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', { cause: rd.errorMessage }));
-          } else {
-            return rd;
-          }
-        }),
-        switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-          const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => {
-            const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
-              this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
-                const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-                epersonDtoModel.eperson = member;
-                epersonDtoModel.memberOfGroup = isMember;
-                return epersonDtoModel;
-              });
-            return dto$;
-          }));
-          return dtos$.pipe(map((dtos: EpersonDtoModel[]) => {
-            return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
-          }));
-        }))
-        .subscribe((paginatedListOfDTOs: PaginatedList<EpersonDtoModel>) => {
-          this.ePeopleMembersOfGroupDtos.next(paginatedListOfDTOs);
-        }));
-  }
-
-  /**
-   * Whether the given ePerson is a member of the group currently being edited
-   * @param possibleMember  EPerson that is a possible member (being tested) of the group currently being edited
-   */
-  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
-    return this.groupDataService.getActiveGroup().pipe(take(1),
-      mergeMap((group: Group) => {
-        if (group != null) {
-          return this.ePersonDataService.findAllByHref(group._links.epersons.href, {
-            currentPage: 1,
-            elementsPerPage: 9999
-          }, false)
-            .pipe(
-              getFirstSucceededRemoteData(),
-              getRemoteDataPayload(),
-              map((listEPeopleInGroup: PaginatedList<EPerson>) => listEPeopleInGroup.page.filter((ePersonInList: EPerson) => ePersonInList.id === possibleMember.id)),
-              map((epeople: EPerson[]) => epeople.length > 0));
-        } else {
-          return observableOf(false);
-        }
-      }));
-  }
-
-  /**
-   * Unsubscribe from a subscription if it's still subscribed, and remove it from the map of
-   * active subscriptions
-   *
-   * @param key The key of the subscription to unsubscribe from
-   * @private
-   */
-  protected unsubFrom(key: SubKey) {
-    if (this.subs.has(key)) {
-      this.subs.get(key).unsubscribe();
-      this.subs.delete(key);
-    }
-  }
-
-  /**
-   * Deletes a given EPerson from the members list of the group currently being edited
-   * @param ePerson   EPerson we want to delete as member from group that is currently being edited
-   */
-  deleteMemberFromGroup(ePerson: EpersonDtoModel) {
-    this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
-      if (activeGroup != null) {
-        const response = this.groupDataService.deleteMemberFromGroup(activeGroup, ePerson.eperson);
-        this.showNotifications('deleteMember', response, ePerson.eperson.name, activeGroup);
-        this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery });
-      } else {
-        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
-      }
-    });
-  }
-
-  /**
-   * Adds a given EPerson to the members list of the group currently being edited
-   * @param ePerson   EPerson we want to add as member to group that is currently being edited
-   */
-  addMemberToGroup(ePerson: EpersonDtoModel) {
-    ePerson.memberOfGroup = true;
-    this.groupDataService.getActiveGroup().pipe(take(1)).subscribe((activeGroup: Group) => {
-      if (activeGroup != null) {
-        const response = this.groupDataService.addMemberToGroup(activeGroup, ePerson.eperson);
-        this.showNotifications('addMember', response, ePerson.eperson.name, activeGroup);
-      } else {
-        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.noActiveGroup'));
-      }
-    });
-  }
-
-  /**
-   * Search in the EPeople by name, email or metadata
-   * @param data  Contains scope and query param
-   */
-  search(data: any) {
-    this.unsubFrom(SubKey.SearchResultsDTO);
-    this.subs.set(SubKey.SearchResultsDTO,
-      this.paginationService.getCurrentPagination(this.configSearch.id, this.configSearch).pipe(
-        switchMap((paginationOptions) => {
-
-          const query: string = data.query;
-          const scope: string = data.scope;
-          if (query != null && this.currentSearchQuery !== query && this.groupBeingEdited) {
-            this.router.navigate([], {
-              queryParamsHandling: 'merge'
-            });
-            this.currentSearchQuery = query;
-            this.paginationService.resetPage(this.configSearch.id);
-          }
-          if (scope != null && this.currentSearchScope !== scope && this.groupBeingEdited) {
-            this.router.navigate([], {
-              queryParamsHandling: 'merge'
-            });
-            this.currentSearchScope = scope;
-            this.paginationService.resetPage(this.configSearch.id);
-          }
-          this.searchDone = true;
-
-          return this.ePersonDataService.searchByScope(this.currentSearchScope, this.currentSearchQuery, {
-            currentPage: paginationOptions.currentPage,
-            elementsPerPage: paginationOptions.pageSize
-          });
-        }),
-        getAllCompletedRemoteData(),
-        map((rd: RemoteData<any>) => {
-          if (rd.hasFailed) {
-            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', { cause: rd.errorMessage }));
-          } else {
-            return rd;
-          }
-        }),
-        switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-          const dtos$ = observableCombineLatest(...epersonListRD.payload.page.map((member: EPerson) => {
-            const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
-              this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
-                const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
-                epersonDtoModel.eperson = member;
-                epersonDtoModel.memberOfGroup = isMember;
-                return epersonDtoModel;
-              });
-            return dto$;
-          }));
-          return dtos$.pipe(map((dtos: EpersonDtoModel[]) => {
-            return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
-          }));
-        }))
-        .subscribe((paginatedListOfDTOs: PaginatedList<EpersonDtoModel>) => {
-          this.ePeopleSearchDtos.next(paginatedListOfDTOs);
-        }));
-  }
-
-  /**
-   * unsub all subscriptions
-   */
-  ngOnDestroy(): void {
-    for (const key of this.subs.keys()) {
-      this.unsubFrom(key);
-    }
-    this.paginationService.clearPagination(this.config.id);
-    this.paginationService.clearPagination(this.configSearch.id);
-  }
-
-  /**
-   * Shows a notification based on the success/failure of the request
-   * @param messageSuffix   Suffix for message
-   * @param response        RestResponse observable containing success/failure request
-   * @param nameObject      Object request was about
-   * @param activeGroup     Group currently being edited
-   */
-  showNotifications(messageSuffix: string, response: Observable<RemoteData<any>>, nameObject: string, activeGroup: Group) {
-    response.pipe(getFirstCompletedRemoteData()).subscribe((rd: RemoteData<any>) => {
-      if (rd.hasSucceeded) {
-        this.notificationsService.success(this.translateService.get(this.messagePrefix + '.notification.success.' + messageSuffix, { name: nameObject }));
-        this.ePersonDataService.clearLinkRequests(activeGroup._links.epersons.href);
-      } else {
-        this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure.' + messageSuffix, { name: nameObject }));
-      }
-    });
-  }
-
-  /**
-   * Reset all input-fields to be empty and search all search
-   */
-  clearFormAndResetResult() {
-    this.searchForm.patchValue({
-      query: '',
-    });
-    this.search({ query: '' });
-  }
-
-}

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

@@ -55,18 +55,20 @@
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button *ngIf="(ePerson.memberOfGroup)"
+              <button *ngIf="ePerson.memberOfGroup"
                       (click)="deleteMemberFromGroup(ePerson)"
-                      class="btn btn-outline-danger btn-sm"
+                      [disabled]="actionConfig.remove.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
                       title="{{messagePrefix + '.table.edit.buttons.remove' | translate: {name: ePerson.eperson.name} }}">
-                <i class="fas fa-trash-alt fa-fw"></i>
+                <i [ngClass]="actionConfig.remove.icon"></i>
               </button>
 
-              <button *ngIf="!(ePerson.memberOfGroup)"
+              <button *ngIf="!ePerson.memberOfGroup"
                       (click)="addMemberToGroup(ePerson)"
-                      class="btn btn-outline-primary btn-sm"
+                      [disabled]="actionConfig.add.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
                       title="{{messagePrefix + '.table.edit.buttons.add' | translate: {name: ePerson.eperson.name} }}">
-                <i class="fas fa-plus fa-fw"></i>
+                <i [ngClass]="actionConfig.add.icon"></i>
               </button>
             </div>
           </td>
@@ -113,10 +115,19 @@
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button (click)="deleteMemberFromGroup(ePerson)"
+              <button *ngIf="ePerson.memberOfGroup"
-                      class="btn btn-outline-danger btn-sm"
+                      (click)="deleteMemberFromGroup(ePerson)"
+                      [disabled]="actionConfig.remove.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.remove.css]"
                       title="{{messagePrefix + '.table.edit.buttons.remove' | translate: {name: ePerson.eperson.name} }}">
-                <i class="fas fa-trash-alt fa-fw"></i>
+                <i [ngClass]="actionConfig.remove.icon"></i>
+              </button>
+              <button *ngIf="!ePerson.memberOfGroup"
+                      (click)="addMemberToGroup(ePerson)"
+                      [disabled]="actionConfig.add.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
+                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: {name: ePerson.eperson.name} }}">
+                <i [ngClass]="actionConfig.add.icon"></i>
               </button>
             </div>
           </td>

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

@@ -1,30 +1,32 @@
-import { Component, Input, OnDestroy, OnInit } from '@angular/core';
+import { Component, OnDestroy, OnInit, Input } from '@angular/core';
 import { FormBuilder } from '@angular/forms';
 import { Router } from '@angular/router';
 import { TranslateService } from '@ngx-translate/core';
-import {
-  Observable,
-  of as observableOf,
-  Subscription,
-  BehaviorSubject,
-  combineLatest as observableCombineLatest,
-  ObservedValueOf,
-} from 'rxjs';
-import { map, mergeMap, switchMap, take } from 'rxjs/operators';
-import {buildPaginatedList, PaginatedList} from '../../../../core/data/paginated-list.model';
-import { RemoteData } from '../../../../core/data/remote-data';
 import { EPersonDataService } from '../../../../core/eperson/eperson-data.service';
 import { GroupDataService } from '../../../../core/eperson/group-data.service';
-import { EPerson } from '../../../../core/eperson/models/eperson.model';
+import { NotificationsService } from '../../../../shared/notifications/notifications.service';
+import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { Group } from '../../../../core/eperson/models/group.model';
 import {
+  getAllCompletedRemoteData,
   getFirstSucceededRemoteData,
-  getFirstCompletedRemoteData, getAllCompletedRemoteData, getRemoteDataPayload
+  getRemoteDataPayload,
+  getFirstCompletedRemoteData
 } from '../../../../core/shared/operators';
-import { NotificationsService } from '../../../../shared/notifications/notifications.service';
+import {
+  BehaviorSubject,
+  Subscription,
+  combineLatest as observableCombineLatest,
+  Observable,
+  ObservedValueOf,
+  of as observableOf
+} from 'rxjs';
+import { PaginatedList, buildPaginatedList } from '../../../../core/data/paginated-list.model';
+import { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model';
-import {EpersonDtoModel} from '../../../../core/eperson/models/eperson-dto.model';
+import { switchMap, map, take, mergeMap } from 'rxjs/operators';
-import { PaginationService } from '../../../../core/pagination/pagination.service';
+import { RemoteData } from '../../../../core/data/remote-data';
+import { EPerson } from '../../../../core/eperson/models/eperson.model';
 
 /**
  * Keys to keep track of specific subscriptions
@@ -35,6 +37,17 @@ enum SubKey {
   SearchResultsDTO,
 }
 
+export interface EPersonActionConfig {
+  css?: string;
+  disabled: boolean;
+  icon: string;
+}
+
+export interface EPersonListActionConfig {
+  add: EPersonActionConfig;
+  remove: EPersonActionConfig;
+}
+
 @Component({
   selector: 'ds-members-list',
   templateUrl: './members-list.component.html'
@@ -47,6 +60,20 @@ export class MembersListComponent implements OnInit, OnDestroy {
   @Input()
   messagePrefix: string;
 
+  @Input()
+  actionConfig: EPersonListActionConfig = {
+    add: {
+      css: 'btn-outline-primary',
+      disabled: false,
+      icon: 'fas fa-plus fa-fw',
+    },
+    remove: {
+      css: 'btn-outline-danger',
+      disabled: false,
+      icon: 'fas fa-trash-alt fa-fw'
+    },
+  };
+
   /**
    * EPeople being displayed in search result, initially all members, after search result of search
    */
@@ -91,23 +118,20 @@ export class MembersListComponent implements OnInit, OnDestroy {
   // current active group being edited
   groupBeingEdited: Group;
 
-  paginationSub: Subscription;
-
-
   constructor(
     protected groupDataService: GroupDataService,
     public ePersonDataService: EPersonDataService,
-    private translateService: TranslateService,
+    protected translateService: TranslateService,
-    private notificationsService: NotificationsService,
+    protected notificationsService: NotificationsService,
     protected formBuilder: FormBuilder,
-    private paginationService: PaginationService,
+    protected paginationService: PaginationService,
     private router: Router
   ) {
     this.currentSearchQuery = '';
     this.currentSearchScope = 'metadata';
   }
 
-  ngOnInit() {
+  ngOnInit(): void {
     this.searchForm = this.formBuilder.group(({
       scope: 'metadata',
       query: '',
@@ -126,7 +150,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param page the number of the page to retrieve
    * @private
    */
-  protected retrieveMembers(page: number) {
+  retrieveMembers(page: number): void {
     this.unsubFrom(SubKey.MembersDTO);
     this.subs.set(SubKey.MembersDTO,
       this.paginationService.getCurrentPagination(this.config.id, this.config).pipe(
@@ -140,7 +164,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
         getAllCompletedRemoteData(),
         map((rd: RemoteData<any>) => {
           if (rd.hasFailed) {
-          this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage}));
+            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', { cause: rd.errorMessage }));
           } else {
             return rd;
           }
@@ -166,7 +190,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
   }
 
   /**
-   * Whether or not the given ePerson is a member of the group currently being edited
+   * Whether the given ePerson is a member of the group currently being edited
    * @param possibleMember  EPerson that is a possible member (being tested) of the group currently being edited
    */
   isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
@@ -195,7 +219,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
    * @param key The key of the subscription to unsubscribe from
    * @private
    */
-  private unsubFrom(key: SubKey) {
+  protected unsubFrom(key: SubKey) {
     if (this.subs.has(key)) {
       this.subs.get(key).unsubscribe();
       this.subs.delete(key);
@@ -270,7 +294,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
         getAllCompletedRemoteData(),
         map((rd: RemoteData<any>) => {
           if (rd.hasFailed) {
-            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', {cause: rd.errorMessage}));
+            this.notificationsService.error(this.translateService.get(this.messagePrefix + '.notification.failure', { cause: rd.errorMessage }));
           } else {
             return rd;
           }
@@ -333,4 +357,5 @@ export class MembersListComponent implements OnInit, OnDestroy {
     });
     this.search({ query: '' });
   }
+
 }

src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.html

@@ -1,4 +1,4 @@
-<table id="metadata" class="table table-striped table-hover">
+<table id="metadata" class="table table-striped table-hover table-responsive">
   <thead>
   <tr>
     <th scope="col">{{'item.edit.modify.overview.field'| translate}}</th>

src/app/workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/advanced-workflow-action-select-reviewer.component.html

@@ -7,9 +7,12 @@
                      [groupId]="groupId"
                      [ngClass]="groupId ? 'reviewersListWithGroup' : ''"
                      [multipleReviewers]="multipleReviewers"
-                     (selectedReviewersUpdated)="selectedReviewers = $event"
+                     (selectedReviewersUpdated)="selectedReviewers = $event; displayError = false"
                      messagePrefix="advanced-workflow-action-select-reviewer.groups.form.reviewers-list"
   ></ds-reviewers-list>
+  <small *ngIf="displayError" class="invalid-feedback d-block mb-3">
+    {{ 'advanced-workflow-action.select-reviewer.no-reviewer-selected.error' | translate }}
+  </small>
 
   <ds-modify-item-overview *ngIf="item$ | async"
                            [item]="item$ | async">

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

@@ -9,7 +9,7 @@ import {
 } from '../../../core/tasks/models/select-reviewer-action-advanced-info.model';
 import {
   EPersonListActionConfig
-} from '../../../access-control/group-registry/group-form/eperson-list/eperson-list.component';
+} from '../../../access-control/group-registry/group-form/members-list/members-list.component';
 import { Subscription } from 'rxjs';
 import { EPerson } from '../../../core/eperson/models/eperson.model';
 
@@ -38,6 +38,8 @@ export class AdvancedWorkflowActionSelectReviewerComponent extends AdvancedWorkf
 
   subs: Subscription[] = [];
 
+  displayError = false;
+
   ngOnDestroy(): void {
     this.subs.forEach((subscription: Subscription) => subscription.unsubscribe());
   }
@@ -84,6 +86,15 @@ export class AdvancedWorkflowActionSelectReviewerComponent extends AdvancedWorkf
     return ADVANCED_WORKFLOW_ACTION_SELECT_REVIEWER;
   }
 
+  performAction(): void {
+    if (this.selectedReviewers.length > 0) {
+      super.performAction();
+    } else {
+      this.displayError = true;
+    }
+    console.log(this.displayError);
+  }
+
   createBody(): any {
     return {
       [WORKFLOW_ADVANCED_TASK_OPTION_SELECT_REVIEWER]: true,

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

@@ -8,15 +8,14 @@ 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 {
-  EPersonListComponent,
-  EPersonListActionConfig
-} from '../../../../access-control/group-registry/group-form/eperson-list/eperson-list.component';
 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, EPersonListActionConfig
+} from '../../../../access-control/group-registry/group-form/members-list/members-list.component';
 
 /**
  * Keys to keep track of specific subscriptions
@@ -30,9 +29,9 @@ enum SubKey {
 @Component({
   selector: 'ds-reviewers-list',
   // templateUrl: './reviewers-list.component.html',
-  templateUrl: '../../../../access-control/group-registry/group-form/eperson-list/eperson-list.component.html',
+  templateUrl: '../../../../access-control/group-registry/group-form/members-list/members-list.component.html',
 })
-export class ReviewersListComponent extends EPersonListComponent implements OnInit, OnChanges, OnDestroy {
+export class ReviewersListComponent extends MembersListComponent implements OnInit, OnChanges, OnDestroy {
 
   @Input()
   groupId: string | null;

src/assets/i18n/en.json5

@@ -599,6 +599,8 @@
 
   "advanced-workflow-action-select-reviewer.groups.form.reviewers-list.no-items": "No EPeople found in that search",
 
+  "advanced-workflow-action.select-reviewer.no-reviewer-selected.error": "No reviewer selected.",
+
 
   "auth.errors.invalid-user": "Invalid email address or password.",