From ff1713eb6dca776d66ac0fa4a9fa16bd8a7bad52 Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Tue, 26 May 2026 12:11:13 +0200 Subject: [PATCH] fix(structures): align Structure.delegation_code FK action with Prisma default (#229) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Single-line fix on the `add_org_hierarchy` migration shipped in [#228](https://git.unespace.com/julien/apf_portal/pulls/228) (ADR-0027 PR 1). The FK action on `Structure.delegation_code` was `ON DELETE RESTRICT` (Prisma's default for **required** relations); the field is **optional** (`Structure.delegation Delegation?`), so Prisma's default is `ON DELETE SET NULL`. Mismatch caused `prisma migrate dev` to detect drift and prompt for a corrective migration on every run. Caught locally on first VM-side validation (the workspace dev DB). No production deployment of #228 yet, so the fix is **edit-in-place** rather than a corrective sibling migration — keeps the repo's migration history clean. ## What lands | File | Change | | --- | --- | | `apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql` | `ON DELETE RESTRICT` → `ON DELETE SET NULL` on the `structures_delegation_code_fkey` FK. Inline comment explains the rule (nullable Prisma relation → SET NULL is the matching default; not matching it generates drift on every `migrate dev`). | That's it. One line of SQL, one comment block. No schema.prisma change, no test change, no other file touched. ## Why edit-in-place vs new corrective migration Edit-in-place is safe here because: - The broken migration only exists in **dev local DBs** (Julien's WSL postgres). No staging, no preview env, no prod has applied it. - Every dev who pulls this PR's fix wipes their local DB (`./infra/local/dev.sh down -v`) and reapplies — clean migration history, no "modified after applied" warning. - The repo's migration list stays minimal — adding a corrective migration would carry the wart forever in `prisma/migrations/`. Once a non-dev environment has applied a migration, the rule reverses: corrective migration mandatory, never edit in place. ADR-0015's "trunk-based + squash-merge" and the absence of a deployed environment at this stage gives us this one-time window. ## Recovery procedure for anyone who applied #228 ```bash # From the workspace root ./infra/local/dev.sh down -v # wipe the local postgres volume ./infra/local/dev.sh up # Pull this fix git pull # or git switch fix/adr-0027-pr1-fk-action-on-delete depending on local state # Reapply migrations — no prompt this time cd apps/portal-bff && pnpm exec prisma migrate dev # Should report: "Applied migration `20260526143000_add_org_hierarchy`" and exit cleanly. ``` If anyone has a stray `drift_inspection/` folder under `prisma/migrations/` (artifact of the `--create-only` debugging step), delete it before reapplying — it's not in the repo, it was just diagnostic output. ## Test plan - [x] `node scripts/check-catalogue-drift.mjs` — clean (unchanged by this fix). - [x] `node --test scripts/check-catalogue-drift.spec.mjs` — 22 tests passing (unchanged). - [x] `pnpm exec prettier --check` clean (SQL file unaffected by prettier but verified). - [ ] **Locally on WSL** — wipe DB → pull fix → `prisma migrate dev` exits clean, no drift prompt. - [ ] **Review focus** — the comment block in the migration explaining the rule (future-proof against the same mistake when ADR-0026 PR 1 adds optional relations). ## Notes for the reviewer - **Why a comment block on the FK line, not on every nullable FK in future migrations?** This was caught the first time we hit it; a short note at the offending site is cheaper than amending `CLAUDE.md` with a "remember to match Prisma default actions" rule. If we hit the same mistake on ADR-0026 PR 1's `Person`/`User`/`UserScope` migration, we'll consider promoting it. For now: one inline comment at the place where the rule is non-obvious. - **The drift gate doesn't catch this kind of mistake.** Catalogue drift gate scans string literals against TypeScript catalogues — it doesn't look at SQL referential actions. Postgres CHECK constraints (introduced for `Structure.kind` in this same migration) are not the same surface as FK referential actions. Catching this required actually running `prisma migrate dev`. A possible future improvement: a CI gate that runs `prisma migrate diff --from-migrations --to-schema-datamodel --script --shadow-database-url …` and fails if the diff is non-empty (zero-drift gate). Worth an ADR amendment if it becomes a recurring class of bug. --------- Co-authored-by: Julien Gautier Reviewed-on: https://git.unespace.com/julien/apf_portal/pulls/229 --- .../20260526143000_add_org_hierarchy/migration.sql | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql b/apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql index 49c5cc2..ffe4372 100644 --- a/apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql +++ b/apps/portal-bff/prisma/migrations/20260526143000_add_org_hierarchy/migration.sql @@ -83,9 +83,13 @@ ALTER TABLE "delegations" ADD CONSTRAINT "delegations_region_code_fkey" ON DELETE RESTRICT ON UPDATE CASCADE; -- AddForeignKey +-- ON DELETE SET NULL because delegationCode is optional in the Prisma +-- schema (Structure.delegation Delegation?). Matches Prisma's default +-- for nullable relations; the drift gate would otherwise flag this +-- on every `prisma migrate dev`. ALTER TABLE "structures" ADD CONSTRAINT "structures_delegation_code_fkey" FOREIGN KEY ("delegation_code") REFERENCES "delegations"("code") - ON DELETE RESTRICT ON UPDATE CASCADE; + ON DELETE SET NULL ON UPDATE CASCADE; -- ------------------------------------------------------------------ -- Test-tenant reference seed (per ADR-0027 §"Seeding posture").