Improve back navigation logic in ItemComponent (#80)

* Improve back navigation logic in ItemComponent

Enhances the navigation logic to use window.history.back() when the previous URL does not match the expected route pattern. Also updates the back button visibility filter to allow empty URLs.

* lint fix

* copilot fixes

copilot fixes

* Fix back button visibility logic in ItemComponent

Updated the showBackButton observable to correctly determine when the back button should be shown based on the previous route. Also added ngOnInit call in the related test to ensure proper initialization.

* Remove unused 'filter' import from item.component.ts

The 'filter' operator from 'rxjs/operators' was imported but not used in item.component.ts. This commit cleans up the import statements.

* Add session storage for previous URL in item page

Introduces generic methods in RouteService to store and retrieve URLs in session storage. ItemComponent now uses session storage to persist and retrieve the previous URL for improved back navigation, falling back to browser history if no valid URL is found.

* Add session URL helpers to RouteService stubs in tests

Extended RouteService stubs in test files to include storeUrlInSession and getUrlFromSession methods, matching the interface used by ItemComponent and related tests. This ensures test mocks are up-to-date with the service's API.

* Simplify back navigation logic in ItemComponent

Removed conditional check for previous URL and always navigate using router.navigateByUrl with storedPreviousUrl.

Author: amadulhaxxani

src/app/core/services/route.service.ts

@@ -160,6 +160,34 @@ export class RouteService {
       });
   }
 
+  /**
+   * Store a URL in session storage for later retrieval
+   * Generic method that can be used by any component
+   * @param key The session storage key
+   * @param url The URL to store
+   */
+  public storeUrlInSession(key: string, url: string): void {
+    if (typeof window !== 'undefined' && hasValue(window.sessionStorage)) {
+      // Only write if the value is different to avoid unnecessary writes
+      const currentValue = window.sessionStorage.getItem(key);
+      if (currentValue !== url) {
+        window.sessionStorage.setItem(key, url);
+      }
+    }
+  }
+
+  /**
+   * Retrieve a URL from session storage
+   * Generic method that can be used by any component
+   * @param key The session storage key
+   */
+  public getUrlFromSession(key: string): string | null {
+    if (typeof window !== 'undefined' && hasValue(window.sessionStorage)) {
+      return window.sessionStorage.getItem(key);
+    }
+    return null;
+  }
+
   private getRouteParams(): Observable<Params> {
     let active = this.route;
     while (active.firstChild) {

src/app/item-page/simple/item-types/publication/publication.component.spec.ts

@@ -153,6 +153,12 @@ describe('PublicationComponent', () => {
     const localMockRouteService = {
       getPreviousUrl(): Observable<string> {
         return of('/search?query=test%20query&fakeParam=true');
+      },
+      storeUrlInSession(key: string, url: string): void {
+        // no-op
+      },
+      getUrlFromSession(key: string): string | null {
+        return null;
       }
     };
     beforeEach(waitForAsync(() => {
@@ -183,6 +189,12 @@ describe('PublicationComponent', () => {
     const localMockRouteService = {
       getPreviousUrl(): Observable<string> {
         return of('/item');
+      },
+      storeUrlInSession(key: string, url: string): void {
+        // no-op
+      },
+      getUrlFromSession(key: string): string | null {
+        return null;
       }
     };
     beforeEach(waitForAsync(() => {

src/app/item-page/simple/item-types/shared/item.component.spec.ts

@@ -74,6 +74,12 @@ export function getIIIFEnabled(enabled: boolean): MetadataValue {
 export const mockRouteService = {
   getPreviousUrl(): Observable<string> {
     return observableOf('');
+  },
+  storeUrlInSession(key: string, url: string): void {
+    // no-op
+  },
+  getUrlFromSession(key: string): string | null {
+    return null;
   }
 };
 
@@ -482,6 +488,7 @@ describe('ItemComponent', () => {
 
     it('should hide back button',() => {
       spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(observableOf('/item'));
+      comp.ngOnInit();
       comp.showBackButton.subscribe((val) => {
         expect(val).toBeFalse();
       });

src/app/item-page/simple/item-types/shared/item.component.ts

@@ -5,7 +5,7 @@ import { getItemPageRoute } from '../../../item-page-routing-paths';
 import { RouteService } from '../../../../core/services/route.service';
 import { Observable } from 'rxjs';
 import { getDSpaceQuery, isIiifEnabled, isIiifSearchEnabled } from './item-iiif-utils';
-import { filter, map, take } from 'rxjs/operators';
+import { map, take } from 'rxjs/operators';
 import { Router } from '@angular/router';
 import { select, Store } from '@ngrx/store';
 import { AppState } from 'src/app/app.reducer';
@@ -22,6 +22,11 @@ import { APP_CONFIG, AppConfig } from 'src/config/app-config.interface';
 export class ItemComponent implements OnInit {
   @Input() object: Item;
 
+  /**
+   * Session storage key for storing the previous URL before entering item page
+   */
+  private readonly ITEM_PREVIOUS_URL_SESSION_KEY = 'item-previous-url';
+
   /**
    * This regex matches previous routes. The button is shown
    * for matching paths and hidden in other cases.
@@ -57,6 +62,11 @@ export class ItemComponent implements OnInit {
 
   isAuthenticated$: Observable<boolean>;
 
+  /**
+   * Stores the previous URL retrieved either from RouteService or sessionStorage
+   */
+  private storedPreviousUrl: string;
+
   constructor(protected routeService: RouteService,
               protected router: Router,
               private store: Store<AppState>,
@@ -66,26 +76,36 @@ export class ItemComponent implements OnInit {
 
   /**
    * The function used to return to list from the item.
+   * Uses stored previous URL if available, otherwise falls back to browser history.
    */
   back = () => {
-    this.routeService.getPreviousUrl().pipe(
-          take(1)
-        ).subscribe(
-          (url => {
-            this.router.navigateByUrl(url);
-          })
-        );
+    this.router.navigateByUrl(this.storedPreviousUrl);
   };
 
   ngOnInit(): void {
-
     this.itemPageRoute = getItemPageRoute(this.object);
     // hide/show the back button
     this.showBackButton = this.routeService.getPreviousUrl().pipe(
-      filter(url => this.previousRoute.test(url)),
       take(1),
-      map(() => true)
+      map(url => {
+        const fromRoute = this.pickAllowedPrevious(url);
+
+        if (fromRoute) {
+          this.routeService.storeUrlInSession(this.ITEM_PREVIOUS_URL_SESSION_KEY, fromRoute);
+          this.storedPreviousUrl = fromRoute;
+          return true;
+        }
+
+        const storedUrl = this.routeService.getUrlFromSession(this.ITEM_PREVIOUS_URL_SESSION_KEY);
+        if (this.pickAllowedPrevious(storedUrl)) {
+          this.storedPreviousUrl = storedUrl;
+          return true;
+        }
+
+        return false;
+      })
     );
+
     // check to see if iiif viewer is required.
     this.iiifEnabled = isIiifEnabled(this.object);
     this.iiifSearchEnabled = isIiifSearchEnabled(this.object);
@@ -95,6 +115,13 @@ export class ItemComponent implements OnInit {
     this.isAuthenticated$ = this.store.pipe(select(isAuthenticated));
   }
 
+  /**
+   * Helper to check if a URL is from an allowed previous route and return it, otherwise null
+   */
+  private pickAllowedPrevious(url?: string | null): string | null {
+    return url && this.previousRoute.test(url) ? url : null;
+  }
+
   get hasConfiguredStatistics(): boolean {
     return !!this.appConfig.statistics?.baseUrl && !!this.appConfig.statistics?.endpoint;
   }

src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts

@@ -159,6 +159,12 @@ describe('UntypedItemComponent', () => {
     const localMockRouteService = {
       getPreviousUrl(): Observable<string> {
         return of('/search?query=test%20query&fakeParam=true');
+      },
+      storeUrlInSession(key: string, url: string): void {
+        // no-op
+      },
+      getUrlFromSession(key: string): string | null {
+        return null;
       }
     };
     beforeEach(waitForAsync(() => {
@@ -190,6 +196,12 @@ describe('UntypedItemComponent', () => {
     const localMockRouteService = {
       getPreviousUrl(): Observable<string> {
         return of('/item');
+      },
+      storeUrlInSession(key: string, url: string): void {
+        // no-op
+      },
+      getUrlFromSession(key: string): string | null {
+        return null;
       }
     };
     beforeEach(waitForAsync(() => {

src/app/shared/testing/route-service.stub.ts

@@ -37,6 +37,16 @@ export const routeServiceStub: any = {
   },
   getPreviousUrl: () => {
     return observableOf('/home');
+  },
+  // Added generic session helpers used by ItemComponent
+  storeUrlInSession: (key: string, url: string) => {
+    // no-op for tests
+  },
+  getUrlFromSession: (key: string): string | null => {
+    return null;
+  },
+  clearUrlFromSession: (key: string) => {
+    // no-op for tests
   }
   /* eslint-enable no-empty, @typescript-eslint/no-empty-function */
 };