eho/atomic-design-poc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(brief): tolerate a 404 on GET /brief with a one-shot reset (RB-22)

Browse Source 
BriefStore.load() now treats a 404 from GET /brief as "no brief exists
yet" and calls the existing reset() command once, instead of showing
the generic load-failed error. BriefAdapter.load() gains a
BriefLoadFailure error channel (notFound | error) so the store can
tell a 404 apart from every other failure; every other adapter method
stays on runSubmit, unchanged.

The once-only bound is a field on the store, not a comment: a second
404 (from a later load() call) always falls through to the ordinary
error path, and the recovery path never calls load() again, so no
loop can form.

This is the expand half of CQ-007's split (04-cqrs-light.md). Today's
backend never 404s GET /brief, so the new branch is dead code until
RB-23 (the backend contract half) ships in a later merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
eho
2026-08-27 18:43:27 +02:00
co-authored by Claude Opus 5
parent 7fbac8fca5
commit 7a29f5facc
6 changed files with 309 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/ssp/src/app/brief/application/brief.store.spec.ts
+62 -11
View File
@@ -3,7 +3,12 @@ import { describe, it, expect, vi, afterEach } from 'vitest';
import { Result } from '@shared/kernel/fp';
import { Brief, BriefDecisions, CaseContext, LetterBlock } from '@brief/domain/brief';
import { OrgTemplate } from '@brief/domain/org-template';
import { BriefAdapter, BriefView } from '@brief/infrastructure/brief.adapter';
import {
BRIEF_LOAD_FAILED,
BriefAdapter,
BriefLoadFailure,
BriefView,
} from '@brief/infrastructure/brief.adapter';
import { LetterPreviewAdapter, PREVIEW_FAILED } from '@brief/infrastructure/letter-preview.adapter';
import { RevealBigNummerAdapter } from '@brief/infrastructure/reveal-bignummer.adapter';
import { BriefStore } from './brief.store';
@@ -60,7 +65,8 @@ describe('BriefStore action state (Idle | Busy | Failed)', () => {
brief: { ...brief, status: { tag: 'approved', approvedBy: 'u2', approvedAt: 't' } },
};
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
load: (): Promise<Result<BriefLoadFailure, 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 }),
@@ -79,7 +85,8 @@ describe('BriefStore action state (Idle | Busy | Failed)', () => {
brief: { ...brief, status: { tag: 'approved', approvedBy: 'u2', approvedAt: 't' } },
};
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
load: (): Promise<Result<BriefLoadFailure, 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 }),
@@ -93,7 +100,8 @@ describe('BriefStore action state (Idle | Busy | Failed)', () => {
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 }),
load: (): Promise<Result<BriefLoadFailure, 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' }),
@@ -108,7 +116,8 @@ describe('BriefStore action state (Idle | Busy | Failed)', () => {
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 }),
load: (): Promise<Result<BriefLoadFailure, 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),
});
@@ -156,8 +165,10 @@ function loadedBrief(store: BriefStore): Brief {
}
async function loadedStore(over: Partial<BriefAdapter> = {}): Promise<BriefStore> {
const ok = (v: BriefView): Promise<Result<string, BriefView>> =>
Promise.resolve({ ok: true, value: v });
// Untyped return (inferred as the narrow `{ ok: true; value }` literal) so this one
// helper satisfies both `load` (error channel `BriefLoadFailure`) and `save` (error
// channel `string`) — it only ever produces the `ok: true` branch.
const ok = (v: BriefView) => Promise.resolve({ ok: true, value: v } as const);
const store = setup({ load: () => ok(filledView), save: () => ok(filledView), ...over });
await store.load();
return store;
@@ -255,8 +266,7 @@ describe('BriefStore rejection diff', () => {
...filledBrief,
status: { tag: 'rejected', rejectedBy: 'u2', rejectedAt: 't', comments: 'nee' },
};
const ok = (v: BriefView): Promise<Result<string, BriefView>> =>
Promise.resolve({ ok: true, value: v });
const ok = (v: BriefView) => Promise.resolve({ ok: true, value: v } as const);
const store = setup({
load: () => ok({ ...filledView, brief: submitted }),
save: () => ok(filledView),
@@ -283,7 +293,8 @@ describe('BriefStore.previewLetter', () => {
it('opens the composed letter in a new tab on success', async () => {
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
load: (): Promise<Result<BriefLoadFailure, BriefView>> =>
Promise.resolve({ ok: true, value: view }),
});
await store.load();
const blob = new Blob(['<html></html>'], { type: 'text/html' });
@@ -301,7 +312,8 @@ describe('BriefStore.previewLetter', () => {
it('surfaces the error without opening a tab on failure', async () => {
const store = setup({
load: (): Promise<Result<string, BriefView>> => Promise.resolve({ ok: true, value: view }),
load: (): Promise<Result<BriefLoadFailure, BriefView>> =>
Promise.resolve({ ok: true, value: view }),
});
await store.load();
const open = vi.spyOn(window, 'open').mockImplementation(() => null);
@@ -377,3 +389,42 @@ describe('BriefStore.flushPending (CanDeactivate guard / beforeunload)', () => {
expect(save).not.toHaveBeenCalled();
});
});
// --- RB-22 (CQ-007 expand half): a 404 from GET /brief tolerates by calling the
// existing reset() command, exactly once. Today's backend never 404s (RB-23 adds
// that); this fake adapter is what exercises the branch until then. ---
describe('BriefStore.load — 404 tolerance (RB-22)', () => {
const notFound: Result<BriefLoadFailure, BriefView> = { ok: false, error: { tag: 'notFound' } };
const resetOk: Result<string, BriefView> = { ok: true, value: view };
it('a 404 drives exactly one reset(), which populates the store', async () => {
// Given GET /brief 404s (no brief exists yet) and reset() succeeds.
const load = vi.fn(() => Promise.resolve(notFound));
const reset = vi.fn(() => Promise.resolve(resetOk));
const store = setup({ load, reset });
// When the store loads...
await store.load();
// Then reset() ran exactly once, and the store ends up loaded from its result.
expect(reset).toHaveBeenCalledTimes(1);
expect(store.model().tag).toBe('loaded');
});
it('a second 404 does not drive a second reset()', async () => {
// Given every load() attempt 404s (e.g. the brief still fails to appear).
const load = vi.fn(() => Promise.resolve(notFound));
const reset = vi.fn(() => Promise.resolve(resetOk));
const store = setup({ load, reset });
// When the store loads twice...
await store.load();
await store.load();
// 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 });
});
});
apps/ssp/src/app/brief/application/brief.store.ts
+32 -5
View File
@@ -16,7 +16,7 @@ import {
import { BriefMsg, BriefState, initial, reduce } from '@brief/domain/brief.machine';
import { BlockDiffKind, changedBlocks, diffBlocks } from '@brief/domain/brief-diff';
import { OrgTemplate } from '@brief/domain/org-template';
import { BriefAdapter, BriefView } from '@brief/infrastructure/brief.adapter';
import { BRIEF_LOAD_FAILED, BriefAdapter, BriefView } from '@brief/infrastructure/brief.adapter';
import { LetterPreviewAdapter } from '@brief/infrastructure/letter-preview.adapter';
import { RevealBigNummerAdapter } from '@brief/infrastructure/reveal-bignummer.adapter';
import { uploadContentUrl } from '@shared/upload/upload.adapter';
@@ -119,13 +119,40 @@ export class BriefStore implements PendingSave {
return !!b && canSubmit(b) && !hasBlockingErrors(this.diagnostics());
});
/** True once a 404-triggered recovery has been attempted (RB-22, CQ-007's expand
half — see `recoverFromMissingBrief`). This is the structural once-only bound:
a repeated 404 falls straight to the `error` branch below and can never reach
`adapter.reset()` a second time, regardless of how many times `load()` runs. */
private hasRecoveredFromMissingBrief = false;
async load() {
const r = await this.adapter.load();
if (r.ok) {
this.orgTemplate.set(r.value.orgTemplate);
this.caseContext.set(r.value.caseContext);
this.history.clear();
this.store.dispatch({ tag: 'BriefLoaded', ...r.value });
this.applyLoadedView(r.value);
} else if (r.error.tag === 'notFound' && !this.hasRecoveredFromMissingBrief) {
this.hasRecoveredFromMissingBrief = true;
await this.recoverFromMissingBrief();
} else {
const reason = r.error.tag === 'notFound' ? BRIEF_LOAD_FAILED : r.error.reason;
this.store.dispatch({ tag: 'BriefLoadFailed', reason });
}
}
private applyLoadedView(view: BriefView) {
this.orgTemplate.set(view.orgTemplate);
this.caseContext.set(view.caseContext);
this.history.clear();
this.store.dispatch({ tag: 'BriefLoaded', ...view });
}
/** `GET /brief` 404'd — no brief exists yet for this owner. Recover by calling the
existing `reset()` command directly (the same POST `resetDemo()` uses) and
applying whatever it returns; this NEVER calls `load()` again, so a second 404
(e.g. `reset()` itself failing) cannot loop back into this method. */
private async recoverFromMissingBrief() {
const r = await this.adapter.reset();
if (r.ok) {
this.applyLoadedView(r.value);
} else {
this.store.dispatch({ tag: 'BriefLoadFailed', reason: r.error });
}
apps/ssp/src/app/brief/infrastructure/brief.adapter.ts
+35 -7
View File
@@ -1,6 +1,7 @@
import { Injectable, inject } from '@angular/core';
import { Result, ok, err } from '@shared/kernel/fp';
import { runResult, runSubmit } from '@shared/application/submit';
import { runSubmit } from '@shared/application/submit';
import { problemDetail } from '@shared/infrastructure/api-error';
import {
ApiClient,
BriefDecisionsDto,
@@ -33,9 +34,13 @@ import { Mark, Paragraph, RichTextBlock, RichTextNode } from '@shared/kernel/ric
* The only place brief HTTP lives (ADR-0001 anti-corruption boundary). The wire
* uses FLAT unions (a `type`/`tag` string + nullable fields, the repo convention);
* the `parse*` boundary narrows them into the domain's proper discriminated unions
* and rejects malformed shapes. `load` (the only read) folds through `runResult`;
* every mutation folds through `runSubmit` (ProblemDetails → error string, plus the
* Idempotency-Key mint), then parses the returned brief.
* and rejects malformed shapes. Every mutation folds through `runSubmit`
* (ProblemDetails → error string, plus the Idempotency-Key mint), then parses the
* returned brief. `load` (the only read) does its own try/catch instead of the
* shared `runResult` fold, because it needs one extra bit `runResult` throws away:
* whether the failure was an HTTP 404 (see `BriefLoadFailure` — RB-22, CQ-007's
* expand half). Today's backend never 404s `GET /brief` (RB-23 adds that), so the
* `notFound` branch is unreached until RB-23 ships; this adapter is ready in advance.
*/
export interface BriefView {
@@ -46,16 +51,39 @@ export interface BriefView {
readonly caseContext: CaseContext;
}
/**
* Why `load()` did not return a brief. `notFound` is a bare HTTP 404 — kept
* distinct from every other failure so `BriefStore.load()` can tolerate it (call
* `reset()` instead of showing an error banner) without conflating it with a real
* failure. See the class docstring above.
*/
export type BriefLoadFailure =
{ readonly tag: 'notFound' } | { readonly tag: 'error'; readonly reason: string };
export const BRIEF_LOAD_FAILED = $localize`:@@brief.load.failed:De brief kon niet worden geladen.`;
export const BRIEF_ACTION_FAILED = $localize`:@@brief.action.failed:De actie is niet gelukt. Probeer het later opnieuw.`;
/** True when the thrown value carries an HTTP 404 status — matches both the
generic `SwaggerException` (today's shape, since `GET /brief` declares no 404
response yet) and a parsed `ProblemDetails` (RFC 7807 `status`, the shape once
RB-23 gives the endpoint a documented 404 response). */
function isHttpNotFound(e: unknown): boolean {
return !!e && typeof e === 'object' && (e as { status?: unknown }).status === 404;
}
@Injectable({ providedIn: 'root' })
export class BriefAdapter {
private client = inject(ApiClient);
async load(): Promise<Result<string, BriefView>> {
const r = await runResult(() => this.client.briefGET(), BRIEF_LOAD_FAILED);
return r.ok ? parseBriefView(r.value) : r;
async load(): Promise<Result<BriefLoadFailure, BriefView>> {
try {
const dto = await this.client.briefGET();
const parsed = parseBriefView(dto);
return parsed.ok ? ok(parsed.value) : err({ tag: 'error', reason: parsed.error });
} catch (e) {
if (isHttpNotFound(e)) return err({ tag: 'notFound' });
return err({ tag: 'error', reason: problemDetail(e, BRIEF_LOAD_FAILED) });
}
}
async save(sections: readonly LetterSection[]): Promise<Result<string, BriefView>> {
docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md
+35 -35
View File
@@ -100,41 +100,41 @@ deployed first_, not _must ship together_.
Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authoritative
16-row "Compliance review required" list, carries it — regardless of priority.
| ID | Module | Category | Description | Baseline metric improved | Effort | Risk | Priority | CD batch # | Depends on | Compliance | Status |
| --------- | -------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------ | -------- | -------- | ---------- | ---------- | ------------ | -------- |
| **RB-01** | backend/Program.cs + Data | security | Add an owner/capability check to `GET /uploads/{id}/content` and `/uploads/status`; 404 not 403 | §3c Data 75.5% branch vs 99.0% line (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-02** | backend/Program.cs + Data | privacy | Stop concatenating the BSN into `AuthzAudit.Resource`; assert on **values** in the test | §3c Data 75.5% branch (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-03** | backend/Contracts | privacy | `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` — both cross-owner lists inherit it | §3a bhp/behandeling 91.6%/81.5%; §7 Mapping row | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-04** | backend/Data | privacy | Mask the BSN used as `AuditEntry.Actor` on document audit rows (ownership column untouched) | §3c Data 99.0% line / 75.5% branch | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-05** | backend/Zgw | privacy | Drop the BSN-bearing query + body snippet from the `ZgwHttpClient` exception message | §3c Zgw 98.1%/85.5% (best backend branch) — a design gap, not a test gap | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-06** | backend/Program.cs | security | Delete the dead `POST /registrations` (no FE caller) — or add the `ForeignIds` guard | BL-003 (48 mappings in 940 lines, file CC 78) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-07** | backend/Program.cs | audit | Audit the **allow** path in all five authz gates + the 3 brief transitions and the besluit | §3c Program.cs 84.8% branch; BL-003 | S–M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-08** | backend/Program.cs | security | Route `DELETE /admin/uploads/{id}` through `CasesAdmin`; delete the orphaned `IsAdmin` gate | BL-003; §7 CQRS-light wrappers row | S | Low | **P1** | 2 | RB-07 | **SIGN-OFF** | **done** |
| **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | **done** |
| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | **done** |
| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | open |
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open |
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | open |
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | open |
| **RB-24** | libs/shared/upload | ADR conform. | Move `upload/` into `infrastructure`/`domain`/`application`; **delete** the depcruise carve-out | BL-010; §7 "+1 adapter outside `infrastructure/`", "8 of 9 machines in `domain/`"; §3b shared/domain 0% reach | M | Med | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-25** | libs/shared/upload | testability | `UPLOAD_TRANSPORT` injection token (the `SESSION_PORT` shape) instead of `inject(KeepaliveTransport)` | §3a upload 52.0%/50.0%; §3b file unreached, non-`ui/` | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open |
| **RB-26** | libs/shared/upload | testability | Move the accept/reject decision to `planFileSelection` in `upload.machine.ts` | §3a upload 52.0%/50.0%; §4a module max CC 27 | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open |
| **RB-27** | libs/shared/upload | testability | Extract `uploadOutcome(status, responseText)` out of the XHR closure | file LH 5/64 (**7.8% line**), BRH 3/57 (**5.3% branch**) | S–M | Low | P2 | 5 | RB-25 | **SIGN-OFF** | open |
| **RB-28** | libs/beheer + ssp/brief | testability | `BLOB_PRESENTER` token; the 3 commands' success paths become assertable | §3a beheer/application **40.5% branch — worst FE**; brief.store BRH 32/64 | S–M | Low | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-29** | backend/Domain | testability | Thread the existing `at` through `LetterHtml.ResolveAuto` instead of reading `UtcNow` | §3c Domain 82.0% branch; §4b `LetterHtml.cs` CC 21 | S | Low | P2 | 5 | — | — | open |
| **RB-30** | backend/Data + Domain | testability | Extract 5 brief guards into `Domain/Letters/BriefRules.cs`; add `tests/Domain/BriefRuleTests.cs` | §3c Data **75.5% branch** (BL-005); §4b `BriefStore.cs` CC 17, `ToDto` CC 16 | M | Med | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-31** | 4 app contexts (specs only) | ADR conform. | Replace hand-rolled state literals with `given(reduce, initial)` replays in 4 machine specs | §7 Elm machines 9 (1 has a `*.testing.ts`); §3a herreg 67.8% / brief 68.8% branch | M | Low | P2 | 6 | — | — | open |
| **RB-32** | libs/shared/docs | ADR conform. | Add the missing `language-switcher` row to the CIBG gap register (9 markers vs 8 rows) | §2 libs/shared 86 files / 5 194 lines; §6 layout Ca 22 | S | Low | P3 | 6 | — | — | open |
| **RB-33** | libs/shared/testing | ADR conform. | Adopt `unwrapOk` at its one call site — **or delete it**; both satisfy ADR-0006 §3 | BL-004; §3a libs/shared/testing 3 files, 100% line | S | Low | P3 | 6 | — | — | open |
| ID | Module | Category | Description | Baseline metric improved | Effort | Risk | Priority | CD batch # | Depends on | Compliance | Status |
| --------- | -------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------ | -------- | -------- | ---------- | ---------- | ------------ | --------------- |
| **RB-01** | backend/Program.cs + Data | security | Add an owner/capability check to `GET /uploads/{id}/content` and `/uploads/status`; 404 not 403 | §3c Data 75.5% branch vs 99.0% line (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-02** | backend/Program.cs + Data | privacy | Stop concatenating the BSN into `AuthzAudit.Resource`; assert on **values** in the test | §3c Data 75.5% branch (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-03** | backend/Contracts | privacy | `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` — both cross-owner lists inherit it | §3a bhp/behandeling 91.6%/81.5%; §7 Mapping row | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-04** | backend/Data | privacy | Mask the BSN used as `AuditEntry.Actor` on document audit rows (ownership column untouched) | §3c Data 99.0% line / 75.5% branch | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-05** | backend/Zgw | privacy | Drop the BSN-bearing query + body snippet from the `ZgwHttpClient` exception message | §3c Zgw 98.1%/85.5% (best backend branch) — a design gap, not a test gap | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-06** | backend/Program.cs | security | Delete the dead `POST /registrations` (no FE caller) — or add the `ForeignIds` guard | BL-003 (48 mappings in 940 lines, file CC 78) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
| **RB-07** | backend/Program.cs | audit | Audit the **allow** path in all five authz gates + the 3 brief transitions and the besluit | §3c Program.cs 84.8% branch; BL-003 | S–M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-08** | backend/Program.cs | security | Route `DELETE /admin/uploads/{id}` through `CasesAdmin`; delete the orphaned `IsAdmin` gate | BL-003; §7 CQRS-light wrappers row | S | Low | **P1** | 2 | RB-07 | **SIGN-OFF** | **done** |
| **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | **done** |
| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | **done** |
| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | open |
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open |
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | open |
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | open |
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | **implemented** |
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | open |
| **RB-24** | libs/shared/upload | ADR conform. | Move `upload/` into `infrastructure`/`domain`/`application`; **delete** the depcruise carve-out | BL-010; §7 "+1 adapter outside `infrastructure/`", "8 of 9 machines in `domain/`"; §3b shared/domain 0% reach | M | Med | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-25** | libs/shared/upload | testability | `UPLOAD_TRANSPORT` injection token (the `SESSION_PORT` shape) instead of `inject(KeepaliveTransport)` | §3a upload 52.0%/50.0%; §3b file unreached, non-`ui/` | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open |
| **RB-26** | libs/shared/upload | testability | Move the accept/reject decision to `planFileSelection` in `upload.machine.ts` | §3a upload 52.0%/50.0%; §4a module max CC 27 | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open |
| **RB-27** | libs/shared/upload | testability | Extract `uploadOutcome(status, responseText)` out of the XHR closure | file LH 5/64 (**7.8% line**), BRH 3/57 (**5.3% branch**) | S–M | Low | P2 | 5 | RB-25 | **SIGN-OFF** | open |
| **RB-28** | libs/beheer + ssp/brief | testability | `BLOB_PRESENTER` token; the 3 commands' success paths become assertable | §3a beheer/application **40.5% branch — worst FE**; brief.store BRH 32/64 | S–M | Low | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-29** | backend/Domain | testability | Thread the existing `at` through `LetterHtml.ResolveAuto` instead of reading `UtcNow` | §3c Domain 82.0% branch; §4b `LetterHtml.cs` CC 21 | S | Low | P2 | 5 | — | — | open |
| **RB-30** | backend/Data + Domain | testability | Extract 5 brief guards into `Domain/Letters/BriefRules.cs`; add `tests/Domain/BriefRuleTests.cs` | §3c Data **75.5% branch** (BL-005); §4b `BriefStore.cs` CC 17, `ToDto` CC 16 | M | Med | P2 | 5 | — | **SIGN-OFF** | open |
| **RB-31** | 4 app contexts (specs only) | ADR conform. | Replace hand-rolled state literals with `given(reduce, initial)` replays in 4 machine specs | §7 Elm machines 9 (1 has a `*.testing.ts`); §3a herreg 67.8% / brief 68.8% branch | M | Low | P2 | 6 | — | — | open |
| **RB-32** | libs/shared/docs | ADR conform. | Add the missing `language-switcher` row to the CIBG gap register (9 markers vs 8 rows) | §2 libs/shared 86 files / 5 194 lines; §6 layout Ca 22 | S | Low | P3 | 6 | — | — | open |
| **RB-33** | libs/shared/testing | ADR conform. | Adopt `unwrapOk` at its one call site — **or delete it**; both satisfy ADR-0006 §3 | BL-004; §3a libs/shared/testing 3 files, 100% line | S | Low | P3 | 6 | — | — | open |
---
docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-22.md
+139
View File
@@ -0,0 +1,139 @@
# RB-22 — `BriefStore.load()` tolerates a 404, calling `reset()` exactly once
Status: **implemented** · 2026-08-27 · Source findings: `04-cqrs-light.md` CQ-007 ·
`99-backlog.md` RB-22, "Tickets that were rejected and split" · `implementation/rb-17.md`
(the `runResult`/`runSubmit` seam this store already sits on)
This is the **expand** half of an expand/contract pair. RB-23 (backend: `GET /brief` 404s
when absent; `BriefStore.GetOrCreate` splits into `Get` + `ResetAndCreate`) ships after this
ticket, in a later merge. Today's backend never 404s `GET /brief`, so this ticket's new
branch is dead code in the running app — provably backend-frontend-safe by construction.
## What was wrong
CQ-007 flags `GET /brief` (`Program.cs:603` → `BriefStore.GetOrCreate`,
`Data/BriefStore.cs:50`) as the one endpoint in the backend where a GET performs a
persisted write, breaking the read/write split every other endpoint respects. The fix is
split across both sides of the seam because the FE must be ready to receive a 404 before
the backend can safely start sending one. This ticket is the FE half: `BriefStore.load()`
(`apps/ssp/src/app/brief/application/brief.store.ts`) had no notion of "no brief exists
yet" — every adapter failure, 404 included, dispatched `BriefLoadFailed` and showed the
generic error banner. `BriefAdapter.load()` (`brief.adapter.ts`) also had no way to tell
the store a failure was specifically an HTTP 404: it folded every failure through the
shared `runResult` helper (RB-17), which keeps only a human-readable string and throws
away the HTTP status.
The ticket read as filed against the current code: `BriefStore.load()` is exactly where
CQ-007 says it is, `BriefAdapter.load()` is exactly the read `runResult` call RB-17 pointed
at it, and nothing about either file was factually wrong. Nothing to flag here.
## What changed
| File | Change |
| -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `apps/ssp/src/app/brief/infrastructure/brief.adapter.ts` | `load()`'s error channel becomes `BriefLoadFailure` (`{tag:'notFound'} \| {tag:'error', reason:string}`) instead of a plain `string`. `load()` no longer routes through the shared `runResult` — it does its own try/catch so it can read the thrown value's HTTP `status` before folding it away, via the new local `isHttpNotFound` predicate. Every other method (`save`/`submit`/`approve`/`reject`/`send`/`reset`) is untouched, still on `runSubmit`. |
| `apps/ssp/src/app/brief/application/brief.store.ts` | `load()` branches on `BriefLoadFailure`: `notFound` (and not already recovered) calls the existing `reset()` command directly and applies its result; every other failure (including a repeated `notFound`) dispatches `BriefLoadFailed` as before. Extracted `applyLoadedView` (the success-path body shared by `load()` and the new recovery path) and added `recoverFromMissingBrief`. |
| `apps/ssp/src/app/brief/application/brief.store.spec.ts` | New `describe('BriefStore.load — 404 tolerance (RB-22)')` with the two required cases. Six pre-existing `load:` fakes' explicit `Result<string, BriefView>` return-type annotations updated to `Result<BriefLoadFailure, BriefView>` (they only ever produce the `ok: true` branch, so this is a type-only change); two shared `ok(v)` test helpers that build fakes for both `load` and `save` had their return-type annotation dropped in favour of `as const` inference, since one helper now serves two different error-channel types. |
| `libs/shared/docs/behaviour-spec.mdx` | Regenerated (`npm run gen:behaviour-spec`) — picks up the two new `it()` titles. |
No `backend/` file was touched — `Program.cs` and `Data/BriefStore.cs` are RB-23's, per the
ticket's explicit scope.
## How the once-only bound is structural
`BriefStore` gains one field: `private hasRecoveredFromMissingBrief = false`. `load()`
takes the recovery branch only when `r.error.tag === 'notFound' && !this.hasRecoveredFromMissingBrief`,
and the branch's first statement sets the flag before doing anything else. A second 404 —
whether from a second `load()` call, or in principle from `reset()` itself somehow also
404ing — falls through to the plain `BriefLoadFailed` branch instead, on every subsequent
call, for the life of the store instance. This is a field on the singleton store, not a
comment: nothing in the reachable call graph can flip it back to `false`.
The loop CQ-007's proposed change warns about ("the reset's own load must not be able to
loop") is not merely bounded, it is **structurally absent**: `recoverFromMissingBrief`
calls `this.adapter.reset()` and applies its `BriefView` result directly (the same
`applyLoadedView` the success path uses) — it never calls `this.load()` again. There is no
recursive edge from the recovery path back into `load()` for the once-only flag to have to
stop; the flag exists only to stop a **second, separate** `load()` invocation (e.g. a
caller retrying navigation) from reaching `reset()` again.
## Judgement calls
- **`load()` no longer uses `runResult`, only for this one method.** `runResult`
(`libs/shared/src/application/submit.ts`, RB-17) intentionally keeps only a string —
every other read in the app is fine with that. This is the first read that needs one
more bit (the HTTP status) than `runResult` exposes, so `load()` does its own
try/catch instead, matching `runResult`'s shape (`problemDetail(e, fallback)` on the
non-404 path) but adding the 404 branch first. `libs/shared/src/application/submit.ts`
itself is untouched — changing a shared helper used by many call sites for one adapter's
need was out of scope and unjustified.
- **404 detection reads `(e as {status?:unknown}).status === 404`, not
`SwaggerException.isSwaggerException`.** The generated client throws a plain
`SwaggerException` for `GET /brief` today (no OpenAPI 404 response is declared for it
yet), but throws the parsed `ProblemDetails` object instead for an endpoint whose spec
**does** declare a 404 (both shapes carry a `status` field). Checking `status` alone,
not the `SwaggerException` type, means this predicate keeps working unchanged once
RB-23 regenerates the client with a documented 404 response for `briefGET()` — no
follow-up FE edit needed for detection to keep working.
- **`BriefLoadFailure` is a new exported type, not a sentinel string.** CLAUDE.md's
default reflex is a discriminated union over a second flag; a magic string
(`'__not_found__'`) compared by identity would have kept `load()`'s signature at
`Result<string, BriefView>` and touched fewer test lines, but it is exactly the kind
of stringly-typed control flow the union tool exists to avoid. The touched-test cost
was six type annotations plus two helper signatures, all in the one already-scoped
spec file — judged worth it for the correct shape.
`BriefLoadFailure` is exported.
- **On a repeated 404, the store shows `BRIEF_LOAD_FAILED`** (the same generic banner
text `load()` already used for every other failure), not a distinct "still missing"
message. No new user-facing copy was needed or added, so no new `$localize` id and no
`messages.en.xlf` change — confirmed by diffing for `$localize` occurrences: both
hits in the diff are unchanged context lines, not new additions.
- **`resetDemo()` (the "start over" button) was left untouched**, even though it
duplicates part of the same apply-a-fresh-view logic now factored into
`applyLoadedView`. It also manages `actionState`/`saveState`/`rejectionSnapshot` that
`recoverFromMissingBrief` correctly does not touch (an automatic recovery on first
load is not a user-initiated "start over" action), and refactoring it was not asked
for by this ticket.
## Verification
- **Verified red without the fix.** Temporarily replaced `load()`'s body (via `Edit`,
not `git checkout`) with the pre-fix shape — every failure, `notFound` included,
dispatches `BriefLoadFailed` straight away, no `reset()` call — and reran the spec
file. Both new tests failed:
`expected "vi.fn()" to be called 1 times, but got 0 times` on `reset`, for both "a 404
drives exactly one reset()" and "a second 404 does not drive a second reset()"; the
other 18 tests in the file stayed green. Restored the real fix with a second `Edit`
and reran: all 20 tests in the file green, 30/30 across both touched spec files.
- `npm run ci` (foreground, no background/Monitor): **green**, exit 0 — lint,
typecheck, `dep:check` (341 + 226 modules, 0 violations), `format:check`,
`check:tokens`, `check:seam`, tests (ssp 260/260, behandelportal 37/37, shared
138/138, beheer 23/23 — 458 total), `ng build --localize` (both apps), `npm audit`
(0 vulnerabilities), backend `dotnet test` (260/260 — the known
`OpenZaakIntegrationTests.Admin_cases_…` container-dependent failure did not
reproduce on this run, matching the standing caveat that it needs a live OpenZaak
container and is not this ticket's bug), backend dependency audit clean, `gen:snippets`
drift clean, `gen:behaviour-spec` drift clean once the regenerated file was staged (the
local gate diffs the working tree against the index, so it necessarily shows a diff
before the file is staged/committed — the same documented, expected behaviour RB-17
recorded, not a defect).
- `npx prettier --check` on every touched file (including the reformatted
`99-backlog.md` table and the regenerated `behaviour-spec.mdx`): clean.
## What RB-23 must do
Once `GET /brief` in `Program.cs` returns a real 404 (no `ProblemDetails` body is
required — `BriefAdapter.load()`'s `isHttpNotFound` only reads the HTTP `status`, not the
response body), and `BriefStore.GetOrCreate` splits into `Get` (query) + the existing
`ResetAndCreate` (already there, already used by `POST /brief/reset`), this ticket's
`notFound` branch stops being dead code and starts running on first-ever page load for any
owner with no persisted brief. Run `npm run gen:api` as part of RB-23 so `briefGET()`
regenerates with a documented `status === 404` branch throwing the parsed `ProblemDetails`
(matching the shape most other endpoints already use) — the detection predicate here
already tolerates that shape and needs no FE follow-up change. Two things worth
re-verifying once RB-23 lands, not fixing preemptively here: first, that the resulting
double round-trip (404, then `reset()`) is an acceptable UX cost on a first visit, per
CQ-007's own framing of this as its least certain finding; second, that this store's
`hasRecoveredFromMissingBrief` field — private to one store instance, reset only by a full
reload — is still the right lifetime for the once-only guard once a real 404 can occur in
production traffic, not only in a test's fake adapter.
libs/shared/docs/behaviour-spec.mdx
+6 -1
View File
@@ -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. 451 frontend behaviours across
**is** the suite, reshaped for a business reader. 453 frontend behaviours across
9 contexts; 236 backend behaviours across 41 test
classes.
@@ -180,6 +180,11 @@ classes.
- flushes a pending debounced edit immediately and clears the pending flag
- is a no-op when no edit is pending
#### BriefStore.load — 404 tolerance (RB-22)
- a 404 drives exactly one reset(), which populates the store
- a second 404 does not drive a second reset()
#### BriefStore.previewLetter
- opens the composed letter in a new tab on success