fix(contacts): 400 not 500 deleting a company with related records (PMS-170) #145

Merged

nrupard merged 1 commit from fix/pms-170-company-delete-fk-400 into main

2026-06-09 18:52:26 +02:00

nrupard commented

2026-06-09 18:48:53 +02:00

Owner

What

Fixes PMS-170: DELETE /api/v1/contacts/companies/{id} returned HTTP 500 when the company was still referenced by a child row, instead of a 400.

Cause

delete_company (src/modules/contacts/service.rs) explicitly guards only tickets. Every other table that foreign-keys companies (contracts, invoices, payments, projects, assets, time entries, appointments, sub-companies via parent_company_id, ...) is ON DELETE RESTRICT, so the DELETE raises Postgres 23503, which the generic From<sqlx::Error> maps to AppError::Database -> 500. Surfaced by the PMS-155 E2E teardown (DELETE …/companies/819d811c… -> 500).

Fix

Catch 23503 on the DELETE and return AppError::BadRequest with an actionable message naming the likely blockers - the same shape as the PMS-149 ticket-delete fix. Other database errors propagate unchanged; CASCADE children (contacts, company_sites) still delete cleanly.

Tests

tests/company_delete.rs: seeds a parent company and a child referencing it via parent_company_id (the minimal RESTRICT blocker, no other FKs to satisfy), then asserts DELETE of the parent returns 400.
cargo check --all-targets, clippy --all-targets, fmt --check all clean.

Note

Independent of PMS-178 (audit-list 500, PR #144). With both merged + deployed, the PMS-155 E2E run is fully green (PMS-178 was the test failure; PMS-170 was the best-effort teardown 500 warning).

🤖 Generated with Claude Code

## What Fixes PMS-170: `DELETE /api/v1/contacts/companies/{id}` returned HTTP 500 when the company was still referenced by a child row, instead of a 400. ## Cause `delete_company` (`src/modules/contacts/service.rs`) explicitly guards only `tickets`. Every other table that foreign-keys `companies` (contracts, invoices, payments, projects, assets, time entries, appointments, sub-companies via `parent_company_id`, ...) is `ON DELETE RESTRICT`, so the `DELETE` raises Postgres `23503`, which the generic `From<sqlx::Error>` maps to `AppError::Database` -> 500. Surfaced by the PMS-155 E2E teardown (`DELETE …/companies/819d811c… -> 500`). ## Fix Catch `23503` on the DELETE and return `AppError::BadRequest` with an actionable message naming the likely blockers - the same shape as the PMS-149 ticket-delete fix. Other database errors propagate unchanged; `CASCADE` children (contacts, company_sites) still delete cleanly. ## Tests - `tests/company_delete.rs`: seeds a parent company and a child referencing it via `parent_company_id` (the minimal RESTRICT blocker, no other FKs to satisfy), then asserts `DELETE` of the parent returns 400. - `cargo check --all-targets`, `clippy --all-targets`, `fmt --check` all clean. ## Note Independent of PMS-178 (audit-list 500, PR #144). With both merged + deployed, the PMS-155 E2E run is fully green (PMS-178 was the test failure; PMS-170 was the best-effort teardown 500 warning). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit

2026-06-09 18:48:53 +02:00

fix(contacts): return 400 not 500 when deleting a company with related records

E2E (staging) / Playwright against staging (pull_request) Failing after 1m17s

Details

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

Details

Check / fmt + clippy + compile + tests (pull_request) Successful in 3m41s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 8m35s

Details

7e138359ae

DELETE /api/v1/contacts/companies/{id} returned HTTP 500 when the company was still referenced by a child row. delete_company explicitly guards only `tickets` (400 "existing tickets"); every other table that foreign-keys `companies` (contracts, invoices, payments, projects, assets, time entries, appointments, sub-companies, ...) is ON DELETE RESTRICT, so the DELETE raised Postgres 23503, which the generic From<sqlx::Error> mapped to AppError::Database -> 500.

Catch 23503 on the DELETE and return AppError::BadRequest with an actionable message naming the likely blockers, mirroring the PMS-149 ticket-delete fix. Other database errors still propagate as before. CASCADE children (contacts, company_sites) are unaffected and still delete cleanly.

Add an integration regression test (tests/company_delete.rs) using the self-referential parent/child company FK as the minimal RESTRICT blocker: deleting a company that a child references returns 400.

#PMS-170

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