From 6fa27d1c53f42fb2b9885bde5970351d85a1c96d Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Wed, 19 Aug 2026 16:32:22 +0200 Subject: [PATCH] test: close the remaining FE/BE seams (WP-75) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three seams WP-71 documented but left unguarded. Deletes the FE's isHerregistratieEligible and isStatusConsistent — both uncalled, the first dead by its own doc-comment. Their tests used fixtures completely disjoint from the backend's (the backend even had an exact-window boundary case the FE lacked), so the two sides could diverge indefinitely without failing anything. CLAUDE.md's policy of keeping server-owned rules as FE "reference impls" is what kept them alive, so it is amended: the FE may mirror a server-supplied value for instant feedback, never reimplement the algorithm. registration.policy.ts keeps its three live exports. check-seam.sh now also guards the Besluit tag list — the C# enum and the TS BESLUIT_TAGS array are identical ordered name lists with nothing linking them, and Enum.TryParse fails at request time rather than build time. Anchored on the full declaration so it avoids the "greps all matches" trap WP-69 hit. The phone-format divergence turned out to be real, not latent as recorded: the backend returned 422 for +31612345678 and (06) 12345678, both of which the FE's own parseTelefoonnummer accepts. A grep check would have compared the shared ^0\d{9}$ regex and reported all clear — the difference was in stripping. RejectPhoneChange now strips what the FE strips, pinned by a contract test. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 5 +- .../domain/registration.policy.spec.ts | 51 +++++------------- .../registratie/domain/registration.policy.ts | 26 +-------- .../Domain/Submissions/SubmissionRules.cs | 11 +++- .../Acceptance/PhoneFormatContractTests.cs | 54 +++++++++++++++++++ .../0001-bff-lite-decision-dtos.md | 10 ++-- scripts/check-seam.sh | 32 +++++++++++ 7 files changed, 118 insertions(+), 71 deletions(-) create mode 100644 backend/tests/BigRegister.Tests/Acceptance/PhoneFormatContractTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 294a03b..88fa614 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -178,8 +178,9 @@ FE keeps only **format** validation, never as authority. DTO lives in `contracts/`; a hand-written `parse*`/`toDomain` in `infrastructure/` validates the untrusted shape and maps DTO → domain. Wiring a real .NET backend touches only `infrastructure/` + `contracts/` (see ARCHITECTURE §6). Server-owned -rules stay in `domain/*.policy.ts` as reference impl + unit test, marked server-owned, -but the FE doesn't call them. +rules live **only** on the server, with no FE mirror to drift from it — the FE may +mirror a server-supplied _value_ (a threshold, a bound) for instant feedback, but +never reimplements the _algorithm_. **Business-tunable reference data ("stamdata") is config-as-code, not a DB.** Tables the business controls (profession↔diploma map, thresholds, policy-question text) live as typed diff --git a/apps/ssp/src/app/registratie/domain/registration.policy.spec.ts b/apps/ssp/src/app/registratie/domain/registration.policy.spec.ts index 077d388..3094b95 100644 --- a/apps/ssp/src/app/registratie/domain/registration.policy.spec.ts +++ b/apps/ssp/src/app/registratie/domain/registration.policy.spec.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { Registration, RegistrationStatus } from './registration'; -import { isHerregistratieEligible, isStatusConsistent, statusColor } from './registration.policy'; +import { Registration } from './registration'; +import { herregistratieDeadline, statusColor, statusLabel } from './registration.policy'; const reg = (status: Registration['status']): Registration => ({ bigNummer: '19012345601', @@ -12,25 +12,10 @@ const reg = (status: Registration['status']): Registration => ({ }); describe('registration.policy', () => { - it('only an active registration within the window is eligible', () => { - const active = reg({ tag: 'Geregistreerd', herregistratieDatum: '2027-01-01' }); - expect(isHerregistratieEligible(active, new Date('2026-06-01'))).toBe(true); // within 12 months - expect(isHerregistratieEligible(active, new Date('2020-01-01'))).toBe(false); // too early - }); - - it('struck-off / suspended registrations are never eligible', () => { - expect( - isHerregistratieEligible( - reg({ tag: 'Doorgehaald', doorgehaaldOp: '2024-05-01', reden: 'x' }), - new Date('2027-01-01'), - ), - ).toBe(false); - expect( - isHerregistratieEligible( - reg({ tag: 'Geschorst', geschorstTot: '2026-12-31', reden: 'x' }), - new Date('2027-01-01'), - ), - ).toBe(false); + it('statusLabel echoes the tag', () => { + expect(statusLabel('Geregistreerd')).toBe('Geregistreerd'); + expect(statusLabel('Doorgehaald')).toBe('Doorgehaald'); + expect(statusLabel('Geschorst')).toBe('Geschorst'); }); it('statusColor is total over the union', () => { @@ -39,25 +24,15 @@ describe('registration.policy', () => { expect(statusColor('Geschorst')).toContain('oranje'); }); - it('a well-formed status is always consistent', () => { + it('herregistratieDeadline is only set for an active registration', () => { expect( - isStatusConsistent(reg({ tag: 'Geregistreerd', herregistratieDatum: '2027-01-01' }).status), - ).toBe(true); + herregistratieDeadline(reg({ tag: 'Geregistreerd', herregistratieDatum: '2027-01-01' })), + ).toEqual(new Date('2027-01-01')); expect( - isStatusConsistent( - reg({ tag: 'Doorgehaald', doorgehaaldOp: '2024-05-01', reden: 'x' }).status, - ), - ).toBe(true); + herregistratieDeadline(reg({ tag: 'Doorgehaald', doorgehaaldOp: '2024-05-01', reden: 'x' })), + ).toBeNull(); expect( - isStatusConsistent(reg({ tag: 'Geschorst', geschorstTot: '2026-12-31', reden: 'x' }).status), - ).toBe(true); - }); - - it('a Geregistreerd status without its herregistratieDatum is inconsistent', () => { - // The union itself makes this unrepresentable through normal construction (every - // Geregistreerd literal must carry a herregistratieDatum) — only reachable here by - // bypassing the type system, the way malformed runtime/serialized data could. - const malformed = { tag: 'Geregistreerd' } as unknown as RegistrationStatus; - expect(isStatusConsistent(malformed)).toBe(false); + herregistratieDeadline(reg({ tag: 'Geschorst', geschorstTot: '2026-12-31', reden: 'x' })), + ).toBeNull(); }); }); diff --git a/apps/ssp/src/app/registratie/domain/registration.policy.ts b/apps/ssp/src/app/registratie/domain/registration.policy.ts index 5388e9a..3cb87c8 100644 --- a/apps/ssp/src/app/registratie/domain/registration.policy.ts +++ b/apps/ssp/src/app/registratie/domain/registration.policy.ts @@ -1,5 +1,5 @@ import { assertNever } from '@shared/kernel/fp'; -import { Registration, RegistrationStatus, StatusTag } from './registration'; +import { Registration, StatusTag } from './registration'; /** * Domain logic for a registration — pure functions, NO Angular. This is where @@ -32,27 +32,3 @@ export function statusColor(tag: StatusTag): string { export function herregistratieDeadline(reg: Registration): Date | null { return reg.status.tag === 'Geregistreerd' ? new Date(reg.status.herregistratieDatum) : null; } - -/** A registration may apply for herregistratie only while active and within the - window before its deadline. A struck-off or suspended registration may not. - SERVER-OWNED RULE: this now runs on the backend (BFF), which ships the result - as `decisions.eligibleForHerregistratie` in the dashboard view. Kept here as - the reference implementation + unit test; the frontend no longer calls it. */ -export function isHerregistratieEligible( - reg: Registration, - today: Date, - windowMonths = 12, -): boolean { - const deadline = herregistratieDeadline(reg); - if (!deadline) return false; - const windowStart = new Date(deadline); - windowStart.setMonth(windowStart.getMonth() - windowMonths); - return today >= windowStart; -} - -/** Invariant check used in tests/demos: a non-active status must not carry a - herregistratie date. The union already enforces this structurally; this is - the runtime statement of the same rule. */ -export function isStatusConsistent(status: RegistrationStatus): boolean { - return status.tag === 'Geregistreerd' ? typeof status.herregistratieDatum === 'string' : true; -} diff --git a/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs b/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs index 88089c4..1db366a 100644 --- a/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs +++ b/backend/src/BigRegister.Api/Domain/Submissions/SubmissionRules.cs @@ -21,14 +21,21 @@ public static class SubmissionRules private static readonly Regex PhonePattern = new(@"^0\d{9}$", RegexOptions.Compiled); + private static readonly Regex StrippedChars = + new(@"[\s\-()]", RegexOptions.Compiled); + private static readonly Regex LeadingCountryCode = + new(@"^\+31", RegexOptions.Compiled); // RULE: a contact change needs a well-formed Dutch phone number (10 digits, leading // 0, formatting stripped). The BRP address is authoritative and cannot be changed // here (WP-34), so only the phone is submitted. The server re-validates format - // authoritatively (the FE check is UX-only). + // authoritatively (the FE check is UX-only) — and must strip the SAME formatting the + // FE's parseTelefoonnummer does (whitespace/dashes/parens, a leading +31 → 0; WP-75), + // or the two sides disagree on what's a valid number. public static string? RejectPhoneChange(string telefoon) { - var digits = (telefoon ?? "").Trim().Replace(" ", "").Replace("-", ""); + var stripped = StrippedChars.Replace((telefoon ?? "").Trim(), ""); + var digits = LeadingCountryCode.Replace(stripped, "0"); if (!PhonePattern.IsMatch(digits)) return "Voer een geldig telefoonnummer in, bijv. 0612345678."; return null; } diff --git a/backend/tests/BigRegister.Tests/Acceptance/PhoneFormatContractTests.cs b/backend/tests/BigRegister.Tests/Acceptance/PhoneFormatContractTests.cs new file mode 100644 index 0000000..fb0838e --- /dev/null +++ b/backend/tests/BigRegister.Tests/Acceptance/PhoneFormatContractTests.cs @@ -0,0 +1,54 @@ +using System.Net; +using System.Net.Http.Json; +using BigRegister.Api.Contracts; + +namespace BigRegister.Tests.Acceptance; + +/// +/// Contract test for the FE/BE seam on phone-number stripping (WP-75). Both sides share +/// the same format regex (^0\d{9}$) but, until this test, diverged on what they +/// strip before checking it: the FE's parseTelefoonnummer +/// (registratie/domain/value-objects/telefoonnummer.ts) also drops parentheses and maps a +/// leading +31 to a leading 0; +/// used to strip only spaces and dashes. This was latent, not live — the FE always sends +/// its already-normalised value over the wire — but a crafted/future caller posting a raw, +/// FE-valid number would hit a backend that disagrees with the FE about what's valid. The +/// backend is the authority (ADR-0001), so it must agree with the FE on every FE-valid input. +/// +public class PhoneFormatContractTests(TestWebApplicationFactory factory) : IClassFixture +{ + private readonly HttpClient _client = factory.CreateClient(); + + private Task PostChangeRequest(string telefoon) => + _client.PostAsJsonAsync("/api/v1/change-requests", new ChangeRequestRequest(telefoon)); + + [Fact] + public async Task A_leading_plus31_is_accepted_like_the_frontends_normalised_form() + { + // Given a phone number in international form — parseTelefoonnummer maps a leading + // "+31" to "0" and accepts it (it becomes "0612345678", 10 digits starting 0). + + // When it is posted to the backend exactly as the user typed it (not FE-normalised)... + var res = await PostChangeRequest("+31612345678"); + + // Then the backend must agree it's valid, not reject it as malformed. + res.EnsureSuccessStatusCode(); + var body = (await res.Content.ReadFromJsonAsync())!; + Assert.NotNull(body.Referentie); + } + + [Fact] + public async Task Parentheses_around_the_area_code_are_accepted_like_the_frontend() + { + // Given a phone number with the area code in parentheses — parseTelefoonnummer strips + // "()" along with spaces/dashes and accepts it (it becomes "0612345678"). + + // When it is posted to the backend exactly as the user typed it... + var res = await PostChangeRequest("(06) 12345678"); + + // Then the backend must agree it's valid, not reject it as malformed. + res.EnsureSuccessStatusCode(); + var body = (await res.Content.ReadFromJsonAsync())!; + Assert.NotNull(body.Referentie); + } +} diff --git a/docs/reference/architecture/0001-bff-lite-decision-dtos.md b/docs/reference/architecture/0001-bff-lite-decision-dtos.md index 649bf78..7e18872 100644 --- a/docs/reference/architecture/0001-bff-lite-decision-dtos.md +++ b/docs/reference/architecture/0001-bff-lite-decision-dtos.md @@ -87,8 +87,8 @@ would compute. Two slices were implemented to demonstrate **both** policy shapes - `BigProfileStore` now derives `profile` and `decisions` from the single validated view (was a 3-resource `map2`). One request → one consistent snapshot. - `herregistratie.page.ts` reads `decisions.eligibleForHerregistratie` instead of - calling `isHerregistratieEligible()`. That rule is now marked server-owned in - `registration.policy.ts` (kept as reference impl + unit test; FE no longer calls it). + computing it client-side. That rule is server-owned: it lives only in + `HerregistratieRule.cs`, with no FE mirror to drift from it (WP-75). - The unused upstream adapters/mocks (`brp.adapter.ts`, `registration.json`, `brp.json`) were deleted — those calls live behind the BFF now. @@ -103,8 +103,10 @@ would compute. Two slices were implemented to demonstrate **both** policy shapes - `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. + (intake-typed) writes anything, 400ing an incomplete scholing answer instead of + silently accepting a crafted POST that skips it. (WP-72 deleted the legacy + `POST /intakes` endpoint this once also covered — deleting the surface is a stronger + fix than 400ing on it.) ## Migration sequence (for the real app) diff --git a/scripts/check-seam.sh b/scripts/check-seam.sh index 7ce848c..ea5f417 100755 --- a/scripts/check-seam.sh +++ b/scripts/check-seam.sh @@ -31,3 +31,35 @@ if [ "$backend_value" != "$frontend_value" ]; then fi echo "OK: scholing threshold default matches on both sides ($backend_value)" + +# WP-75: fail if the backend's Besluit enum and the frontend's BESLUIT_TAGS list (the wire +# convention: a string, not a raw enum) drift apart. Enum.TryParse at Program.cs:494 +# is the only coupling and it fails at REQUEST time, not build time — this is a build-time +# tripwire for the same names/order both sides assume. +BESLUIT_BACKEND_FILE='backend/src/BigRegister.Api/Domain/Applications/AanvraagStatus.cs' +BESLUIT_FRONTEND_FILE='apps/behandelportal/src/app/behandeling/domain/besluit.machine.ts' + +besluit_backend_raw=$(grep -oE 'public enum Besluit \{[^}]*\}' "$BESLUIT_BACKEND_FILE" | grep -oE '\{[^}]*\}') +besluit_frontend_raw=$(grep -oE "const BESLUIT_TAGS = \[[^]]*\]" "$BESLUIT_FRONTEND_FILE" | grep -oE '\[[^]]*\]') + +if [ -z "$besluit_backend_raw" ]; then + echo "FAIL: could not find 'public enum Besluit { ... }' in $BESLUIT_BACKEND_FILE" + exit 1 +fi +if [ -z "$besluit_frontend_raw" ]; then + echo "FAIL: could not find 'const BESLUIT_TAGS = [ ... ]' in $BESLUIT_FRONTEND_FILE" + exit 1 +fi + +besluit_backend_value=$(echo "$besluit_backend_raw" | tr -d '{}' | tr -d ' ') +besluit_frontend_value=$(echo "$besluit_frontend_raw" | tr -d '[]' | tr -d "' " ) + +if [ "$besluit_backend_value" != "$besluit_frontend_value" ]; then + echo "FAIL: FE/BE seam drift on the Besluit tag list" + echo " $BESLUIT_BACKEND_FILE: Besluit { $besluit_backend_value }" + echo " $BESLUIT_FRONTEND_FILE: BESLUIT_TAGS = [ $besluit_frontend_value ]" + echo 'Both lists are the same wire-convention names/order (Enum.TryParse at Program.cs) and must match.' + exit 1 +fi + +echo "OK: Besluit tag list matches on both sides ($besluit_backend_value)"