From 5d73ca21f6545e0520a11d5a4e0d7f701d40e785 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Tue, 18 Aug 2026 22:42:14 +0200 Subject: [PATCH] feat(backend): enforce the scholing threshold server-side (WP-69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0001's own canonical "config value" example was unenforced: GET /intake/policy echoed ScholingThreshold, but no request DTO carried a scholing answer, so the server had nothing to re-validate. A crafted POST could skip a requirement the wizard presents as mandatory. IntakePolicy.RejectIncompleteScholing is the authority — three-valued completeness (below threshold an answer is required; "nee" is legal and still submits; punten only belong to a followed scholing), living in the class that owns the constant so scripts/check-seam.sh keeps guarding the FE/BE literal pair. Both submit paths call it; a violation 400s with ProblemDetails and leaves the aanvraag a Concept. Gated on Type == "intake" (the endpoint's switch lumps herregistratie with intake, which has no scholing question), and guarded by `reject is null` so a zero-uren submission is still decided on its merits. Also fixes a live FE bug in the same rule: validateStep required punten whenever scholingGevolgd was 'ja' regardless of lageUren, while the template renders those fields only when lageUren — so answering 'ja' then raising uren either blocked the user on an invisible field or emitted aanvullendeScholing: undefined alongside punten. punten now derives from aanvullendeScholing, so that combination is unrepresentable in ValidIntake. Note: EndpointTests' Worked_hours_submission_succeeds was itself asserting the vulnerable payload ({ uren: 40 }, no answer) and needed a complete answer added; the zero-hours rows are the ordering regression net and are unmodified. Co-Authored-By: Claude Sonnet 5 --- .../domain/intake.acceptance.spec.ts | 29 +++ .../domain/intake.machine.spec.ts | 28 +++ .../herregistratie/domain/intake.machine.ts | 19 +- .../intake-wizard/intake-wizard.component.ts | 9 +- backend/README.md | 5 +- backend/src/BigRegister.Api/Contracts/Dtos.cs | 12 +- .../Domain/Intake/IntakePolicy.cs | 40 +++- backend/src/BigRegister.Api/Program.cs | 18 +- backend/swagger.json | 28 +++ .../Acceptance/IntakeSubmissionTests.cs | 136 +++++++++++++ .../Domain/IntakeRuleTests.cs | 43 ++++ .../tests/BigRegister.Tests/EndpointTests.cs | 5 +- ...9-intake-scholing-threshold-enforcement.md | 184 ++++++++++++++++-- .../0001-bff-lite-decision-dtos.md | 4 + libs/shared/docs/behaviour-spec.mdx | 26 ++- libs/shared/src/infrastructure/api-client.ts | 10 + 16 files changed, 560 insertions(+), 36 deletions(-) create mode 100644 backend/tests/BigRegister.Tests/Acceptance/IntakeSubmissionTests.cs create mode 100644 backend/tests/BigRegister.Tests/Domain/IntakeRuleTests.cs diff --git a/apps/ssp/src/app/herregistratie/domain/intake.acceptance.spec.ts b/apps/ssp/src/app/herregistratie/domain/intake.acceptance.spec.ts index 362ac13..cfb926b 100644 --- a/apps/ssp/src/app/herregistratie/domain/intake.acceptance.spec.ts +++ b/apps/ssp/src/app/herregistratie/domain/intake.acceptance.spec.ts @@ -147,6 +147,35 @@ describe('intake acceptance journeys', () => { }); }); + it('raising uren above the threshold after answering scholing drops both fields (WP-69 §6)', () => { + // Given a journey that answered the scholing question while uren was low. + const atReview = givenIntake( + { tag: 'SetAnswer', key: 'buitenlandGewerkt', value: 'nee' }, + { tag: 'Next' }, + { tag: 'SetAnswer', key: 'uren', value: '500' }, + { tag: 'SetAnswer', key: 'scholingGevolgd', value: 'ja' }, + { tag: 'SetAnswer', key: 'punten', value: '150' }, + { tag: 'Next' }, + ); + expect(atReview.tag).toBe('Answering'); + expect(atReview.tag === 'Answering' && atReview.cursor).toBe(2); // review + + // When the user goes back and raises uren above the threshold, then submits... + const done = given(reduce, atReview)( + { tag: 'GaNaarStap', cursor: 1 }, + { tag: 'SetAnswer', key: 'uren', value: '1500' }, + { tag: 'Next' }, // the now-hidden scholing question no longer blocks + { tag: 'Submit' }, + { tag: 'SubmitConfirmed' }, + ); + + // Then the submission succeeds, and BOTH the stale answer and its punten are gone — + // exactly the crafted-POST-shaped payload WP-69's server rule rejects. + expect(done.tag).toBe('Submitted'); + expect(done.tag === 'Submitted' && done.data.aanvullendeScholing).toBeUndefined(); + expect(done.tag === 'Submitted' && done.data.punten).toBeUndefined(); + }); + it('SetPolicy (server-owned threshold) can turn an already-answered uren into one that now requires scholing', () => { const atWerkStep = givenIntake( { tag: 'SetAnswer', key: 'buitenlandGewerkt', value: 'nee' }, diff --git a/apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts b/apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts index a248ce7..c5360f3 100644 --- a/apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts +++ b/apps/ssp/src/app/herregistratie/domain/intake.machine.spec.ts @@ -142,6 +142,34 @@ describe('submit', () => { expect(withScholing.data.punten).toBe(200); }); + it('does not require punten for a hidden question (WP-69 §6)', () => { + // scholingGevolgd is a stale 'ja' from when uren was low, but uren is now above + // threshold — the template hides the question, so punten must not be required either. + const good = expectTag( + submit(answering({ buitenlandGewerkt: 'nee', uren: '1500', scholingGevolgd: 'ja' })), + 'Submitting', + ); + expect(good.data.aanvullendeScholing).toBeUndefined(); + }); + + it('drops punten when raising uren hides the question (WP-69 §6)', () => { + // Same stale answer, but this time punten was also filled in while uren was low. + const good = expectTag( + submit( + answering({ + buitenlandGewerkt: 'nee', + uren: '1500', + scholingGevolgd: 'ja', + punten: '150', + }), + ), + 'Submitting', + ); + // ValidIntake stays honest: neither the stale 'ja' nor its punten leak through. + expect(good.data.aanvullendeScholing).toBeUndefined(); + expect(good.data.punten).toBeUndefined(); + }); + it('resolve maps Submitting to Submitted on a successful submit', () => { const submitting = submit(answering(complete)); expect(resolve(submitting, ok(undefined)).tag).toBe('Submitted'); diff --git a/apps/ssp/src/app/herregistratie/domain/intake.machine.ts b/apps/ssp/src/app/herregistratie/domain/intake.machine.ts index bb1320b..e970425 100644 --- a/apps/ssp/src/app/herregistratie/domain/intake.machine.ts +++ b/apps/ssp/src/app/herregistratie/domain/intake.machine.ts @@ -112,8 +112,12 @@ function validateStep(step: StepId, a: Answers, scholingThreshold: number): Resu if (!u.ok) errors.uren = u.error; if (lageUren(a, scholingThreshold) && !a.scholingGevolgd) errors.scholingGevolgd = $localize`:@@validation.maakKeuze:Maak een keuze.`; - // Nascholingspunten are only asked (and required) when scholing was followed. - if (a.scholingGevolgd === 'ja') { + // Nascholingspunten are only asked (and required) when the question is actually + // visible (lageUren) AND scholing was followed — matching the template's + // `@if (scholingZichtbaar())`. Without the `lageUren` guard, answering 'ja' and then + // raising uren above the threshold left an error on a field the template no longer + // renders (WP-69 §6). + if (lageUren(a, scholingThreshold) && a.scholingGevolgd === 'ja') { const p = parseUren(a.punten ?? ''); if (!p.ok) errors.punten = p.error; } @@ -142,14 +146,19 @@ function validateAll(a: Answers, scholingThreshold: number): Result? Documents = null); -public sealed record IntakeRequest(int Uren); + +// AanvullendeScholing/ScholingPunten (WP-69): the wizard's scholing answer, re-validated +// server-side as the authority by IntakePolicy.RejectIncompleteScholing. Named +// ScholingPunten (not Punten) — the sibling SubmitApplicationRequest is shared by all three +// wizard types and the herregistratie wizard has its own unrelated `punten`. +public sealed record IntakeRequest(int Uren, bool? AanvullendeScholing = null, int? ScholingPunten = null); public sealed record HerregistratieRequest(int Uren, IReadOnlyList? Documents = null); public sealed record ChangeRequestRequest(string Telefoon); @@ -120,9 +125,12 @@ public sealed record DraftSyncRequest( IReadOnlyList? DocumentIds = null); // Submit carries only the fields the server re-validates per wizard type. +// AanvullendeScholing/ScholingPunten (WP-69) — see IntakeRequest; intake-typed aanvragen +// only (gated by IntakePolicy.RejectIncompleteScholing's caller), null for the others. public sealed record SubmitApplicationRequest( string? DiplomaHerkomst = null, int? Uren = null, - IReadOnlyList? Documents = null); + IReadOnlyList? Documents = null, + bool? AanvullendeScholing = null, int? ScholingPunten = null); public sealed record SubmitApplicationResponse(string Referentie, AanvraagStatusDto Status); diff --git a/backend/src/BigRegister.Api/Domain/Intake/IntakePolicy.cs b/backend/src/BigRegister.Api/Domain/Intake/IntakePolicy.cs index c6aaa62..484c36e 100644 --- a/backend/src/BigRegister.Api/Domain/Intake/IntakePolicy.cs +++ b/backend/src/BigRegister.Api/Domain/Intake/IntakePolicy.cs @@ -4,17 +4,39 @@ namespace BigRegister.Domain.Intake; /// Config value (ADR-0001's "config value" shape). Below this many NL work-hours the /// scholing question is required. The frontend receives this value /// (GET /intake/policy) and applies it for instant UX feedback -/// (intake.machine.ts's lageUren). -/// -/// WP-68 (F5): the class doc used to claim "the backend re-validates on submit as the -/// authority" — it doesn't. Neither SubmitApplicationRequest nor IntakeRequest -/// carries a scholing answer at all, so there is nothing for the server to re-validate; -/// both submit paths only apply SubmissionRules.RejectZeroUren. A crafted POST can -/// bypass the scholing requirement entirely. Enforcing this needs a wire change (the -/// request DTOs must carry the wizard's scholing answer) and is deferred to WP-69 — this -/// comment states the gap rather than a false guarantee. +/// (intake.machine.ts's lageUren); is the +/// backend re-validating it as the authority on submit (WP-69) — both +/// POST /applications/{id}/submit (intake-typed aanvragen only) and the legacy +/// POST /intakes call it before writing anything, and a violation 400s +/// (ProblemDetails), never silently accepts an incomplete answer. /// public static class IntakePolicy { public const int ScholingThreshold = 1000; + + /// + /// Completeness rule for the scholing question (WP-69) — not merit: below + /// an answer must be present, but "nee" is a legal answer + /// that still submits (turning "few uren + no scholing" into a rejection is out of scope, + /// see the WP). Three-valued, so two parameters (uren, punten) couldn't express it: + /// + /// below threshold and no answer at all ⇒ incomplete; + /// answered true (scholing gevolgd) ⇒ punten required and non-negative + /// (mirrors parseUren); + /// answered anything but true ⇒ punten must be absent (a stale answer left + /// behind by raising uren is not a legal payload). + /// + /// Returns the Dutch detail message for a 400 ProblemDetails, or null when complete. + /// Boundary is <, not <= — mirrors lageUren. + /// + public static string? RejectIncompleteScholing(int uren, bool? aanvullendeScholing, int? scholingPunten) + { + if (uren < ScholingThreshold && aanvullendeScholing is null) + return $"Beantwoord de vraag over aanvullende scholing: bij minder dan {ScholingThreshold} gewerkte uren is dit verplicht."; + if (aanvullendeScholing == true && (scholingPunten is null || scholingPunten < 0)) + return "Vul het aantal behaalde nascholingspunten in."; + if (aanvullendeScholing != true && scholingPunten is not null) + return "Nascholingspunten horen alleen bij een gevolgde aanvullende scholing."; + return null; + } } diff --git a/backend/src/BigRegister.Api/Program.cs b/backend/src/BigRegister.Api/Program.cs index 67caa9b..5c23d50 100644 --- a/backend/src/BigRegister.Api/Program.cs +++ b/backend/src/BigRegister.Api/Program.cs @@ -195,8 +195,16 @@ api.MapPost("/herregistraties", (HerregistratieRequest req, HttpContext ctx) => .ProducesProblem(StatusCodes.Status422UnprocessableEntity); api.MapPost("/intakes", (IntakeRequest req, HttpContext ctx) => - Submit(ctx, "intake", SubmissionRules.RejectZeroUren(req.Uren))) +{ + // WP-69: completeness check outside Submit(...) — deliberately not folded into `reject`, + // so this 400 is never cached in IdempotencyStore the way a 422 rejection would be. + var reject = SubmissionRules.RejectZeroUren(req.Uren); + if (reject is null && IntakePolicy.RejectIncompleteScholing(req.Uren, req.AanvullendeScholing, req.ScholingPunten) is { } incomplete) + return Results.Problem(detail: incomplete, statusCode: StatusCodes.Status400BadRequest); + return Submit(ctx, "intake", reject); +}) .Produces() +.ProducesProblem(StatusCodes.Status400BadRequest) .ProducesProblem(StatusCodes.Status422UnprocessableEntity); api.MapPost("/change-requests", (ChangeRequestRequest req, HttpContext ctx) => @@ -361,6 +369,14 @@ api.MapPost("/applications/{id}/submit", (string id, SubmitApplicationRequest re _ /* herregistratie | intake */ => (SubmissionRules.RejectZeroUren(req.Uren ?? 0), true), }; + // WP-69: intake-only (herregistratie has no scholing question) — guarded by `reject is + // null` so a { uren: 0 } submission is still decided on merit (RejectZeroUren) and + // completeness is moot; placed before the document-ownership check and + // ApplicationStore.Submit so a rejected submit leaves the aanvraag a Concept (retryable). + if (existing.Type == "intake" && reject is null && + IntakePolicy.RejectIncompleteScholing(req.Uren ?? 0, req.AanvullendeScholing, req.ScholingPunten) is { } incompleteScholing) + return Results.Problem(detail: incompleteScholing, statusCode: StatusCodes.Status400BadRequest); + var docs = req.Documents; var documentIds = docs?.Where(d => d.Channel == "digital" && d.DocumentId is not null).Select(d => d.DocumentId!).ToList(); diff --git a/backend/swagger.json b/backend/swagger.json index 524a2c6..f16fd58 100644 --- a/backend/swagger.json +++ b/backend/swagger.json @@ -314,6 +314,16 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Content", "content": { @@ -2172,6 +2182,15 @@ "uren": { "type": "integer", "format": "int32" + }, + "aanvullendeScholing": { + "type": "boolean", + "nullable": true + }, + "scholingPunten": { + "type": "integer", + "format": "int32", + "nullable": true } }, "additionalProperties": false @@ -2837,6 +2856,15 @@ "$ref": "#/components/schemas/DocumentRefDto" }, "nullable": true + }, + "aanvullendeScholing": { + "type": "boolean", + "nullable": true + }, + "scholingPunten": { + "type": "integer", + "format": "int32", + "nullable": true } }, "additionalProperties": false diff --git a/backend/tests/BigRegister.Tests/Acceptance/IntakeSubmissionTests.cs b/backend/tests/BigRegister.Tests/Acceptance/IntakeSubmissionTests.cs new file mode 100644 index 0000000..c21eb64 --- /dev/null +++ b/backend/tests/BigRegister.Tests/Acceptance/IntakeSubmissionTests.cs @@ -0,0 +1,136 @@ +using System.Net; +using System.Net.Http.Json; +using BigRegister.Api.Contracts; +using BigRegister.Api.Data; +using BigRegister.Domain.Applications; +using BigRegister.Tests.Builders; + +namespace BigRegister.Tests.Acceptance; + +/// +/// Behaviour-level tests for the scholing-threshold enforcement (WP-69) over both live HTTP +/// paths — POST /applications/{id}/submit (the wizard's real path) and the legacy +/// POST /intakes (dead from the UI, still a live crafted-POST surface). Built through +/// the type-state builder, mirroring +/// rather than the full wizard/upload dance — the builder's default owner IS +/// 's default caller, +/// so no header juggling. +/// +public class IntakeSubmissionTests(TestWebApplicationFactory factory) : IClassFixture +{ + private readonly HttpClient _client = factory.CreateClient(); + + private static void Persist(Aanvraag aanvraag) + { + using var db = Db.Create(); + db.Applications.Add(aanvraag); + db.SaveChanges(); + } + + private Task Submit(string id, object body) => + _client.PostAsJsonAsync($"/api/v1/applications/{id}/submit", body); + + [Fact] + public async Task Below_threshold_without_an_answer_is_rejected_and_stays_a_concept() + { + // Given an intake-typed Concept aanvraag (not yet submitted). + var aanvraag = Given.Concept(type: "intake").Build(); + Persist(aanvraag); + + // When it is submitted with uren below the threshold and no scholing answer at all... + var res = await Submit(aanvraag.Id, new { uren = 500 }); + + // Then the request is rejected as a contract violation (400, not a merit rejection)... + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + + // ...and the aanvraag is left a retryable Concept, never marked Submitted. + var stillConcept = ApplicationStore.GetAny(aanvraag.Id)!; + Assert.False(stillConcept.Submitted); + } + + [Fact] + public async Task Below_threshold_with_an_answer_succeeds() + { + // Given an intake-typed Concept. + var aanvraag = Given.Concept(type: "intake").Build(); + Persist(aanvraag); + + // When submitted below the threshold with "niet gevolgd" — a complete, legal answer... + var res = await Submit(aanvraag.Id, new { uren = 500, aanvullendeScholing = false }); + + // Then the submission succeeds. + res.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task Above_threshold_needs_no_answer() + { + // Given an intake-typed Concept. + var aanvraag = Given.Concept(type: "intake").Build(); + Persist(aanvraag); + + // When submitted with uren at/above the threshold and no scholing answer... + var res = await Submit(aanvraag.Id, new { uren = 1000 }); + + // Then it succeeds — the question is moot above the threshold. + res.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task Punten_without_gevolgd_is_rejected() + { + // Given an intake-typed Concept. + var aanvraag = Given.Concept(type: "intake").Build(); + Persist(aanvraag); + + // When submitted above the threshold with punten but no "gevolgd" answer — the stale + // shape §6 fixes on the frontend, still reachable as a crafted POST... + var res = await Submit(aanvraag.Id, new { uren = 1500, scholingPunten = 150 }); + + // Then it is rejected. + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + } + + [Fact] + public async Task Herregistratie_is_unaffected_by_the_intake_only_gate() + { + // Given a herregistratie-typed Concept (no scholing question in that wizard). + var aanvraag = Given.Concept(type: "herregistratie").Build(); + Persist(aanvraag); + + // When submitted below the intake threshold with no scholing answer at all... + var res = await Submit(aanvraag.Id, new { uren = 500 }); + + // Then it still succeeds — the gate is intake-only. + res.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task Zero_uren_is_still_afgewezen_not_a_400() + { + // Given an intake-typed Concept. + var aanvraag = Given.Concept(type: "intake").Build(); + Persist(aanvraag); + + // When submitted with zero uren and no scholing answer — completeness would also + // reject this, but the merit rejection (RejectZeroUren) must win (the ordering guard)... + var res = await Submit(aanvraag.Id, new { uren = 0 }); + + // Then the submission is accepted and resolves to Afgewezen — not a 400. + res.EnsureSuccessStatusCode(); + var body = (await res.Content.ReadFromJsonAsync())!; + Assert.Equal("Afgewezen", body.Status.Tag); + } + + [Fact] + public async Task Legacy_intakes_endpoint_enforces_it_too() + { + // Given no aanvraag needed — the legacy endpoint mints its own reference. + // When a crafted POST hits the dead-from-the-UI /intakes endpoint below threshold, + // with no scholing answer... + var res = await _client.PostAsJsonAsync("/api/v1/intakes", new { uren = 500 }); + + // Then it is rejected too — the crafted-POST surface this WP closes. + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + } +} diff --git a/backend/tests/BigRegister.Tests/Domain/IntakeRuleTests.cs b/backend/tests/BigRegister.Tests/Domain/IntakeRuleTests.cs new file mode 100644 index 0000000..2b49b27 --- /dev/null +++ b/backend/tests/BigRegister.Tests/Domain/IntakeRuleTests.cs @@ -0,0 +1,43 @@ +using BigRegister.Domain.Intake; + +namespace BigRegister.Tests.Domain; + +public class IntakeRuleTests +{ + // The arguments ARE the Given (WP-69/bdd.mdx) — these degenerate to When/Then. + + [Fact] + public void Below_threshold_with_no_answer_is_incomplete() => + Assert.NotNull(IntakePolicy.RejectIncompleteScholing(999, aanvullendeScholing: null, scholingPunten: null)); + + [Fact] + public void At_the_threshold_no_answer_is_required() => + // Pins `<` vs `<=` — lageUren's own boundary. + Assert.Null(IntakePolicy.RejectIncompleteScholing(1000, aanvullendeScholing: null, scholingPunten: null)); + + [Fact] + public void Niet_gevolgd_is_a_complete_answer_below_threshold() => + // "nee" is legal — this WP is completeness, not merit (§1's scope). + Assert.Null(IntakePolicy.RejectIncompleteScholing(500, aanvullendeScholing: false, scholingPunten: null)); + + [Fact] + public void Gevolgd_without_punten_is_incomplete() => + Assert.NotNull(IntakePolicy.RejectIncompleteScholing(500, aanvullendeScholing: true, scholingPunten: null)); + + [Fact] + public void Gevolgd_with_zero_punten_is_valid() => + Assert.Null(IntakePolicy.RejectIncompleteScholing(500, aanvullendeScholing: true, scholingPunten: 0)); + + [Fact] + public void Gevolgd_with_negative_punten_is_refused() => + Assert.NotNull(IntakePolicy.RejectIncompleteScholing(500, aanvullendeScholing: true, scholingPunten: -1)); + + [Theory] + [InlineData(null)] // stale-punten shape (§6): raising uren above threshold left an unanswered + // question but punten still set from when it was visible + [InlineData(false)] + public void Punten_without_gevolgd_is_refused(bool? aanvullendeScholing) => + // uren ABOVE threshold so the "missing answer" branch can't also explain the rejection — + // this row isolates the "stale punten" rule on its own. + Assert.NotNull(IntakePolicy.RejectIncompleteScholing(1500, aanvullendeScholing, scholingPunten: 150)); +} diff --git a/backend/tests/BigRegister.Tests/EndpointTests.cs b/backend/tests/BigRegister.Tests/EndpointTests.cs index d3a4774..f851844 100644 --- a/backend/tests/BigRegister.Tests/EndpointTests.cs +++ b/backend/tests/BigRegister.Tests/EndpointTests.cs @@ -103,7 +103,10 @@ public class EndpointTests(TestWebApplicationFactory factory) : IClassFixture (RejectZeroUren(...), true)`. + `herregistratie.machine.ts` has no scholing question, so the new check **must** be gated on + `existing.Type == "intake"` or the herregistratie wizard starts 400-ing for every low-uren user. +- **`RejectMissingScholing(uren, scholing)` is under-specified.** The rule is three-valued + (answer present / `true` needs punten / punten without `true` is illegal); two parameters + cannot express it. +- **A live FE bug shares this rule and must be fixed here.** `intake.machine.ts:116` requires + `punten` whenever `scholingGevolgd === 'ja'` **regardless of `lageUren`**, while the template + renders both fields only inside `@if (scholingZichtbaar())` (= `lageUren`). Answer scholing + `'ja'`, then raise `uren` above the threshold: either the user is blocked by an error on an + **invisible** field, or `validateAll` emits `aanvullendeScholing: undefined` **together with** + `punten: 150` — exactly the payload the new server rule rejects. Both branches reachable today. + ## Decisions -Not yet made — this is a placeholder WP opened by WP-68, not a ready-to-implement one. Needs -a `planner` pass before work starts. Open questions to resolve then: +Made by a `planner` pass on 2026-08-18 — do not relitigate. -- The request DTOs need a scholing answer field (likely mirroring `intake.machine.ts`'s - `ValidIntake.aanvullendeScholing`/`punten`) — this is a wire change, so it touches - `contracts/`, the wizard's submit payload, and `npm run gen:api`. -- Whether to add the rule to `SubmissionRules` (alongside `RejectZeroUren`) or give - `IntakePolicy` its own `RejectMissingScholing(uren, scholing)`, matching the class that - already owns the threshold. -- Reading the answer out of the wizard's `Draft` JSON was rejected in WP-68 — the backend's - documented posture is that the draft is opaque (`AppDbContext`'s header comment) — so the - answer must arrive as an explicit request field, not be extracted from the opaque snapshot. +### 1. What the rule is (and deliberately is not) -## Out of scope (for now) +The FE rule is **completeness**, not merit: below the threshold the scholing question must be +**answered**; `'nee'` is a legal answer that still submits. So the server authority is: -Implementation — this WP exists to track the gap; do not implement without a Decisions block. +- `uren < IntakePolicy.ScholingThreshold` ⇒ an answer must be present; +- answer `true` ⇒ punten present and `>= 0` (mirrors `parseUren`); +- answer not `true` ⇒ punten must be **absent**. + +**Out of scope, deliberately:** turning "few uren + no scholing" into an `Afgewezen` decision. +The wizard accepts that today; inventing a substantive rejection would create a _new_ FE/BE +divergence in the WP that closes one. **Boundary is `<`, not `<=`** — mirrors `lageUren`. + +### 2. Wire shape + +Two nullable fields appended (positionally last, defaulted) to both request records in +`Contracts/Dtos.cs`: `bool? AanvullendeScholing = null, int? ScholingPunten = null`. + +- **`ScholingPunten`, not `Punten`** — `SubmitApplicationRequest` is shared by all three wizard + types and the herregistratie wizard has its own unrelated `punten`. +- **Illegal states are representable on the wire, unrepresentable past the boundary.** A JSON + DTO consumed by NSwag can't carry a union without hand-written polymorphism, and both fields + must be optional for the other wizards anyway. Closure happens at the rule boundary — the same + posture WP-68 took for `AanvraagStatus`. _Rejected:_ a nested `ScholingDto` (removes one of + three illegal combinations, adds a DTO); a closed `ScholingAnswer` type (one call site, not + persisted — ceremony). +- **Not persisted.** Submit-time rule input, not aggregate state: no `Aanvraag` column, **no EF + migration**. The draft JSON stays opaque (WP-68) — the answer arrives as an explicit field. + +### 3. Rule home — `IntakePolicy`, not `SubmissionRules` + +`public static string? RejectIncompleteScholing(int uren, bool? aanvullendeScholing, int? scholingPunten)` +— same "reason or null" idiom as `SubmissionRules`, so endpoints compose both identically. + +The rule _is_ the threshold's enforcement and the class already owns the constant. Putting it in +`SubmissionRules` would either re-declare `1000` there (silent drift — exactly what WP-71's +`check:seam` exists to catch, and which it would **not** catch outside `IntakePolicy.cs`) or make +the generic cross-wizard class depend on one wizard's policy. `SubmissionRules.cs` and +`SubmissionRuleTests.cs` are **not modified**. + +**`check:seam` constraint (load-bearing):** `scripts/check-seam.sh` greps _all_ +`ScholingThreshold\s*=\s*[0-9]+` matches in `IntakePolicy.cs`. The new code must **reference** +the const (`uren < ScholingThreshold`, `$"…{ScholingThreshold}…"`) and must never introduce a +second literal (e.g. a default parameter `int scholingThreshold = 1000`) — a second match makes +`backend_value` two lines and fails with a misleading "drift" message. + +### 4. HTTP shape: 400 ProblemDetails, matching WP-68 F1 + +A missing/contradictory conditionally-required field is a **contract violation**, not a business +outcome → `Results.Problem(detail: …, statusCode: 400)`. Deliberately unlike `RejectZeroUren`, +which is a _merit_ rejection (422 legacy / `Afgewezen` + 200 on the aanvraag path). + +**Ordering: the zero-uren rejection wins.** Guard with `reject is null &&` so `{ uren: 0 }` is +decided on merit and completeness is moot — this keeps `EndpointTests`' 422 rows passing +**unmodified**. Place the check **before** the document-ownership check and +`ApplicationStore.Submit`, so a rejected submit leaves the aanvraag a Concept (retryable). + +Gated on `existing.Type == "intake"`. `/applications/{id}/submit` already declares +`.ProducesProblem(400)` (WP-68 F1) — no metadata change; `/intakes` needs one added, with the +check _outside_ the `Submit(...)` helper so the 400 is not cached in `IdempotencyStore`. + +Detail copy (Dutch, like all backend ProblemDetails — backend copy is not `$localize`d): +missing answer → `$"Beantwoord de vraag over aanvullende scholing: bij minder dan {ScholingThreshold} gewerkte uren is dit verplicht."`; +`true` without punten → `"Vul het aantal behaalde nascholingspunten in."`; +punten without `true` → `"Nascholingspunten horen alleen bij een gevolgde aanvullende scholing."` + +### 5. Backwards compatibility + +Fields optional on the wire, conditionally required by the rule (the same DTO serves registratie +and herregistratie, which never send them). **In-flight Concept drafts (WP-22) are unaffected** — +the draft JSON already holds `scholingGevolgd`/`punten`, its format doesn't change, and the new FE +derives the request fields at submit time. The one real incompatibility is a **stale FE bundle** +submitting a below-threshold intake: it gets a 400 with an actionable Dutch detail via +`problemDetail()`. Accepted — the POC has no API versioning, and the alternatives (grace period, +inferring from the draft) are what WP-68 forbade. _Rejected:_ a feature flag whose only purpose +is to leave a security gap open. + +### 6. Frontend changes + +- Wizard payload gains `aanvullendeScholing` + `scholingPunten` (`undefined` members are dropped + by `JSON.stringify` and bind to `null` server-side). +- **`intake.machine.ts` needs two narrowing edits** (see Stale premises — this is a live bug): + `validateStep('werk')` requires punten only when `lageUren(…) && scholingGevolgd === 'ja'` + (matching the template's `@if`), and `validateAll` computes punten from + `aanvullendeScholing === true` rather than `scholingGevolgd === 'ja'`, so a stale answer left by + raising `uren` can't leak into `ValidIntake`. `Answers` (the raw record) is unchanged — stale + raw answers are fine; `ValidIntake`, the _parsed_ type, must be honest. +- **No change** to `SCHOLING_THRESHOLD_DEFAULT`, `lageUren`, `SetPolicy`, the policy + adapter/store, or the template. **No new `$localize` id ⇒ no `messages.en.xlf` change.** + +### 7. Test plan (WP-71 conventions) + +G/W/T bodies, `Domain/RuleTests.cs`, `Acceptance/`, fixtures via the `Given` builder — +never hand-built initializers. + +**New `Domain/IntakeRuleTests.cs`** (pure rule, no HTTP; the arguments _are_ the Given, so these +degenerate to When/Then per `bdd.mdx`): answer required below threshold; not required _at_ the +threshold (pins `<` vs `<=`); `niet gevolgd` is a complete answer (pins §1's scope); `gevolgd` +requires punten; zero punten valid; negative refused; `[Theory]` — punten without `gevolgd` +refused (two rows, incl. the stale-punten shape §6 removes). + +**New `Acceptance/IntakeSubmissionTests.cs`** (HTTP, both paths, `Given.Concept(type: "intake")` + +- a local `Persist` mirroring `BesluitLifecycleTests`; the builder's default owner **is** + `StubIdentityProvider`'s default caller, so no header juggling): below threshold without an + answer → 400 **and still a Concept**; answered → 200; above threshold → 200; punten without + gevolgd → 400; **herregistratie unaffected** (guards the `Type` gate); `{ uren: 0 }` still + `Afgewezen` + 200, not 400 (pins the ordering); legacy `/intakes` enforces it too. + +**Not modified:** `EndpointTests.cs` (its 422 rows are the ordering regression net), +`SubmissionRuleTests.cs`, `ApplicationTests.cs`, `Builders/AanvraagBuilder.cs`. + +**Frontend:** `intake.machine.spec.ts` — drops punten when raising uren hides the question; does +not require punten for a hidden question. `intake.acceptance.spec.ts` — one journey: low uren → +`'ja'` + punten → back → raise uren → submit → both fields `undefined`. + +### 8. Sequencing + +1. `IntakePolicy.RejectIncompleteScholing` + `Domain/IntakeRuleTests.cs` (red→green, no wire change). +2. `Contracts/Dtos.cs` + both endpoints + `/intakes`' `.ProducesProblem(400)`. +3. `Acceptance/IntakeSubmissionTests.cs`; `dotnet test`. +4. **`npm run gen:api`** — after step 2, before the FE payload change. Commit `backend/swagger.json` + - `libs/shared/src/infrastructure/api-client.ts`. CI's drift job fails if skipped/hand-edited. +5. FE: `intake.machine.ts` narrowing + specs, then the wizard payload. +6. `npm run gen:snippets` (expect no diff) and **`npm run gen:behaviour-spec`** (will diff — new + test names; commit it or CI's drift step fails). +7. Docs in the same diff: rewrite `IntakePolicy`'s doc-comment from "gap deferred to WP-69" to what + it now guarantees; one line in ADR-0001 §"config value"/worked example B; `backend/README.md`'s + `/api/intakes` row (add the 400) + its `IntakePolicy.cs` bullet. +8. `npm run ci`. + +## Out of scope + +- Turning "few uren + no scholing" into an `Afgewezen` **decision** (§1) — a decision flag, an FE + change, and a separate WP. +- Deleting the dead `/intakes` + `/herregistraties` endpoints (with `EndpointTests`, + `backend/README.md`, `gen:api`) — real cleanup, but not this WP's security fix. +- The herregistratie wizard's `jaren`/`punten`, equally un-re-validated server-side. +- `docs/reference/fp-tea-atomic-design.md:587` / `ARCHITECTURE.md:464` still teach a + `visibleSteps`-with-a-`'scholing'`-step intake the fixed-3-step wizard no longer matches. + +## Risks + +- **Ordering regression (highest).** Running completeness before `RejectZeroUren` silently turns + `{ uren: 0 }` from 422/`Afgewezen` into 400 and breaks two existing endpoint tests. The + `reject is null &&` guard is load-bearing — keep the comment saying why. +- **`check:seam` false failure** if a second `ScholingThreshold = ` literal lands in + `IntakePolicy.cs` (§3). The message will say "FE/BE seam drift" and mislead. +- **Missing the `Type == "intake"` gate** breaks the herregistratie wizard for every low-uren + user; the `herregistratie is unaffected` test is the only net. +- **Stale-bundle 400 loop:** the wizard's `Retry` re-sends the identical payload, so a pre-deploy + tab loops until reloaded. Acceptable for a POC. diff --git a/docs/reference/architecture/0001-bff-lite-decision-dtos.md b/docs/reference/architecture/0001-bff-lite-decision-dtos.md index 65a18e8..649bf78 100644 --- a/docs/reference/architecture/0001-bff-lite-decision-dtos.md +++ b/docs/reference/architecture/0001-bff-lite-decision-dtos.md @@ -101,6 +101,10 @@ would compute. Two slices were implemented to demonstrate **both** policy shapes machine state and is set via a `SetPolicy` message. A `SCHOLING_THRESHOLD_DEFAULT` remains only as the offline fallback. - `intake-wizard.component.ts` fetches the policy and dispatches `SetPolicy`. +- WP-69: the backend re-validates the threshold as the authority on submit — + `IntakePolicy.RejectIncompleteScholing` runs before `POST /applications/{id}/submit` + (intake-typed) and the legacy `POST /intakes` write anything, 400ing an incomplete + scholing answer instead of silently accepting a crafted POST that skips it. ## Migration sequence (for the real app) diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 52edd58..febc184 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -20,8 +20,8 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page -**is** the suite, reshaped for a business reader. 401 frontend behaviours across -8 contexts; 206 backend behaviours across 33 test +**is** the suite, reshaped for a business reader. 404 frontend behaviours across +8 contexts; 219 backend behaviours across 35 test classes. ## Frontend (by context) @@ -312,6 +312,7 @@ classes. - low uren requires the scholing question, and punten only once scholing is followed - buitenland gewerkt requires land and hours abroad before advancing - gaNaarStap corrects an earlier answer without losing later ones +- raising uren above the threshold after answering scholing drops both fields (WP-69 §6) - SetPolicy (server-owned threshold) can turn an already-answered uren into one that now requires scholing #### intake hasProgress @@ -349,6 +350,8 @@ classes. - reaches Submitting ONLY with valid answers - punten is required only when aanvullende scholing was gevolgd - low hours requires the scholing answer before submit +- does not require punten for a hidden question (WP-69 §6) +- drops punten when raising uren hides the question (WP-69 §6) - resolve maps Submitting to Submitted on a successful submit - resolve maps Submitting to Failed on a failed submit @@ -957,6 +960,25 @@ classes. - Different idempotency keys are independent submissions - A rejected submission replays the same rejection not a retry +### IntakeRuleTests + +- Below threshold with no answer is incomplete +- At the threshold no answer is required +- Niet gevolgd is a complete answer below threshold +- Gevolgd without punten is incomplete +- Gevolgd with zero punten is valid +- Gevolgd with negative punten is refused + +### IntakeSubmissionTests + +- Below threshold without an answer is rejected and stays a concept +- Below threshold with an answer succeeds +- Above threshold needs no answer +- Punten without gevolgd is rejected +- Herregistratie is unaffected by the intake only gate +- Zero uren is still afgewezen not a 400 +- Legacy intakes endpoint enforces it too + ### LetterHtmlTests - Render matches the golden file diff --git a/libs/shared/src/infrastructure/api-client.ts b/libs/shared/src/infrastructure/api-client.ts index e61f502..44e6937 100644 --- a/libs/shared/src/infrastructure/api-client.ts +++ b/libs/shared/src/infrastructure/api-client.ts @@ -483,6 +483,12 @@ export class ApiClient { result200 = _responseText === "" ? null : JSON.parse(_responseText, this.jsonParseReviver) as ReferentieResponse; 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 === 422) { return response.text().then((_responseText) => { let result422: any = null; @@ -2210,6 +2216,8 @@ export interface IntakePolicyDto { export interface IntakeRequest { uren?: number; + aanvullendeScholing?: boolean | undefined; + scholingPunten?: number | undefined; } export interface LetterBlockDto { @@ -2419,6 +2427,8 @@ export interface SubmitApplicationRequest { diplomaHerkomst?: string | undefined; uren?: number | undefined; documents?: DocumentRefDto[] | undefined; + aanvullendeScholing?: boolean | undefined; + scholingPunten?: number | undefined; } export interface SubmitApplicationResponse {