refactor(registratie): extract the read half of createDraftSync (RB-21)
createDraftSync mixed a read path (findConcept, load, the read half of resume) with its write path (ensureId, flush, submit, reset) in one 187-line function -- CQ-001's finding. Move findConcept and loadConcept into a new application/find-concept.ts as free functions that take the adapter, so they get a direct spec with no Angular TestBed. createDraftSync keeps the closure state (id, ensuring, resumeGate) and the whole write path unchanged -- this is a move, not a redesign. The resumeGate coupling that lets the write path wait for the read path stays exactly where it was. createDraftSync shrinks from 187 to 169 lines. draft-sync.spec.ts is unchanged -- it never called resume()/load() directly, and its 409 recovery test for submit() still exercises the extracted findConcept through ensureId's catch branch. 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