ehoandClaude Sonnet 5
831940f1b9
Each wizard flattened its per-field error record into the shell's WizardError[] summary with its own copy of the same loop. Extract one pure helper, wizard-errors.ts, next to naarStapLabel. Add a spec that covers a flat record, an empty record, skipped undefined/empty-string values, the idPrefix, and a skipped nested object. registratie-wizard.machine.ts changes Errors from an interface to a type alias, because only a type alias gets an implicit index signature and is assignable to the helper's Record<string, unknown> parameter. The other two machines already declare their error maps as type aliases, so this also makes the three consistent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
171 lines
8.2 KiB
Markdown
171 lines
8.2 KiB
Markdown
# 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<string, unknown>, 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<string, unknown>` 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<string, unknown>` and the call site will not compile. The other two machines already
|
|
declare their error maps as type aliases (`Partial<Record<…, string>>`), 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.
|