ehoandClaude Opus 5
12f17d9d73
Two backlog trees are complete: `docs/project/backlog/` (75 files, every WP done) and `docs/project/refactor-backlog-setup/` (the arc before it). Move both under `docs/project/archive/` with `git mv`, so history stays intact through `git log --follow`. `SHOWCASE-ROADMAP.md` moves with them, because it points at the now-archived backlog README. Add `docs/project/archive/README.md`. It states that these trees are historical and names the two directories that are still live. Repoint every inbound reference named in RD-30's Files table: CLAUDE.md, the root README, both backend READMEs, `LetterHtml.cs`, `a11y.mdx`, the `document-feature` and `new-ssp` skills, and the readable-codebase PLAN, README, and RD-19 ticket. Fix two upward-relative links inside the moved WP files (WP-68, WP-69) that gained a directory level and would otherwise break. Repoint `.prettierignore`'s two agent-prompt exclusions to their new path, so prettier keeps leaving those files' exact wording alone. Mark RD-30 done and check off its acceptance criteria; flip its README row to done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
91 lines
5.3 KiB
Markdown
91 lines
5.3 KiB
Markdown
# WP-73 — `RegistrationStatus` and `Aanvraag` as closed unions
|
|
|
|
Status: done (6bc00a9)
|
|
Phase: 12 — DDD hardening
|
|
|
|
## Why
|
|
|
|
Two backend domain types still allowed illegal states, against `CLAUDE.md`'s non-negotiable #3.
|
|
|
|
`RegistrationStatus` was a flat record whose **own doc-comment** admitted only `Geregistreerd`
|
|
should carry a herregistratie deadline — and noted the frontend modelled it correctly as a
|
|
discriminated union while the backend did not. It also made `reden` nullable on all three
|
|
variants where the FE requires it on two.
|
|
|
|
`Aanvraag` was a mutable EF class with 14 public setters. Its `StatusAt` carried **five
|
|
`Referentie!` null-forgiving derefs** plus a `SubmittedAt!.Value` — the compiler saying out loud
|
|
that "Submitted ⇒ Referentie != null" was convention, not type. WP-68 left it mutable
|
|
deliberately; WP-70/71 bought most of the safety with a test-only builder, which was itself a
|
|
hand-rolled prototype of the union this WP builds for real.
|
|
|
|
## Decisions (pre-made)
|
|
|
|
1. **Full union, not private setters.** The cheaper option (flip 14 setters to `private set`,
|
|
3 files, no migration) was rejected in favour of the honest modelling.
|
|
2. `RegistrationStatus` → abstract record + three sealed variants behind a private base ctor.
|
|
Chosen over WP-68's static-factory shape (`AanvraagStatus`) because with only 4 read sites the
|
|
abstract record is affordable and makes **reading** safe too, not just construction.
|
|
3. `Aanvraag` → `Concept | Submitted | Decided` (with `Decided` further split into
|
|
`Goedgekeurd | Afgewezen | MeerInfoGevraagd`), the EF row demoted to `AanvraagEntity` behind
|
|
a two-way mapper.
|
|
4. The `(Owner, Type)` "at most one unsubmitted aanvraag" rule is an **aggregate-set** invariant —
|
|
it cannot live on the entity and stays procedural in `CreateConcept` under the lock. Stated in
|
|
code so nobody tries to move it.
|
|
5. No migration, no schema change, no wire change.
|
|
|
|
## Acceptance criteria
|
|
|
|
- [x] **Illegal construction is a compile error, proven not assumed.** Each was attempted, the
|
|
compiler error recorded, then reverted:
|
|
|
|
| Attempted illegal state | Compiler error |
|
|
| -------------------------------------- | ------------------------------------------------------------------------------------------------- |
|
|
| `Decided` with no referentie | `CS9035: Required member 'Aanvraag.Decided.Referentie' must be set` |
|
|
| `Geschorst` with a HerregistratieDatum | `CS1739: The best overload for 'Geschorst' does not have a parameter named 'HerregistratieDatum'` |
|
|
| `Afwijzen` with no toelichting | `CS9035: Required member 'Aanvraag.Decided.Afgewezen.Toelichting' must be set` |
|
|
|
|
- [x] All five `Referentie!` derefs and the `SubmittedAt!.Value` are **gone**, not suppressed.
|
|
`IZaakSource.CreateZaak` narrows to `Aanvraag.Submitted`, removing the same class of deref
|
|
in both `LocalZaakSource` and `OpenZaakZaakSource`.
|
|
- [x] `reden` is now required on `Geschorst`/`Doorgehaald`, matching the FE union.
|
|
- [x] `HerregistratieRule.IsStatusConsistent` deleted as dead code — the type now guarantees what
|
|
it checked, and its test **could no longer construct the illegal state it existed to
|
|
catch**. That failure to compile is the proof the refactor worked.
|
|
- [x] Backend 242 → 241, exactly that one deleted test. No other count change.
|
|
- [x] `RegistrationStatusDto` and the application DTOs byte-identical — confirmed by diffing a
|
|
live backend's `/swagger/v1/swagger.json` against the checked-in copy. No `gen:api`.
|
|
|
|
## The `Draft` decision (made explicitly)
|
|
|
|
`ApplicationStore`'s doc-comment claimed `Draft` was "Concept only" (`Draft != null ⇒ !Submitted`),
|
|
but `Submit` never cleared it — so the invariant was **violated in production**. Resolved in
|
|
favour of the code matching the comment: `Submitted`/`Decided` simply have no `Draft` property,
|
|
so submitting drops it. Verified nothing reads a submitted aanvraag's draft — `draft-sync.ts`'s
|
|
`applyResume` is the only consumer of `ApplicationDetailDto.Draft` and only ever resumes an
|
|
unsubmitted wizard.
|
|
|
|
## Deviations
|
|
|
|
- **`Aanvraag` (EF row) renamed to `AanvraagEntity`.** The domain union needed the bare name to
|
|
match `RegistrationStatus`/`AanvraagStatus` conventions; keeping both would make every file
|
|
importing both namespaces ambiguous (`CS0104`). The table name is unaffected — EF derives it
|
|
from the `Applications` `DbSet` property, not the CLR type.
|
|
- **Step invariant loosened** from `0 <= StepIndex < StepCount` to `<=`: `CreateConcept` produces
|
|
`(0, 0)` before the wizard's first draft sync, which the strict form would reject at creation.
|
|
- `AanvraagBuilder.Decided(...)` now delegates to the real union constructors, dropping its own
|
|
hand-rolled toelichting guard; a one-line wrapper keeps the `.Decided(...).Build()` chain
|
|
source-compatible for existing call sites.
|
|
|
|
## Verification
|
|
|
|
```bash
|
|
cd backend && dotnet format --verify-no-changes && dotnet test BigRegister.slnx --filter "Category!=Integration"
|
|
npm run ci
|
|
```
|
|
|
|
## Follow-ups
|
|
|
|
- Making `Besluit` flow through the generated client as an enum rather than a `string` would
|
|
remove that FE/BE seam entirely rather than guarding it (WP-75 added the guard) — but it is a
|
|
wire change.
|