fix(structures): align Structure.delegation_code FK action with Prisma default #229

Merged
julien merged 1 commits from fix/adr-0027-pr1-fk-action-on-delete into main 2026-05-26 12:11:14 +02:00
Owner

Summary

Single-line fix on the add_org_hierarchy migration shipped in #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 RESTRICTON 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

# 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

  • node scripts/check-catalogue-drift.mjs — clean (unchanged by this fix).
  • node --test scripts/check-catalogue-drift.spec.mjs — 22 tests passing (unchanged).
  • 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.
## 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.
julien added 1 commit 2026-05-26 12:10:52 +02:00
fix(structures): align Structure.delegation_code FK action with Prisma default
CI / scan (pull_request) Successful in 4m18s
CI / commits (pull_request) Successful in 4m19s
CI / check (pull_request) Failing after 4m32s
CI / a11y (pull_request) Successful in 2m55s
CI / perf (pull_request) Successful in 7m50s
b625090c63
ON DELETE RESTRICT was Prisma's default for required relations;
Structure.delegation is optional (`Delegation?`), so the matching
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. No production deployment
of #228 yet, so editing the migration in place rather than shipping a
corrective sibling - keeps the repo's migration history minimal.

Inline comment at the offending line explains the rule (nullable
Prisma relation -> SET NULL) to head off the same mistake when
ADR-0026 PR 1 adds optional FKs (Person.user, User.scopes).
julien merged commit ff1713eb6d into main 2026-05-26 12:11:14 +02:00
julien deleted branch fix/adr-0027-pr1-fk-action-on-delete 2026-05-26 12:11:17 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: julien/apf_portal#229