From e270b8612f19ae48eddc09fe3b75eb5108cb908b Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 19:15:32 +0200 Subject: [PATCH] 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 --- backend/src/BigRegister.Api/Program.cs | 181 +++++++------- backend/swagger.json | 134 +++++------ .../refactor-backlog/99-backlog.md | 70 +++--- .../refactor-backlog/implementation/rb-19.md | 220 ++++++++++++++++++ libs/shared/src/infrastructure/api-client.ts | 176 +++++++------- 5 files changed, 511 insertions(+), 270 deletions(-) create mode 100644 docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-19.md diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index 7fbea17..f0f6ae8 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -242,36 +242,12 @@ api.MapPost("/change-requests", (ChangeRequestRequest req, HttpContext ctx) => // --- Document upload --- +// --- reads --- + // Server-owned category config per wizard. The FE renders these; it never hardcodes. api.MapGet("/uploads/categories", (string wizardId, string? diplomaHerkomst, string? taalvaardigheid) => new UploadCategoriesDto(DocumentRules.CategoriesFor(wizardId, diplomaHerkomst, taalvaardigheid).Select(c => c.ToDto()).ToList())); -// Multipart upload. Hand-written on the FE (XHR for progress), so it is excluded -// from the OpenAPI doc to keep the NSwag-generated client JSON-only. Validates type -// and size authoritatively; stores metadata only (no file bytes / PII held). -api.MapPost("/uploads", async (HttpRequest request, HttpContext ctx, IDocumentSource documents) => -{ - if (!request.HasFormContentType) return Results.Problem(detail: "Verwacht multipart/form-data.", statusCode: 400); - var form = await request.ReadFormAsync(); - var file = form.Files.GetFile("file"); - string categoryId = form["categoryId"].ToString(), localId = form["localId"].ToString(), wizardId = form["wizardId"].ToString(); - if (file is null || categoryId == "" || localId == "" || wizardId == "") - return Results.Problem(detail: "Onvolledige upload.", statusCode: 400); - - var category = DocumentRules.Find(wizardId, categoryId); - var reject = DocumentRules.RejectUpload(category, file.ContentType, file.Length); - if (reject is not null) return Results.Problem(detail: reject, statusCode: 400); - - using var ms = new MemoryStream(); - await file.CopyToAsync(ms); - // WP-51: route through IDocumentSource — LocalDocumentSource is the same DocumentStore.Add - // call this used to make inline; OpenZaakDocumentSource (Zgw:Enabled=true) also registers - // the file as a DRC enkelvoudiginformatieobject. Response DTO unchanged either way. - var response = documents.Upload(localId, categoryId, wizardId, file.FileName, file.ContentType, ms.ToArray(), ctx.Zorgverlener()); - return Results.Created($"/api/v1/uploads/{response.DocumentId}", response); -}) -.ExcludeFromDescription(); - // Serve stored bytes so a re-opened wizard can preview/download an upload. Inline // for pdf/image (browser renders it), attachment otherwise (download). // Scoped like DELETE on the same resource (RB-01/BIO-004): the owning citizen, or a @@ -305,6 +281,34 @@ api.MapGet("/uploads/status", (string? localIds, HttpContext ctx) => return new UploadStatusDto(results); }); +// --- writes --- + +// Multipart upload. Hand-written on the FE (XHR for progress), so it is excluded +// from the OpenAPI doc to keep the NSwag-generated client JSON-only. Validates type +// and size authoritatively; stores metadata only (no file bytes / PII held). +api.MapPost("/uploads", async (HttpRequest request, HttpContext ctx, IDocumentSource documents) => +{ + if (!request.HasFormContentType) return Results.Problem(detail: "Verwacht multipart/form-data.", statusCode: 400); + var form = await request.ReadFormAsync(); + var file = form.Files.GetFile("file"); + string categoryId = form["categoryId"].ToString(), localId = form["localId"].ToString(), wizardId = form["wizardId"].ToString(); + if (file is null || categoryId == "" || localId == "" || wizardId == "") + return Results.Problem(detail: "Onvolledige upload.", statusCode: 400); + + var category = DocumentRules.Find(wizardId, categoryId); + var reject = DocumentRules.RejectUpload(category, file.ContentType, file.Length); + if (reject is not null) return Results.Problem(detail: reject, statusCode: 400); + + using var ms = new MemoryStream(); + await file.CopyToAsync(ms); + // WP-51: route through IDocumentSource — LocalDocumentSource is the same DocumentStore.Add + // call this used to make inline; OpenZaakDocumentSource (Zgw:Enabled=true) also registers + // the file as a DRC enkelvoudiginformatieobject. Response DTO unchanged either way. + var response = documents.Upload(localId, categoryId, wizardId, file.FileName, file.ContentType, ms.ToArray(), ctx.Zorgverlener()); + return Results.Created($"/api/v1/uploads/{response.DocumentId}", response); +}) +.ExcludeFromDescription(); + // User delete: owner-scoped; 409 once linked to a finalised submission. api.MapDelete("/uploads/{documentId}", (string documentId, HttpContext ctx) => DocumentStore.DeleteOwned(documentId, ctx.Zorgverlener().Bsn) switch @@ -332,6 +336,8 @@ api.MapDelete("/admin/uploads/{documentId}", (string documentId, HttpContext ctx // --- Applications (aanvragen): the system of record the dashboard reads. --- +// --- reads --- + // WP-53: routed through IZaakSource (like /admin/cases already was) rather than calling // ApplicationStore directly — under Zgw:Enabled=true a citizen's own dashboard list comes from // OpenZaak (BSN-filtered) too, closing the last "reads a static store directly" gap @@ -346,6 +352,8 @@ api.MapGet("/applications/{id}", (string id, HttpContext ctx) => .Produces() .Produces(StatusCodes.Status404NotFound); +// --- writes --- + api.MapPost("/applications", (CreateApplicationRequest req, HttpContext ctx) => { // Feature flag (WP-47): self-service registration can be closed by an admin. @@ -480,12 +488,40 @@ api.MapPost("/applications/{id}/submit", (string id, SubmitApplicationRequest re .Produces(StatusCodes.Status404NotFound); // --- Admin cases (WP-36): cross-owner list + admin delete, gated by `cases:manage`. --- + +// --- reads --- + api.MapGet("/admin/cases", (HttpContext ctx, IZaakSource zaken) => CasesAdmin(ctx, () => Results.Ok(zaken.ListCases(DateTimeOffset.UtcNow)))) .Gate("CasesAdmin") .Produces>() .ProducesProblem(StatusCodes.Status403Forbidden); +// Queryable authz/PII-reveal audit trail (WP-41) — data-minimised, no PII. Admin-gated +// via the existing CasesAdmin (cases:manage); a dedicated audit:read cap is a later refinement. +api.MapGet("/admin/audit", (HttpContext ctx) => CasesAdmin(ctx, () => + Results.Ok(AuthzAuditStore.List() + .Select(a => new AuthzAuditDto(a.At.ToString("o"), a.Action, a.Resource, a.Decision, a.Role, a.CorrelationId)) + .ToList()))) +.Gate("CasesAdmin") +.Produces>() +.ProducesProblem(StatusCodes.Status403Forbidden); + +// --- writes --- + +// Admin delete removes ANY case (any owner, submitted or not) — unlike the user-facing +// DELETE /applications/{id}. A missing id is a 404. +api.MapDelete("/admin/cases/{id}", (string id, HttpContext ctx) => CasesAdmin(ctx, () => +{ + if (!ApplicationStore.DeleteAny(id)) return Results.NotFound(); + app.Logger.LogInformation("admin case delete id={Id}", id); + return Results.NoContent(); +})) +.Gate("CasesAdmin") +.Produces(StatusCodes.Status204NoContent) +.Produces(StatusCodes.Status404NotFound) +.ProducesProblem(StatusCodes.Status403Forbidden); + // --- Werkvoorraad (WP-64): the behandelportal's queue of aanvragen needing treatment. --- // Cross-owner like /admin/cases, but gated by the medewerker capability (`CanBeoordelen`, // WP-62) rather than the admin role, and pre-filtered to the two "still open" status tags — @@ -618,29 +654,6 @@ api.MapPost("/zgw/notificaties", (HttpContext ctx, NotificatieDto body) => // /uploads and /brief/reveal-bignummer. .ExcludeFromDescription(); -// Admin delete removes ANY case (any owner, submitted or not) — unlike the user-facing -// DELETE /applications/{id}. A missing id is a 404. -api.MapDelete("/admin/cases/{id}", (string id, HttpContext ctx) => CasesAdmin(ctx, () => -{ - if (!ApplicationStore.DeleteAny(id)) return Results.NotFound(); - app.Logger.LogInformation("admin case delete id={Id}", id); - return Results.NoContent(); -})) -.Gate("CasesAdmin") -.Produces(StatusCodes.Status204NoContent) -.Produces(StatusCodes.Status404NotFound) -.ProducesProblem(StatusCodes.Status403Forbidden); - -// Queryable authz/PII-reveal audit trail (WP-41) — data-minimised, no PII. Admin-gated -// via the existing CasesAdmin (cases:manage); a dedicated audit:read cap is a later refinement. -api.MapGet("/admin/audit", (HttpContext ctx) => CasesAdmin(ctx, () => - Results.Ok(AuthzAuditStore.List() - .Select(a => new AuthzAuditDto(a.At.ToString("o"), a.Action, a.Resource, a.Decision, a.Role, a.CorrelationId)) - .ToList()))) -.Gate("CasesAdmin") -.Produces>() -.ProducesProblem(StatusCodes.Status403Forbidden); - // PRD-0002 §6: coarse, role-derived capabilities for nav/menu-level checks (NOT // tied to a specific brief's live status — see BriefDecisionsDto for that). // WP-64: `aanvraag:beoordelen` is caller-kind-derived (CanBeoordelen), not role-derived like @@ -673,6 +686,8 @@ api.MapPut("/admin/flags/{key}", (string key, SetFeatureFlagRequest req, HttpCon // dev-only stand-in via X-Role (mirrors the FE ?role= toggle) — no real // identities in this POC. --- +// --- reads --- + api.MapGet("/brief", (HttpContext ctx) => { // RB-23/CQ-007: a read that used to allocate a row on first call. The owner's first @@ -685,6 +700,27 @@ api.MapGet("/brief", (HttpContext ctx) => .Produces() .Produces(StatusCodes.Status404NotFound); +// Server-rendered HTML preview (WP-25): "what you compose is what is sent" — the +// same LetterHtml.Render a sent brief archived. Hand-written on the FE (fetch → +// blob → new tab), so excluded from the OpenAPI doc, same seam as uploads. Sent +// letters serve their frozen archive; anything else renders live with a watermark. +api.MapGet("/brief/preview", (HttpContext ctx) => +{ + // RB-23: BriefStore.GetOrCreate is gone (split into Get + ResetAndCreate). This GET + // must not create a brief as a side effect either, so it 404s under the same + // precondition as GET /brief — in the running app the FE only reaches this endpoint + // from the brief page, which has already loaded (and, if needed, reset) a brief. + var e = BriefStore.Get(ctx.Zorgverlener().Bsn); + if (e is null) return Results.NotFound(); + if (e.Status.Tag == "sent" && e.ArchivedHtml is { } archived) + return Results.Content(archived, "text/html"); + var template = OrgTemplateStore.TemplateForBrief(e.SubOrgId, null); + return Results.Content(LetterHtml.Render(e, template, Now(), watermark: true), "text/html"); +}) +.ExcludeFromDescription(); + +// --- writes --- + api.MapPut("/brief", (SaveBriefRequest req, HttpContext ctx) => { var isDrafter = Authz.ResolvePrincipal(ctx).Role == PrincipalRole.Drafter; @@ -765,37 +801,6 @@ api.MapPost("/brief/reveal-bignummer", (HttpContext ctx) => // OpenAPI doc, same seam as /brief/preview and uploads. .ExcludeFromDescription(); -// Server-rendered HTML preview (WP-25): "what you compose is what is sent" — the -// same LetterHtml.Render a sent brief archived. Hand-written on the FE (fetch → -// blob → new tab), so excluded from the OpenAPI doc, same seam as uploads. Sent -// letters serve their frozen archive; anything else renders live with a watermark. -api.MapGet("/brief/preview", (HttpContext ctx) => -{ - // RB-23: BriefStore.GetOrCreate is gone (split into Get + ResetAndCreate). This GET - // must not create a brief as a side effect either, so it 404s under the same - // precondition as GET /brief — in the running app the FE only reaches this endpoint - // from the brief page, which has already loaded (and, if needed, reset) a brief. - var e = BriefStore.Get(ctx.Zorgverlener().Bsn); - if (e is null) return Results.NotFound(); - if (e.Status.Tag == "sent" && e.ArchivedHtml is { } archived) - return Results.Content(archived, "text/html"); - var template = OrgTemplateStore.TemplateForBrief(e.SubOrgId, null); - return Results.Content(LetterHtml.Render(e, template, Now(), watermark: true), "text/html"); -}) -.ExcludeFromDescription(); - -// Proefbrief: the admin's unpublished draft template rendered over a fixture -// brief, so the appearance can be checked before publishing touches real letters. -api.MapGet("/admin/org-template/{subOrgId}/preview", (string subOrgId, HttpContext ctx) => OrgAdmin(ctx, () => -{ - var view = OrgTemplateStore.AdminView(subOrgId); - if (view is null) return Results.NotFound(); - var fixture = BriefSeed.NewBrief("proefbrief"); - return Results.Content(LetterHtml.Render(fixture, view.Draft, Now(), watermark: true), "text/html"); -})) -.Gate("OrgAdmin") -.ExcludeFromDescription(); - api.MapPost("/brief/reset", (HttpContext ctx) => { // Demo "start over": recreate a fresh draft. No guards — showcase affordance only. @@ -810,6 +815,8 @@ api.MapPost("/brief/reset", (HttpContext ctx) => // as drafter/approver); the same Authz check gates every endpoint and feeds the // `orgtemplate:edit` capability on /me, so emit and enforce cannot drift. --- +// --- reads --- + api.MapGet("/admin/org-templates", (HttpContext ctx) => OrgAdmin(ctx, () => Results.Ok(OrgTemplateStore.List()))) .Gate("OrgAdmin") @@ -825,6 +832,20 @@ api.MapGet("/admin/org-template/{subOrgId}", (string subOrgId, HttpContext ctx) .ProducesProblem(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); +// Proefbrief: the admin's unpublished draft template rendered over a fixture +// brief, so the appearance can be checked before publishing touches real letters. +api.MapGet("/admin/org-template/{subOrgId}/preview", (string subOrgId, HttpContext ctx) => OrgAdmin(ctx, () => +{ + var view = OrgTemplateStore.AdminView(subOrgId); + if (view is null) return Results.NotFound(); + var fixture = BriefSeed.NewBrief("proefbrief"); + return Results.Content(LetterHtml.Render(fixture, view.Draft, Now(), watermark: true), "text/html"); +})) +.Gate("OrgAdmin") +.ExcludeFromDescription(); + +// --- writes --- + api.MapPut("/admin/org-template/{subOrgId}", (string subOrgId, SaveOrgTemplateRequest req, HttpContext ctx) => OrgAdmin(ctx, () => { var reject = OrgTemplateRules.RejectDraft(req.Draft); diff --git a/backend/swagger.json b/backend/swagger.json index 9985fce..e0ccac2 100644 --- a/backend/swagger.json +++ b/backend/swagger.json @@ -686,6 +686,73 @@ } } }, + "/api/v1/admin/audit": { + "get": { + "tags": [ + "BigRegister.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/AuthzAuditDto" + } + } + } + } + }, + "403": { + "description": "Forbidden", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, + "/api/v1/admin/cases/{id}": { + "delete": { + "tags": [ + "BigRegister.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" + ], + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "description": "No Content" + }, + "404": { + "description": "Not Found" + }, + "403": { + "description": "Forbidden", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/v1/werkvoorraad": { "get": { "tags": [ @@ -832,73 +899,6 @@ } } }, - "/api/v1/admin/cases/{id}": { - "delete": { - "tags": [ - "BigRegister.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" - ], - "parameters": [ - { - "name": "id", - "in": "path", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "204": { - "description": "No Content" - }, - "404": { - "description": "Not Found" - }, - "403": { - "description": "Forbidden", - "content": { - "application/problem+json": { - "schema": { - "$ref": "#/components/schemas/ProblemDetails" - } - } - } - } - } - } - }, - "/api/v1/admin/audit": { - "get": { - "tags": [ - "BigRegister.Api, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" - ], - "responses": { - "200": { - "description": "OK", - "content": { - "application/json": { - "schema": { - "type": "array", - "items": { - "$ref": "#/components/schemas/AuthzAuditDto" - } - } - } - } - }, - "403": { - "description": "Forbidden", - "content": { - "application/problem+json": { - "schema": { - "$ref": "#/components/schemas/ProblemDetails" - } - } - } - } - } - } - }, "/api/v1/me": { "get": { "tags": [ 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 1825aa4..b41a430 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** | **done** | -| **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** | **done** | -| **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 | — | — | **done** | -| **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** | **done** | -| **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** | **done** | -| **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** | **done** | +| **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** | **implemented** | +| **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** | **done** | +| **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 | — | — | **done** | +| **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** | **done** | +| **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** | **done** | +| **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-19.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-19.md new file mode 100644 index 0000000..65afb53 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-19.md @@ -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 -- 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. diff --git a/libs/shared/src/infrastructure/api-client.ts b/libs/shared/src/infrastructure/api-client.ts index 8a30596..835788c 100644 --- a/libs/shared/src/infrastructure/api-client.ts +++ b/libs/shared/src/infrastructure/api-client.ts @@ -956,6 +956,94 @@ export class ApiClient { return Promise.resolve(null as any); } + /** + * @return OK + */ + audit(): Promise { + let url_ = this.baseUrl + "/api/v1/admin/audit"; + url_ = url_.replace(/[?&]$/, ""); + + let options_: RequestInit = { + method: "GET", + headers: { + "Accept": "application/json" + } + }; + + return this.http.fetch(url_, options_).then((_response: Response) => { + return this.processAudit(_response); + }); + } + + protected processAudit(response: Response): Promise { + const status = response.status; + let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); }; + if (status === 200) { + return response.text().then((_responseText) => { + let result200: any = null; + result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as AuthzAuditDto[]; + return result200; + }); + } else if (status === 403) { + return response.text().then((_responseText) => { + let result403: any = null; + result403 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; + return throwException("Forbidden", status, _responseText, _headers, result403); + }); + } else if (status !== 200 && status !== 204) { + return response.text().then((_responseText) => { + return throwException("An unexpected server error occurred.", status, _responseText, _headers); + }); + } + return Promise.resolve(null as any); + } + + /** + * @return No Content + */ + cases(id: string): Promise { + let url_ = this.baseUrl + "/api/v1/admin/cases/{id}"; + if (id === undefined || id === null) + throw new globalThis.Error("The parameter 'id' must be defined."); + url_ = url_.replace("{id}", encodeURIComponent("" + id)); + url_ = url_.replace(/[?&]$/, ""); + + let options_: RequestInit = { + method: "DELETE", + headers: { + } + }; + + return this.http.fetch(url_, options_).then((_response: Response) => { + return this.processCases(_response); + }); + } + + protected processCases(response: Response): Promise { + const status = response.status; + let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); }; + if (status === 204) { + return response.text().then((_responseText) => { + return; + }); + } else if (status === 403) { + return response.text().then((_responseText) => { + let result403: any = null; + result403 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; + return throwException("Forbidden", status, _responseText, _headers, result403); + }); + } else if (status === 404) { + return response.text().then((_responseText) => { + return throwException("Not Found", status, _responseText, _headers); + }); + } else if (status !== 200 && status !== 204) { + return response.text().then((_responseText) => { + return throwException("An unexpected server error occurred.", status, _responseText, _headers); + }); + } + return Promise.resolve(null as any); + } + /** * @return OK */ @@ -1112,94 +1200,6 @@ export class ApiClient { return Promise.resolve(null as any); } - /** - * @return No Content - */ - cases(id: string): Promise { - let url_ = this.baseUrl + "/api/v1/admin/cases/{id}"; - if (id === undefined || id === null) - throw new globalThis.Error("The parameter 'id' must be defined."); - url_ = url_.replace("{id}", encodeURIComponent("" + id)); - url_ = url_.replace(/[?&]$/, ""); - - let options_: RequestInit = { - method: "DELETE", - headers: { - } - }; - - return this.http.fetch(url_, options_).then((_response: Response) => { - return this.processCases(_response); - }); - } - - protected processCases(response: Response): Promise { - const status = response.status; - let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); }; - if (status === 204) { - return response.text().then((_responseText) => { - return; - }); - } else if (status === 403) { - return response.text().then((_responseText) => { - let result403: any = null; - result403 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; - return throwException("Forbidden", status, _responseText, _headers, result403); - }); - } else if (status === 404) { - return response.text().then((_responseText) => { - return throwException("Not Found", status, _responseText, _headers); - }); - } else if (status !== 200 && status !== 204) { - return response.text().then((_responseText) => { - return throwException("An unexpected server error occurred.", status, _responseText, _headers); - }); - } - return Promise.resolve(null as any); - } - - /** - * @return OK - */ - audit(): Promise { - let url_ = this.baseUrl + "/api/v1/admin/audit"; - url_ = url_.replace(/[?&]$/, ""); - - let options_: RequestInit = { - method: "GET", - headers: { - "Accept": "application/json" - } - }; - - return this.http.fetch(url_, options_).then((_response: Response) => { - return this.processAudit(_response); - }); - } - - protected processAudit(response: Response): Promise { - const status = response.status; - let _headers: any = {}; if (response.headers && response.headers.forEach) { response.headers.forEach((v: any, k: any) => _headers[k] = v); }; - if (status === 200) { - return response.text().then((_responseText) => { - let result200: any = null; - result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as AuthzAuditDto[]; - return result200; - }); - } else if (status === 403) { - return response.text().then((_responseText) => { - let result403: any = null; - result403 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; - return throwException("Forbidden", status, _responseText, _headers, result403); - }); - } else if (status !== 200 && status !== 204) { - return response.text().then((_responseText) => { - return throwException("An unexpected server error occurred.", status, _responseText, _headers); - }); - } - return Promise.resolve(null as any); - } - /** * @return OK */