Compare commits

..
Author SHA1 Message Date
TudorandClaude Opus 5.5 59ea8a4bdd fix(search): stop replaying a failed LA-averages request forever
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m12s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m18s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 18s
The search page fetched LA averages with cache: 'force-cache', which serves
any stored response, however old, without asking the server. One failed
request (a staging deploy restart; the July proxy outage) was stored and
replayed on every later visit, and the error was swallowed, so the
"vs LA avg" delta silently vanished from every secondary row in that
browser. A Playwright profile still held a 500 dated 5 July.

The default cache mode honours the API's Cache-Control (five minutes), so
a good answer is still reused and an error never is. Browsers holding a
stored failure recover on their next visit.

A journey now checks that a mainstream secondary's row shows the
comparison: nothing did, which is how it could go missing unnoticed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 23:20:14 +01:00
tudor c931d1078c Merge pull request 'fix(search): tag only what the register records, and count the whole school' (#176) from fix/search-row-facts into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 0s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 19s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m24s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 29s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 3m11s
Reviewed-on: #176
2026-10-02 22:05:56 +00:00
tudor 807133c305 Merge pull request 'fix(school): show Nursery only for nursery classes, and say Girls' school' (#175) from fix/header-nursery-and-gender into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 20s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m25s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 27s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 3m8s
Reviewed-on: #175
2026-10-02 22:04:29 +00:00
TudorandClaude Opus 5.5 8020191832 test(e2e): journeys for row tags and pupil counts, and ask for page_size
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m13s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m19s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 17s
Three journeys pin the fixes: a non-selective secondary's row has no
Selective tag, a school recorded with no religious character has no faith
tag, and the list and the school page agree on a secondary's pupil count.
Run against staging before the fix, all three fail.

The list API reads page_size. Nine journeys asked for per_page, which it
ignores, so each got the default 25 rows whatever it asked for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 22:51:31 +01:00
TudorandClaude Opus 5.5 5f9caad7f4 fix: count pupils as the whole school, not the GCSE year group
fact_performance's total_pupils is the cohort a year's results were
measured on. For a secondary that is Year 11 alone, and the list API sent
it as the card's "pupils": Burntwood showed 245 in search and 1,462 on its
page. The list and place payloads now carry the register's whole-school
count, and nothing when the register has none. Map popups and the compare
basket read the same field.

The header and the wellbeing section fell back to the same results figure
when the census had no record. They now fall back to the register count.
The shell took yearlyData only for that fallback, so the prop is gone and
the results array no longer ships to the client for the chrome.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 22:51:31 +01:00
TudorandClaude Opus 5.5 e65c93b68e fix(search): tag only what the register records
The secondary row tested the admissions policy with includes('selective'),
which "Non-selective" passes, so every comprehensive (Burntwood, Graveney)
was tagged Selective. It now needs an exact "Selective".

Both rows excluded only "Does not apply" from the religious character, so a
school recorded as "None" got "Faith priority" (Putney High) or a bare
"None" chip (Abacus Belsize Primary). hasReligiousCharacter() treats both of
the register's no-faith values as no faith.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 22:50:44 +01:00
TudorandClaude Opus 5.5 e3f21a5bc7 fix(school): address review on the header chip fixes
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 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m18s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 15s
- E2E: list with page_size (per_page was ignored), fail clearly if the
  gender filter is ignored, and require nursery_provision on the detail
  payload so the Nursery assertion cannot pass vacuously.
- singleSexLabel ignores case, as hasNurseryClasses does.
- Header test: type withSchool with Partial<School>, and match the
  single-sex labels exactly instead of any span ending in "school".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 22:16:49 +01:00
TudorandClaude Opus 5.5 dea435a906 fix(school): show Nursery only for nursery classes, and say Girls' school
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 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m19s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 16s
GIAS NurseryProvision is text ("Has Nursery Classes", "No Nursery
Classes", "Not applicable"), but the header and the place table tested it
for truthiness. Every school with a value got a Nursery chip or a "Yes",
including secondaries aged 11-18. hasNurseryClasses() matches the one
value that means a nursery, and the type now says the field is a string.

The single-sex chip appended 's to the plural GIAS gender, giving
"Girls's school". singleSexLabel() gives "Girls' school" / "Boys' school".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-02 22:02:32 +01:00
20 changed files with 513 additions and 47 deletions

No files matched your search

+15 -2
View File
@@ -621,6 +621,19 @@ def verify_admin_api_key(x_api_key: str = Header(None)) -> bool:
return True
def _with_whole_school_pupils(rows: pd.DataFrame, source: pd.DataFrame) -> pd.DataFrame:
"""Set total_pupils to the size of the school.
fact_performance's total_pupils is the cohort a year's results were
measured on. For a secondary that is the GCSE year group alone (Burntwood:
245 against 1,462 on roll). Cards, map popups and place rows label it
"pupils", so they take the register's whole-school count instead, and
nothing when the register has none.
"""
whole = source["gias_total_pupils"] if "gias_total_pupils" in source.columns else None
return rows.assign(total_pupils=whole)
# Input validation helpers
def _names_in_group(names: pd.Series, in_group) -> set:
"""The distinct names in a column that a group predicate accepts.
@@ -859,7 +872,7 @@ async def get_schools(
if c in df_latest.columns
]
# fact_performance guarantees one row per (urn, year); df_latest has one row per urn.
schools_df = df_latest[available_cols]
schools_df = _with_whole_school_pupils(df_latest[available_cols], df_latest)
# Location-based search (uses pre-geocoded data from database)
search_coords = None
@@ -1545,7 +1558,7 @@ async def get_place(request: Request, kind: str, slug: str,
# variants that exist rather than 404s.
"phases": [ph for ph in ("primary", "secondary")
if place.publishes_phase(ph)]},
"schools": clean_for_json(rows[cols]),
"schools": clean_for_json(_with_whole_school_pupils(rows[cols], rows)),
"averages": averages,
}
+67
View File
@@ -0,0 +1,67 @@
"""Cards, map popups and place rows label total_pupils "pupils".
fact_performance's total_pupils is the cohort a year's results were measured
on. For a secondary that is the GCSE year group alone: Burntwood showed 245 in
search against 1,462 on roll. The list and place payloads therefore carry the
register's whole-school count, and nothing when the register has none.
"""
import numpy as np
import pandas as pd
import pytest
from fastapi.testclient import TestClient
def _schools_df() -> pd.DataFrame:
base = {
"local_authority": "Essex", "school_type": "Academy converter",
"year": 202425, "ofsted_grade": 2.0, "ofsted_date": None,
"town": "Brentwood", "postcode": "CM13 1AA", "status": "Open",
"address": "1 Test Street", "latitude": 51.6, "longitude": 0.3,
"gender": "Mixed", "rwm_expected_pct": np.nan, "attainment_8_score": 50.0,
}
rows = [
# Secondary: results cohort 245, register 1,462.
{**base, "urn": 100001, "school_name": "Alpha High", "phase": "Secondary",
"total_pupils": 245, "gias_total_pupils": 1462},
# Register count missing: no count, never the cohort.
{**base, "urn": 100002, "school_name": "Beta High", "phase": "Secondary",
"total_pupils": 180, "gias_total_pupils": np.nan},
]
# Enough schools in one town for it to have a place page.
rows += [
{**base, "urn": 100010 + i, "school_name": f"Gamma High {i}", "phase": "Secondary",
"total_pupils": 200, "gias_total_pupils": 1000 + i}
for i in range(5)
]
return pd.DataFrame(rows)
@pytest.fixture()
def client(monkeypatch):
from backend import app as app_module
monkeypatch.setattr(app_module, "load_latest_school_data", _schools_df)
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
monkeypatch.setattr(app_module, "_place_registry", None)
return TestClient(app_module.app, raise_server_exceptions=False)
def _pupils(schools: list[dict]) -> dict[int, object]:
return {s["urn"]: s.get("total_pupils") for s in schools}
def test_the_list_carries_the_whole_school_count(client):
resp = client.get("/api/schools?page_size=50")
assert resp.status_code == 200, resp.text
pupils = _pupils(resp.json()["schools"])
assert pupils[100001] == 1462
assert pupils[100002] is None
def test_a_place_page_carries_the_whole_school_count(client):
resp = client.get("/api/places/town/brentwood")
assert resp.status_code == 200, resp.text
pupils = _pupils(resp.json()["schools"])
assert pupils[100001] == 1462
assert pupils[100002] is None
+110 -9
View File
@@ -53,7 +53,7 @@ async function settledScrollLeft(scroller: Locator): Promise<number> {
* whatever primaries the environment holds.
*/
async function twoPrimaryUrns(page: Page): Promise<[string, string]> {
const res = await page.request.get('/api/schools?search=primary&per_page=50');
const res = await page.request.get('/api/schools?search=primary&page_size=50');
expect(res.ok()).toBeTruthy();
const body = await res.json();
const urns: string[] = (body.schools ?? [])
@@ -66,7 +66,7 @@ async function twoPrimaryUrns(page: Page): Promise<[string, string]> {
}
async function twoSecondaryUrns(page: Page): Promise<[string, string]> {
const res = await page.request.get('/api/schools?search=school&per_page=100');
const res = await page.request.get('/api/schools?search=school&page_size=100');
expect(res.ok()).toBeTruthy();
const body = await res.json();
const urns: string[] = (body.schools ?? [])
@@ -355,6 +355,84 @@ test('school type groups and the faith filter narrow to what they name', async (
}
});
/*
* Search rows printed tags the register does not hold. Every non-selective
* secondary was "Selective" ("non-selective" contains "selective"), and a
* school with no religious character got "Faith priority" or a bare "None"
* chip, because only "Does not apply" was excluded. Data-invariant: each test
* picks its school from the API and reads only that school's row.
*/
async function rowTags(page: Page, school: { urn: number; school_name: string }) {
await searchByName(page, school.school_name);
const link = page.locator(`a[href^="/school/${school.urn}-"]`).first();
await expect(link).toBeVisible({ timeout: 15_000 });
return link.locator('xpath=ancestor::div[contains(@class, "__rowContent")][1]')
.locator('[class*="__line2"]');
}
test('a non-selective secondary is not tagged Selective in search', async ({ page }) => {
const res = await page.request.get(
'/api/schools?search=school&phase=secondary&admissions_policy=non-selective&page_size=1');
expect(res.ok()).toBeTruthy();
const [school] = (await res.json()).schools ?? [];
test.skip(!school, 'no non-selective secondary in this environment');
expect(school.admissions_policy, 'the admissions filter was ignored').toBe('Non-selective');
const tags = await rowTags(page, school);
await expect(tags).toBeVisible();
await expect(tags.getByText('Selective', { exact: true })).toHaveCount(0);
});
test('a school with no religious character carries no faith tag in search', async ({ page }) => {
const res = await page.request.get('/api/schools?search=school&faith=none&page_size=100');
expect(res.ok()).toBeTruthy();
// Not a selective school: the Selective tag would win and hide the bug.
const school = ((await res.json()).schools ?? []).find(
(s: { religious_denomination?: string; admissions_policy?: string }) =>
s.religious_denomination === 'None' && !/selective/i.test(s.admissions_policy ?? ''));
test.skip(!school, 'no school recorded with religious character "None" here');
const tags = await rowTags(page, school);
await expect(tags).toBeVisible();
await expect(tags.getByText('Faith priority', { exact: true })).toHaveCount(0);
await expect(tags.getByText('None', { exact: true })).toHaveCount(0);
});
test('search and the school page agree on how many pupils a secondary has', async ({ page }) => {
// Search showed the GCSE year group as "pupils": Burntwood had 245 in
// search and 1,462 on its page. Both now carry the register's count.
const res = await page.request.get('/api/schools?search=school&phase=secondary&page_size=20');
expect(res.ok()).toBeTruthy();
const school = ((await res.json()).schools ?? []).find(
(s: { total_pupils?: number | null }) => s.total_pupils != null);
test.skip(!school, 'no secondary with a pupil count in this environment');
const detail = await (await page.request.get(`/api/schools/${school.urn}`)).json();
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.
@@ -544,7 +622,7 @@ test('school with no performance data still gets a working detail page', async (
const candidates: number[] = [];
for (const q of ['post 16', 'specialist college', 'sixth form']) {
const resp = await page.request.get(
`/api/schools?search=${encodeURIComponent(q)}&per_page=20`
`/api/schools?search=${encodeURIComponent(q)}&page_size=20`
);
if (!resp.ok()) continue;
const body = await resp.json();
@@ -1123,7 +1201,7 @@ test('compare metric-help popover stays within the mobile viewport', async ({ pa
test('admissions year/trend toggle still switches views after the server/client split', async ({ page }) => {
// Find a school with at least two years carrying an offer rate — the toggle
// only appears then. Data-invariant: uses whatever the environment holds.
const res = await page.request.get('/api/schools?search=primary&per_page=50');
const res = await page.request.get('/api/schools?search=primary&page_size=50');
expect(res.ok()).toBeTruthy();
const candidates: number[] = ((await res.json()).schools ?? []).map((s: { urn: number }) => s.urn);
@@ -2083,7 +2161,7 @@ test('English schools with Welsh postcodes are kept', async ({ page }) => {
test('a Welsh school URL 404s while an English one still resolves', async ({ page }) => {
// Paired on purpose: the Welsh assertion alone would also pass if the whole
// site were down, which is the failure this test most needs to distinguish.
const english = await page.request.get('/api/schools?search=primary&per_page=1');
const english = await page.request.get('/api/schools?search=primary&page_size=1');
expect(english.ok()).toBeTruthy();
const [first] = (await english.json()).schools ?? [];
expect(first, 'no English school available to compare against').toBeTruthy();
@@ -2193,7 +2271,7 @@ test('a filtered homepage still canonicalises to the bare root', async ({ page }
});
test('a school page canonicalises to its own slug on the www host', async ({ page }) => {
const res = await page.request.get('/api/schools?search=primary&per_page=1');
const res = await page.request.get('/api/schools?search=primary&page_size=1');
expect(res.ok()).toBeTruthy();
const [first] = (await res.json()).schools ?? [];
expect(first, 'no school available').toBeTruthy();
@@ -2268,7 +2346,7 @@ function blocksEverything(robots: string, agent: string): boolean {
}
test('a school page on staging is noindexed too, not just the homepage', async ({ page }) => {
const list = await page.request.get('/api/schools?search=primary&per_page=1');
const list = await page.request.get('/api/schools?search=primary&page_size=1');
const [first] = (await list.json()).schools ?? [];
expect(first, 'no school available').toBeTruthy();
@@ -2879,7 +2957,7 @@ test('with autosuggest off, the search box is a plain input', async ({ page }) =
async function secondaryWithDestinations(page: Page): Promise<{
urn: string; destinations: any;
}> {
const res = await page.request.get('/api/schools?search=school&per_page=100');
const res = await page.request.get('/api/schools?search=school&page_size=100');
expect(res.ok()).toBeTruthy();
const body = await res.json();
const urns: string[] = (body.schools ?? [])
@@ -2975,7 +3053,7 @@ test('switching to disadvantaged pupils never reveals a withheld figure', async
});
test('a school with no sixth form has no post-16 destinations section', async ({ page }) => {
const res = await page.request.get('/api/schools?search=school&per_page=100');
const res = await page.request.get('/api/schools?search=school&page_size=100');
const body = await res.json();
const noSixthForm = (body.schools ?? [])
.filter((s: { phase?: string; has_sixth_form?: boolean }) =>
@@ -3286,3 +3364,26 @@ for (const width of [360, 390, 430]) {
expect(failing).toEqual([]);
});
}
/*
* The header's facts row read GIAS text as booleans: "Not applicable" put a
* "Nursery" chip on secondaries aged 11–18, and "Girls" became "Girls's
* school". Data-invariant: the chip follows whatever the API says.
*/
test('a girls\' secondary header names it properly and shows Nursery only when it has one', async ({ page }) => {
const res = await page.request.get('/api/schools?search=school&phase=secondary&gender=girls&page_size=1');
expect(res.ok()).toBeTruthy();
const [school] = (await res.json()).schools ?? [];
test.skip(!school, 'no girls\' secondary in this environment');
expect(school.gender, 'the gender filter was ignored').toBe('Girls');
const detail = await (await page.request.get(`/api/schools/${school.urn}`)).json();
// Without the field, the Nursery assertion below would pass vacuously.
expect(detail.school_info).toHaveProperty('nursery_provision');
await page.goto(`/school/${school.urn}`);
const header = page.locator('header', { has: page.getByRole('heading', { level: 1 }) });
await expect(header.getByText("Girls' school", { exact: true })).toBeVisible({ timeout: 15_000 });
await expect(header.getByText(/'s school/)).toHaveCount(0);
await expect(header.getByText('Nursery', { exact: true }))
.toHaveCount(detail.school_info.nursery_provision === 'Has Nursery Classes' ? 1 : 0);
});
@@ -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');
}
});
@@ -361,12 +361,12 @@ describe('PlaceView school attributes', () => {
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
rwm_expected_pct: 82, attainment_8_score: null,
age_range: '4-11', religious_denomination: 'Church of England',
nursery_provision: true,
nursery_provision: 'Has Nursery Classes',
parliamentary_constituency: 'Chelmsford' } as never,
{ urn: 2, school_name: 'Beta High', phase: 'Secondary',
rwm_expected_pct: null, attainment_8_score: 47,
age_range: '11-16', religious_denomination: 'Does not apply',
nursery_provision: false,
nursery_provision: 'No Nursery Classes',
parliamentary_constituency: 'Witham' } as never,
],
averages: { rwm_expected_pct: 63, attainment_8_score: 45 },
@@ -459,6 +459,18 @@ describe('PlaceView school attributes', () => {
expect(cells.slice(2)).toEqual(['—', '—', '—', '—']);
});
it('reads "Not applicable" as no nursery, not as a yes', () => {
// GIAS sends text. Tested for truthiness, every value was a "Yes".
const notApplicable: PlaceDetail = {
...withAttributes,
schools: [{ urn: 5, school_name: 'Epsilon Primary', phase: 'Primary',
rwm_expected_pct: 70, nursery_provision: 'Not applicable' } as never],
};
const { container } = render(<PlaceView detail={notApplicable} phase="primary"
englandAverage={61} neighbours={[]} />);
expect(container.querySelector('tbody')!.textContent).not.toContain('Yes');
});
it('gives an all-through school its nursery under primary only', () => {
// All-through schools render in both groups. Nursery belongs to the
// primary reading of the same school, not the secondary one.
@@ -467,7 +479,7 @@ describe('PlaceView school attributes', () => {
schools: [{ urn: 4, school_name: 'Delta Academy', phase: 'All-through',
rwm_expected_pct: 66, attainment_8_score: 51,
age_range: '4-18', religious_denomination: 'None',
nursery_provision: true,
nursery_provision: 'Has Nursery Classes',
parliamentary_constituency: 'Chelmsford' } as never],
};
const { container } = render(<PlaceView detail={allThrough}
@@ -0,0 +1,36 @@
/**
* SchoolRow (primary search results): line 2 prints the religious character
* only when the school has one. The register's "None" was printed as a chip.
*/
import '@testing-library/jest-dom';
import { render, screen } from '@testing-library/react';
import { SchoolRow } from '@/components/SchoolRow';
import type { School } from '@/lib/types';
const base = {
urn: 100001,
school_name: 'Alpha Primary School',
local_authority: 'Testshire',
school_type: 'Free schools',
phase: 'Primary',
gender: 'Mixed',
age_range: '4-11',
rwm_expected_pct: 70,
} as unknown as School;
describe('SchoolRow religious character', () => {
it.each(['None', 'Does not apply', ''])(
'prints nothing when the register says %p',
(religious_denomination) => {
render(<SchoolRow school={{ ...base, religious_denomination }} />);
expect(screen.queryByText('None')).not.toBeInTheDocument();
expect(screen.queryByText('Does not apply')).not.toBeInTheDocument();
},
);
it('prints a religious character the school has', () => {
render(<SchoolRow school={{ ...base, religious_denomination: 'Church of England' }} />);
expect(screen.getByText('Church of England')).toBeInTheDocument();
});
});
@@ -65,3 +65,38 @@ describe('SecondarySchoolRow proposed-to-close tag', () => {
expect(screen.queryByText(/Proposed to close/)).not.toBeInTheDocument();
});
});
describe('SecondarySchoolRow admissions tag', () => {
it('tags a selective school', () => {
render(<SecondarySchoolRow school={{ ...base, admissions_policy: 'Selective' }} />);
expect(screen.getByText('Selective')).toBeInTheDocument();
});
it('does not tag a non-selective school as selective', () => {
// "Non-selective" contains "selective": a substring test tagged every
// comprehensive (Burntwood, Graveney) as Selective.
render(<SecondarySchoolRow school={{ ...base, admissions_policy: 'Non-selective' }} />);
expect(screen.queryByText('Selective')).not.toBeInTheDocument();
});
it.each(['None', 'Does not apply', '', null])(
'gives no faith tag when the religious character is %p',
(religious_denomination) => {
render(
<SecondarySchoolRow
school={{ ...base, admissions_policy: 'Not applicable', religious_denomination }}
/>,
);
expect(screen.queryByText('Faith priority')).not.toBeInTheDocument();
},
);
it('tags a school with a religious character', () => {
render(
<SecondarySchoolRow
school={{ ...base, admissions_policy: 'Not applicable', religious_denomination: 'Church of England' }}
/>,
);
expect(screen.getByText('Faith priority')).toBeInTheDocument();
});
});
@@ -0,0 +1,67 @@
/**
* The facts row under the school name.
*
* nursery_provision is GIAS text, not a boolean: "Has Nursery Classes",
* "No Nursery Classes" or "Not applicable". Tested for truthiness, every one
* of those read as a nursery, so secondaries aged 11–18 showed "Nursery".
*/
import { screen } from '@testing-library/react';
import type { School } from '@/lib/types';
import { primaryFixture, secondaryFixture } from '../support/schoolFixtures';
import { renderSchoolDetail, 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/SatsChart', () => ({
__esModule: true,
default: () => <div data-testid="sats-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 withSchool<T extends { schoolInfo: School }>(fixture: T, info: Partial<School>): T {
return { ...fixture, schoolInfo: { ...fixture.schoolInfo, ...info } };
}
describe('school header nursery chip', () => {
it('shows Nursery when GIAS says the school has nursery classes', () => {
renderSchoolDetail(withSchool(primaryFixture, { nursery_provision: 'Has Nursery Classes' }));
expect(screen.getByText('Nursery', { selector: 'span' })).toBeInTheDocument();
});
it.each(['No Nursery Classes', 'Not applicable', null])(
'hides Nursery when GIAS says %p',
(value) => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, { nursery_provision: value }));
expect(screen.queryByText('Nursery', { selector: 'span' })).not.toBeInTheDocument();
},
);
});
describe('school header single-sex chip', () => {
it.each([['Girls', "Girls' school"], ['Boys', "Boys' school"]])(
'labels a %s school with a plural possessive',
(gender, label) => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, { gender }));
expect(screen.getByText(label)).toBeInTheDocument();
expect(screen.queryByText(/'s school/)).not.toBeInTheDocument();
},
);
it('says nothing for a mixed school', () => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, { gender: 'Mixed' }));
expect(screen.queryByText(/^(Girls|Boys|Mixed)'s? school$/)).not.toBeInTheDocument();
});
});
@@ -0,0 +1,56 @@
/**
* "Pupils" on a school page is the size of the school.
*
* A year's results row carries the cohort its figures were measured on. For a
* secondary that is the GCSE year group alone (Burntwood: 245, against 1,462
* on roll), so it must never stand in for the whole-school count.
*/
import { screen } from '@testing-library/react';
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/SatsChart', () => ({
__esModule: true,
default: () => <div data-testid="sats-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 withoutCensus(schoolTotal: number | null) {
const yearlyData = secondaryFixture.yearlyData.map((r) => ({ ...r, total_pupils: 245 }));
return {
...secondaryFixture,
census: null,
yearlyData,
schoolInfo: { ...secondaryFixture.schoolInfo, total_pupils: schoolTotal },
};
}
describe('pupil count without a census record', () => {
it('uses the register count in the header, not the results cohort', () => {
renderSecondarySchoolDetail(withoutCensus(1462));
const pupils = screen.getByText('Pupils:').parentElement!;
expect(pupils).toHaveTextContent('1,462');
expect(pupils).not.toHaveTextContent('245');
});
it('shows no count rather than the results cohort when the register has none', () => {
renderSecondarySchoolDetail(withoutCensus(null));
expect(screen.queryByText('Pupils:')).not.toBeInTheDocument();
expect(screen.queryByText('Total pupils')).not.toBeInTheDocument();
});
});
+31
View File
@@ -15,6 +15,8 @@ import {
computeYBounds,
formatAgeRange,
formatAgeSpan,
hasNurseryClasses,
singleSexLabel,
} from '@/lib/utils';
describe('formatPercentage', () => {
@@ -346,3 +348,32 @@ describe('formatAgeRange', () => {
expect(formatAgeRange('4-11')).toBe('Ages 4–11');
});
});
describe('hasNurseryClasses', () => {
it('is true only for the GIAS value that means it', () => {
// GIAS sends text, and two of its three values mean no nursery.
expect(hasNurseryClasses('Has Nursery Classes')).toBe(true);
expect(hasNurseryClasses('No Nursery Classes')).toBe(false);
expect(hasNurseryClasses('Not applicable')).toBe(false);
expect(hasNurseryClasses(null)).toBe(false);
expect(hasNurseryClasses(undefined)).toBe(false);
});
});
describe('singleSexLabel', () => {
it('uses the plural possessive GIAS values need', () => {
expect(singleSexLabel('Girls')).toBe("Girls' school");
expect(singleSexLabel('Boys')).toBe("Boys' school");
});
it('ignores case, as hasNurseryClasses does', () => {
expect(singleSexLabel(' girls ')).toBe("Girls' school");
expect(singleSexLabel('BOYS')).toBe("Boys' school");
});
it('returns null for a mixed or unknown school', () => {
expect(singleSexLabel('Mixed')).toBeNull();
expect(singleSexLabel(null)).toBeNull();
expect(singleSexLabel(undefined)).toBeNull();
});
});
@@ -39,7 +39,6 @@ export function renderSchoolDetail(fixture: any) {
withProviders(
<SchoolDetailShell
schoolInfo={fixture.schoolInfo}
yearlyData={fixture.yearlyData}
census={fixture.census}
navItems={navItems}
>
@@ -67,7 +66,6 @@ export function renderSecondarySchoolDetail(fixture: any) {
withProviders(
<SchoolDetailShell
schoolInfo={fixture.schoolInfo}
yearlyData={fixture.yearlyData}
census={fixture.census}
navItems={navItems}
>
@@ -249,7 +249,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
{isSecondary ? (
<SchoolDetailShell
schoolInfo={school_info}
yearlyData={yearly_data}
census={census ?? null}
navItems={secondaryNavItems}
>
@@ -273,7 +272,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
) : (
<SchoolDetailShell
schoolInfo={school_info}
yearlyData={yearly_data}
census={census ?? null}
navItems={primaryNavItems}
>
+6 -2
View File
@@ -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]);
+2 -4
View File
@@ -9,7 +9,7 @@
*/
import type { School } from '@/lib/types';
import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
import styles from './SchoolRow.module.css';
interface SchoolRowProps {
@@ -34,9 +34,7 @@ export function SchoolRow({
const ofstedBadge = buildOfstedListBadge(school);
const showGender = school.gender && school.gender.toLowerCase() !== 'mixed';
const showDenomination =
school.religious_denomination &&
school.religious_denomination !== 'Does not apply';
const showDenomination = hasReligiousCharacter(school.religious_denomination);
// The school's OWN figure and its year-over-year trend are same-school
// measures — shown whenever there's a real value (not the all-zero
+5 -5
View File
@@ -11,14 +11,14 @@
'use client';
import type { School } from '@/lib/types';
import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, isProposedToClose, isSpecialSchool } from '@/lib/utils';
import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool } from '@/lib/utils';
import styles from './SecondarySchoolRow.module.css';
function detectAdmissionsTag(school: School): string | null {
const policy = school.admissions_policy?.toLowerCase() ?? '';
if (policy.includes('selective')) return 'Selective';
const denom = school.religious_denomination ?? '';
if (denom && denom !== 'Does not apply') return 'Faith priority';
// Exact match: "Non-selective" contains "selective", so a substring test
// tagged every comprehensive as Selective.
if (school.admissions_policy?.trim().toLowerCase() === 'selective') return 'Selective';
if (hasReligiousCharacter(school.religious_denomination)) return 'Faith priority';
return null;
}
+5 -4
View File
@@ -13,7 +13,7 @@ import Link from 'next/link';
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
import { placeUrl, authoritySlug } from '@/lib/places';
import type { School } from '@/lib/types';
import { schoolUrl, formatAgeSpan } from '@/lib/utils';
import { schoolUrl, formatAgeSpan, hasNurseryClasses } from '@/lib/utils';
import { absoluteUrl } from '@/lib/site';
import { TrackPlaceView } from './TrackPlaceView';
import styles from './PlaceView.module.css';
@@ -131,9 +131,10 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
{showNursery && (
<td className={styles.attr}>
{/* Undefined is a mart the pipeline has not rebuilt, and
false is a school without one. Neither is a "Yes", and
neither is worth two different words. */}
{s.nursery_provision ? 'Yes' : NO_VALUE}
"No Nursery Classes" or "Not applicable" is a school
without one. None is a "Yes", and none is worth a
different word. */}
{hasNurseryClasses(s.nursery_provision) ? 'Yes' : NO_VALUE}
</td>
)}
<td className={styles.attrWide}>
@@ -21,7 +21,7 @@ import { useRouter } from 'next/navigation';
import { useComparison } from '@/hooks/useComparison';
import { SchoolHeroMap, type SchoolHeroMapHandle } from '../SchoolHeroMap';
import type { School, SchoolResult, SchoolCensus } from '@/lib/types';
import { formatAgeRange, isProposedToClose } from '@/lib/utils';
import { formatAgeRange, hasNurseryClasses, isProposedToClose, singleSexLabel } from '@/lib/utils';
import type { NavItem } from '@/lib/schoolSections';
import { track, getNavigationSource } from '@/lib/analytics';
import styles from './SchoolDetailShell.module.css';
@@ -34,8 +34,6 @@ import styles from './SchoolDetailShell.module.css';
*/
export interface SchoolDetailShellProps {
schoolInfo: School;
/** Only for the header's pupil-count fallback. */
yearlyData: SchoolResult[];
census: SchoolCensus | null;
/** Section list for the sticky nav, computed on the server. */
navItems: NavItem[];
@@ -44,7 +42,7 @@ export interface SchoolDetailShellProps {
}
export function SchoolDetailShell({
schoolInfo, yearlyData, census, navItems, children,
schoolInfo, census, navItems, children,
}: SchoolDetailShellProps) {
const router = useRouter();
const { addSchool, removeSchool, isSelected } = useComparison();
@@ -122,14 +120,14 @@ export function SchoolDetailShell({
return () => window.removeEventListener('keydown', onKey);
}, [sectionsOpen]);
// The chrome needs only these four. The section-shape flags are computed
// The chrome needs only these few. The section-shape flags are computed
// once on the server (lib/schoolSections) and consumed by the section
// composers; recomputing them here would duplicate that work for values
// this component never renders.
const latestResults = yearlyData.length > 0 ? yearlyData[yearlyData.length - 1] : null;
const phase = schoolInfo.phase ?? '';
const isAllThrough = phase.toLowerCase() === 'all-through';
const hasLocation = schoolInfo.latitude != null && schoolInfo.longitude != null;
const singleSex = singleSexLabel(schoolInfo.gender);
const handleComparisonToggle = () => {
if (isInComparison) {
@@ -214,13 +212,11 @@ export function SchoolDetailShell({
{isAllThrough && (
<span className={styles.metaItem}>All-through (primary &amp; secondary)</span>
)}
{schoolInfo.gender && schoolInfo.gender !== 'Mixed' && (
<span className={styles.metaItem}>{schoolInfo.gender}&apos;s school</span>
)}
{singleSex && <span className={styles.metaItem}>{singleSex}</span>}
{schoolInfo.age_range && (
<span className={styles.metaItem}>{formatAgeRange(schoolInfo.age_range)}</span>
)}
{schoolInfo.nursery_provision && (
{hasNurseryClasses(schoolInfo.nursery_provision) && (
<span className={styles.metaItem}>Nursery</span>
)}
{schoolInfo.has_sixth_form && (
@@ -283,7 +279,9 @@ export function SchoolDetailShell({
</span>
)}
{(() => {
const total = census?.total_pupils ?? latestResults?.total_pupils ?? null;
// Never latestResults.total_pupils: that is the results
// cohort, which for a secondary is the GCSE year group alone.
const total = census?.total_pupils ?? schoolInfo.total_pupils ?? null;
if (total == null) return null;
return (
<span className={styles.headerDetail}>
@@ -54,7 +54,8 @@ export function WellbeingSection({
</div>
)}
{(() => {
const total = census?.total_pupils ?? schoolInfo.total_pupils ?? latestResults?.total_pupils ?? null;
// Not latestResults.total_pupils: that is the GCSE year group.
const total = census?.total_pupils ?? schoolInfo.total_pupils ?? null;
if (total == null) return null;
const female = census?.female_pupils ?? null;
const male = census?.male_pupils ?? null;
+2 -1
View File
@@ -20,7 +20,8 @@ export interface School {
religious_denomination: string | null;
age_range: string | null;
has_sixth_form?: boolean | null;
nursery_provision?: boolean | null;
/** GIAS text; read it through hasNurseryClasses(). */
nursery_provision?: string | null;
status?: string | null; // GIAS establishment status ("Open" / "Open, but proposed to close")
// Address
+29
View File
@@ -99,6 +99,26 @@ export function formatAgeRange(ageRange: string | null | undefined): string {
return /^\d+–\d+$/.test(span) ? `Ages ${span}` : span;
}
/**
* GIAS NurseryProvision is text: "Has Nursery Classes", "No Nursery Classes"
* or "Not applicable". Only the first means a nursery, so never test the raw
* value for truthiness.
*/
export function hasNurseryClasses(value: string | null | undefined): boolean {
return value?.trim().toLowerCase() === 'has nursery classes';
}
/**
* "Girls' school" / "Boys' school" for a single-sex school, null otherwise.
* GIAS genders are plural, so the possessive is a bare apostrophe.
*/
export function singleSexLabel(gender: string | null | undefined): string | null {
const g = gender?.trim().toLowerCase();
if (g === 'girls') return "Girls' school";
if (g === 'boys') return "Boys' school";
return null;
}
// ============================================================================
// Number Formatting
// ============================================================================
@@ -902,6 +922,15 @@ export function isSpecialSchool(school: { school_type?: string | null }): boolea
return /\bspecial\b/.test(t) || /pupil referral/.test(t) || /alternative provision/.test(t);
}
/**
* 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.
*/
export function hasReligiousCharacter(value: string | null | undefined): boolean {
const v = value?.trim().toLowerCase() ?? '';
return v !== '' && v !== 'none' && v !== 'does not apply';
}
/**
* The school's combined Reading, Writing & Maths figure, or null when there is
* no real one to show.