From 27ec82814231f8c564110b628de938b9a35d2bd7 Mon Sep 17 00:00:00 2001
From: Kristof De Langhe <kristof.delanghe@atmire.com>
Date: Fri, 26 Jul 2019 13:28:32 +0200
Subject: [PATCH] 63945: Bitstream delete notifications, de-caching, reloading
 and small layout changes

---
 resources/i18n/en.json                        |  17 +++
 .../item-bitstreams.component.html            |  66 +++++-----
 .../item-bitstreams.component.ts              | 119 ++++++++++++++++--
 src/app/core/core.module.ts                   |   2 +
 src/app/core/data/bitstream-data.service.ts   |   8 ++
 src/app/core/data/request.service.ts          |  11 ++
 6 files changed, 182 insertions(+), 41 deletions(-)

diff --git a/resources/i18n/en.json b/resources/i18n/en.json
index cc81394386..b503696178 100644
--- a/resources/i18n/en.json
+++ b/resources/i18n/en.json
@@ -286,6 +286,23 @@
             "remove": "Remove",
             "undo": "Undo changes"
           }
+        },
+        "notifications": {
+          "outdated": {
+            "title": "Changed outdated",
+            "content": "The item you're currently working on has been changed by another user. Your current changes are discarded to prevent conflicts"
+          },
+          "discarded": {
+            "title": "Changed discarded",
+            "content": "Your changes were discarded. To reinstate your changes click the 'Undo' button"
+          },
+          "saved": {
+            "title": "Bitstreams saved",
+            "content": "Your changes to this item's bitstreams were saved."
+          },
+          "failed": {
+            "title": "Error deleting bitstream"
+          }
         }
       }
     }
diff --git a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html
index 77f22e12ea..d74f8fe352 100644
--- a/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html
+++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-bitstreams.component.html
@@ -1,24 +1,26 @@
 <div class="item-bitstreams">
-  <div class="button-row top d-flex">
-    <button class="btn btn-danger ml-auto" *ngIf="!(isReinstatable() | async)"
-            [disabled]="!(hasChanges() | async)"
-            (click)="discard()"><i
-      class="fas fa-times"></i>
-      <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.discard-button" | translate}}</span>
-    </button>
-    <button class="btn btn-warning ml-auto" *ngIf="isReinstatable() | async"
-            (click)="reinstate()"><i
-      class="fas fa-undo-alt"></i>
-      <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.reinstate-button" | translate}}</span>
-    </button>
-    <button class="btn btn-primary" [disabled]="!(hasChanges() | async)"
-            (click)="submit()"><i
-      class="fas fa-save"></i>
-      <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.save-button" | translate}}</span>
-    </button>
+  <div class="float-right">
+    <div class="button-row top mb-4 mt-2">
+      <button class="btn btn-danger" *ngIf="!(isReinstatable() | async)"
+              [disabled]="!(hasChanges() | async)"
+              (click)="discard()"><i
+        class="fas fa-times"></i>
+        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.discard-button" | translate}}</span>
+      </button>
+      <button class="btn btn-warning" *ngIf="isReinstatable() | async"
+              (click)="reinstate()"><i
+        class="fas fa-undo-alt"></i>
+        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.reinstate-button" | translate}}</span>
+      </button>
+      <button class="btn btn-primary" [disabled]="!(hasChanges() | async)"
+              (click)="submit()"><i
+        class="fas fa-save"></i>
+        <span class="d-none d-sm-inline">&nbsp;{{"item.edit.bitstreams.save-button" | translate}}</span>
+      </button>
+    </div>
   </div>
 
-  <table class="table table-responsive table-striped table-bordered">
+  <table *ngIf="updatesMap && updatesMap.size > 0" class="table table-responsive table-striped table-bordered">
     <tbody>
       <tr>
         <th>{{'item.edit.bitstreams.headers.name' | translate}}</th>
@@ -29,27 +31,29 @@
       <ng-container *ngFor="let updatesItem of updatesMap | keyvalue">
         <tr>
           <th>{{'item.edit.bitstreams.headers.bundle' | translate}}: {{ updatesItem.key }}</th>
-          <td></td>
+          <td class="w-100"></td>
           <td></td>
           <td></td>
         </tr>
-        <ng-container *ngVar="((updatesItem.value | async) | dsObjectValues) as updateValues">
-          <tr *ngFor="let updateValue of updateValues"
-              ds-item-edit-bitstream
-              [fieldUpdate]="updateValue"
-              [url]="url"
-              [ngClass]="{
-                'table-warning': updateValue.changeType === 0,
-                'table-danger': updateValue.changeType === 2,
-                'table-success': updateValue.changeType === 1
-              }">
-          </tr>
+        <ng-container *ngIf="updatesItem.value">
+          <ng-container *ngVar="((updatesItem.value | async) | dsObjectValues) as updateValues">
+            <tr *ngFor="let updateValue of updateValues"
+                ds-item-edit-bitstream
+                [fieldUpdate]="updateValue"
+                [url]="url"
+                [ngClass]="{
+                  'table-warning': updateValue.changeType === 0,
+                  'table-danger': updateValue.changeType === 2,
+                  'table-success': updateValue.changeType === 1
+                }">
+            </tr>
+          </ng-container>
         </ng-container>
       </ng-container>
     </tbody>
   </table>
   <div class="button-row bottom">
-    <div class="float-right">
+    <div class="my-2 float-right">
       <button class="btn btn-danger" *ngIf="!(isReinstatable() | async)"
               [disabled]="!(hasChanges() | async)"
               (click)="discard()"><i
diff --git 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
index 2bd6ff04a2..5b3ff8f830 100644
--- 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
@@ -1,8 +1,8 @@
-import { Component, Inject, OnDestroy } from '@angular/core';
+import { ChangeDetectorRef, Component, Inject, OnDestroy } from '@angular/core';
 import { AbstractItemUpdateComponent } from '../abstract-item-update/abstract-item-update.component';
-import { map, switchMap, take } from 'rxjs/operators';
+import { filter, map, switchMap, take } from 'rxjs/operators';
 import { Bitstream } from '../../../core/shared/bitstream.model';
-import { getBundleNames, toBitstreamsArray, toBundleMap } from '../../../core/shared/item-bitstreams-utils';
+import { toBitstreamsArray, toBundleMap } from '../../../core/shared/item-bitstreams-utils';
 import { Observable } from 'rxjs/internal/Observable';
 import { FieldUpdate, FieldUpdates } from '../../../core/data/object-updates/object-updates.reducer';
 import { Subscription } from 'rxjs/internal/Subscription';
@@ -16,7 +16,14 @@ import { BitstreamDataService } from '../../../core/data/bitstream-data.service'
 import { FieldChangeType } from '../../../core/data/object-updates/object-updates.actions';
 import { isNotEmptyOperator } from '../../../shared/empty.util';
 import { zip as observableZip } from 'rxjs';
-import { RestResponse } from '../../../core/cache/response.models';
+import { ErrorResponse, RestResponse } from '../../../core/cache/response.models';
+import { ObjectCacheService } from '../../../core/cache/object-cache.service';
+import { RequestService } from '../../../core/data/request.service';
+import { getSucceededRemoteData } from '../../../core/shared/operators';
+import { Item } from '../../../core/shared/item.model';
+import { RemoteData } from '../../../core/data/remote-data';
+
+export const ORIGINAL_BUNDLE = 'ORIGINAL';
 
 @Component({
   selector: 'ds-item-bitstreams',
@@ -28,12 +35,24 @@ import { RestResponse } from '../../../core/cache/response.models';
  */
 export class ItemBitstreamsComponent extends AbstractItemUpdateComponent implements OnDestroy {
 
-  bundleNames$: Observable<string[]>;
-
+  /**
+   * A Map of the current item's bundles and respective FieldUpdates
+   * key:   Bundle Name
+   * value: FieldUpdates about the bundle's bitstreams
+   */
   updatesMap: Map<string, Observable<FieldUpdates>>;
 
+  /**
+   * A subscription keeping the updatesMap up-to-date
+   */
   updatesMapSub: Subscription;
 
+  /**
+   * 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,
@@ -42,7 +61,10 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     public translateService: TranslateService,
     @Inject(GLOBAL_CONFIG) public EnvConfig: GlobalConfig,
     public route: ActivatedRoute,
-    public bitstreamService: BitstreamDataService
+    public bitstreamService: BitstreamDataService,
+    public objectCache: ObjectCacheService,
+    public requestService: RequestService,
+    public cdRef: ChangeDetectorRef
   ) {
     super(itemService, objectUpdatesService, router, notificationsService, translateService, EnvConfig, route);
   }
@@ -52,13 +74,19 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
    */
   ngOnInit(): void {
     super.ngOnInit();
-    this.bundleNames$ = this.item.bitstreams.pipe(getBundleNames());
+    this.initializeItemUpdate();
   }
 
+  /**
+   * Initialize the notification messages prefix
+   */
   initializeNotificationsPrefix(): void {
     this.notificationsPrefix = 'item.edit.bitstreams.notifications.';
   }
 
+  /**
+   * Initialize the original fields for the object-updates-service
+   */
   initializeOriginalFields(): void {
     this.item.bitstreams.pipe(
       toBitstreamsArray(),
@@ -68,6 +96,9 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     });
   }
 
+  /**
+   * Initialize field updates
+   */
   initializeUpdates(): void {
     this.updates$ = this.item.bitstreams.pipe(
       toBitstreamsArray(),
@@ -80,10 +111,36 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
       bundleMap.forEach((bitstreams: Bitstream[], bundleName: string) => {
         updatesMap.set(bundleName, this.objectUpdatesService.getFieldUpdatesExclusive(this.url, bitstreams));
       });
+      if (updatesMap.size === 0) {
+        // Add an ORIGINAL bundle by default if the item doesn't contain any bitstreams
+        updatesMap.set(ORIGINAL_BUNDLE, undefined);
+      }
       this.updatesMap = updatesMap;
     });
   }
 
+  /**
+   * 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.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.initializeOriginalFields();
+      this.initializeUpdates();
+      this.cdRef.detectChanges();
+    });
+  }
+
+  /**
+   * Submit the current changes
+   * Bitstreams marked as deleted send out a delete request to the rest API
+   * Display notifications and reset the current item/updates
+   */
   submit() {
     const removedBitstreams$ = this.item.bitstreams.pipe(
       toBitstreamsArray(),
@@ -94,11 +151,53 @@ export class ItemBitstreamsComponent extends AbstractItemUpdateComponent impleme
     );
     removedBitstreams$.pipe(
       take(1),
-      switchMap((removedBistreams: Bitstream[]) => observableZip(...removedBistreams.map((bitstream: Bitstream) => this.bitstreamService.delete(bitstream))))
-    ).subscribe();
+      switchMap((removedBistreams: Bitstream[]) => observableZip(...removedBistreams.map((bitstream: Bitstream) => this.bitstreamService.deleteAndReturnResponse(bitstream))))
+    ).subscribe((responses: RestResponse[]) => {
+      this.displayNotifications(responses);
+      this.reset();
+    });
   }
 
+  /**
+   * Display notifications
+   * - Error notification for each failed response with their message
+   * - Success notification in case there's at least one successful response
+   * @param responses
+   */
+  displayNotifications(responses: RestResponse[]) {
+    const failedResponses = responses.filter((response: RestResponse) => !response.isSuccessful);
+    const successfulResponses = responses.filter((response: RestResponse) => response.isSuccessful);
+
+    failedResponses.forEach((response: ErrorResponse) => {
+      this.notificationsService.error(this.getNotificationTitle('failed'), response.errorMessage);
+    });
+    if (successfulResponses.length > 0) {
+      this.notificationsService.success(this.getNotificationTitle('saved'), this.getNotificationContent('saved'));
+    }
+  }
+
+  /**
+   * De-cache the current item (it should automatically reload due to itemUpdateSubscription)
+   */
+  reset() {
+    this.updatesMap = undefined;
+    this.refreshItemCache();
+    this.initializeItemUpdate();
+  }
+
+  /**
+   * Remove the current item's cache from object- and request-cache
+   */
+  refreshItemCache() {
+    this.objectCache.remove(this.item.self);
+    this.requestService.removeByHrefSubstring(this.item.self);
+  }
+
+  /**
+   * Unsubscribe from open subscriptions whenever the component gets destroyed
+   */
   ngOnDestroy(): void {
     this.updatesMapSub.unsubscribe();
+    this.itemUpdateSubscription.unsubscribe();
   }
 }
diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts
index 6550435aa3..10a6e3048e 100644
--- a/src/app/core/core.module.ts
+++ b/src/app/core/core.module.ts
@@ -87,6 +87,7 @@ import { MyDSpaceResponseParsingService } from './data/mydspace-response-parsing
 import { ClaimedTaskDataService } from './tasks/claimed-task-data.service';
 import { PoolTaskDataService } from './tasks/pool-task-data.service';
 import { TaskResponseParsingService } from './tasks/task-response-parsing.service';
+import { BitstreamDataService } from './data/bitstream-data.service';
 
 const IMPORTS = [
   CommonModule,
@@ -175,6 +176,7 @@ const PROVIDERS = [
   TaskResponseParsingService,
   ClaimedTaskDataService,
   PoolTaskDataService,
+  BitstreamDataService,
   // register AuthInterceptor as HttpInterceptor
   {
     provide: HTTP_INTERCEPTORS,
diff --git a/src/app/core/data/bitstream-data.service.ts b/src/app/core/data/bitstream-data.service.ts
index 76839c7ea3..4ba5abbe4b 100644
--- a/src/app/core/data/bitstream-data.service.ts
+++ b/src/app/core/data/bitstream-data.service.ts
@@ -14,6 +14,7 @@ import { HttpClient } from '@angular/common/http';
 import { DSOChangeAnalyzer } from './dso-change-analyzer.service';
 import { FindAllOptions } from './request.models';
 import { Observable } from 'rxjs/internal/Observable';
+import { RestResponse } from '../cache/response.models';
 
 @Injectable()
 export class BitstreamDataService extends DataService<Bitstream> {
@@ -37,4 +38,11 @@ export class BitstreamDataService extends DataService<Bitstream> {
   getBrowseEndpoint(options: FindAllOptions = {}, linkPath: string = this.linkPath): Observable<string> {
     return this.halService.getEndpoint(linkPath);
   }
+
+  deleteAndReturnResponse(bitstream: Bitstream): Observable<RestResponse> {
+    const response$ = super.deleteAndReturnResponse(bitstream);
+    this.objectCache.remove(bitstream.self);
+    this.requestService.removeByHrefSubstring(bitstream.self);
+    return response$;
+  }
 }
diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts
index 83071382ed..36d554b1ec 100644
--- a/src/app/core/data/request.service.ts
+++ b/src/app/core/data/request.service.ts
@@ -315,4 +315,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))
+    );
+  }
+
 }