docs(adr): land ADR-C-001, ADR-C-003, ADR-C-007 and ADR-C-009
The architect approved the four ADR-fix tickets. All four change what the architecture documents claim. No code changes. ADR-0001, ADR-C-001: the worked example claimed the POC has no real backend. It rewrites against `backend/src/BigRegister.Api`. Every path it named is repointed. The out-of-scope list drops two discharged bullets: 33 `parse*` boundaries exist, and `npm run gen:api` is real. ADR-0001, ADR-C-003: a new section states that the generated client is the wire contract. A hand-written `contracts/*.dto.ts` is the exception for two cases only. The four survivors stay, because NSwag emits every property as optional and flattens `RegistrationStatusDto` into five optional strings. The `parse*` trust boundary stays mandatory, because a generated type is a compile-time claim about the wire and not a runtime guarantee. ADR-0003, ADR-C-007: four paths moved in WP-67 and are repointed. Point 4 kept the principle and changed its example to `skeleton` and `spinner`. Two of its claims were false and the amendment says so: `app-alert` wraps the vendored `.feedback` classes, and `site-header` composes the vendored `.titlebar`. ADR-0004, ADR-C-009: the exception section states a four-part test instead of one named exception. `OrgTemplateStore` and `FeatureFlagStore` both pass it. RB-07 gated this ticket, because clause 4 needs an audited allow path. RB-07 landed that, so the ADR does not ratify a control that the code lacks. Three tickets need a matching CLAUDE.md correction in the same diff. CLAUDE.md section 2 loses the false `alert` example. Section 4 gets the generated-client rule and the four-part test. Two findings were wrong. ADR-C-001 asked to keep an out-of-scope bullet that reads "SessionStore is in-memory". The session persists to `localStorage` now, so the bullet covers multi-tab sync only. ADR-C-007 flagged one half of point 4 and missed that the other half is equally false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -69,44 +69,96 @@ the governance/transparency artifact.
|
||||
The frontend keeps only **format** validation (postcode shape, integer parsing) for
|
||||
instant feedback — never as the authority.
|
||||
|
||||
### Where the contract lives, after codegen
|
||||
|
||||
The paragraph above says "manage it with one source of truth that generates types for
|
||||
both sides". That target state has arrived, so this section states which artifact is now
|
||||
the contract.
|
||||
|
||||
**The generated client is the wire contract.** `libs/shared/src/infrastructure/api-client.ts`
|
||||
is regenerated from the backend's OpenAPI document by `npm run gen:api`, and CI fails on
|
||||
drift (the `api-client-drift` job regenerates it and runs `git diff --exit-code`). It is the
|
||||
single source of truth for the shape of every endpoint. An adapter consumes its types
|
||||
directly; 19 of the 20 infrastructure adapters do.
|
||||
|
||||
**A hand-written `contracts/*.dto.ts` is the exception, for two cases only:**
|
||||
|
||||
1. **Codegen does not reach the endpoint** — a hand-rolled `fetch`/XHR path that the
|
||||
generator never sees.
|
||||
2. **The generator types the shape too loosely** — the generated type compiles but is
|
||||
weaker than the wire really is.
|
||||
|
||||
In either case the hand-written file must still import nothing. It describes the wire, not
|
||||
the domain.
|
||||
|
||||
**The `parse*` trust boundary is unchanged and stays mandatory**, whichever way the type
|
||||
arrived. A generated type is a compile-time claim about the wire, not a runtime guarantee:
|
||||
the server can send anything. `infrastructure/` validates the untrusted shape and maps it
|
||||
onto the domain, exactly as before.
|
||||
|
||||
**The four surviving hand-written contracts stay.** They are
|
||||
`apps/ssp/src/app/registratie/contracts/{brp-address,dashboard-view,duo-diplomas}.dto.ts`
|
||||
and `libs/beheer/src/contracts/stamdata.dto.ts`. All four fall under case 2, and the
|
||||
dashboard view shows why: the generator emits every property as optional, and it flattens
|
||||
a discriminated union into a bag of optional fields.
|
||||
|
||||
```ts
|
||||
// generated — every field optional, `tag` a bare string, all variants merged
|
||||
interface RegistrationStatusDto {
|
||||
tag?: string | undefined;
|
||||
herregistratieDatum?: string | undefined;
|
||||
geschorstTot?: string | undefined;
|
||||
reden?: string | undefined;
|
||||
doorgehaaldOp?: string | undefined;
|
||||
}
|
||||
|
||||
// hand-written — a real discriminated union, per-variant fields required
|
||||
type RegistrationStatusDto =
|
||||
| { tag: 'Geregistreerd'; herregistratieDatum: string }
|
||||
| { tag: 'Geschorst'; geschorstTot: string; reden: string }
|
||||
| { tag: 'Doorgehaald'; doorgehaaldOp: string; reden: string };
|
||||
```
|
||||
|
||||
Adopting the generated shape here would push `undefined` handling into every consumer and
|
||||
make an illegal state representable, which CLAUDE.md §3 forbids. Retiring these four is
|
||||
therefore **not** a cleanup to schedule; it becomes correct only if the backend annotates
|
||||
its DTOs so the generator emits required properties and real unions.
|
||||
|
||||
## Worked example in this POC
|
||||
|
||||
This POC has no real backend (static mock JSON + fake submit timers), so the
|
||||
"BFF output" is a static file; the `decisions` block stands in for what the backend
|
||||
would compute. Two slices were implemented to demonstrate **both** policy shapes:
|
||||
Implemented against the real backend, `backend/src/BigRegister.Api`. Two slices demonstrate
|
||||
**both** policy shapes.
|
||||
|
||||
**A. Dashboard profile → one aggregated, decision-enriched call (decision-flag).**
|
||||
|
||||
- Contract: `src/app/registratie/contracts/dashboard-view.dto.ts`
|
||||
- Endpoint: `GET /api/v1/dashboard-view` (`Program.cs`), one call replacing three.
|
||||
- Contract: `apps/ssp/src/app/registratie/contracts/dashboard-view.dto.ts`
|
||||
(`DashboardViewDto` = registration + person + `decisions`).
|
||||
- Endpoint: `public/mock/dashboard-view.json` (one call replaces three).
|
||||
- Boundary parse: `parseDashboardView()` in
|
||||
`src/app/registratie/infrastructure/dashboard-view.adapter.ts` validates the
|
||||
untrusted shape and maps DTO → domain (hand-written; no schema lib for one
|
||||
contract).
|
||||
- `BigProfileStore` now derives `profile` and `decisions` from the single
|
||||
validated view (was a 3-resource `map2`). One request → one consistent snapshot.
|
||||
- `herregistratie.page.ts` reads `decisions.eligibleForHerregistratie` instead of
|
||||
computing it client-side. That rule is server-owned: it lives only in
|
||||
`HerregistratieRule.cs`, with no FE mirror to drift from it (WP-75).
|
||||
- The unused upstream adapters/mocks (`brp.adapter.ts`, `registration.json`,
|
||||
`brp.json`) were deleted — those calls live behind the BFF now.
|
||||
`apps/ssp/src/app/registratie/infrastructure/dashboard-view.adapter.ts` validates the
|
||||
untrusted shape and maps DTO → domain (hand-written; no schema lib).
|
||||
- `BigProfileStore` derives `profile` and `decisions` from the single validated view (was a
|
||||
3-resource `map2`). One request → one consistent snapshot.
|
||||
- `herregistratie.page.ts` reads `decisions.eligibleForHerregistratie` instead of computing
|
||||
it client-side. That rule is server-owned: it lives only in `HerregistratieRule.cs`, with
|
||||
no FE mirror to drift from it (WP-75).
|
||||
|
||||
**B. Intake scholing threshold → config value.**
|
||||
|
||||
- Contract: `src/app/herregistratie/contracts/intake-policy.dto.ts`.
|
||||
- Endpoint: `public/mock/intake-policy.json` (`{ "scholingThreshold": 1000 }`).
|
||||
- Endpoint: `GET /api/v1/intake/policy` (`Program.cs`), serving
|
||||
`IntakePolicy.ScholingThreshold`.
|
||||
- Contract: the generated `IntakePolicyDto`; the adapter is
|
||||
`apps/ssp/src/app/herregistratie/infrastructure/intake-policy.adapter.ts`.
|
||||
- `intake.machine.ts`: the hardcoded `LAGE_UREN_DREMPEL` constant is gone;
|
||||
`lageUren(a, scholingThreshold)` and validation take the value, which lives in
|
||||
machine state and is set via a `SetPolicy` message. A `SCHOLING_THRESHOLD_DEFAULT`
|
||||
remains only as the offline fallback.
|
||||
`lageUren(a, scholingThreshold)` and validation take the value, which lives in machine
|
||||
state and is set via a `SetPolicy` message. A `SCHOLING_THRESHOLD_DEFAULT` remains only
|
||||
as the offline fallback.
|
||||
- `intake-wizard.component.ts` fetches the policy and dispatches `SetPolicy`.
|
||||
- WP-69: the backend re-validates the threshold as the authority on submit —
|
||||
`IntakePolicy.RejectIncompleteScholing` runs before `POST /applications/{id}/submit`
|
||||
(intake-typed) writes anything, 400ing an incomplete scholing answer instead of
|
||||
silently accepting a crafted POST that skips it. (WP-72 deleted the legacy
|
||||
`POST /intakes` endpoint this once also covered — deleting the surface is a stronger
|
||||
fix than 400ing on it.)
|
||||
(intake-typed) writes anything, 400ing an incomplete scholing answer instead of silently
|
||||
accepting a crafted POST that skips it. (WP-72 deleted the legacy `POST /intakes` endpoint
|
||||
this once also covered — deleting the surface is a stronger fix than 400ing on it.)
|
||||
|
||||
## Migration sequence (for the real app)
|
||||
|
||||
@@ -119,12 +171,17 @@ would compute. Two slices were implemented to demonstrate **both** policy shapes
|
||||
|
||||
## Out of scope here (next steps, not built in the worked example)
|
||||
|
||||
- Runtime DTO validation on **every** endpoint (only the dashboard view has it).
|
||||
- Optimistic-update race fix in `BigProfileStore`
|
||||
(`beginHerregistratie`/`rollbackHerregistratie` can leave `pending` wrong under
|
||||
concurrent submits).
|
||||
- Session persistence / multi-tab sync (`SessionStore` is in-memory).
|
||||
- Real OpenAPI/TypeSpec codegen toolchain.
|
||||
- Multi-tab session sync. The session itself now persists (`localStorage`, read back
|
||||
through `parseStoredPrincipal`), but a change in one tab does not reach another — no
|
||||
`storage` listener exists.
|
||||
|
||||
Two bullets were discharged and removed. Runtime DTO validation is no longer "only the
|
||||
dashboard view": 33 `parse*` boundary functions exist. The OpenAPI codegen toolchain is
|
||||
real: `npm run gen:api` generates `libs/shared/src/infrastructure/api-client.ts` and CI
|
||||
drift-checks it.
|
||||
|
||||
ponytail: build the pattern once on one slice; copy it across screens when the real
|
||||
backend lands, rather than scaffolding all of it up front.
|
||||
|
||||
@@ -19,28 +19,47 @@ layer — not a palette swap.
|
||||
## Decision
|
||||
|
||||
1. **Vendor the package** under `public/cibg-huisstijl/` (not an npm dep — it was delivered as files),
|
||||
loaded via a `<link>` in `src/index.html` so the CSS's relative `url(../fonts|icons|images)`
|
||||
references resolve at runtime. Storybook serves the same via `staticDirs`.
|
||||
2. **Token bridge over token rewrite.** `src/styles.scss` redefines the app's ~54 `--rhc-*` tokens
|
||||
loaded via a `<link>` in each app's `index.html` (`apps/ssp/src/index.html` and
|
||||
`apps/behandelportal/src/index.html` — two since WP-67) so the CSS's relative
|
||||
`url(../fonts|icons|images)` references resolve at runtime. `public/` stays at the repo
|
||||
root and both apps' `angular.json` targets copy it. Both Storybook instances serve the
|
||||
same via `staticDirs: ['../public']`.
|
||||
2. **Token bridge over token rewrite.** `libs/shared/styles.scss` — one copy, both apps'
|
||||
`angular.json` point at it (WP-67) — redefines the app's ~54 `--rhc-*` tokens
|
||||
onto CIBG values (`--bs-*` where one exists, CIBG palette hex otherwise). The `--rhc-*` names are
|
||||
now an internal alias set; the _values_ are CIBG. This avoided rewriting 300+ token references and
|
||||
keeps the "components reference tokens" convention intact. (`styles.scss` is exempt from
|
||||
`check:tokens`, so palette hex lives in that one file only.)
|
||||
keeps the "components reference tokens" convention intact. (`libs/shared/styles.scss` is
|
||||
exempt from `check:tokens`, so palette hex lives in that one file only.)
|
||||
3. **Re-skin atoms, keep their `input()` APIs.** Each `shared/ui` atom now emits Bootstrap/CIBG classes
|
||||
(`app-button` → `btn btn-primary`, `text-input` → `form-control`, radio/checkbox → `form-check-*`);
|
||||
domain pages compose the same atoms and barely changed.
|
||||
4. **Hand-roll what CIBG's build drops.** CIBG omits Bootstrap's `.alert` and `.navbar`, so `app-alert`
|
||||
is a small token-styled surface and the header/side-nav use `.nav` + a local blue bar. Local class
|
||||
names that collide with Bootstrap components were renamed (`.card` → `.app-card`, badge → `.status-badge`).
|
||||
4. **Hand-roll what CIBG's build drops, and mark it.** Where the vendored build has no class for a
|
||||
concept, the component is a small token-styled surface carrying a `// CIBG-GAP EXTENSION:` marker.
|
||||
The clearest live examples are `skeleton` and `spinner`: CIBG documents "Laadindicatie" but the
|
||||
vendored build ships no loading-skeleton or loading-spinner class, so both are built from the token
|
||||
bridge. Local class names that collide with Bootstrap components were renamed (`.card` → `.app-card`,
|
||||
badge → `.status-badge`).
|
||||
|
||||
Two claims this point used to make were wrong and are corrected here. **`.alert` is not a gap:**
|
||||
`app-alert` is a thin wrapper over the vendored `.feedback feedback-*` classes — the design system
|
||||
owns surface and icon, and the component adds only the icon's a11y label and a flex fix. It carries
|
||||
no gap marker, correctly. **The header is not hand-rolled either:** `site-header` composes the
|
||||
vendored `.titlebar` and `.logo__*` classes and leaves the robijn fill (`--ro-layout`) untouched.
|
||||
The `shell` template's `.layout`/`.main`/`.content` classes are page scaffolding, not a substitute
|
||||
for a missing design-system component, so they carry no marker either.
|
||||
|
||||
5. **System-font stack; no licensed fonts.** `--bs-font-sans-serif` is overridden to `system-ui`; the
|
||||
licensed RO/Rijks **text** woffs are removed from the vendored copy (CIBG icon font kept). Logo stays
|
||||
a text wordmark. Interactivity stays Angular-driven (no Bootstrap JS).
|
||||
|
||||
## Consequences
|
||||
|
||||
- Wiring the design system touches `styles.scss` (token bridge), `index.html`, `angular.json`
|
||||
(`public/` already copied), and `.storybook/` — plus the class strings in ~40 `shared/ui` +
|
||||
`shared/layout` + a few domain components. The `@rijkshuisstijl-community/*` deps are dropped.
|
||||
- Wiring the design system touches `libs/shared/styles.scss` (token bridge), both apps'
|
||||
`index.html`, both `angular.json` targets (`public/` already copied), and both Storybook config
|
||||
dirs (`.storybook-ssp/` and `.storybook-behandelportal/` — separate since WP-67, because a single
|
||||
merged tsconfig cannot resolve both apps' `@auth/*` at once) — plus the class strings in ~40
|
||||
`libs/shared/ui` + `libs/shared/layout` + a few domain components. The
|
||||
`@rijkshuisstijl-community/*` deps are dropped.
|
||||
- `check:tokens` still guards raw hex in components; the token bridge + hand-rolled surfaces comply.
|
||||
- Known benign build warning: _"Unable to locate stylesheet: /cibg-huisstijl/css/huisstijl.min.css"_ —
|
||||
Angular's index optimizer doesn't process a `public/` stylesheet at build time. The asset is copied
|
||||
@@ -49,6 +68,6 @@ layer — not a palette swap.
|
||||
intentionally dropped, so we accept the warning.
|
||||
- Renaming the internal token names from `--rhc-*` to `--app-*` is possible later but out of scope.
|
||||
- Hand-rolled components (point 4) are tracked in the **CIBG gap register**
|
||||
(`src/docs/cibg-gaps.mdx`, Storybook "Foundations/CIBG Gap Register"): every deviation from the
|
||||
design system carries a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than silently
|
||||
drifting.
|
||||
(`libs/shared/docs/cibg-gaps.mdx`, Storybook "Foundations/CIBG Gap Register"): every deviation
|
||||
from the design system carries a `// CIBG-GAP EXTENSION:` marker so it's auditable rather than
|
||||
silently drifting.
|
||||
|
||||
@@ -20,7 +20,7 @@ was neither isolated nor validated:
|
||||
- All reference data and thresholds are **compiled-in C# constants**, served through
|
||||
screen-shaped BFF-lite endpoints; the frontend renders decisions and holds no reference
|
||||
data (ADR-0001).
|
||||
- User-facing UI copy is already **`$localize`** (`src/locale/*.xlf`) — git-tracked, and a
|
||||
- User-facing UI copy is already **`$localize`** (`apps/<app>/src/locale/*.xlf`) — git-tracked, and a
|
||||
second locale is a translation file, not a code change. That is already the compile-time
|
||||
model for text.
|
||||
- The profession↔diploma map lived as a _private_ `Dictionary` inside `DiplomaRules`, mixed
|
||||
@@ -62,17 +62,49 @@ production database, never runtime-editable.
|
||||
| Kind | Home | Gate |
|
||||
| ------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- |
|
||||
| Reference tables + tunable numbers (professions↔diplomas, thresholds, policy questions, document categories) | `Stamdata/` typed C# **or** typed JSON data-file (`professions.json`), optionally valid-timed | compiler (shape; + values when C#) + `StamdataValidationTests` (values, references, validity windows) |
|
||||
| User-facing UI copy | `$localize` → `src/locale/*.xlf` | build (`i18nMissingTranslation: error`) |
|
||||
| User-facing UI copy | `$localize` → `apps/<app>/src/locale/*.xlf` | build (`i18nMissingTranslation: error`) |
|
||||
| Letter / brief passage content | config-as-code in the backend (seed content), **not** the DB | compiler + endpoint tests |
|
||||
|
||||
### The deliberate exception: org-templates
|
||||
### The deliberate exception: operational configuration
|
||||
|
||||
Per-organization letterhead (return address, footer, signature, margins) **is**
|
||||
runtime-editable in SQLite, via the org-template admin editor (WP-23/26). That is
|
||||
intentional and does not contradict this ADR: it is _operational configuration_ owned by an
|
||||
admin persona, versioned with publish/rollback inside the app, and specific to one
|
||||
sub-organization's identity — not the shared business rules a wrong value would break for
|
||||
everyone. Stamdata (the rules and reference tables the whole register runs on) stays code.
|
||||
"Never runtime-editable" above is the rule for **stamdata** — the shared reference tables
|
||||
and business rules the whole register runs on. It is not a ban on all persisted
|
||||
configuration. Some configuration is operational rather than business-rule, and belongs to
|
||||
an admin persona at runtime.
|
||||
|
||||
This section states the **test** rather than a list, so the next surface can check itself
|
||||
instead of arguing by analogy. Runtime-editable persistence is permitted only when all four
|
||||
hold:
|
||||
|
||||
1. **The catalog lives in code.** What may be set — the keys, the schema, the defaults,
|
||||
the descriptions — is compiled in and reviewed through git. The store holds values, never
|
||||
the definition of what a value means.
|
||||
2. **An unknown or unlisted key fails closed.** A row the code catalog does not know cannot
|
||||
invent a setting, enable a feature, or be written. A bad row is inert, not authoritative.
|
||||
3. **The value is operational.** Per-organisation identity, or an on/off rollout switch —
|
||||
not a shared business rule whose wrong value breaks the register for everyone. This is the
|
||||
clause that keeps stamdata out.
|
||||
4. **Writes are admin-capability-gated and audited.** The write path goes through an `Authz`
|
||||
capability gate, and the gate records the decision — allow as well as deny — in
|
||||
`AuthzAuditStore`.
|
||||
|
||||
**Two surfaces pass this test today.**
|
||||
|
||||
| Surface | (1) catalog in code | (2) fails closed | (3) operational | (4) gated + audited |
|
||||
| ----------------------------- | ----------------------------------------------- | ------------------------------------------------------------ | --------------------------------- | ------------------------------- |
|
||||
| `OrgTemplateStore` (WP-23/26) | the `OrgTemplateDto` shape + `OrgTemplateRules` | unknown `subOrgId` → `null` → the endpoint 404s | one sub-organisation's letterhead | `OrgAdmin` → `orgtemplate:edit` |
|
||||
| `FeatureFlagStore` (WP-47) | `Domain/Features/FeatureFlags.Catalog` | unknown key → `Set` returns false (404); `IsEnabled` → false | an on/off rollout switch | `FlagsAdmin` → `flags:manage` |
|
||||
|
||||
Clause (4) became true for both only with RB-07, which moved `AuditAuthz` from each gate's
|
||||
deny branch into the gate itself so the allow path is recorded too. Before that, both
|
||||
surfaces were gated and **not** audited, and this ADR would have ratified a control the code
|
||||
did not implement.
|
||||
|
||||
Org-templates also carry publish/rollback versioning inside the app, which is stronger than
|
||||
the test requires but not part of it.
|
||||
|
||||
Stamdata itself — the rules and reference tables — fails clause (3) by construction and
|
||||
stays code.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
Reference in New Issue
Block a user