fix(timesheets): request full page in history range test (PMS-507) #361

Merged

David merged 2 commits from fix/PMS-507-timesheet-history-range into main

2026-06-25 20:14:06 +02:00

Claude-Run commented

2026-06-25 09:04:53 +02:00

Member

The integration test list_timesheets_status_filter_and_range built its pagination via PaginationParams::default(). That type derives Default, so ::default() yields per_page = 0; per_page() then clamps zero up to one, producing LIMIT 1. The serde defaults (page=1, per_page=25) only apply when a query string is deserialized through the handler, never on a direct ::default() in a service-level test. With ORDER BY week_start DESC LIMIT 1 the status=all listing returned only the newest (pending) week, so assert_eq!(all.len(), 3) failed with 1. The per-status assertions each expect exactly one row, so the clamp left them passing and masked the cap.

Production is unaffected: the route handler deserializes PaginationParams from the query string and gets the serde default per_page = 25. The defect was confined to the test helper, which now constructs an explicit full page (page=1, per_page=100).

#PMS-507

The integration test `list_timesheets_status_filter_and_range` built its pagination via `PaginationParams::default()`. That type derives `Default`, so `::default()` yields `per_page = 0`; `per_page()` then clamps zero up to one, producing `LIMIT 1`. The serde defaults (page=1, per_page=25) only apply when a query string is deserialized through the handler, never on a direct `::default()` in a service-level test. With `ORDER BY week_start DESC LIMIT 1` the `status=all` listing returned only the newest (pending) week, so `assert_eq!(all.len(), 3)` failed with 1. The per-status assertions each expect exactly one row, so the clamp left them passing and masked the cap. Production is unaffected: the route handler deserializes `PaginationParams` from the query string and gets the serde default `per_page = 25`. The defect was confined to the test helper, which now constructs an explicit full page (page=1, per_page=100). #PMS-507

Claude-Run added 2 commits

2026-06-25 09:04:53 +02:00

feat(timesheets): status filter + multi-week range + decision audit on list (PMS-506)

E2E / Playwright against staging (pull_request) Successful in 59s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 4m39s

Details

Integration / integration tests (pull_request) Failing after 12m22s

Details

60b7fe51bf

The week-rolled `GET /timesheets` endpoint accepted only `user_id` +
`week` filters and dropped the decision audit (approved_by_id /
approved_at / rejection_reason) on its way through the
`TimesheetRow -> TimesheetSummaryResponse` conversion. The SPA's
admin approvals queue then filtered the response down to
`approval_status == "pending"`, so approved + rejected weeks
"disappeared" after the admin clicked Approve - matches the QA
report.

Server contract changes:
- TimesheetFilter gains `status: Option<String>` (`pending` /
  `approved` / `rejected` / `all`), `from: Option<NaiveDate>`, and
  `to: Option<NaiveDate>`. status validates server-side (422 on an
  unknown value); from/to overrides the legacy single-`week` field.
- list_timesheets resolves `from`/`to` against the Monday anchor,
  caps the span at 26 weeks (422 if longer), widens the date WHERE
  to scan the range, and HAVING-filters the rolled CASE expression
  on the requested status. The aggregate carries decided_at
  (`MAX(approved_at)`), decided_by_id, and rejection_reason - all
  identical across the rolled rows because approve_week / reject_week
  run a single UPDATE per week - via `ARRAY_AGG ... FILTER`.
- TimesheetSummaryResponse exposes decided_by_id / decided_at /
  rejection_reason as `Option`s; they serialize as absent on pending
  weeks.

Integration tests (tests/timesheet_history.rs):
- list_timesheets_status_filter_and_range seeds three weeks (one
  approved, one rejected with a reason, one pending), then asserts
  each status filter returns exactly the right subset with the right
  audit fields populated.
- list_timesheets_rejects_unknown_status pins the 422 from the
  enum guard.
- list_timesheets_caps_range_at_26_weeks pins the 26-week scan cap.

Client side (status filter + range mode + history rendering on
`TimesheetApprovalsPage`) lands in the mokosh-apps PR under the same
ticket id.

#PMS-506

fix(timesheets): request full page in history range test (PMS-507)

E2E / Playwright against staging (pull_request) Successful in 52s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 2m50s

Details

Integration / integration tests (pull_request) Successful in 10m15s

Details

43583f1534

The integration test `list_timesheets_status_filter_and_range` built its pagination via `PaginationParams::default()`. That type derives `Default`, so `::default()` yields `per_page = 0`; `per_page()` then clamps zero up to one, producing `LIMIT 1`. The serde defaults (page=1, per_page=25) only apply when a query string is deserialized through the handler, never on a direct `::default()` in a service-level test. With `ORDER BY week_start DESC LIMIT 1` the `status=all` listing returned only the newest (pending) week, so `assert_eq!(all.len(), 3)` failed with 1. The per-status assertions each expect exactly one row, so the clamp left them passing and masked the cap.

Production is unaffected: the route handler deserializes `PaginationParams` from the query string and gets the serde default `per_page = 25`. The defect was confined to the test helper, which now constructs an explicit full page (page=1, per_page=100).

#PMS-507

David added 1 commit

2026-06-25 20:06:52 +02:00

Merge origin/main into fix/PMS-507-timesheet-history-range

E2E / Playwright against staging (pull_request) Successful in 1m1s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 4m11s

Details

Create release / Create release from merged PR (pull_request) Successful in 4s

Details

Integration / integration tests (pull_request) Successful in 12m42s

Details

b4403d264c

Resolve the conflict in tests/timesheet_history.rs: main independently
landed an equivalent explicit-page fix (per_page=25) in the same helper.
Keep this branch's full-page intent (per_page=100) per the PR title, with
the seeded 3-week fixture comfortably under the page size.

#PMS-507