Skip to content

Commit cfc036f

Browse files
jr-rkclaude
andcommitted
fix(statistics): clear statistics table pagination params on destroy
Statistics pagination is URL-driven with queryParamsHandling: 'merge', so a page/rpp selection made on one scope's statistics page persisted in the URL when navigating to another scope. Clear the table's own params on destroy, like other paginated components do. Verified on the seeded local instance that ds-pagination already renders and pages correctly at community, collection and item level via the shared StatisticsTableComponent (DSpace#726); no further frontend change is needed for lower-level pagination. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 83be561 commit cfc036f

6 files changed

Lines changed: 36 additions & 5 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('CollectionStatisticsPageComponent', () => {
8383
{ provide: DSpaceObjectDataService, useValue: {} },
8484
{ provide: DSONameService, useValue: nameService },
8585
{ provide: AuthService, useValue: authService },
86-
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
86+
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
8787
],
8888
})
8989
.compileComponents();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('CommunityStatisticsPageComponent', () => {
8383
{ provide: DSpaceObjectDataService, useValue: {} },
8484
{ provide: DSONameService, useValue: nameService },
8585
{ provide: AuthService, useValue: authService },
86-
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
86+
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
8787
],
8888
})
8989
.compileComponents();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('ItemStatisticsPageComponent', () => {
8383
{ provide: DSpaceObjectDataService, useValue: {} },
8484
{ provide: DSONameService, useValue: nameService },
8585
{ provide: AuthService, useValue: authService },
86-
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
86+
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
8787
],
8888
})
8989
.compileComponents();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe('SiteStatisticsPageComponent', () => {
8484
{ provide: DSONameService, useValue: nameService },
8585
{ provide: SiteDataService, useValue: siteService },
8686
{ provide: AuthService, useValue: authService },
87-
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }) } },
87+
{ provide: PaginationService, useValue: { getCurrentPagination: () => observableOf({ currentPage: 1, pageSize: 10 }), clearPagination: () => undefined } },
8888
],
8989
})
9090
.compileComponents();

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('StatisticsTableComponent', () => {
4747

4848
const paginationService = {
4949
getCurrentPagination: (_id: string, _options: PaginationComponentOptions) => currentPagination$.asObservable(),
50+
clearPagination: (_id: string) => undefined,
5051
};
5152

5253
const setPage = (currentPage: number, pageSize = 10) => {
@@ -221,4 +222,25 @@ describe('StatisticsTableComponent', () => {
221222
expect(de.query(By.css(`td.item_${numberOfPoints - 1}-views-data`))).toBeTruthy();
222223
});
223224
});
225+
226+
describe('on destroy', () => {
227+
228+
it('should clear its own pagination params so they do not leak to the next scope', () => {
229+
component.report = Object.assign(new UsageReport(), {
230+
id: 'uuid_TotalVisits',
231+
points: [],
232+
});
233+
component.ngOnInit();
234+
const spy = spyOn(paginationService, 'clearPagination');
235+
236+
fixture.destroy();
237+
238+
expect(spy).toHaveBeenCalledWith('stats-uuid_TotalVisits');
239+
});
240+
241+
it('should not throw when destroyed before initialisation', () => {
242+
const uninitialised = TestBed.createComponent(StatisticsTableComponent).componentInstance;
243+
expect(() => uninitialised.ngOnDestroy()).not.toThrow();
244+
});
245+
});
224246
});

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import {
77
Component,
88
Input,
9+
OnDestroy,
910
OnInit,
1011
} from '@angular/core';
1112
import {
@@ -43,7 +44,7 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c
4344
standalone: true,
4445
imports: [NgIf, NgFor, AsyncPipe, TranslateModule, PaginationComponent],
4546
})
46-
export class StatisticsTableComponent implements OnInit {
47+
export class StatisticsTableComponent implements OnInit, OnDestroy {
4748

4849
/**
4950
* The usage report to display a statistics table for
@@ -110,6 +111,14 @@ export class StatisticsTableComponent implements OnInit {
110111
);
111112
}
112113

114+
ngOnDestroy() {
115+
// Drop this table's page/rpp query params so they don't leak (via queryParamsHandling: 'merge') to the next
116+
// scope's statistics page. Guarded in case the component is destroyed before ngOnInit ran.
117+
if (this.paginationOptions) {
118+
this.paginationService.clearPagination(this.paginationOptions.id);
119+
}
120+
}
121+
113122
/**
114123
* Get the row label to display for a statistics point.
115124
* @param point the statistics point to get the label for

0 commit comments

Comments
 (0)