diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index 208be87..c80d2fe 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -212,12 +212,24 @@ api.MapGet("/stamdata/{table}", (string table, string? peildatum, HttpContext ct { var t = StamdataCatalog.Find(table); if (t is null) return Results.NotFound(); - var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows(); + DateOnly? peildatumWaarde = null; + // RB-16/BIO-019: DateOnly.Parse threw FormatException on unparseable input, surfacing as + // an unhandled 500 (and, in Development, an exception detail leaked to the caller) — an + // admin-gated but still user-supplied string needs the same 400 path every other bad-input + // check in this file uses, not a crash. + if (peildatum is { Length: > 0 } p) + { + if (!DateOnly.TryParse(p, out var parsed)) + return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest); + peildatumWaarde = parsed; + } + var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows(); return Results.Ok(new StamdataTableDto(t.Id, t.Label, t.Columns.Select(ToColumnDto).ToList(), t.Temporal, rows)); })) .Gate("StamdataAdmin") .WithName("stamdataTable") .Produces() +.ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); diff --git a/backend/swagger.json b/backend/swagger.json index dbad12d..1a0dad1 100644 --- a/backend/swagger.json +++ b/backend/swagger.json @@ -194,6 +194,16 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "403": { "description": "Forbidden", "content": { diff --git a/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs b/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs index ac197ae..a676cc1 100644 --- a/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs +++ b/backend/tests/BigRegister.Tests/StamdataEndpointTests.cs @@ -59,6 +59,16 @@ public class StamdataEndpointTests(TestWebApplicationFactory factory) : IClassFi Assert.Empty(table.Rows); } + /// RB-16/BIO-019: DateOnly.Parse used to throw FormatException on unparseable input, + /// surfacing as an unhandled 500 instead of the 400-with-problem-details every other + /// bad-input check in this endpoint file returns. + [Fact] + public async Task Unparseable_peildatum_is_400_not_500() + { + var res = await _client.SendAsync(Req(HttpMethod.Get, "/api/v1/stamdata/professions?peildatum=not-a-date", role: "admin")); + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + } + [Fact] public async Task Unknown_table_is_404() { diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md new file mode 100644 index 0000000..557c4ff --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-16.md @@ -0,0 +1,82 @@ +# RB-16 — `DateOnly.TryParse` on `?peildatum=` + +Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-019 · `99-backlog.md` RB-16 + +## What was wrong + +`Program.cs:215` (pre-change) — +`var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows();`. +`DateOnly.Parse` throws `FormatException` on anything unparseable; there was no +`TryParse`, no 400 path, and `.Produces` on the endpoint declared only 200/403/404 — so +an unparseable `?peildatum=` value 500'd, and in Development the exception detail was +returned to the caller. §3c's baseline named `backend/Stamdata` 96.8% line but **71.7% +branch** (BL-005) — this was one of the unentered branches. + +## What changed + +| File | Change | +| ---------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Program.cs` — `GET /stamdata/{table}` | `DateOnly.Parse` replaced with `DateOnly.TryParse`; an unparseable value now returns `Results.Problem(detail: …, statusCode: 400)` instead of throwing; endpoint doc gained `.ProducesProblem(StatusCodes.Status400BadRequest)` | +| `tests/BigRegister.Tests/StamdataEndpointTests.cs` | **new** `Unparseable_peildatum_is_400_not_500` | +| `backend/swagger.json`, `libs/shared/src/infrastructure/api-client.ts` | regenerated (`npm run gen:api`) — the new 400 response is now part of the documented contract | + +## What the fix looks like + +```csharp +DateOnly? peildatumWaarde = null; +if (peildatum is { Length: > 0 } p) +{ + if (!DateOnly.TryParse(p, out var parsed)) + return Results.Problem(detail: $"Ongeldige peildatum '{p}'.", statusCode: StatusCodes.Status400BadRequest); + peildatumWaarde = parsed; +} +var rows = peildatumWaarde is { } d ? t.RowsOn(d) : t.Rows(); +``` + +Matches the shape every other bad-input check in this file already uses (e.g. the +upload endpoint's `Results.Problem(detail: …, statusCode: 400)` for a malformed +multipart request) — a `Results.Problem` with a Dutch detail message, not a bespoke +response shape. + +## Judgement calls + +- **No FE change needed, and none made.** `libs/beheer/src/infrastructure/ +stamdata.adapter.ts`'s `load()` already routes every call through `runSubmit` + (`libs/shared/src/application/submit.ts`), which try/catches any thrown + `ApiException` — including the client's new 400 branch — into a generic `Result` + error string via `problemDetail`. There is no status-code-specific branching to + extend; ADR-0001's "the FE renders the decision, it does not recompute the rule" + already covers "the server rejected this input" as a case the generic error path + handles, same as the existing 403. +- **Regenerated the API client and committed it in this ticket's diff**, rather than + leaving it to drift. RB-09's implementation note records a real prior incident where + a response-shape change (RB-08's 403 → `ProducesProblem`) landed without a + regeneration and the drift went unnoticed until the next ticket's `gen:api` run. This + ticket's `.ProducesProblem(400)` is exactly that same category of change, so + `npm run gen:api` was run immediately as part of implementing it, not deferred. +- **The Dutch detail message follows the file's own convention** (`$"Ongeldige +peildatum '{p}'."`) rather than English — every other `Results.Problem(detail: …)` + call in `Program.cs` (change-request rejection, upload validation, submit rejection) + is Dutch; this is server-internal wire text, not `$localize`-wrapped UI copy (the FE + never renders it verbatim — CLAUDE.md's `$localize` rule is about user-facing copy + the FE owns, not backend `ProblemDetails.detail` strings), so no locale entry was + needed. + +## Verification + +- **Reverted the fix only** (`DateOnly.Parse` restored, ternary un-nested, via Edit — + test left in place) and ran `StamdataEndpointTests`: + `Unparseable_peildatum_is_400_not_500` failed red (`Expected: BadRequest, Actual: +InternalServerError` — confirming the endpoint really did 500, not some other status). + Restored the fix (via Edit) and re-ran: all 6 tests in the class green. +- `dotnet build`: clean, 0 warnings. +- `dotnet format BigRegister.slnx --verify-no-changes`: clean. +- `dotnet test --filter "Category!=Integration"`: **260 passed, 0 failed** (259 + 1 + new). +- `npm run gen:api`: exit 0; `backend/swagger.json` gained the 400 response shape for + this one endpoint; `libs/shared/src/infrastructure/api-client.ts` gained the + matching `status === 400` branch. Both regenerated files committed alongside the + code change. +- `npm test` (all four Vitest projects — ssp/behandelportal/shared/beheer): **445 + passed, 0 failed**, confirming the regenerated client doesn't break any existing FE + consumer of `stamdataTable(...)`. diff --git a/libs/shared/src/infrastructure/api-client.ts b/libs/shared/src/infrastructure/api-client.ts index 3d4da57..04e7639 100644 --- a/libs/shared/src/infrastructure/api-client.ts +++ b/libs/shared/src/infrastructure/api-client.ts @@ -341,6 +341,12 @@ export class ApiClient { result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as StamdataTableDto; return result200; }); + } else if (status === 400) { + return response.text().then((_responseText) => { + let result400: any = null; + result400 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ProblemDetails; + return throwException("Bad Request", status, _responseText, _headers, result400); + }); } else if (status === 403) { return response.text().then((_responseText) => { let result403: any = null;