diff --git a/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts b/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts index f513e33..f603a84 100644 --- a/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts +++ b/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts @@ -122,7 +122,7 @@ export function submit(s: WizardState): WizardState { } /** The primary button's action: advance, or submit from the last step. No-op - outside Editing — this is the one decision `onPrimary()` used to make. */ + outside Editing — this is the one decision the old component-side handler used to make. */ export function primary(s: WizardState): WizardState { if (s.tag !== 'Editing') return s; return s.step === 3 ? submit(s) : next(s); diff --git a/apps/ssp/src/app/herregistratie/domain/intake.machine.ts b/apps/ssp/src/app/herregistratie/domain/intake.machine.ts index 354e744..883f661 100644 --- a/apps/ssp/src/app/herregistratie/domain/intake.machine.ts +++ b/apps/ssp/src/app/herregistratie/domain/intake.machine.ts @@ -201,7 +201,7 @@ export function submit(s: IntakeState): IntakeState { } /** The primary button's action: advance, or submit from the review step. No-op - outside Answering — this is the one decision `onPrimary()` used to make. */ + outside Answering — this is the one decision the old component-side handler used to make. */ export function primary(s: IntakeState): IntakeState { if (s.tag !== 'Answering') return s; return currentStep(s) === 'review' ? submit(s) : next(s); diff --git a/apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts b/apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts index 4aaf099..f93de66 100644 --- a/apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts +++ b/apps/ssp/src/app/herregistratie/ui/herregistratie-wizard/herregistratie-wizard.component.ts @@ -1,4 +1,3 @@ -/* eslint-disable max-lines */ // single-step wizard shell — removed by RD-20 import { Component, computed, inject, input } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { FormFieldComponent } from '@shared/ui/form-field/form-field.component'; @@ -56,10 +55,10 @@ import { UploadState, initialUpload, deliveryRefs } from '@shared/domain/upload. [canGoBack]="step() > 1" [errors]="errorList()" [errorMessage]="errorMessage()" - (primary)="onPrimary()" + (primary)="dispatch({ tag: 'Primary' })" (back)="dispatch({ tag: 'Back' })" (cancel)="restart()" - (retry)="onRetry()" + (retry)="dispatch({ tag: 'Retry' })" (goToStep)="goToStep($event)" > @switch (step()) { @@ -149,7 +148,21 @@ import { UploadState, initialUpload, deliveryRefs } from '@shared/domain/upload. }) export class HerregistratieWizardComponent { private profile = inject(BigProfileStore); - private store = createStore(initial, reduce); + // Effect fires once, on Editing -> Submitting (RD-05's tag-transition rule; `Seed` is + // exempt, so a story mounting straight into `Submitting` does not call the network). + private store = createStore(initial, reduce, { + Submitting: async (s, store) => { + this.profile.beginHerregistratie(); + const r = await this.draftSync.submit({ uren: s.data.uren, documents: s.data.documents }); + if (r.ok) { + store.dispatch({ tag: 'SubmitConfirmed' }); + this.profile.confirmHerregistratie(); + } else { + store.dispatch({ tag: 'SubmitFailed', error: r.error }); + this.profile.rollbackHerregistratie(); + } + }, + }); /** Preview/download link for a completed upload; delegates to the upload controller (application layer), which knows the dev-simulation `demo-*` ids @@ -253,37 +266,9 @@ export class HerregistratieWizardComponent { ); } - onPrimary() { - const s = this.state(); - if (s.tag !== 'Editing') return; - this.dispatch(s.step < 3 ? { tag: 'Next' } : { tag: 'Submit' }); - this.runIfSubmitting(); - } - - onRetry() { - this.dispatch({ tag: 'Retry' }); - this.runIfSubmitting(); - } - /** Reset the wizard to a fresh, empty start. */ restart() { this.draftSync.reset(); this.dispatch({ tag: 'Seed', state: initial }); } - - /** The effect: when we entered Submitting, submit through the aanvraag lifecycle, - flip the optimistic cross-page flag, then dispatch the result (commit/rollback). */ - private async runIfSubmitting() { - const s = this.state(); - if (s.tag !== 'Submitting') return; - this.profile.beginHerregistratie(); - const r = await this.draftSync.submit({ uren: s.data.uren, documents: s.data.documents }); - if (r.ok) { - this.dispatch({ tag: 'SubmitConfirmed' }); - this.profile.confirmHerregistratie(); - } else { - this.dispatch({ tag: 'SubmitFailed', error: r.error }); - this.profile.rollbackHerregistratie(); - } - } } diff --git a/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts b/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts index 81f8414..72dc03c 100644 --- a/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts +++ b/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.ts @@ -64,10 +64,10 @@ import { IntakePolicyStore } from '@herregistratie/application/intake-policy.sto [canGoBack]="cursor() > 0" [errors]="errorList()" [errorMessage]="errorMessage()" - (primary)="onPrimary()" + (primary)="dispatch({ tag: 'Primary' })" (back)="dispatch({ tag: 'Back' })" (cancel)="restart()" - (retry)="onRetry()" + (retry)="dispatch({ tag: 'Retry' })" (goToStep)="dispatch({ tag: 'GaNaarStap', cursor: $event })" > @switch (step()) { @@ -269,7 +269,28 @@ export class IntakeWizardComponent { // Server-owned policy (scholing threshold): fetched from the backend via the // application facade, not hardcoded. The backend stays the authority on submit. private policyStore = inject(IntakePolicyStore); - private store = createStore(initial, reduce); + // Effect fires once, on Answering -> Submitting (RD-05's tag-transition rule; `Seed` is + // exempt, so a story mounting straight into `Submitting` does not call the network). + private store = createStore(initial, reduce, { + Submitting: async (s, store) => { + this.profile.beginHerregistratie(); + // WP-69: the scholing answer rides along so the server can re-validate it as the + // authority (IntakePolicy.RejectIncompleteScholing) — undefined members are dropped by + // JSON.stringify, so a wizard above the threshold sends neither field. + const r = await this.draftSync.submit({ + uren: s.data.uren, + aanvullendeScholing: s.data.aanvullendeScholing, + scholingPunten: s.data.punten, + }); + if (r.ok) { + store.dispatch({ tag: 'SubmitConfirmed' }); + this.profile.confirmHerregistratie(); + } else { + store.dispatch({ tag: 'SubmitFailed', error: r.error }); + this.profile.rollbackHerregistratie(); + } + }, + }); /** Optional seed so Storybook / the showcase can mount any state directly. */ seed = input(initial); @@ -365,43 +386,8 @@ export class IntakeWizardComponent { }); } - onPrimary() { - const s = this.state(); - if (s.tag !== 'Answering') return; - this.dispatch(this.step() === 'review' ? { tag: 'Submit' } : { tag: 'Next' }); - this.runIfSubmitting(); - } - - onRetry() { - this.dispatch({ tag: 'Retry' }); - this.runIfSubmitting(); - } - restart() { this.draftSync.reset(); this.dispatch({ tag: 'Seed', state: initial }); } - - /** The effect: when we enter Submitting, submit through the aanvraag lifecycle, - flip the optimistic cross-page flag, then dispatch the outcome (commit/rollback). */ - private async runIfSubmitting() { - const s = this.state(); - if (s.tag !== 'Submitting') return; - this.profile.beginHerregistratie(); - // WP-69: the scholing answer rides along so the server can re-validate it as the - // authority (IntakePolicy.RejectIncompleteScholing) — undefined members are dropped by - // JSON.stringify, so a wizard above the threshold sends neither field. - const r = await this.draftSync.submit({ - uren: s.data.uren, - aanvullendeScholing: s.data.aanvullendeScholing, - scholingPunten: s.data.punten, - }); - if (r.ok) { - this.dispatch({ tag: 'SubmitConfirmed' }); - this.profile.confirmHerregistratie(); - } else { - this.dispatch({ tag: 'SubmitFailed', error: r.error }); - this.profile.rollbackHerregistratie(); - } - } } diff --git a/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts b/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts index 1158420..d5fd063 100644 --- a/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts +++ b/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts @@ -288,7 +288,7 @@ export function submit(s: RegistratieState): RegistratieState { } /** The primary button's action: advance, or submit from the controle step. - No-op outside Invullen — this is the one decision `onPrimary()` used to make. */ + No-op outside Invullen — this is the one decision the old component-side handler used to make. */ export function primary(s: RegistratieState): RegistratieState { if (s.tag !== 'Invullen') return s; return currentStep(s) === 'controle' ? submit(s) : next(s); diff --git a/apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts b/apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts index 26cc230..8333e83 100644 --- a/apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts +++ b/apps/ssp/src/app/registratie/ui/registratie-wizard/registratie-wizard.component.ts @@ -90,10 +90,10 @@ const NL_TAALVAARDIGHEID_VRAAG = 'nl-taalvaardigheid'; [errorMessage]="errorMessage()" i18n-submittingLabel="@@regWizard.submitting" submittingLabel="Uw registratie wordt verwerkt…" - (primary)="onPrimary()" + (primary)="dispatch({ tag: 'Primary' })" (back)="dispatch({ tag: 'Back' })" (cancel)="restart()" - (retry)="onRetry()" + (retry)="dispatch({ tag: 'Retry' })" (goToStep)="dispatch({ tag: 'GaNaarStap', cursor: $event })" > @switch (step()) { @@ -368,7 +368,18 @@ const NL_TAALVAARDIGHEID_VRAAG = 'nl-taalvaardigheid'; }) export class RegistratieWizardComponent { private lookup = inject(RegistratieLookupStore); - private store = createStore(initial, reduce); + // Effect fires once, on Invullen -> Indienen (RD-05's tag-transition rule; `Seed` is + // exempt, so a story mounting straight into `Indienen` does not call the network). + private store = createStore(initial, reduce, { + Indienen: async (s, store) => { + const r = await this.draftSync.submit({ + diplomaHerkomst: s.data.diplomaHerkomst, + documents: s.data.documents, + }); + if (r.ok) store.dispatch({ tag: 'SubmitConfirmed', referentie: r.value.referentie ?? '' }); + else store.dispatch({ tag: 'SubmitFailed', error: r.error }); + }, + }); /** Preview/download link for a completed upload; delegates to the upload controller (application layer), which knows the dev-simulation `demo-*` ids @@ -610,18 +621,6 @@ export class RegistratieWizardComponent { // failed submit) now lives in the shared WizardShellComponent. } - onPrimary() { - const s = this.state(); - if (s.tag !== 'Invullen') return; - this.dispatch(this.step() === 'controle' ? { tag: 'Submit' } : { tag: 'Next' }); - this.runIfIndienen(); - } - - onRetry() { - this.dispatch({ tag: 'Retry' }); - this.runIfIndienen(); - } - /** Reset the wizard to a fresh start. Reload the BRP lookup so the address re-prefills, keeping the form and the"vooraf ingevuld" note consistent. */ restart() { @@ -629,17 +628,4 @@ export class RegistratieWizardComponent { this.dispatch({ tag: 'Seed', state: initial }); this.lookup.reloadAdres(); } - - /** The effect: when we enter Indienen, submit through the aanvraag lifecycle - (duo → auto-approve, handmatig → manual), then dispatch the outcome. */ - private async runIfIndienen() { - const s = this.state(); - if (s.tag !== 'Indienen') return; - const r = await this.draftSync.submit({ - diplomaHerkomst: s.data.diplomaHerkomst, - documents: s.data.documents, - }); - if (r.ok) this.dispatch({ tag: 'SubmitConfirmed', referentie: r.value.referentie ?? '' }); - else this.dispatch({ tag: 'SubmitFailed', error: r.error }); - } } diff --git a/docs/project/readable-codebase/RD-08-wizards-to-effect-map.md b/docs/project/readable-codebase/RD-08-wizards-to-effect-map.md new file mode 100644 index 0000000..ad0c299 --- /dev/null +++ b/docs/project/readable-codebase/RD-08-wizards-to-effect-map.md @@ -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. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index f2ea454..e5f0dca 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -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. diff --git a/libs/shared/src/application/store.ts b/libs/shared/src/application/store.ts index 7ea33f0..e080e7f 100644 --- a/libs/shared/src/application/store.ts +++ b/libs/shared/src/application/store.ts @@ -26,7 +26,7 @@ export interface Store { * An effect body must never throw: end it in a `Result` from * `runSubmit`/`runResult` (`submit.ts`) and let the failure travel as a * dispatched message, not an exception. `dispatch` floats the effect's - * promise, exactly as the `runIfSubmitting()` call it replaces did. + * promise, exactly as the hand-written effect method it replaces did. */ export type StoreEffects = Model extends { tag: string } ? {