[Port to dtq-dev] Improve back navigation logic in ItemComponent (#1184)

* Improve back navigation logic in ItemComponent (ufal#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.

* 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.

(cherry picked from commit 5dfb106)

* Treat /home as previous route and add tests

Include "/home" in ItemComponent's previousRoute regex so the back button is shown for home. Add unit tests to verify the back button appears for a home previous URL and that a home previous URL is prioritized over a session-stored URL (ensuring the session is updated and navigation uses the home URL).

(cherry picked from commit 3eb32e5)

---------

Co-authored-by: Amad Ul Hassan <hassan@ufal.mff.cuni.cz>

Author: kosarko

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

@@ -156,6 +156,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(() => {
@@ -186,6 +192,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

@@ -76,6 +76,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;
   }
 };
 
@@ -425,6 +431,7 @@ describe('ItemComponent', () => {
 
     const searchUrl = '/search?query=test&spc.page=2';
     const browseUrl = '/browse/title?scope=0cc&bbm.page=3';
+    const homeUrl = '/home';
     const recentSubmissionsUrl = '/collections/be7b8430-77a5-4016-91c9-90863e50583a?cp.page=3';
 
     beforeEach(waitForAsync(() => {
@@ -485,6 +492,7 @@ describe('ItemComponent', () => {
 
     it('should hide back button',() => {
       spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(observableOf('/item'));
+      comp.ngOnInit();
       comp.showBackButton.subscribe((val) => {
         expect(val).toBeFalse();
       });
@@ -510,6 +518,32 @@ describe('ItemComponent', () => {
         expect(val).toBeTrue();
       });
     });
+
+    it('should show back button for home', () => {
+      spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(observableOf(homeUrl));
+      comp.ngOnInit();
+      comp.showBackButton.subscribe((val) => {
+        expect(val).toBeTrue();
+      });
+    });
+
+    it('should prioritize home previous url over session fallback', () => {
+      const staleSessionUrl = searchUrl;
+      const getPreviousUrlSpy = spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(observableOf(homeUrl));
+      const getUrlFromSessionSpy = spyOn(mockRouteService, 'getUrlFromSession').and.returnValue(staleSessionUrl);
+      const storeUrlInSessionSpy = spyOn(mockRouteService, 'storeUrlInSession');
+
+      comp.ngOnInit();
+      comp.showBackButton.subscribe((val) => {
+        expect(val).toBeTrue();
+        expect(getPreviousUrlSpy).toHaveBeenCalled();
+        expect(getUrlFromSessionSpy).not.toHaveBeenCalled();
+        expect(storeUrlInSessionSpy).toHaveBeenCalledWith('item-previous-url', homeUrl);
+
+        comp.back();
+        expect(router.navigateByUrl).toHaveBeenCalledWith(homeUrl);
+      });
+    });
   });
 
 });

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,11 +22,16 @@ 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.
    */
-  previousRoute = /^(\/search|\/browse|\/collections|\/admin\/search|\/mydspace)/;
+  previousRoute = /^(\/home|\/search|\/browse|\/collections|\/admin\/search|\/mydspace)/;
 
   /**
    * Used to show or hide the back to results button in the view.
@@ -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

@@ -162,6 +162,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(() => {
@@ -193,6 +199,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 */
 };