diff --git a/apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts b/apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts index e26a30f..8087f7a 100644 --- a/apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts +++ b/apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts @@ -6,6 +6,8 @@ import { AlertComponent } from '@shared/ui/alert/alert.component'; import { FormFieldComponent } from '@shared/ui/form-field/form-field.component'; import { TextInputComponent } from '@shared/ui/text-input/text-input.component'; import { RadioGroupComponent, RadioOption } from '@shared/ui/radio-group/radio-group.component'; +import { DataBlockComponent } from '@shared/ui/data-block/data-block.component'; +import { DataRowComponent } from '@shared/ui/data-row/data-row.component'; import { createStore } from '@shared/application/store'; import { whenTag } from '@shared/kernel/fp'; import { BesluitState, BesluitMsg, initial, reduce } from '@behandeling/domain/besluit.machine'; @@ -29,10 +31,35 @@ import { createSubmitBesluit } from '@behandeling/application/submit-besluit'; FormFieldComponent, TextInputComponent, RadioGroupComponent, + DataBlockComponent, + DataRowComponent, ], template: ` @if (state().tag === 'Submitted') { Het besluit is vastgelegd. + } @else if (state().tag === 'Failed') { + Besluit vastleggen + + Het vastleggen is niet gelukt: + {{ failedError() }} + + +
+ @if (toelichting()) { +
+ } + + +
+ Opnieuw proberen +
} @else { Besluit vastleggen @@ -70,13 +97,6 @@ import { createSubmitBesluit } from '@behandeling/application/submit-besluit'; /> - @if (failedError()) { - Het vastleggen is niet gelukt: - {{ failedError() }} - } - {{ state().tag === 'Submitting' ? submitBezigLabel : submitLabel }} @@ -86,7 +106,19 @@ import { createSubmitBesluit } from '@behandeling/application/submit-besluit'; }) export class BesluitFormComponent { private submit = createSubmitBesluit(); - 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) => { + const r = await this.submit(this.id(), s.data); + if (r.ok) { + store.dispatch({ tag: 'SubmitConfirmed' }); + this.decided.emit(); + } else { + store.dispatch({ tag: 'SubmitFailed', error: r.error }); + } + }, + }); id = input.required(); decided = output(); @@ -109,12 +141,33 @@ export class BesluitFormComponent { protected readonly submitLabel = $localize`:@@besluit.submit:Besluit vastleggen`; protected readonly submitBezigLabel = $localize`:@@besluit.submitBezig:Bezig met vastleggen…`; + // Same ids as the form-field labels above, reused for the Failed data-block's row + // keys (the pattern change-request-form already uses for its read-only BRP rows). + protected readonly besluitLabelText = $localize`:@@besluit.besluitLabel:Besluit`; + protected readonly toelichtingLabelText = $localize`:@@besluit.toelichtingLabel:Toelichting`; + private editing = computed(() => whenTag(this.state(), 'Editing')); protected errors = computed(() => this.editing()?.errors ?? {}); protected failedError = computed(() => whenTag(this.state(), 'Failed')?.error ?? ''); - protected besluit = computed(() => this.editing()?.draft.besluit ?? ''); - protected toelichting = computed(() => this.editing()?.draft.toelichting ?? ''); + /** The value shown in the field — the live draft while editing, the parsed value + while submitting/failed (so the user sees what they sent, same idiom as + change-request-form.telefoon()). */ + protected besluit = computed(() => { + const s = this.state(); + if (s.tag === 'Editing') return s.draft.besluit; + if (s.tag === 'Submitting' || s.tag === 'Failed') return s.data.besluit; + return ''; + }); + protected toelichting = computed(() => { + const s = this.state(); + if (s.tag === 'Editing') return s.draft.toelichting; + if (s.tag === 'Submitting' || s.tag === 'Failed') return s.data.toelichting ?? ''; + return ''; + }); + protected besluitOptieLabel = computed( + () => this.BESLUIT_OPTIONS.find((o) => o.value === this.besluit())?.label ?? '', + ); constructor() { queueMicrotask(() => this.dispatch({ tag: 'Seed', state: this.seed() })); @@ -122,19 +175,5 @@ export class BesluitFormComponent { onSubmit() { this.dispatch({ tag: 'Submit' }); - this.runIfSubmitting(); - } - - /** Effect: when we entered Submitting, call the command, then dispatch the outcome. */ - private async runIfSubmitting() { - const s = this.state(); - if (s.tag !== 'Submitting') return; - const r = await this.submit(this.id(), s.data); - if (r.ok) { - this.dispatch({ tag: 'SubmitConfirmed' }); - this.decided.emit(); - } else { - this.dispatch({ tag: 'SubmitFailed', error: r.error }); - } } } diff --git a/apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts b/apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts index f4d3a42..c7bf467 100644 --- a/apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts +++ b/apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts @@ -65,6 +65,26 @@ import { createSubmitChangeRequest } from '@registratie/application/submit-chang >Nieuwe wijziging doorgeven + } @else if (state().tag === 'Failed') { + Contactgegevens wijzigen + + Het indienen is niet gelukt: + {{ failedError() }} + + +
+ + +
+ Opnieuw proberen +
} @else { Contactgegevens wijzigen @@ -108,13 +128,6 @@ import { createSubmitChangeRequest } from '@registratie/application/submit-chang - @if (failedError()) { - Het indienen is niet gelukt: - {{ failedError() }} - } - {{ state().tag === 'Submitting' ? submitBezigLabel : submitLabel }} @@ -127,7 +140,15 @@ export class ChangeRequestFormComponent { // adapter); the UI holds only this bound command. Field initializer = injection // context, like createStore below. private submit = createSubmitChangeRequest(); - 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) => { + const r = await this.submit(s.data); + if (r.ok) store.dispatch({ tag: 'SubmitConfirmed', referentie: r.value }); + else store.dispatch({ tag: 'SubmitFailed', error: r.error }); + }, + }); /** BRP address, shown read-only. Undefined until the profile loads. */ brpAdres = input(undefined); @@ -148,6 +169,10 @@ export class ChangeRequestFormComponent { protected readonly postcodeLabel = $localize`:@@address.postcode:Postcode`; protected readonly woonplaatsLabel = $localize`:@@address.woonplaats:Woonplaats`; + // Same id as the telefoon form-field's label above, reused for the Failed + // data-block's row key. + protected readonly telefoonLabelText = $localize`:@@changeRequest.telefoonLabel:Telefoonnummer`; + private editing = computed(() => whenTag(this.state(), 'Editing')); protected errors = computed(() => this.editing()?.errors ?? {}); protected failedError = computed(() => whenTag(this.state(), 'Failed')?.error ?? ''); @@ -168,15 +193,5 @@ export class ChangeRequestFormComponent { onSubmit() { this.dispatch({ tag: 'Submit' }); - this.runIfSubmitting(); - } - - /** Effect: when we entered Submitting, call the command, then dispatch the outcome. */ - private async runIfSubmitting() { - const s = this.state(); - if (s.tag !== 'Submitting') return; - const r = await this.submit(s.data); - if (r.ok) this.dispatch({ tag: 'SubmitConfirmed', referentie: r.value }); - else this.dispatch({ tag: 'SubmitFailed', error: r.error }); } } diff --git a/docs/project/readable-codebase/RD-06-fix-unrecoverable-submit-failure.md b/docs/project/readable-codebase/RD-06-fix-unrecoverable-submit-failure.md new file mode 100644 index 0000000..2a7af4f --- /dev/null +++ b/docs/project/readable-codebase/RD-06-fix-unrecoverable-submit-failure.md @@ -0,0 +1,168 @@ +# RD-06 — Fix the unrecoverable submit failure in the two single-step forms + +Status: done +Source: PLAN.md 1a (the two bugs) + +## Why + +**This is a bug fix, not a refactor.** Two forms have an unrecoverable dead end reachable from +any failed submit. Both machines already support recovery; only the UI affordance is missing. + +Verified mechanics, per component: + +**`besluit-form.component.ts` — the fields are wiped.** + +- The template branches on `Submitted` only (line 34); `Failed` falls into the `@else` at + line 36, which renders the editable form. +- `editing = whenTag(state(), 'Editing')` (line 112) is `undefined` in `Failed`, so + `besluit()` and `toelichting()` (lines 116-117) both return `''`. **The user's decision + disappears from the screen.** +- The submit button's only disable condition is `Submitting` (line 80), so in `Failed` it is + **enabled**. +- Clicking dispatches `Submit`, and `besluit.machine.ts:75` is `if (s.tag !== 'Editing') +return s` — **a no-op**. + +**`change-request-form.component.ts` — the fields are frozen.** + +- Same `@if Submitted / @else form` shape (lines 55, 68), same always-enabled submit button + (line 118), same no-op `Submit` (`change-request.machine.ts:62`). +- Different in one way: `telefoon()` (lines 158-163) **does** read `Failed.data`, with the + comment "so the user sees what they sent". So the value stays on screen — but `SetField` is + `Editing`-only (`change-request.machine.ts:60`), so **typing does nothing**. + +Either way the only escape is a page reload. + +**The machines are already correct.** `Failed` carries `data: Valid` +(`besluit.machine.ts:34`, `change-request.machine.ts:32`), and `Retry` maps +`Failed → Submitting` with that preserved data (`besluit.machine.ts:80`, +`change-request.machine.ts:67`). Neither UI ever dispatches it. + +## Read first + +- `libs/shared/src/application/store.ts` — the effect map RD-05 added, and its `Seed` rule +- `apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts` (140 lines) +- `apps/behandelportal/src/app/behandeling/domain/besluit.machine.ts` (92 lines) +- `apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts` +- `apps/ssp/src/app/registratie/domain/change-request.machine.ts` +- `libs/shared/src/layout/wizard-shell/wizard-shell.component.ts:134` — the retry button and + the `$localize` id to reuse +- Both `*.stories.ts` for the two components + +## Decisions (pre-made, don't relitigate) + +1. **Give `Failed` its own template branch.** Do not let it fall through to the editable form. + One change fixes both symptoms: nothing renders wiped fields, and no dead submit button + exists. Shape: + + ``` + @if (state().tag === 'Submitted') { … } + @else if (state().tag === 'Failed') { error + what was sent + Retry } + @else { the form } + ``` + +2. **Show what was sent, read from `Failed.data`.** `change-request-form.telefoon()` already + does exactly this and says why in its comment. `besluit-form` copies that pattern rather + than inventing one. The user must be able to see what they are retrying. + +3. **The Retry button dispatches `{ tag: 'Retry' }`.** No machine change is needed — both + reducers already handle it, and RD-05's effect map fires on `Failed → Submitting` because + that is a tag transition. `onRetry` needs no effect call of its own. + +4. **Reuse the `$localize` id `@@wizard.opnieuwProberen`** with byte-identical source text + `Opnieuw proberen`. Verified present with an English target in **both** + `apps/ssp/src/locale/messages.en.xlf:2429` and + `apps/behandelportal/src/locale/messages.en.xlf:2373` (`Try +again`). **No new xlf target is needed.** A different source text under the same + id fails extraction, so do not reword it. + +5. **Migrate both components to RD-05's effect map.** Delete `runIfSubmitting` from both; + register the body as `{ Submitting: (s, store) => … }` on `createStore`. The narrowed state + arrives as argument one, so the + `const s = this.state(); if (s.tag !== 'Submitting') return;` preamble goes away. `onSubmit` + becomes a single `dispatch`. + +6. **Do not add a "go back and edit after a failure" path.** Neither machine has a + `Failed → Editing` message, and adding one is scope creep for a bug fix. Retry recovers the + dead end, which is what this ticket is for. Record the edit-after-failure gap as a + follow-up in the Out of scope section. + +7. **Do not change the `Reset` message or the `Seed` contract.** The stories mount states via + `Seed`, and RD-05 exempts `Seed` from firing effects precisely so they do not perform real + HTTP. + +## Files + +- `apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.component.ts` +- `apps/behandelportal/src/app/behandeling/ui/besluit-form/besluit-form.stories.ts` +- `apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.component.ts` +- `apps/ssp/src/app/registratie/ui/change-request-form/change-request-form.stories.ts` + +No machine file changes. No xlf changes. + +## Steps + +1. In `besluit-form`, add the `Failed` branch per decisions 1-3, reading the besluit and + toelichting from `Failed.data`. +2. Replace `runIfSubmitting` with an effect-map entry per decision 5; reduce `onSubmit` to one + dispatch. +3. Repeat both steps for `change-request-form`. +4. Add a `Failed` story to each component's `*.stories.ts` so the new branch has a rendered, + axe-checked state. Seed it directly, per decision 7. +5. Update this ticket's `Status:` to `done` and the README's RD-06 row to `done`. +6. Commit all of it together. + +## Acceptance criteria + +```bash +npm run ci # exits 0 +npm run ci --full # exits 0 — required: this ticket adds stories +``` + +Then prove the dead end is gone. There is no automated coverage for this, so verify by +seeding the `Failed` state in Storybook (`npm run storybook` and +`npm run storybook:behandelportal`) and checking all three, for **both** components: + +1. The submitted values are visible — not blank. +2. No enabled control dispatches `Submit`. +3. The Retry button is present, and clicking it leaves `Failed` (it enters `Submitting`). + +Prove the method is gone from both forms rather than renamed. Baseline today is **5 files** +(3 wizards + these 2 forms); note the registratie wizard spells it `runIfIndienen`, so both +names must be matched: + +```bash +grep -rl "runIfSubmitting\|runIfIndienen" apps/ssp apps/behandelportal | sort +# MUST be exactly these 3 (the wizards, migrated later by RD-08): +# apps/ssp/.../herregistratie-wizard/herregistratie-wizard.component.ts +# apps/ssp/.../intake-wizard/intake-wizard.component.ts +# apps/ssp/.../registratie-wizard/registratie-wizard.component.ts +``` + +## Verification + +`npm run ci --full`. `--full` is mandatory here: this ticket adds stories, and only +`build-storybook` plus the axe run exercise them. + +## Out of scope + +- The 3 wizards. That is RD-08, after RD-07 adds `Primary`. +- **Editing after a failure.** Both machines can only `Retry` the same data or `Reset` to + empty. A `Failed → Editing` transition that maps `data` back to a `draft` would be a genuine + UX improvement and needs a new message plus a reducer spec. Recorded here as a follow-up; + not part of this fix. +- `WizardStatus`/`WizardPhase`. That is RD-10. + +## Risks + +- **The Storybook trap.** Both components mount `Submitting` via `Seed` in stories that use a + real `provideHttpClient()` with **no request mocking** (`besluit-form.stories.ts:33`, + `change-request-form.stories.ts:34`). RD-05's `Seed` exemption is what stops those firing + real network calls. If a story flips to `Failed` on load, or `storybook-a11y` goes red, the + exemption is not working — fix that, do not delete the story. +- **Do not reword the retry label.** Same id, same source text, or extraction fails + (decision 4). +- **`behaviour-spec.mdx` drift** if you add or rename any spec. Run + `npm run gen:behaviour-spec` in the same commit if you do. +- **`besluit-form` has no `*.spec.ts`.** Do not add a component TestBed spec for this — the + house tests UI through Storybook (CLAUDE.md decision 5). The machines already have specs, + and this ticket changes no machine. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index fb39439..5355557 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -100,7 +100,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-03 | `overzicht` context: page + 2 nav sections, boundary edge, admin-links token | 02 | yes | done | | RD-04 | Story titles to `Domein//`; add the missing stories | 03 | yes | todo | | 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 | todo | +| 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 | | todo | | RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | todo | | RD-09 | **Docs + generator:** `form-machine.hbs`, ARCHITECTURE, fp-tea, skill | 08 | | todo |