From 7a16b1b52fad2e52d4b4c35c0d99fb80d0464f9e Mon Sep 17 00:00:00 2001 From: Tudor Date: Thu, 27 Aug 2026 21:17:16 +0100 Subject: [PATCH] fix(admissions): flag-off pages must not speak for the council MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The secondary admissions section words the absence of a cut-off distance: " has not published a cut-off distance for this school." That sentence is true when the authority publishes nothing. It is false when the authority does publish and the admission_distance flag is simply off — and off is the current state, so every secondary page with an EES admissions row has been making a claim about a council on our behalf. The backend already draws the distinction the copy needs. /api/schools/{urn} omits the admission_distance key entirely while the flag is dark rather than sending null, precisely so that "we are not publishing cut-offs" stays distinguishable from "this school has no cut-off"; lib/types.ts says so in as many words. The page then collapsed the two with `?? null` before the section ever saw them. So stop collapsing it: thread the raw field to SecondarySchoolSections and word the absence only when the feature is on. Null still gets the sentence naming the authority — that case is unchanged and still tested. Primary pages are unaffected: AdmissionsSection carries no absence copy and renders nothing when there is no figure. DistanceSection already treated absent and null alike; only its type widens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM --- e2e/tests/journeys.spec.ts | 46 +++++++++++++++++++ .../components/lastDistanceOffered.test.tsx | 11 +++++ nextjs-app/app/school/[slug]/page.tsx | 2 +- .../components/school/DistanceSection.tsx | 2 +- .../school/SecondaryAdmissionsSection.tsx | 12 +++-- .../school/SecondarySchoolSections.tsx | 5 +- 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 966f2ab..e006c5d 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1304,6 +1304,52 @@ test('with the distance feature off, the section is absent rather than empty', a .toHaveCount(0); }); +/** + * A secondary school carrying an EES admissions row, which is what makes its + * Admissions section render while the distance feature is dark. + */ +async function secondarySchoolWithAdmissions(page: Page) { + const list = await page.request.get('/api/schools?phase=secondary&page_size=40'); + if (!list.ok()) return null; + const body = await list.json(); + for (const s of (body?.schools ?? []).slice(0, 25)) { + const res = await page.request.get(`/api/schools/${s.urn}`); + if (!res.ok()) continue; + const detail = await res.json(); + if (detail?.admissions == null) continue; + return { urn: s.urn as number }; + } + return null; +} + +test('with the distance feature off, a secondary page makes no claim about publication', async ({ page }) => { + /* + * Shipping dark must not put words in the council's mouth. The secondary + * template is the only one that words the absence, and "X has not published + * a cut-off distance for this school" is false wherever X does publish and + * we are simply withholding it. + * + * This is why the API omits the key rather than sending null: absent means + * "cut-offs are not published at all", null means "this school has none". + * Only the second is a fact about the school, and only the second is sayable. + */ + test.skip(await distanceFeatureIsOn(page), + 'the admission_distance flag is on in this environment'); + + const found = await secondarySchoolWithAdmissions(page); + test.skip(found === null, 'no secondary school in the sample has an admissions row'); + + await page.goto(`/school/${found!.urn}`); + await expect(page.locator('h1').first()).toBeVisible({ timeout: 15_000 }); + + // The Admissions section is still there — this is not a test that the whole + // section vanished, which would pass for the wrong reason. + await expect(page.locator('#admissions')).toHaveCount(1); + + await expect(page.getByText(/has not published a cut-off distance/)).toHaveCount(0); + await expect(page.getByText(/Contact the admissions authority/)).toHaveCount(0); +}); + test('/api/flags is not reachable from the public internet', async ({ page }) => { // It names every unreleased feature and whether it is on. Next reads it // server-side over the Docker network; the public proxy must deny it. diff --git a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx index 9f16df5..3f9efa1 100644 --- a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx +++ b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx @@ -98,6 +98,17 @@ describe('secondary detail page', () => { expect(screen.getByText(/has not published a cut-off distance/)).toBeInTheDocument(); }); + + it('makes no claim about publication when the feature is switched off', () => { + // Absent, not null. The API omits the key entirely while the + // admission_distance flag is off, and "Islington has not published a + // cut-off distance" is then a statement about us, not about Islington — + // false wherever the authority does publish one. + renderSecondarySchoolDetail({ ...secondaryFixture, admissionDistance: undefined }); + + expect(screen.queryByText(/has not published a cut-off distance/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Contact the admissions authority/)).not.toBeInTheDocument(); + }); }); // ── The Distance section ─────────────────────────────────────────────── diff --git a/nextjs-app/app/school/[slug]/page.tsx b/nextjs-app/app/school/[slug]/page.tsx index 4e9f6aa..bef695a 100644 --- a/nextjs-app/app/school/[slug]/page.tsx +++ b/nextjs-app/app/school/[slug]/page.tsx @@ -232,7 +232,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { census={census ?? null} admissions={admissions ?? null} admissionsHistory={admissions_history ?? []} - admissionDistance={admission_distance ?? null} + admissionDistance={admission_distance} deprivation={deprivation ?? null} finance={finance ?? null} nationalAvg={nationalAvg} diff --git a/nextjs-app/components/school/DistanceSection.tsx b/nextjs-app/components/school/DistanceSection.tsx index 8990960..45651b5 100644 --- a/nextjs-app/components/school/DistanceSection.tsx +++ b/nextjs-app/components/school/DistanceSection.tsx @@ -24,7 +24,7 @@ export function DistanceSection({ admissionDistance, schoolInfo, }: { - admissionDistance: SchoolAdmissionDistance | null; + admissionDistance: SchoolAdmissionDistance | null | undefined; schoolInfo: School; }) { // Without a figure there is nothing to compare against, and without diff --git a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx index 49c8970..63471dc 100644 --- a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx +++ b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx @@ -21,11 +21,17 @@ export function SecondaryAdmissionsSection({ published cut-off and no EES admissions row. */ admissions: SchoolAdmissions | null; admissionsHistory: SchoolAdmissions[]; - admissionDistance: SchoolAdmissionDistance | null; + admissionDistance: SchoolAdmissionDistance | null | undefined; schoolInfo: School; hasSixthForm: boolean; }) { const cutoff = describeCutoff(admissionDistance); + /* Absent means cut-offs are not being published at all; null means this + school has no published cut-off. Only the second is a fact about the + school, and only the second can be stated. Saying "X has not published a + cut-off" while the feature is dark describes us, and is false wherever the + authority does publish one. */ + const featureOn = admissionDistance !== undefined; // Moved with this section from SecondarySchoolDetailView, its only consumer. const admissionsTag = (() => { const policy = schoolInfo.admissions_policy?.toLowerCase() ?? ''; @@ -102,7 +108,7 @@ export function SecondaryAdmissionsSection({ {CUTOFF_NOTE} {CUTOFF_MEASUREMENT_NOTE} {cutoff.routeNote && <> {cutoff.routeNote}}

- ) : ( + ) : featureOn ? (

{describeCutoffAbsence({ localAuthority: schoolInfo.local_authority, @@ -110,7 +116,7 @@ export function SecondaryAdmissionsSection({ admissionsHistory, })}

- )} + ) : null} {hasSixthForm && (
diff --git a/nextjs-app/components/school/SecondarySchoolSections.tsx b/nextjs-app/components/school/SecondarySchoolSections.tsx index 1ba3d7a..10bf3c8 100644 --- a/nextjs-app/components/school/SecondarySchoolSections.tsx +++ b/nextjs-app/components/school/SecondarySchoolSections.tsx @@ -36,7 +36,10 @@ export interface SecondarySchoolSectionsProps { /** Needed to tell a year with no published cut-off apart from a year the * school simply was not oversubscribed. */ admissionsHistory: SchoolAdmissions[]; - admissionDistance: SchoolAdmissionDistance | null; + /** Absent — not null — while the admission_distance flag is off. The two + * mean different things to the reader and must stay distinguishable: + * see SecondaryAdmissionsSection, which words the absence. */ + admissionDistance: SchoolAdmissionDistance | null | undefined; deprivation: SchoolDeprivation | null; finance: SchoolFinance | null; nationalAvg: NationalAverages | null;