Merge pull request 'fix(school): send the school page its admissions policy, and read it exactly' (#179) from fix/school-page-selective-flag into main

Reviewed-on: #179

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

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"

e2e/tests/journeys.spec.ts

@@ -410,6 +410,29 @@ test('search and the school page agree on how many pupils a secondary has', asyn
   expect(school.total_pupils).toBe(detail.school_info.total_pupils);
 });
 
+test('a secondary search row compares its Attainment 8 with the LA average', async ({ page }) => {
+  // The comparison vanished unnoticed: the averages were fetched with
+  // force-cache, so one stored failure hid it in that browser for good.
+  // Playwright disables the HTTP cache when it intercepts requests, so this
+  // guards the comparison itself; the unit test pins the cache mode.
+  const la = await (await page.request.get('/api/la-averages')).json();
+  const averages: Record<string, number> = la.secondary?.attainment_8_by_la ?? {};
+  const res = await page.request.get('/api/schools?search=school&phase=secondary&page_size=50');
+  expect(res.ok()).toBeTruthy();
+  const school = ((await res.json()).schools ?? []).find(
+    (s: { attainment_8_score?: number | null; local_authority?: string; school_type?: string }) =>
+      s.attainment_8_score != null && s.local_authority != null && averages[s.local_authority] != null
+      && !/special|pupil referral|alternative provision/i.test(s.school_type ?? ''));
+  test.skip(!school, 'no mainstream secondary with an LA average here');
+
+  await searchByName(page, school.school_name);
+  const link = page.locator(`a[href^="/school/${school.urn}-"]`).first();
+  await expect(link).toBeVisible({ timeout: 15_000 });
+  const stats = link.locator('xpath=ancestor::div[contains(@class, "__rowContent")][1]')
+    .locator('[class*="__line3"]');
+  await expect(stats.getByText(/vs LA avg/)).toBeVisible();
+});
+
 test('a phase outside primary/secondary filters to that phase, not to everything', async ({ page }) => {
   // The search page offers every GIAS phase, but the API only knew the grouped
   // ones and silently dropped the rest — so "Nursery" returned primaries.
@@ -3396,3 +3419,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);
+});

nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx

@@ -1,6 +1,6 @@
 import { act, fireEvent, render, screen } from '@testing-library/react';
 import { HomeView } from '@/components/HomeView';
-import { fetchSchools } from '@/lib/api';
+import { fetchLAaverages, fetchSchools } from '@/lib/api';
 import { primaryFixture } from '../support/schoolFixtures';
 import type { SchoolsResponse, School } from '@/lib/types';
 
@@ -84,3 +84,23 @@ test('failed map requests can be retried by reopening the map', async () => {
   expect(fetchSchools).toHaveBeenCalledTimes(2);
   expect(screen.getByTestId('map')).toHaveTextContent('Retry result');
 });
+
+test('LA averages are not fetched with force-cache, so one failure is not replayed for good', async () => {
+  // force-cache serves any stored response, however old, without asking the
+  // server. A request that failed once (a staging deploy restart, the July
+  // proxy outage) was stored and replayed on every later visit, and the
+  // "vs LA avg" delta vanished from every secondary row in that browser.
+  // The default mode honours the API's Cache-Control and never reuses an
+  // error.
+  params = new URLSearchParams('search=high');
+  const secondary: SchoolsResponse = {
+    ...response('Alpha High'),
+    schools: [{ ...primaryFixture.schoolInfo, school_name: 'Alpha High', phase: 'Secondary', attainment_8_score: 50 }],
+  };
+  render(<HomeView initialSchools={secondary} filters={filters} />);
+  await act(async () => {});
+  expect(fetchLAaverages).toHaveBeenCalled();
+  for (const [options] of jest.mocked(fetchLAaverages).mock.calls) {
+    expect(options?.cache).not.toBe('force-cache');
+  }
+});

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: () => <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();
+  });
+});

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');

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', () => {

nextjs-app/components/HomeView.tsx

@@ -358,10 +358,14 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
     return () => controller.abort();
   }, [resultsView, searchParams, initialSchools.schools]);
 
-  // Fetch LA averages when secondary or mixed schools are visible
+  // Fetch LA averages when secondary or mixed schools are visible. Default
+  // cache mode, never force-cache: force-cache replays any stored response
+  // without asking the server, so one failed request (a deploy restart) hid
+  // every "vs LA avg" delta in that browser for good. The API's Cache-Control
+  // already lets the browser reuse a good answer for five minutes.
   useEffect(() => {
     if (!isSecondaryView && !isMixedView) return;
-    fetchLAaverages({ cache: 'force-cache' })
+    fetchLAaverages()
       .then(data => setLaAverages(data.secondary.attainment_8_by_la))
       .catch(() => {});
   }, [isSecondaryView, isMixedView]);

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 (isSelective(schoolInfo.admissions_policy)) return 'Selective';
-    if (policy.includes('selective')) return 'Selective';
+    if (hasReligiousCharacter(schoolInfo.religious_denomination)) return 'Faith priority';
-    const denom = schoolInfo.religious_denomination ?? '';
-    if (denom && denom !== 'Does not apply') return 'Faith priority';
     return null;
   })();
 

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 (isSelective(admissionsPolicy)) {
-  if (policy.includes('selective')) {
     return 'Places at this school are ranked by the entrance test rather than by '
       + 'distance, so no cut-off distance applies.';
   }

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

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
+ * Whether GIAS records a religious character. "None", "Does not apply" and
- * the register's two ways of saying it has none, and neither is a faith.
+ * "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';
 }
 
 /**