From cf1acd9598dfde5b3e312d7f58dc3799163b154a Mon Sep 17 00:00:00 2001 From: Amad Ul Hassan Date: Tue, 20 Jan 2026 12:34:08 +0100 Subject: [PATCH 1/2] Improve back navigation logic in ItemComponent (ufal/dspace-angular#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. (cherry picked from commit 5dfb106f6f5418b88c5a37d13736810dc8f252e8) --- src/app/core/services/route.service.ts | 28 +++++++++++ .../publication/publication.component.spec.ts | 12 +++++ .../item-types/shared/item.component.spec.ts | 7 +++ .../item-types/shared/item.component.ts | 49 ++++++++++++++----- .../untyped-item.component.spec.ts | 12 +++++ src/app/shared/testing/route-service.stub.ts | 10 ++++ 6 files changed, 107 insertions(+), 11 deletions(-) diff --git a/src/app/core/services/route.service.ts b/src/app/core/services/route.service.ts index 53817625709..ac9c78fa5a6 100644 --- a/src/app/core/services/route.service.ts +++ b/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 { let active = this.route; while (active.firstChild) { diff --git a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts index e84e01ead0c..0cea5b2a1ac 100644 --- a/src/app/item-page/simple/item-types/publication/publication.component.spec.ts +++ b/src/app/item-page/simple/item-types/publication/publication.component.spec.ts @@ -156,6 +156,12 @@ describe('PublicationComponent', () => { const localMockRouteService = { getPreviousUrl(): Observable { 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 { return of('/item'); + }, + storeUrlInSession(key: string, url: string): void { + // no-op + }, + getUrlFromSession(key: string): string | null { + return null; } }; beforeEach(waitForAsync(() => { diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 148db4705c6..44a37b281ae 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/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 { return observableOf(''); + }, + storeUrlInSession(key: string, url: string): void { + // no-op + }, + getUrlFromSession(key: string): string | null { + return null; } }; @@ -485,6 +491,7 @@ describe('ItemComponent', () => { it('should hide back button',() => { spyOn(mockRouteService, 'getPreviousUrl').and.returnValue(observableOf('/item')); + comp.ngOnInit(); comp.showBackButton.subscribe((val) => { expect(val).toBeFalse(); }); diff --git a/src/app/item-page/simple/item-types/shared/item.component.ts b/src/app/item-page/simple/item-types/shared/item.component.ts index 46fa4fdfa7d..630bd4c4a4b 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.ts +++ b/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; + /** + * Stores the previous URL retrieved either from RouteService or sessionStorage + */ + private storedPreviousUrl: string; + constructor(protected routeService: RouteService, protected router: Router, private store: Store, @@ -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; } diff --git a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts index 6ad81900920..762bdb9a557 100644 --- a/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts +++ b/src/app/item-page/simple/item-types/untyped-item/untyped-item.component.spec.ts @@ -162,6 +162,12 @@ describe('UntypedItemComponent', () => { const localMockRouteService = { getPreviousUrl(): Observable { 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 { return of('/item'); + }, + storeUrlInSession(key: string, url: string): void { + // no-op + }, + getUrlFromSession(key: string): string | null { + return null; } }; beforeEach(waitForAsync(() => { diff --git a/src/app/shared/testing/route-service.stub.ts b/src/app/shared/testing/route-service.stub.ts index 8384c3efbcc..b38378b2abc 100644 --- a/src/app/shared/testing/route-service.stub.ts +++ b/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 */ }; From 54dd8917406b5bb682ba9da9117373ae6f666d12 Mon Sep 17 00:00:00 2001 From: amadulhaxxani Date: Thu, 19 Feb 2026 13:07:39 +0100 Subject: [PATCH 2/2] 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 3eb32e56a8d4248254857f0d306cbfb5653dc8d9) --- .../item-types/shared/item.component.spec.ts | 27 +++++++++++++++++++ .../item-types/shared/item.component.ts | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/app/item-page/simple/item-types/shared/item.component.spec.ts b/src/app/item-page/simple/item-types/shared/item.component.spec.ts index 44a37b281ae..b9d7867775a 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.spec.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.spec.ts @@ -431,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(() => { @@ -517,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); + }); + }); }); }); diff --git a/src/app/item-page/simple/item-types/shared/item.component.ts b/src/app/item-page/simple/item-types/shared/item.component.ts index 630bd4c4a4b..5300de886a9 100644 --- a/src/app/item-page/simple/item-types/shared/item.component.ts +++ b/src/app/item-page/simple/item-types/shared/item.component.ts @@ -31,7 +31,7 @@ export class ItemComponent implements OnInit { * 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.