refactor: migrate the 3 wizards to the effect map and Primary (RD-08)
The three wizards paired a dispatch with a hand-written effect call (onPrimary/onRetry + runIfSubmitting/runIfIndienen). A missed call failed silently. RD-05 added the effect map and RD-07 added the Primary message; this ticket moves each wizard onto both. Each wizard now registers its submit effect on createStore, keyed by its own submitting tag (Submitting for herregistratie and intake, Indienen for registratie — the type catches a wrong key at compile time). The optimistic begin/confirm/rollback calls stay inside the effect body, unchanged. The template dispatches Primary and Retry directly, matching how Back already worked. onPrimary, onRetry, and runIfSubmitting/runIfIndienen are deleted from all three components. herregistratie-wizard drops under the 250-rule-line budget, so its eslint-disable max-lines header is removed in this same commit (RD-02's self-cleaning mechanism). intake-wizard and registratie-wizard stay over budget and keep theirs, both already flagged for RD-22/RD-23. Three doc comments (in the three machine files, plus one in store.ts) named the deleted onPrimary()/runIfSubmitting() identifiers in prose. Reworded them so the "idiom is gone from the repo" grep check is not defeated by its own explanatory comments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Claude Sonnet 5
@@ -0,0 +1,165 @@
|
||||
# RD-08 — Migrate the 3 wizards to the effect map and `Primary`
|
||||
|
||||
Status: done
|
||||
Source: PLAN.md 1a
|
||||
|
||||
## Why
|
||||
|
||||
This is the ticket that removes the silent-failure idiom from the last three call sites. Each
|
||||
wizard still pairs a `dispatch` with a hand-written effect call, and forgetting the second
|
||||
line fails silently:
|
||||
|
||||
```ts
|
||||
onPrimary() { … this.dispatch(…); } // decides Next vs Submit in the UI
|
||||
onRetry() { this.dispatch({ tag: 'Retry' }); this.runIfSubmitting(); }
|
||||
```
|
||||
|
||||
RD-05 added the effect map; RD-07 added `Primary`. After this ticket the wizard shell's five
|
||||
outputs all map 1:1 onto messages in the template, and each component loses three methods.
|
||||
|
||||
## Read first
|
||||
|
||||
- `libs/shared/src/application/store.ts` — the effect map, its trigger rule, and the `Seed`
|
||||
exemption
|
||||
- The three machines' new `primary` exports (RD-07)
|
||||
- `apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts`
|
||||
— `onPrimary` 256, `onRetry` 263, `runIfSubmitting` 276-288
|
||||
- `apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts` — `onPrimary`
|
||||
368, `onRetry` 375, `runIfSubmitting` 387-406
|
||||
- `apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts` —
|
||||
`onPrimary` 613, `onRetry` 620, `runIfIndienen` 635-644
|
||||
- `docs/project/readable-codebase/RD-06-…md` — the same migration, already done for the two
|
||||
single-step forms. Copy its shape.
|
||||
|
||||
## Decisions (pre-made, don't relitigate)
|
||||
|
||||
1. **The effect-map key is the machine's own submitting tag, and it is not the same for all
|
||||
three:**
|
||||
|
||||
| Wizard | Effect-map key |
|
||||
| ----------------------- | -------------- |
|
||||
| `herregistratie-wizard` | `Submitting` |
|
||||
| `intake-wizard` | `Submitting` |
|
||||
| `registratie-wizard` | **`Indienen`** |
|
||||
|
||||
`StoreEffects` keys are typed as `Model['tag']`, so writing `Submitting` for the registratie
|
||||
wizard is a **compile error**, not a silent no-op. That is the type doing its job — do not
|
||||
work around it by widening the type.
|
||||
|
||||
2. **Move each effect body verbatim into the map.** The narrowed state arrives as argument one,
|
||||
so delete the `const s = this.state(); if (s.tag !== '…') return;` preamble and use the
|
||||
parameter. Keep everything else identical, including the optimistic store calls.
|
||||
|
||||
3. **The optimistic `begin`/`confirm`/`rollback` calls stay inside the effect body**, exactly
|
||||
where they are today (`herregistratie-wizard:279,283,286`; `intake-wizard:390,401,404`).
|
||||
The effect slot is the sanctioned place for side effects, so `reduce` stays pure. The
|
||||
registratie wizard has **no** optimistic calls — do not add any.
|
||||
|
||||
4. **Delete `onPrimary` and `onRetry` entirely** and dispatch from the template, matching how
|
||||
`(back)` already does it:
|
||||
|
||||
```text
|
||||
(primary)="dispatch({ tag: 'Primary' })"
|
||||
(retry)="dispatch({ tag: 'Retry' })"
|
||||
(back)="dispatch({ tag: 'Back' })" <-- already this shape today
|
||||
```
|
||||
|
||||
`Retry` needs no effect call because `Failed → Submitting`/`Indienen` is a tag transition,
|
||||
so the map fires it. This is the 1:1 output-to-message mapping
|
||||
`.claude/skills/form-machine/SKILL.md:74-78` already claims.
|
||||
|
||||
5. **Leave the two `dispatch` calls that sit inside Angular `effect()`s alone** —
|
||||
`intake-wizard`'s `SetPolicy` and `registratie-wizard`'s `PrefillAdres`. Both land on an
|
||||
unchanged tag, so no effect fires, and both are already `untracked`. Do not key an effect on
|
||||
an editing tag (`Editing`/`Answering`/`Invullen`) — that is the livelock
|
||||
`store.spec.ts:18-31` guards against.
|
||||
|
||||
6. **Preserve the WP-69 comment in `intake-wizard`'s effect body verbatim**, including its
|
||||
ticket reference. It explains why the scholing answer rides along for server-side
|
||||
re-validation. RD-19 strips ticket prefixes across the repo later; do not do it early and do
|
||||
not drop the explanation.
|
||||
|
||||
7. **If a wizard drops below 250 rule-lines, delete its `eslint-disable max-lines` header in
|
||||
this same commit.** See Risks — this is expected for `herregistratie-wizard` and is
|
||||
RD-02's mechanism working, not a problem to route around.
|
||||
|
||||
## Files
|
||||
|
||||
- `apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts`
|
||||
- `apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts`
|
||||
- `apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts`
|
||||
|
||||
No machine changes. No story changes expected. No xlf changes.
|
||||
|
||||
## Steps
|
||||
|
||||
1. For each wizard: register the effect body on `createStore` under the key from decision 1,
|
||||
dropping the guard preamble.
|
||||
2. Delete `runIfSubmitting` / `runIfIndienen`.
|
||||
3. Delete `onPrimary` and `onRetry`; dispatch `Primary` and `Retry` from the template
|
||||
(decision 4).
|
||||
4. Run `npm run lint`. If a file is now under budget, delete its `eslint-disable max-lines`
|
||||
header (decision 7).
|
||||
5. Update this ticket's `Status:` to `done` and the README's RD-08 row to `done`.
|
||||
6. Commit all of it together.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
The idiom is gone from the whole repo. Anchor on the **declaration**, not on any occurrence
|
||||
of the name:
|
||||
|
||||
```bash
|
||||
grep -rn "runIfSubmitting\|runIfIndienen" apps libs # MUST return nothing
|
||||
grep -rnE "^ (onPrimary|onRetry)\(\)" apps libs # MUST return nothing
|
||||
```
|
||||
|
||||
The second pattern is anchored deliberately. A bare `grep -rn "onPrimary\|onRetry"` **cannot
|
||||
pass**: `libs/shared/src/application/upload-controller.ts:94` has an unrelated
|
||||
`onRetry(localId)` method, called from two wizard templates as
|
||||
`uploadCtl.onRetry($event)`. Those three matches are correct code and must stay.
|
||||
|
||||
Behaviour is unchanged. Walk each wizard end to end with `npm start`:
|
||||
|
||||
1. The primary button advances through every step and submits on the last one.
|
||||
2. A failed submit shows the error, and Retry re-submits (it must leave `Failed`).
|
||||
3. Clicking the primary button twice quickly submits **once** — the trigger rule's
|
||||
tag-transition condition gives this for free.
|
||||
4. For the two herregistratie wizards, the dashboard's "herregistratie in behandeling" notice
|
||||
still appears after submit and disappears after a rollback.
|
||||
|
||||
```bash
|
||||
npm run ci # exits 0
|
||||
npm run ci --full # exits 0 — the wizards all have stories
|
||||
```
|
||||
|
||||
## Verification
|
||||
|
||||
`npm run ci --full`. `--full` is required: all three wizards have stories, and
|
||||
`registratie-wizard.stories.ts:88`, `intake-wizard.stories.ts:38` and
|
||||
`herregistratie-wizard.stories.ts:70` each seed a submitting state.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Splitting any wizard into step components. RD-22 and RD-23, after RD-20.
|
||||
- `WizardStatus` → `WizardPhase`. RD-10.
|
||||
- Stripping the WP-69 ticket reference (decision 6). RD-19.
|
||||
- Touching `shellStatus`, `errorList`, or any other computed. Only the three methods and the
|
||||
template bindings change here.
|
||||
|
||||
## Risks
|
||||
|
||||
- **`herregistratie-wizard` is 252 rule-lines and will likely drop below 250.** Deleting three
|
||||
methods (~21 lines) and adding a map registration (~12) nets roughly −9. Its
|
||||
`eslint-disable max-lines` then becomes unnecessary and
|
||||
`reportUnusedDisableDirectives: 'error'` **fails the build**. That is RD-02's self-cleaning
|
||||
mechanism doing its job: delete the header (decision 7). RD-20 also expects to remove it —
|
||||
whichever ticket gets there first removes it, and the other finds nothing to do.
|
||||
- **The Storybook trap.** All three wizards seed a submitting state in their stories with a
|
||||
real `provideHttpClient()` and no request mocking. RD-05's `Seed` exemption is what stops
|
||||
them firing real network calls. If a story flips to a failed state on load, or
|
||||
`storybook-a11y` goes red, the exemption is not working — fix that, do not delete the story.
|
||||
- **Double-submit protection is now structural, not incidental.** The old code could submit
|
||||
twice if `dispatch` and the effect call were paired twice. The tag-transition rule prevents
|
||||
it. Do not add a `busy` guard on top; that would be a second mechanism for one rule.
|
||||
- **`behaviour-spec.mdx` drift** if you add or rename a spec. Run `npm run gen:behaviour-spec`
|
||||
in the same commit if so.
|
||||
@@ -102,7 +102,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di
|
||||
| RD-05 | `createStore` gains the effect map + specs | 02 | | done |
|
||||
| RD-06 | **Bug fix:** 2 single-step forms to the effect map + retry affordance | 05 | yes | done |
|
||||
| RD-07 | Add `Primary` to the 3 wizard machines + specs | 05 | | done |
|
||||
| RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | todo |
|
||||
| RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | done |
|
||||
| RD-09 | **Docs + generator:** `form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | | todo |
|
||||
| RD-10 | `WizardStatus` to a payload-carrying `WizardPhase` | 08 | yes | todo |
|
||||
| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | todo |
|
||||
@@ -178,3 +178,16 @@ Three rules when you write a ticket file, because the agent reads its ticket and
|
||||
Risks section is a trap that fires.
|
||||
3. **State acceptance as a command, not a sentence.** "Lands about 230 lines" is a design
|
||||
estimate and nothing can check it. `npm run lint` has an exit code.
|
||||
4. **Run every acceptance command against the tree before you hand the ticket over.** A
|
||||
command that cannot pass is worse than no command: the agent either wastes a cycle or,
|
||||
worse, "fixes" correct code to satisfy it. Three real misses so far, all in tickets written
|
||||
by the supervisor:
|
||||
- RD-06 grepped only `runIfSubmitting`, missing that one wizard spells it `runIfIndienen`.
|
||||
- RD-08 grepped bare `onPrimary\|onRetry`, which can never return nothing — an unrelated
|
||||
`uploadCtl.onRetry` exists in `upload-controller.ts`.
|
||||
- RD-08 said "no machine changes" while also requiring a repo-wide grep to come back
|
||||
clean, which forced comment edits in three machines. The two instructions contradicted
|
||||
each other.
|
||||
|
||||
Anchor greps on a declaration (`^ onRetry\(\)`) rather than a name, and make the Files
|
||||
list agree with the Acceptance commands.
|
||||
|
||||
Reference in New Issue
Block a user