fix(audit): correct SQL placeholder numbering in audit-log list (PMS-178) #144

Merged

nrupard merged 1 commit from fix/pms-178-audit-list-placeholders into main

2026-06-09 18:46:29 +02:00

nrupard commented

2026-06-09 18:43:01 +02:00

Owner

What

Fixes PMS-178: GET /api/v1/audit-log returned HTTP 500 whenever any filter (entity_type, action, user_id, from, to) was supplied.

Cause

In AuditService::list the dynamic filter conditions were numbered starting at $3, colliding with the fixed LIMIT $2 OFFSET $3, and the bind order (tenant, limit, offset, filters…) did not match the placeholder indices, so the code bound more values than the prepared statement declared and Postgres rejected it. The same where_clause was also shared by the data query (binds limit/offset) and the count query (does not), so no single placeholder scheme was correct for both. No-filter reads happened to work, hiding it until the RequireAdmin audit read was first exercised with filters by the PMS-155 E2E suite (GET /audit-log?entity_type=companies&action=create -> 500).

Fix

Number tenant + each present filter sequentially from $1.
Append LIMIT/OFFSET as the last two placeholders of the data query and bind them last; the count query binds neither.
Bind tenant + filters in the same order for both queries so the shared where_clause lines up.

Tests

New integration regression test tests/audit_list.rs: a seeded super-admin issues GET /api/v1/audit-log across several filter combinations (none, entity_type+action, user_id, from+to+entity_type+action) and asserts each returns 200. An empty result set is still a 200; the bug was at the query layer.
cargo check --all-targets, clippy --all-targets, fmt --check all clean.

Note

Once this deploys to staging, the PMS-155 E2E audit spec goes green. The remaining [teardown] DELETE …companies… -> 500 line in that run is a separate bug, PMS-170 (unguarded FK -> 500 on company delete), not addressed here.

🤖 Generated with Claude Code

## What Fixes PMS-178: `GET /api/v1/audit-log` returned HTTP 500 whenever any filter (`entity_type`, `action`, `user_id`, `from`, `to`) was supplied. ## Cause In `AuditService::list` the dynamic filter conditions were numbered starting at `$3`, colliding with the fixed `LIMIT $2 OFFSET $3`, and the bind order (`tenant, limit, offset, filters…`) did not match the placeholder indices, so the code bound more values than the prepared statement declared and Postgres rejected it. The same `where_clause` was also shared by the data query (binds limit/offset) and the count query (does not), so no single placeholder scheme was correct for both. No-filter reads happened to work, hiding it until the `RequireAdmin` audit read was first exercised with filters by the PMS-155 E2E suite (`GET /audit-log?entity_type=companies&action=create -> 500`). ## Fix - Number tenant + each present filter sequentially from `$1`. - Append `LIMIT`/`OFFSET` as the last two placeholders of the data query and bind them last; the count query binds neither. - Bind tenant + filters in the same order for both queries so the shared `where_clause` lines up. ## Tests - New integration regression test `tests/audit_list.rs`: a seeded super-admin issues `GET /api/v1/audit-log` across several filter combinations (none, entity_type+action, user_id, from+to+entity_type+action) and asserts each returns 200. An empty result set is still a 200; the bug was at the query layer. - `cargo check --all-targets`, `clippy --all-targets`, `fmt --check` all clean. ## Note Once this deploys to staging, the PMS-155 E2E audit spec goes green. The remaining `[teardown] DELETE …companies… -> 500` line in that run is a separate bug, PMS-170 (unguarded FK -> 500 on company delete), not addressed here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit

2026-06-09 18:43:01 +02:00

fix(audit): correct SQL placeholder numbering in audit-log list query

E2E (staging) / Playwright against staging (pull_request) Failing after 40s

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 1m43s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 3m9s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

7acb419f82

GET /api/v1/audit-log returned 500 whenever any filter (entity_type, action, user_id, from, to) was supplied. The dynamic filter conditions in AuditService::list started numbering at $3, colliding with the fixed LIMIT $2 OFFSET $3, and the bind order (tenant, limit, offset, filters) did not match the placeholder indices, so the statement bound more values than it declared and Postgres rejected it. The same where_clause was also shared between the data query (which binds limit/offset) and the count query (which does not), so one placeholder scheme could not be correct for both. No-filter reads happened to work, which is why it stayed hidden until the audit-log read (RequireAdmin) was first exercised with filters by the PMS-155 E2E suite.

Number tenant + filters sequentially from $1, append LIMIT/OFFSET as the last two placeholders of the data query (bound last), and bind tenant + filters in the same order for both queries so the shared where_clause lines up. Add an integration regression test (tests/audit_list.rs) asserting the filtered list returns 200 across several filter combinations.

#PMS-178

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>