Merged dspace-cris-2024_02_x into task/dspace-cris-2024_02_x/DSC-2953

Author: Andrea Barbasso

bitbucket-pipelines.yml

@@ -1,6 +1,3 @@
-options:
-  runs-on: self.hosted
-
 definitions:
   caches:
     cypress-dsc-2024-02-x: ~/.cache/Cypress

src/app/admin/admin-import-batch-page/batch-import-page.component.html

@@ -21,7 +21,8 @@ <h1 id="header">{{'admin.batch-import.page.header' | translate}}</h1>
   </div>
 
   <ds-switch
-    [options]="switchOptions"
+    [onOption]="switchOnOption"
+    [offOption]="switchOffOption"
     [selectedValue]="isUpload ? 'upload' : 'url'"
     (selectedValueChange)="toggleUpload()">
   </ds-switch>
@@ -47,8 +48,13 @@ <h1 id="header">{{'admin.batch-import.page.header' | translate}}</h1>
     <button class="btn btn-secondary" id="backButton"
             (click)="this.onReturn();">{{'admin.metadata-import.page.button.return' | translate}}</button>
     <button class="btn btn-primary" id="proceedButton"
+            [dsBtnDisabled]="isProcessing"
             (click)="this.importMetadata();">
-            <i class="fas fa-arrow-right mr-1"></i>
+            @if (isProcessing) {
+              <i class="fas fa-spinner fa-spin me-1"></i>
+            } @else {
+              <i class="fas fa-arrow-right me-1"></i>
+            }
             {{'admin.metadata-import.page.button.proceed' | translate}}</button>
   </div>
 </div>

src/app/admin/admin-import-batch-page/batch-import-page.component.spec.ts

@@ -87,6 +87,40 @@ describe('BatchImportPageComponent', () => {
     expect(component).toBeTruthy();
   });
 
+  describe('processing state', () => {
+    beforeEach(() => {
+      component.fileURL = 'http://example.com/file.zip';
+    });
+
+    it('should disable proceed button when processing', () => {
+      component.isProcessing = true;
+      fixture.detectChanges();
+      const button = fixture.debugElement.query(By.css('#proceedButton')).nativeElement;
+      expect(button.getAttribute('aria-disabled')).toBe('true');
+    });
+
+    it('should show spinner icon when processing', () => {
+      component.isProcessing = true;
+      fixture.detectChanges();
+      const spinner = fixture.debugElement.query(By.css('#proceedButton .fa-spinner'));
+      expect(spinner).toBeTruthy();
+    });
+
+    it('should show arrow icon when not processing', () => {
+      component.isProcessing = false;
+      fixture.detectChanges();
+      const arrow = fixture.debugElement.query(By.css('#proceedButton .fa-arrow-right'));
+      expect(arrow).toBeTruthy();
+    });
+
+    it('should not invoke script if already processing', () => {
+      component.isProcessing = true;
+      const proceed = fixture.debugElement.query(By.css('#proceedButton')).nativeElement;
+      proceed.click();
+      expect(scriptService.invoke).not.toHaveBeenCalled();
+    });
+  });
+
   describe('if back button is pressed', () => {
     beforeEach(fakeAsync(() => {
       const proceed = fixture.debugElement.query(By.css('#backButton')).nativeElement;

src/app/admin/admin-import-batch-page/batch-import-page.component.ts

@@ -10,7 +10,10 @@ import {
   TranslateModule,
   TranslateService,
 } from '@ngx-translate/core';
-import { take } from 'rxjs/operators';
+import {
+  finalize,
+  take,
+} from 'rxjs/operators';
 
 import { DSONameService } from '../../core/breadcrumbs/dso-name.service';
 import {
@@ -23,6 +26,7 @@ import { getFirstCompletedRemoteData } from '../../core/shared/operators';
 import { getProcessDetailRoute } from '../../process-page/process-page-routing.paths';
 import { Process } from '../../process-page/processes/process.model';
 import { ProcessParameter } from '../../process-page/processes/process-parameter.model';
+import { BtnDisabledDirective } from '../../shared/btn-disabled.directive';
 import { ImportBatchSelectorComponent } from '../../shared/dso-selector/modal-wrappers/import-batch-selector/import-batch-selector.component';
 import {
   isEmpty,
@@ -43,6 +47,7 @@ import { FileDropzoneNoUploaderComponent } from '../../shared/upload/file-dropzo
     NgIf,
     TranslateModule,
     FormsModule,
+    BtnDisabledDirective,
     FileDropzoneNoUploaderComponent,
     SwitchComponent,
   ],
@@ -59,6 +64,11 @@ export class BatchImportPageComponent {
    */
   validateOnly = true;
 
+  /**
+   * Whether a batch import is currently being processed
+   */
+  isProcessing = false;
+
   /**
    * dso object for community or collection
    */
@@ -75,12 +85,14 @@ export class BatchImportPageComponent {
   fileURL: string;
 
   /**
-   * The custom options for the 'ds-switch' component
+   * The "on" option for the 'ds-switch' component (upload)
    */
-  switchOptions: SwitchOption[] = [
-    { value: 'upload', icon: 'fa fa-upload', label: 'admin.metadata-import.page.toggle.upload', iconColor: SwitchColor.Primary },
-    { value: 'url', icon: 'fa fa-link', label: 'admin.metadata-import.page.toggle.url', iconColor: SwitchColor.Primary },
-  ];
+  switchOnOption: SwitchOption = { value: 'upload', icon: 'fa fa-upload', label: 'admin.metadata-import.page.toggle.upload', iconColor: SwitchColor.Primary };
+
+  /**
+   * The "off" option for the 'ds-switch' component (url)
+   */
+  switchOffOption: SwitchOption = { value: 'url', icon: 'fa fa-link', label: 'admin.metadata-import.page.toggle.url', iconColor: SwitchColor.Primary };
 
   public constructor(private location: Location,
                      protected translate: TranslateService,
@@ -117,6 +129,9 @@ export class BatchImportPageComponent {
    * Starts import-metadata script with --zip fileName (and the selected file)
    */
   public importMetadata() {
+    if (this.isProcessing) {
+      return;
+    }
     if (this.fileObject == null && isEmpty(this.fileURL)) {
       if (this.isUpload) {
         this.notificationsService.error(this.translate.get('admin.metadata-import.page.error.addFile'));
@@ -140,8 +155,10 @@ export class BatchImportPageComponent {
         parameterValues.push(Object.assign(new ProcessParameter(), { name: '-v', value: true }));
       }
 
+      this.isProcessing = true;
       this.scriptDataService.invoke(BATCH_IMPORT_SCRIPT_NAME, parameterValues, [this.fileObject]).pipe(
         getFirstCompletedRemoteData(),
+        finalize(() => this.isProcessing = false),
       ).subscribe((rd: RemoteData<Process>) => {
         if (rd.hasSucceeded) {
           const title = this.translate.get('process.new.notification.success.title');

src/app/core/breadcrumbs/edit-item-breadcrumb.resolver.spec.ts

@@ -0,0 +1,49 @@
+import { getTestScheduler } from 'jasmine-marbles';
+
+import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
+import { Item } from '../shared/item.model';
+import { editItemBreadcrumbResolver } from './edit-item-breadcrumb.resolver';
+
+describe('editItemBreadcrumbResolver', () => {
+  describe('resolve', () => {
+    let resolver: any;
+    let dsoBreadcrumbService: any;
+    let itemService: any;
+    let testItem: Item;
+    let uuid;
+    let breadcrumbUrl;
+    let currentUrl;
+
+    beforeEach(() => {
+      uuid = '1234-65487-12354-1235';
+      breadcrumbUrl = `/items/${uuid}`;
+      currentUrl = `/edititems/${uuid}`;
+      testItem = Object.assign(new Item(), {
+        uuid: uuid,
+        type: 'item',
+      });
+      dsoBreadcrumbService = {};
+      itemService = {
+        findById: jasmine.createSpy('findById').and.returnValue(createSuccessfulRemoteDataObject$(testItem)),
+      };
+      resolver = editItemBreadcrumbResolver;
+    });
+
+    it('should resolve a breadcrumb config for the item when the route id is a plain uuid', () => {
+      const resolvedConfig = resolver({ params: { id: uuid } } as any, { url: currentUrl } as any, dsoBreadcrumbService, itemService);
+      const expectedConfig = { provider: dsoBreadcrumbService, key: testItem, url: breadcrumbUrl };
+      getTestScheduler().expectObservable(resolvedConfig).toBe('(a|)', { a: expectedConfig });
+      getTestScheduler().flush();
+      expect(itemService.findById.calls.mostRecent().args.slice(0, 2)).toEqual([uuid, true]);
+    });
+
+    it('should strip the workspace/workflow item id suffix before resolving, when the route id is composite', () => {
+      const compositeId = `${uuid}:12345`;
+      const resolvedConfig = resolver({ params: { id: compositeId } } as any, { url: currentUrl } as any, dsoBreadcrumbService, itemService);
+      const expectedConfig = { provider: dsoBreadcrumbService, key: testItem, url: breadcrumbUrl };
+      getTestScheduler().expectObservable(resolvedConfig).toBe('(a|)', { a: expectedConfig });
+      getTestScheduler().flush();
+      expect(itemService.findById.calls.mostRecent().args.slice(0, 2)).toEqual([uuid, true]);
+    });
+  });
+});

src/app/core/breadcrumbs/edit-item-breadcrumb.resolver.ts

@@ -0,0 +1,38 @@
+import { inject } from '@angular/core';
+import {
+  ActivatedRouteSnapshot,
+  ResolveFn,
+  RouterStateSnapshot,
+} from '@angular/router';
+import { Observable } from 'rxjs';
+
+import { BreadcrumbConfig } from '../../breadcrumbs/breadcrumb/breadcrumb-config.model';
+import { getItemPageLinksToFollow } from '../../item-page/item.resolver';
+import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
+import { ItemDataService } from '../data/item-data.service';
+import { DSpaceObject } from '../shared/dspace-object.model';
+import { Item } from '../shared/item.model';
+import { DSOBreadcrumbResolverByUuid } from './dso-breadcrumb.resolver';
+import { DSOBreadcrumbsService } from './dso-breadcrumbs.service';
+
+/**
+ * The resolve function that resolves the BreadcrumbConfig object for an Item from "Edit all details" page
+ */
+export const editItemBreadcrumbResolver: ResolveFn<BreadcrumbConfig<Item>> = (
+  route: ActivatedRouteSnapshot,
+  state: RouterStateSnapshot,
+  breadcrumbService: DSOBreadcrumbsService = inject(DSOBreadcrumbsService),
+  dataService: ItemDataService = inject(ItemDataService),
+): Observable<BreadcrumbConfig<Item>> => {
+  const linksToFollow: FollowLinkConfig<DSpaceObject>[] = getItemPageLinksToFollow() as FollowLinkConfig<DSpaceObject>[];
+  const itemIdString = route.params.id;
+  const itemId = itemIdString?.split(':')[0];
+  return DSOBreadcrumbResolverByUuid(
+    route,
+    state,
+    itemId,
+    breadcrumbService,
+    dataService,
+    ...linksToFollow,
+  ) as Observable<BreadcrumbConfig<Item>>;
+};

src/app/core/data/feature-authorization/authorization-data.service.spec.ts

@@ -333,5 +333,34 @@ describe('AuthorizationDataService', () => {
         });
       });
     });
+
+    describe('when the store-based authorization is not populated after loading (SSR fallback)', () => {
+      beforeEach(() => {
+        // The request finished loading but the NgRx authorization store was never populated for
+        // this feature. This happens during SSR: the store-based flow yields `undefined` even
+        // though the REST request succeeded. The service must then fall back to a direct REST
+        // authorization check instead of hanging or wrongly returning false.
+        authorizationService.getAuthorizationForObject = () => observableOf(undefined);
+        authorizationService.isRequestLoading = () => observableOf(false);
+      });
+
+      it('should fall back to a direct REST check and return true when the feature is granted', (done) => {
+        spyOn(service, 'searchByObject').and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList(validPayload)));
+        service.isAuthorized(featureID).subscribe((result) => {
+          expect(service.searchByObject).toHaveBeenCalled();
+          expect(result).toEqual(true);
+          done();
+        });
+      });
+
+      it('should fall back to a direct REST check and return false when the feature is not granted', (done) => {
+        spyOn(service, 'searchByObject').and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList(emptyPayload)));
+        service.isAuthorized(featureID).subscribe((result) => {
+          expect(service.searchByObject).toHaveBeenCalled();
+          expect(result).toEqual(false);
+          done();
+        });
+      });
+    });
   });
 });

src/app/core/data/feature-authorization/authorization-data.service.ts

@@ -120,28 +120,37 @@ export class AuthorizationDataService extends BaseDataService<Authorization> imp
           return dsoRequest$.pipe(
             take(1),
             // Get correct item and check that has not already pending authorizations
-            switchMap((object) => this.readOrFetchAuthorization(object, featureId, !objectUrl)),
+            switchMap((object) => this.readOrFetchAuthorization(object, featureId, !objectUrl, useCachedVersionIfAvailable, reRequestOnStale)),
           );
         } else {
           // we fallback on old method if site service had initialization issues or if some parameters more than the only feature ID are provided.
-          return this.searchByObject(featureId, objectUrl, ePersonUuid, {}, useCachedVersionIfAvailable, reRequestOnStale, followLink('feature')).pipe(
-            getFirstCompletedRemoteData(),
-            map((authorizationRD) => {
-              if (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page)) {
-                return authorizationRD.payload.page;
-              } else {
-                return [];
-              }
-            }),
-            catchError(() => observableOf([])),
-            oneAuthorizationMatchesFeature(featureId),
-          );
+          return this.searchByObjectAndMatchFeature(featureId, objectUrl, ePersonUuid, useCachedVersionIfAvailable, reRequestOnStale);
         }
       }),
     );
   }
 
-  private readOrFetchAuthorization(dso: DSpaceObject, featureId: FeatureID, isSite = false): Observable<boolean> {
+  /**
+   * Perform a direct authorization check via the REST "object" search endpoint, bypassing the
+   * NgRx authorization store. This reads the {@link RemoteData} directly, so it returns a reliable
+   * result even in situations where the store-based flow does not get populated (e.g. during SSR).
+   */
+  private searchByObjectAndMatchFeature(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string, useCachedVersionIfAvailable = true, reRequestOnStale = true): Observable<boolean> {
+    return this.searchByObject(featureId, objectUrl, ePersonUuid, {}, useCachedVersionIfAvailable, reRequestOnStale, followLink('feature')).pipe(
+      getFirstCompletedRemoteData(),
+      map((authorizationRD) => {
+        if (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page)) {
+          return authorizationRD.payload.page;
+        } else {
+          return [];
+        }
+      }),
+      catchError(() => observableOf([])),
+      oneAuthorizationMatchesFeature(featureId),
+    );
+  }
+
+  private readOrFetchAuthorization(dso: DSpaceObject, featureId: FeatureID, isSite = false, useCachedVersionIfAvailable = true, reRequestOnStale = true): Observable<boolean> {
     const requestId = getRequestIdFromParams(dso.uniqueType, [getNormalizedUuid(dso)], [featureId]);
     // if is the site init we wait for the authorization to be loaded otherwise services that run on resolver won't find a state.
     const waitForEntry$: Observable<boolean> = isSite
@@ -174,8 +183,18 @@ export class AuthorizationDataService extends BaseDataService<Authorization> imp
               take(1), // Ensure we only continue after loading finishes
               switchMap(() =>
                 this.authorizationService.getAuthorizationForObject(featureId, dso.self).pipe(
-                  filter(result => result !== undefined), // Ensure we only emit valid results
                   take(1),
+                  switchMap((result) => {
+                    if (result !== undefined) {
+                      return observableOf(result);
+                    }
+                    // The store-based flow did not yield a value even though the request finished.
+                    // This happens during SSR, where the NgRx authorization state is not populated
+                    // reliably (the guard then wrongly saw `false` and redirected to /403, while CSR
+                    // worked). Fall back to a direct REST authorization check, which reads the
+                    // RemoteData directly and returns a correct result.
+                    return this.searchByObjectAndMatchFeature(featureId, dso.self, undefined, useCachedVersionIfAvailable, reRequestOnStale);
+                  }),
                 ),
               ),
             );

src/app/core/notifications/suggestions/target/suggestion-target-data.service.spec.ts

@@ -120,7 +120,7 @@ describe('SuggestionTargetDataService test', () => {
       scheduler.schedule(() => service.getTargetsByUser('testId', options).subscribe());
       scheduler.flush();
 
-      expect(requestService.send).toHaveBeenCalledWith(expected, true);
+      expect(requestService.send).toHaveBeenCalledWith(expected, false);
     });
   });
 

src/app/core/notifications/suggestions/target/suggestion-target-data.service.ts

@@ -84,6 +84,8 @@ export class SuggestionTargetDataService extends IdentifiableDataService<Suggest
    *    The user Id for which to find targets.
    * @param options
    *    Find list options object.
+   * @param useCachedVersionIfAvailable
+   *    If this is true, the request will only be sent if there's no valid cached version. Defaults to true
    * @param linksToFollow
    *    List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved.
    * @return Observable<RemoteData<PaginatedList<SuggestionTarget>>>
@@ -92,12 +94,13 @@ export class SuggestionTargetDataService extends IdentifiableDataService<Suggest
   public getTargetsByUser(
     userId: string,
     options: FindListOptions = {},
+    useCachedVersionIfAvailable = false,
     ...linksToFollow: FollowLinkConfig<SuggestionTarget>[]
   ): Observable<RemoteData<PaginatedList<SuggestionTarget>>> {
     options.searchParams = [new RequestParam('target', userId)];
-
-    return this.searchBy(this.searchFindByTargetMethod, options, true, true, ...linksToFollow);
+    return this.searchBy(this.searchFindByTargetMethod, options, useCachedVersionIfAvailable, true, ...linksToFollow);
   }
+
   /**
    * Return a Suggestion Target for a given id
    *