Conversation
|
Webhooks could probably be unified but I figured it's better to keep this focused. They are also in an awkward spot as they lack paging in webui so I don't think I can just add pagination for functions that are used by web routes without looking at UI. |
This reverts commit bd9fb97. revert date changes
This reverts commit 7052dd6edebe7ceb09a16d351100fd01c04f26be.
|
test failure looks related: |
There was a problem hiding this comment.
Pull request overview
This PR standardizes API pagination behavior across multiple endpoints, adding Link and X-Total-Count headers where appropriate and introducing proper paging for the admin webhooks endpoint, while also refactoring the system/default webhook query model.
Changes:
- Centralizes pagination through
utils.GetListOptions, clamping pages to a minimum of 1 and wiringctx.SetLinkHeaderandSetTotalCountHeaderinto numerous user, org, repo, shared, and notification endpoints. - Refactors system/default webhook listing to a new
ListSystemWebhookOptions+GetGlobalWebhooksmodel and updates the admin/admin/hooksendpoint to be fully paginated and to emit paging headers. - Adds or adjusts pagination headers for specific resource listings (teams, org members, wiki pages, actions runs/jobs/tasks, secrets/variables, notifications, issue dependencies, etc.).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| routers/api/v1/utils/page.go | Ensures GetListOptions always uses at least page 1 and a normalized page size for all consumers. |
| routers/api/v1/utils/hook.go | Makes owner hook listing use ListOptions and emit Link/X-Total-Count headers. |
| routers/api/v1/user/watch.go | Uses shared ListOptions and adds Link/X-Total-Count for watched-repos endpoints. |
| routers/api/v1/user/star.go | Adds Link headers for starred-repo listings using the user’s star count and current limit. |
| routers/api/v1/user/key.go | Adds pagination support and headers for user public key listings; introduces a bug when filtering by fingerprint. |
| routers/api/v1/user/follower.go | Wires in ListOptions and Link headers for followers and following lists. |
| routers/api/v1/user/action.go | Paginates user-level actions variables and adds Link/X-Total-Count headers. |
| routers/api/v1/shared/block.go | Uses ListOptions and emits pagination headers for user block listings. |
| routers/api/v1/shared/action.go | Adds paging and Link/X-Total-Count headers to shared actions jobs and runs listings. |
| routers/api/v1/repo/wiki.go | Adds pagination headers for wiki page list; adds a FIXME noting missing SetLinkHeader for revisions. |
| routers/api/v1/repo/status.go | Paginates combined commit status-by-ref and adds corresponding pagination headers. |
| routers/api/v1/repo/issue_dependency.go | Switches dependency listing to ListOptions and adds pagination headers, but currently uses the page size as the reported total. |
| routers/api/v1/repo/action.go | Adds paging + Link headers for repo actions secrets, variables, and tasks. |
| routers/api/v1/org/team.go | Makes org/team and user/team listings use ListOptions and emit Link/X-Total-Count, addressing the linked pagination issue. |
| routers/api/v1/org/member.go | Paginates org member listing and adds Link/X-Total-Count headers. |
| routers/api/v1/org/action.go | Adds pagination headers for org-level actions secrets and variables. |
| routers/api/v1/notify/user.go | Adds Link headers alongside existing total-count logic for notification lists. |
| routers/api/v1/notify/repo.go | Adds Link headers for repo-scoped notifications. |
| routers/api/v1/admin/hooks.go | Refactors admin hooks listing to use GetGlobalWebhooks with ListOptions and emit pagination headers. |
| models/webhook/webhook_system.go | Introduces ListSystemWebhookOptions and GetGlobalWebhooks to support paginated global webhook queries. |
| models/webhook/webhook_system_test.go | Updates tests to cover GetGlobalWebhooks/ListSystemWebhookOptions while retaining an outdated test name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
routers/api/v1/user/key.go
Outdated
| } | ||
| } | ||
|
|
||
| ctx.SetLinkHeader(count, listOptions.PageSize) |
There was a problem hiding this comment.
When the fingerprint query parameter is provided, the query above uses db.Find without ListOptions, so pagination (page/limit) is ignored, but this line still sets the Link header based on listOptions.PageSize. That combination will generate pagination metadata that doesn't match the actual results (for example, clients may see multiple pages while every page returns the full, unpaginated result set); please either apply listOptions and a FindAndCount call in the fingerprint != "" branch as well, or skip SetLinkHeader/SetTotalCountHeader when filtering by fingerprint.
There was a problem hiding this comment.
Disagree with that. The promise is that it returns list of keys so the this is consistent with other lists (or at least that's what we're aiming for).
Besides from a quick look the function seems smart enough to skip those if there are no more pages
There was a problem hiding this comment.
I think the review is right. line 70 should also add ListOptions: listOptions,
There was a problem hiding this comment.
Added though I don't think it's needed - do we expect more than one key with the exact same fingerprint?
* giteaofficial/main: Refactor merge conan and container auth preserve actions taskID (go-gitea#36560) Fix assignee sidebar links and empty placeholder after go-gitea#32465 refactor (go-gitea#36559) Fix various version parsing problems (go-gitea#36553) Fix highlight diff result (go-gitea#36539) Refactor Nuget Auth to reuse Basic Auth Token Validation (go-gitea#36558) Update go dependencies (go-gitea#36548) Prevent navigation keys from triggering actions during IME composition (go-gitea#36540) Fix various mermaid bugs (go-gitea#36547) Add `elk` layout support to mermaid (go-gitea#36486) Allow configuring default PR base branch (fixes go-gitea#36412) (go-gitea#36425) [skip ci] Updated translations via Crowdin Color command/error logs in Actions log (go-gitea#36538) Add paging headers (go-gitea#36521) Fix issues filter dropdown showing empty label scope section (go-gitea#36535) [SECURITY] fix: Adjust the toolchain version (go-gitea#36537) Hide `add-matcher` and `remove-matcher` from actions job logs (go-gitea#36520) Improve timeline entries for WIP prefix changes in pull requests (go-gitea#36518)
Adds support for paging in admin/hooks api endpoint fixes: go-gitea#36516 --------- Co-authored-by: techknowlogick <techknowlogick@gitea.com> Co-authored-by: techknowlogick <matti@mdranta.net>
Adds support for paging in admin/hooks api endpoint
fixes: #36516