fix(api): repair list ORDER BY misuse + time-entries count placeholders (PMS-145) #116

Merged

longjacksonle merged 1 commit from fix/pms-145-time-entries-count-placeholders into main

2026-06-05 21:09:51 +02:00

longjacksonle commented

2026-06-05 20:53:14 +02:00

Owner

What

Closes PMS-145. GET /api/v1/time-entries returned 500 (found via the ticket detail page's Time Logged section, which filters by ticket_id). Two independent bugs, both fixed here.

1. ORDER BY misuse (the actual 500, and broader)

PaginationParams::order_by appends a direction to a single column name, but four callers passed a default_field that already embedded a direction:

Caller	was	effect
`time_tracking::list_time_entries`	`"date DESC, start_time DESC"`	`ORDER BY date DESC, start_time DESC DESC` -> 500
`billing::list_invoices`	`"invoice_date DESC"`	`... DESC DESC` -> 500
`billing::list_payments`	`"payment_date DESC"`	`... DESC DESC` -> 500
`projects::list_projects`	`"created_at DESC"`	`... DESC DESC` -> 500

So those list endpoints 500'd on every call (filtered or not). Bare-column callers (tickets, contacts) were unaffected. Fixed each to pass a bare column (default direction is already DESC), and hardened order_by to keep only the first whitespace token so this can't recur.

2. Count-query placeholder bug (latent behind #1)

list_time_entries reused one where_clause for both queries, but the data query numbers filters from $4 while the count query binds only $1 then filters from $2 - so with any filter the count referenced an unbound $4. Fixed with independent numbering (mirrors billing::list_invoices).

Tests

Extends the time-tracking integration test to list the logged entry filtered by ticket_id and assert 200 + entry present + total.

test service_desk_time_slice_happy_path ... ok      (time_tracking)
test generate_invoice_from_time_entries ... ok      (billing)
test payment_against_generated_invoice_transitions_status ... ok
test utils::pagination::tests::test_order_by ... ok

cargo fmt --check clean; cargo clippy --lib --tests clean.

Unblocks

mokosh-apps PR #68 (MAPPS-123) Time Logged section, which degrades gracefully until this lands.

🤖 Generated with Claude Code

## What Closes **PMS-145**. `GET /api/v1/time-entries` returned **500** (found via the ticket detail page's Time Logged section, which filters by `ticket_id`). Two independent bugs, both fixed here. ### 1. ORDER BY misuse (the actual 500, and broader) `PaginationParams::order_by` appends a direction to a **single column name**, but four callers passed a `default_field` that already embedded a direction: | Caller | was | effect | |---|---|---| | `time_tracking::list_time_entries` | `"date DESC, start_time DESC"` | `ORDER BY date DESC, start_time DESC DESC` -> 500 | | `billing::list_invoices` | `"invoice_date DESC"` | `... DESC DESC` -> 500 | | `billing::list_payments` | `"payment_date DESC"` | `... DESC DESC` -> 500 | | `projects::list_projects` | `"created_at DESC"` | `... DESC DESC` -> 500 | So those list endpoints 500'd on **every** call (filtered or not). Bare-column callers (tickets, contacts) were unaffected. Fixed each to pass a bare column (default direction is already DESC), and hardened `order_by` to keep only the first whitespace token so this can't recur. ### 2. Count-query placeholder bug (latent behind #1) `list_time_entries` reused one `where_clause` for both queries, but the data query numbers filters from `$4` while the count query binds only `$1` then filters from `$2` - so with any filter the count referenced an unbound `$4`. Fixed with independent numbering (mirrors `billing::list_invoices`). ## Tests Extends the time-tracking integration test to list the logged entry filtered by `ticket_id` and assert 200 + entry present + total. ``` test service_desk_time_slice_happy_path ... ok (time_tracking) test generate_invoice_from_time_entries ... ok (billing) test payment_against_generated_invoice_transitions_status ... ok test utils::pagination::tests::test_order_by ... ok ``` `cargo fmt --check` clean; `cargo clippy --lib --tests` clean. ## Unblocks mokosh-apps PR #68 (MAPPS-123) Time Logged section, which degrades gracefully until this lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

longjacksonle added 1 commit

2026-06-05 20:53:14 +02:00

fix(api): repair list ORDER BY misuse + time-entries count placeholders

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

Details

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

Details

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

Details

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

Details

df0ce4bd50

Closes PMS-145. GET /api/v1/time-entries returned 500 (surfaced by the ticket detail page's Time Logged section, which filters by ticket_id). Two independent bugs:

1. ORDER BY misuse. PaginationParams::order_by appends a sort direction to a single column name, but four callers passed a default_field that already embedded a direction (and, for time entries, a second column): "date DESC, start_time DESC", "invoice_date DESC", "payment_date DESC", "created_at DESC". Each produced "ORDER BY <col> DESC DESC" -> "syntax error at or near DESC" -> 500 on EVERY call to those list endpoints (time entries, invoices, payments, projects), filtered or not. The bare-column callers (tickets, contacts) were fine. Fixed each call site to pass a bare column (default direction is already DESC), and hardened order_by to keep only the first whitespace token so the footgun is non-fatal in future.

2. Count-query placeholder bug in list_time_entries (latent behind bug 1). It reused one where_clause for both the data and count queries, but the data query numbers filters from $4 (after $1 tenant, $2 limit, $3 offset) while the count query binds only $1 tenant then filters from $2. With any filter the count query referenced an unbound $4. Fixed by numbering the data and count placeholders independently, mirroring billing::list_invoices.

Test: extends the time-tracking integration test to list the logged entry filtered by ticket_id and assert 200 + the entry + total. pagination unit tests, billing, and time_tracking integration tests all green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>