Merge RB-21 — extract the read half of createDraftSync into find-concept.ts
CQ-001: createDraftSync was the longest function in the repo and owned three query paths next to its write path. findConcept and loadConcept are now free functions that take the adapter, so they have a direct spec without TestBed. The closure state (id, ensuring, resumeGate) stays where it was, because the coupling is load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -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<void> => {
|
||||
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.
|
||||
Reference in New Issue
Block a user