diff --git a/apps/ssp/src/app/brief/application/brief.store.spec.ts b/apps/ssp/src/app/brief/application/brief.store.spec.ts index 2a493f8..b27dd61 100644 --- a/apps/ssp/src/app/brief/application/brief.store.spec.ts +++ b/apps/ssp/src/app/brief/application/brief.store.spec.ts @@ -179,7 +179,7 @@ const filledView: BriefView = { ...view, brief: filledBrief }; function loadedBrief(store: BriefStore): Brief { const s = store.model(); - if (s.tag !== 'loaded') throw new Error('not loaded'); + if (s.tag !== 'Loaded') throw new Error('not loaded'); return s.brief; } @@ -431,7 +431,7 @@ describe('BriefStore.load — 404 tolerance (RB-22)', () => { // Then reset() ran exactly once, and the store ends up loaded from its result. expect(reset).toHaveBeenCalledTimes(1); - expect(store.model().tag).toBe('loaded'); + expect(store.model().tag).toBe('Loaded'); }); it('a second 404 does not drive a second reset()', async () => { @@ -447,6 +447,6 @@ describe('BriefStore.load — 404 tolerance (RB-22)', () => { // Then reset() ran exactly once — the once-only bound holds across calls, not // just within one — and the second 404 surfaces as an ordinary load failure. expect(reset).toHaveBeenCalledTimes(1); - expect(store.model()).toEqual({ tag: 'failed', reason: BRIEF_LOAD_FAILED }); + expect(store.model()).toEqual({ tag: 'Failed', reason: BRIEF_LOAD_FAILED }); }); }); diff --git a/apps/ssp/src/app/brief/application/brief.store.ts b/apps/ssp/src/app/brief/application/brief.store.ts index 80ab564..15b166a 100644 --- a/apps/ssp/src/app/brief/application/brief.store.ts +++ b/apps/ssp/src/app/brief/application/brief.store.ts @@ -4,7 +4,7 @@ import { createStore } from '@shared/application/store'; import { ActionState, SaveState } from '@shared/application/action-state'; import { createHistory } from '@shared/application/history'; import { createDebouncedSave } from '@shared/application/debounced-save'; -import { machineRemoteData } from '@shared/application/machine-remote-data'; +import { fromLoadLifecycle } from '@shared/application/remote-data'; import { Brief, CaseContext, @@ -29,7 +29,7 @@ import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; * outcome. Mirrors `BigProfileStore`. All of `canEdit`/`canApprove`/`canReject`/ * `canSend`, `diagnostics`, `unresolved`, `canSubmit` are DERIVED here — never * stored. The permission flags come from the server's decision DTO (PRD-0002 phase - * P1) via `BriefState.loaded.decisions` — this store never computes them itself. + * P1) via `BriefState.Loaded.decisions` — this store never computes them itself. */ @Injectable({ providedIn: 'root' }) export class BriefStore implements PendingSave { @@ -95,11 +95,11 @@ export class BriefStore implements PendingSave { /** The load lifecycle as `RemoteData`, for `` — the machine keeps owning the letter's own domain lifecycle (draft/submitted/approved/…); this is purely a projection of its loading/failed tags onto the shared async seam. */ - readonly remoteData = computed(() => machineRemoteData(this.model())); + readonly remoteData = computed(() => fromLoadLifecycle(this.model())); private brief = computed(() => { const s = this.model(); - return s.tag === 'loaded' ? s.brief : null; + return s.tag === 'Loaded' ? s.brief : null; }); readonly canEdit = computed(() => this.decisions()?.canEdit ?? false); @@ -111,7 +111,7 @@ export class BriefStore implements PendingSave { private decisions = computed(() => { const s = this.model(); - return s.tag === 'loaded' ? s.decisions : null; + return s.tag === 'Loaded' ? s.decisions : null; }); readonly diagnostics = computed(() => (this.brief() ? allDiagnostics(this.brief()!) : [])); readonly unresolved = computed(() => (this.brief() ? unresolvedPlaceholders(this.brief()!) : [])); @@ -182,7 +182,7 @@ export class BriefStore implements PendingSave { } private restore(step: (current: Brief) => Brief | undefined) { const s = this.model(); - if (s.tag !== 'loaded') return; + if (s.tag !== 'Loaded') return; const target = step(s.brief); if (target === undefined) return; this.store.dispatch({ tag: 'Seed', state: { ...s, brief: target } }); diff --git a/apps/ssp/src/app/brief/application/org-template.store.ts b/apps/ssp/src/app/brief/application/org-template.store.ts index 67bb1fe..daf40a7 100644 --- a/apps/ssp/src/app/brief/application/org-template.store.ts +++ b/apps/ssp/src/app/brief/application/org-template.store.ts @@ -2,7 +2,7 @@ import { Injectable, computed, effect, inject, signal } from '@angular/core'; import { createStore } from '@shared/application/store'; import { ActionState, SaveState } from '@shared/application/action-state'; import { createDebouncedSave } from '@shared/application/debounced-save'; -import { machineRemoteData } from '@shared/application/machine-remote-data'; +import { fromLoadLifecycle } from '@shared/application/remote-data'; import { UploadAdapter, uploadContentUrl } from '@shared/infrastructure/upload.adapter'; import { UploadShellService } from '@shared/application/upload-shell.service'; import { UploadMsg, initialUpload, rejectReason } from '@shared/domain/upload.machine'; @@ -22,7 +22,7 @@ import { OrgTemplateAdapter } from '@brief/infrastructure/org-template.adapter'; import { PendingSave, registerPendingSave } from '@shared/application/pending-saves'; import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; -type LoadedState = Extract; +type LoadedState = Extract; const LOGO_CATEGORY = 'org-logo'; const NO_SUBORGS = $localize`:@@orgTemplate.noSubOrgs:Er zijn geen organisatiesjablonen om te beheren.`; @@ -58,11 +58,11 @@ export class OrgTemplateStore implements PendingSave { /** The publish impact-confirm gate (PRD §7h: show N affected letters before POST). */ readonly pendingPublish = signal(false); - readonly remoteData = computed(() => machineRemoteData(this.model())); + readonly remoteData = computed(() => fromLoadLifecycle(this.model())); private loaded = computed(() => { const s = this.model(); - return s.tag === 'loaded' ? s : null; + return s.tag === 'Loaded' ? s : null; }); readonly draft = computed(() => this.loaded()?.draft ?? null); readonly uploadState = computed(() => this.loaded()?.upload ?? initialUpload); @@ -101,7 +101,7 @@ export class OrgTemplateStore implements PendingSave { // the length guard makes it idempotent (no dispatch loop). effect(() => { const s = this.model(); - if (s.tag !== 'loaded' || s.upload.categories.length > 0) return; + if (s.tag !== 'Loaded' || s.upload.categories.length > 0) return; const status = this.categoriesRes.status(); if (status === 'resolved' || status === 'local') this.dispatchUpload({ diff --git a/apps/ssp/src/app/brief/domain/brief.machine.spec.ts b/apps/ssp/src/app/brief/domain/brief.machine.spec.ts index 5ce6a1d..1593c83 100644 --- a/apps/ssp/src/app/brief/domain/brief.machine.spec.ts +++ b/apps/ssp/src/app/brief/domain/brief.machine.spec.ts @@ -76,7 +76,7 @@ const loaded = (status: BriefStatus = { tag: 'draft' }, sections?: Brief['sectio }); const sectionBlocks = (s: BriefState, key: string) => - s.tag === 'loaded' ? s.brief.sections.find((x) => x.sectionKey === key)!.blocks : []; + s.tag === 'Loaded' ? s.brief.sections.find((x) => x.sectionKey === key)!.blocks : []; const passageIds = (s: BriefState, key: string) => sectionBlocks(s, key) @@ -92,12 +92,12 @@ describe('brief.machine reduce', () => { availablePassages: [], decisions, }).tag, - ).toBe('loaded'); + ).toBe('Loaded'); }); it('BriefLoadFailed moves loading to failed with the reason', () => { expect(reduce(initialLoading(), { tag: 'BriefLoadFailed', reason: 'x' })).toEqual({ - tag: 'failed', + tag: 'Failed', reason: 'x', }); }); @@ -210,7 +210,7 @@ describe('brief.machine reduce', () => { comments: 'graag aanpassen', }); const next = reduce(s, { tag: 'FreeTextBlockAdded', sectionKey: 'slot' }); - expect(next.tag === 'loaded' && next.brief.status.tag).toBe('draft'); + expect(next.tag === 'Loaded' && next.brief.status.tag).toBe('draft'); expect(sectionBlocks(next, 'slot')).toHaveLength(1); }); @@ -220,7 +220,7 @@ describe('brief.machine reduce', () => { // fill the required section via the besluit, then submit const filled = reduce(loaded(), besluit('positief')); const submitted = reduce(filled, { tag: 'Submitted', by: 'u1', at: 't', decisions }); - expect(submitted.tag === 'loaded' && submitted.brief.status).toEqual({ + expect(submitted.tag === 'Loaded' && submitted.brief.status).toEqual({ tag: 'submitted', submittedBy: 'u1', submittedAt: 't', @@ -232,7 +232,7 @@ describe('brief.machine reduce', () => { // approve from draft is a no-op expect(reduce(loaded(), { tag: 'Approved', by: 'u2', at: 't', decisions })).toEqual(loaded()); const approved = reduce(submitted, { tag: 'Approved', by: 'u2', at: 't2', decisions }); - expect(approved.tag === 'loaded' && approved.brief.status).toEqual({ + expect(approved.tag === 'Loaded' && approved.brief.status).toEqual({ tag: 'approved', approvedBy: 'u2', approvedAt: 't2', @@ -248,7 +248,7 @@ describe('brief.machine reduce', () => { comments: 'nee', decisions, }); - expect(rejected.tag === 'loaded' && rejected.brief.status).toEqual({ + expect(rejected.tag === 'Loaded' && rejected.brief.status).toEqual({ tag: 'rejected', rejectedBy: 'u2', rejectedAt: 't2', @@ -262,7 +262,7 @@ describe('brief.machine reduce', () => { // send from submitted is a no-op expect(reduce(submitted, { tag: 'Sent', at: 't', decisions })).toBe(submitted); const sent = reduce(approved, { tag: 'Sent', at: 't3', decisions }); - expect(sent.tag === 'loaded' && sent.brief.status).toEqual({ tag: 'sent', sentAt: 't3' }); + expect(sent.tag === 'Loaded' && sent.brief.status).toEqual({ tag: 'sent', sentAt: 't3' }); }); it('a status transition replaces decisions with the fresh server value', () => { @@ -280,10 +280,10 @@ describe('brief.machine reduce', () => { at: 't2', decisions: staleApprover, }); - expect(approved.tag === 'loaded' && approved.decisions).toEqual(staleApprover); + expect(approved.tag === 'Loaded' && approved.decisions).toEqual(staleApprover); }); }); function initialLoading(): BriefState { - return { tag: 'loading' }; + return { tag: 'Loading' }; } diff --git a/apps/ssp/src/app/brief/domain/brief.machine.ts b/apps/ssp/src/app/brief/domain/brief.machine.ts index 0821e7a..1834265 100644 --- a/apps/ssp/src/app/brief/domain/brief.machine.ts +++ b/apps/ssp/src/app/brief/domain/brief.machine.ts @@ -37,16 +37,16 @@ import { passagesForBesluit } from './besluit'; */ export type BriefState = - | { tag: 'loading' } + | { tag: 'Loading' } | { - tag: 'loaded'; + tag: 'Loaded'; brief: Brief; availablePassages: readonly LibraryPassage[]; decisions: BriefDecisions; } - | { tag: 'failed'; reason: string }; + | { tag: 'Failed'; reason: string }; -export const initial: BriefState = { tag: 'loading' }; +export const initial: BriefState = { tag: 'Loading' }; export type BriefMsg = | { @@ -110,7 +110,7 @@ function mapBlocks(brief: Brief, f: (blocks: readonly LetterBlock[]) => LetterBl /** Apply an edit to the brief, guarded by status. A rejected letter reopens to draft. */ function withEdit(s: BriefState, f: (b: Brief) => Brief): BriefState { - if (s.tag !== 'loaded' || !isEditable(s.brief.status)) return s; + if (s.tag !== 'Loaded' || !isEditable(s.brief.status)) return s; let brief = f(s.brief); if (brief.status.tag === 'rejected') brief = { ...brief, status: { tag: 'draft' } }; return { ...s, brief }; @@ -189,13 +189,13 @@ export function reduce(s: BriefState, m: BriefMsg): BriefState { switch (m.tag) { case 'BriefLoaded': return { - tag: 'loaded', + tag: 'Loaded', brief: m.brief, availablePassages: m.availablePassages, decisions: m.decisions, }; case 'BriefLoadFailed': - return { tag: 'failed', reason: m.reason }; + return { tag: 'Failed', reason: m.reason }; case 'Seed': return m.state; @@ -203,7 +203,7 @@ export function reduce(s: BriefState, m: BriefMsg): BriefState { // drafter's free text. `availablePassages` lives on the loaded state, so this stays pure. case 'BesluitSelected': return withEdit(s, (b) => - s.tag === 'loaded' && isSectionEditable(b, 'kern') + s.tag === 'Loaded' && isSectionEditable(b, 'kern') ? composeKern(b, s.availablePassages, m.besluit, m.reasons) : b, ); @@ -275,6 +275,6 @@ function transition( decisions: BriefDecisions, guard: (b: Brief) => boolean = () => true, ): BriefState { - if (s.tag !== 'loaded' || s.brief.status.tag !== from || !guard(s.brief)) return s; + if (s.tag !== 'Loaded' || s.brief.status.tag !== from || !guard(s.brief)) return s; return { ...s, brief: { ...s.brief, status: next() }, decisions }; } diff --git a/apps/ssp/src/app/brief/domain/org-template.machine.spec.ts b/apps/ssp/src/app/brief/domain/org-template.machine.spec.ts index 1b8f24b..1ceb8f8 100644 --- a/apps/ssp/src/app/brief/domain/org-template.machine.spec.ts +++ b/apps/ssp/src/app/brief/domain/org-template.machine.spec.ts @@ -26,7 +26,7 @@ const view = (over: Partial = {}): OrgTemplateAdminView => }); const loaded = (): OrgTemplateState => - reduce({ tag: 'loading' }, { tag: 'DraftLoaded', view: view() }); + reduce({ tag: 'Loading' }, { tag: 'DraftLoaded', view: view() }); const logoCategory: DocumentCategory = { categoryId: 'org-logo', @@ -41,7 +41,7 @@ const logoCategory: DocumentCategory = { describe('org-template.machine', () => { it('DraftLoaded moves to loaded with the draft, clean', () => { - const s = expectTag(loaded(), 'loaded'); + const s = expectTag(loaded(), 'Loaded'); expect(s.draft.orgName).toBe('CIBG'); expect(s.subOrgId).toBe('cibg-registers'); expect(s.unsentBriefs).toBe(2); @@ -49,14 +49,14 @@ describe('org-template.machine', () => { }); it('LoadFailed carries the reason', () => { - const s = reduce({ tag: 'loading' }, { tag: 'LoadFailed', reason: 'boom' }); - expect(s).toEqual({ tag: 'failed', reason: 'boom' }); + const s = reduce({ tag: 'Loading' }, { tag: 'LoadFailed', reason: 'boom' }); + expect(s).toEqual({ tag: 'Failed', reason: 'boom' }); }); it('FieldEdited edits the draft and marks dirty', () => { const s = expectTag( reduce(loaded(), { tag: 'FieldEdited', field: 'orgName', value: 'CIBG Nieuw' }), - 'loaded', + 'Loaded', ); expect(s.draft.orgName).toBe('CIBG Nieuw'); expect(s.dirty).toBe(true); @@ -65,7 +65,7 @@ describe('org-template.machine', () => { it('MarginEdited edits one edge and marks dirty', () => { const s = expectTag( reduce(loaded(), { tag: 'MarginEdited', edge: 'topMm', value: 40 }), - 'loaded', + 'Loaded', ); expect(s.draft.margins.topMm).toBe(40); expect(s.draft.margins.leftMm).toBe(20); @@ -75,9 +75,9 @@ describe('org-template.machine', () => { it('DraftSaved clears dirty when the saved draft is the current one', () => { const edited = expectTag( reduce(loaded(), { tag: 'FieldEdited', field: 'orgName', value: 'X' }), - 'loaded', + 'Loaded', ); - const s = expectTag(reduce(edited, { tag: 'DraftSaved', savedDraft: edited.draft }), 'loaded'); + const s = expectTag(reduce(edited, { tag: 'DraftSaved', savedDraft: edited.draft }), 'Loaded'); expect(s.dirty).toBe(false); expect(s.draft.orgName).toBe('X'); }); @@ -85,20 +85,20 @@ describe('org-template.machine', () => { it('DraftSaved keeps dirty when an edit landed during the save round-trip', () => { const editing = expectTag( reduce(loaded(), { tag: 'FieldEdited', field: 'orgName', value: 'X' }), - 'loaded', + 'Loaded', ); const savedDraft = editing.draft; // a further edit changes the draft reference before the save resolves const raced = reduce(editing, { tag: 'FieldEdited', field: 'orgName', value: 'Y' }); - const s = expectTag(reduce(raced, { tag: 'DraftSaved', savedDraft }), 'loaded'); + const s = expectTag(reduce(raced, { tag: 'DraftSaved', savedDraft }), 'Loaded'); expect(s.dirty).toBe(true); }); it('edits are no-ops in non-loaded states', () => { expect( - reduce({ tag: 'loading' }, { tag: 'FieldEdited', field: 'orgName', value: 'x' }), + reduce({ tag: 'Loading' }, { tag: 'FieldEdited', field: 'orgName', value: 'x' }), ).toEqual({ - tag: 'loading', + tag: 'Loading', }); }); @@ -122,7 +122,7 @@ describe('org-template.machine', () => { tag: 'Upload', msg: { type: 'UploadComplete', localId: 'a', documentId: 'doc-1' }, }), - 'loaded', + 'Loaded', ); expect(done.draft.logoDocumentId).toBe('doc-1'); expect(done.dirty).toBe(true); @@ -138,7 +138,7 @@ describe('org-template.machine', () => { tag: 'Upload', msg: { type: 'UploadRemoved', localId: 'a' }, }), - 'loaded', + 'Loaded', ); expect(removed.draft.logoDocumentId).toBeUndefined(); expect(removed.dirty).toBe(true); @@ -154,7 +154,7 @@ describe('org-template.machine', () => { tag: 'DraftLoaded', view: view({ draft: { ...template, subOrgId: 'cibg-vakbekwaamheid' } }), }), - 'loaded', + 'Loaded', ); expect(switched.upload.categories).toHaveLength(1); expect(switched.upload.uploads).toHaveLength(0); diff --git a/apps/ssp/src/app/brief/domain/org-template.machine.ts b/apps/ssp/src/app/brief/domain/org-template.machine.ts index 7932941..2682973 100644 --- a/apps/ssp/src/app/brief/domain/org-template.machine.ts +++ b/apps/ssp/src/app/brief/domain/org-template.machine.ts @@ -22,10 +22,10 @@ export type OrgTemplateTextField = | 'signatureClosing'; export type OrgTemplateState = - | { tag: 'loading' } - | { tag: 'failed'; reason: string } + | { tag: 'Loading' } + | { tag: 'Failed'; reason: string } | { - tag: 'loaded'; + tag: 'Loaded'; subOrgId: string; draft: OrgTemplate; publishedVersion: number; @@ -36,7 +36,7 @@ export type OrgTemplateState = upload: UploadState; }; -export const initial: OrgTemplateState = { tag: 'loading' }; +export const initial: OrgTemplateState = { tag: 'Loading' }; export type OrgTemplateMsg = | { tag: 'Loading' } @@ -51,18 +51,18 @@ export type OrgTemplateMsg = /** Edit the loaded draft; a no-op in any non-loaded state (illegal by construction). */ function editDraft(s: OrgTemplateState, f: (draft: OrgTemplate) => OrgTemplate): OrgTemplateState { - return s.tag === 'loaded' ? { ...s, draft: f(s.draft), dirty: true } : s; + return s.tag === 'Loaded' ? { ...s, draft: f(s.draft), dirty: true } : s; } export function reduce(s: OrgTemplateState, m: OrgTemplateMsg): OrgTemplateState { switch (m.tag) { case 'Loading': - return { tag: 'loading' }; + return { tag: 'Loading' }; case 'LoadFailed': - return { tag: 'failed', reason: m.reason }; + return { tag: 'Failed', reason: m.reason }; case 'DraftLoaded': return { - tag: 'loaded', + tag: 'Loaded', subOrgId: m.view.draft.subOrgId, draft: m.view.draft, publishedVersion: m.view.publishedVersion, @@ -71,16 +71,16 @@ export function reduce(s: OrgTemplateState, m: OrgTemplateMsg): OrgTemplateState dirty: false, // Keep the loaded logo category across sub-org switches (it's the same // `org-logo` category, loaded once); drop only any in-flight/finished uploads. - upload: s.tag === 'loaded' ? { ...s.upload, uploads: [], rejections: {} } : initialUpload, + upload: s.tag === 'Loaded' ? { ...s.upload, uploads: [], rejections: {} } : initialUpload, }; case 'FieldEdited': return editDraft(s, (d) => ({ ...d, [m.field]: m.value })); case 'MarginEdited': return editDraft(s, (d) => ({ ...d, margins: { ...d.margins, [m.edge]: m.value } })); case 'DraftSaved': - return s.tag === 'loaded' && s.draft === m.savedDraft ? { ...s, dirty: false } : s; + return s.tag === 'Loaded' && s.draft === m.savedDraft ? { ...s, dirty: false } : s; case 'Upload': { - if (s.tag !== 'loaded') return s; + if (s.tag !== 'Loaded') return s; const upload = reduceUpload(s.upload, m.msg); // A completed/removed logo upload also updates the draft's logoDocumentId. if (m.msg.type === 'UploadComplete') diff --git a/apps/ssp/src/app/brief/ui/brief.page.ts b/apps/ssp/src/app/brief/ui/brief.page.ts index 3022fd7..b5f1a48 100644 --- a/apps/ssp/src/app/brief/ui/brief.page.ts +++ b/apps/ssp/src/app/brief/ui/brief.page.ts @@ -172,7 +172,7 @@ export class BriefPage { Success value is unwrapped here instead of through `let-`. */ protected readonly loaded = computed(() => { const s = this.model(); - return s.tag === 'loaded' ? s : undefined; + return s.tag === 'Loaded' ? s : undefined; }); protected reload() { diff --git a/docs/project/readable-codebase/RD-11-fold-load-lifecycle.md b/docs/project/readable-codebase/RD-11-fold-load-lifecycle.md new file mode 100644 index 0000000..aa1495d --- /dev/null +++ b/docs/project/readable-codebase/RD-11-fold-load-lifecycle.md @@ -0,0 +1,171 @@ +# RD-11 — Fold the lifecycle projection into `remote-data.ts`, and PascalCase the 3 machines + +Status: done +Source: PLAN.md 1b#3 + +## Why + +`machine-remote-data.ts` is 24 lines defining a **third** encoding of "in flight / ok / +failed": `LoadLifecycle = { tag: 'loading' } | { tag: 'failed'; reason } | { tag: 'loaded' }`. +It has three call sites, all the identical line, and `LoadLifecycle` is never imported by +name anywhere — it is a purely structural constraint. + +That constraint is the **only** reason three machines carry lowercase state tags while their +message tags are PascalCase in the same file. `stamdata-editor.machine.spec.ts:61` shows the +confusion in one line today: + +```ts +expect(reduce(seedLoaded(), { tag: 'Loading' })).toEqual({ tag: 'loading' }); +``` + +A PascalCase message producing a lowercase state. Relocate the projection with PascalCase +keys and the dialect drift resolves itself — no separate renaming pass, and one named concept +disappears. + +## Read first + +- `libs/shared/src/application/machine-remote-data.ts` — all 24 lines +- `libs/shared/src/application/machine-remote-data.spec.ts` — 20 lines, to be merged +- `libs/shared/src/application/remote-data.ts` — note `fromResource`, the neighbour and + precedent for the new function +- `libs/shared/docs/remote-data.mdx:72` — teaches `s.tag === 'loaded'`, so it must change too +- `apps/ssp/src/app/brief/domain/brief.ts:68` — **`BriefStatus`. Read this before renaming + anything.** See decision 4. + +## Decisions (pre-made, don't relitigate) + +1. **Relocate, do not simply delete.** The mapping has to exist somewhere, because + `` takes a `RemoteData`. Deleting the module re-inlines a 6-line switch in three + stores, recreating the duplication WP-31 removed. Move it into `remote-data.ts` as + `fromLoadLifecycle`, beside `fromResource`, where it reads as what it is: **a `RemoteData` + constructor, not a sixth encoding.** Keep the `Extract` Success + payload so all three call sites stay one line. + +2. **Key it PascalCase**: `Loading | Failed{reason} | Loaded`. Merge + `machine-remote-data.spec.ts` into `remote-data.spec.ts` and delete both old files. + +3. **Rename only the three load-lifecycle tags, and catch all four syntactic forms.** Measured + counts of the construction form alone (39 across 11 files) understate it. The forms are: + + | Form | Example | + | ------------- | ---------------------------------------------- | + | construction | `tag: 'loading'` | + | comparison | `s.tag === 'loaded'`, `s.tag !== 'loaded'` | + | type-level | `Extract` | + | documentation | `remote-data.mdx:72` | + + Files in scope: the three machines (`brief.machine.ts`, `org-template.machine.ts`, + `stamdata-editor.machine.ts`), their three specs, `brief.store.ts`, `brief.store.spec.ts`, + `org-template.store.ts`, `stamdata.store.ts`, `brief.page.ts`, and `remote-data.mdx`. + +4. **`BriefStatus` IS NOT IN SCOPE. This is the one way to break this ticket.** + `brief.machine.ts` contains **two** independent lowercase tag families: + - `BriefState`'s load lifecycle — `loading`/`failed`/`loaded` — **rename these** + - `BriefStatus`'s letter status — `draft`/`submitted`/`approved`/`rejected`/`sent`, defined + in `brief.ts:68` — **leave these alone** + + `brief.machine.ts:278` has both in one line: + `if (s.tag !== 'loaded' || s.brief.status.tag !== from …)`. The first is in scope, the + second is not. `BriefStatus` is parsed off the wire from `BriefViewDto`, so renaming its + tags breaks the parse boundary and the backend contract. **Never rename by "all lowercase + tags in this file".** + +5. **Three more collision sites must not be touched.** They use the same words for unrelated + things, and anchoring on `tag: '` already excludes them — but verify rather than assume: + - `scenario.ts` / `scenario.interceptor.ts` — `'loading'` is a `?scenario=` **URL param + value**, not a state tag + - `upload.machine.ts` and the four upload UI components — `UploadStatus` discriminates on + **`type:`**, not `tag:`, with `'failed'`/`'complete'`/`'uploading'` + - `registratie-lookup.store.ts` — `'loading'` is an Angular `resource()` status + +6. **Do not touch `ActionState`, `SaveState`, or `pendingPublish`.** RD-12, RD-13 and RD-14 + own those, and they must follow this ticket or the same tags get renamed twice. + +## Files + +Add to / edit: `libs/shared/src/application/remote-data.ts` (+ `.spec.ts`), +`libs/shared/docs/remote-data.mdx`. +Delete: `libs/shared/src/application/machine-remote-data.ts` (+ `.spec.ts`). +Rename tags in: `brief.machine.ts` (+ spec), `org-template.machine.ts` (+ spec), +`stamdata-editor.machine.ts` (+ spec), `brief.store.ts` (+ spec), `org-template.store.ts`, +`stamdata.store.ts`, `brief.page.ts`. + +## Steps + +1. Add `fromLoadLifecycle` to `remote-data.ts` with PascalCase keys (decisions 1-2). +2. Rename the load-lifecycle tags across the files in decision 3, one file at a time, letting + the type-checker find the next site. **Do not blanket-sed.** +3. Point the three stores at `fromLoadLifecycle`; delete `machine-remote-data.ts` and merge + its spec cases into `remote-data.spec.ts`. +4. Update `remote-data.mdx:72`. +5. Run `npm run gen:behaviour-spec` — `behaviour-spec.mdx:839` has a `machineRemoteData` + section that must become the new name. +6. Update this ticket's `Status:` to `done` and the README's RD-11 row to `done`. +7. Commit all of it together. + +## Acceptance criteria + +The third encoding is gone and nothing lowercase survives in the three machines: + +```bash +git grep -n "machineRemoteData\|LoadLifecycle" -- apps libs # MUST return nothing +ls libs/shared/src/application/machine-remote-data* # MUST be "No such file" + +M="apps/ssp/src/app/brief/domain apps/ssp/src/app/brief/application \ + apps/ssp/src/app/brief/ui libs/beheer/src/domain libs/beheer/src/application" +git grep -n "tag: 'loading'\|tag: 'failed'\|tag: 'loaded'" -- $M # MUST return nothing +git grep -n "tag === 'loaded'\|tag !== 'loaded'" -- $M # MUST return nothing +``` + +`BriefStatus` is untouched — this is the check that matters most (decision 4): + +```bash +# Measured before this ticket was written: the total is exactly 54. It MUST still be 54. +git grep -c "tag: 'draft'\|tag: 'submitted'\|tag: 'approved'\|tag: 'rejected'\|tag: 'sent'" \ + -- apps/ssp/src/app/brief | awk -F: '{s+=$NF} END {print s}' # MUST print 54 + +git diff --stat apps/ssp/src/app/brief/domain/brief.ts # MUST be empty — brief.ts unchanged +``` + +If that number moves, you have renamed a wire contract. Stop and revert rather than adjusting +the number. + +The collision sites are untouched: + +```bash +git diff --name-only | git grep -c "scenario\|upload" || true # expect no such files +``` + +```bash +npm run ci # exits 0 +``` + +## Verification + +`npm run ci`. Also run `npm run ci -- --full` **with an explicit long timeout**: this edits +`remote-data.mdx`, which Storybook globs, and a broken MDX import is invisible to plain `ci`. + +Note for whoever runs it: an 8-minute command cannot complete in the default 120s foreground +window and the harness will move it to the background. Pass `timeout: 600000` on the Bash +call so it runs to completion in the foreground, then commit. + +## Out of scope + +- `ActionState` / `SaveState` / `pendingPublish` — RD-12, RD-13, RD-14 (decision 6). +- `BriefStatus` (decision 4). If a `BriefStatus` tag changes, the ticket has failed. +- `UploadStatus`'s `type:` discriminant — optional RD-35. +- The `NO_SUBORGS`/`NO_TABLES`-should-be-`Empty` finding — optional RD-34. + +## Risks + +- **`BriefStatus` (decision 4) is the failure mode to fear.** Its tags are a wire contract. + Rename by union, never by file. +- **Do not blanket-sed `'loading'`/`'failed'`/`'loaded'`.** Five files legitimately use those + words for other purposes (decision 5). Renaming one file at a time and following the + type-checker is slower and correct. +- **`brief.store.ts` and `brief.page.ts` use only the comparison form**, so a + construction-only grep misses them. That is why decision 3 lists four forms. +- **`behaviour-spec.mdx` drift**: it has a `machineRemoteData` section heading at :839 which + changes with the function name. Run `gen:behaviour-spec` in the same commit. +- **`remote-data.ts` carries a `// #region showcase:fold` marker at :30.** If your edit moves + or splits that region, run `npm run gen:snippets` in the same commit too. diff --git a/docs/project/readable-codebase/README.md b/docs/project/readable-codebase/README.md index 56482a1..b2447fd 100644 --- a/docs/project/readable-codebase/README.md +++ b/docs/project/readable-codebase/README.md @@ -105,7 +105,7 @@ two. Note that RD-15 exists because 22 abandoned agent worktrees are still on di | RD-08 | Migrate the 3 wizards to the effect map + `Primary` | 07 | yes | done | | RD-09 | Teach the effect map: ARCHITECTURE §2d + fp-tea (2 docs, no generator) | 08 | | done | | RD-10 | `WizardStatus` to a payload-carrying `WizardPhase` | 08 | yes | done | -| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | todo | +| RD-11 | Fold the lifecycle projection into `remote-data.ts`; PascalCase 3 machines | 01 | | done | | RD-12 | `ActionState` becomes `action` on `BriefState.Loaded` | 11 | | todo | | RD-13 | Same for org-template, folding `pendingPublish` in | 12 | | todo | | RD-14 | Move `SaveState` to `debounced-save.ts`; delete `action-state.ts` | 13 | | todo | diff --git a/libs/beheer/src/application/stamdata.store.ts b/libs/beheer/src/application/stamdata.store.ts index 0574b13..2ef9343 100644 --- a/libs/beheer/src/application/stamdata.store.ts +++ b/libs/beheer/src/application/stamdata.store.ts @@ -1,6 +1,6 @@ import { Injectable, computed, inject, signal } from '@angular/core'; import { createStore } from '@shared/application/store'; -import { machineRemoteData } from '@shared/application/machine-remote-data'; +import { fromLoadLifecycle } from '@shared/application/remote-data'; import { createHistory } from '@shared/application/history'; import { ChangeCounts, @@ -20,7 +20,7 @@ import { import { StamdataAdapter } from '@beheer/infrastructure/stamdata.adapter'; import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; -type LoadedState = Extract; +type LoadedState = Extract; /** * Root singleton for the stamdata maintenance editor (ADR-0004). The Elm machine owns the @@ -42,11 +42,11 @@ export class StamdataStore { so toggling it never round-trips or drops unsaved edits (see domain `activeOn`). */ readonly previewDate = signal(''); - readonly remoteData = computed(() => machineRemoteData(this.model())); + readonly remoteData = computed(() => fromLoadLifecycle(this.model())); private loaded = computed(() => { const s = this.model(); - return s.tag === 'loaded' ? s : null; + return s.tag === 'Loaded' ? s : null; }); readonly table = computed(() => this.loaded()?.table ?? null); readonly rows = computed(() => this.loaded()?.rows ?? []); diff --git a/libs/beheer/src/domain/stamdata-editor.machine.spec.ts b/libs/beheer/src/domain/stamdata-editor.machine.spec.ts index f6cf4af..ba4bd9d 100644 --- a/libs/beheer/src/domain/stamdata-editor.machine.spec.ts +++ b/libs/beheer/src/domain/stamdata-editor.machine.spec.ts @@ -24,40 +24,40 @@ const seedLoaded = (): StamdataEditorState => describe('stamdata-editor reduce', () => { it('Loaded snapshots original independently of rows', () => { const s = seedLoaded(); - expect(s.tag).toBe('loaded'); - if (s.tag !== 'loaded') return; + expect(s.tag).toBe('Loaded'); + if (s.tag !== 'Loaded') return; const edited = reduce(s, { tag: 'CellEdited', row: 0, column: 'beroep', value: 'Chirurg' }); - if (edited.tag !== 'loaded') return; + if (edited.tag !== 'Loaded') return; expect(edited.rows[0]['beroep']).toBe('Chirurg'); expect(edited.original[0]['beroep']).toBe('Arts'); // snapshot untouched → diff works }); it('RowAdded appends an empty row shaped by the schema', () => { const s = reduce(seedLoaded(), { tag: 'RowAdded' }); - if (s.tag !== 'loaded') return; + if (s.tag !== 'Loaded') return; expect(s.rows).toHaveLength(2); expect(s.rows[1]).toEqual({ program: '', beroep: '', geldigVan: '', geldigTot: '' }); }); it('RowRemoved drops the row at the index', () => { const s = reduce(reduce(seedLoaded(), { tag: 'RowAdded' }), { tag: 'RowRemoved', row: 0 }); - if (s.tag !== 'loaded') return; + if (s.tag !== 'Loaded') return; expect(s.rows).toHaveLength(1); }); it('edit messages are ignored unless loaded', () => { expect(reduce(initial, { tag: 'RowAdded' })).toBe(initial); expect( - reduce({ tag: 'failed', reason: 'x' }, { tag: 'CellEdited', row: 0, column: 'a', value: 'b' }) + reduce({ tag: 'Failed', reason: 'x' }, { tag: 'CellEdited', row: 0, column: 'a', value: 'b' }) .tag, - ).toBe('failed'); + ).toBe('Failed'); }); it('LoadFailed and Loading transition regardless of prior state', () => { expect(reduce(seedLoaded(), { tag: 'LoadFailed', reason: 'boom' })).toEqual({ - tag: 'failed', + tag: 'Failed', reason: 'boom', }); - expect(reduce(seedLoaded(), { tag: 'Loading' })).toEqual({ tag: 'loading' }); + expect(reduce(seedLoaded(), { tag: 'Loading' })).toEqual({ tag: 'Loading' }); }); }); diff --git a/libs/beheer/src/domain/stamdata-editor.machine.ts b/libs/beheer/src/domain/stamdata-editor.machine.ts index 524808a..f908dbf 100644 --- a/libs/beheer/src/domain/stamdata-editor.machine.ts +++ b/libs/beheer/src/domain/stamdata-editor.machine.ts @@ -11,9 +11,9 @@ import { StamRow, StamTable, emptyRow } from '@beheer/domain/stamdata'; * apply path is a reviewed PR, not a runtime write — ADR-0004). */ export type StamdataEditorState = - | { tag: 'loading' } - | { tag: 'failed'; reason: string } - | { tag: 'loaded'; table: StamTable; rows: StamRow[]; original: readonly StamRow[] }; + | { tag: 'Loading' } + | { tag: 'Failed'; reason: string } + | { tag: 'Loaded'; table: StamTable; rows: StamRow[]; original: readonly StamRow[] }; export type StamdataEditorMsg = | { tag: 'Loading' } @@ -24,29 +24,29 @@ export type StamdataEditorMsg = | { tag: 'RowRemoved'; row: number } | { tag: 'Seed'; state: StamdataEditorState }; // mount a specific state (stories/tests) -export const initial: StamdataEditorState = { tag: 'loading' }; +export const initial: StamdataEditorState = { tag: 'Loading' }; const copy = (rows: readonly StamRow[]): StamRow[] => rows.map((r) => ({ ...r })); export function reduce(s: StamdataEditorState, m: StamdataEditorMsg): StamdataEditorState { switch (m.tag) { case 'Loading': - return { tag: 'loading' }; + return { tag: 'Loading' }; case 'Loaded': // original is an independent snapshot so later edits never mutate it (drives the diff). - return { tag: 'loaded', table: m.table, rows: copy(m.rows), original: copy(m.rows) }; + return { tag: 'Loaded', table: m.table, rows: copy(m.rows), original: copy(m.rows) }; case 'LoadFailed': - return { tag: 'failed', reason: m.reason }; + return { tag: 'Failed', reason: m.reason }; case 'CellEdited': - if (s.tag !== 'loaded') return s; + if (s.tag !== 'Loaded') return s; return { ...s, rows: s.rows.map((r, i) => (i === m.row ? { ...r, [m.column]: m.value } : r)), }; case 'RowAdded': - return s.tag === 'loaded' ? { ...s, rows: [...s.rows, emptyRow(s.table)] } : s; + return s.tag === 'Loaded' ? { ...s, rows: [...s.rows, emptyRow(s.table)] } : s; case 'RowRemoved': - return s.tag === 'loaded' ? { ...s, rows: s.rows.filter((_, i) => i !== m.row) } : s; + return s.tag === 'Loaded' ? { ...s, rows: s.rows.filter((_, i) => i !== m.row) } : s; case 'Seed': return m.state; default: diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 2afe646..9b596e3 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -819,6 +819,12 @@ classes. - is empty-safe: undefined, null, and empty string all yield the empty string - returns empty for an unparseable string rather than "Invalid Date" +#### fromLoadLifecycle + +- maps Loading → Loading +- maps Failed → Failure carrying an Error with the reason +- maps Loaded → Success carrying the whole loaded state + #### httpClientFetch - sends the pending idempotency key as a header for a write, not a fresh one per attempt @@ -836,12 +842,6 @@ classes. - keeps query + hash on both targets - the root maps nl → / and en → /en/ -#### machineRemoteData - -- maps loading → Loading -- maps failed → Failure carrying an Error with the reason -- maps loaded → Success carrying the whole loaded state - #### parseBsn (elfproef) - accepts a valid BSN (passes the elfproef) diff --git a/libs/shared/docs/remote-data.mdx b/libs/shared/docs/remote-data.mdx index 8e65744..5530db2 100644 --- a/libs/shared/docs/remote-data.mdx +++ b/libs/shared/docs/remote-data.mdx @@ -69,7 +69,7 @@ The idiom this repo uses instead — see `brief.page.ts`, `dashboard.page.ts`, // in the component class protected readonly loaded = computed(() => { const s = this.model(); // or store.someRemoteData() - return s.tag === 'loaded' ? s : undefined; + return s.tag === 'Loaded' ? s : undefined; }); ``` @@ -94,8 +94,8 @@ timing/outcome of `/api/*` calls. Try it on `/brief` or `/dashboard`. A store's own state machine (its `*.machine.ts`) should own the **domain** lifecycle of what it holds (draft → submitted → approved, in the brief's case) — not the network fetch's loading/failure, which is a generic concern `RemoteData` already models. Where a -machine's own `loading`/`failed` tags purely mirror the fetch (nothing extra beyond "not -loaded yet" / "the GET failed"), project them onto a `RemoteData` computed at the store -layer for `` to render, the way `BriefStore.remoteData` does — the machine +machine's own `Loading`/`Failed`/`Loaded` tags purely mirror the fetch (nothing extra +beyond "not loaded yet" / "the GET failed"), project them with `fromLoadLifecycle` at the +store layer for `` to render, the way `BriefStore.remoteData` does — the machine keeps deciding what the _letter_ is doing, `RemoteData` keeps deciding what the _fetch_ is doing. diff --git a/libs/shared/src/application/machine-remote-data.spec.ts b/libs/shared/src/application/machine-remote-data.spec.ts deleted file mode 100644 index b0c11fd..0000000 --- a/libs/shared/src/application/machine-remote-data.spec.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, it, expect } from 'vitest'; -import { machineRemoteData } from './machine-remote-data'; -import { loading, success } from '../testing/remote-data'; - -describe('machineRemoteData', () => { - it('maps loading → Loading', () => { - expect(machineRemoteData({ tag: 'loading' })).toEqual(loading()); - }); - - it('maps failed → Failure carrying an Error with the reason', () => { - const rd = machineRemoteData({ tag: 'failed', reason: 'boom' }); - expect(rd.tag).toBe('Failure'); - if (rd.tag === 'Failure') expect(rd.error.message).toBe('boom'); - }); - - it('maps loaded → Success carrying the whole loaded state', () => { - const loaded = { tag: 'loaded', foo: 42 } as const; - expect(machineRemoteData(loaded)).toEqual(success(loaded)); - }); -}); diff --git a/libs/shared/src/application/machine-remote-data.ts b/libs/shared/src/application/machine-remote-data.ts deleted file mode 100644 index 8b1c3cc..0000000 --- a/libs/shared/src/application/machine-remote-data.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { RemoteData } from '@shared/application/remote-data'; - -/** The standard load-lifecycle tags an editor machine exposes. */ -export type LoadLifecycle = - { tag: 'loading' } | { tag: 'failed'; reason: string } | { tag: 'loaded' }; - -/** - * Project an Elm-machine state onto `RemoteData` for the `` seam. The machine - * keeps owning its own domain lifecycle (draft/submitted/…); this is purely the - * loading/failed/loaded → async mapping, which was byte-identical across BriefStore, - * OrgTemplateStore and StamdataStore (WP-31). Wrap the call in a `computed`. - */ -export function machineRemoteData( - s: S, -): RemoteData> { - switch (s.tag) { - case 'loading': - return { tag: 'Loading' }; - case 'failed': - return { tag: 'Failure', error: new Error(s.reason) }; - default: // 'loaded' - return { tag: 'Success', value: s as Extract }; - } -} diff --git a/libs/shared/src/application/remote-data.spec.ts b/libs/shared/src/application/remote-data.spec.ts index 45e2d05..323ddbd 100644 --- a/libs/shared/src/application/remote-data.spec.ts +++ b/libs/shared/src/application/remote-data.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { RemoteData, map2, map, successOf } from './remote-data'; +import { RemoteData, fromLoadLifecycle, map2, map, successOf } from './remote-data'; import { loading, failure, empty, success } from '../testing/remote-data'; const loadingRd: RemoteData = loading(); @@ -33,3 +33,20 @@ describe('successOf', () => { expect(successOf(empty())).toBeUndefined(); }); }); + +describe('fromLoadLifecycle', () => { + it('maps Loading → Loading', () => { + expect(fromLoadLifecycle({ tag: 'Loading' })).toEqual(loading()); + }); + + it('maps Failed → Failure carrying an Error with the reason', () => { + const rd = fromLoadLifecycle({ tag: 'Failed', reason: 'boom' }); + expect(rd.tag).toBe('Failure'); + if (rd.tag === 'Failure') expect(rd.error.message).toBe('boom'); + }); + + it('maps Loaded → Success carrying the whole loaded state', () => { + const loadedState = { tag: 'Loaded', foo: 42 } as const; + expect(fromLoadLifecycle(loadedState)).toEqual(success(loadedState)); + }); +}); diff --git a/libs/shared/src/application/remote-data.ts b/libs/shared/src/application/remote-data.ts index 2b9b4c1..6f35d15 100644 --- a/libs/shared/src/application/remote-data.ts +++ b/libs/shared/src/application/remote-data.ts @@ -27,6 +27,26 @@ export function fromResource( return { tag: 'Loading' }; } +/** + * Project an Elm-machine's load lifecycle onto `RemoteData`, for the `` seam. The + * machine keeps owning its own domain lifecycle (draft/submitted/…); this is purely the + * Loading/Failed/Loaded → async mapping, which was byte-identical across BriefStore, + * OrgTemplateStore and StamdataStore (WP-31). A `RemoteData` constructor, not a sixth + * encoding — wrap the call in a `computed`. + */ +export function fromLoadLifecycle< + S extends { tag: 'Loading' } | { tag: 'Failed'; reason: string } | { tag: 'Loaded' }, +>(s: S): RemoteData> { + switch (s.tag) { + case 'Loading': + return { tag: 'Loading' }; + case 'Failed': + return { tag: 'Failure', error: new Error(s.reason) }; + default: // 'Loaded' + return { tag: 'Success', value: s as Extract }; + } +} + // #region showcase:fold /** Exhaustive fold: you must handle every case, checked at compile time. */ export function foldRemote(