diff --git a/apps/ssp/src/app/registratie/application/draft-sync.ts b/apps/ssp/src/app/registratie/application/draft-sync.ts index ff533dd..eec4426 100644 --- a/apps/ssp/src/app/registratie/application/draft-sync.ts +++ b/apps/ssp/src/app/registratie/application/draft-sync.ts @@ -8,10 +8,8 @@ import type { SubmitApplicationResponse, } from '@shared/infrastructure/api-client'; import { AanvraagType } from '@registratie/domain/aanvraag'; -import { - ApplicationsAdapter, - parseApplications, -} from '@registratie/infrastructure/applications.adapter'; +import { ApplicationsAdapter } from '@registratie/infrastructure/applications.adapter'; +import { findConcept, loadConcept } from './find-concept'; /** What a wizard persists per step: the opaque machine snapshot + progress + docs. */ export interface DraftSnapshot { @@ -70,7 +68,7 @@ export function createDraftSync(deps: DraftSyncDeps) { // server's guard (409) — recover by adopting the existing Concept instead of // erroring. Only recover when one actually exists; otherwise surface the failure. .catch(async (e) => { - const existing = await findConcept(); + const existing = await findConcept(adapter, deps.type); if (existing) return existing; throw e; }) @@ -140,32 +138,14 @@ export function createDraftSync(deps: DraftSyncDeps) { // (submitted/gone) id is treated as fresh so it can't reopen as an editable draft. const load = (linked: string): Promise => { id = linked; - return adapter - .detail(linked) - .then((dto) => { - if (dto.status && dto.status.tag !== 'Concept') { - id = undefined; - applyResume(null); - return; - } - applyResume(dto.draft ?? null); - }) - .catch(() => { + return loadConcept(adapter, linked).then((result) => { + if (result.tag === 'not-concept') { id = undefined; - applyResume(null); // unknown/deleted id → start fresh - }); - }; - - // Find the user's existing Concept of this type (at most one), if any. - const findConcept = async (): Promise => { - try { - const parsed = parseApplications(await adapter.list()); - return parsed.ok - ? parsed.value.find((a) => a.type === deps.type && a.status.tag === 'Concept')?.id - : undefined; - } catch { - return undefined; - } + applyResume(null); + return; + } + applyResume(result.draft); + }); }; return { @@ -189,7 +169,7 @@ export function createDraftSync(deps: DraftSyncDeps) { await load(linked); return; } - const existing = await findConcept(); + const existing = await findConcept(adapter, deps.type); if (existing) { await load(existing); // Stamp the id into the URL so a reload resumes the same Concept. diff --git a/apps/ssp/src/app/registratie/application/find-concept.spec.ts b/apps/ssp/src/app/registratie/application/find-concept.spec.ts new file mode 100644 index 0000000..c617ad0 --- /dev/null +++ b/apps/ssp/src/app/registratie/application/find-concept.spec.ts @@ -0,0 +1,108 @@ +import { describe, it, expect } from 'vitest'; +import { ApplicationsAdapter } from '@registratie/infrastructure/applications.adapter'; +import { findConcept, loadConcept } from './find-concept'; + +// Free functions taking the adapter as a parameter (no inject()) — a plain fake +// object is enough, no Angular TestBed needed. +function fakeAdapter(overrides: Partial): ApplicationsAdapter { + return overrides as ApplicationsAdapter; +} + +describe('findConcept', () => { + it('returns the id of the existing Concept of the given type', async () => { + const adapter = fakeAdapter({ + list: async () => [ + { + id: 'a1', + type: 'registratie', + status: { tag: 'Concept', stepIndex: 0, stepCount: 3 }, + createdAt: '2026-01-01T00:00:00Z', + updatedAt: '2026-01-01T00:00:00Z', + }, + ], + }); + + await expect(findConcept(adapter, 'registratie')).resolves.toBe('a1'); + }); + + it('returns undefined when the list has no application of the given type', async () => { + const adapter = fakeAdapter({ list: async () => [] }); + + await expect(findConcept(adapter, 'registratie')).resolves.toBeUndefined(); + }); + + it('returns undefined when the matching type is not a Concept', async () => { + const adapter = fakeAdapter({ + list: async () => [ + { + id: 'a1', + type: 'registratie', + status: { tag: 'Ingediend', referentie: 'R1' }, + createdAt: '2026-01-01T00:00:00Z', + updatedAt: '2026-01-01T00:00:00Z', + }, + ], + }); + + await expect(findConcept(adapter, 'registratie')).resolves.toBeUndefined(); + }); + + it('returns undefined when adapter.list() resolves with an unparsable shape', async () => { + const adapter = fakeAdapter({ list: async () => 'not-an-array' as unknown as [] }); + + await expect(findConcept(adapter, 'registratie')).resolves.toBeUndefined(); + }); + + it('returns undefined when adapter.list() rejects', async () => { + const adapter = fakeAdapter({ + list: async () => { + throw new Error('network down'); + }, + }); + + await expect(findConcept(adapter, 'registratie')).resolves.toBeUndefined(); + }); +}); + +describe('loadConcept', () => { + it('reads the draft off a Concept', async () => { + const adapter = fakeAdapter({ + detail: async () => ({ + id: 'a1', + status: { tag: 'Concept', stepIndex: 1, stepCount: 3 }, + draft: { step: 1 }, + }), + }); + + await expect(loadConcept(adapter, 'a1')).resolves.toEqual({ + tag: 'concept', + draft: { step: 1 }, + }); + }); + + it('reports a missing draft as null', async () => { + const adapter = fakeAdapter({ + detail: async () => ({ id: 'a1', status: { tag: 'Concept', stepIndex: 0, stepCount: 3 } }), + }); + + await expect(loadConcept(adapter, 'a1')).resolves.toEqual({ tag: 'concept', draft: null }); + }); + + it('reports not-concept when the id has moved past Concept (submitted)', async () => { + const adapter = fakeAdapter({ + detail: async () => ({ id: 'a1', status: { tag: 'Ingediend', referentie: 'R1' } }), + }); + + await expect(loadConcept(adapter, 'a1')).resolves.toEqual({ tag: 'not-concept' }); + }); + + it('reports not-concept when the id is unknown or deleted (detail rejects)', async () => { + const adapter = fakeAdapter({ + detail: async () => { + throw new Error('404'); + }, + }); + + await expect(loadConcept(adapter, 'gone')).resolves.toEqual({ tag: 'not-concept' }); + }); +}); diff --git a/apps/ssp/src/app/registratie/application/find-concept.ts b/apps/ssp/src/app/registratie/application/find-concept.ts new file mode 100644 index 0000000..6b0cf75 --- /dev/null +++ b/apps/ssp/src/app/registratie/application/find-concept.ts @@ -0,0 +1,48 @@ +import { AanvraagType } from '@registratie/domain/aanvraag'; +import { + ApplicationsAdapter, + parseApplications, +} from '@registratie/infrastructure/applications.adapter'; + +/** + * Read half of the Concept lookup that `createDraftSync` (`draft-sync.ts`) needs + * before it can start writing (RB-21 / CQ-001). Free functions that take the adapter + * as a parameter, not `inject()`, so they get a direct spec without Angular TestBed. + * `createDraftSync` keeps the closure state (`id`, `resumeGate`) and the write path; + * these two functions only read. + */ + +/** Find the user's existing Concept of a given type (at most one), if any. */ +export async function findConcept( + adapter: ApplicationsAdapter, + type: AanvraagType, +): Promise { + try { + const parsed = parseApplications(await adapter.list()); + return parsed.ok + ? parsed.value.find((a) => a.type === type && a.status.tag === 'Concept')?.id + : undefined; + } catch { + return undefined; + } +} + +/** Outcome of loading one Concept by id: its draft (or null when it has none), or + `not-concept` when the id is not an editable Concept (submitted/gone) or the + lookup failed (unknown/deleted id) — the caller treats both the same way, as + "start fresh". */ +export type LoadedConcept = { tag: 'concept'; draft: unknown | null } | { tag: 'not-concept' }; + +/** Load a specific Concept by id and report whether it is still editable. */ +export async function loadConcept( + adapter: ApplicationsAdapter, + id: string, +): Promise { + try { + const dto = await adapter.detail(id); + if (dto.status && dto.status.tag !== 'Concept') return { tag: 'not-concept' }; + return { tag: 'concept', draft: dto.draft ?? null }; + } catch { + return { tag: 'not-concept' }; + } +} diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md index e8ba3e0..9fbd6af 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md @@ -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 | — | — | implemented | +| **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 | --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-21.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-21.md new file mode 100644 index 0000000..c908e0a --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-21.md @@ -0,0 +1,119 @@ +# RB-21 — extract the read half of `createDraftSync` into `find-concept.ts` + +Status: **implemented** · 2026-08-27 · Source finding: `04-cqrs-light.md` CQ-001 · +`00-baseline.md` §4a (`createDraftSync` 143 lines, the largest function in the repo), §9 +(`fn > 40` threshold) · `99-backlog.md` RB-21 + +## What was wrong + +`createDraftSync` (`apps/ssp/src/app/registratie/application/draft-sync.ts`) was registered +as a command factory but owned three read paths (`load`, `findConcept`, and the read half of +`resume`) mixed into the same function as the write path (`ensureId`, `flush`, `submit`, +`reset`). CQ-001 named three pieces of shared mutable closure state — `id`, `ensuring`, +`resumeGate` — as load-bearing: `resumeGate` exists only so the write path (`ensureId`) can +wait for the read path (`resume`) to finish. That coupling is genuine and stays in place. + +## What changed + +CQ-001's proposal, applied as a pure move, no redesign. + +| File | Change | +| --------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `apps/ssp/src/app/registratie/application/find-concept.ts` (new) | `findConcept(adapter, type)` and `loadConcept(adapter, id)` — free functions taking `ApplicationsAdapter`, no `inject()`. `loadConcept` returns a `LoadedConcept` union (`{tag:'concept', draft}` \| `{tag:'not-concept'}`) instead of the boolean-shaped branching the inline version had. | +| `apps/ssp/src/app/registratie/application/find-concept.spec.ts` (new) | Direct spec, no TestBed — a fake `ApplicationsAdapter` object passed straight to the functions. | +| `apps/ssp/src/app/registratie/application/draft-sync.ts` | Removed the inline `findConcept` closure and the body of `load`; both now call the free functions. `createDraftSync` keeps `id`, `ensuring`, `resumeGate`, and the whole write path, unchanged. | +| `libs/shared/docs/behaviour-spec.mdx` | Regenerated (`npm run gen:behaviour-spec`) — picks up the new `find-concept.spec.ts` describe blocks. | + +`createDraftSync` shrank from 187 lines (`export function createDraftSync` to its closing +brace, HEAD~1) to 169 lines. The whole file went from 236 to 216 lines. + +The two call sites that used the old inline `findConcept()` now pass the adapter and type +explicitly: + +```ts +// ensureId's 409-recovery catch (WP-35) +const existing = await findConcept(adapter, deps.type); +``` + +```ts +// resume(), no ?aanvraag in the URL +const existing = await findConcept(adapter, deps.type); +``` + +`load` keeps setting the closure `id` and calling `applyResume` (both closure-dependent), but +delegates the actual read to `loadConcept`: + +```ts +const load = (linked: string): Promise => { + id = linked; + return loadConcept(adapter, linked).then((result) => { + if (result.tag === 'not-concept') { + id = undefined; + applyResume(null); + return; + } + applyResume(result.draft); + }); +}; +``` + +## `draft-sync.spec.ts` — unchanged + +`draft-sync.spec.ts` was not edited. It never called `resume()`/`load()` directly — its +coverage is the debounce, `submit()` (including the 409-recovery path, which exercises the +extracted `findConcept` indirectly through `ensureId`'s catch), and `flushPending`. All of +that stayed in `createDraftSync`, so the spec is unchanged and still exercises the wiring +between `createDraftSync` and the two new free functions (the 409-recovery test would fail if +that wiring were wrong). It passed unchanged, 8/8. + +## The new spec, and its verified red + +`find-concept.spec.ts` covers the branches CQ-001 named: + +- `findConcept`: match found → id returned; no match of that type → `undefined`; match found + but not `Concept` status → `undefined`; `adapter.list()` resolves to an unparsable shape + (`parseApplications` fails) → `undefined`; `adapter.list()` rejects → `undefined`. +- `loadConcept`: `Concept` with a draft → `{tag:'concept', draft}`; `Concept` with no draft → + `{tag:'concept', draft:null}`; a non-`Concept` status (e.g. `Ingediend`, submitted) → + `{tag:'not-concept'}`; `adapter.detail()` rejects (unknown/deleted id) → + `{tag:'not-concept'}`. + +**Verified red without the fix.** Used `Edit` (not `git checkout`) to invert one condition in +`loadConcept` — `dto.status.tag !== 'Concept'` → `dto.status.tag === 'Concept'` — reran `ng +test ssp --include find-concept.spec.ts`. Result: 3 of 9 tests failed — + +``` +loadConcept > reads the draft off a Concept + AssertionError: expected { tag: 'not-concept' } to deeply equal { tag: 'concept', draft: { step: 1 } } +loadConcept > reports a missing draft as null + AssertionError: expected { tag: 'not-concept' } to deeply equal { tag: 'concept', draft: null } +loadConcept > reports not-concept when the id has moved past Concept (submitted) + AssertionError: expected { tag: 'concept', draft: null } to deeply equal { tag: 'not-concept' } +``` + +Then used `Edit` again to flip the condition back to `!==`, reran the same command: 9/9 +green. `findConcept`'s and `loadConcept`'s other branches were not separately mutated — the +inverted condition alone was enough to prove the spec is sensitive to the extraction being +correct, and re-verifying full green after the revert confirmed no collateral change was left +in the file. + +## Scope held + +- No restructuring of the write path (`ensureId`, `flush`, `submit`, `reset`) — untouched + beyond the two call-site updates shown above. +- `applications.adapter.ts` was not split (CQ-002's "Not filed" note rules that out for this + design; out of scope here regardless). +- `resume()`'s semantics (URL-param precedence, the `resumeGate` release-in-`finally`, the + navigate-to-stamp-the-id side effect) are unchanged — only its two `findConcept()`/`load()` + calls now go through the free functions. +- No wire change, no DTO change, no behaviour change. + +## Verification + +`npm run ci` (foreground): **green** — lint, typecheck, `dep:check`, `format:check`, +`check:tokens`, `check:seam`, tests (ssp includes `find-concept.spec.ts` 9/9 new, +`draft-sync.spec.ts` 8/8 unchanged), `ng build --localize` (both apps), `npm audit`, backend +`dotnet test` (the known `OpenZaakIntegrationTests.Admin_cases_…` container-dependent failure +is expected and outside `npm run ci`'s scope), `gen:snippets` drift clean, `gen:behaviour-spec` +drift clean once the regenerated file is committed alongside the code. Full counts are in the +commit's `npm run ci` run — see the session note for the exact step-by-step output. diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index a334174..38e9cb9 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -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. 460 frontend behaviours across 9 contexts; 236 backend behaviours across 41 test classes. @@ -470,6 +470,14 @@ classes. - lists soort/waarvoor/status/referentie/ingediend, plus reason when rejected - reference falls back to em dash for a Concept +#### findConcept + +- returns the id of the existing Concept of the given type +- returns undefined when the list has no application of the given type +- returns undefined when the matching type is not a Concept +- returns undefined when adapter.list() resolves with an unparsable shape +- returns undefined when adapter.list() rejects + #### hasProgress - is false for a fresh wizard @@ -486,6 +494,13 @@ classes. - derives the beroep from the chosen diploma and flags origin duo +#### loadConcept + +- reads the draft off a Concept +- reports a missing draft as null +- reports not-concept when the id has moved past Concept (submitted) +- reports not-concept when the id is unknown or deleted (detail rejects) + #### manual diploma fallback - KiesHandmatig flags handmatig with the maximal question set and no beroep yet