Compare commits

...
2 Commits
Author SHA1 Message Date
TudorandClaude Opus 5.5 da5d63593f test(e2e): a no-faith comprehensive makes no Selective or faith claim
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m13s
PR Checks / Backend Smoke (pull_request) Successful in 10s
PR Checks / Build Backend (no push) (pull_request) Successful in 19s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m20s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 16s
The Admissions section of a non-selective secondary with no religious
character must say neither "Selective:" nor "Faith priority:". Run against
staging before the fix, it fails on "Faith priority" (Burntwood's "(None)").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 23:43:43 +01:00
TudorandClaude Opus 5.5 ef4a2ccccb 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 <noreply@anthropic.com>
2026-10-02 23:43:43 +01:00
10 changed files with 172 additions and 13 deletions

No files matched your search

+2
View File
@@ -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"),
@@ -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"
+28
View File
@@ -3396,3 +3396,31 @@ test('a selective school is flagged Selective, on its page and in search', async
const tags = await rowTags(page, school);
await expect(tags.getByText('Selective', { exact: true })).toBeVisible();
});
/*
* The Admissions section repeated the search rows' old tag logic: "selective"
* matched inside "Non-selective", and "None" counted as a faith, so Burntwood
* read "this school has a faith-based admissions priority (None)". The
* Selective half stayed hidden only because the school page's API did not
* send admissions_policy. Data-invariant: the school comes from the API, and
* must have an Admissions section to read.
*/
test('a non-selective school with no faith makes neither claim in its Admissions section', async ({ page }) => {
const res = await page.request.get(
'/api/schools?search=school&phase=secondary&faith=none&admissions_policy=non-selective&page_size=20');
expect(res.ok()).toBeTruthy();
let urn: number | null = null;
for (const s of ((await res.json()).schools ?? []).slice(0, 8)) {
if (s.religious_denomination !== 'None') continue;
const detail = await (await page.request.get(`/api/schools/${s.urn}`)).json();
if (detail.admissions) { urn = s.urn; break; }
}
test.skip(urn == null, 'no non-selective, no-faith secondary with admissions data here');
await page.goto(`/school/${urn}`);
const admissions = page.locator('section#admissions');
await expect(admissions).toBeVisible({ timeout: 15_000 });
await expect(admissions.getByText('Selective:', { exact: true })).toHaveCount(0);
await expect(admissions.getByText('Faith priority:', { exact: true })).toHaveCount(0);
await expect(admissions.getByText(/\(None\)/)).toHaveCount(0);
});
@@ -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: () => <div data-testid="performance-chart" />,
}));
jest.mock('@/components/AdmissionsTrendChart', () => ({
__esModule: true,
default: () => <div data-testid="admissions-trend-chart" />,
}));
jest.mock('@/components/SchoolHeroMap', () => ({
SchoolHeroMap: () => <div data-testid="hero-map" />,
__esModule: true,
}));
function admissionsOf(info: Partial<School>) {
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();
});
});
@@ -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');
@@ -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', () => {
@@ -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;
})();
@@ -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.';
}
+2 -2
View File
@@ -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)) {
+13 -3
View File
@@ -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';
}
/**