61142: EditRelationshipList component to proper reload relationships and fix performance issues

src/app/+item-page/edit-item-page/edit-item-page.module.ts

@@ -17,6 +17,7 @@ import { EditInPlaceFieldComponent } from './item-metadata/edit-in-place-field/e
 import { ItemBitstreamsComponent } from './item-bitstreams/item-bitstreams.component';
 import { ItemRelationshipsComponent } from './item-relationships/item-relationships.component';
 import { EditRelationshipComponent } from './item-relationships/edit-relationship/edit-relationship.component';
+import { EditRelationshipListComponent } from './item-relationships/edit-relationship-list/edit-relationship-list.component';
 
 /**
  * Module that contains all components related to the Edit Item page administrator functionality
@@ -42,7 +43,8 @@ import { EditRelationshipComponent } from './item-relationships/edit-relationshi
     ItemRelationshipsComponent,
     ItemBitstreamsComponent,
     EditInPlaceFieldComponent,
-    EditRelationshipComponent
+    EditRelationshipComponent,
+    EditRelationshipListComponent
   ]
 })
 export class EditItemPageModule {

src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.html

@@ -0,0 +1,15 @@
+<ng-container *ngVar="(updates$ | async) as updates">
+  <div *ngIf="updates">
+    <h5>{{getRelationshipMessageKey(relationshipLabel) | translate}}</h5>
+    <ng-container *ngVar="(updates | dsObjectValues) as updateValues">
+      <div *ngFor="let updateValue of updateValues; trackBy: trackUpdate"
+           ds-edit-relationship
+           class="relationship-row d-block"
+           [fieldUpdate]="updateValue || {}"
+           [url]="url"
+           [ngClass]="{'alert alert-danger': updateValue.changeType === 2}">
+      </div>
+      <ds-loading *ngIf="updateValues.length == 0" message="{{'loading.items' | translate}}"></ds-loading>
+    </ng-container>
+  </div>
+</ng-container>

src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.scss

@@ -0,0 +1,12 @@
+@import '../../../../../styles/variables.scss';
+
+.relationship-row:not(.alert-danger) {
+  padding: $alert-padding-y 0;
+}
+
+.relationship-row.alert-danger {
+  margin-left: -$alert-padding-x;
+  margin-right: -$alert-padding-x;
+  margin-top: -1px;
+  margin-bottom: -1px;
+}

src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts

@@ -0,0 +1,137 @@
+import { EditRelationshipListComponent } from './edit-relationship-list.component';
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
+import { RelationshipType } from '../../../../core/shared/item-relationships/relationship-type.model';
+import { ResourceType } from '../../../../core/shared/resource-type';
+import { Relationship } from '../../../../core/shared/item-relationships/relationship.model';
+import { of as observableOf } from 'rxjs/internal/observable/of';
+import { RemoteData } from '../../../../core/data/remote-data';
+import { Item } from '../../../../core/shared/item.model';
+import { PaginatedList } from '../../../../core/data/paginated-list';
+import { PageInfo } from '../../../../core/shared/page-info.model';
+import { FieldChangeType } from '../../../../core/data/object-updates/object-updates.actions';
+import { SharedModule } from '../../../../shared/shared.module';
+import { TranslateModule } from '@ngx-translate/core';
+import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service';
+import { RelationshipService } from '../../../../core/data/relationship.service';
+import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core';
+import { By } from '@angular/platform-browser';
+
+let comp: EditRelationshipListComponent;
+let fixture: ComponentFixture<EditRelationshipListComponent>;
+let de: DebugElement;
+
+let objectUpdatesService;
+let relationshipService;
+
+const url = 'http://test-url.com/test-url';
+
+let item;
+let author1;
+let author2;
+let fieldUpdate1;
+let fieldUpdate2;
+let relationships;
+let relationshipType;
+
+describe('EditRelationshipListComponent', () => {
+  beforeEach(async(() => {
+    relationshipType = Object.assign(new RelationshipType(), {
+      type: ResourceType.RelationshipType,
+      id: '1',
+      uuid: '1',
+      leftLabel: 'isAuthorOfPublication',
+      rightLabel: 'isPublicationOfAuthor'
+    });
+
+    relationships = [
+      Object.assign(new Relationship(), {
+        self: url + '/2',
+        id: '2',
+        uuid: '2',
+        leftId: 'author1',
+        rightId: 'publication',
+        relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType))
+      }),
+      Object.assign(new Relationship(), {
+        self: url + '/3',
+        id: '3',
+        uuid: '3',
+        leftId: 'author2',
+        rightId: 'publication',
+        relationshipType: observableOf(new RemoteData(false, false, true, undefined, relationshipType))
+      })
+    ];
+
+    item = Object.assign(new Item(), {
+      self: 'fake-item-url/publication',
+      id: 'publication',
+      uuid: 'publication',
+      relationships: observableOf(new RemoteData(false, false, true, undefined, new PaginatedList(new PageInfo(), relationships)))
+    });
+
+    author1 = Object.assign(new Item(), {
+      id: 'author1',
+      uuid: 'author1'
+    });
+    author2 = Object.assign(new Item(), {
+      id: 'author2',
+      uuid: 'author2'
+    });
+
+    fieldUpdate1 = {
+      field: author1,
+      changeType: undefined
+    };
+    fieldUpdate2 = {
+      field: author2,
+      changeType: FieldChangeType.REMOVE
+    };
+
+    objectUpdatesService = jasmine.createSpyObj('objectUpdatesService',
+      {
+        getFieldUpdatesExclusive: observableOf({
+          [author1.uuid]: fieldUpdate1,
+          [author2.uuid]: fieldUpdate2
+        })
+      }
+    );
+
+    relationshipService = jasmine.createSpyObj('relationshipService',
+      {
+        getRelatedItemsByLabel: observableOf([author1, author2]),
+      }
+    );
+
+    TestBed.configureTestingModule({
+      imports: [SharedModule, TranslateModule.forRoot()],
+      declarations: [EditRelationshipListComponent],
+      providers: [
+        { provide: ObjectUpdatesService, useValue: objectUpdatesService },
+        { provide: RelationshipService, useValue: relationshipService }
+      ], schemas: [
+        NO_ERRORS_SCHEMA
+      ]
+    }).compileComponents();
+  }));
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(EditRelationshipListComponent);
+    comp = fixture.componentInstance;
+    de = fixture.debugElement;
+    comp.item = item;
+    comp.url = url;
+    comp.relationshipLabel = relationshipType.leftLabel;
+    fixture.detectChanges();
+  });
+
+  describe('changeType is REMOVE', () => {
+    beforeEach(() => {
+      fieldUpdate1.changeType = FieldChangeType.REMOVE;
+      fixture.detectChanges();
+    });
+    it('the div should have class alert-danger', () => {
+      const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement;
+      expect(element.classList).toContain('alert-danger');
+    });
+  });
+});

src/app/+item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts

@@ -0,0 +1,99 @@
+import { Component, Input, OnChanges, OnInit, SimpleChanges } from '@angular/core';
+import { ObjectUpdatesService } from '../../../../core/data/object-updates/object-updates.service';
+import { Observable } from 'rxjs/internal/Observable';
+import { FieldUpdate, FieldUpdates } from '../../../../core/data/object-updates/object-updates.reducer';
+import { RelationshipService } from '../../../../core/data/relationship.service';
+import { Item } from '../../../../core/shared/item.model';
+import { switchMap } from 'rxjs/operators';
+import { hasValue } from '../../../../shared/empty.util';
+
+@Component({
+  selector: 'ds-edit-relationship-list',
+  styleUrls: ['./edit-relationship-list.component.scss'],
+  templateUrl: './edit-relationship-list.component.html',
+})
+/**
+ * A component creating a list of editable relationships of a certain type
+ * The relationships are rendered as a list of related items
+ */
+export class EditRelationshipListComponent implements OnInit, OnChanges {
+  /**
+   * The item to display related items for
+   */
+  @Input() item: Item;
+
+  /**
+   * The URL to the current page
+   * Used to fetch updates for the current item from the store
+   */
+  @Input() url: string;
+
+  /**
+   * The label of the relationship-type we're rendering a list for
+   */
+  @Input() relationshipLabel: string;
+
+  /**
+   * The FieldUpdates for the relationships in question
+   */
+  updates$: Observable<FieldUpdates>;
+
+  constructor(
+    protected objectUpdatesService: ObjectUpdatesService,
+    protected relationshipService: RelationshipService
+  ) {
+  }
+
+  ngOnInit(): void {
+    this.initUpdates();
+  }
+
+  ngOnChanges(changes: SimpleChanges): void {
+    this.initUpdates();
+  }
+
+  /**
+   * Initialize the FieldUpdates using the related items
+   */
+  initUpdates() {
+    this.updates$ = this.getUpdatesByLabel(this.relationshipLabel);
+  }
+
+  /**
+   * Transform the item's relationships of a specific type into related items
+   * @param label   The relationship type's label
+   */
+  public getRelatedItemsByLabel(label: string): Observable<Item[]> {
+    return this.relationshipService.getRelatedItemsByLabel(this.item, label);
+  }
+
+  /**
+   * Get FieldUpdates for the relationships of a specific type
+   * @param label   The relationship type's label
+   */
+  public getUpdatesByLabel(label: string): Observable<FieldUpdates> {
+    return this.getRelatedItemsByLabel(label).pipe(
+      switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items))
+    )
+  }
+
+  /**
+   * Get the i18n message key for a relationship
+   * @param label   The relationship type's label
+   */
+  public getRelationshipMessageKey(label: string): string {
+    if (hasValue(label) && label.indexOf('Of') > -1) {
+      return `relationships.${label.substring(0, label.indexOf('Of') + 2)}`
+    } else {
+      return label;
+    }
+  }
+
+  /**
+   * Prevent unnecessary rerendering so fields don't lose focus
+   */
+  trackUpdate(index, update: FieldUpdate) {
+    return update && update.field ? update.field.uuid : undefined;
+  }
+
+}

src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.html

@@ -18,21 +18,7 @@
     </button>
   </div>
   <div *ngFor="let label of relationLabels$ | async" class="mb-4">
-    <ng-container *ngVar="(getUpdatesByLabel(label) | async) as updates">
+    <ds-edit-relationship-list [item]="item" [url]="url" [relationshipLabel]="label" ></ds-edit-relationship-list>
-      <div *ngIf="updates">
-        <h5>{{getRelationshipMessageKey(label) | translate}}</h5>
-        <ng-container *ngVar="(updates | dsObjectValues) as updateValues">
-          <div *ngFor="let updateValue of updateValues; trackBy: trackUpdate"
-               ds-edit-relationship
-               class="relationship-row d-block"
-               [fieldUpdate]="updateValue || {}"
-               [url]="url"
-               [ngClass]="{'alert alert-danger': updateValue.changeType === 2}">
-          </div>
-          <ds-loading *ngIf="updateValues.length == 0" message="{{'loading.items' | translate}}"></ds-loading>
-        </ng-container>
-      </div>
-    </ng-container>
   </div>
   <div class="button-row bottom">
     <div class="float-right">

src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.scss

@@ -20,14 +20,3 @@
 
 
 }
-
-.relationship-row:not(.alert-danger) {
-  padding: $alert-padding-y 0;
-}
-
-.relationship-row.alert-danger {
-  margin-left: -$alert-padding-x;
-  margin-right: -$alert-padding-x;
-  margin-top: -1px;
-  margin-bottom: -1px;
-}

src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.spec.ts

@@ -1,6 +1,6 @@
 import { async, ComponentFixture, TestBed } from '@angular/core/testing';
 import { ItemRelationshipsComponent } from './item-relationships.component';
-import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core';
+import { ChangeDetectorRef, DebugElement, NO_ERRORS_SCHEMA } from '@angular/core';
 import { INotification, Notification } from '../../../shared/notifications/models/notification.model';
 import { NotificationType } from '../../../shared/notifications/models/notification-type';
 import { RouterStub } from '../../../shared/testing/router-stub';
@@ -24,8 +24,8 @@ import { FieldChangeType } from '../../../core/data/object-updates/object-update
 import { RelationshipService } from '../../../core/data/relationship.service';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { getTestScheduler } from 'jasmine-marbles';
-import { By } from '@angular/platform-browser';
 import { RestResponse } from '../../../core/cache/response.models';
+import { RequestService } from '../../../core/data/request.service';
 
 let comp: any;
 let fixture: ComponentFixture<ItemRelationshipsComponent>;
@@ -33,6 +33,7 @@ let de: DebugElement;
 let el: HTMLElement;
 let objectUpdatesService;
 let relationshipService;
+let requestService;
 let objectCache;
 const infoNotification: INotification = new Notification('id', NotificationType.Info, 'info');
 const warningNotification: INotification = new Notification('id', NotificationType.Warning, 'warning');
@@ -158,6 +159,13 @@ describe('ItemRelationshipsComponent', () => {
       }
     );
 
+    requestService = jasmine.createSpyObj('requestService',
+      {
+        removeByHrefSubstring: {},
+        hasByHrefObservable: observableOf(false)
+      }
+    );
+
     objectCache = jasmine.createSpyObj('objectCache', {
       remove: undefined
     });
@@ -174,7 +182,9 @@ describe('ItemRelationshipsComponent', () => {
         { provide: NotificationsService, useValue: notificationsService },
         { provide: GLOBAL_CONFIG, useValue: { item: { edit: { undoTimeout: 10 } } } as any },
         { provide: RelationshipService, useValue: relationshipService },
-        { provide: ObjectCacheService, useValue: objectCache }
+        { provide: ObjectCacheService, useValue: objectCache },
+        { provide: RequestService, useValue: requestService },
+        ChangeDetectorRef
       ], schemas: [
         NO_ERRORS_SCHEMA
       ]
@@ -210,17 +220,6 @@ describe('ItemRelationshipsComponent', () => {
     });
   });
 
-  describe('changeType is REMOVE', () => {
-    beforeEach(() => {
-      fieldUpdate1.changeType = FieldChangeType.REMOVE;
-      fixture.detectChanges();
-    });
-    it('the div should have class alert-danger', () => {
-      const element = de.queryAll(By.css('.relationship-row'))[1].nativeElement;
-      expect(element.classList).toContain('alert-danger');
-    });
-  });
-
   describe('submit', () => {
     beforeEach(() => {
       comp.submit();
@@ -229,6 +228,7 @@ describe('ItemRelationshipsComponent', () => {
     it('it should delete the correct relationship and de-cache the current item', () => {
       expect(relationshipService.deleteRelationship).toHaveBeenCalledWith(relationships[1].uuid);
       expect(objectCache.remove).toHaveBeenCalledWith(item.self);
+      expect(requestService.removeByHrefSubstring).toHaveBeenCalledWith(item.self);
     });
   });
 });

src/app/+item-page/edit-item-page/item-relationships/item-relationships.component.ts

@@ -1,8 +1,8 @@
-import { Component, Inject } from '@angular/core';
+import { ChangeDetectorRef, Component, Inject, OnDestroy } from '@angular/core';
 import { Item } from '../../../core/shared/item.model';
 import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer';
 import { Observable } from 'rxjs/internal/Observable';
-import { map, switchMap, take, tap } from 'rxjs/operators';
+import { filter, map, switchMap, take } from 'rxjs/operators';
 import { combineLatest as observableCombineLatest, zip as observableZip } from 'rxjs';
 import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
 import { ItemDataService } from '../../../core/data/item-data.service';
@@ -20,6 +20,7 @@ import { RemoteData } from '../../../core/data/remote-data';
 import { ObjectCacheService } from '../../../core/cache/object-cache.service';
 import { getSucceededRemoteData } from '../../../core/shared/operators';
 import { RequestService } from '../../../core/data/request.service';
+import { Subscription } from 'rxjs/internal/Subscription';
 
 @Component({
   selector: 'ds-item-relationships',
@@ -29,13 +30,19 @@ import { RequestService } from '../../../core/data/request.service';
 /**
  * Component for displaying an item's relationships edit page
  */
-export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
+export class ItemRelationshipsComponent extends AbstractItemUpdateComponent implements OnDestroy {
 
   /**
    * The labels of all different relations within this item
    */
   relationLabels$: Observable<string[]>;
 
+  /**
+   * 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 relationships are deleted
+   */
+  itemUpdateSubscription: Subscription;
+
   constructor(
     protected itemService: ItemDataService,
     protected objectUpdatesService: ObjectUpdatesService,
@@ -46,7 +53,8 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
     protected route: ActivatedRoute,
     protected relationshipService: RelationshipService,
     protected objectCache: ObjectCacheService,
-    protected requestService: RequestService
+    protected requestService: RequestService,
+    protected cdRef: ChangeDetectorRef
   ) {
     super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route);
   }
@@ -57,6 +65,16 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
   ngOnInit(): void {
     super.ngOnInit();
     this.relationLabels$ = this.relationshipService.getItemRelationshipLabels(this.item);
+
+    // Update the item (and view) when it's removed in the request cache
+    this.itemUpdateSubscription = this.requestService.hasByHrefObservable(this.item.self).pipe(
+      filter((exists: boolean) => !exists),
+      switchMap(() => this.itemService.findById(this.item.uuid)),
+      getSucceededRemoteData(),
+    ).subscribe((itemRD: RemoteData<Item>) => {
+      this.item = itemRD.payload;
+      this.cdRef.detectChanges();
+    });
   }
 
   /**
@@ -104,13 +122,12 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
       switchMap((removedIds: string[]) => observableZip(...removedIds.map((uuid: string) => this.relationshipService.deleteRelationship(uuid)))),
       map((responses: RestResponse[]) => responses.filter((response: RestResponse) => response.isSuccessful))
     ).subscribe((responses: RestResponse[]) => {
-      // Make sure the lists are up-to-date and send a notification that the removal was successful
+      // Remove the item's cache to make sure the lists are reloaded with the newest values
-      // TODO: Fix lists refreshing correctly
       this.objectCache.remove(this.item.self);
       this.requestService.removeByHrefSubstring(this.item.self);
-      // this.itemService.findById(this.item.id).pipe(getSucceededRemoteData(), take(1)).subscribe((itemRD: RemoteData<Item>) => this.item = itemRD.payload);
       this.initializeOriginalFields();
       this.initializeUpdates();
+      // Send a notification that the removal was successful
       this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved'));
     });
   }
@@ -125,33 +142,10 @@ export class ItemRelationshipsComponent extends AbstractItemUpdateComponent {
   }
 
   /**
-   * Transform the item's relationships of a specific type into related items
+   * Unsubscribe from the item update when the component is destroyed
-   * @param label   The relationship type's label
    */
-  public getRelatedItemsByLabel(label: string): Observable<Item[]> {
+  ngOnDestroy(): void {
-    return this.relationshipService.getRelatedItemsByLabel(this.item, label);
+    this.itemUpdateSubscription.unsubscribe();
-  }
-
-  /**
-   * Get FieldUpdates for the relationships of a specific type
-   * @param label   The relationship type's label
-   */
-  public getUpdatesByLabel(label: string): Observable<FieldUpdates> {
-    return this.getRelatedItemsByLabel(label).pipe(
-      switchMap((items: Item[]) => this.objectUpdatesService.getFieldUpdatesExclusive(this.url, items))
-    )
-  }
-
-  /**
-   * Get the i18n message key for a relationship
-   * @param label   The relationship type's label
-   */
-  public getRelationshipMessageKey(label: string): string {
-    if (label.indexOf('Of') > -1) {
-      return `relationships.${label.substring(0, label.indexOf('Of') + 2)}`
-    } else {
-      return label;
-    }
   }
 
 }

src/app/core/cache/object-cache.service.ts

@@ -3,7 +3,7 @@ import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
 import { applyPatch, Operation } from 'fast-json-patch';
 import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 
-import { distinctUntilChanged, filter, map, mergeMap, take, } from 'rxjs/operators';
+import { distinctUntilChanged, filter, map, mergeMap, take, tap, } from 'rxjs/operators';
 import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util';
 import { CoreState } from '../core.reducers';
 import { coreSelector } from '../core.selectors';
@@ -224,6 +224,18 @@ export class ObjectCacheService {
     return result;
   }
 
+  /**
+   * Create an observable that emits a new value whenever the availability of the cached object changes.
+   * The value it emits is a boolean stating if the object exists in cache or not.
+   * @param selfLink  The self link of the object to observe
+   */
+  hasBySelfLinkObservable(selfLink: string): Observable<boolean> {
+    return this.store.pipe(
+      select(entryFromSelfLinkSelector(selfLink)),
+      map((entry: ObjectCacheEntry) => this.isValid(entry))
+    );
+  }
+
   /**
    * Check whether an ObjectCacheEntry should still be cached
    *

src/app/core/data/request.service.ts

@@ -265,4 +265,15 @@ export class RequestService {
     return result;
   }
 
+  /**
+   * Create an observable that emits a new value whenever the availability of the cached request changes.
+   * The value it emits is a boolean stating if the request exists in cache or not.
+   * @param href  The href of the request to observe
+   */
+  hasByHrefObservable(href: string): Observable<boolean> {
+    return this.getByHref(href).pipe(
+      map((requestEntry: RequestEntry) => this.isValid(requestEntry))
+    );
+  }
+
 }