feat(api): paginate every list handler (PMS-127) #65

Merged
nrupard merged 2 commits from feat/pms-127-pagination into main 2026-06-04 20:43:13 +02:00
Owner

Summary

  • Route every list_* handler across 18 modules through utils/pagination.rs. 53 handlers now accept Query<PaginationParams>, ask the service for (Vec<T>, u64), and serialize via PaginatedResponse::from_params. Closes cross-cutting issue #7 from dev-docs/codebase-state.md.
  • Tenants list_tenants service signature normalized to &PaginationParams (the one inconsistent shape left in the tree).
  • One intentional hold-out: calendar::who_is_on_call_now stays Json<Vec<OnCallNowResponse>> because it is a point-in-time resolver, not a paginated list.
  • assets::list_asset_audit_log previously hard-capped at LIMIT 500; that cap is dropped in favour of the standard pagination clamps.
  • Decryption pipelines (billing, notifications, rmm, assets) materialize the decrypted Vec first then return (items, total) to keep the Result<Vec<_>>::collect() step intact.

Breaking change

Every list endpoint that was previously returning a bare JSON array now returns the {"data": [...], "meta": {...}} envelope. Client (mokosh-clients / mokosh-apps) must be updated in lock-step to read body.data. Endpoints already paginated before this change (tenants/list, contacts/companies, contacts/contacts, tickets/list, billing/invoices, billing/payments, projects/list, time_tracking/time_entries, audit/list, portal/list_*) are unaffected.

Test plan

  • CI: cargo fmt --all --check
  • CI: cargo clippy --all-targets -- -D warnings
  • CI: cargo check --all-targets
  • CI: cargo test --lib + every integration test still passes (work-types and inbox tests updated to read body["data"]).
  • Manual: paginated calls like ?page=2&per_page=10 return the requested page with correct meta.total.

Closes PMS-127.

## Summary - Route every `list_*` handler across 18 modules through `utils/pagination.rs`. 53 handlers now accept `Query<PaginationParams>`, ask the service for `(Vec<T>, u64)`, and serialize via `PaginatedResponse::from_params`. Closes cross-cutting issue #7 from `dev-docs/codebase-state.md`. - Tenants `list_tenants` service signature normalized to `&PaginationParams` (the one inconsistent shape left in the tree). - One intentional hold-out: `calendar::who_is_on_call_now` stays `Json<Vec<OnCallNowResponse>>` because it is a point-in-time resolver, not a paginated list. - `assets::list_asset_audit_log` previously hard-capped at `LIMIT 500`; that cap is dropped in favour of the standard pagination clamps. - Decryption pipelines (billing, notifications, rmm, assets) materialize the decrypted Vec first then return `(items, total)` to keep the `Result<Vec<_>>::collect()` step intact. ## Breaking change Every list endpoint that was previously returning a bare JSON array now returns the `{"data": [...], "meta": {...}}` envelope. Client (`mokosh-clients` / `mokosh-apps`) must be updated in lock-step to read `body.data`. Endpoints already paginated before this change (tenants/list, contacts/companies, contacts/contacts, tickets/list, billing/invoices, billing/payments, projects/list, time_tracking/time_entries, audit/list, portal/list_*) are unaffected. ## Test plan - [ ] CI: `cargo fmt --all --check` - [ ] CI: `cargo clippy --all-targets -- -D warnings` - [ ] CI: `cargo check --all-targets` - [ ] CI: `cargo test --lib` + every integration test still passes (work-types and inbox tests updated to read `body["data"]`). - [ ] Manual: paginated calls like `?page=2&per_page=10` return the requested page with correct `meta.total`. Closes PMS-127.
feat(api): route every list handler through utils/pagination (PMS-127)
All checks were successful
Check / fmt + clippy + compile + tests (pull_request) Successful in 57s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m9s
91e1a40296
Cross-cutting issue #7 (`dev-docs/codebase-state.md`): the pagination helper existed but most list endpoints bypassed it, returning bare `Json<Vec<T>>` that grew unbounded as a tenant's data grew. Sweeps every `list_*` (plus the few `get_*` lookup helpers that returned full arrays) across eighteen modules so they accept `Query<PaginationParams>`, ask the service for `(Vec<T>, u64)`, and serialize via `PaginatedResponse::from_params`. Tenants' `list_tenants` service signature is normalized to `&PaginationParams` (previously took `(page, per_page)` directly, the one inconsistent shape left in the tree).

53 handlers were converted across `billing`, `assets`, `contacts`, `projects`, `notifications`, `auth`, `rmm`, `contracts`, `time_tracking`, `knowledge_base`, `calendar`, `sla`, `settings`, `tickets`, and `tenants`. The one intentional hold-out is `calendar::who_is_on_call_now` (line ~294 in routes.rs): it is a point-in-time resolver, not a paginated list, and stays `Json<Vec<OnCallNowResponse>>`. The `assets::list_asset_audit_log` previously hard-capped at `LIMIT 500`; that cap is dropped in favour of the standard pagination limits.

Each converted service method paired its existing SELECT with a COUNT that mirrors the same WHERE bindings so `total` stays in sync with the page. Decryption pipelines (`billing::list_payment_gateways`, `notifications::list_channels`, `rmm::list_connections`, `assets::list_configuration_items`, `assets::list_credentials`) materialise the decrypted Vec first then return `(items, total)` to keep the `Result<Vec<_>>::collect()` step intact.

The two integration tests that read raw arrays (`tests/time_tracking.rs` work-types and `tests/notifications.rs` inbox) are updated to read `body["data"]`; the other tests already hit endpoints that were paginated before this change.

This is a BREAKING response-shape change for every list endpoint that was previously returning a bare array. Client coordination required.

#PMS-127
docs(pagination): clarify audit-log cap drop + dual-Query convention
All checks were successful
Check / fmt + clippy + compile + tests (pull_request) Successful in 5m2s
Create release / Create release from merged PR (pull_request) Has been skipped
Build OCI container / Build and push mokosh-api image (push) Successful in 8m4s
a03bffff57
Code review feedback on PMS-127:

- `assets::list_asset_audit_log` previously hard-capped at `LIMIT 500`. The pagination clamp (`MAX_PER_PAGE = 100`) is stricter on single-response payload, but the cap drop reads as a regression on first glance. Document that the per-page clamp is intentional and that page-by-page enumeration of the full history is the explicit point of the endpoint.

- Most paginated handlers stack two `Query<_>` extractors (filter + pagination). axum parses the query string twice, which only works while no `*Filter` (or `PaginationParams`) sets `#[serde(deny_unknown_fields)]`. Document the convention in `utils/pagination.rs` so a future filter author does not accidentally 400 every paginated request by adding strict serde to their filter type.

#PMS-127
nrupard deleted branch feat/pms-127-pagination 2026-06-04 20:43:13 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!65
No description provided.