115215: Fixed advanced workflow options (select reviewer & rating) throwing 404 error

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

@@ -20,25 +20,33 @@
         </tr>
         </thead>
         <tbody>
-        <tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
+        <tr *ngFor="let epersonDTO of (ePeopleMembersOfGroup | async)?.page">
-          <td class="align-middle">{{eperson.id}}</td>
+          <td class="align-middle">{{epersonDTO.eperson.id}}</td>
           <td class="align-middle">
-            <a [routerLink]="getEPersonEditRoute(eperson.id)">
+            <a [routerLink]="getEPersonEditRoute(epersonDTO.eperson.id)">
-              {{ dsoNameService.getName(eperson) }}
+              {{ dsoNameService.getName(epersonDTO.eperson) }}
             </a>
           </td>
           <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
+            {{messagePrefix + '.table.email' | translate}}: {{ epersonDTO.eperson.email ? epersonDTO.eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
+            {{messagePrefix + '.table.netid' | translate}}: {{ epersonDTO.eperson.netid ? epersonDTO.eperson.netid : '-' }}
           </td>
           <td class="align-middle">
             <div class="btn-group edit-field">
-              <button (click)="deleteMemberFromGroup(eperson)"
+              <button (click)="deleteMemberFromGroup(epersonDTO.eperson)"
+                      *ngIf="epersonDTO.ableToDelete"
                       [disabled]="actionConfig.remove.disabled"
                       [ngClass]="['btn btn-sm', actionConfig.remove.css]"
-                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(eperson) } }}">
+                      title="{{messagePrefix + '.table.edit.buttons.remove' | translate: { name: dsoNameService.getName(epersonDTO.eperson) } }}">
                 <i [ngClass]="actionConfig.remove.icon"></i>
               </button>
+              <button *ngIf="!epersonDTO.ableToDelete"
+                      (click)="addMemberToGroup(epersonDTO.eperson)"
+                      [disabled]="actionConfig.add.disabled"
+                      [ngClass]="['btn btn-sm', actionConfig.add.css]"
+                      title="{{messagePrefix + '.table.edit.buttons.add' | translate: { name: dsoNameService.getName(epersonDTO.eperson) } }}">
+                <i [ngClass]="actionConfig.add.icon"></i>
+              </button>
             </div>
           </td>
         </tr>

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

@@ -222,13 +222,13 @@ describe('MembersListComponent', () => {
 
     describe('if first delete button is pressed', () => {
       beforeEach(() => {
+        spyOn(component, 'search').and.callThrough();
         const deleteButton: DebugElement = fixture.debugElement.query(By.css('#ePeopleMembersOfGroup tbody .fa-trash-alt'));
         deleteButton.nativeElement.click();
         fixture.detectChanges();
       });
-      it('then no ePerson remains as a member of the active group.', () => {
+      it('should trigger the search to add the user back to the search table', () => {
-        const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
+        expect(component.search).toHaveBeenCalled();
-        expect(epersonsFound.length).toEqual(0);
       });
     });
   });
@@ -264,13 +264,13 @@ describe('MembersListComponent', () => {
 
       describe('if first add button is pressed', () => {
         beforeEach(() => {
+          spyOn(component, 'search').and.callThrough();
           const addButton: DebugElement = fixture.debugElement.query(By.css('#epersonsSearch tbody .fa-plus'));
           addButton.nativeElement.click();
           fixture.detectChanges();
         });
-        it('then all (two) ePersons are member of the active group. No non-members left', () => {
+        it('should trigger the search to remove the user from the search table', () => {
-          epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
+          expect(component.search).toHaveBeenCalled();
-          expect(epersonsFound.length).toEqual(0);
         });
       });
     });

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

@@ -24,21 +24,29 @@ import {
 } from '@ngx-translate/core';
 import {
   BehaviorSubject,
+  combineLatest as observableCombineLatest,
   Observable,
+  ObservedValueOf,
+  of as observableOf,
   Subscription,
 } from 'rxjs';
 import {
+  defaultIfEmpty,
   map,
   switchMap,
   take,
 } from 'rxjs/operators';
 
 import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
-import { PaginatedList } from '../../../../core/data/paginated-list.model';
+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 { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { Group } from '../../../../core/eperson/models/group.model';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import {
@@ -137,7 +145,7 @@ export class MembersListComponent implements OnInit, OnDestroy {
   /**
    * List of EPeople members of currently active group being edited
    */
-  ePeopleMembersOfGroup: BehaviorSubject<PaginatedList<EPerson>> = new BehaviorSubject<PaginatedList<EPerson>>(undefined);
+  ePeopleMembersOfGroup: BehaviorSubject<PaginatedList<EpersonDtoModel>> = new BehaviorSubject(undefined);
 
   /**
    * Pagination config used to display the list of EPeople that are result of EPeople search
@@ -226,10 +234,35 @@ export class MembersListComponent implements OnInit, OnDestroy {
             return rd;
           }
         }),
-        getRemoteDataPayload())
+        switchMap((epersonListRD: RemoteData<PaginatedList<EPerson>>) => {
-        .subscribe((paginatedListOfEPersons: PaginatedList<EPerson>) => {
+          const dtos$ = observableCombineLatest([...epersonListRD.payload.page.map((member: EPerson) => {
-          this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
+            const dto$: Observable<EpersonDtoModel> = observableCombineLatest(
+              this.isMemberOfGroup(member), (isMember: ObservedValueOf<Observable<boolean>>) => {
+                const epersonDtoModel: EpersonDtoModel = new EpersonDtoModel();
+                epersonDtoModel.eperson = member;
+                epersonDtoModel.ableToDelete = isMember;
+                return epersonDtoModel;
+              });
+            return dto$;
+          })]);
+          return dtos$.pipe(defaultIfEmpty([]), map((dtos: EpersonDtoModel[]) => {
+            return buildPaginatedList(epersonListRD.payload.pageInfo, dtos);
           }));
+        }),
+      ).subscribe((paginatedListOfDTOs: PaginatedList<EpersonDtoModel>) => {
+        this.ePeopleMembersOfGroup.next(paginatedListOfDTOs);
+      }),
+    );
+  }
+
+  /**
+   * We always return true since this is only used by the top section (which represents all the users part of the group
+   * in {@link MembersListComponent})
+   *
+   * @param possibleMember  EPerson that is a possible member (being tested) of the group currently being edited
+   */
+  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
+    return observableOf(true);
   }
 
   /**

src/app/core/eperson/eperson-data.service.ts

@@ -107,13 +107,17 @@ export class EPersonDataService extends IdentifiableDataService<EPerson> impleme
    * @param scope   Scope of the EPeople search, default byMetadata
    * @param query   Query of search
    * @param options Options of search request
+   * @param useCachedVersionIfAvailable If this is true, the request will only be sent if there's
+   *                                    no valid cached version. Defaults to true
+   * @param reRequestOnStale            Whether or not the request should automatically be re-
+   *                                    requested after the response becomes stale
    */
-  public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean): Observable<RemoteData<PaginatedList<EPerson>>> {
+  public searchByScope(scope: string, query: string, options: FindListOptions = {}, useCachedVersionIfAvailable?: boolean, reRequestOnStale = true): Observable<RemoteData<PaginatedList<EPerson>>> {
     switch (scope) {
       case 'metadata':
-        return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable);
+        return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale);
       case 'email':
-        return this.getEPersonByEmail(query.trim()).pipe(
+        return this.getEPersonByEmail(query.trim(), useCachedVersionIfAvailable, reRequestOnStale).pipe(
           map((rd: RemoteData<EPerson | NoContent>) => {
             if (rd.hasSucceeded) {
               // Turn the single EPerson or NoContent in to a PaginatedList<EPerson>
@@ -145,7 +149,7 @@ export class EPersonDataService extends IdentifiableDataService<EPerson> impleme
           }),
         );
       default:
-        return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable);
+        return this.getEpeopleByMetadata(query.trim(), options, useCachedVersionIfAvailable, reRequestOnStale);
     }
   }
 

src/app/item-page/edit-item-page/modify-item-overview/modify-item-overview.component.spec.ts

@@ -37,6 +37,7 @@ describe('ModifyItemOverviewComponent', () => {
     fixture = TestBed.createComponent(ModifyItemOverviewComponent);
     comp = fixture.componentInstance;
     comp.item = mockItem;
+    comp.ngOnChanges();
 
     fixture.detectChanges();
   });

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

@@ -5,7 +5,7 @@ import {
 import {
   Component,
   Input,
-  OnInit,
+  OnChanges,
 } from '@angular/core';
 import { TranslateModule } from '@ngx-translate/core';
 
@@ -21,12 +21,12 @@ import { MetadataMap } from '../../../core/shared/metadata.models';
 /**
  * Component responsible for rendering a table containing the metadatavalues from the to be edited item
  */
-export class ModifyItemOverviewComponent implements OnInit {
+export class ModifyItemOverviewComponent implements OnChanges  {
 
   @Input() item: Item;
   metadata: MetadataMap;
 
-  ngOnInit(): void {
+  ngOnChanges(): void {
-    this.metadata = this.item.metadata;
+    this.metadata = this.item?.metadata;
   }
 }

src/app/shared/mydspace-actions/claimed-task/abstract/claimed-task-actions-abstract.component.ts

@@ -1,6 +1,7 @@
 import {
   Component,
   Injector,
+  Input,
   OnDestroy,
 } from '@angular/core';
 import { Router } from '@angular/router';
@@ -44,7 +45,7 @@ export abstract class ClaimedTaskActionsAbstractComponent extends MyDSpaceReload
   /**
    * The item object that belonging to the ClaimedTask object
    */
-  item: Item;
+  @Input() item: Item;
 
   /**
    * Anchor used to reload the pool task.
@@ -56,7 +57,7 @@ export abstract class ClaimedTaskActionsAbstractComponent extends MyDSpaceReload
   /**
    * The workflowitem object that belonging to the ClaimedTask object
    */
-  workflowitem: WorkflowItem;
+  @Input() workflowitem: WorkflowItem;
 
   protected constructor(protected injector: Injector,
                         protected router: Router,

src/app/shared/mydspace-actions/claimed-task/switcher/claimed-task-actions-decorator.ts

@@ -1,8 +1,10 @@
 import {
+  ADVANCED_WORKFLOW_ACTION_RATING,
   ADVANCED_WORKFLOW_TASK_OPTION_RATING,
   AdvancedWorkflowActionRatingComponent,
 } from '../../../../workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-rating/advanced-workflow-action-rating.component';
 import {
+  ADVANCED_WORKFLOW_ACTION_SELECT_REVIEWER,
   ADVANCED_WORKFLOW_TASK_OPTION_SELECT_REVIEWER,
   AdvancedWorkflowActionSelectReviewerComponent,
 } from '../../../../workflowitems-edit-page/advanced-workflow-action/advanced-workflow-action-select-reviewer/advanced-workflow-action-select-reviewer.component';
@@ -54,8 +56,8 @@ export const WORKFLOW_TASK_OPTION_DECORATOR_MAP = new Map<string, WorkflowTaskOp
 ]);
 
 export const ADVANCED_WORKFLOW_TASK_OPTION_DECORATOR_MAP = new Map<string, AdvancedWorkflowTaskOptionComponent>([
-  [ADVANCED_WORKFLOW_TASK_OPTION_RATING, AdvancedWorkflowActionRatingComponent],
+  [ADVANCED_WORKFLOW_ACTION_RATING, AdvancedWorkflowActionRatingComponent],
-  [ADVANCED_WORKFLOW_TASK_OPTION_SELECT_REVIEWER, AdvancedWorkflowActionSelectReviewerComponent],
+  [ADVANCED_WORKFLOW_ACTION_SELECT_REVIEWER, AdvancedWorkflowActionSelectReviewerComponent],
 ]);
 
 /**

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

@@ -224,27 +224,6 @@ describe('ReviewersListComponent', () => {
         })).not.toBeTruthy();
       });
     });
-  });
-
-  describe('when a group is selected', () => {
-    beforeEach(() => {
-      component.ngOnChanges({
-        groupId: new SimpleChange(undefined, GroupMock.id, true),
-      });
-      fixture.detectChanges();
-    });
-
-    it('should show all ePerson members of 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: DebugElement) => {
-          return (foundEl.nativeElement.textContent.trim() === ePerson.uuid);
-        })).toBeTruthy();
-      });
-    });
-  });
-
 
     it('should replace the value when a new member is added when multipleReviewers is false', () => {
       spyOn(component.selectedReviewersUpdated, 'emit');
@@ -277,5 +256,25 @@ describe('ReviewersListComponent', () => {
       expect(component.selectedReviewers).toEqual([]);
       expect(component.selectedReviewersUpdated.emit).toHaveBeenCalledWith([]);
     });
+  });
+
+  describe('when a group is selected', () => {
+    beforeEach(() => {
+      component.ngOnChanges({
+        groupId: new SimpleChange(undefined, GroupMock.id, true),
+      });
+      fixture.detectChanges();
+    });
+
+    it('should show all ePerson members of 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: DebugElement) => {
+          return (foundEl.nativeElement.textContent.trim() === ePerson.uuid);
+        })).toBeTruthy();
+      });
+    });
+  });
 
 });

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

@@ -26,6 +26,10 @@ import {
   TranslateModule,
   TranslateService,
 } from '@ngx-translate/core';
+import {
+  Observable,
+  of as observableOf,
+} from 'rxjs';
 
 import {
   EPersonListActionConfig,
@@ -36,10 +40,12 @@ import { PaginatedList } from '../../../../core/data/paginated-list.model';
 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 { EpersonDtoModel } from '../../../../core/eperson/models/eperson-dto.model';
 import { Group } from '../../../../core/eperson/models/group.model';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
 import { ContextHelpDirective } from '../../../../shared/context-help.directive';
+import { hasValue } from '../../../../shared/empty.util';
 import { NotificationsService } from '../../../../shared/notifications/notifications.service';
 import { PaginationComponent } from '../../../../shared/pagination/pagination.component';
 
@@ -101,7 +107,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
     super(groupService, ePersonDataService, translateService, notificationsService, formBuilder, paginationService, router, dsoNameService);
   }
 
-  ngOnInit() {
+  override ngOnInit(): void {
     this.searchForm = this.formBuilder.group(({
       scope: 'metadata',
       query: '',
@@ -114,6 +120,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
       if (this.groupId === null) {
         this.retrieveMembers(this.config.currentPage);
       } else {
+        this.unsubFrom(SubKey.ActiveGroup);
         this.subs.set(SubKey.ActiveGroup, this.groupService.findById(this.groupId).pipe(
           getFirstSucceededRemoteDataPayload(),
         ).subscribe((activeGroup: Group) => {
@@ -136,25 +143,37 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
   retrieveMembers(page: number): void {
     this.config.currentPage = page;
     if (this.groupId === null) {
-      this.unsubFrom(SubKey.Members);
+      const paginatedListOfEPersons: PaginatedList<EpersonDtoModel> = new PaginatedList();
-      const paginatedListOfEPersons: PaginatedList<EPerson> = new PaginatedList();
+      paginatedListOfEPersons.page = this.selectedReviewers.map((ePerson: EPerson) => Object.assign(new EpersonDtoModel(), {
-      paginatedListOfEPersons.page = this.selectedReviewers;
+        eperson: ePerson,
+        ableToDelete: this.isMemberOfGroup(ePerson),
+      }));
       this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
     } else {
       super.retrieveMembers(page);
     }
   }
 
+  /**
+   * Checks whether the given {@link possibleMember} is part of the {@link selectedReviewers}.
+   *
+   * @param possibleMember The {@link EPerson} that needs to be checked
+   */
+  isMemberOfGroup(possibleMember: EPerson): Observable<boolean> {
+    return observableOf(hasValue(this.selectedReviewers.find((reviewer: EPerson) => reviewer.id === possibleMember.id)));
+  }
+
   /**
    * Removes the {@link eperson} from the {@link selectedReviewers}
    *
    * @param eperson The {@link EPerson} to remove
    */
   deleteMemberFromGroup(eperson: EPerson) {
-    const index = this.selectedReviewers.indexOf(eperson);
+    const index = this.selectedReviewers.findIndex((reviewer: EPerson) => reviewer.id === eperson.id);
     if (index !== -1) {
       this.selectedReviewers.splice(index, 1);
     }
+    this.retrieveMembers(this.config.currentPage);
     this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }
 
@@ -169,6 +188,7 @@ export class ReviewersListComponent extends MembersListComponent implements OnIn
       this.selectedReviewers = [];
     }
     this.selectedReviewers.push(eperson);
+    this.retrieveMembers(this.config.currentPage);
     this.selectedReviewersUpdated.emit(this.selectedReviewers);
   }