UoE/Clear statistics table pagination params on navigation - #25
Merged
milanmajchrak merged 3 commits intoJul 20, 2026
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds scoped “Top item views” reporting to community and collection statistics pages, aligning pagination behavior across all DSO scopes and preventing pagination query params from leaking between scopes.
Changes:
- Adds the
TopItemsreport type to community and collection statistics pages and provides the corresponding i18n table title. - Clears per-table pagination query params on
StatisticsTableComponentteardown to prevent cross-scope pagination leakage. - Extends/updates unit specs to cover the new report/table presence, label behavior, and pagination cleanup.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/assets/i18n/en.json5 | Adds the statistics.table.title.TopItems translation string used for the new scoped table title. |
| src/app/statistics-page/statistics-table/statistics-table.component.ts | Clears the table’s pagination params on destroy to avoid leaking page/rpp between scopes. |
| src/app/statistics-page/statistics-table/statistics-table.component.spec.ts | Adds coverage for pagination cleanup on destroy and TopItems label handling. |
| src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts | Updates PaginationService mock to include clearPagination for teardown behavior. |
| src/app/statistics-page/item-statistics-page/item-statistics-page.component.spec.ts | Updates PaginationService mock to include clearPagination for teardown behavior. |
| src/app/statistics-page/community-statistics-page/community-statistics-page.component.ts | Includes TopItems in the community statistics report types list. |
| src/app/statistics-page/community-statistics-page/community-statistics-page.component.spec.ts | Verifies the community page renders the new TopItems report table. |
| src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.ts | Includes TopItems in the collection statistics report types list. |
| src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts | Verifies the collection page renders the new TopItems report table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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>
jr-rk
force-pushed
the
uoe/statistics-topitems-community-collection
branch
from
July 16, 2026 13:19
f4fe2f7 to
cfc036f
Compare
jr-rk
changed the title
The stale stats-* params only survive 'merge' navigations (e.g. the navbar search form); plain navigations drop them. Clarify that clearPagination stages nulls applied on the service's next updateRoute, and why no navigation is issued from ngOnDestroy (racing the in-flight navigation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Let Angular invoke ngOnDestroy through fixture.destroy() on a component whose ngOnInit never ran (createComponent without detectChanges), instead of calling the hook directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
milanmajchrak
merged commit Jul 20, 2026
e7dae74
into
datashare-UoEMainLibrary-dspace-8_x
11 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Frontend half of dataquest-dev/dspace-customers#807. The shared
StatisticsTableComponentalready paginates every statistics table at all levels (site/community/collection/item) via URL-drivends-pagination(DSpace#726 / #11); the only missing piece was cleanup — pagination params (stats-<uuid>_<reportType>.page/.rpp) leaked into the URL across scope navigation because pagination usesqueryParamsHandling: 'merge'.Changes
ngOnDestroy→PaginationService.clearPagination(id)— same idiom as the other paginated components; deferred (not eager) so simultaneous multi-table destroys don't race the in-flight navigation.Connected PRs
After fix:
🤖 Generated with Claude Code