Merge pull request DSpace#4250 from atmire/fix-embargoed-thumbnails_contribute-7.6

[Port dspace-7_x] Show restricted thumbnails to users with access rights

Author: tdonohue

src/app/item-page/media-viewer/media-viewer.component.html

@@ -16,12 +16,7 @@
     </ng-container>
   </div>
   <ng-template #showThumbnail>
-    <ds-themed-media-viewer-image *ngIf="mediaOptions.image && mediaOptions.video"
-      [image]="(thumbnailsRD$ | async)?.payload?.page[0]?._links.content.href || thumbnailPlaceholder"
-      [preview]="false"
-    ></ds-themed-media-viewer-image>
-    <ds-thumbnail *ngIf="!(mediaOptions.image && mediaOptions.video)"
-                  [thumbnail]="(thumbnailsRD$ | async)?.payload?.page[0]">
-    </ds-thumbnail>
+    <ds-themed-thumbnail [thumbnail]="(thumbnailsRD$ | async)?.payload?.page[0]">
+    </ds-themed-thumbnail>
   </ng-template>
 </ng-container>

src/app/item-page/media-viewer/media-viewer.component.spec.ts

@@ -139,9 +139,9 @@ describe('MediaViewerComponent', () => {
       expect(mediaItem.thumbnail).toBe(null);
     });
 
-    it('should display a default, thumbnail', () => {
+    it('should display a default thumbnail', () => {
       const defaultThumbnail = fixture.debugElement.query(
-        By.css('ds-themed-media-viewer-image')
+        By.css('ds-themed-thumbnail')
       );
       expect(defaultThumbnail.nativeElement).toBeDefined();
     });

src/app/thumbnail/thumbnail.component.html

@@ -1,15 +1,15 @@
 <div class="thumbnail" [class.limit-width]="limitWidth">
-  <div *ngIf="isLoading" class="thumbnail-content outer">
+  <div *ngIf="(isLoading$ | async)" class="thumbnail-content outer">
     <div class="inner">
       <div class="centered">
         <ds-themed-loading [spinner]="true"></ds-themed-loading>
       </div>
     </div>
   </div>
-  <!-- don't use *ngIf="!isLoading" so the thumbnail can load in while the animation is playing -->
-  <img *ngIf="src !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': isLoading}"
-       [src]="src | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
-  <div *ngIf="src === null && !isLoading" class="thumbnail-content outer">
+  <!-- don't use *ngIf="!(isLoading$ | async)" so the thumbnail can load in while the animation is playing -->
+  <img *ngIf="(src$ | async) !== null" class="thumbnail-content img-fluid" [ngClass]="{'d-none': (isLoading$ | async)}"
+       [src]="(src$ | async) | dsSafeUrl" [alt]="alt | translate" (error)="errorHandler()" (load)="successHandler()">
+  <div *ngIf="(src$ | async) === null && !(isLoading$ | async)" class="thumbnail-content outer">
     <div class="inner">
       <div class="thumbnail-placeholder centered lead">
         {{ placeholder | translate }}

src/app/thumbnail/thumbnail.component.spec.ts

@@ -73,31 +73,31 @@ describe('ThumbnailComponent', () => {
 
     describe('loading', () => {
       it('should start out with isLoading$ true', () => {
-        expect(comp.isLoading).toBeTrue();
+      expect(comp.isLoading$.getValue()).toBeTrue();
       });
 
       it('should set isLoading$ to false once an image is successfully loaded', () => {
         comp.setSrc('http://bit.stream');
         fixture.debugElement.query(By.css('img.thumbnail-content')).triggerEventHandler('load', new Event('load'));
-        expect(comp.isLoading).toBeFalse();
+      expect(comp.isLoading$.getValue()).toBeFalse();
       });
 
       it('should set isLoading$ to false once the src is set to null', () => {
         comp.setSrc(null);
-        expect(comp.isLoading).toBeFalse();
+      expect(comp.isLoading$.getValue()).toBeFalse();
       });
 
       it('should show a loading animation while isLoading$ is true', () => {
       expect(de.query(By.css('ds-themed-loading'))).toBeTruthy();
 
-        comp.isLoading = false;
+      comp.isLoading$.next(false);
         fixture.detectChanges();
       expect(fixture.debugElement.query(By.css('ds-themed-loading'))).toBeFalsy();
       });
 
       describe('with a thumbnail image', () => {
         beforeEach(() => {
-          comp.src = 'https://bit.stream';
+        comp.src$.next('https://bit.stream');
           fixture.detectChanges();
         });
 
@@ -106,7 +106,7 @@ describe('ThumbnailComponent', () => {
           expect(img).toBeTruthy();
           expect(img.classes['d-none']).toBeTrue();
 
-          comp.isLoading = false;
+        comp.isLoading$.next(false);
           fixture.detectChanges();
           img = fixture.debugElement.query(By.css('img.thumbnail-content'));
           expect(img).toBeTruthy();
@@ -117,14 +117,14 @@ describe('ThumbnailComponent', () => {
 
       describe('without a thumbnail image', () => {
         beforeEach(() => {
-          comp.src = null;
+        comp.src$.next(null);
           fixture.detectChanges();
         });
 
         it('should only show the HTML placeholder once done loading', () => {
           expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeFalsy();
 
-          comp.isLoading = false;
+        comp.isLoading$.next(false);
           fixture.detectChanges();
           expect(fixture.debugElement.query(By.css('div.thumbnail-placeholder'))).toBeTruthy();
         });
@@ -220,14 +220,14 @@ describe('ThumbnailComponent', () => {
     describe('fallback', () => {
       describe('if there is a default image', () => {
         it('should display the default image', () => {
-          comp.src = 'http://bit.stream';
+          comp.src$.next('http://bit.stream');
           comp.defaultImage = 'http://default.img';
           comp.errorHandler();
-          expect(comp.src).toBe(comp.defaultImage);
+          expect(comp.src$.getValue()).toBe(comp.defaultImage);
         });
 
         it('should include the alt text', () => {
-          comp.src = 'http://bit.stream';
+          comp.src$.next('http://bit.stream');
           comp.defaultImage = 'http://default.img';
           comp.errorHandler();
 
@@ -239,10 +239,10 @@ describe('ThumbnailComponent', () => {
 
       describe('if there is no default image', () => {
         it('should display the HTML placeholder', () => {
-          comp.src = 'http://default.img';
+          comp.src$.next('http://default.img');
           comp.defaultImage = null;
           comp.errorHandler();
-          expect(comp.src).toBe(null);
+          expect(comp.src$.getValue()).toBe(null);
 
           fixture.detectChanges();
           const placeholder = fixture.debugElement.query(By.css('div.thumbnail-placeholder')).nativeElement;
@@ -334,7 +334,7 @@ describe('ThumbnailComponent', () => {
         it('should show the default image', () => {
           comp.defaultImage = 'default/image.jpg';
           comp.ngOnChanges({});
-          expect(comp.src).toBe('default/image.jpg');
+          expect(comp.src$.getValue()).toBe('default/image.jpg');
         });
       });
     });
@@ -382,7 +382,7 @@ describe('ThumbnailComponent', () => {
     });
 
     it('should start out with isLoading$ true', () => {
-      expect(comp.isLoading).toBeTrue();
+      expect(comp.isLoading$.getValue()).toBeTrue();
       expect(de.query(By.css('ds-themed-loading'))).toBeTruthy();
     });
 

src/app/thumbnail/thumbnail.component.ts

@@ -2,7 +2,7 @@ import { Component, Inject, Input, OnChanges, PLATFORM_ID, SimpleChanges } from
 import { Bitstream } from '../core/shared/bitstream.model';
 import { hasNoValue, hasValue } from '../shared/empty.util';
 import { RemoteData } from '../core/data/remote-data';
-import { of as observableOf } from 'rxjs';
+import { of as observableOf, BehaviorSubject } from 'rxjs';
 import { switchMap } from 'rxjs/operators';
 import { FeatureID } from '../core/data/feature-authorization/feature-id';
 import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service';
@@ -35,7 +35,7 @@ export class ThumbnailComponent implements OnChanges {
   /**
    * The src attribute used in the template to render the image.
    */
-  src: string = undefined;
+  src$: BehaviorSubject<string> = new BehaviorSubject<string>(undefined);
 
   retriedWithToken = false;
 
@@ -58,7 +58,7 @@ export class ThumbnailComponent implements OnChanges {
    * Whether the thumbnail is currently loading
    * Start out as true to avoid flashing the alt text while a thumbnail is being loaded.
    */
-  isLoading = true;
+  isLoading$: BehaviorSubject<boolean> = new BehaviorSubject(true);
 
   constructor(
     @Inject(PLATFORM_ID) private platformID: any,
@@ -114,7 +114,7 @@ export class ThumbnailComponent implements OnChanges {
    * Otherwise, fall back to the default image or a HTML placeholder
    */
   errorHandler() {
-    const src = this.src;
+    const src = this.src$.getValue();
     const thumbnail = this.bitstream;
     const thumbnailSrc = thumbnail?._links?.content?.href;
 
@@ -166,16 +166,29 @@ export class ThumbnailComponent implements OnChanges {
    * @param src
    */
   setSrc(src: string): void {
-    this.src = src;
-    if (src === null) {
-      this.isLoading = false;
+    // only update the src if it has changed (the parent component may fire the same one multiple times
+    if (this.src$.getValue() !== src) {
+      // every time the src changes we need to start the loading animation again, as it's possible
+      // that it is first set to null when the parent component initializes and then set to
+      // the actual value
+      //
+      // isLoading$ will be set to false by the error or success handler afterwards, except in the
+      // case where src is null, then we have to set it manually here (because those handlers won't
+      // trigger)
+      if (src !== null && this.isLoading$.getValue() === false) {
+        this.isLoading$.next(true);
+      }
+      this.src$.next(src);
+      if (src === null && this.isLoading$.getValue() === true) {
+        this.isLoading$.next(false);
+      }
     }
   }
 
   /**
    * Stop the loading animation once the thumbnail is successfully loaded
    */
   successHandler() {
-    this.isLoading = false;
+    this.isLoading$.next(false);
   }
 }