Files
atomic-design-poc/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-19.md
T
ehoandClaude Opus 5 e270b8612f refactor(backend): reorder Program.cs sections into reads-then-writes (RB-19)
CQ-006 found that Program.cs states a reads-then-writes principle at the
top of the file, then abandons it for five feature sections that mix GET
and mutating endpoints in mapping order. This is a pure reorder: within
Document upload, Applications, Admin cases, Brief, and Organization
templates, every GET now precedes every POST/PUT/DELETE, each split by a
`--- reads ---`/`--- writes ---` sub-banner in the style WP-65 already
established for Beoordeling/Besluit.

DELETE /admin/cases/{id} and GET /admin/audit move up beside GET
/admin/cases, closing the 129-line gap CQ-006 measured. GET
/admin/org-template/{subOrgId}/preview moves from the Brief section to
the Organization-templates section it actually belongs to.

No route, signature, DTO, or handler body changed. Every block was cut
by exact line-range slicing, never retyped. The sorted list of mapped
HTTP-method-plus-path strings is byte-identical before and after; every
.Gate(...) count is unchanged; the three routes that moved with a gate
were checked by eye against the wrapper their handler actually calls,
per RB-12's stated limitation that the route-table test only proves a
marker is present, not that it still matches the handler.

npm run gen:api regenerated backend/swagger.json and
libs/shared/src/infrastructure/api-client.ts; both diffs are ordering
only (sorted-file diff is empty), committed alongside per the ticket's
own guidance.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 19:16:05 +02:00

16 KiB

RB-19 — reorder Program.cs: reads before writes per section, regroup admin-cases + org-template preview

Status: implemented · 2026-08-27 · Source findings: 04-cqrs-light.md CQ-006 · 99-backlog.md RB-19 · Depends on implementation/rb-12.md (the route-table test this ticket leans on as its regression net)

This is a pure reorder. No route, signature, DTO, or handler-body text changed. The sorted list of HTTP METHOD + path mapping calls is byte-identical before and after (see "Verification" below) — that identity is the strongest evidence this ticket did what it says and nothing else.

What was wrong

CQ-006, verbatim: Program.cs opens by declaring direction as its organising principle (a "GET: screen-shaped reads" banner, then a "POST: submits" banner), then from the Document-upload section onward switches to feature grouping without saying so, and every subsequent section interleaves reads and writes. One feature (Beoordeling/Besluit, WP-65) already got the fix — a :441/:464-style banner pair splitting its query endpoint from its command endpoint — and CQ-006 asks for the same treatment on the five sections that predate that pattern: Document upload, Applications, Admin cases, Brief, and Organization templates. Separately, DELETE /admin/cases/{id} sat 129 lines away from GET /admin/cases, with werkvoorraad, beoordeling, besluit and the ZGW notification hook in between; and GET /admin/org-template/{subOrgId}/preview was filed under the Brief section's banner instead of the Org-templates section it actually belongs to.

What changed

Section (banner) Before After
Document upload categories, POST /uploads, content, status, DELETE, admin-DELETE categories, content, status, --- reads ---/--- writes --- sub-banners, POST /uploads moved after the reads, DELETE, admin-DELETE
Applications already reads-first (2 GETs, then POST/PUT/DELETE/POST-submit) unchanged order; sub-banners inserted only
Admin cases GET /admin/cases, (werkvoorraad/beoordeling/besluit/zgw-notificaties in between), DELETE /admin/cases/{id}, GET /admin/audit GET /admin/cases, GET /admin/audit (moved up), --- writes ---, DELETE /admin/cases/{id} (moved up) — all three now contiguous; werkvoorraad/beoordeling/besluit/zgw-notificaties follow, unmoved and unchanged
Brief GET /brief, PUT, submit, approve, reject, send, reveal-bignummer, GET /brief/preview, (org-template preview), POST /reset GET /brief, GET /brief/preview (moved up beside GET /brief), --- writes ---, PUT, submit, approve, reject, send, reveal-bignummer, POST /reset — org-template preview removed from this section
Organization templates list, detail, PUT, publish, rollback list, detail, GET /admin/org-template/{subOrgId}/preview (moved in from Brief), --- writes ---, PUT, publish, rollback

Every section above got a // --- reads --- / // --- writes --- sub-banner pair (matching the short, bare form already used at the file's top-level :170/:236 banners) inserted at the reads→writes boundary. Werkvoorraad, Beoordeling and Besluit — not named by CQ-006 as mixed, and already correctly split (Beoordeling is the read, Besluit is the write, each with its own WP-65 banner) — were left exactly as they were, including their absolute position relative to each other; only the block ahead of them (admin-cases) grew, pushing their line numbers down without touching their content.

backend/swagger.json and libs/shared/src/infrastructure/api-client.ts were regenerated (npm run gen:api) and are part of this commit — see "The regenerated pair" below.

Design: line-range slicing, not manual retyping

Every moved block was cut with a Python script operating on exact 1-indexed line ranges against the file as it stood after merging in refactor/adr-c-006-shared-route-guards (this branch's actual base — see "Base commit" below), then reassembled in the new order. No handler body was retyped by hand. This is the same guarantee the ticket's "cut/paste, not retype" instruction asks for, made structural rather than a promise to be careful: a line-range slice cannot silently change a character inside a block it does not touch. The script is not part of this commit (a one-shot tool, not project code); the diff it produced is what is being reviewed.

Judgement calls

  • Sub-banner wording is bare // --- reads --- / // --- writes ---, not prose matching WP-65's descriptive style. The ticket asks for ":441/:464-style" banners; WP-65's actual banners are long, feature-specific paragraphs ("read side only (recording a decision is WP-65's second half)…"). Inventing five more paragraphs like that would mean writing new explanatory prose about code this ticket is not meant to re-explain — CQ-006 is explicit that this is "a structure finding, not a correctness one," and the ticket itself forbids "no fixed comments beyond the banners this ticket adds." The file's own top-level banners (:170 "GET: screen-shaped reads", :236 "POST: submits") already establish a bare, label-only banner as a legitimate style in this exact file — the sub-banners here are that same style, nested one level deeper.
  • Werkvoorraad/Beoordeling/Besluit end up sandwiched between Admin-cases and zgw/notificaties, in that order, unmoved. Moving GET /admin/audit and DELETE /admin/cases/{id} up next to GET /admin/cases (as instructed) necessarily pushes everything that used to sit between them — werkvoorraad, beoordeling, besluit, zgw/notificaties — down, but does not reorder those four relative to each other. They were not named as mixed by CQ-006 and were not touched beyond their line numbers changing.
  • GET /brief/preview and POST /uploads are both .ExcludeFromDescription()-marked (hand-written FE fetch/XHR calls, never through the generated client) — moving them produced zero diff in swagger.json. This is not a coincidence being reported as one: an excluded endpoint has no OpenAPI operation to reorder in the first place, so the regenerated pair's diff below is smaller than "every moved route" might suggest — it only shows the two endpoints that are both documented and reordered relative to each other (GET /admin/audit, DELETE /admin/cases/{id}).
  • No handler types, no Features/ folder, no mediator — out of mandate per CQ-006's own text (filed separately as OOM-A) and the ticket's explicit "out of scope" section. Nothing beyond comments and mapping order changed.

Base commit

Step zero's warning matched this worktree's actual starting state: git log --oneline -8 showed ae7781e at HEAD, not edd20c0, and edd20c0 docs(backlog): mark RB-23 done after merge was absent from the log entirely — the bad-base lineage named in the ticket. git merge refactor/adr-c-006-shared-route-guards was run, after which edd20c0 appeared as HEAD~0's direct ancestor and every RB-01..RB-23 commit was present. All work in this ticket happened after that merge.

Verification

The sorted-route-list diff (the key evidence). Extracted every .Map(Get|Post|Put| Delete)("...") call from Program.cs before and after, sorted each list, and diffed them:

$ grep -oE '\.Map(Get|Post|Put|Delete)\("[^"]*"' Program.cs.before-reorder | sort > before.txt
$ grep -oE '\.Map(Get|Post|Put|Delete)\("[^"]*"' Program.cs | sort > after.txt
$ diff before.txt after.txt
$ echo "exit=$?"
exit=0
$ wc -l before.txt after.txt
  47 before.txt
  47 after.txt

Empty diff, same count (47 api.Map* calls — the two app.MapGet health probes are outside the /api/v1 group and were never in scope for this reorder; they were untouched either way). The set of routes is provably unchanged.

.Gate(...) count, before/after, by wrapper name:

      3 .Gate("Beoordelen")
      4 .Gate("CasesAdmin")
      1 .Gate("FlagsAdmin")
      6 .Gate("OrgAdmin")
      2 .Gate("StamdataAdmin")

Identical in both directions — no gate call was added, removed, or renamed.

Per-route eyeball check of every route that changed position, per RB-12's stated limitation (the route-table test only proves a .Gate(...) marker is present, not that it still names the wrapper the handler body actually calls):

Route Moved .Gate(...) after Wrapper actually called inside the handler Match
GET /admin/audit up, beside GET /admin/cases CasesAdmin CasesAdmin(ctx, () => ...) yes
DELETE /admin/cases/{id} up, beside GET /admin/cases CasesAdmin CasesAdmin(ctx, () => { ... }) yes
GET /admin/org-template/{subOrgId}/preview Brief section → Org-templates section OrgAdmin OrgAdmin(ctx, () => { ... }) yes
POST /uploads within Document-upload, past the three reads (none — allow-listed, ownership-scoped) n/a
GET /brief/preview within Brief, up beside GET /brief (none — allow-listed, ownership-scoped) n/a
GET /uploads/{documentId}/content incidental one-slot shift (POST /uploads moved past it) (none — allow-listed) n/a
GET /uploads/status incidental one-slot shift (POST /uploads moved past it) (none — allow-listed) n/a

Five routes were deliberately relocated by this ticket; two more shifted position only as a byproduct of POST /uploads moving past them (their own order relative to each other is unchanged). All three gated routes among these were checked by eye against the handler body they wrap, not just against RouteInventoryTests' marker check — all three match.

RouteInventoryTests:

Passed!  - Failed: 0, Passed: 2, Skipped: 0, Total: 2, Duration: 770 ms

Both Every_mapped_route_is_authz_gated_or_on_the_named_allow_list and Every_gate_marker_names_a_known_admin_wrapper pass.

Full backend suite: dotnet test --filter "Category!=Integration"262/262 passing, plus the one known, pre-existing, container-dependent failure (OpenZaakIntegrationTests.Admin_cases_returns_the_seeded_zaak_mapped_through_real_HTTP_and_JWT), which does not run under npm run ci and reproduces on a clean tree with no OpenZaak container running — not this ticket's bug.

dotnet build (both projects): 0 warnings, 0 errors. dotnet format BigRegister.slnx --verify-no-changes: clean.

No new test was added. Per the ticket's Definition of Done: this is a zero-semantic- change commit, and §3c's pre-existing 97.4% line / 84.8% branch coverage of Program.cs is the regression net CQ-006 itself names. Nothing about this diff needs a new test to be trustworthy — a passing pre-existing suite plus an empty sorted-route diff is stronger evidence for "nothing changed" than a new test asserting the same thing would be.

The regenerated pair

npm run gen:api was run after the reorder. It produced a diff in both backend/swagger.json (2 hunks) and libs/shared/src/infrastructure/api-client.ts (5 hunks) — both pure reordering, zero content change. Confirmed by sorting every line of each file (before vs. after) and diffing the sorted output: empty in both cases. The only two OpenAPI paths that moved position in the document are /api/v1/admin/audit and /api/v1/admin/cases/{id} — the two documented (non-ExcludeFromDescription) endpoints this ticket actually reordered relative to their OpenAPI-document neighbours; the generated client's audit()/cases() methods and their process* helpers moved by the same amount, unchanged in every other respect (parameters, return types, status-code branches, JSDoc). Both regenerated files are committed alongside Program.cs, per the ticket's explicit instruction: "if the only change is ordering inside swagger.json, say so explicitly and commit the regenerated pair rather than leaving CI's drift job to fail."

npm run ci: every job through "backend dependency audit" passed before this ticket's files were committed; the one job that legitimately failed pre-commit was "api- client drift" (git diff --exit-code against the not-yet-committed regenerated files — expected, since that step compares the working tree to HEAD, and HEAD still had the pre-reorder client at that point). After committing, npm run ci was re-run to confirm a clean, fully green result against the committed tree — see the final PASS/exit-code reported in this ticket's closing message.

What a reviewer should check

This diff is too large to read top-to-bottom without guidance. The fastest way to review it with confidence:

  1. Trust the sorted-route diff, not a manual read of every hunk. The "Verification" section above shows the set of HTTP METHOD + path strings is byte-identical before and after. If you want to reproduce it yourself: check out this commit's parent, extract the same grep -oE pattern from both revisions of Program.cs, sort, diff.
  2. Spot-check the five per-route table entries above, not the whole file — those are the only routes whose position (and, for three of them, gate-vs-handler match) actually matters for this ticket's correctness claim.
  3. Diff git show <this-commit> -- backend/src/BigRegister.Api/Program.cs with whitespace-insensitive word diff (git diff -w --color-words) if you want to confirm no character inside a moved handler body changed — the line-range-slicing approach in "Design" above makes this a formality rather than a real risk, but it is cheap to re-check.
  4. Do not expect Werkvoorraad/Beoordeling/Besluit/zgw-notificaties to have moved position relative to each other — only their absolute line numbers shifted, as a side effect of the admin-cases block growing above them.
  5. The regenerated swagger.json/api-client.ts diff is expected and pre-verified as ordering-only (sorted-file diff is empty) — it does not need a second manual read.