From ef4a2ccccbe2575ad6008d147912ff0db15cfb6e Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 23:43:43 +0100 Subject: [PATCH] fix(school): send the school page its admissions policy, and read it exactly The header's Selective flag read school_info.admissions_policy, which the detail endpoint never sent, so no school page could flag Selective while its search row did (staging E2E: The Grammar School at Leeds). The detail payload now carries it, and a contract test checks it carries every field the header's flags read. Sending it would have switched on two older copies of the tag logic #176 fixed in the rows. The Admissions section and the cut-off note both tested includes('selective'), so every non-selective secondary would have read "entry is by selective examination". The section also counted "None" as a faith: Burntwood reads "a faith-based admissions priority (None)" today. All of them now share isSelective() and hasReligiousCharacter(), which also treats "Not applicable" as no faith, as the place table already does. Co-Authored-By: Claude Opus 5.5 --- backend/app.py | 2 + backend/tests/test_school_page_flag_fields.py | 55 ++++++++++++++++++ .../secondaryAdmissionsTag.test.tsx | 58 +++++++++++++++++++ .../__tests__/lib/lastDistanceOffered.test.ts | 7 +++ nextjs-app/__tests__/lib/schoolFacts.test.ts | 2 + .../school/SecondaryAdmissionsSection.tsx | 8 +-- .../components/school/lastDistanceOffered.ts | 5 +- nextjs-app/lib/schoolFacts.ts | 4 +- nextjs-app/lib/utils.ts | 16 ++++- 9 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 backend/tests/test_school_page_flag_fields.py create mode 100644 nextjs-app/__tests__/components/secondaryAdmissionsTag.test.tsx diff --git a/backend/app.py b/backend/app.py index a746d74..d0019e5 100644 --- a/backend/app.py +++ b/backend/app.py @@ -1047,6 +1047,8 @@ async def get_school_details(request: Request, urn: int): "age_range": latest.get("age_range", ""), "has_sixth_form": latest.get("has_sixth_form"), "nursery_provision": latest.get("nursery_provision"), + # The header's Selective flag reads it (lib/schoolFacts). + "admissions_policy": latest.get("admissions_policy"), "status": latest.get("status"), "latitude": latest.get("latitude"), "longitude": latest.get("longitude"), diff --git a/backend/tests/test_school_page_flag_fields.py b/backend/tests/test_school_page_flag_fields.py new file mode 100644 index 0000000..10cf075 --- /dev/null +++ b/backend/tests/test_school_page_flag_fields.py @@ -0,0 +1,55 @@ +"""The school page's header flags (nextjs-app/lib/schoolFacts.ts) read seven +school_info fields, so the detail payload must carry every one. + +It lacked admissions_policy, so no school page could flag Selective while its +search row did: Tiffin and The Grammar School at Leeds showed the tag in +search and nothing on their own pages. +""" + +import numpy as np +import pandas as pd +import pytest +from fastapi.testclient import TestClient + +# The fields schoolFlags() picks from School (FlagFields in lib/schoolFacts.ts). +FLAG_FIELDS = ( + "type_group", "admissions_policy", "gender", "religious_denomination", + "nursery_provision", "has_sixth_form", "phase", +) + + +def _school_df() -> pd.DataFrame: + return pd.DataFrame([{ + "urn": 136910, "school_name": "Tiffin School", "phase": "Secondary", + "school_type": "Academy converter", "admissions_policy": "Selective", + "gender": "Boys", "religious_denomination": "Christian", + "nursery_provision": "Not applicable", "has_sixth_form": True, + "age_range": "11-18", "local_authority": "Kingston upon Thames", + "address": "Queen Elizabeth Road, Kingston upon Thames, KT2 6RL", + "postcode": "KT2 6RL", "latitude": 51.41, "longitude": -0.30, + "year": 202425, "attainment_8_score": 75.0, "total_pupils": 200, + "gias_total_pupils": 1478, "ofsted_grade": np.nan, + }]) + + +@pytest.fixture() +def client(monkeypatch): + from backend import app as app_module + + monkeypatch.setattr(app_module, "load_school_data", _school_df) + monkeypatch.setattr(app_module, "load_latest_school_data", _school_df) + monkeypatch.setattr(app_module, "get_supplementary_data", lambda db, urn: {}) + monkeypatch.setattr(app_module, "_place_registry", None) + return TestClient(app_module.app, raise_server_exceptions=False) + + +def test_the_school_page_carries_every_field_its_flags_read(client): + resp = client.get("/api/schools/136910") + assert resp.status_code == 200, resp.text + info = resp.json()["school_info"] + assert [f for f in FLAG_FIELDS if f not in info] == [] + + +def test_the_school_page_says_a_selective_school_is_selective(client): + info = client.get("/api/schools/136910").json()["school_info"] + assert info["admissions_policy"] == "Selective" diff --git a/nextjs-app/__tests__/components/secondaryAdmissionsTag.test.tsx b/nextjs-app/__tests__/components/secondaryAdmissionsTag.test.tsx new file mode 100644 index 0000000..8b6bd32 --- /dev/null +++ b/nextjs-app/__tests__/components/secondaryAdmissionsTag.test.tsx @@ -0,0 +1,58 @@ +/** + * The Admissions section's Selective and faith notes. + * + * It tested the policy with includes('selective'), which "Non-selective" + * passes, and excluded only "Does not apply" from the religious character, so + * Burntwood (no religious character, recorded as "None") read "this school + * has a faith-based admissions priority (None)". The Selective half never + * fired only because the school page's API did not send admissions_policy. + */ + +import { screen, within } from '@testing-library/react'; +import type { School } from '@/lib/types'; +import { secondaryFixture } from '../support/schoolFixtures'; +import { renderSecondarySchoolDetail } from '../support/renderSchoolDetail'; + +jest.mock('@/lib/analytics', () => ({ + track: jest.fn(), + getNavigationSource: () => 'direct', +})); +jest.mock('@/components/PerformanceChart', () => ({ + PerformanceChart: () =>
, +})); +jest.mock('@/components/AdmissionsTrendChart', () => ({ + __esModule: true, + default: () =>
, +})); +jest.mock('@/components/SchoolHeroMap', () => ({ + SchoolHeroMap: () =>
, + __esModule: true, +})); + +function admissionsOf(info: Partial) { + renderSecondarySchoolDetail({ ...secondaryFixture, schoolInfo: { ...secondaryFixture.schoolInfo, ...info } }); + return within(screen.getByRole('heading', { name: 'Admissions' }).closest('section')!); +} + +describe('Admissions section notes', () => { + it('notes the entrance test for a selective school', () => { + const section = admissionsOf({ admissions_policy: 'Selective', religious_denomination: 'None' }); + expect(section.getByText('Selective:')).toBeInTheDocument(); + }); + + it('does not call a non-selective school selective', () => { + const section = admissionsOf({ admissions_policy: 'Non-selective', religious_denomination: 'None' }); + expect(section.queryByText('Selective:')).not.toBeInTheDocument(); + }); + + it.each(['None', 'Does not apply'])('claims no faith priority when the register says %p', (religious_denomination) => { + // Not "Non-selective": a misread Selective would win and hide this case. + const section = admissionsOf({ admissions_policy: 'Not applicable', religious_denomination }); + expect(section.queryByText('Faith priority:')).not.toBeInTheDocument(); + }); + + it('notes the religious character of a faith school', () => { + const section = admissionsOf({ admissions_policy: 'Non-selective', religious_denomination: 'Church of England' }); + expect(section.getByText('Faith priority:')).toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts b/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts index c148136..6017db2 100644 --- a/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts +++ b/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts @@ -157,6 +157,13 @@ describe('compareToCutoff', () => { }); describe('describeCutoffAbsence', () => { + it('does not read "Non-selective" as selective', () => { + // A substring test matched "selective" inside "Non-selective". + const s = describeCutoffAbsence({ localAuthority: 'Wandsworth', admissionsPolicy: 'Non-selective' }); + expect(s).not.toMatch(/entrance test/); + expect(s).toMatch(/^Wandsworth has not published/); + }); + it('explains a selective school by how it admits, not as missing data', () => { const s = describeCutoffAbsence({ localAuthority: 'Kent', admissionsPolicy: 'Selective' }); expect(s).toContain('entrance test'); diff --git a/nextjs-app/__tests__/lib/schoolFacts.test.ts b/nextjs-app/__tests__/lib/schoolFacts.test.ts index 575944f..e1f4ff2 100644 --- a/nextjs-app/__tests__/lib/schoolFacts.test.ts +++ b/nextjs-app/__tests__/lib/schoolFacts.test.ts @@ -59,6 +59,8 @@ describe('schoolFlags', () => { it('never prints Non-selective, and never a no-faith value', () => { expect(labels({ admissions_policy: 'Non-selective', religious_denomination: 'Does not apply' })).toEqual([]); expect(labels({ admissions_policy: 'Not applicable', religious_denomination: 'None' })).toEqual([]); + // PlaceView's rule: GIAS sometimes says "Not applicable" for no faith too. + expect(labels({ religious_denomination: 'Not applicable' })).toEqual([]); }); it('prints the religious character as the register records it', () => { diff --git a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx index 847be54..98c4d66 100644 --- a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx +++ b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx @@ -7,7 +7,7 @@ */ import type { School, SchoolAdmissions, SchoolAdmissionDistance } from '@/lib/types'; -import { formatPercentage } from '@/lib/utils'; +import { formatPercentage, hasReligiousCharacter, isSelective } from '@/lib/utils'; import { Section, sectionStyles as styles } from './sectionShared'; import { describeCutoff, describeCutoffAbsence, @@ -33,10 +33,8 @@ export function SecondaryAdmissionsSection({ const featureOn = admissionDistance !== undefined; // Moved with this section from SecondarySchoolDetailView, its only consumer. const admissionsTag = (() => { - const policy = schoolInfo.admissions_policy?.toLowerCase() ?? ''; - if (policy.includes('selective')) return 'Selective'; - const denom = schoolInfo.religious_denomination ?? ''; - if (denom && denom !== 'Does not apply') return 'Faith priority'; + if (isSelective(schoolInfo.admissions_policy)) return 'Selective'; + if (hasReligiousCharacter(schoolInfo.religious_denomination)) return 'Faith priority'; return null; })(); diff --git a/nextjs-app/components/school/lastDistanceOffered.ts b/nextjs-app/components/school/lastDistanceOffered.ts index 3153240..33f3725 100644 --- a/nextjs-app/components/school/lastDistanceOffered.ts +++ b/nextjs-app/components/school/lastDistanceOffered.ts @@ -16,7 +16,7 @@ */ import type { SchoolAdmissionDistance } from '@/lib/types'; -import { formatCutoffDistance, formatMiles, formatEntryYear } from '@/lib/utils'; +import { formatCutoffDistance, formatMiles, formatEntryYear, isSelective } from '@/lib/utils'; export interface CutoffDisplay { /** Headline figure, e.g. "0.31 miles". */ @@ -193,8 +193,7 @@ export function describeCutoffAbsence({ admissionsPolicy, admissionsHistory = [], }: AbsenceInput): string { - const policy = (admissionsPolicy ?? '').toLowerCase(); - if (policy.includes('selective')) { + if (isSelective(admissionsPolicy)) { return 'Places at this school are ranked by the entrance test rather than by ' + 'distance, so no cut-off distance applies.'; } diff --git a/nextjs-app/lib/schoolFacts.ts b/nextjs-app/lib/schoolFacts.ts index 24e1ce2..7032d5d 100644 --- a/nextjs-app/lib/schoolFacts.ts +++ b/nextjs-app/lib/schoolFacts.ts @@ -7,7 +7,7 @@ */ import type { School } from './types'; -import { hasNurseryClasses, hasReligiousCharacter, singleSexLabel } from './utils'; +import { hasNurseryClasses, hasReligiousCharacter, isSelective, singleSexLabel } from './utils'; /** The search filter's type groups (backend/school_groups.py), without its * parenthesised notes: fees have a flag of their own. */ @@ -54,7 +54,7 @@ export function schoolFlags(school: FlagFields): SchoolFlag[] { const phase = school.phase?.trim().toLowerCase(); if (school.type_group === 'independent') condition('Fee-paying'); - if (school.admissions_policy?.trim().toLowerCase() === 'selective') condition('Selective'); + if (isSelective(school.admissions_policy)) condition('Selective'); const singleSex = singleSexLabel(school.gender); if (singleSex) condition(singleSex); if (hasReligiousCharacter(school.religious_denomination)) { diff --git a/nextjs-app/lib/utils.ts b/nextjs-app/lib/utils.ts index 4789080..8fbea04 100644 --- a/nextjs-app/lib/utils.ts +++ b/nextjs-app/lib/utils.ts @@ -923,12 +923,22 @@ export function isSpecialSchool(school: { school_type?: string | null }): boolea } /** - * Whether GIAS records a religious character. "None" and "Does not apply" are - * the register's two ways of saying it has none, and neither is a faith. + * Whether GIAS records a religious character. "None", "Does not apply" and + * "Not applicable" are the register's ways of saying it has none; the place + * table (PlaceView's NO_FAITH) reads the same three. */ export function hasReligiousCharacter(value: string | null | undefined): boolean { const v = value?.trim().toLowerCase() ?? ''; - return v !== '' && v !== 'none' && v !== 'does not apply'; + return v !== '' && v !== 'none' && v !== 'does not apply' && v !== 'not applicable'; +} + +/** + * Whether GIAS records the school as selective. Exact: "Non-selective" + * contains "selective", so a substring test read every comprehensive as + * selective. (The register files a partly selective school as non-selective.) + */ +export function isSelective(admissionsPolicy: string | null | undefined): boolean { + return admissionsPolicy?.trim().toLowerCase() === 'selective'; } /**