LIBDRUM-1005. Added additional test to cover "no filename" branch

The RestrictedAccessComponent has a code branch (and a specific
message) when a filename is not provided, so added a test to verify
that part of the code.

Co-authored-by: Perplexity AI <noreply@perplexity.ai>
https://umd-dit.atlassian.net/browse/LIBDRUM-1005

Author: dsteelma-umd

src/app/restricted-access/restricted-access.component.spec.ts

@@ -116,7 +116,7 @@ describe('RestrictedAccessComponent', () => {
 
   // Helper function for setting up anonymous tests with a specific embargo
   // restriction
-  function setupAnonymous(bitstreamOverrides) {
+  function setupAnonymous(bitstreamOverrides: Partial<Bitstream>) {
     beforeEach(waitForAsync(() => {
       init(bitstreamOverrides);
       (authService.isAuthenticated as jasmine.Spy).and.returnValue(observableOf(false));
@@ -134,17 +134,11 @@ describe('RestrictedAccessComponent', () => {
   // Helper function verifying that a HTTP 401 Unauthorized status code is
   // set, and that a redirect to the bitstream is not performed.
   function verify401StatusCodeAndNoRedirectToDownload() {
-    it('should set 401 Unauthorized and not redirect to the file', waitForAsync(() => {
-      fixture.whenStable().then(() => {
-        expect(serverResponseService.setUnauthorized).toHaveBeenCalled();
-      });
-      fixture.whenStable().then(() => {
-        expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
-      });
-      fixture.whenStable().then(() => {
-        expect(hardRedirectService.redirect).not.toHaveBeenCalled();
-      });
-    }));
+    it('should set 401 Unauthorized and not redirect to the file', () => {
+      expect(serverResponseService.setUnauthorized).toHaveBeenCalled();
+      expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
+      expect(hardRedirectService.redirect).not.toHaveBeenCalled();
+    });
   }
 
   describe('when the user is anonymous (not logged in)', () => {
@@ -154,9 +148,7 @@ describe('RestrictedAccessComponent', () => {
       verify401StatusCodeAndNoRedirectToDownload();
 
       it('should set the restrictedAccessMessage indicating the file is embargoed forever', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.embargo.forever.message');
-        });
+        expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.embargo.forever.message');
       }));
     });
 
@@ -165,39 +157,33 @@ describe('RestrictedAccessComponent', () => {
 
       verify401StatusCodeAndNoRedirectToDownload();
 
-      it('should set the restrictedAccessMessage indicating an end date', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessMessage.value).toBe(
-            'bitstream.restricted-access.embargo.restricted-until.message');
-        });
-      }));
+      it('should set the restrictedAccessMessage indicating an end date', () => {
+        expect(component.restrictedAccessMessage.value).toBe(
+          'bitstream.restricted-access.embargo.restricted-until.message');
+      });
     });
 
     describe('when embargoRestriction is NONE (embargo over, but file is restricted for another reason)', () => {
       setupAnonymous({ embargoRestriction: 'NONE' });
 
       verify401StatusCodeAndNoRedirectToDownload();
 
-      it('should set the restrictedAccessMessage to a simple "forbidden" message', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.anonymous.forbidden.message');
-        });
-      }));
+      it('should set the restrictedAccessMessage to a simple "forbidden" message', () => {
+        expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.anonymous.forbidden.message');
+      });
     });
 
     describe('when file is restricted for non-embargo reasons (such as Campus IP restriction)', () => {
       setupAnonymous({ embargoRestriction:null });
 
       verify401StatusCodeAndNoRedirectToDownload();
 
-      it('should set the restrictedAccessMessage to a simple "forbidden" message', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.anonymous.forbidden.message');
-        });
-      }));
+      it('should set the restrictedAccessMessage to a simple "forbidden" message', () => {
+        expect(component.restrictedAccessMessage.value).toBe('bitstream.restricted-access.anonymous.forbidden.message');
+      });
     });
 
-    describe('when the user is authorized (even if there is an embargo)', () => {
+    describe('but the user is authorized (even if there is an embargo)', () => {
       beforeEach(waitForAsync(() => {
         init();
         (authService.isAuthenticated as jasmine.Spy).and.returnValue(observableOf(false));
@@ -211,23 +197,17 @@ describe('RestrictedAccessComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should redirect to the content link', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(hardRedirectService.redirect).toHaveBeenCalled();
-        });
-      }));
+      it('should redirect to the content link', () => {
+        expect(hardRedirectService.redirect).toHaveBeenCalled();
+      });
 
-      it('should NOT call setUnauthorized', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT call setUnauthorized', () => {
+        expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
+      });
 
-      it('should NOT call setForbidden', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT call setForbidden', () => {
+        expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
+      });
     });
   });
 
@@ -246,34 +226,45 @@ describe('RestrictedAccessComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should call setForbidden on ServerResponseService', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setForbidden).toHaveBeenCalled();
-        });
-      }));
+      it('should call setForbidden on ServerResponseService', () => {
+        expect(serverResponseService.setForbidden).toHaveBeenCalled();
+      });
 
-      it('should NOT call setUnauthorized on ServerResponseService', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT call setUnauthorized on ServerResponseService', () => {
+        expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
+      });
 
-      it('should NOT redirect to a download', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(hardRedirectService.redirect).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT redirect to a download', () => {
+        expect(hardRedirectService.redirect).not.toHaveBeenCalled();
+      });
+
+      it('should set the restrictedAccessHeader', () => {
+        expect(component.restrictedAccessHeader.value).toBe('bitstream.restricted-access.user.forbidden.header');
+      });
+
+      it('should set the restrictedAccessMessage', () => {
+        expect(component.restrictedAccessMessage.value).toBe(
+          'bitstream.restricted-access.user.forbidden.with_file.message');
+      });
+    });
 
-      it('should set the restrictedAccessHeader', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessHeader.value).toBe('bitstream.restricted-access.user.forbidden.header');
-        });
+    describe('returns 403 Forbidden with a generic message when no filename is not provided', () => {
+      beforeEach(waitForAsync(() => {
+        init({ metadata: {} });
+        (authService.isAuthenticated as jasmine.Spy).and.returnValue(observableOf(true));
+        (authorizationService.isAuthorized as jasmine.Spy).and.returnValue(observableOf(false));
+        initTestBed();
       }));
-      it('should set the restrictedAccessMessage', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(component.restrictedAccessMessage.value).toBe(
-            'bitstream.restricted-access.user.forbidden.with_file.message');
-        });
+
+      beforeEach(() => {
+        fixture = TestBed.createComponent(RestrictedAccessComponent);
+        component = fixture.componentInstance;
+        fixture.detectChanges();
+      });
+
+      it('should set the generic forbidden message', waitForAsync(() => {
+        expect(component.restrictedAccessMessage.value).toBe(
+          'bitstream.restricted-access.user.forbidden.generic.message');
       }));
     });
 
@@ -291,23 +282,17 @@ describe('RestrictedAccessComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should NOT call setUnauthorized', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT call setUnauthorized', () => {
+        expect(serverResponseService.setUnauthorized).not.toHaveBeenCalled();
+      });
 
-      it('should NOT call setForbidden', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
-        });
-      }));
+      it('should NOT call setForbidden', () => {
+        expect(serverResponseService.setForbidden).not.toHaveBeenCalled();
+      });
 
-      it('should redirect to the file download link', waitForAsync(() => {
-        fixture.whenStable().then(() => {
-          expect(hardRedirectService.redirect).toHaveBeenCalledWith('content-url-with-headers');
-        });
-      }));
+      it('should redirect to the file download link', () => {
+        expect(hardRedirectService.redirect).toHaveBeenCalledWith('content-url-with-headers');
+      });
     });
   });
 

src/app/restricted-access/restricted-access.component.ts

@@ -5,8 +5,11 @@ import {
 } from '@angular/common';
 import {
   Component,
+  DestroyRef,
+  inject,
   OnInit,
 } from '@angular/core';
+import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
 import {
   ActivatedRoute,
   Router,
@@ -18,6 +21,7 @@ import {
 import {
   BehaviorSubject,
   combineLatest as observableCombineLatest,
+  EMPTY,
   filter,
   map,
   Observable,
@@ -68,6 +72,8 @@ export class RestrictedAccessComponent implements OnInit {
   bitstreamRD$: Observable<RemoteData<Bitstream>>;
   bitstream$: Observable<Bitstream>;
 
+  private destroyRef = inject(DestroyRef);
+
   constructor(
     private route: ActivatedRoute,
     protected router: Router,
@@ -112,15 +118,17 @@ export class RestrictedAccessComponent implements OnInit {
               return [isAuthorized, isLoggedIn, bitstream, fileLink];
             }));
         } else {
-          return [[isAuthorized, isLoggedIn, bitstream, '']];
+          return observableOf([isAuthorized, isLoggedIn, bitstream, ''] as [boolean, boolean, Bitstream, string]);
         }
       }),
-    ).subscribe(([isAuthorized, isLoggedIn, bitstream, fileLink]: [boolean, boolean, Bitstream, string]) => {
-      if (isAuthorized && isNotEmpty(fileLink)) {
-        // This shouldn't happen, as the download is authorized, and the file link is available, so just redirect to
-        // actual download page.
-        this.hardRedirectService.redirect(fileLink);
-      } else {
+      switchMap(([isAuthorized, isLoggedIn, bitstream, fileLink]: [boolean, boolean, Bitstream, string]) => {
+        if (isAuthorized && isNotEmpty(fileLink)) {
+          // This shouldn't happen, as the download is authorized, and the file link is available, so just redirect to
+          // actual download page.
+          this.hardRedirectService.redirect(fileLink);
+          return EMPTY;
+        }
+
         let header$: Observable<string>;
         let message$: Observable<string>;
 
@@ -130,7 +138,7 @@ export class RestrictedAccessComponent implements OnInit {
           this.responseService.setForbidden();
           header$ = this.translateService.get('bitstream.restricted-access.user.forbidden.header', {});
 
-          if (bitstream && bitstream.metadata['dc.title'] &&  bitstream.metadata['dc.title'][0] && bitstream.metadata['dc.title'][0].value) {
+          if (bitstream && bitstream.metadata['dc.title'] && bitstream.metadata['dc.title'][0] && bitstream.metadata['dc.title'][0].value) {
             const filename = bitstream.metadata['dc.title'][0].value;
             message$ = this.translateService.get(
               'bitstream.restricted-access.user.forbidden.with_file.message', { 'filename': filename });
@@ -145,11 +153,12 @@ export class RestrictedAccessComponent implements OnInit {
           [header$, message$] = this.configureAnonymous(bitstream);
         }
 
-        zip(header$, message$).subscribe(([header, message]) => {
-          this.restrictedAccessHeader.next(header);
-          this.restrictedAccessMessage.next(message);
-        });
-      }
+        return zip(header$, message$);
+      }),
+      takeUntilDestroyed(this.destroyRef),
+    ).subscribe(([header, message]: [string, string]) => {
+      this.restrictedAccessHeader.next(header);
+      this.restrictedAccessMessage.next(message);
     });
   }