fix cache issues with edit metadata and resource policy pages

2025-10-07 01:54:15 +00:00 · 2021-02-05 17:37:59 +01:00
parent 43eb15349b
commit 2afc6a07f0
12 changed files with 104 additions and 107 deletions
--- a/karma.conf.js
+++ b/karma.conf.js
@@ -14,7 +14,8 @@ module.exports = function (config) {
      require('karma-mocha-reporter'),
    ],
    client: {
-      clearContext: false // leave Jasmine Spec Runner output visible in browser
+      clearContext: false, // leave Jasmine Spec Runner output visible in browser
+      captureConsole: false
    },
    coverageIstanbulReporter: {
      dir: require('path').join(__dirname, './coverage/dspace-angular'),
--- a/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts
+++ b/src/app/+item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts
@@ -1,16 +1,22 @@
-import { Component, OnInit } from '@angular/core';
-import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer';
-import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
+import { Component, OnInit, OnDestroy } from '@angular/core';
+import {
+  FieldUpdate,
+  FieldUpdates
+} from '../../../core/data/object-updates/object-updates.reducer';
+import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs';
 import { Item } from '../../../core/shared/item.model';
 import { ItemDataService } from '../../../core/data/item-data.service';
 import { ObjectUpdatesService } from '../../../core/data/object-updates/object-updates.service';
-import { ActivatedRoute, Router } from '@angular/router';
+import { ActivatedRoute, Router, Data } from '@angular/router';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { TranslateService } from '@ngx-translate/core';
-import { first, map } from 'rxjs/operators';
+import { first, map, switchMap, tap } from 'rxjs/operators';
 import { RemoteData } from '../../../core/data/remote-data';
 import { AbstractTrackableComponent } from '../../../shared/trackable/abstract-trackable.component';
 import { environment } from '../../../../environments/environment';
+import { ITEM_PAGE_LINKS_TO_FOLLOW } from '../../item-page.resolver';
+import { getAllSucceededRemoteData } from '../../../core/shared/operators';
+import { hasValue } from '../../../shared/empty.util';

@Component({
  selector: 'ds-abstract-item-update',
@@ -19,7 +25,7 @@ import { environment } from '../../../../environments/environment';
 /**
 * Abstract component for managing object updates of an item
 */
-export class AbstractItemUpdateComponent extends AbstractTrackableComponent implements OnInit {
+export class AbstractItemUpdateComponent extends AbstractTrackableComponent implements OnInit, OnDestroy {
  /**
   * The item to display the edit page for
   */
@@ -30,6 +36,12 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
   */
  updates$: Observable<FieldUpdates>;

+  /**
+   * A subscription that checks when the item is deleted in cache and reloads the item by sending a new request
+   * This is used to update the item in cache after bitstreams are deleted
+   */
+  itemUpdateSubscription: Subscription;
+
  constructor(
    public itemService: ItemDataService,
    public objectUpdatesService: ObjectUpdatesService,
@@ -45,14 +57,20 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
   * Initialize common properties between item-update components
   */
  ngOnInit(): void {
-    observableCombineLatest(this.route.data, this.route.parent.data).pipe(
-      map(([data, parentData]) => Object.assign({}, data, parentData)),
-      map((data) => data.dso),
-      first(),
-      map((data: RemoteData<Item>) => data.payload)
-    ).subscribe((item: Item) => {
-      this.item = item;
+    this.itemUpdateSubscription = observableCombineLatest([this.route.data, this.route.parent.data]).pipe(
+      map(([data, parentData]: [Data, Data]) => Object.assign({}, data, parentData)),
+      map((data: any) => data.dso),
+      tap((rd: RemoteData<Item>) => {
+        this.item = rd.payload;
+      }),
+      switchMap((rd: RemoteData<Item>) => {
+        return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...ITEM_PAGE_LINKS_TO_FOLLOW);
+      }),
+      getAllSucceededRemoteData()
+    ).subscribe((rd: RemoteData<Item>) => {
+      this.item = rd.payload;
      this.postItemInit();
+      this.initializeUpdates();
    });

    this.discardTimeOut = environment.item.edit.undoTimeout;
@@ -72,6 +90,12 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
    this.initializeUpdates();
  }

+  ngOnDestroy() {
+    if (hasValue(this.itemUpdateSubscription)) {
+      this.itemUpdateSubscription.unsubscribe();
+    }
+  }
+
  /**
   * Actions to perform after the item has been initialized
   * Abstract method: Should be overwritten in the sub class
--- a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts
+++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.spec.ts
@@ -134,6 +134,7 @@ describe('ItemBitstreamsComponent', () => {
    });
    itemService = Object.assign({
      getBitstreams: () => createSuccessfulRemoteDataObject$(createPaginatedList([bitstream1, bitstream2])),
+      findByHref: () => createSuccessfulRemoteDataObject$(item),
      findById: () => createSuccessfulRemoteDataObject$(item),
      getBundles: () => createSuccessfulRemoteDataObject$(createPaginatedList([bundle]))
    });
--- a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts
+++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.ts
@@ -12,11 +12,13 @@ import { hasValue, isNotEmpty } from '../../../shared/empty.util';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { RequestService } from '../../../core/data/request.service';
 import { getFirstSucceededRemoteData, getRemoteDataPayload } from '../../../core/shared/operators';
-import { Item } from '../../../core/shared/item.model';
 import { RemoteData } from '../../../core/data/remote-data';
 import { PaginatedList } from '../../../core/data/paginated-list.model';
 import { Bundle } from '../../../core/shared/bundle.model';
-import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer';
+import {
+  FieldUpdate,
+  FieldUpdates
+} from '../../../core/data/object-updates/object-updates.reducer';
 import { Bitstream } from '../../../core/shared/bitstream.model';
 import { FieldChangeType } from '../../../core/data/object-updates/object-updates.actions';
 import { BundleDataService } from '../../../core/data/bundle-data.service';
@@ -93,14 +95,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
    super(itemService, objectUpdatesService, router, notificationsService, translateService, route);
  }

-  /**
-   * Set up and initialize all fields
-   */
-  ngOnInit(): void {
-    super.ngOnInit();
-    this.initializeItemUpdate();
-  }
-
  /**
   * Actions to perform after the item has been initialized
   */
@@ -119,25 +113,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
    this.notificationsPrefix = 'item.edit.bitstreams.notifications.';
  }

-  /**
-   * Update the item (and view) when it's removed in the request cache
-   * Also re-initialize the original fields and updates
-   */
-  initializeItemUpdate(): void {
-    this.itemUpdateSubscription = this.requestService.hasByHref$(this.item.self).pipe(
-      filter((exists: boolean) => !exists),
-      switchMap(() => this.itemService.findById(this.item.uuid)),
-      getFirstSucceededRemoteData(),
-    ).subscribe((itemRD: RemoteData<Item>) => {
-      if (hasValue(itemRD)) {
-        this.item = itemRD.payload;
-        this.postItemInit();
-        this.initializeOriginalFields();
-        this.initializeUpdates();
-        this.cdRef.detectChanges();
-      }
-    });
-  }

  /**
   * Submit the current changes
@@ -274,7 +249,6 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
   */
  reset() {
    this.refreshItemCache();
-    this.initializeItemUpdate();
  }

  /**
--- a/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts
+++ b/src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.spec.ts
@@ -108,6 +108,11 @@ describe('ItemMetadataComponent', () => {
            [metadatum1.key]: [metadatum1],
            [metadatum2.key]: [metadatum2],
            [metadatum3.key]: [metadatum3]
+          },
+          _links: {
+            self: {
+              href: 'https://rest.api/core/items/a36d8bd2-8e8c-4969-9b1f-a574c2064983'
+            }
          }
        },
        {
--- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts
+++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts
@@ -133,6 +133,7 @@ describe('ItemRelationshipsComponent', () => {
    };

    itemService = jasmine.createSpyObj('itemService', {
+      findByHref: createSuccessfulRemoteDataObject$(item),
      findById: createSuccessfulRemoteDataObject$(item)
    });
    routeStub = {
--- a/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts
+++ b/src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts
@@ -7,8 +7,12 @@ import {
  RelationshipIdentifiable,
 } from '../../../core/data/object-updates/object-updates.reducer';
 import { Observable } from 'rxjs/internal/Observable';
-import { filter, map, startWith, switchMap, take } from 'rxjs/operators';
-import { combineLatest as observableCombineLatest, of as observableOf, zip as observableZip } from 'rxjs';
+import { map, startWith, switchMap, take } from 'rxjs/operators';
+import {
+  combineLatest as observableCombineLatest,
+  of as observableOf,
+  zip as observableZip
+} from 'rxjs';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
 import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
 import { ItemDataService } from '../../../core/data/item-data.service';
@@ -39,7 +43,6 @@ import { hasValue } from '../../../shared/empty.util';
 */
 export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {

-  itemRD$: Observable<RemoteData<Item>>;

  /**
   * The allowed relationship types for this type of item as an observable list
@@ -67,41 +70,6 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
    super(itemService, objectUpdatesService, router, notificationsService, translateService, route);
  }

-  /**
-   * Set up and initialize all fields
-   */
-  ngOnInit(): void {
-    super.ngOnInit();
-    this.initializeItemUpdate();
-  }
-
-  /**
-   * Update the item (and view) when it's removed in the request cache
-   */
-  public initializeItemUpdate(): void {
-    this.itemRD$ = this.requestService.hasByHref$(this.item.self).pipe(
-      filter((exists: boolean) => !exists),
-      switchMap(() => this.itemService.findById(
-        this.item.uuid,
-        true,
-        true,
-        followLink('owningCollection'),
-        followLink('bundles'),
-        followLink('relationships')),
-      ),
-      filter((itemRD) => !!itemRD.statusCode),
-    );
-
-    this.itemRD$.pipe(
-      getFirstSucceededRemoteData(),
-      getRemoteDataPayload(),
-    ).subscribe((item) => {
-      this.item = item;
-      this.cdr.detectChanges();
-      this.initializeUpdates();
-    });
-  }
-
  /**
   * Initialize the values and updates of the current item's relationship fields
   */
@@ -186,11 +154,9 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
      actions.forEach((action) =>
        action.subscribe((response) => {
          if (response.length > 0) {
-            this.itemRD$.subscribe(() => {
-              this.initializeOriginalFields();
-              this.cdr.detectChanges();
-              this.displayNotifications(response);
-            });
+            this.initializeOriginalFields();
+            this.cdr.detectChanges();
+            this.displayNotifications(response);
          }
        })
      );
@@ -261,6 +227,7 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
   * Sends all initial values of this item to the object updates service
   */
  public initializeOriginalFields() {
+    console.log('init');
    return this.relationshipService.getRelatedItems(this.item).pipe(
      take(1),
    ).subscribe((items: Item[]) => {
--- a/src/app/+item-page/item-page.resolver.ts
+++ b/src/app/+item-page/item-page.resolver.ts
@@ -4,10 +4,17 @@ import { Observable } from 'rxjs';
 import { RemoteData } from '../core/data/remote-data';
 import { ItemDataService } from '../core/data/item-data.service';
 import { Item } from '../core/shared/item.model';
-import { followLink } from '../shared/utils/follow-link-config.model';
+import { followLink, FollowLinkConfig } from '../shared/utils/follow-link-config.model';
 import { FindListOptions } from '../core/data/request.models';
 import { getFirstCompletedRemoteData } from '../core/shared/operators';

+export const ITEM_PAGE_LINKS_TO_FOLLOW: FollowLinkConfig<Item>[] = [
+  followLink('owningCollection'),
+  followLink('bundles', new FindListOptions(), true, true, true, followLink('bitstreams')),
+  followLink('relationships'),
+  followLink('version', undefined, true, true, true, followLink('versionhistory')),
+];
+
 /**
 * This class represents a resolver that requests a specific item before the route is activated
 */
@@ -27,10 +34,7 @@ export class ItemPageResolver implements Resolve<RemoteData<Item>> {
    return this.itemService.findById(route.params.id,
      true,
      false,
-      followLink('owningCollection'),
-      followLink('bundles', new FindListOptions(), true, true, true, followLink('bitstreams')),
-      followLink('relationships'),
-      followLink('version', undefined, true, true, true, followLink('versionhistory')),
+      ...ITEM_PAGE_LINKS_TO_FOLLOW
    ).pipe(
      getFirstCompletedRemoteData(),
    );
--- a/src/app/shared/resource-policies/form/resource-policy-form.component.html
+++ b/src/app/shared/resource-policies/form/resource-policy-form.component.html
@@ -6,7 +6,7 @@
           [displaySubmit]="false"></ds-form>
  <div class="container-fluid">
    <label for="ResourcePolicyObject">{{'resource-policies.form.eperson-group-list.label' | translate}}</label>
-    <input id="ResourcePolicyObject" class="form-control mb-3" type="text" readonly [value]="getResourcePolicyTargetName()">
+    <input id="ResourcePolicyObject" class="form-control mb-3" type="text" readonly [value]="resourcePolicyTargetName$ | async">
    <ngb-tabset *ngIf="canSetGrant()" type="pills">
      <ngb-tab [title]="'resource-policies.form.eperson-group-list.tab.eperson' | translate">
        <ng-template ngbTabContent>
--- a/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts
+++ b/src/app/shared/resource-policies/form/resource-policy-form.component.spec.ts
@@ -29,6 +29,7 @@ import { stringToNgbDateStruct } from '../../date.util';
 import { ResourcePolicy } from '../../../core/resource-policy/models/resource-policy.model';
 import { RESOURCE_POLICY } from '../../../core/resource-policy/models/resource-policy.resource-type';
 import { EPersonMock } from '../../testing/eperson.mock';
+import { isNotEmptyOperator } from '../../empty.util';

 export const mockResourcePolicyFormData = {
  name: [
@@ -307,14 +308,15 @@ describe('ResourcePolicyFormComponent test suite', () => {

    });

-    it('should init resourcePolicyGrant properly', () => {
+    it('should init resourcePolicyGrant properly', (done) => {
      compAsAny.isActive = true;
-
-      scheduler = getTestScheduler();
-      scheduler.schedule(() => comp.ngOnInit());
-      scheduler.flush();
-
-      expect(compAsAny.resourcePolicyGrant).toEqual(GroupMock);
+      comp.ngOnInit();
+      comp.resourcePolicyTargetName$.pipe(
+        isNotEmptyOperator()
+      ).subscribe(() => {
+        expect(compAsAny.resourcePolicyGrant).toEqual(GroupMock);
+        done();
+      });
    });

    it('should not can set grant', () => {
--- a/src/app/shared/resource-policies/form/resource-policy-form.component.ts
+++ b/src/app/shared/resource-policies/form/resource-policy-form.component.ts
@@ -1,6 +1,12 @@
 import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core';

-import { Observable, of as observableOf, race as observableRace, Subscription } from 'rxjs';
+import {
+  Observable,
+  of as observableOf,
+  combineLatest as observableCombineLatest,
+  Subscription,
+  BehaviorSubject
+} from 'rxjs';
 import { filter, map, take } from 'rxjs/operators';
 import {
  DynamicDatePickerModel,
@@ -26,7 +32,7 @@ import {
 import { DsDynamicTextAreaModel } from '../../form/builder/ds-dynamic-form-ui/models/ds-dynamic-textarea.model';
 import { DSpaceObject } from '../../../core/shared/dspace-object.model';
 import { DSONameService } from '../../../core/breadcrumbs/dso-name.service';
-import { hasValue, isEmpty, isNotEmpty } from '../../empty.util';
+import { hasValue, isEmpty, isNotEmpty, hasValueOperator } from '../../empty.util';
 import { FormService } from '../../form/form.service';
 import { RESOURCE_POLICY } from '../../../core/resource-policy/models/resource-policy.resource-type';
 import { RemoteData } from '../../../core/data/remote-data';
@@ -90,7 +96,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy {
  public formModel: DynamicFormControlModel[];

  /**
-   * The eperson or group that will be grant of the permission
+   * The eperson or group that will be granted the permission
   * @type {DSpaceObject}
   */
  public resourcePolicyGrant: DSpaceObject;
@@ -101,6 +107,12 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy {
   */
  public resourcePolicyGrantType: string;

+  /**
+   * The name of the eperson or group that will be granted the permission
+   * @type {BehaviorSubject<string>}
+   */
+  public resourcePolicyTargetName$: BehaviorSubject<string> = new BehaviorSubject('');
+
  /**
   * A boolean representing if component is active
   * @type {boolean}
@@ -146,12 +158,18 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy {
      const groupRD$ = this.groupService.findByHref(this.resourcePolicy._links.group.href, false).pipe(
        getFirstSucceededRemoteData()
      );
-      const dsoRD$: Observable<RemoteData<DSpaceObject>> = observableRace(epersonRD$, groupRD$);
+      const dsoRD$: Observable<RemoteData<DSpaceObject>> = observableCombineLatest([epersonRD$, groupRD$]).pipe(
+        map((rdArr: RemoteData<DSpaceObject>[]) => {
+          return rdArr.find((rd: RemoteData<DSpaceObject>) => isNotEmpty(rd.payload));
+        }),
+        hasValueOperator(),
+      );
      this.subs.push(
        dsoRD$.pipe(
          filter(() => this.isActive),
        ).subscribe((dsoRD: RemoteData<DSpaceObject>) => {
          this.resourcePolicyGrant = dsoRD.payload;
+          this.resourcePolicyTargetName$.next(this.getResourcePolicyTargetName());
        })
      );
    }
@@ -242,7 +260,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy {
  }

  /**
-   * Return the name of the eperson or group that will be grant of the permission
+   * Return the name of the eperson or group that will be granted the permission
   *
   * @return the object name
   */
@@ -251,7 +269,7 @@ export class ResourcePolicyFormComponent implements OnInit, OnDestroy {
  }

  /**
-   * Update reference to the eperson or group that will be grant of the permission
+   * Update reference to the eperson or group that will be granted the permission
   */
  updateObjectSelected(object: DSpaceObject, isEPerson: boolean): void {
    this.resourcePolicyGrant = object;
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -2728,7 +2728,7 @@

  "resource-policies.form.action-type.required": "You must select the resource policy action.",

-  "resource-policies.form.eperson-group-list.label": "The eperson or group that will be grant of the permission",
+  "resource-policies.form.eperson-group-list.label": "The eperson or group that will be granted the permission",

  "resource-policies.form.eperson-group-list.select.btn": "Select",