ehoandClaude Sonnet 5
ae7781efef
CI / changes (push) Successful in 8s
CI / lint (push) Successful in 1m9s
CI / frontend (push) Successful in 2m27s
CI / backend (push) Successful in 1m56s
CI / e2e (push) Successful in 3m16s
CI / semgrep (push) Successful in 1m7s
CI / api-client-drift (push) Successful in 1m50s
CI / storybook-a11y (push) Successful in 11m4s
Four close-outs and their README rows. The behaviour spec is regenerated once here rather than per-track — it derives from every test name in the repo, so any track running it would have conflicted with the other three. Records two findings the arc surfaced but did not cause: the /brief/preview staleness for non-DemoOwner identities (blocking per-spec identity isolation in brief-v2.spec.ts), and that WP-72/73 had to share a commit because both edit Program.cs — separate execution waves prevented build collisions but did not produce separable diffs. Co-Authored-By: Claude Sonnet 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.
|