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 f2bf768..45c0284 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 @@ -9,6 +9,7 @@ import { WizardPhase, naarStapLabel, } from '@shared/layout/wizard-shell/wizard-shell.component'; +import { toWizardErrors } from '@shared/layout/wizard-shell/wizard-errors'; import { ConfirmationComponent } from '@shared/ui/confirmation/confirmation.component'; import { createStore } from '@shared/application/store'; import { whenTag } from '@shared/kernel/fp'; @@ -251,12 +252,7 @@ export class HerregistratieWizardComponent { } }); /** Current step's field errors, flattened for the shell's error summary. */ - protected errorList = computed(() => { - const e = this.editing()?.errors ?? {}; - return (Object.keys(e) as (keyof typeof e)[]) - .filter((k) => e[k]) - .map((k) => ({ id: k, message: e[k]! })); - }); + protected errorList = computed(() => toWizardErrors(this.editing()?.errors ?? {})); constructor() { // An explicit seed (stories/tests) wins; otherwise resume the backend draft 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 2ee3ff1..d6b87e9 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 @@ -15,6 +15,7 @@ import { WizardPhase, naarStapLabel, } from '@shared/layout/wizard-shell/wizard-shell.component'; +import { toWizardErrors } from '@shared/layout/wizard-shell/wizard-errors'; import { createStore } from '@shared/application/store'; import { whenTag } from '@shared/kernel/fp'; import { BigProfileStore } from '@registratie/application/big-profile.store'; @@ -360,12 +361,9 @@ export class IntakeWizardComponent { }); /** Current step's field errors, flattened for the shell's error summary. The field ids match the answer keys, so the summary anchors jump to the field. */ - protected errorList = computed(() => { - const e = this.answering()?.errors ?? {}; - return (Object.keys(e) as (keyof Answers)[]) - .filter((k) => e[k]) - .map((k) => ({ id: k, message: e[k]! })); - }); + protected errorList = computed(() => + toWizardErrors(this.answering()?.errors ?? {}), + ); protected err = (k: keyof Answers) => this.answering()?.errors[k] ?? ''; protected set = (key: keyof Answers, value: string) => 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 d5fd063..67abd47 100644 --- a/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts +++ b/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts @@ -66,7 +66,7 @@ export type DraftField = 'straat' | 'postcode' | 'woonplaats' | 'email'; /** Per-field error map. `antwoorden` holds per-policy-question errors, keyed by question id (a step can show several questions). */ -export interface Errors { +export type Errors = { straat?: string; postcode?: string; woonplaats?: string; @@ -75,7 +75,7 @@ export interface Errors { diploma?: string; documenten?: string; antwoorden?: Record; -} +}; export type RegistratieState = | { tag: 'Invullen'; draft: Draft; cursor: number; errors: Errors; upload: UploadState } 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 50a1d4c..21e75b4 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 @@ -17,6 +17,7 @@ import { WizardPhase, naarStapLabel, } from '@shared/layout/wizard-shell/wizard-shell.component'; +import { toWizardErrors } from '@shared/layout/wizard-shell/wizard-errors'; import { ASYNC } from '@shared/ui/async/async.component'; import { AddressFieldsComponent } from '@registratie/ui/address-fields/address-fields.component'; import { createStore } from '@shared/application/store'; @@ -468,14 +469,7 @@ export class RegistratieWizardComponent { /** Current step's errors (incl. per-question), flattened for the error summary. */ protected errorList = computed(() => { const e = this.invullen()?.errors ?? {}; - const out: WizardError[] = []; - for (const [k, v] of Object.entries(e)) { - if (k !== 'antwoorden' && typeof v === 'string' && v) out.push({ id: k, message: v }); - } - for (const [qid, msg] of Object.entries(e.antwoorden ?? {})) { - if (msg) out.push({ id: 'vraag-' + qid, message: msg }); - } - return out; + return [...toWizardErrors(e), ...toWizardErrors(e.antwoorden ?? {}, 'vraag-')]; }); protected adresSamenvatting = computed(() => { const d = this.draft(); diff --git a/docs/project/readable-codebase/RD-20-wizard-errors.md b/docs/project/readable-codebase/RD-20-wizard-errors.md new file mode 100644 index 0000000..35bc178 --- /dev/null +++ b/docs/project/readable-codebase/RD-20-wizard-errors.md @@ -0,0 +1,170 @@ +# RD-20 — `wizard-errors.ts` + spec, adopted by all three wizards + +Status: done +Source: PLAN.md 3c + +## Why + +Each of the three wizards flattens its per-field error record into the shell's +`WizardError[]` summary, and each writes the flattening itself. Two of the three are the same +six lines with a different key type. The third does the same job plus a nested group. + +The loop is pure, it has no spec, and it lives in a component — so the one part of the wizard +that decides what the user sees in the error summary is the one part no test covers. + +Extract one pure helper beside `naarStapLabel`, which lives in that folder for exactly this +reason. + +## Read first + +- `libs/shared/src/layout/wizard-shell/wizard-shell.component.ts:19-27` — `naarStapLabel` and + the `WizardError` interface. The new file sits beside this one. +- The three call sites, in the order they get easier: + - `herregistratie-wizard.component.ts:253-259` + - `intake-wizard.component.ts:361-368` + - `registratie-wizard.component.ts:468-479` — the one with the nested group. +- `apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts:69` — `Errors`, declared + as an `interface`. Decision 3 is about this line. + +## Decisions (pre-made, don't relitigate) + +1. **One helper, two parameters, both used today:** + + ```ts + /** Flatten a machine's per-field error record into the shell's summary list. + Values that are not a non-empty string are skipped, so a nested group + (the registratie wizard's `antwoorden`) is appended by the caller with its + own `idPrefix` rather than special-cased here. */ + export function toWizardErrors(errors: Record, idPrefix = ''): WizardError[] { + const out: WizardError[] = []; + for (const [k, v] of Object.entries(errors)) { + if (typeof v === 'string' && v) out.push({ id: idPrefix + k, message: v }); + } + return out; + } + ``` + + `Record` plus the `typeof` guard narrows `v` to `string` with **no cast**, + which keeps the file inside the repo's `any`-free rule. Do not reach for a generic key type; + the three machines key their errors differently and the helper does not care. + +2. **New file `libs/shared/src/layout/wizard-shell/wizard-errors.ts`, plus its spec.** It takes + the `WizardError` type from the component file with a **type-only import**: + + ```ts + import type { WizardError } from './wizard-shell.component'; + ``` + + `import type` is erased at compile time, so the helper stays pure and its spec needs no + TestBed and pulls in no Angular at run time. Moving the `WizardError` interface into the new + file would be tidier on paper and would touch four more files for no behaviour; not worth it. + +3. **`registratie-wizard.machine.ts:69` changes from `interface Errors` to `type Errors`.** + This is required, not cosmetic. TypeScript gives an implicit index signature to a type alias + but **not** to an interface, so `Errors` as an interface is not assignable to + `Record` and the call site will not compile. The other two machines already + declare their error maps as type aliases (`Partial>`), so this also makes + the three consistent. Nothing `extends` or `implements` `Errors` — verified. + +4. **The three call sites become:** + + | Wizard | Body | + | -------------- | ------------------------------------------------------------------------- | + | herregistratie | `toWizardErrors(this.editing()?.errors ?? {})` | + | intake | `toWizardErrors(this.answering()?.errors ?? {})` | + | registratie | `[...toWizardErrors(e), ...toWizardErrors(e.antwoorden ?? {}, 'vraag-')]` | + + The registratie site keeps its `const e = this.invullen()?.errors ?? {};` line. The + `vraag-` prefix and the skipping of non-string values together replace its + `if (k !== 'antwoorden' && typeof v === 'string' && v)` filter. + +5. **Do not touch the three `shellStatus` switches.** The tags genuinely differ per machine and + an exhaustive switch is the house style. This ticket is about the error list only. + +6. **No story.** The helper is a pure function. Its spec is the test; `wizard-shell.stories.ts` + already renders the error summary. + +## Files + +- `libs/shared/src/layout/wizard-shell/wizard-errors.ts` (new) +- `libs/shared/src/layout/wizard-shell/wizard-errors.spec.ts` (new) +- `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` +- `apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts` (decision 3, one line) +- `libs/shared/docs/behaviour-spec.mdx` (regenerated, never hand-edited) + +## Steps + +1. Write `wizard-errors.ts` per decisions 1 and 2. +2. Write `wizard-errors.spec.ts`. Cover: a flat record; an empty record; a record whose values + are `undefined` or `''` (both skipped); the `idPrefix`; and a record holding a nested object + value (skipped, which is what lets decision 4's registratie case work). +3. Apply decision 3 — one word, `interface` → `type`, and the `{` stays. +4. Convert the three call sites per decision 4. +5. Run `npm run gen:behaviour-spec` — the new spec titles otherwise fail the drift check. +6. `git add -A`, then run the acceptance commands. +7. Update this ticket's `Status:` to `done` and the README's RD-20 row to `done`. +8. Commit all of it together. + +## Acceptance criteria + +Measured against the tree before handover. + +```bash +git grep -c "export function toWizardErrors" -- libs/shared/src/layout/wizard-shell/wizard-errors.ts # MUST be 1 +git grep -l "toWizardErrors" -- apps libs | wc -l # is 0 -> MUST be 5 (helper, spec, 3 wizards) +``` + +The hand-rolled flattening is gone from all three: + +```bash +git grep -c "filter((k) => e\[k\])" -- apps | awk -F: '{s+=$NF} END {print s+0}' # is 2 -> MUST be 0 +git grep -c "k !== 'antwoorden'" -- apps | awk -F: '{s+=$NF} END {print s+0}' # is 1 -> MUST be 0 +``` + +Decision 3 landed: + +```bash +git grep -c "export interface Errors" -- apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts # is 1 -> MUST be 0 +git grep -c "export type Errors" -- apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts # MUST be 1 +``` + +```bash +npm run ci # exits 0 +``` + +## Verification + +`npm run ci` is enough. `--full` is **not** required: no story changes, and the new file is +under `libs/shared/src/layout/`, not `libs/shared/src/ui/`. Regenerating `behaviour-spec.mdx` +does not trigger `--full` on its own — RD-17 set that precedent. + +`npm run lint` inside the gate is what proves the three `/* eslint-disable max-lines */` +directives are still needed. `reportUnusedDisableDirectives` is `error`, so a directive that +stops being necessary fails the build. + +## Out of scope + +- Splitting any wizard into step components. RD-22 (intake) and RD-23 (registratie) own that, + and this ticket makes both smaller first. +- The `shellStatus` switches (decision 5). +- `change-request-form` and `besluit-form`. Neither builds a `WizardError[]`; they render field + errors directly. There is nothing to share. + +## Risks + +- **The `interface` → `type` change is load-bearing** (decision 3). Skipping it produces + "Index signature for type 'string' is missing in type 'Errors'" at the registratie call site, + and the tempting wrong fix is to widen the helper's parameter to `object`, which forces an + `any` and fails lint. +- **PLAN says this ticket deletes an `eslint-disable max-lines` from + `herregistratie-wizard.component.ts`. That is stale — there is no such directive.** Earlier + tickets already brought the file under the budget: it measures ~248 effective lines against a + limit of 250. The two disables that do exist (intake, registratie) stay; RD-22 and RD-23 + remove them. Both files are far above 250 (~362 and ~574 effective), so this ticket's saving + cannot make either directive unused. +- **`behaviour-spec.mdx` drift** from the new spec titles. Run `gen:behaviour-spec` in the same + commit, and never edit that file by hand. +- **Keep `import type`, not a value import** (decision 2). A value import of the component into + the helper drags Angular into a pure module and its spec. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index 1d2137f..ef07a59 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -114,7 +114,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-17 | `successOf`/`successOr` sweep — 10 sites, 8 files | 01 | | done | | RD-18 | Ticket-reference sweep, frontend — 181 refs, 100 files | 01 | yes | done | | RD-19 | Ticket-reference sweep, backend — 370 refs, 86 files | 01 | | done | -| RD-20 | `wizard-errors.ts` + spec, adopted by all 3 wizards | 02 | | todo | +| RD-20 | `wizard-errors.ts` + spec, adopted by all 3 wizards | 02 | | done | | RD-21 | `rich-text-dom.ts` helpers + spec cases | 02 | yes | todo | | RD-22 | `intake-wizard` to 3 step components | 08, 20 | yes | todo | | RD-23 | `registratie-wizard` to 3 steps + the upload-controller move | 08, 20 | yes | todo | diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index c733132..26cd4ee 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. 532 frontend behaviours across +**is** the suite, reshaped for a business reader. 538 frontend behaviours across 9 contexts; 261 backend behaviours across 42 test classes. @@ -973,6 +973,15 @@ classes. - unwraps a Success value - is the fallback for every other state +#### toWizardErrors + +- flattens a flat record of field errors +- returns an empty list for an empty record +- skips a value that is undefined +- skips a value that is an empty string +- prefixes every id with idPrefix when given +- skips a value that is a nested object, so the caller can flatten it separately + #### upload lifecycle messages - queued → progress → complete diff --git a/libs/shared/src/layout/wizard-shell/wizard-errors.spec.ts b/libs/shared/src/layout/wizard-shell/wizard-errors.spec.ts new file mode 100644 index 0000000..26aaad1 --- /dev/null +++ b/libs/shared/src/layout/wizard-shell/wizard-errors.spec.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from 'vitest'; +import { toWizardErrors } from './wizard-errors'; + +describe('toWizardErrors', () => { + it('flattens a flat record of field errors', () => { + expect(toWizardErrors({ straat: 'Verplicht', postcode: 'Ongeldig' })).toEqual([ + { id: 'straat', message: 'Verplicht' }, + { id: 'postcode', message: 'Ongeldig' }, + ]); + }); + + it('returns an empty list for an empty record', () => { + expect(toWizardErrors({})).toEqual([]); + }); + + it('skips a value that is undefined', () => { + expect(toWizardErrors({ straat: undefined })).toEqual([]); + }); + + it('skips a value that is an empty string', () => { + expect(toWizardErrors({ straat: '' })).toEqual([]); + }); + + it('prefixes every id with idPrefix when given', () => { + expect(toWizardErrors({ q1: 'Verplicht' }, 'vraag-')).toEqual([ + { id: 'vraag-q1', message: 'Verplicht' }, + ]); + }); + + it('skips a value that is a nested object, so the caller can flatten it separately', () => { + expect(toWizardErrors({ straat: 'Verplicht', antwoorden: { q1: 'Verplicht' } })).toEqual([ + { id: 'straat', message: 'Verplicht' }, + ]); + }); +}); diff --git a/libs/shared/src/layout/wizard-shell/wizard-errors.ts b/libs/shared/src/layout/wizard-shell/wizard-errors.ts new file mode 100644 index 0000000..09e307e --- /dev/null +++ b/libs/shared/src/layout/wizard-shell/wizard-errors.ts @@ -0,0 +1,13 @@ +import type { WizardError } from './wizard-shell.component'; + +/** Flatten a machine's per-field error record into the shell's summary list. + Values that are not a non-empty string are skipped, so a nested group + (the registratie wizard's `antwoorden`) is appended by the caller with its + own `idPrefix` rather than special-cased here. */ +export function toWizardErrors(errors: Record, idPrefix = ''): WizardError[] { + const out: WizardError[] = []; + for (const [k, v] of Object.entries(errors)) { + if (typeof v === 'string' && v) out.push({ id: idPrefix + k, message: v }); + } + return out; +}