-
Notifications
You must be signed in to change notification settings - Fork 10
Add pagination query parameters to Dataset&File Version Summaries use case #395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pagination query parameters to Dataset&File Version Summaries use case #395
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds pagination support to the Dataset and File Version Summaries use cases by introducing optional limit and offset query parameters. This allows clients to retrieve version summaries in paginated chunks rather than loading all versions at once.
Key Changes:
- Added optional
limitandoffsetparameters toGetDatasetVersionsSummariesandGetFileVersionSummariesuse cases and their corresponding repository methods - Updated documentation to reflect the new pagination parameters
- Added comprehensive integration tests to verify pagination functionality
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/datasets/domain/useCases/GetDatasetVersionsSummaries.ts | Added optional limit and offset parameters to the execute method |
| src/datasets/domain/repositories/IDatasetsRepository.ts | Updated interface signature to include pagination parameters |
| src/datasets/infra/repositories/DatasetsRepository.ts | Implemented query parameter building and passing to API endpoint |
| src/files/domain/useCases/GetFileVersionSummaries.ts | Added optional limit and offset parameters to the execute method |
| src/files/domain/repositories/IFilesRepository.ts | Updated interface signature to include pagination parameters |
| src/files/infra/repositories/FilesRepository.ts | Implemented query parameter building and passing to API endpoint |
| test/unit/datasets/GetDatasetVersionsSummaries.test.ts | Updated test to verify undefined pagination parameters are passed correctly |
| test/integration/datasets/DatasetsRepository.test.ts | Added comprehensive pagination test with 22 versions across multiple pages |
| test/integration/files/FilesRepository.test.ts | Added comprehensive pagination test and updated existing test expectations |
| docs/useCases.md | Updated documentation to describe the new pagination parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tasetfile-version-summaries-use-case
g-saracca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one comment about how to handle query params and one question, are we returning the total count from these use cases, similar how we do for the GetCollectionItems?
You will need the total count to know when to keep asking or when you reach the final page.
| const queryParams: { limit?: string; offset?: string } = { | ||
| limit: limit?.toString(), | ||
| offset: offset?.toString() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can follow same approach as here for limit and offset, using URLSearchParams
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the api returns totalCount but I didn't add it to use case. I modified the code to apply totalCount, please review again. Thank you! @g-saracca
|
@ChengShi-1 approved, note that this is a breaking change, you will need a frontend change to avoid things breaking 👍🏼 |
|
awesome - merging! |
What this PR does / why we need it:
Add pagination query parameters to Dataset&File Version Summaries use case
Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
Suggestions on how to test this:
Is there a release notes or changelog update needed for this change?:
yes
Additional documentation: