Skip to content

added pagination to Versions tab on Dataset and File page#975

Open
ekraffmiller wants to merge 3 commits intodevelopfrom
add-version-pagination
Open

added pagination to Versions tab on Dataset and File page#975
ekraffmiller wants to merge 3 commits intodevelopfrom
add-version-pagination

Conversation

@ekraffmiller
Copy link
Copy Markdown
Contributor

@ekraffmiller ekraffmiller commented Apr 28, 2026

What this PR does / why we need it:

To improve performance, we need to add pagination to the versions tab, and load only one page of results at a time.

Which issue(s) this PR closes:

Special notes for your reviewer:

Suggestions on how to test this:

The PR was deployed on qa.dataverse.org. To see an example of a dataset with many versions, try https://qa.dataverse.org/modern/datasets?persistentId=doi:10.7910/DVN/2USIUM

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

add pagination controls to Dataset Page Version tab and File Page Version tab

Is there a release notes or changelog update needed for this change?:

yes

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds pagination to the Dataset and File “Versions” tabs by introducing version-specific pagination info, wiring pagination controls into both tables, and updating data-fetching hooks/tests to support paged retrieval and total counts.

Changes:

  • Add PaginationControls to Dataset/File Versions tables and drive fetching via pagination state.
  • Update versions summaries hooks to return totalCount for pagination.
  • Add component + hook tests covering pagination calls and total count behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/component/sections/file/file-version/useGetFileVersionsSummaries.spec.tsx Adds hook test to verify pagination arg forwarding and returning totalCount.
tests/component/sections/file/file-version/FileVersions.spec.tsx Adds component test asserting repository calls change when paging.
tests/component/sections/dataset/dataset-versions/useGetDatasetVersionsSummaries.spec.tsx Adds hook test to verify pagination arg forwarding and returning totalCount.
tests/component/sections/dataset/dataset-versions/DatasetVersions.spec.tsx Adds component test asserting repository calls change when paging.
src/sections/shared/pagination/PaginationControls.tsx Extends supported pagination types to include dataset/file version pagination.
src/sections/file/file-version/useGetFileVersionsSummaries.ts Changes fetchSummaries to return totalCount for pagination.
src/sections/file/file-version/FileVersions.tsx Adds pagination state + controls and triggers paged fetching when in view/page changes.
src/sections/file/file-version/FileVersion.module.scss Adds loading-state styling for the versions table body.
src/sections/dataset/dataset-versions/useGetDatasetVersionsSummaries.ts Changes fetchSummaries to return totalCount for pagination.
src/sections/dataset/dataset-versions/DatasetVersions.tsx Adds pagination state + controls and triggers paged fetching (incl. +1 fetch for “previous version” logic).
src/sections/dataset/dataset-versions/DatasetVersions.module.scss Adds loading-state styling for the versions table body.
src/sections/dataset/Dataset.tsx Updates DatasetVersions props usage for the deaccessioned/single-tab branch.
src/dataset/domain/models/DatasetVersionPaginationInfo.ts Adds offsetOverride to support display/fetch page-size mismatch.
Comments suppressed due to low confidence (2)

src/sections/file/file-version/FileVersions.tsx:66

  • error is checked after the early return for !fileVersionSummaries, so an initial fetch failure will render the loading skeleton indefinitely instead of the error alert. Swap the order (render error first) or make the skeleton conditional on isLoading rather than only on fileVersionSummaries being undefined.
  if (!fileVersionSummaries) {
    return <FileVersionsLoadingSkeleton />
  }

  if (error) {

src/sections/dataset/dataset-versions/DatasetVersions.tsx:106

  • error is checked after returning the loading skeleton when !datasetVersionSummaries, so an initial fetch error will keep showing the skeleton instead of an error message. Handle error before the skeleton return, or gate the skeleton on isLoadingDatasetVersionSummaries.
  if (!datasetVersionSummaries) {
    return <DatasetVersionsLoadingSkeleton />
  }

  if (error) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


void fetchSummaries(paginationInfoToFetch).then((totalCount) => {
if (typeof totalCount === 'number') {
setPaginationInfo(paginationInfoToFetch.withTotal(totalCount))
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setPaginationInfo(paginationInfoToFetch.withTotal(totalCount)) inside the async fetchSummaries callback can race with page/pageSize changes (e.g., user clicks Next quickly). A slower response from an earlier request can overwrite the newer pagination state and jump the UI back to an older page. Consider updating only totalItems via a functional state update (based on the latest state) or tracking a request id/cancellation before applying the result.

Suggested change
setPaginationInfo(paginationInfoToFetch.withTotal(totalCount))
setPaginationInfo((currentPaginationInfo) => currentPaginationInfo.withTotal(totalCount))

Copilot uses AI. Check for mistakes.

void fetchSummaries(paginationInfoToFetch).then((totalCount) => {
if (typeof totalCount === 'number') {
setPaginationInfo(paginationInfoToDisplay.withTotal(totalCount))
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to FileVersions: updating pagination state from an async fetchSummaries response (setPaginationInfo(paginationInfoToDisplay.withTotal(totalCount))) can race with page/pageSize changes and revert the UI to an older page if responses arrive out of order. Prefer a functional update that only updates totalItems on the latest pagination state, or guard the update with a request id.

Suggested change
setPaginationInfo(paginationInfoToDisplay.withTotal(totalCount))
setPaginationInfo((currentPaginationInfo) =>
currentPaginationInfo.withTotal(totalCount)
)

Copilot uses AI. Check for mistakes.
Comment on lines +20 to +32
@@ -25,9 +28,8 @@ export function PaginationControls({
initialPaginationInfo,
showPageSizeSelector = true
}: PaginationProps) {
const [paginationInfo, setPaginationInfo] = useState<
DatasetPaginationInfo | FilePaginationInfo | NotificationsPaginationInfo
>(initialPaginationInfo)
const [paginationInfo, setPaginationInfo] =
useState<SupportedPaginationInfo>(initialPaginationInfo)
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PaginationControls stores SupportedPaginationInfo in state, but the props are typed as PaginationInfo<SupportedPaginationInfo>. This loses the concrete pagination subtype and forces callers (e.g., DatasetVersions/FileVersions) to reconstruct new pagination objects in onPaginationInfoChange instead of passing setPaginationInfo directly. Consider making PaginationControls generic (<T extends PaginationInfo<T>>) and typing initialPaginationInfo: T / onPaginationInfoChange: (p: T) => void (or at least align both props to SupportedPaginationInfo) so types stay consistent and subtype-specific behavior is preserved.

Copilot uses AI. Check for mistakes.
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 28, 2026

Coverage Status

coverage: 98.063% (+0.5%) from 97.565% — add-version-pagination into develop

@ekraffmiller ekraffmiller linked an issue Apr 29, 2026 that may be closed by this pull request
… PaginationControls to be generic, Moved error checks before the initial skeleton fallback
@ekraffmiller ekraffmiller marked this pull request as ready for review April 29, 2026 18:40
@ekraffmiller ekraffmiller moved this to Ready for Review ⏩ in IQSS Dataverse Project Apr 29, 2026
@ekraffmiller ekraffmiller added the GREI Re-arch GREI re-architecture-related label Apr 29, 2026
@ekraffmiller ekraffmiller requested a review from ChengShi-1 April 29, 2026 19:03
@ekraffmiller ekraffmiller added FY26 Sprint 22 FY26 Sprint 22 (2026-04-22 - 2026-05-06) Size: 3 A percentage of a sprint. 2.1 hours. labels Apr 29, 2026
@ChengShi-1 ChengShi-1 self-assigned this Apr 29, 2026
@ChengShi-1 ChengShi-1 moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Apr 29, 2026
Copy link
Copy Markdown
Contributor

@ChengShi-1 ChengShi-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks goodImage
Image
but one of the test is failing, could you solve it?

@github-project-automation github-project-automation Bot moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Apr 29, 2026
@ChengShi-1 ChengShi-1 removed their assignment Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FY26 Sprint 22 FY26 Sprint 22 (2026-04-22 - 2026-05-06) GREI Re-arch GREI re-architecture-related Project: HDV SPA Rollout Size: 3 A percentage of a sprint. 2.1 hours.

Projects

Status: Ready for QA ⏩

Development

Successfully merging this pull request may close these issues.

Add pagination to version tabs Performance with many versions of a dataset

4 participants