[OP-19967] Fix wiki_page_links pagination silently ignoring offset - #24774
Open
jtauschl wants to merge 2 commits into
Open
[OP-19967] Fix wiki_page_links pagination silently ignoring offset#24774jtauschl wants to merge 2 commits into
jtauschl wants to merge 2 commits into
Conversation
Both the work-package-scoped and the global GET handlers only passed per_page: into PageLinkCollectionRepresenter.new, never page:. API::Decorators::OffsetPaginatedCollection defaults page to 1 whenever it's not explicitly given, so every request to either endpoint always rendered (and reported) page 1 of the underlying relation, silently ignoring the client's offset query parameter -- total was reported correctly, but offset itself always echoed back 1 and results never advanced past the first page. Pass params[:offset] through as page: at both call sites.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
recheck |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes API V3 pagination for wiki page links by ensuring the offset query parameter is actually applied when rendering WikiPageLinkCollection responses. It aligns the page-links endpoints with the expected behavior of API::Decorators::OffsetPaginatedCollection, which only paginates correctly when the current page is explicitly provided.
Changes:
- Forward
params[:offset]aspage:intoPageLinkCollectionRepresenterfor the work-package-scoped endpoint. - Forward
params[:offset]aspage:intoPageLinkCollectionRepresenterfor the global endpoint. - Add request-spec regressions asserting
offset=2&pageSize=2returns a distinct second page and reportsoffsetcorrectly for both endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| modules/wikis/spec/requests/api/v3/page_links/page_links_api_spec.rb | Adds regression request specs ensuring pagination offset is applied and reported correctly on both endpoints. |
| modules/wikis/lib/api/v3/page_links/work_package_wiki_page_links_api.rb | Fixes work-package-scoped endpoint by passing params[:offset] as page: to the collection representer. |
| modules/wikis/lib/api/v3/page_links/page_links_api.rb | Fixes global endpoint by passing params[:offset] as page: to the collection representer. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
RSpecRails/HaveHttpStatus flagged the raw status-code comparison in the two new pagination regression tests; have_http_status is this codebase's established idiom, already used everywhere else in this same file.
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.
Ticket
https://community.openproject.org/wp/OP-19967
Summary
Both the work-package-scoped and the global GET handlers only passed
per_page:intoPageLinkCollectionRepresenter.new, neverpage:.API::Decorators::OffsetPaginatedCollectiondefaultspageto 1 whenever it's not explicitly given, so every request to either endpoint always rendered (and reported) page 1 of the underlying relation, silently ignoring the client'soffsetquery parameter --totalwas reported correctly, butoffsetitself always echoed back 1 and results never advanced past the first page.Change
Pass
params[:offset]through aspage:at both call sites.Test plan
offsetis correctly reportedoffset=2returned the same elements asoffset=1on both endpoints; after the fix, each returns distinct, correct results independently