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)"