diff --git a/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts b/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts index f603a84..507780d 100644 --- a/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts +++ b/apps/ssp/src/app/herregistratie/domain/herregistratie.machine.ts @@ -7,6 +7,7 @@ import { reduceUpload, requiredCategoriesSatisfied, deliveryRefs, + digitalDocumentIds, } from '@shared/domain/upload.machine'; /** What the user is typing (raw, possibly invalid). */ @@ -53,7 +54,7 @@ export function hasProgress(s: Extract): boolea !!s.draft.uren || !!s.draft.jaren || !!s.draft.punten || - deliveryRefs(s.upload).some((r) => r.channel === 'digital' && !!r.documentId) + digitalDocumentIds(s.upload).length > 0 ); } 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 77f9701..254da49 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 @@ -25,7 +25,7 @@ import { import { createDraftSync } from '@registratie/application/draft-sync'; import { DocumentUploadComponent } from '@shared/ui/organisms/upload/document-upload/document-upload.component'; import { createUploadController } from '@shared/application/upload-controller'; -import { UploadState, initialUpload, deliveryRefs } from '@shared/domain/upload.machine'; +import { UploadState, initialUpload, digitalDocumentIds } from '@shared/domain/upload.machine'; /** Organism: multi-step herregistratie wizard. ALL state lives in one signal driven by the pure `reduce` function (see herregistratie.machine.ts) via an @@ -148,8 +148,14 @@ import { UploadState, initialUpload, deliveryRefs } from '@shared/domain/upload. }) export class HerregistratieWizardComponent { private profile = inject(BigProfileStore); - // 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). + + /** Optional seed so Storybook / the showcase can mount any state directly. */ + seed = input(initial); + + // --- The store: all state in one signal, changed only by a pure reduce ----- + // The effect fires once, on the `Editing -> Submitting` transition. `Seed` is exempt, + // so a story that mounts straight into `Submitting` does not call the network. + // `draftSync` is declared below (both callbacks are deferred, so the cycle is safe). private store = createStore(initial, reduce, { Submitting: async (s, store) => { this.profile.beginHerregistratie(); @@ -163,36 +169,10 @@ export class HerregistratieWizardComponent { } }, }); - - /** Preview/download link for a completed upload; delegates to the upload - controller (application layer), which knows the dev-simulation `demo-*` ids - have no stored bytes and returns no link for them. */ - protected previewUrlFor = (documentId: string): string | undefined => - this.uploadCtl.previewUrlFor(documentId); - - /** Optional seed so Storybook / the showcase can mount any state directly. */ - seed = input(initial); - readonly state = this.store.model; // public so the showcase can highlight the live state protected dispatch = this.store.dispatch; - // Backend draft-sync (new persistence for this wizard): create a Concept on first - // progress, debounced-sync the snapshot, resume by `?aanvraag=`. - private draftSync = createDraftSync({ - type: 'herregistratie', - snapshot: () => { - const s = this.state(); - if (s.tag !== 'Editing' || !hasProgress(s)) return null; - const documentIds = deliveryRefs(s.upload) - .filter((r) => r.channel === 'digital' && r.documentId) - .map((r) => r.documentId!); - return { draft: s, stepIndex: s.step - 1, stepCount: this.stepLabels.length, documentIds }; - }, - onResume: (draft) => this.dispatch({ tag: 'Seed', state: draft as WizardState }), - enabled: () => this.seed() === initial, - }); - - // Stepper labels + per-step heading titles (presentational only). + // --- Static copy: stepper labels and per-step headings --------------------- readonly stepLabels = [ $localize`:@@herregWizard.step.werkervaring:Werkervaring`, $localize`:@@herregWizard.step.nascholing:Nascholing`, @@ -204,6 +184,7 @@ export class HerregistratieWizardComponent { $localize`:@@herregWizard.title.documenten:Documenten aanleveren`, ]; + // --- State projections: one narrow, then read-only views of it ------------- private editing = computed(() => whenTag(this.state(), 'Editing')); protected step = computed(() => this.editing()?.step ?? 1); protected draft = computed( @@ -214,11 +195,35 @@ export class HerregistratieWizardComponent { protected errJaren = computed(() => this.editing()?.errors.jaren ?? ''); protected errPunten = computed(() => this.editing()?.errors.punten ?? ''); protected errDocumenten = computed(() => this.editing()?.errors.documenten ?? ''); + + // --- Controllers: persistence and uploads ---------------------------------- + // Create a Concept on first progress, then debounced-sync the snapshot. + // `?aanvraag=` resumes it. + private draftSync = createDraftSync({ + type: 'herregistratie', + snapshot: () => { + const s = this.state(); + if (s.tag !== 'Editing' || !hasProgress(s)) return null; + return { + draft: s, + stepIndex: s.step - 1, + stepCount: this.stepLabels.length, + documentIds: digitalDocumentIds(s.upload), + }; + }, + onResume: (draft) => this.dispatch({ tag: 'Seed', state: draft as WizardState }), + enabled: () => this.seed() === initial, + }); protected uploadCtl = createUploadController({ wizardId: 'herregistratie', getUpload: () => this.upload(), dispatch: (msg) => this.dispatch({ tag: 'Upload', msg }), }); + /** Preview/download link for a completed upload; delegates to the upload + controller (application layer), which knows the dev-simulation `demo-*` ids + have no stored bytes and returns no link for them. */ + protected previewUrlFor = (documentId: string): string | undefined => + this.uploadCtl.previewUrlFor(documentId); // --- Presentational wiring for the shared wizard shell --------------------- protected stepTitle = computed(() => this.stepTitles[this.step() - 1]); @@ -228,11 +233,6 @@ export class HerregistratieWizardComponent { ? naarStapLabel(step + 1, this.stepLabels[step]) : $localize`:@@herregWizard.indienen:Herregistratie aanvragen`; }); - - /** Stepper emits a 0-based index for an earlier (visited) step. */ - protected goToStep(index: number) { - this.dispatch({ tag: 'GaNaarStap', step: (index + 1) as 1 | 2 | 3 }); - } /** Maps this machine's own tags onto the shell's `WizardPhase` vocabulary, composing the localized failure prefix so the `Failed` message arrives intact. */ protected phase = computed(() => { @@ -254,6 +254,12 @@ export class HerregistratieWizardComponent { /** Current step's field errors, flattened for the shell's error summary. */ protected errorList = computed(() => toWizardErrors(this.editing()?.errors ?? {})); + // --- Event handlers: narrow a child event into a message ------------------- + /** Stepper emits a 0-based index for an earlier (visited) step. */ + protected goToStep(index: number) { + this.dispatch({ tag: 'GaNaarStap', step: (index + 1) as 1 | 2 | 3 }); + } + constructor() { // An explicit seed (stories/tests) wins; otherwise resume the backend draft // (`?aanvraag=`) or start fresh. Persistence is the draftSync controller's job. 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 f58565e..bfe20b9 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 @@ -32,8 +32,9 @@ import { ReviewStep } from './review.step'; /** Organism: a BRANCHING intake questionnaire. All state lives in one signal driven by the pure `reduce` (intake.machine.ts). Which step renders is derived from the answers via `visibleSteps`, never stored — so editing an earlier - answer immediately changes the remaining steps. Answers are persisted to - sessionStorage so a page reload keeps the user's progress (cleared on tab close). */ + answer immediately changes the remaining steps. The draft persists to the + backend as a Concept aanvraag (createDraftSync), so a reload — or a "Verder + gaan" from the dashboard via `?aanvraag=` — resumes progress. */ @Component({ selector: 'app-intake-wizard', imports: [ @@ -106,8 +107,14 @@ 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); - // 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). + + /** Optional seed so Storybook / the showcase can mount any state directly. */ + seed = input(initial); + + // --- The store: all state in one signal, changed only by a pure reduce ----- + // The effect fires once, on the `Answering -> Submitting` transition. `Seed` is exempt, + // so a story that mounts straight into `Submitting` does not call the network. + // `draftSync` is declared below (both callbacks are deferred, so the cycle is safe). private store = createStore(initial, reduce, { Submitting: async (s, store) => { this.profile.beginHerregistratie(); @@ -129,24 +136,22 @@ export class IntakeWizardComponent { }, }); - /** Optional seed so Storybook / the showcase can mount any state directly. */ - seed = input(initial); - readonly state = this.store.model; readonly dispatch = this.store.dispatch; - // Backend draft-sync (replaces sessionStorage); the intake has no uploads. - private draftSync = createDraftSync({ - type: 'intake', - snapshot: () => { - const s = this.state(); - if (s.tag !== 'Answering' || !hasProgress(s)) return null; - return { draft: s, stepIndex: s.cursor, stepCount: STEPS.length, documentIds: [] }; - }, - onResume: (draft) => this.dispatch({ tag: 'Seed', state: draft as IntakeState }), - enabled: () => this.seed() === initial, - }); + // --- Static copy: stepper labels and per-step headings --------------------- + readonly stepLabels = [ + $localize`:@@intake.step.buitenland:Buitenland`, + $localize`:@@intake.step.werk:Werk`, + $localize`:@@intake.step.controle:Controle`, + ]; + private stepTitles: Record = { + buitenland: $localize`:@@intake.title.buitenland:Werken in het buitenland`, + werk: $localize`:@@intake.title.werk:Werkervaring in Nederland`, + review: $localize`:@@intake.title.review:Controleren en indienen`, + }; + // --- State projections: one narrow, then read-only views of it ------------- private answering = computed(() => whenTag(this.state(), 'Answering')); /** Public so the showcase can render the (fixed) step list next to the wizard. */ readonly steps = STEPS; @@ -159,17 +164,21 @@ export class IntakeWizardComponent { ); protected errors = computed(() => this.answering()?.errors ?? {}); + // --- Controllers: persistence and uploads ---------------------------------- + // Create a Concept on first progress, then debounced-sync the snapshot. + // `?aanvraag=` resumes it. The intake has no uploads. + private draftSync = createDraftSync({ + type: 'intake', + snapshot: () => { + const s = this.state(); + if (s.tag !== 'Answering' || !hasProgress(s)) return null; + return { draft: s, stepIndex: s.cursor, stepCount: STEPS.length, documentIds: [] }; + }, + onResume: (draft) => this.dispatch({ tag: 'Seed', state: draft as IntakeState }), + enabled: () => this.seed() === initial, + }); + // --- Presentational wiring for the shared wizard shell --------------------- - readonly stepLabels = [ - $localize`:@@intake.step.buitenland:Buitenland`, - $localize`:@@intake.step.werk:Werk`, - $localize`:@@intake.step.controle:Controle`, - ]; - private stepTitles: Record = { - buitenland: $localize`:@@intake.title.buitenland:Werken in het buitenland`, - werk: $localize`:@@intake.title.werk:Werkervaring in Nederland`, - review: $localize`:@@intake.title.review:Controleren en indienen`, - }; protected stepTitle = computed(() => this.stepTitles[this.step()]); protected primaryLabel = computed(() => { if (this.step() === 'review') return $localize`:@@intake.indienen:Aanvraag indienen`; 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 67abd47..ed68248 100644 --- a/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts +++ b/apps/ssp/src/app/registratie/domain/registratie-wizard.machine.ts @@ -9,6 +9,7 @@ import { reduceUpload, requiredCategoriesSatisfied, deliveryRefs, + digitalDocumentIds, } from '@shared/domain/upload.machine'; /** @@ -109,7 +110,7 @@ export function hasProgress(s: Extract): !!d.email || !!d.diplomaId || !!d.beroep || - deliveryRefs(s.upload).some((r) => r.channel === 'digital' && !!r.documentId) + digitalDocumentIds(s.upload).length > 0 ); } diff --git a/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.spec.ts b/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.spec.ts new file mode 100644 index 0000000..1543be0 --- /dev/null +++ b/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.spec.ts @@ -0,0 +1,43 @@ +import { describe, it, expect } from 'vitest'; +import { DuoLookupDto } from '@registratie/contracts/duo-diplomas.dto'; +import { diplomaMsg } from './diploma-msg'; +import { HANDMATIG } from './beroep.step'; + +const data: DuoLookupDto = { + diplomas: [ + { + id: 'd1', + naam: 'Verpleegkunde', + instelling: 'Hogeschool Utrecht', + jaar: 2019, + beroep: 'Verpleegkundige', + policyQuestions: [ + { id: 'q1', vraag: 'Vraag 1', type: 'ja-nee' }, + { id: 'q2', vraag: 'Vraag 2', type: 'tekst' }, + ], + }, + ], + handmatig: { + beroepen: ['Verpleegkundige', 'Arts'], + policyQuestions: [{ id: 'm1', vraag: 'Handmatige vraag', type: 'ja-nee' }], + }, +}; + +describe('diplomaMsg', () => { + it('resolves a known diploma into KiesDiploma with the server-derived beroep', () => { + expect(diplomaMsg(data, 'd1')).toEqual({ + tag: 'KiesDiploma', + diplomaId: 'd1', + beroep: 'Verpleegkundige', + vraagIds: ['q1', 'q2'], + }); + }); + + it('resolves the manual sentinel into KiesHandmatig with the maximal question set', () => { + expect(diplomaMsg(data, HANDMATIG)).toEqual({ tag: 'KiesHandmatig', vraagIds: ['m1'] }); + }); + + it('returns null for an unknown diploma id', () => { + expect(diplomaMsg(data, 'onbekend')).toBeNull(); + }); +}); diff --git a/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.ts b/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.ts new file mode 100644 index 0000000..71e53cc --- /dev/null +++ b/apps/ssp/src/app/registratie/ui/registratie-wizard/diploma-msg.ts @@ -0,0 +1,28 @@ +import { DuoLookupDto } from '@registratie/contracts/duo-diplomas.dto'; +import { RegistratieMsg } from '@registratie/domain/registratie-wizard.machine'; +import { HANDMATIG } from './beroep.step'; + +/** + * Resolve the diploma that the user picked into the machine message it implies. + * + * The DUO payload maps a diploma id onto a server-derived beroep and the policy + * questions that apply to it. Reading that map is message construction, so it + * belongs beside the container, not in the beroep step. The backend stays the + * authority on both values (ADR-0001) — this function only selects them. + * + * Returns null when the id matches no known diploma. The caller then dispatches + * nothing and the wizard keeps its current state. + */ +export function diplomaMsg(data: DuoLookupDto, id: string): RegistratieMsg | null { + if (id === HANDMATIG) { + return { tag: 'KiesHandmatig', vraagIds: data.handmatig.policyQuestions.map((q) => q.id) }; + } + const diploma = data.diplomas.find((d) => d.id === id); + if (!diploma) return null; + return { + tag: 'KiesDiploma', + diplomaId: diploma.id, + beroep: diploma.beroep, + vraagIds: diploma.policyQuestions.map((q) => q.id), + }; +} 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 3ca2c36..4f8241d 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 @@ -26,17 +26,18 @@ import { STEPS, } from '@registratie/domain/registratie-wizard.machine'; import { createDraftSync } from '@registratie/application/draft-sync'; -import { UploadState, initialUpload, deliveryRefs } from '@shared/domain/upload.machine'; +import { UploadState, initialUpload, digitalDocumentIds } from '@shared/domain/upload.machine'; import { AdresStep } from './adres.step'; -import { BeroepStep, HANDMATIG } from './beroep.step'; +import { BeroepStep } from './beroep.step'; import { ControleStep } from './controle.step'; +import { diplomaMsg } from './diploma-msg'; /** Organism: the BIG-registration wizard. All state lives in one signal driven by the pure `reduce` (registratie-wizard.machine.ts). The BRP address prefills the draft via an effect; the DUO diploma list renders through ; choosing - a diploma reveals its server-derived beroep. The draft is persisted to the - backend as a Concept aanvraag (createDraftSync) so a reload — or a"Verder gaan" - from the dashboard via `?aanvraag=` — resumes progress. Built from existing + a diploma reveals its server-derived beroep. The draft persists to the backend + as a Concept aanvraag (createDraftSync), so a reload — or a "Verder gaan" from + the dashboard via `?aanvraag=` — resumes progress. Built from existing atoms/molecules. */ @Component({ selector: 'app-registratie-wizard', @@ -117,8 +118,14 @@ import { ControleStep } from './controle.step'; }) export class RegistratieWizardComponent { private lookup = inject(RegistratieLookupStore); - // 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). + + /** Optional seed so Storybook / tests can mount any state directly. */ + seed = input(initial); + + // --- The store: all state in one signal, changed only by a pure reduce ----- + // The effect fires once, on the `Invullen -> Indienen` transition. `Seed` is exempt, + // so a story that mounts straight into `Indienen` does not call the network. + // `draftSync` is declared below (both callbacks are deferred, so the cycle is safe). private store = createStore(initial, reduce, { Indienen: async (s, store) => { const r = await this.draftSync.submit({ @@ -129,50 +136,56 @@ export class RegistratieWizardComponent { else store.dispatch({ tag: 'SubmitFailed', error: r.error }); }, }); + readonly state = this.store.model; + readonly dispatch = this.store.dispatch; - /** Optional seed so Storybook / tests can mount any state directly. */ - seed = input(initial); - + // --- Static copy: stepper labels and per-step headings --------------------- readonly stepLabels = [ $localize`:@@regWizard.step.adres:Adres`, $localize`:@@regWizard.step.beroep:Beroep`, $localize`:@@regWizard.step.controle:Controle`, - ]; // short labels for the stepper + ]; private stepTitles = [ $localize`:@@regWizard.title.adres:Adres en correspondentievoorkeur`, $localize`:@@regWizard.title.beroep:Beroep op basis van uw diploma`, $localize`:@@regWizard.title.controle:Controleren en indienen`, ]; - readonly state = this.store.model; - readonly dispatch = this.store.dispatch; + // --- State projections: one narrow, then read-only views of it ------------- private invullen = computed(() => whenTag(this.state(), 'Invullen')); protected cursor = computed(() => this.invullen()?.cursor ?? 0); protected draft = computed(() => this.invullen()?.draft ?? { antwoorden: {} }); protected errors = computed(() => this.invullen()?.errors ?? {}); protected upload = computed(() => this.invullen()?.upload ?? initialUpload); - // Backend draft-sync (replaces sessionStorage): create a Concept once the user has - // made progress, then debounced-sync the whole machine snapshot; resume by `?aanvraag`. + protected step = computed(() => STEPS[Math.min(this.cursor(), STEPS.length - 1)]); + protected referentie = computed(() => whenTag(this.state(), 'Ingediend')?.referentie ?? ''); + /** From the lookup store, not the machine: the beroep step renders it, and + `onDiplomaKeuze` reads it to resolve the picked id into a message. */ + protected duoData = computed(() => successOr(this.lookup.duoLookup(), null)); + + // --- Controllers: persistence and uploads ---------------------------------- + // Create a Concept once the user has made progress, then debounced-sync the whole + // machine snapshot. `?aanvraag=` resumes it. private draftSync = createDraftSync({ type: 'registratie', snapshot: () => { const s = this.state(); if (s.tag !== 'Invullen' || !hasProgress(s)) return null; - const documentIds = deliveryRefs(s.upload) - .filter((r) => r.channel === 'digital' && r.documentId) - .map((r) => r.documentId!); - return { draft: s, stepIndex: s.cursor, stepCount: STEPS.length, documentIds }; + return { + draft: s, + stepIndex: s.cursor, + stepCount: STEPS.length, + documentIds: digitalDocumentIds(s.upload), + }; }, onResume: (draft) => this.dispatch({ tag: 'Seed', state: draft as RegistratieState }), enabled: () => this.seed() === initial, }); - protected step = computed(() => STEPS[Math.min(this.cursor(), STEPS.length - 1)]); + + // --- Presentational wiring for the shared wizard shell --------------------- protected stepTitle = computed( () => this.stepTitles[Math.min(this.cursor(), this.stepTitles.length - 1)], ); - protected referentie = computed(() => whenTag(this.state(), 'Ingediend')?.referentie ?? ''); - - // --- Presentational wiring for the shared wizard shell --------------------- protected primaryLabel = computed(() => { if (this.step() === 'controle') return $localize`:@@regWizard.indienen:Registratie indienen`; const next = this.cursor() + 1; @@ -203,30 +216,12 @@ export class RegistratieWizardComponent { return [...toWizardErrors(e), ...toWizardErrors(e.antwoorden ?? {}, 'vraag-')]; }); - /** Parsed lookup as a plain value (or null) — needed here only to resolve - `onDiplomaKeuze`'s message from an id (the DUO payload maps an id to a - beroep and its question ids; that is machine-message construction, and it - belongs in the container, not the beroep step). */ - protected duoData = computed(() => successOr(this.lookup.duoLookup(), null)); - + // --- Event handlers: narrow a child event into a message ------------------- protected onDiplomaKeuze(id: string) { const data = this.duoData(); if (!data) return; - if (id === HANDMATIG) { - this.dispatch({ - tag: 'KiesHandmatig', - vraagIds: data.handmatig.policyQuestions.map((q) => q.id), - }); - return; - } - const d = data.diplomas.find((x) => x.id === id); - if (d) - this.dispatch({ - tag: 'KiesDiploma', - diplomaId: d.id, - beroep: d.beroep, - vraagIds: d.policyQuestions.map((q) => q.id), - }); + const msg = diplomaMsg(data, id); + if (msg) this.dispatch(msg); } /** Narrows the beroep step's plain-string `kanaalChange` into the machine's @@ -260,12 +255,10 @@ export class RegistratieWizardComponent { }); }); }); - // A11y: focus management (step heading on step change, error summary on a - // failed submit) now lives in the shared WizardShellComponent. } /** 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. */ + re-prefills, keeping the form and the "vooraf ingevuld" note consistent. */ restart() { this.draftSync.reset(); // discard the current Concept; a fresh one starts on next progress this.dispatch({ tag: 'Seed', state: initial }); diff --git a/docs/project/readable-codebase/RD-38-wizard-container-readability.md b/docs/project/readable-codebase/RD-38-wizard-container-readability.md new file mode 100644 index 0000000..d7e340d --- /dev/null +++ b/docs/project/readable-codebase/RD-38-wizard-container-readability.md @@ -0,0 +1,100 @@ +# RD-38 — One member order for the three wizard containers + +Status: done +Source: user report — "the registratie-wizard component still doesn't look readable" + +## Why + +RD-22 and RD-23 split the intake and registratie wizards into step components. That brought +every container under the 250-line budget, so `max-lines` reports nothing. The user read the +result and still called it unreadable. Line count was never the problem. + +Three problems remained, and the user named all three: + +1. **The member order is scrambled, and it differs per file.** You bounce up and down to + follow one thread, and the three containers cannot be compared side by side. +2. **Pure logic sits in the container**, where it has no test. +3. **Comments carry archaeology** — ticket numbers, history, and one comment that describes + code which is no longer in the file. + +RD-02 measures a file. This ticket is about what a file reads like at a fixed size. + +## Evidence, before the change + +- `registratie` declared `stepLabels`/`stepTitles` between the `seed` input and + `state`/`dispatch`; `intake` declared them near the bottom; `herregistratie` declared them + after `draftSync`. Three files, three orders for the same three roles. +- `registratie` declared `draftSync` in the middle of a run of `computed`s. +- `herregistratie`'s `snapshot` read `this.stepLabels.length` seven lines before + `stepLabels` was declared. +- `intake`'s class comment claimed answers persist to `sessionStorage`. Thirty lines below, + another comment said draft-sync replaced it. The class comment was false. +- All three cited "RD-05's tag-transition rule" in an identical sentence. + +## Decisions (pre-made, don't relitigate) + +1. **One nine-section member order, identical in all three files.** Injected stores → inputs + → the store (`createStore`, `state`, `dispatch`) → static copy → state projections → + controllers → shell wiring → event handlers → constructor and `restart`. Each section + carries a `// --- ---` header. Section 7's header already existed in all three + files, so it is reused byte for byte. + +2. **The store ⇄ `draftSync` cycle stays.** The store's effect map calls `this.draftSync`, + and `draftSync`'s `snapshot` calls `this.state()`. Both are arrow functions that run after + construction, so there is no temporal-dead-zone hazard; the cycle cannot be removed by + reordering. One comment at the effect map names it. + +3. **Extract `digitalDocumentIds` into `libs/shared/src/domain/upload.machine.ts`.** The + "digital and finished uploading" filter was written out four times, in two shapes: mapped + to ids in the two container snapshots, and as `.some(...)` inside two machines' + `hasProgress`. One function serves all four, and it removes the `r.documentId!` non-null + assertion from both containers. It joins an existing file beside `deliveryRefs`, and its + spec joins the existing `deliveryRefs` block. + +4. **Extract `diplomaMsg` into a new sibling of the step files.** `onDiplomaKeuze` was the + fattest member in the three containers and had no test. It is pure: a `DuoLookupDto` and a + selected id in, a machine message or `null` out. `HANDMATIG` does not move — the new file + is its sibling and imports it exactly as the container did. + +5. **`phase` stays in all three containers.** It maps this machine's tags onto the shell's + `WizardPhase` vocabulary and composes a `$localize` failure message. That is a container's + job. It cannot move to `domain/`: `WizardPhase` comes from an Angular component, and + `domain/` points inward only. Moving it to a per-wizard sibling would add three files and + three specs to relocate 17 readable lines each. Decision 1 already fixes what was wrong + with it — it belongs in the shell-wiring section, and only two of three files had it there. + +6. **The four `err*` computeds in `herregistratie` stay.** They are one-line projections + feeding four template bindings. Folding them into one `errors()` would edit the template, + which is behaviour-shaped work this ticket does not do. + +7. **Comment policy, four rules.** Delete ticket references and keep the sentence (RD-18 + decision 1 stripped `WP-`/`RB-` for the same reason; `RD-` is the same debt). Delete + history — a comment says what the code does now. Delete a comment that describes code + which is not in the file. Keep a comment that states a current why: the `untracked` + loop-avoidance notes, the `IntakePolicy.RejectIncompleteScholing` seam pointer, and the + `demo-*` preview note all stay. + +## Traps + +- **`messages.en.xlf`.** Every `$localize` id in these files is translated. The reorder moves + the copy arrays; it must not touch an id or its source text. +- **`enabled: () => this.seed() === initial`** is a reference-identity check against the + exported `initial` singleton. Never clone or rebuild it — breaking the identity turns + draft-sync on inside Storybook and the tests. +- **Public members are load-bearing.** `showcase/vragenlijst.section.ts` reads + `IntakeWizardComponent.steps`; `showcase/form-machine.section.ts` reads + `HerregistratieWizardComponent.state`. No `private`/`protected`/`readonly` modifier changes. +- **`intake.machine.ts` carries `#region showcase:steps` markers** that feed `gen:snippets`. + This ticket does not touch them. +- **New specs change a generated document.** `scripts/ci-local.sh` regenerates + `libs/shared/docs/behaviour-spec.mdx` and diffs it. Four new `it()` titles land there, so + the regenerated file belongs in the same commit. + +## Acceptance + +- `npm run ci` passes. +- Each container's member list and every modifier are unchanged: + `diff <(git show HEAD:$f | grep -oE '^ (private|protected|readonly)? ?[a-zA-Z]+ *[=(]' | sort) <(...)` + reports no difference for all three files. +- `git grep -nE "\bRD-[0-9]+" -- 'apps/ssp/src/app/*/ui/*wizard*'` returns nothing. +- `git grep -n "sessionStorage" -- 'apps/ssp/src/app/herregistratie/ui'` returns nothing. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index 2fe7a41..773b4fd 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -132,6 +132,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-35 | _(optional, last, alone)_ upload `type:` discriminant to `tag:` | 27 | | todo | | 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 | 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 a967796..632bb84 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. 556 frontend behaviours across +**is** the suite, reshaped for a business reader. 562 frontend behaviours across 9 contexts; 261 backend behaviours across 42 test classes. @@ -545,6 +545,12 @@ classes. - lists soort/waarvoor/status/referentie/ingediend, plus reason when rejected - reference falls back to em dash for a Concept +#### diplomaMsg + +- resolves a known diploma into KiesDiploma with the server-derived beroep +- resolves the manual sentinel into KiesHandmatig with the maximal question set +- returns null for an unknown diploma id + #### findConcept - returns the id of the existing Concept of the given type @@ -857,6 +863,12 @@ classes. - emits documentId for completed digital uploads and channel for post - omits digital categories with no completed upload +#### digitalDocumentIds + +- returns the id of a completed digital upload +- omits an upload that is still in flight +- omits a category that the user delivers by post + #### flushPendingGuard - flushes then allows navigation when a write is pending diff --git a/libs/shared/src/domain/upload.machine.spec.ts b/libs/shared/src/domain/upload.machine.spec.ts index c81f811..947d710 100644 --- a/libs/shared/src/domain/upload.machine.spec.ts +++ b/libs/shared/src/domain/upload.machine.spec.ts @@ -8,6 +8,7 @@ import { categorySatisfied, requiredCategoriesSatisfied, deliveryRefs, + digitalDocumentIds, inFlight, rejectReason, planFileSelection, @@ -284,6 +285,28 @@ describe('deliveryRefs', () => { }); }); +describe('digitalDocumentIds', () => { + it('returns the id of a completed digital upload', () => { + let s = select(stateWith([cat({ categoryId: 'a' })]), 'a', 'u1'); + s = reduceUpload(s, { type: 'UploadComplete', localId: 'u1', documentId: 'doc1' }); + expect(digitalDocumentIds(s)).toEqual(['doc1']); + }); + + it('omits an upload that is still in flight', () => { + const s = select(stateWith([cat({ categoryId: 'a' })]), 'a', 'u1'); // still queued + expect(digitalDocumentIds(s)).toEqual([]); + }); + + it('omits a category that the user delivers by post', () => { + let s = stateWith([cat({ categoryId: 'a' }), cat({ categoryId: 'b' })], { + deliveryChannel: { b: 'post' }, + }); + s = select(s, 'a', 'u1'); + s = reduceUpload(s, { type: 'UploadComplete', localId: 'u1', documentId: 'doc1' }); + expect(digitalDocumentIds(s)).toEqual(['doc1']); + }); +}); + describe('rejectReason', () => { it('rejects a disallowed type', () => { expect( diff --git a/libs/shared/src/domain/upload.machine.ts b/libs/shared/src/domain/upload.machine.ts index 90d2898..ff046d2 100644 --- a/libs/shared/src/domain/upload.machine.ts +++ b/libs/shared/src/domain/upload.machine.ts @@ -312,6 +312,14 @@ export function deliveryRefs( return refs; } +/** Ids of the documents that the user delivers digitally and that finished uploading. + A category set to post, or one whose upload is still in flight, contributes nothing. */ +export function digitalDocumentIds(s: UploadState): string[] { + return deliveryRefs(s) + .filter((r) => r.channel === 'digital' && r.documentId) + .map((r) => r.documentId as string); +} + /** Used by the shell to find what to poll on return: still-in-flight uploads. */ export const inFlight = (s: UploadState): Upload[] => s.uploads.filter((u) => u.status.type === 'queued' || u.status.type === 'uploading');