Merge pull request #3089 from DSpace/backport-3079-to-dspace-7_x

[Port dspace-7_x] Fixed delete item page freezing when having relationships
2025-10-17 23:13:04 +00:00 · 2024-05-29 16:54:40 -05:00
parent 8c35ca86c8 7993020463
commit 8c6df7ac8e
10 changed files with 103 additions and 70 deletions
--- a/src/app/core/data/base/delete-data.spec.ts
+++ b/src/app/core/data/base/delete-data.spec.ts
@@ -198,6 +198,11 @@ describe('DeleteDataImpl', () => {
            method: RestRequestMethod.DELETE,
            href: 'some-href?copyVirtualMetadata=a&copyVirtualMetadata=b&copyVirtualMetadata=c',
          }));
+
+          const callback = (rdbService.buildFromRequestUUIDAndAwait as jasmine.Spy).calls.argsFor(0)[1];
+          callback();
+          expect(service.invalidateByHref).toHaveBeenCalledWith('some-href');
+
          done();
        });
      });
--- a/src/app/core/data/base/delete-data.ts
+++ b/src/app/core/data/base/delete-data.ts
@@ -68,15 +68,16 @@ export class DeleteDataImpl<T extends CacheableObject> extends IdentifiableDataS
  deleteByHref(href: string, copyVirtualMetadata?: string[]): Observable<RemoteData<NoContent>> {
    const requestId = this.requestService.generateRequestId();

+    let deleteHref: string = href;
    if (copyVirtualMetadata) {
      copyVirtualMetadata.forEach((id) =>
-        href += (href.includes('?') ? '&' : '?')
+        deleteHref += (deleteHref.includes('?') ? '&' : '?')
          + 'copyVirtualMetadata='
          + id,
      );
    }

-    const request = new DeleteRequest(requestId, href);
+    const request = new DeleteRequest(requestId, deleteHref);
    if (hasValue(this.responseMsToLive)) {
      request.responseMsToLive = this.responseMsToLive;
    }
--- a/src/app/item-page/edit-item-page/item-delete/item-delete.component.html
+++ b/src/app/item-page/edit-item-page/item-delete/item-delete.component.html
@@ -6,30 +6,29 @@
            <p>{{descriptionMessage | translate}}</p>
            <ds-modify-item-overview [item]="item"></ds-modify-item-overview>

-            <ng-container *ngVar="(types$ | async) as types">
+            <ng-container *ngVar="(typeDTOs$ | async) as types">

                <div *ngIf="types && types.length > 0" class="mb-4">

                    {{'virtual-metadata.delete-item.info' | translate}}

-                    <div *ngFor="let type of types" class="mb-4">
-
-                        <div *ngVar="(isSelected(type) | async) as selected"
+                    <div *ngFor="let typeDto of types" class="mb-4">
+                        <div *ngVar="(typeDto.isSelected$ | async) as selected"
                             class="d-flex flex-row">

-                            <div class="m-2" (click)="setSelected(type, !selected)">
+                            <div class="m-2" (click)="setSelected(typeDto.relationshipType, !selected)">
                                <label>
-                                    <input type="checkbox" [checked]="selected">
+                                    <input type="checkbox" [checked]="selected" [disabled]="isDeleting$ | async">
                                </label>
                            </div>

                            <div class="flex-column flex-grow-1">
-                                <h5 (click)="setSelected(type, !selected)">
-                                    {{getRelationshipMessageKey(getLabel(type) | async) | translate}}
+                                <h5 (click)="setSelected(typeDto.relationshipType, !selected)">
+                                    {{getRelationshipMessageKey(typeDto.label$ | async) | translate}}
                                </h5>
-                                <div *ngFor="let relationship of (getRelationships(type) | async)"
+                                <div *ngFor="let relationshipDto of (typeDto.relationshipDTOs$ | async)"
                                     class="d-flex flex-row">
-                                    <ng-container *ngVar="(getRelatedItem(relationship) | async) as relatedItem">
+                                    <ng-container *ngVar="(relationshipDto.relatedItem$ | async) as relatedItem">

                                        <ds-listable-object-component-loader
                                                *ngIf="relatedItem"
@@ -46,7 +45,7 @@
                                        </div>

                                        <ng-template #virtualMetadataModal>
-                                            <div>
+                                            <div class="thumb-font-1">
                                                <div class="modal-header">
                                                    {{'virtual-metadata.delete-item.modal-head' | translate}}
                                                    <button type="button" class="close"
@@ -60,7 +59,7 @@
                                                            [object]="relatedItem"
                                                            [viewMode]="viewMode">
                                                    </ds-listable-object-component-loader>
-                                                    <div *ngFor="let metadata of (getVirtualMetadata(relationship) | async)">
+                                                    <div *ngFor="let metadata of (relationshipDto.virtualMetadata$ | async)">
                                                        <div>
                                                            <div class="font-weight-bold">
                                                                {{metadata.metadataField}}
@@ -87,10 +86,11 @@
            </ng-container>

            <div class="space-children-mr">
-              <button (click)="performAction()"
+              <button [disabled]="isDeleting$ | async" (click)="performAction()"
                      class="btn btn-outline-secondary perform-action">{{confirmMessage | translate}}
              </button>
-              <button [routerLink]="[itemPageRoute, 'edit']" class="btn btn-outline-secondary cancel">
+              <button [disabled]="isDeleting$ | async" [routerLink]="[itemPageRoute, 'edit']"
+                      class="btn btn-outline-secondary cancel">
                {{cancelMessage| translate}}
              </button>
            </div>
--- a/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts
+++ b/src/app/item-page/edit-item-page/item-delete/item-delete.component.ts
@@ -1,3 +1,4 @@
+// eslint-disable-next-line max-classes-per-file
 import { Component, Input, OnInit, OnDestroy } from '@angular/core';
 import { defaultIfEmpty, filter, map, switchMap, take } from 'rxjs/operators';
 import {
@@ -37,6 +38,34 @@ import { getItemEditRoute } from '../../item-page-routing-paths';
 import { RemoteData } from '../../../core/data/remote-data';
 import { NoContent } from '../../../core/shared/NoContent.model';

+/**
+ * Data Transfer Object used to prevent the HTML template to call function returning Observables
+ */
+class RelationshipTypeDTO {
+
+  relationshipType: RelationshipType;
+
+  isSelected$: Observable<boolean>;
+
+  label$: Observable<string>;
+
+  relationshipDTOs$: Observable<RelationshipDTO[]>;
+
+}
+
+/**
+ * Data Transfer Object used to prevent the HTML template to call function returning Observables
+ */
+class RelationshipDTO {
+
+  relationship: Relationship;
+
+  relatedItem$: Observable<Item>;
+
+  virtualMetadata$: Observable<VirtualMetadata[]>;
+
+}
+
@Component({
  selector: 'ds-item-delete',
  templateUrl: '../item-delete/item-delete.component.html'
@@ -64,7 +93,7 @@ export class ItemDeleteComponent
   * A list of the relationship types for which this item has relations as an observable.
   * The list doesn't contain duplicates.
   */
-  types$: BehaviorSubject<RelationshipType[]> = new BehaviorSubject([]);
+  typeDTOs$: BehaviorSubject<RelationshipTypeDTO[]> = new BehaviorSubject([]);

  /**
   * A map which stores the relationships of this item for each type as observable lists
@@ -93,6 +122,8 @@ export class ItemDeleteComponent
   */
  private subs: Subscription[] = [];

+  public isDeleting$: BehaviorSubject<boolean> = new BehaviorSubject(false);
+
  constructor(protected route: ActivatedRoute,
              protected router: Router,
              protected notificationsService: NotificationsService,
@@ -146,14 +177,25 @@ export class ItemDeleteComponent
              }, [])
            ),
          );
-        })
-      ).subscribe((types: RelationshipType[]) => this.types$.next(types)));
+        }),
+      ).subscribe((types: RelationshipType[]) => this.typeDTOs$.next(types.map((relationshipType: RelationshipType) => Object.assign(new RelationshipTypeDTO(), {
+        relationshipType: relationshipType,
+        isSelected$: this.isSelected(relationshipType),
+        label$: this.getLabel(relationshipType),
+        relationshipDTOs$: this.getRelationships(relationshipType).pipe(
+          map((relationships: Relationship[]) => relationships.map((relationship: Relationship) => Object.assign(new RelationshipDTO(), {
+            relationship: relationship,
+            relatedItem$: this.getRelatedItem(relationship),
+            virtualMetadata$: this.getVirtualMetadata(relationship),
+          } as RelationshipDTO))),
+        ),
+      })))));
    }

-    this.subs.push(this.types$.pipe(
+    this.subs.push(this.typeDTOs$.pipe(
      take(1),
-    ).subscribe((types) =>
-      this.objectUpdatesService.initialize(this.url, types, this.item.lastModified)
+    ).subscribe((types: RelationshipTypeDTO[]) =>
+      this.objectUpdatesService.initialize(this.url, types.map((relationshipTypeDto: RelationshipTypeDTO) => relationshipTypeDto.relationshipType), this.item.lastModified),
    ));
  }

@@ -326,34 +368,33 @@ export class ItemDeleteComponent
   * @param selected  whether the type should be selected
   */
  setSelected(type: RelationshipType, selected: boolean): void {
-    this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected);
+    if (this.isDeleting$.value === false) {
+      this.objectUpdatesService.setSelectedVirtualMetadata(this.url, this.item.uuid, type.uuid, selected);
+    }
  }

  /**
   * Perform the delete operation
   */
-  performAction() {
-
-    this.subs.push(this.types$.pipe(
-      switchMap((types) =>
+  performAction(): void {
+    this.isDeleting$.next(true);
+    this.subs.push(this.typeDTOs$.pipe(
+      switchMap((types: RelationshipTypeDTO[]) =>
        combineLatest(
-          types.map((type) => this.isSelected(type))
+          types.map((type: RelationshipTypeDTO) => type.isSelected$),
        ).pipe(
          defaultIfEmpty([]),
-          map((selection) => types.filter(
-            (type, index) => selection[index]
+          map((selection: boolean[]) => types.filter(
+            (type: RelationshipTypeDTO, index: number) => selection[index],
          )),
-          map((selectedTypes) => selectedTypes.map((type) => type.id)),
-        )
+          map((selectedDtoTypes: RelationshipTypeDTO[]) => selectedDtoTypes.map((typeDto: RelationshipTypeDTO) => typeDto.relationshipType.id)),
+        ),
      ),
-      switchMap((types) =>
-        this.itemDataService.delete(this.item.id, types).pipe(getFirstCompletedRemoteData())
-      )
-    ).subscribe(
-      (rd: RemoteData<NoContent>) => {
-        this.notify(rd.hasSucceeded);
-      }
-    ));
+      switchMap((types: string[]) => this.itemDataService.delete(this.item.id, types)),
+      getFirstCompletedRemoteData(),
+    ).subscribe((rd: RemoteData<NoContent>) => {
+      this.notify(rd.hasSucceeded);
+    }));
  }

  /**
@@ -363,10 +404,10 @@ export class ItemDeleteComponent
  notify(succeeded: boolean) {
    if (succeeded) {
      this.notificationsService.success(this.translateService.get('item.edit.' + this.messageKey + '.success'));
-      this.router.navigate(['']);
+      void this.router.navigate(['']);
    } else {
      this.notificationsService.error(this.translateService.get('item.edit.' + this.messageKey + '.error'));
-      this.router.navigate([getItemEditRoute(this.item)]);
+      void this.router.navigate([getItemEditRoute(this.item)]);
    }
  }

--- a/src/app/item-page/full/full-item-page.component.spec.ts
+++ b/src/app/item-page/full/full-item-page.component.spec.ts
@@ -15,7 +15,6 @@ import { BehaviorSubject, of as observableOf } from 'rxjs';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { By } from '@angular/platform-browser';
 import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
-import { AuthService } from '../../core/auth/auth.service';
 import { createPaginatedList } from '../../shared/testing/utils.test';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { createRelationshipsObservable } from '../simple/item-types/shared/item.component.spec';
@@ -54,7 +53,6 @@ describe('FullItemPageComponent', () => {
  let comp: FullItemPageComponent;
  let fixture: ComponentFixture<FullItemPageComponent>;

-  let authService: AuthService;
  let routeStub: ActivatedRouteStub;
  let routeData;
  let authorizationDataService: AuthorizationDataService;
@@ -75,11 +73,6 @@ describe('FullItemPageComponent', () => {
  };

  beforeEach(waitForAsync(() => {
-    authService = jasmine.createSpyObj('authService', {
-      isAuthenticated: observableOf(true),
-      setRedirectUrl: {}
-    });
-
    routeData = {
      dso: createSuccessfulRemoteDataObject(mockItem),
    };
@@ -117,7 +110,6 @@ describe('FullItemPageComponent', () => {
        { provide: ActivatedRoute, useValue: routeStub },
        { provide: ItemDataService, useValue: {} },
        { provide: MetadataService, useValue: metadataServiceStub },
-        { provide: AuthService, useValue: authService },
        { provide: AuthorizationDataService, useValue: authorizationDataService },
        { provide: ServerResponseService, useValue: serverResponseService },
        { provide: SignpostingDataService, useValue: signpostingDataService },
--- a/src/app/item-page/full/full-item-page.component.ts
+++ b/src/app/item-page/full/full-item-page.component.ts
@@ -13,7 +13,6 @@ import { Item } from '../../core/shared/item.model';

 import { fadeInOut } from '../../shared/animations/fade';
 import { hasValue } from '../../shared/empty.util';
-import { AuthService } from '../../core/auth/auth.service';
 import { Location } from '@angular/common';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -49,7 +48,6 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
    protected route: ActivatedRoute,
    protected router: Router,
    protected items: ItemDataService,
-    protected authService: AuthService,
    protected authorizationService: AuthorizationDataService,
    protected _location: Location,
    protected responseService: ServerResponseService,
@@ -57,7 +55,7 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
    protected linkHeadService: LinkHeadService,
    @Inject(PLATFORM_ID) protected platformId: string,
  ) {
-    super(route, router, items, authService, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
+    super(route, router, items, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
  }

  /*** AoT inheritance fix, will hopefully be resolved in the near future **/
--- a/src/app/item-page/item-page.resolver.spec.ts
+++ b/src/app/item-page/item-page.resolver.spec.ts
@@ -3,14 +3,15 @@ import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { DSpaceObject } from '../core/shared/dspace-object.model';
 import { MetadataValueFilter } from '../core/shared/metadata.models';
 import { first } from 'rxjs/operators';
-import { Router } from '@angular/router';
+import { Router, RouterModule } from '@angular/router';
 import { TestBed } from '@angular/core/testing';
-import { RouterTestingModule } from '@angular/router/testing';
+import { AuthServiceStub } from '../shared/testing/auth-service.stub';
+import { AuthService } from '../core/auth/auth.service';

 describe('ItemPageResolver', () => {
  beforeEach(() => {
    TestBed.configureTestingModule({
-      imports: [RouterTestingModule.withRoutes([{
+      imports: [RouterModule.forRoot([{
        path: 'entities/:entity-type/:id',
        component: {} as any
      }])]
@@ -21,7 +22,8 @@ describe('ItemPageResolver', () => {
    let resolver: ItemPageResolver;
    let itemService: any;
    let store: any;
-    let router: any;
+    let router: Router;
+    let authService: AuthServiceStub;

    const uuid = '1234-65487-12354-1235';
    let item: DSpaceObject;
@@ -41,7 +43,8 @@ describe('ItemPageResolver', () => {
        store = jasmine.createSpyObj('store', {
          dispatch: {},
        });
-        resolver = new ItemPageResolver(itemService, store, router);
+        authService = new AuthServiceStub();
+        resolver = new ItemPageResolver(itemService, store, router, authService as unknown as AuthService);
      });

      it('should redirect to the correct route for the entity type', (done) => {
--- a/src/app/item-page/item-page.resolver.ts
+++ b/src/app/item-page/item-page.resolver.ts
@@ -9,6 +9,8 @@ import { map } from 'rxjs/operators';
 import { hasValue } from '../shared/empty.util';
 import { getItemPageRoute } from './item-page-routing-paths';
 import { ItemResolver } from './item.resolver';
+import { redirectOn4xx } from '../core/shared/authorized.operators';
+import { AuthService } from '../core/auth/auth.service';

 /**
 * This class represents a resolver that requests a specific item before the route is activated and will redirect to the
@@ -19,7 +21,8 @@ export class ItemPageResolver extends ItemResolver {
  constructor(
    protected itemService: ItemDataService,
    protected store: Store<any>,
-    protected router: Router
+    protected router: Router,
+    protected authService: AuthService,
  ) {
    super(itemService, store, router);
  }
@@ -33,6 +36,7 @@ export class ItemPageResolver extends ItemResolver {
   */
  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<RemoteData<Item>> {
    return super.resolve(route, state).pipe(
+      redirectOn4xx(this.router, this.authService),
      map((rd: RemoteData<Item>) => {
        if (rd.hasSucceeded && hasValue(rd.payload)) {
          const thisRoute = state.url;
--- a/src/app/item-page/simple/item-page.component.spec.ts
+++ b/src/app/item-page/simple/item-page.component.spec.ts
@@ -19,7 +19,6 @@ import {
  createSuccessfulRemoteDataObject,
  createSuccessfulRemoteDataObject$
 } from '../../shared/remote-data.utils';
-import { AuthService } from '../../core/auth/auth.service';
 import { createPaginatedList } from '../../shared/testing/utils.test';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -57,7 +56,6 @@ const mockSignpostingLinks: SignpostingLink[] = [mocklink, mocklink2];
 describe('ItemPageComponent', () => {
  let comp: ItemPageComponent;
  let fixture: ComponentFixture<ItemPageComponent>;
-  let authService: AuthService;
  let authorizationDataService: AuthorizationDataService;
  let serverResponseService: jasmine.SpyObj<ServerResponseService>;
  let signpostingDataService: jasmine.SpyObj<SignpostingDataService>;
@@ -74,10 +72,6 @@ describe('ItemPageComponent', () => {
  });

  beforeEach(waitForAsync(() => {
-    authService = jasmine.createSpyObj('authService', {
-      isAuthenticated: observableOf(true),
-      setRedirectUrl: {}
-    });
    authorizationDataService = jasmine.createSpyObj('authorizationDataService', {
      isAuthorized: observableOf(false),
    });
@@ -107,7 +101,6 @@ describe('ItemPageComponent', () => {
        { provide: ItemDataService, useValue: {} },
        { provide: MetadataService, useValue: mockMetadataService },
        { provide: Router, useValue: {} },
-        { provide: AuthService, useValue: authService },
        { provide: AuthorizationDataService, useValue: authorizationDataService },
        { provide: ServerResponseService, useValue: serverResponseService },
        { provide: SignpostingDataService, useValue: signpostingDataService },
--- a/src/app/item-page/simple/item-page.component.ts
+++ b/src/app/item-page/simple/item-page.component.ts
@@ -11,9 +11,7 @@ import { Item } from '../../core/shared/item.model';
 import { fadeInOut } from '../../shared/animations/fade';
 import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators';
 import { ViewMode } from '../../core/shared/view-mode.model';
-import { AuthService } from '../../core/auth/auth.service';
 import { getItemPageRoute } from '../item-page-routing-paths';
-import { redirectOn4xx } from '../../core/shared/authorized.operators';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -72,7 +70,6 @@ export class ItemPageComponent implements OnInit, OnDestroy {
    protected route: ActivatedRoute,
    protected router: Router,
    protected items: ItemDataService,
-    protected authService: AuthService,
    protected authorizationService: AuthorizationDataService,
    protected responseService: ServerResponseService,
    protected signpostingDataService: SignpostingDataService,
@@ -88,7 +85,6 @@ export class ItemPageComponent implements OnInit, OnDestroy {
  ngOnInit(): void {
    this.itemRD$ = this.route.data.pipe(
      map((data) => data.dso as RemoteData<Item>),
-      redirectOn4xx(this.router, this.authService)
    );
    this.itemPageRoute$ = this.itemRD$.pipe(
      getAllSucceededRemoteDataPayload(),