atomic-design-poc/docs/backlog/WP-07-brief-idioms.md

WP-07 — Brief on the shared idioms + RemoteData MDX

Status: done Phase: 1 — FP/DDD core Depends on: WP-06 (typed <app-async>)

Why

The brief context drifted from the repo's own reflexes: brief.store.ts (~lines 22-25) holds busy: signal<boolean> + lastError: signal<string|null> + a separate saveState union side by side — representable illegal combos, the exact "second boolean" smell CLAUDE.md §3 bans. And brief.page.ts hand-renders its load lifecycle with @switch + spinner/alert instead of RemoteData + <app-async> — the only async flow in the app bypassing the shared molecule.

Read first

• CLAUDE.md §3; src/app/shared/application/remote-data.ts
• src/app/brief/application/brief.store.ts, src/app/brief/ui/brief.page.ts, src/app/brief/domain/brief.machine.ts (+ spec)
• src/app/registratie/application/applications.store.ts (a store doing it right)

Decisions (pre-made, don't relitigate)

• Transient submit/save state becomes one tagged union (Idle | Busy | Failed{error}), replacing busy+lastError. saveState keeps its union shape (align tag style).
• Load lifecycle → RemoteData + <app-async>; the machine keeps owning the letter's domain lifecycle (loading tags move out of the machine only if they purely mirror the fetch — keep the seam: RemoteData = fetch, machine = letter).
• Keep the debounced-save sequencing identical; only re-type the state.

Files

• src/app/brief/application/brief.store.ts
• src/app/brief/ui/brief.page.ts
• src/app/brief/domain/brief.machine.ts + brief.machine.spec.ts
• New src/docs/remote-data.mdx — title Foundations/RemoteData & Async

Steps

1. Replace the signal trio with one union signal; update consumers (letter-composer bar, autosave status line).
2. Route the page's load through RemoteData + <app-async> (typed via WP-06); wire the existing loading/empty/failure templates.
3. Update machine/store specs for the union transitions.
4. MDX page: the four states, map2/andThen, the delay-gated spinner, and the ?scenario= dev toggle — linking brief.page.ts and dashboard.page.ts as live examples.

Acceptance criteria

• No boolean-plus-error signal pairs in brief/.
• /brief renders all four async states (checked with ?scenario=slow|error; see Deviation for why empty isn't meaningful here).
• Specs cover the transition union (Busy→Failed, Busy→Idle) — brief.store.spec.ts (new).
• MDX renders under Foundations.

Verification

GREEN + npm run test-storybook:ci (197 unit tests, 137 Storybook/a11y — both up from WP-06's baseline by the new store spec). Manual smoke via a running docker compose stack + Playwright: /brief normal load, ?scenario=slow (spinner), ?scenario=error (failure alert + working retry), and /brief?role=approver — all with no console errors.

Deviation from the original plan

The machine's loading/failed tags were NOT moved out of BriefState. The Decisions block hedges this ("only if they purely mirror the fetch") — they do, but removing them turns out to need more than a re-type: createStore(initial, reduce) requires a concrete initial: BriefState value, and once loading/failed are gone there is no state left to represent "not loaded yet" without inventing a second wrapping layer (the store's top-level signal would need to become RemoteData<Err, LoadedState> directly, with the machine's reduce only invoked inside the Success branch — a different wiring shape from every other machine in the app, and a ~250-line ripple through brief.machine.spec.ts). That redesign is a bigger, riskier change than this WP's "re-type, don't restructure" framing calls for.

Instead, BriefStore.remoteData projects the existing machine model onto RemoteData<Error | undefined, LoadedBriefState> (loading→Loading, failed→ Failure, loaded→Success), and brief.page.ts renders that projection through <app-async>. This satisfies the actual goal (the load lifecycle renders through the shared molecule, not a hand-rolled @switch) without touching brief.machine.ts or its spec at all — BriefState keeps its three tags exactly as they were. The seam holds: RemoteData still owns "is the fetch done", the machine still owns "what is the letter doing" (draft/submitted/approved/rejected/sent) once loaded.

?scenario=empty doesn't apply to /brief. It rewrites the HTTP body to [], which fails parseBriefView's !dto.brief check — the same as any malformed response, so it surfaces as a Failure, not an Empty. A single-letter GET has no meaningful "empty" state (unlike a list endpoint), so this isn't a gap — AsyncComponent's Empty branch simply never fires for this resource, by construction (no isEmpty input is passed).

Reused the WP-06 fallback for the loaded slot. <ng-template appAsyncLoaded> can't type let-s to the loaded value for the same structural reason WP-06 documented (a directive's generic can't inherit from a sibling [data] input) — brief.page.ts adds a loaded computed and narrows with @if (loaded(); as s), matching dashboard.page.ts/registration-detail.page.ts.

Out of scope

Brief component stories (WP-15); machine renaming conventions (WP-08).

Risks

Autosave (debounced) interplay with the new transition union — flush ordering must stay as-is; brief.store.spec.ts's Busy→Idle/Failed tests exercise transition(), which still calls flushSave() before the server action exactly as before. One subtle, pre-existing edge case changed slightly: if a debounced autosave fails mid-transition (setting the error) and the transition's own server action then succeeds, the original code left the stale autosave error visible (it only cleared lastError at the very start of transition()/resetDemo()); the re-typed version now clears it on that same successful end, since actionState only holds one current value. Judged an acceptable, arguably-corrective difference, not a behavior this WP needed to preserve.