From f3e5745145706d9e3b4528f8331466da2d6aa8bf Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Mon, 7 Sep 2026 11:53:03 +0200 Subject: [PATCH] fix: the wizards' seed input never arrived (RD-39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three wizard containers read `this.seed()` in the constructor. Angular binds component inputs after the constructor runs, so the value was always the `initial` default, `seeded !== initial` was always false, and every mount took the `draftSync.resume()` branch. The `seed` input was dead code. The two single-step forms built on the same idiom read the input inside the microtask and work correctly. That contrast is the diagnosis. Impact: 21 seeded wizard stories rendered step 1 instead of the state they asked for. Storybook is this repo's UI test surface, so the states with no other coverage were exactly the ones not rendering — Submitting, Submitted, Failed, Ingediend, Mislukt. The a11y runner checks that whatever rendered is accessible, never that the right thing rendered, so nothing caught it. Production was unaffected: no route binds `seed`. Read the input inside the microtask, matching the two forms. Turn the spec's old `componentInstance.dispatch(...)` workaround into a real regression test through `componentRef.setInput('seed', ...)`. Verified: with the intake fix reverted the two spec cases fail; with it, 319 pass. `npm run ci --full` is green, and the newly rendered markup produced no axe violations. A browser check of seven seeded stories across all three wizards asserts text only reachable from a seed. Co-Authored-By: Claude Opus 5 --- .../herregistratie-wizard.component.ts | 11 ++- .../intake-wizard.component.spec.ts | 30 +++++-- .../intake-wizard/intake-wizard.component.ts | 11 ++- .../registratie-wizard.component.ts | 11 ++- .../RD-39-seed-input-timing.md | 82 +++++++++++++++++++ docs/project/readable-codebase/README.md | 1 + libs/shared/docs/behaviour-spec.mdx | 3 +- 7 files changed, 129 insertions(+), 20 deletions(-) create mode 100644 docs/project/readable-codebase/RD-39-seed-input-timing.md 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 254da49..1e99ba8 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 @@ -263,10 +263,13 @@ export class HerregistratieWizardComponent { constructor() { // An explicit seed (stories/tests) wins; otherwise resume the backend draft // (`?aanvraag=`) or start fresh. Persistence is the draftSync controller's job. - const seeded = this.seed(); - queueMicrotask(() => - seeded !== initial ? this.dispatch({ tag: 'Seed', state: seeded }) : this.draftSync.resume(), - ); + // Read `seed()` INSIDE the microtask: Angular binds inputs after the constructor + // runs, so an eager read here always returns the `initial` default. + queueMicrotask(() => { + const seeded = this.seed(); + if (seeded !== initial) this.dispatch({ tag: 'Seed', state: seeded }); + else void this.draftSync.resume(); + }); } /** Reset the wizard to a fresh, empty start. */ diff --git a/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.spec.ts b/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.spec.ts index 69b0804..fe153e9 100644 --- a/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.spec.ts +++ b/apps/ssp/src/app/herregistratie/ui/intake-wizard/intake-wizard.component.spec.ts @@ -26,14 +26,30 @@ const buitenlandJa: IntakeState = { scholingThreshold: 1000, }; +/** Mount with a seed and let the constructor's microtask apply it. */ +async function mountSeeded(state: IntakeState) { + TestBed.configureTestingModule({ + providers: [provideHttpClient(), provideApiClient()], + }); + const fixture = TestBed.createComponent(IntakeWizardComponent); + fixture.componentRef.setInput('seed', state); + await Promise.resolve(); // the seed is applied in a queueMicrotask + fixture.detectChanges(); + return fixture; +} + describe('IntakeWizardComponent', () => { - it('renders each field group as its own grey
', () => { - TestBed.configureTestingModule({ - providers: [provideHttpClient(), provideApiClient()], - }); - const fixture = TestBed.createComponent(IntakeWizardComponent); - fixture.componentInstance.dispatch({ tag: 'Seed', state: buitenlandJa }); - fixture.detectChanges(); + // Regression: the constructor must read `seed()` INSIDE its microtask. Angular binds + // inputs after the constructor runs, so an eager read silently yields the `initial` + // default and every seeded story renders step 1 instead of the state it asked for. + it('honours the seed input', async () => { + const fixture = await mountSeeded(buitenlandJa); + + expect(fixture.componentInstance.state()).toEqual(buitenlandJa); + }); + + it('renders each field group as its own grey
', async () => { + const fixture = await mountSeeded(buitenlandJa); const fieldsets: HTMLElement[] = Array.from( fixture.nativeElement.querySelectorAll('form.form-horizontal fieldset'), 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 bfe20b9..56cbab7 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 @@ -212,10 +212,13 @@ export class IntakeWizardComponent { constructor() { // An explicit seed (stories/tests) wins; otherwise resume the backend draft // (`?aanvraag=`) or start fresh. Persistence is the draftSync controller's job. - const seeded = this.seed(); - queueMicrotask(() => - seeded !== initial ? this.dispatch({ tag: 'Seed', state: seeded }) : this.draftSync.resume(), - ); + // Read `seed()` INSIDE the microtask: Angular binds inputs after the constructor + // runs, so an eager read here always returns the `initial` default. + queueMicrotask(() => { + const seeded = this.seed(); + if (seeded !== initial) this.dispatch({ tag: 'Seed', state: seeded }); + else void this.draftSync.resume(); + }); // Apply the server-owned threshold into machine state as it arrives. Track // only the policy value; untrack the dispatch (it reads the state signal // internally, which would otherwise make this effect loop on its own write). 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 4f8241d..eb2a740 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 @@ -233,10 +233,13 @@ export class RegistratieWizardComponent { constructor() { // An explicit seed (stories/tests) wins; otherwise resume from the backend draft // (`?aanvraag=`), or start fresh. Persistence is the draftSync controller's job. - const seeded = this.seed(); - queueMicrotask(() => - seeded !== initial ? this.dispatch({ tag: 'Seed', state: seeded }) : this.draftSync.resume(), - ); + // Read `seed()` INSIDE the microtask: Angular binds inputs after the constructor + // runs, so an eager read here always returns the `initial` default. + queueMicrotask(() => { + const seeded = this.seed(); + if (seeded !== initial) this.dispatch({ tag: 'Seed', state: seeded }); + else void this.draftSync.resume(); + }); // Prefill the address from the BRP lookup as it arrives. Track only the facade's // parsed prefill signal; untrack the dispatch (it reads the state signal, which // would otherwise make this effect loop on its own write). Don't clobber diff --git a/docs/project/readable-codebase/RD-39-seed-input-timing.md b/docs/project/readable-codebase/RD-39-seed-input-timing.md new file mode 100644 index 0000000..0f57bca --- /dev/null +++ b/docs/project/readable-codebase/RD-39-seed-input-timing.md @@ -0,0 +1,82 @@ +# RD-39 — The wizards' `seed` input never arrived + +Status: done +Source: found while planning the machine-wiring migration (ADR-0007 arc) + +## Why + +All three wizard containers read their `seed` input **in the constructor**: + +```ts +const seeded = this.seed(); // always the `initial` default +queueMicrotask(() => + seeded !== initial ? this.dispatch({ tag: 'Seed', state: seeded }) : this.draftSync.resume(), +); +``` + +Angular binds component inputs **after** the constructor runs. So `seeded` was +always the `initial` default, `seeded !== initial` was always false, and every +mount took the `draftSync.resume()` branch. The `seed` input was dead code. + +The two single-step forms built on the same idiom prove the diagnosis by +contrast. They read the input **inside** the microtask, and they work: + +``` +change-request-form.component.ts:191 queueMicrotask(() => this.dispatch({ tag: 'Seed', state: this.seed() })); +besluit-form.component.ts:176 queueMicrotask(() => this.dispatch({ tag: 'Seed', state: this.seed() })); +``` + +## Impact + +Every seeded wizard story rendered step 1 instead of the state it asked for — +21 stories across the three wizards. Storybook is this repo's UI test surface +(CLAUDE.md "Testing"), so the states that had no other coverage were exactly +the ones silently not rendering: `Submitting`, `Submitted`, `Failed`, +`Ingediend`, `Mislukt`. + +Nothing caught it. `.storybook-ssp/test-runner.ts` runs axe only — it checks +that whatever rendered is accessible, never that the right thing rendered. +`intake-wizard.component.spec.ts` had worked around it by calling +`componentInstance.dispatch(...)` instead of setting the input, which is the +shape of a test written against a broken input path. + +**Production was unaffected**: no route binds `seed`, so the resume branch was +always the correct one there. + +## Decisions + +1. **Read `seed()` inside the microtask**, matching the two forms. Six lines + across three files. The `if/else` replaces the ternary because the branches + are statements, not values. +2. **Keep the microtask.** It is what defers the dispatch past input binding. + The larger fix — `start(seed)` on an application store called from + `ngOnInit` — belongs to the ADR-0007 migration, not here. This ticket makes + the existing seam correct; it does not move it. +3. **Turn the workaround into the regression test.** + `intake-wizard.component.spec.ts` now mounts through + `componentRef.setInput('seed', …)` via a `mountSeeded` helper, and a new + `honours the seed input` case asserts the machine state directly. Both fail + on the old code, which is the point. +4. **Do not touch `draftSync.enabled`.** `enabled: () => this.seed() === +initial` reads the input lazily inside a lambda called from an effect, so it + was already correct. Changing the input to `| null` is part of ADR-0007. + +## Verification performed + +- `npx ng test ssp` with the intake fix reverted: **2 failed** (both new spec + cases). With the fix: **319 passed**. That differential is the proof. +- `npm run ci --full` green, including 112 storybook a11y tests. The newly + rendered markup (``, error alerts, ``) + produced **no** axe violations — the predicted a11y fallout did not happen. +- Browser check against the built Storybook, seven seeded stories across all + three wizards, each asserted to contain text only reachable from its seed + (`Netwerkfout` for `Failed`, `referentienummer` for `Ingediend`, `Documenten` + for step 3), plus a negative control that step 1 does **not** show step 3's + title. All seven pass. + +## Trap for the next person + +`Failed`, `Submitted`, `Ingediend` and `Mislukt` are unreachable without a +network submit, so they can only be produced by a seed. If a future change +breaks the input path again, those four stories silently fall back to step 1 +and axe still passes. The spec added here is the guard; keep it. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index 773b4fd..de98e1b 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -133,6 +133,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-36 | `ui/dashboard/` → `ui/overzicht-secties/` + 2 stale `dashboard.page` paths | 04 | yes | todo | | RD-37 | **a11y:** 5 suppressions name a closed ticket — decide the `li[…]` host | 01 | yes | todo | | RD-38 | One member order for the 3 wizard containers + 2 pure extractions | 22, 23 | | done | +| RD-39 | **Bug:** the wizards' `seed` input never arrived (21 stories) | 38 | yes | done | The ID order already respects every dependency, so it is the recommended running order. diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 632bb84..be0c6e2 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -20,7 +20,7 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page -**is** the suite, reshaped for a business reader. 562 frontend behaviours across +**is** the suite, reshaped for a business reader. 563 frontend behaviours across 9 contexts; 261 backend behaviours across 42 test classes. @@ -383,6 +383,7 @@ classes. #### IntakeWizardComponent +- honours the seed input - renders each field group as its own grey <fieldset> #### STEPS (fixed) and inline questions