Merge branch 'DSpace:main' into patch-21-squashed

Author: saschaszott

cypress/e2e/item-edit.cy.ts

@@ -18,6 +18,11 @@ describe('Edit Item > Edit Metadata tab', () => {
     // <ds-edit-item-page> tag must be loaded
     cy.get('ds-edit-item-page').should('be.visible');
 
+    // wait for all the ds-dso-edit-metadata-value components to be rendered
+    cy.get('ds-dso-edit-metadata-value div[role="row"]').each(($row: HTMLDivElement) => {
+      cy.wrap($row).find('div[role="cell"]').should('be.visible');
+    });
+
     // Analyze <ds-edit-item-page> for accessibility issues
     testA11y('ds-edit-item-page');
   });

cypress/e2e/submission.cy.ts

@@ -137,7 +137,7 @@ describe('New Submission page', () => {
 
     // Upload our DSpace logo via drag & drop onto submission form
     // cy.get('div#section_upload')
-    cy.get('div.ds-document-drop-zone').selectFile('src/assets/images/dspace-logo.png', {
+    cy.get('div.ds-document-drop-zone').selectFile('src/assets/images/dspace-logo.svg', {
       action: 'drag-drop',
     });
 

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

@@ -1,16 +1,13 @@
 import { AbstractControl } from '@angular/forms';
-import {
-  mapToCanActivate,
-  Route,
-} from '@angular/router';
+import { Route } from '@angular/router';
 import {
   DYNAMIC_ERROR_MESSAGES_MATCHER,
   DynamicErrorMessagesMatcher,
 } from '@ng-dynamic-forms/core';
 
 import { i18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver';
-import { GroupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard';
-import { SiteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
+import { groupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard';
+import { siteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
 import {
   EPERSON_PATH,
   GROUP_PATH,
@@ -20,15 +17,15 @@ import { EPeopleRegistryComponent } from './epeople-registry/epeople-registry.co
 import { EPersonFormComponent } from './epeople-registry/eperson-form/eperson-form.component';
 import { EPersonResolver } from './epeople-registry/eperson-resolver.service';
 import { GroupFormComponent } from './group-registry/group-form/group-form.component';
-import { GroupPageGuard } from './group-registry/group-page.guard';
+import { groupPageGuard } from './group-registry/group-page.guard';
 import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
 
 /**
  * Condition for displaying error messages on email form field
  */
 export const ValidateEmailErrorStateMatcher: DynamicErrorMessagesMatcher =
   (control: AbstractControl, model: any, hasFocus: boolean) => {
-    return (control.touched && !hasFocus) || (control.errors?.emailTaken && hasFocus);
+    return ( control.touched && !hasFocus ) || ( control.errors?.emailTaken && hasFocus );
   };
 
 const providers = [
@@ -46,7 +43,7 @@ export const ROUTES: Route[] = [
     },
     providers,
     data: { title: 'admin.access-control.epeople.title', breadcrumbKey: 'admin.access-control.epeople' },
-    canActivate: mapToCanActivate([SiteAdministratorGuard]),
+    canActivate: [siteAdministratorGuard],
   },
   {
     path: `${EPERSON_PATH}/create`,
@@ -56,7 +53,7 @@ export const ROUTES: Route[] = [
     },
     providers,
     data: { title: 'admin.access-control.epeople.add.title', breadcrumbKey: 'admin.access-control.epeople.add' },
-    canActivate: mapToCanActivate([SiteAdministratorGuard]),
+    canActivate: [siteAdministratorGuard],
   },
   {
     path: `${EPERSON_PATH}/:id/edit`,
@@ -67,7 +64,7 @@ export const ROUTES: Route[] = [
     },
     providers,
     data: { title: 'admin.access-control.epeople.edit.title', breadcrumbKey: 'admin.access-control.epeople.edit' },
-    canActivate: mapToCanActivate([SiteAdministratorGuard]),
+    canActivate: [siteAdministratorGuard],
   },
   {
     path: GROUP_PATH,
@@ -77,7 +74,7 @@ export const ROUTES: Route[] = [
     },
     providers,
     data: { title: 'admin.access-control.groups.title', breadcrumbKey: 'admin.access-control.groups' },
-    canActivate: mapToCanActivate([GroupAdministratorGuard]),
+    canActivate: [groupAdministratorGuard],
   },
   {
     path: `${GROUP_PATH}/create`,
@@ -90,7 +87,7 @@ export const ROUTES: Route[] = [
       title: 'admin.access-control.groups.title.addGroup',
       breadcrumbKey: 'admin.access-control.groups.addGroup',
     },
-    canActivate: mapToCanActivate([GroupAdministratorGuard]),
+    canActivate: [groupAdministratorGuard],
   },
   {
     path: `${GROUP_PATH}/:groupId/edit`,
@@ -103,7 +100,7 @@ export const ROUTES: Route[] = [
       title: 'admin.access-control.groups.title.singleGroup',
       breadcrumbKey: 'admin.access-control.groups.singleGroup',
     },
-    canActivate: mapToCanActivate([GroupPageGuard]),
+    canActivate: [groupPageGuard],
   },
   {
     path: 'bulk-access',
@@ -112,6 +109,6 @@ export const ROUTES: Route[] = [
       breadcrumb: i18nBreadcrumbResolver,
     },
     data: { title: 'admin.access-control.bulk-access.title', breadcrumbKey: 'admin.access-control.bulk-access' },
-    canActivate: mapToCanActivate([SiteAdministratorGuard]),
+    canActivate: [siteAdministratorGuard],
   },
 ];

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

@@ -20,25 +20,33 @@ <h3>{{messagePrefix + '.headMembers' | translate}}</h3>
         </tr>
         </thead>
         <tbody>
-        <tr *ngFor="let eperson of (ePeopleMembersOfGroup | async)?.page">
-          <td class="align-middle">{{eperson.id}}</td>
+        <tr *ngFor="let epersonDTO of (ePeopleMembersOfGroup | async)?.page">
+          <td class="align-middle">{{epersonDTO.eperson.id}}</td>
           <td class="align-middle">
-            <a [routerLink]="getEPersonEditRoute(eperson.id)">
-              {{ dsoNameService.getName(eperson) }}
+            <a [routerLink]="getEPersonEditRoute(epersonDTO.eperson.id)">
+              {{ dsoNameService.getName(epersonDTO.eperson) }}
             </a>
           </td>
           <td class="align-middle">
-            {{messagePrefix + '.table.email' | translate}}: {{ eperson.email ? eperson.email : '-' }}<br/>
-            {{messagePrefix + '.table.netid' | translate}}: {{ eperson.netid ? eperson.netid : '-' }}
+            {{messagePrefix + '.table.email' | translate}}: {{ epersonDTO.eperson.email ? epersonDTO.eperson.email : '-' }}<br/>
+            {{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.', () => {
-        const epersonsFound = fixture.debugElement.queryAll(By.css('#ePeopleMembersOfGroup tbody tr'));
-        expect(epersonsFound.length).toEqual(0);
+      it('should trigger the search to add the user back to the search table', () => {
+        expect(component.search).toHaveBeenCalled();
       });
     });
   });
@@ -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', () => {
-          epersonsFound = fixture.debugElement.queryAll(By.css('#epersonsSearch tbody tr'));
-          expect(epersonsFound.length).toEqual(0);
+        it('should trigger the search to remove the user from the search table', () => {
+          expect(component.search).toHaveBeenCalled();
         });
       });
     });

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())
-        .subscribe((paginatedListOfEPersons: PaginatedList<EPerson>) => {
-          this.ePeopleMembersOfGroup.next(paginatedListOfEPersons);
-        }));
+        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.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/access-control/group-registry/group-page.guard.spec.ts

@@ -1,14 +1,24 @@
+import {
+  TestBed,
+  waitForAsync,
+} from '@angular/core/testing';
 import {
   ActivatedRouteSnapshot,
   Router,
+  UrlTree,
 } from '@angular/router';
-import { of as observableOf } from 'rxjs';
+import {
+  Observable,
+  of as observableOf,
+} from 'rxjs';
 
 import { AuthService } from '../../core/auth/auth.service';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { HALEndpointService } from '../../core/shared/hal-endpoint.service';
-import { GroupPageGuard } from './group-page.guard';
+import { groupPageGuard } from './group-page.guard';
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; // Increase timeout to 10 seconds
 
 describe('GroupPageGuard', () => {
   const groupsEndpointUrl = 'https://test.org/api/eperson/groups';
@@ -20,42 +30,54 @@ describe('GroupPageGuard', () => {
     },
   } as unknown as ActivatedRouteSnapshot;
 
-  let guard: GroupPageGuard;
   let halEndpointService: HALEndpointService;
   let authorizationService: AuthorizationDataService;
   let router: Router;
   let authService: AuthService;
 
-  beforeEach(() => {
+  function init() {
     halEndpointService = jasmine.createSpyObj(['getEndpoint']);
-    (halEndpointService as any).getEndpoint.and.returnValue(observableOf(groupsEndpointUrl));
+    ( halEndpointService as any ).getEndpoint.and.returnValue(observableOf(groupsEndpointUrl));
 
     authorizationService = jasmine.createSpyObj(['isAuthorized']);
     // NOTE: value is set in beforeEach
 
     router = jasmine.createSpyObj(['parseUrl']);
-    (router as any).parseUrl.and.returnValue = {};
+    ( router as any ).parseUrl.and.returnValue = {};
 
     authService = jasmine.createSpyObj(['isAuthenticated']);
-    (authService as any).isAuthenticated.and.returnValue(observableOf(true));
+    ( authService as any ).isAuthenticated.and.returnValue(observableOf(true));
 
-    guard = new GroupPageGuard(halEndpointService, authorizationService, router, authService);
-  });
+    TestBed.configureTestingModule({
+      providers: [
+        { provide: AuthorizationDataService, useValue: authorizationService },
+        { provide: Router, useValue: router },
+        { provide: AuthService, useValue: authService },
+        { provide: HALEndpointService, useValue: halEndpointService },
+      ],
+    });
+  }
+
+  beforeEach(waitForAsync(() => {
+    init();
+  }));
 
   it('should be created', () => {
-    expect(guard).toBeTruthy();
+    expect(groupPageGuard).toBeTruthy();
   });
 
   describe('canActivate', () => {
     describe('when the current user can manage the group', () => {
       beforeEach(() => {
-        (authorizationService as any).isAuthorized.and.returnValue(observableOf(true));
+        ( authorizationService as any ).isAuthorized.and.returnValue(observableOf(true));
       });
 
       it('should return true', (done) => {
-        guard.canActivate(
-          routeSnapshotWithGroupId, { url: 'current-url' } as any,
-        ).subscribe((result) => {
+        const result$ = TestBed.runInInjectionContext(() => {
+          return groupPageGuard()(routeSnapshotWithGroupId, { url: 'current-url' } as any);
+        }) as Observable<boolean | UrlTree>;
+
+        result$.subscribe((result) => {
           expect(authorizationService.isAuthorized).toHaveBeenCalledWith(
             FeatureID.CanManageGroup, groupEndpointUrl, undefined,
           );
@@ -71,15 +93,18 @@ describe('GroupPageGuard', () => {
       });
 
       it('should not return true', (done) => {
-        guard.canActivate(
-          routeSnapshotWithGroupId, { url: 'current-url' } as any,
-        ).subscribe((result) => {
+        const result$ = TestBed.runInInjectionContext(() => {
+          return groupPageGuard()(routeSnapshotWithGroupId, { url: 'current-url' } as any);
+        }) as Observable<boolean | UrlTree>;
+
+        result$.subscribe((result) => {
           expect(authorizationService.isAuthorized).toHaveBeenCalledWith(
             FeatureID.CanManageGroup, groupEndpointUrl, undefined,
           );
           expect(result).not.toBeTrue();
           done();
         });
+
       });
     });
   });