feat(api): standardize list responses on a shared {total, items} envelope - #90
Merged
Merged
Conversation
…lope Closes the half of L13 / APIR-8 that #61 deliberately deferred (that PR was held to the single `entries`->`items` audit rename; the broader consolidation was tracked in docs/ROADMAP.md § Backend structural cleanup). Thirteen endpoints that returned bare JSON arrays now return {total, items}: registries, git credentials, users, notification channels, scan schedules, API tokens, backups, filter presets, Docker environments, Trivy VEX documents, Trivy ignore rules, auth sessions, and scan artifacts. They stay unpaginated, so `total` equals `len(items)` today — enveloping them now makes adding pagination later an additive change rather than a second breaking one. The split is a rule, not a per-endpoint call: persisted resource collections take the envelope; fixed enumerations and live non-persisted data stay bare. Four endpoints are therefore deliberately unchanged (`/registries/options`, `/git-credentials/options`, `/notifications/events`, `/docker-environments/{id}/images`) and are documented as such in CONTRIBUTING.md § API conventions so a later review reads them as a decision. Implemented behind shared helpers per the ROADMAP note: new `app/api/pagination.py` holds `Page[ItemT]` and `full_page(items)`, and the three pre-existing envelopes (AuditPageOut, ScanHistoryPage, FindingsPage) are re-based onto it as subclasses, which keeps their OpenAPI component names stable. The frontend absorbs the change at the client boundary via `apiList`, so client signatures stay `Promise<T[]>` and no page component changed. `GET /api/scans` keeps its frozen Phase-P4 bare-array contract and is only marked deprecated, with a description naming the replacement (`GET /api/scans/history`) and the reason — exactly APIR-8's stated fix direction. Breaking for API-token consumers; recorded in CHANGELOG.md with the endpoint list and an action-required note. See docs/ARCHIVE.md § Deviations for the changes made in this pass.
…r seam Two follow-ups on the list-envelope work, no behavior change: - Expand the `apiList` comment to say explicitly that it discards `total` and returns only `items` — that is what keeps the thirteen client functions on their existing `Promise<T[]>` signatures — and that the envelope is therefore not purely a server-side detail: when real pagination lands, `total` has to be threaded back through to any page wanting a "showing N of M" count. - Add one test unwrapping a verbatim serialized `full_page()` payload. The backend proves it sends the envelope and the page-level tests mock the client functions, so nothing previously checked that the client's unwrapping matches the server's shape; both suites would have stayed green while the app broke at runtime.
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
1 participant
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.
Closes the half of L13 / APIR-8 that #61 deliberately deferred. That PR was held to the single
entries→itemsaudit rename per maintainer direction; the broader consolidation was tracked indocs/ROADMAP.md§ Backend structural cleanup, and this is it.What changed
Thirteen endpoints that returned bare JSON arrays now return the shared
{total, items}envelope — the shape/api/scans/history,/api/scans/{id}/findings, and/api/auditalready used:/api/registries·/api/git-credentials·/api/users·/api/notifications·/api/scan-schedules·/api/api-tokens·/api/backups·/api/filter-presets·/api/docker-environments·/api/trivy/vex-documents·/api/trivy/ignore-rules·/api/auth/sessions·/api/scans/{id}/artifactsThey remain unpaginated, so
totalalways equalsitems.lengthtoday. The point of enveloping them now is that adding pagination later becomes an additivelimit/offsetparameter rather than a second breaking change.The split is a rule, not a per-endpoint call. Persisted resource collections — rows that grow with usage, where a count is a meaningful answer — take the envelope. Fixed enumerations and live, non-persisted data stay bare arrays, because
totalthere answers a question nobody asks. Four endpoints are therefore deliberately unchanged:GET /api/registries/optionsGET /api/git-credentials/optionsGET /api/notifications/eventslist[str]vocabularyGET /api/docker-environments/{id}/imagesThe rule and these exceptions are documented in
CONTRIBUTING.md§ API conventions so a future review reads them as a decision rather than as drift — the same treatment the other deferrals got tracking references for.Behind shared helpers, per the ROADMAP note. New
backend/app/api/pagination.pyholds a genericPagemodel and afull_page(items)constructor; each route declaresresponse_model=Page[ThingOut]and returnsfull_page([...])rather than hand-rolling the dict. The three pre-existing envelopes (AuditPageOut,ScanHistoryPage,FindingsPage) are re-based onto it as subclasses, which collapses the duplicated field declarations while keeping their OpenAPI component names unchanged — a bare parameterizedPagewould have renamed them toPage_AuditEntryOut_and churned a future generated client for no gain.GET /api/scanswas left frozen and only deprecated. It is the one bare array that is paginated (limit/offset, no total), so the one the envelope would materially fix — but its shape is a documented frozen contract from Phase P4. It now carriesdeprecated=Trueplus a description naming both the replacement (GET /api/scans/history) and the reason, so a reader of the generated client sees the why and the where-to-go rather than only a flag.listScansin the TS client carries the matching@deprecatedJSDoc. This closes APIR-8 as its own fix direction stated it.Frontend
The change is absorbed at the client boundary: a new
apiListhelper infrontend/src/api/client.tsunwraps the envelope, so the thirteen client functions keep returning a promise of a plain array. No page component and no existing test changed — the SPA's page-level tests mock the API-client functions rather thanfetch, so all 18 frontend test files passed untouched.FindingsPage/ScanHistoryPagebecame aliases of a shared genericPageinterface.Contract change
Breaking for external consumers driving these endpoints with an API token: read rows from
.items. Recorded inCHANGELOG.mdunder Unreleased with the full endpoint list and an action-required note. The web UI is unaffected.Verification
ruffandblack --checkclean.tsc --noEmit, andvite buildclean.backend/tests/test_list_envelope.pyparametrizes over every enveloped endpoint (shape, key set,total == len(items)), over the bare-array exceptions (asserting they stay bare), thattotaltracks rows as a collection grows, and that/api/scansis still a bare array and carries the deprecation marker naming its replacement.itemskey.docker compose up//healthz— no Docker daemon available here. This PR touches no Dockerfile, Compose, or health-endpoint code.Docs:
CONTRIBUTING.md§ API conventions (new),CHANGELOG.md,docs/reviews/STATUS.md(L13/APIR-8 moved out of § 2 "Deferred by decision"),docs/ROADMAP.md(list-envelope half struck; the secret-CRUD-router consolidation remains open). Seedocs/ARCHIVE.md§ Deviations for the changes made in this pass.