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>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,220 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user