Compare commits

...
Author SHA1 Message Date
TudorandClaude Opus 5 7a16b1b52f fix(admissions): flag-off pages must not speak for the council
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m8s
The secondary admissions section words the absence of a cut-off distance:
"<LA> 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 21:17:16 +01:00
6 changed files with 72 additions and 6 deletions

No files matched your search

+46
View File
@@ -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.
@@ -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 ───────────────────────────────────────────────
+1 -1
View File
@@ -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}
@@ -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
@@ -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}</>}
</p>
) : (
) : featureOn ? (
<p className={styles.sectionSubtitle} style={{ marginTop: '1rem' }}>
{describeCutoffAbsence({
localAuthority: schoolInfo.local_authority,
@@ -110,7 +116,7 @@ export function SecondaryAdmissionsSection({
admissionsHistory,
})}
</p>
)}
) : null}
{hasSixthForm && (
<div className={styles.sixthFormNote}>
@@ -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;