feat(tickets): add DELETE /tickets/{id} so E2E cleans up its company (PMS-149) #133

Merged
nrupard merged 2 commits from feat/pms-149-ticket-delete-route into main 2026-06-08 22:39:36 +02:00
Owner

Problem

The E2E suite leaked a company on every run. e2e/tests/tickets.spec.ts creates a run-tagged company and a ticket under it, but the tickets module exposed no DELETE route and delete_company (src/modules/contacts/service.rs:642) hard-refuses while any ticket references the company (400 "Cannot delete company with existing tickets"). global.teardown.ts's company DELETE therefore returned 400, was logged as companies failed=1, and the company plus its ticket stayed in the dedicated E2E tenant forever (the 24h stale sweep hit the same FK refusal every later run).

Change

  • Add TicketService::delete_ticket: tenant-scoped, 404 when the ticket is absent, mutation + audit row in one transaction (mirrors delete_company, PMS-117 audit convention). ticket_notes, ticket_status_history, and sla_tracking are removed by their existing ON DELETE CASCADE FKs (migrations/005_tickets.sql).
  • Wire DELETE /api/v1/tickets/{id} behind RequireAuth in ticket_routes, returning the same AppResult<()> (200) convention as delete_company.
  • global.teardown.ts: sweep tickets before contacts and companies (delete order matters for the FK guard), and extend the name matcher to read a ticket's title.
  • tickets.spec.ts: hard-delete the ticket then the company inline so the happy path exercises the new route; teardown still backstops failed runs.
  • Refresh the now-stale "tickets have no DELETE route" comments in the teardown header and e2e/README.md.

Verification

  • cargo fmt --all --check, cargo clippy --all-targets, and tsc --noEmit (e2e) all clean (run via the org rust-builder image; host has no toolchain).
  • The full Playwright E2E suite runs against staging in CI; after this change a green run leaves zero net new companies and tickets in the E2E tenant.

Closes PMS-149.

## Problem The E2E suite leaked a company on every run. `e2e/tests/tickets.spec.ts` creates a run-tagged company and a ticket under it, but the tickets module exposed no DELETE route and `delete_company` (`src/modules/contacts/service.rs:642`) hard-refuses while any ticket references the company (`400 "Cannot delete company with existing tickets"`). `global.teardown.ts`'s company DELETE therefore returned 400, was logged as `companies failed=1`, and the company plus its ticket stayed in the dedicated E2E tenant forever (the 24h stale sweep hit the same FK refusal every later run). ## Change - Add `TicketService::delete_ticket`: tenant-scoped, `404` when the ticket is absent, mutation + audit row in one transaction (mirrors `delete_company`, PMS-117 audit convention). `ticket_notes`, `ticket_status_history`, and `sla_tracking` are removed by their existing `ON DELETE CASCADE` FKs (`migrations/005_tickets.sql`). - Wire `DELETE /api/v1/tickets/{id}` behind `RequireAuth` in `ticket_routes`, returning the same `AppResult<()>` (200) convention as `delete_company`. - `global.teardown.ts`: sweep tickets before contacts and companies (delete order matters for the FK guard), and extend the name matcher to read a ticket's `title`. - `tickets.spec.ts`: hard-delete the ticket then the company inline so the happy path exercises the new route; teardown still backstops failed runs. - Refresh the now-stale "tickets have no DELETE route" comments in the teardown header and `e2e/README.md`. ## Verification - `cargo fmt --all --check`, `cargo clippy --all-targets`, and `tsc --noEmit` (e2e) all clean (run via the org rust-builder image; host has no toolchain). - The full Playwright E2E suite runs against staging in CI; after this change a green run leaves zero net new companies and tickets in the E2E tenant. Closes PMS-149.
feat(tickets): add DELETE /tickets/{id} so E2E cleans up its company
Some checks failed
E2E (staging) / Playwright against staging (pull_request) Failing after 31s
Check / fmt + clippy + compile + tests (pull_request) Successful in 57s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m37s
8a2e442e8c
The E2E tickets spec creates a run-tagged company plus a ticket under it, but the tickets module exposed no DELETE route and `delete_company` hard-refuses while any ticket references the company ("Cannot delete company with existing tickets"). Teardown's company DELETE therefore 400'd and one company plus its ticket leaked into the dedicated E2E tenant on every run, accumulating forever (the 24h stale sweep hit the same FK refusal).

Add `TicketService::delete_ticket` (tenant-scoped, 404 when absent, mutation + audit row in one transaction mirroring `delete_company`; ticket_notes / status-history / sla_tracking cascade via their existing ON DELETE CASCADE FKs) and wire `DELETE /api/v1/tickets/{id}` behind `RequireAuth`.

Update `global.teardown.ts` to sweep tickets before contacts and companies, extend the name-based matcher to read a ticket's `title`, and delete the ticket + company inline in `tickets.spec.ts` so the happy path exercises the new route. Refresh the stale "no DELETE route" notes in the teardown header and e2e/README.md.

#PMS-149

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(tickets): map ticket-delete FK violation to 400, not 500
Some checks failed
E2E (staging) / Playwright against staging (pull_request) Failing after 1m3s
Create release / Create release from merged PR (pull_request) Has been skipped
Check / fmt + clippy + compile + tests (pull_request) Successful in 3m25s
Build OCI container / Build and push mokosh-api image (push) Successful in 8m14s
451fa6d0be
Code review of the new DELETE /tickets/{id} caught that delete_ticket deleted blindly and relied on cascade, but several FKs to tickets(id) use the default RESTRICT, not ON DELETE CASCADE: time_entries (006_time_tracking.sql), child tickets via parent_ticket_id (005_tickets.sql:119), and billing rows (010_billing.sql:68). Deleting a production ticket that has time logged or a sub-ticket raised SQLSTATE 23503, which the blanket sqlx::Error -> AppError conversion (only 23505 is special-cased) turned into a generic 500 "Database operation failed" - inconsistent with the sibling delete_company, which returns a clean 400.

Catch 23503 on the ticket DELETE and return AppError::BadRequest with an actionable message; the transaction rolls back on the early return. This is localized to delete_ticket (covers every present and future referencing table without changing repo-wide error mapping) and keeps the endpoint's contract consistent with delete_company. The E2E path is unaffected: a freshly created test ticket has no time entries, sub-tickets, or billing, so the cascade still removes its notes/status-history. Doc comment corrected to describe the real FK graph.

#PMS-149

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nrupard deleted branch feat/pms-149-ticket-delete-route 2026-06-08 22:39:36 +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!133
No description provided.