feat(fp): WP-07 — brief on the shared idioms + RemoteData MDX

Collapse brief.store's busy signal + nullable lastError into one Idle |
Busy | Failed union (saveState gets matching tag-object style), and route
brief.page's load through RemoteData + <app-async> instead of a hand-rolled
@switch, via a BriefStore.remoteData projection of the machine's existing
loading/failed tags -- the machine keeps owning the letter's own status
lifecycle untouched. New brief.store.spec.ts covers the Busy->Idle/Failed
transitions; new Foundations/RemoteData & Async MDX page documents the
pattern and the WP-06 typed-loaded-slot fallback. Deviation from the
original plan recorded in the WP file.
This commit is contained in:
2026-07-03 21:39:29 +02:00
parent 199cbe1f8c
commit e3cd908f4f
7 changed files with 1997 additions and 1610 deletions

View File

@@ -46,7 +46,7 @@ for its existing violations, so every WP ends green.
| [WP-04](WP-04-ui-not-infrastructure.md) | Boundaries II: `ui ↛ infrastructure` + showcase sanction | 0 · gates | done |
| [WP-05](WP-05-parse-boundaries.md) | Parse-don't-validate closure + MDX | 1 · FP/DDD | done |
| [WP-06](WP-06-typed-async.md) | Generic async template contexts — kill `$any()` | 1 · FP/DDD | done |
| [WP-07](WP-07-brief-idioms.md) | Brief on the shared idioms + RemoteData MDX | 1 · FP/DDD | todo |
| [WP-07](WP-07-brief-idioms.md) | Brief on the shared idioms + RemoteData MDX | 1 · FP/DDD | done |
| [WP-08](WP-08-store-idiom.md) | One store idiom + machine naming + TEA MDX | 1 · FP/DDD | todo |
| [WP-09](WP-09-pure-logic.md) | Pure-logic closure: dates + missing command specs | 1 · FP/DDD | todo |
| [WP-10](WP-10-button-fidelity.md) | CIBG button fidelity | 2 · CIBG | todo |

View File

@@ -1,6 +1,6 @@
# WP-07 — Brief on the shared idioms + RemoteData MDX
Status: todo
Status: done
Phase: 1 — FP/DDD core
Depends on: WP-06 (typed `<app-async>`)
@@ -50,15 +50,53 @@ bypassing the shared molecule.
## Acceptance criteria
- [ ] No boolean-plus-error signal pairs in `brief/`.
- [ ] `/brief` renders all four async states (check with `?scenario=slow|empty|error`).
- [ ] Specs cover the transition union (Busy→Failed, Busy→Idle).
- [ ] MDX renders under Foundations.
- [x] No boolean-plus-error signal pairs in `brief/`.
- [x] `/brief` renders all four async states (checked with `?scenario=slow|error`; see
Deviation for why `empty` isn't meaningful here).
- [x] Specs cover the transition union (Busy→Failed, Busy→Idle) — `brief.store.spec.ts`
(new).
- [x] MDX renders under Foundations.
## Verification
GREEN + `npm run test-storybook:ci`. Manual: `npm start``/brief` with
`?scenario=slow`, `?scenario=error`; exercise autosave + submit + rejection flow.
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
@@ -67,4 +105,11 @@ 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; the machine spec pins it.
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.

File diff suppressed because one or more lines are too long

View File

@@ -0,0 +1,88 @@
import { TestBed } from '@angular/core/testing';
import { describe, it, expect } from 'vitest';
import { Result } from '@shared/kernel/fp';
import { Brief, BriefDecisions } from '@brief/domain/brief';
import { BriefAdapter, BriefView } from '@brief/infrastructure/brief.adapter';
import { BriefStore } from './brief.store';
const decisions: BriefDecisions = {
canEdit: true,
canApprove: true,
canReject: true,
canSend: true,
};
const brief: Brief = {
briefId: 'b1',
beroep: 'arts',
templateId: 't1',
placeholders: [],
sections: [],
status: { tag: 'draft' },
drafterId: 'u1',
};
const view: BriefView = { brief, availablePassages: [], decisions };
function setup(adapter: Partial<BriefAdapter>): BriefStore {
TestBed.configureTestingModule({ providers: [{ provide: BriefAdapter, useValue: adapter }] });
return TestBed.inject(BriefStore);
}
describe('BriefStore action state (Idle | Busy | Failed)', () => {
it('is Busy synchronously once a transition starts, then Idle on success', async () => {
const approved: BriefView = {
...view,
brief: { ...brief, status: { tag: 'approved', approvedBy: 'u2', approvedAt: 't' } },
};
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
save: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
approve: (): Promise<Result<string, BriefView>> =>
Promise.resolve({ ok: true, value: approved }),
});
await store.load();
const pending = store.approve();
expect(store.busy()).toBe(true); // set synchronously, before any await resolves
await pending;
expect(store.busy()).toBe(false);
expect(store.lastError()).toBeNull();
});
it('goes Busy then Failed on a failing transition, surfacing the error', async () => {
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
save: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
approve: (): Promise<Result<string, BriefView>> =>
Promise.resolve({ ok: false, error: 'niet toegestaan' }),
});
await store.load();
await store.approve();
expect(store.busy()).toBe(false);
expect(store.lastError()).toBe('niet toegestaan');
});
it('a subsequent successful transition clears a prior Failed state', async () => {
let approveResult: Result<string, BriefView> = { ok: false, error: 'eerste poging mislukt' };
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
save: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
approve: (): Promise<Result<string, BriefView>> => Promise.resolve(approveResult),
});
await store.load();
await store.approve();
expect(store.lastError()).toBe('eerste poging mislukt');
approveResult = {
ok: true,
value: { ...view, brief: { ...brief, status: { tag: 'approved', approvedBy: 'u2', approvedAt: 't' } } },
};
await store.approve();
expect(store.busy()).toBe(false);
expect(store.lastError()).toBeNull();
});
});

View File

@@ -1,5 +1,6 @@
import { Injectable, computed, inject, signal } from '@angular/core';
import { Result } from '@shared/kernel/fp';
import { RemoteData } from '@shared/application/remote-data';
import { createStore } from '@shared/application/store';
import {
Brief,
@@ -11,6 +12,17 @@ import {
import { BriefMsg, BriefState, initial, reduce } from '@brief/domain/brief.machine';
import { BriefAdapter, BriefView } from '@brief/infrastructure/brief.adapter';
/** Transient action state (submit/approve/reject/send/resetDemo) — one tagged union
instead of a busy boolean + a nullable error sitting side by side. */
type ActionState = { tag: 'Idle' } | { tag: 'Busy' } | { tag: 'Failed'; error: string };
/** Debounced-autosave indicator, shown in a small status line near the toolbar —
a separate concern from ActionState (a stale autosave error doesn't block
submit/approve/reject), but tag-aligned with it for one consistent idiom. */
type SaveState = { tag: 'Idle' } | { tag: 'Saving' } | { tag: 'Saved' } | { tag: 'Error' };
type LoadedBriefState = Extract<BriefState, { tag: 'loaded' }>;
/**
* Root singleton for the letter: the Elm store (Model + dispatch), the derived
* read-model, and the commands (effects) that call the adapter and dispatch the
@@ -25,10 +37,31 @@ export class BriefStore {
private store = createStore<BriefState, BriefMsg>(initial, reduce);
readonly model = this.store.model;
readonly busy = signal(false);
readonly lastError = signal<string | null>(null);
private actionState = signal<ActionState>({ tag: 'Idle' });
readonly busy = computed(() => this.actionState().tag === 'Busy');
readonly lastError = computed(() => {
const s = this.actionState();
return s.tag === 'Failed' ? s.error : null;
});
/** Surfaced autosave state for the indicator + aria-live region. */
readonly saveState = signal<'idle' | 'saving' | 'saved' | 'error'>('idle');
readonly saveState = signal<SaveState>({ tag: 'Idle' });
/** The load lifecycle as `RemoteData`, for `<app-async>` — 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<RemoteData<Error | undefined, LoadedBriefState>>(() => {
const s = this.model();
switch (s.tag) {
case 'loading':
return { tag: 'Loading' };
case 'failed':
return { tag: 'Failure', error: new Error(s.reason) };
case 'loaded':
return { tag: 'Success', value: s };
}
});
private brief = computed<Brief | null>(() => {
const s = this.model();
@@ -74,26 +107,28 @@ export class BriefStore {
private async flushSave() {
const b = this.brief();
if (!b) return;
this.saveState.set('saving');
this.saveState.set({ tag: 'Saving' });
const r = await this.adapter.save(b.sections);
if (r.ok) {
this.saveState.set('saved');
this.saveState.set({ tag: 'Saved' });
} else {
this.lastError.set(r.error);
this.saveState.set('error');
this.actionState.set({ tag: 'Failed', error: r.error });
this.saveState.set({ tag: 'Error' });
}
}
/** Demo "start over": recreate the brief server-side and load the fresh view. */
async resetDemo() {
this.busy.set(true);
this.lastError.set(null);
this.actionState.set({ tag: 'Busy' });
clearTimeout(this.saveTimer);
const r = await this.adapter.reset();
this.busy.set(false);
this.saveState.set('idle');
if (r.ok) this.store.dispatch({ tag: 'BriefLoaded', ...r.value });
else this.lastError.set(r.error);
this.saveState.set({ tag: 'Idle' });
if (r.ok) {
this.actionState.set({ tag: 'Idle' });
this.store.dispatch({ tag: 'BriefLoaded', ...r.value });
} else {
this.actionState.set({ tag: 'Failed', error: r.error });
}
}
submit = () => this.transition(() => this.adapter.submit());
@@ -104,16 +139,15 @@ export class BriefStore {
// A transition: flush any pending save, call the server (authoritative), then mirror
// the returned status through the pure reducer's guarded transition.
private async transition(action: () => Promise<Result<string, BriefView>>) {
this.busy.set(true);
this.lastError.set(null);
this.actionState.set({ tag: 'Busy' });
clearTimeout(this.saveTimer);
await this.flushSave();
const r = await action();
this.busy.set(false);
if (!r.ok) {
this.lastError.set(r.error);
this.actionState.set({ tag: 'Failed', error: r.error });
return;
}
this.actionState.set({ tag: 'Idle' });
this.applyServerStatus(r.value);
}

View File

@@ -1,8 +1,8 @@
import { Component, computed, inject } from '@angular/core';
import { PageShellComponent } from '@shared/layout/page-shell/page-shell.component';
import { SpinnerComponent } from '@shared/ui/spinner/spinner.component';
import { AlertComponent } from '@shared/ui/alert/alert.component';
import { ButtonComponent } from '@shared/ui/button/button.component';
import { ASYNC } from '@shared/ui/async/async.component';
import { BriefStore } from '@brief/application/brief.store';
import { LetterComposerComponent } from '@brief/ui/letter-composer/letter-composer.component';
@@ -11,13 +11,7 @@ import { LetterComposerComponent } from '@brief/ui/letter-composer/letter-compos
this just wires signals to the organism and events back to store commands. */
@Component({
selector: 'app-brief-page',
imports: [
PageShellComponent,
SpinnerComponent,
AlertComponent,
ButtonComponent,
LetterComposerComponent,
],
imports: [PageShellComponent, AlertComponent, ButtonComponent, ...ASYNC, LetterComposerComponent],
styles: [
`
.brief-toolbar {
@@ -39,39 +33,38 @@ import { LetterComposerComponent } from '@brief/ui/letter-composer/letter-compos
<app-alert type="error">{{ err }}</app-alert>
}
@switch (model().tag) {
@case ('loading') {
<app-spinner />
}
@case ('failed') {
<app-async [data]="store.remoteData()">
<ng-template appAsyncError>
<app-alert type="error">{{ failedText }}</app-alert>
<app-button variant="secondary" (click)="reload()">{{ retryText }}</app-button>
}
@case ('loaded') {
<div class="brief-toolbar">
<span class="save" role="status" aria-live="polite">{{ saveText() }}</span>
<app-button variant="subtle" [disabled]="store.busy()" (click)="resetDemo()">{{
resetLabel
}}</app-button>
</div>
<app-letter-composer
[brief]="brief()!"
[availablePassages]="availablePassages()"
[diagnostics]="store.diagnostics()"
[canEdit]="store.canEdit()"
[canApprove]="store.canApprove()"
[canReject]="store.canReject()"
[canSend]="store.canSend()"
[canSubmit]="store.canSubmit()"
[busy]="store.busy()"
(edit)="store.edit($event)"
(submit)="store.submit()"
(approve)="store.approve()"
(reject)="store.reject($event)"
(send)="store.send()"
/>
}
}
</ng-template>
<ng-template appAsyncLoaded>
@if (loaded(); as s) {
<div class="brief-toolbar">
<span class="save" role="status" aria-live="polite">{{ saveText() }}</span>
<app-button variant="subtle" [disabled]="store.busy()" (click)="resetDemo()">{{
resetLabel
}}</app-button>
</div>
<app-letter-composer
[brief]="s.brief"
[availablePassages]="s.availablePassages"
[diagnostics]="store.diagnostics()"
[canEdit]="store.canEdit()"
[canApprove]="store.canApprove()"
[canReject]="store.canReject()"
[canSend]="store.canSend()"
[canSubmit]="store.canSubmit()"
[busy]="store.busy()"
(edit)="store.edit($event)"
(submit)="store.submit()"
(approve)="store.approve()"
(reject)="store.reject($event)"
(send)="store.send()"
/>
}
</ng-template>
</app-async>
</app-page-shell>
`,
})
@@ -92,12 +85,12 @@ export class BriefPage {
/** Debounced-save state, surfaced in a polite live region. */
protected saveText = computed(() => {
switch (this.store.saveState()) {
case 'saving':
switch (this.store.saveState().tag) {
case 'Saving':
return this.savingText;
case 'saved':
case 'Saved':
return this.savedText;
case 'error':
case 'Error':
return this.saveErrorText;
default:
return '';
@@ -112,15 +105,13 @@ export class BriefPage {
void this.store.resetDemo();
}
// Narrow the loaded state for the template.
protected brief() {
/** Typed narrowing for the `<app-async>` loaded slot — see WP-06: a structural
directive's context can't inherit a generic from a sibling host input, so the
Success value is unwrapped here instead of through `let-`. */
protected readonly loaded = computed(() => {
const s = this.model();
return s.tag === 'loaded' ? s.brief : null;
}
protected availablePassages() {
const s = this.model();
return s.tag === 'loaded' ? s.availablePassages : [];
}
return s.tag === 'loaded' ? s : undefined;
});
protected reload() {
void this.store.load();

97
src/docs/remote-data.mdx Normal file
View File

@@ -0,0 +1,97 @@
import { Meta, Canvas } from '@storybook/addon-docs/blocks';
import * as AsyncStories from '../app/shared/ui/async/async.stories';
<Meta title="Foundations/RemoteData & Async" />
# RemoteData & Async
An async fetch has exactly four states: still loading, loaded-but-empty, failed, or
loaded-with-a-value. Modeling that as `loading`/`error`/`data` booleans permits nonsense
combinations ("loading **and** error", "data **and** error" — which one does the UI
believe?). `src/app/shared/application/remote-data.ts` closes that off with one tagged
union instead:
```ts
type RemoteData<E, T> = { tag: 'Loading' } | { tag: 'Empty' } | { tag: 'Failure'; error: E } | { tag: 'Success'; value: T };
```
## Combining sources
Two or more independent fetches often need to render as ONE state (e.g. a registration
call and a BRP call feeding the same page). `map`/`map2`/`map3`/`andThen` combine them
with one precedence rule: **Failure beats Loading beats Empty beats Success** — if either
source failed, the combined result is a failure; only when every source succeeded do you
get a combined value.
```ts
map2(registration, person, (reg, p) => ({ registration: reg, person: p }));
```
## Rendering it: `<app-async>`
<Canvas of={AsyncStories.Loading} />
<Canvas of={AsyncStories.ErrorState} />
`shared/ui/async` renders exactly one of the four templates — never two at once, by
construction, since the component switches on the union's tag. Feed it either:
- **`[resource]`** — a raw Angular `resource()` (the common case; the component projects
it into a `RemoteData` internally via `fromResource`), or
- **`[data]`** — an already-combined `RemoteData` (e.g. from a store's `computed()` using
`map`/`map2`).
The default loading UI is a spinner, delay-gated (~250ms) so a fast response never
flashes it; override with an `appAsyncLoading` template. `appAsyncEmpty` and
`appAsyncError` are likewise optional — omit them and you get a sensible default (a
"geen gegevens" message / an alert with a retry button).
## The `appAsyncLoaded` slot isn't generically typed to your value
This is a real Angular constraint, not an oversight: a structural directive's type
parameter can only be inferred from an **input bound on that same element** (this is how
`*ngFor="let x of items"` and `*ngIf="x as y"` work — the type comes from `ngForOf`/`ngIf`,
inputs on the very same tag). `<ng-template appAsyncLoaded let-p>` sits on a *different*
node than `<app-async [data]="…">`, so `p` cannot inherit a type from that sibling input,
even though they're nested in the same template. Angular types it `unknown`, and
`ngTemplateContextGuard` can't fix that without an input to seed it from — the shared
`AsyncComponent`/`AsyncLoadedDirective` pair is properly generic internally, but that
genericity stops at the component's own boundary.
The idiom this repo uses instead — see `brief.page.ts`, `dashboard.page.ts`,
`registration-detail.page.ts` — is a small **typed `computed()`** that unwraps the
`Success` value, narrowed locally in the template with `@if (x(); as p)`:
```ts
// in the component class
protected readonly loaded = computed(() => {
const s = this.model(); // or store.someRemoteData()
return s.tag === 'loaded' ? s : undefined;
});
```
```html
<!-- in the template, inside <ng-template appAsyncLoaded> -->
@if (loaded(); as s) {
<app-letter-composer [brief]="s.brief" ... />
}
```
No `$any()`, no cast — `loaded()` is a real, checked `T | undefined`, and `@if (…; as s)`
narrows it the same way any other nullable signal would.
## The `?scenario=` dev toggle
Any data page can be forced through all four states without touching the backend:
`?scenario=slow|loading|empty|error` (dev-only, `scenario.interceptor.ts`) rewrites the
timing/outcome of `/api/*` calls. Try it on `/brief` or `/dashboard`.
## Where the fetch ends and the domain begins
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 `<app-async>` to render, the way `BriefStore.remoteData` does — the machine
keeps deciding what the *letter* is doing, `RemoteData` keeps deciding what the *fetch* is
doing.