UoE/Clear statistics table pagination params on navigation

UoE/Clear statistics table pagination params on navigation

Author: milanmajchrak

src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts

@@ -83,7 +83,7 @@ describe('CollectionStatisticsPageComponent', () => {
         { provide: DSpaceObjectDataService, useValue: {} },
         { provide: DSONameService, useValue: nameService },
         { provide: AuthService, useValue: authService },
-        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
+        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
       ],
     })
       .compileComponents();

src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts

@@ -83,7 +83,7 @@ describe('CommunityStatisticsPageComponent', () => {
         { provide: DSpaceObjectDataService, useValue: {} },
         { provide: DSONameService, useValue: nameService },
         { provide: AuthService, useValue: authService },
-        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
+        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
       ],
     })
       .compileComponents();

src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts

@@ -83,7 +83,7 @@ describe('ItemStatisticsPageComponent', () => {
         { provide: DSpaceObjectDataService, useValue: {} },
         { provide: DSONameService, useValue: nameService },
         { provide: AuthService, useValue: authService },
-        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
+        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
       ],
     })
       .compileComponents();

src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts

@@ -84,7 +84,7 @@ describe('SiteStatisticsPageComponent', () => {
         { provide: DSONameService, useValue: nameService },
         { provide: SiteDataService, useValue: siteService },
         { provide: AuthService, useValue: authService },
-        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
+        { provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
       ],
     })
       .compileComponents();

src/app/statistics-page/statistics-table/statistics-table.component.spec.ts

@@ -47,6 +47,7 @@ describe('StatisticsTableComponent', () => {
 
   const paginationService = {
     getCurrentPagination: (_id: string, _options: PaginationComponentOptions) => currentPagination$.asObservable(),
+    clearPagination: (_id: string) => undefined,
   };
 
   const setPage = (currentPage: number, pageSize = 10) => {
@@ -221,4 +222,27 @@ describe('StatisticsTableComponent', () => {
       expect(de.query(By.css(`td.item_${numberOfPoints - 1}-views-data`))).toBeTruthy();
     });
   });
+
+  describe('on destroy', () => {
+
+    it('should clear its own pagination params so they do not leak to the next scope', () => {
+      component.report = Object.assign(new UsageReport(), {
+        id: 'uuid_TotalVisits',
+        points: [],
+      });
+      component.ngOnInit();
+      const spy = spyOn(paginationService, 'clearPagination');
+
+      fixture.destroy();
+
+      expect(spy).toHaveBeenCalledWith('stats-uuid_TotalVisits');
+    });
+
+    it('should not throw when destroyed before initialisation', () => {
+      // createComponent alone runs only the constructor; destroying without detectChanges makes
+      // Angular itself invoke ngOnDestroy on a component whose ngOnInit never ran.
+      const uninitialised = TestBed.createComponent(StatisticsTableComponent);
+      expect(() => uninitialised.destroy()).not.toThrow();
+    });
+  });
 });

src/app/statistics-page/statistics-table/statistics-table.component.ts

@@ -6,6 +6,7 @@ import {
 import {
   Component,
   Input,
+  OnDestroy,
   OnInit,
 } from '@angular/core';
 import {
@@ -43,7 +44,7 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c
   standalone: true,
   imports: [NgIf, NgFor, AsyncPipe, TranslateModule, PaginationComponent],
 })
-export class StatisticsTableComponent implements OnInit {
+export class StatisticsTableComponent implements OnInit, OnDestroy {
 
   /**
    * The usage report to display a statistics table for
@@ -110,6 +111,18 @@ export class StatisticsTableComponent implements OnInit {
     );
   }
 
+  ngOnDestroy() {
+    // Stage this table's stats-* params for removal. Plain navigations drop query params anyway, but
+    // 'merge' navigations (e.g. the navbar search form) carry them along; PaginationService applies the
+    // staged nulls on its next updateRoute, scrubbing them from the URL. Deliberately NOT navigating from
+    // here: several tables are destroyed at once and an eager update would race the in-flight navigation.
+    // Same idiom as the other paginated components. Guarded in case the component is destroyed before
+    // ngOnInit ran.
+    if (this.paginationOptions) {
+      this.paginationService.clearPagination(this.paginationOptions.id);
+    }
+  }
+
   /**
    * Get the row label to display for a statistics point.
    * @param point the statistics point to get the label for