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
19 changed files with 190 additions and 596 deletions

No files matched your search

+7 -20
View File
@@ -634,18 +634,6 @@ def _with_whole_school_pupils(rows: pd.DataFrame, source: pd.DataFrame) -> pd.Da
return rows.assign(total_pupils=whole)
def _with_type_group(rows: pd.DataFrame) -> pd.DataFrame:
"""Name each row's search-filter type group, or None for a type in no group.
The school page and the search rows print it ("State school") in place of
the GIAS establishment type, and an independent school gets a Fee-paying
flag from it.
"""
if "school_type" not in rows.columns:
return rows
return rows.assign(type_group=rows["school_type"].map(type_group_for))
# Input validation helpers
def _names_in_group(names: pd.Series, in_group) -> set:
"""The distinct names in a column that a group predicate accepts.
@@ -977,7 +965,7 @@ async def get_schools(
total = len(schools_df)
start_idx = (page - 1) * page_size
end_idx = start_idx + page_size
schools_df = _with_type_group(schools_df.iloc[start_idx:end_idx])
schools_df = schools_df.iloc[start_idx:end_idx]
return {
"schools": clean_for_json(schools_df),
@@ -1041,7 +1029,6 @@ async def get_school_details(request: Request, urn: int):
"school_name": latest.get("school_name", ""),
"local_authority": latest.get("local_authority", ""),
"school_type": latest.get("school_type", ""),
"type_group": type_group_for(latest.get("school_type")),
"address": latest.get("address", ""),
"religious_denomination": latest.get("religious_denomination", ""),
"age_range": latest.get("age_range", ""),
@@ -1535,12 +1522,13 @@ async def get_place(request: Request, kind: str, slug: str,
# warning. Ordered de-duplication keeps the column order and the warning
# cannot come back.
#
# parliamentary_constituency is not in SCHOOL_COLUMNS and the place table
# shows it. The `in rows.columns` guard is what keeps a mart the pipeline
# has not rebuilt working: it and nursery_provision are the optional GIAS
# columns data_loader degrades to NULL.
# nursery_provision and parliamentary_constituency are not in
# SCHOOL_COLUMNS and the place table shows both. The `in rows.columns`
# guard is what keeps a mart the pipeline has not rebuilt working: those
# two are the optional GIAS columns data_loader degrades to NULL.
cols = [c for c in dict.fromkeys(
SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
"nursery_provision",
"parliamentary_constituency",
"rwm_expected_pct", "attainment_8_score",
"total_pupils"])
@@ -1570,8 +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(
_with_type_group(_with_whole_school_pupils(rows[cols], rows))),
"schools": clean_for_json(_with_whole_school_pupils(rows[cols], rows)),
"averages": averages,
}
-1
View File
@@ -569,7 +569,6 @@ SCHOOL_COLUMNS = [
"religious_denomination",
"age_range",
"has_sixth_form",
"nursery_provision",
"status",
"gender",
"admissions_policy",
-81
View File
@@ -1,81 +0,0 @@
"""Payloads name each school's type group.
The school page and the search rows say "State school" or "Independent
school" in the search filter's own terms, not GIAS's 34 establishment types,
and an independent school gets a Fee-paying flag. A type in no group keeps a
null group, and the page prints the register's own name for it. Search rows
also carry nursery_provision, for their "Nursery class" flag.
"""
import numpy as np
import pandas as pd
import pytest
from fastapi.testclient import TestClient
def _schools_df() -> pd.DataFrame:
base = {
"local_authority": "Essex", "phase": "Primary", "year": 202425,
"ofsted_grade": 2.0, "ofsted_date": None, "attainment_8_score": np.nan,
"town": "Brentwood", "postcode": "CM13 1AA", "status": "Open",
"address": "1 Test Street", "latitude": 51.6, "longitude": 0.3,
"rwm_expected_pct": 60.0, "nursery_provision": "No Nursery Classes",
}
rows = [
{**base, "urn": 100001, "school_name": "Alpha Academy",
"school_type": "Academy converter", "nursery_provision": "Has Nursery Classes"},
{**base, "urn": 100002, "school_name": "Beta Prep",
"school_type": "Other independent school"},
{**base, "urn": 100003, "school_name": "Gamma Unit",
"school_type": "Secure units"},
]
# Enough schools in one town for it to have a place page.
rows += [
{**base, "urn": 100010 + i, "school_name": f"Delta Primary {i}",
"school_type": "Community school"}
for i in range(3)
]
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, "get_supplementary_data", lambda db, urn: {})
monkeypatch.setattr(app_module, "_place_registry", None)
return TestClient(app_module.app, raise_server_exceptions=False)
def _by_urn(schools: list[dict]) -> dict[int, dict]:
return {s["urn"]: s for s in schools}
def test_the_list_names_each_type_group(client):
resp = client.get("/api/schools?page_size=50")
assert resp.status_code == 200, resp.text
schools = _by_urn(resp.json()["schools"])
assert schools[100001]["type_group"] == "state"
assert schools[100002]["type_group"] == "independent"
assert schools[100003]["type_group"] is None
def test_the_list_carries_nursery_provision(client):
schools = _by_urn(client.get("/api/schools?page_size=50").json()["schools"])
assert schools[100001]["nursery_provision"] == "Has Nursery Classes"
def test_the_school_page_names_its_type_group(client):
resp = client.get("/api/schools/100002")
assert resp.status_code == 200, resp.text
assert resp.json()["school_info"]["type_group"] == "independent"
def test_a_place_page_names_each_type_group(client):
resp = client.get("/api/places/town/brentwood")
assert resp.status_code == 200, resp.text
schools = _by_urn(resp.json()["schools"])
assert schools[100001]["type_group"] == "state"
assert schools[100003]["type_group"] is None
+27 -36
View File
@@ -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.
@@ -487,10 +510,9 @@ test('school detail page shows GIAS identity/contact details and drops the unwir
if (info.telephone) {
await expect(page.locator('a[href^="tel:"]').first()).toBeVisible();
}
// Constituency and county left the header in the facts-and-flags redesign:
// neither helps a parent decide. The place pages keep both.
await expect(page.getByText('Constituency:')).toHaveCount(0);
await expect(page.getByText('County:')).toHaveCount(0);
if (info.parliamentary_constituency) {
await expect(page.getByText('Constituency:').first()).toBeVisible();
}
});
test('header details collapse behind a "Show all details" toggle on mobile', async ({ page }) => {
@@ -3362,37 +3384,6 @@ test('a girls\' secondary header names it properly and shows Nursery only when i
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 class', { exact: true }))
await expect(header.getByText('Nursery', { exact: true }))
.toHaveCount(detail.school_info.nursery_provision === 'Has Nursery Classes' ? 1 : 0);
});
/*
* The header states facts in fixed slots and flags only what applies: a
* selective school is flagged Selective, and its type reads in the search
* filter's words, not as a GIAS establishment type. The search row carries
* the same flag. Data-invariant: the school comes from the API.
*/
const TYPE_GROUP_LABELS: Record<string, string> = {
state: 'State school', independent: 'Independent school', special: 'Special school (SEND)',
post16: 'Sixth form or college', alternative: 'Alternative provision',
};
test('a selective school is flagged Selective, on its page and in search', async ({ page }) => {
const res = await page.request.get(
'/api/schools?search=school&phase=secondary&admissions_policy=selective&page_size=1');
expect(res.ok()).toBeTruthy();
const [school] = (await res.json()).schools ?? [];
test.skip(!school, 'no selective secondary in this environment');
expect(school.admissions_policy, 'the admissions filter was ignored').toBe('Selective');
expect(school, 'the list must name the type group').toHaveProperty('type_group');
await page.goto(`/school/${school.urn}`);
const header = page.locator('header', { has: page.getByRole('heading', { level: 1 }) });
const flags = header.getByRole('list', { name: 'Admission and provision' });
await expect(flags.getByText('Selective', { exact: true })).toBeVisible({ timeout: 15_000 });
const typeLabel = TYPE_GROUP_LABELS[school.type_group] ?? school.school_type;
await expect(header.getByText(typeLabel, { exact: true })).toBeVisible();
const tags = await rowTags(page, school);
await expect(tags.getByText('Selective', { exact: true })).toBeVisible();
});
@@ -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');
}
});
@@ -34,21 +34,3 @@ describe('SchoolRow religious character', () => {
expect(screen.getByText('Church of England')).toBeInTheDocument();
});
});
describe('SchoolRow shares the school page flags', () => {
it("prints the type in the search filter's terms", () => {
render(<SchoolRow school={{ ...base, type_group: 'state' }} />);
expect(screen.getByText('State school')).toBeInTheDocument();
expect(screen.queryByText('Free schools')).not.toBeInTheDocument();
});
it('flags a nursery class', () => {
render(<SchoolRow school={{ ...base, nursery_provision: 'Has Nursery Classes' }} />);
expect(screen.getByText('Nursery class')).toBeInTheDocument();
});
it("flags a boys' school", () => {
render(<SchoolRow school={{ ...base, gender: 'Boys' }} />);
expect(screen.getByText("Boys' school")).toBeInTheDocument();
});
});
@@ -79,7 +79,7 @@ describe('SecondarySchoolRow admissions tag', () => {
expect(screen.queryByText('Selective')).not.toBeInTheDocument();
});
it.each(['None', 'Does not apply'])(
it.each(['None', 'Does not apply', '', null])(
'gives no faith tag when the religious character is %p',
(religious_denomination) => {
render(
@@ -87,32 +87,16 @@ describe('SecondarySchoolRow admissions tag', () => {
school={{ ...base, admissions_policy: 'Not applicable', religious_denomination }}
/>,
);
expect(screen.queryByText(religious_denomination)).not.toBeInTheDocument();
expect(screen.queryByText('Faith priority')).not.toBeInTheDocument();
},
);
it('tags the religious character the register records, not "Faith priority"', () => {
it('tags a school with a religious character', () => {
render(
<SecondarySchoolRow
school={{ ...base, admissions_policy: 'Not applicable', religious_denomination: 'Church of England' }}
/>,
);
expect(screen.getByText('Church of England')).toBeInTheDocument();
expect(screen.queryByText('Faith priority')).not.toBeInTheDocument();
});
});
describe('SecondarySchoolRow shares the school page flags', () => {
it("prints the type in the search filter's terms", () => {
render(<SecondarySchoolRow school={{ ...base, type_group: 'state' }} />);
expect(screen.getByText('State school')).toBeInTheDocument();
expect(screen.queryByText('Academy')).not.toBeInTheDocument();
});
it("flags a girls' school and fees", () => {
render(<SecondarySchoolRow school={{ ...base, type_group: 'independent', gender: 'Girls' }} />);
expect(screen.getByText("Girls' school")).toBeInTheDocument();
expect(screen.getByText('Fee-paying')).toBeInTheDocument();
expect(screen.getByText('Faith priority')).toBeInTheDocument();
});
});
@@ -1,13 +1,12 @@
/**
* The school header: one fact line (phase, ages, type, pupils), then flags
* only for what applies, then the address and the details.
* 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, within } from '@testing-library/react';
import { screen } from '@testing-library/react';
import type { School } from '@/lib/types';
import { primaryFixture, secondaryFixture } from '../support/schoolFixtures';
import { renderSchoolDetail, renderSecondarySchoolDetail } from '../support/renderSchoolDetail';
@@ -36,54 +35,22 @@ function withSchool<T extends { schoolInfo: School }>(fixture: T, info: Partial<
return { ...fixture, schoolInfo: { ...fixture.schoolInfo, ...info } };
}
const flagList = () => screen.queryByRole('list', { name: 'Admission and provision' });
const flagLabels = () => within(flagList()!).getAllByRole('listitem').map((li) => li.textContent);
describe('school header fact line', () => {
it('states phase, ages, type and pupils, in that order', () => {
renderSchoolDetail(withSchool(primaryFixture, { type_group: 'state' }));
const line = screen.getByText('Ages 4–11').parentElement!;
expect(line).toHaveTextContent(/^PrimaryAges 4–11State school420 pupils$/);
});
it("prints the register's type name for a type in no group", () => {
renderSchoolDetail(withSchool(primaryFixture, { type_group: null, school_type: 'Secure units' }));
expect(screen.getByText('Secure units')).toBeInTheDocument();
});
it('no longer prints the GIAS establishment type', () => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, { type_group: 'state' }));
expect(screen.queryByText('Academy converter')).not.toBeInTheDocument();
});
});
describe('school header flags', () => {
it('shows no flag list for a school with nothing to flag', () => {
renderSchoolDetail(withSchool(primaryFixture, { type_group: 'state' }));
expect(flagList()).not.toBeInTheDocument();
});
it('lists who-can-apply flags, then what the school offers', () => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, {
type_group: 'independent', admissions_policy: 'Selective', gender: 'Girls',
religious_denomination: 'Church of England', has_sixth_form: true,
}));
expect(flagLabels()).toEqual(['Fee-paying', 'Selective', "Girls' school", 'Church of England', 'Sixth form']);
});
it('shows Nursery class when GIAS says the school has nursery classes', () => {
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(flagLabels()).toEqual(['Nursery class']);
expect(screen.getByText('Nursery', { selector: 'span' })).toBeInTheDocument();
});
it.each(['No Nursery Classes', 'Not applicable', null])(
'shows no nursery flag when GIAS says %p',
'hides Nursery when GIAS says %p',
(value) => {
renderSecondarySchoolDetail(withSchool(secondaryFixture, { nursery_provision: value }));
expect(screen.queryByText(/^Nursery/)).not.toBeInTheDocument();
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) => {
@@ -98,44 +65,3 @@ describe('school header flags', () => {
expect(screen.queryByText(/^(Girls|Boys|Mixed)'s? school$/)).not.toBeInTheDocument();
});
});
describe('school header address', () => {
it('names the council after the postcode', () => {
renderSchoolDetail(primaryFixture);
expect(screen.getByText(/TE1 1ST · Westshire/)).toBeInTheDocument();
});
it('leaves the council out when the address already names it', () => {
renderSchoolDetail(withSchool(primaryFixture, { local_authority: 'Testville' }));
expect(screen.queryByText(/· Testville/)).not.toBeInTheDocument();
});
});
describe('school header details', () => {
const detailed = {
headteacher_name: 'Mrs A Head', capacity: 426, county: 'Surrey',
parliamentary_constituency: 'Putney', religious_denomination: 'Church of England',
};
it('drops county, constituency and religious character', () => {
renderSchoolDetail(withSchool(primaryFixture, detailed));
expect(screen.queryByText('County:')).not.toBeInTheDocument();
expect(screen.queryByText('Constituency:')).not.toBeInTheDocument();
expect(screen.queryByText('Religious character:')).not.toBeInTheDocument();
});
it('shows the capacity', () => {
renderSchoolDetail(withSchool(primaryFixture, detailed));
expect(screen.getByText('Capacity:').parentElement).toHaveTextContent('Capacity: 426');
});
it('names an academy trust', () => {
renderSchoolDetail(withSchool(primaryFixture, { trust_name: 'BURNTWOOD TRUST' }));
expect(screen.getByText('Academy trust:').parentElement).toHaveTextContent('BURNTWOOD TRUST');
});
it("hides a trust that has the school's own name", () => {
renderSchoolDetail(withSchool(primaryFixture, { trust_name: 'TEST PRIMARY SCHOOL' }));
expect(screen.queryByText('Academy trust:')).not.toBeInTheDocument();
});
});
@@ -43,13 +43,14 @@ function withoutCensus(schoolTotal: number | null) {
describe('pupil count without a census record', () => {
it('uses the register count in the header, not the results cohort', () => {
renderSecondarySchoolDetail(withoutCensus(1462));
expect(screen.getByText('1,462 pupils')).toBeInTheDocument();
expect(screen.queryByText('245 pupils')).not.toBeInTheDocument();
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(/^[\d,]+ pupils$/)).not.toBeInTheDocument();
expect(screen.queryByText('Pupils:')).not.toBeInTheDocument();
expect(screen.queryByText('Total pupils')).not.toBeInTheDocument();
});
});
@@ -1,72 +0,0 @@
/**
* The facts the school header and the search rows print: a type in the search
* filter's terms, and flags only for what applies.
*/
import { schoolFlags, schoolTypeLabel } from '@/lib/schoolFacts';
import type { School } from '@/lib/types';
const school = (over: Partial<School>): School =>
({ urn: 1, school_name: 'Test School', ...over }) as School;
const labels = (over: Partial<School>) => schoolFlags(school(over)).map((f) => f.label);
describe('schoolTypeLabel', () => {
it.each([
['state', 'State school'],
['independent', 'Independent school'],
['special', 'Special school (SEND)'],
['post16', 'Sixth form or college'],
['alternative', 'Alternative provision'],
])("names the %s group in the search filter's terms", (type_group, label) => {
expect(schoolTypeLabel(school({ type_group, school_type: 'Academy converter' }))).toBe(label);
});
it("prints the register's own name for a type in no group", () => {
expect(schoolTypeLabel(school({ type_group: null, school_type: 'Secure units' }))).toBe('Secure units');
});
it('returns null when there is no type at all', () => {
expect(schoolTypeLabel(school({}))).toBeNull();
});
});
describe('schoolFlags', () => {
it('flags nothing for a mixed, non-faith, non-selective state school', () => {
expect(labels({
type_group: 'state', gender: 'Mixed', admissions_policy: 'Non-selective',
religious_denomination: 'None', nursery_provision: 'No Nursery Classes',
has_sixth_form: false, phase: 'Secondary',
})).toEqual([]);
});
it('lists who-can-apply flags first, then what the school offers, in a fixed order', () => {
expect(labels({
type_group: 'independent', admissions_policy: 'Selective', gender: 'Boys',
religious_denomination: 'Christian', nursery_provision: 'Has Nursery Classes',
has_sixth_form: true, phase: 'All-through',
})).toEqual(['Fee-paying', 'Selective', "Boys' school", 'Christian', 'Nursery class', 'Sixth form']);
});
it('marks who-can-apply flags as conditions and offers as provision', () => {
const kinds = Object.fromEntries(
schoolFlags(school({ admissions_policy: 'Selective', has_sixth_form: true, phase: 'Secondary' }))
.map((f) => [f.label, f.kind]),
);
expect(kinds).toEqual({ Selective: 'condition', 'Sixth form': 'provision' });
});
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([]);
});
it('prints the religious character as the register records it', () => {
expect(labels({ religious_denomination: 'Church of England/Methodist' })).toEqual(['Church of England/Methodist']);
});
it('does not flag a nursery class on a nursery school, or a sixth form on a post-16 one', () => {
expect(labels({ phase: 'Nursery', nursery_provision: 'Has Nursery Classes' })).toEqual([]);
expect(labels({ phase: '16 plus', has_sixth_form: true })).toEqual([]);
});
});
+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]);
@@ -100,19 +100,6 @@
color: var(--text-secondary);
}
/* Changes who can apply or what it costs. Outlined, not tinted: a fact, not
a verdict. An inset ring keeps the box the size of its neighbours. */
.conditionTag {
display: inline-block;
padding: 0.0625rem 0.4rem;
font-size: 0.75rem;
font-weight: 600;
line-height: 1.4;
border-radius: 4px;
box-shadow: inset 0 0 0 1px rgba(var(--ink-rgb), 0.4);
color: var(--text-primary);
}
/* Line 3: stats */
.line3 {
display: flex;
+7 -14
View File
@@ -3,14 +3,13 @@
* Four-line row for primary school search results
*
* Line 1: School name · Ofsted badge (framework-aware)
* Line 2: Phase · Type · Age range · the school page's flags (lib/schoolFacts)
* Line 2: School type · Age range · Denomination · Gender
* Line 3: Reading, Writing & Maths % · trend arrow · vs-national delta · Pupils
* Line 4: Local authority · Distance
*/
import type { School } from '@/lib/types';
import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
import { schoolFlags, schoolTypeLabel } from '@/lib/schoolFacts';
import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
import styles from './SchoolRow.module.css';
interface SchoolRowProps {
@@ -34,8 +33,8 @@ export function SchoolRow({
const phase = getPhaseStyle(school.phase);
const ofstedBadge = buildOfstedListBadge(school);
const typeLabel = schoolTypeLabel(school);
const flags = schoolFlags(school);
const showGender = school.gender && school.gender.toLowerCase() !== 'mixed';
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
@@ -78,16 +77,10 @@ export function SchoolRow({
{phase.label}
</span>
)}
{typeLabel && <span className={styles.attr}>{typeLabel}</span>}
{school.school_type && <span className={styles.attr}>{school.school_type}</span>}
{school.age_range && <span className={styles.attr}>{formatAgeRange(school.age_range)}</span>}
{flags.map((flag) => (
<span
key={flag.label}
className={flag.kind === 'condition' ? styles.conditionTag : styles.attr}
>
{flag.label}
</span>
))}
{showDenomination && <span className={styles.attr}>{school.religious_denomination}</span>}
{showGender && <span className={styles.attr}>{school.gender}</span>}
{isProposedToClose(school) && (
<span className={`${styles.attr} ${styles.attrClosing}`}>⚠ Proposed to close</span>
)}
@@ -183,17 +183,9 @@
white-space: nowrap;
}
/* Changes who can apply or what it costs. Outlined, not tinted: a fact, not
a verdict. An inset ring keeps the box the size of its neighbours. */
.conditionTag {
display: inline-block;
padding: 0.0625rem 0.4rem;
font-size: 0.75rem;
font-weight: 600;
line-height: 1.4;
border-radius: 4px;
box-shadow: inset 0 0 0 1px rgba(var(--ink-rgb), 0.4);
color: var(--text-primary);
.selectiveTag {
background: rgba(var(--status-below-rgb), 0.1);
color: var(--status-below);
}
/* ── Ofsted badge ────────────────────────────────────── */
+29 -13
View File
@@ -3,7 +3,7 @@
* Four-line row for secondary school search results
*
* Line 1: School name · Ofsted badge
* Line 2: Phase · Type · Age range · the school page's flags (lib/schoolFacts)
* Line 2: School type · Age range · Gender · Sixth form · Admissions tag
* Line 3: Attainment 8 (large) · ±LA avg delta · Pupils
* Line 4: LA name · distance
*/
@@ -11,10 +11,22 @@
'use client';
import type { School } from '@/lib/types';
import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, isProposedToClose, isSpecialSchool } from '@/lib/utils';
import { schoolFlags, schoolTypeLabel } from '@/lib/schoolFacts';
import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool } from '@/lib/utils';
import styles from './SecondarySchoolRow.module.css';
function detectAdmissionsTag(school: School): string | null {
// 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;
}
function hasSixthForm(school: School): boolean {
// GIAS OfficialSixthForm flag; missing (pipeline not yet re-run) => false.
return school.has_sixth_form ?? false;
}
interface SecondarySchoolRowProps {
school: School;
isLocationSearch?: boolean;
@@ -52,8 +64,9 @@ export function SecondarySchoolRow({
? att8 - laAvgAttainment8
: null;
const typeLabel = schoolTypeLabel(school);
const flags = schoolFlags(school);
const admissionsTag = detectAdmissionsTag(school);
const sixthForm = hasSixthForm(school);
const showGender = school.gender && school.gender.toLowerCase() !== 'mixed';
return (
<div className={`${styles.row} ${phase.key ? styles[`phase${phase.key}`] : ''} ${isInCompare ? styles.rowInCompare : ''}`}>
@@ -77,16 +90,19 @@ export function SecondarySchoolRow({
{phase.label}
</span>
)}
{typeLabel && <span className={styles.attr}>{typeLabel}</span>}
{school.school_type && <span className={styles.attr}>{school.school_type}</span>}
{school.age_range && <span className={styles.attr}>{formatAgeRange(school.age_range)}</span>}
{flags.map((flag) => (
<span
key={flag.label}
className={flag.kind === 'condition' ? styles.conditionTag : styles.provisionTag}
>
{flag.label}
{showGender && (
<span className={styles.provisionTag}>{school.gender}</span>
)}
{sixthForm && (
<span className={styles.provisionTag}>Sixth form</span>
)}
{admissionsTag && (
<span className={`${styles.provisionTag} ${admissionsTag === 'Selective' ? styles.selectiveTag : ''}`}>
{admissionsTag}
</span>
))}
)}
{isProposedToClose(school) && (
<span className={`${styles.provisionTag} ${styles.closingTag}`}>⚠ Proposed to close</span>
)}
@@ -145,90 +145,20 @@
}
/* Fact line: the phase pill, then plain register values joined by dots.
The line starts 1.125rem left of the column and clips that strip, so a
value that wraps to the start of a line loses the dot in front of it
instead of opening the line with one. */
.facts {
.meta {
display: flex;
flex-wrap: wrap;
align-items: center;
row-gap: 0.25rem;
margin: 0 0 0.5rem -1.125rem;
clip-path: inset(0 0 0 1.125rem);
font-size: 0.875rem;
color: var(--text-secondary);
gap: 0.5rem;
margin-bottom: 0.5rem;
}
.facts > * {
margin-left: 1.125rem;
}
.fact {
position: relative;
}
.fact::before {
content: "·";
position: absolute;
left: -0.75rem;
color: var(--text-muted);
}
/* The search rows' phase pill, in the same phase colours. */
.phasePill {
padding: 0.0625rem 0.4rem;
font-size: 0.75rem;
font-weight: 600;
line-height: 1.4;
border-radius: 4px;
white-space: nowrap;
}
.phasePillPrimary { background: var(--phase-primary-bg); color: var(--phase-primary-text); }
.phasePillSecondary { background: var(--phase-secondary-bg); color: var(--phase-secondary-text); }
.phasePillAllThrough { background: var(--phase-all-through-bg); color: var(--phase-all-through-text); }
.phasePillPost16 { background: var(--phase-post16-bg); color: var(--phase-post16-text); }
.phasePillNursery { background: var(--phase-nursery-bg); color: var(--phase-nursery-text); }
/* Flags are facts, not verdicts, so they carry no hue. Outlined changes who
can apply or what it costs; filled is what the school offers. Text may
wrap: the longest religious character runs past a 360px line. */
.flags {
display: flex;
flex-wrap: wrap;
gap: 0.375rem;
margin: 0 0 0.625rem;
padding: 0;
list-style: none;
}
.flagCondition,
.flagProvision {
padding: 0.0625rem 0.5rem;
font-size: 0.75rem;
font-weight: 600;
line-height: 1.4;
border: 1px solid transparent;
border-radius: 4px;
}
.flagCondition {
border-color: rgba(var(--ink-rgb), 0.4);
color: var(--text-primary);
}
.flagProvision {
background: rgba(var(--ink-rgb), 0.07);
.metaItem {
font-size: 0.8125rem;
color: var(--text-secondary);
padding: 0.125rem 0.5rem;
background: var(--bg-secondary);
border-radius: 3px;
}
@@ -773,8 +703,17 @@
word-break: break-word;
}
/* Secondary header info (headteacher, website, phone, trust, capacity)
isn't needed above the fold on phones/tablets, so it's
/* Pills wrap horizontally instead of stacking — short tokens like
"Manchester" / "Voluntary aided" fit 2 per row instead of 3 full
rows of empty horizontal space. */
.meta {
flex-direction: row;
flex-wrap: wrap;
gap: 0.375rem;
}
/* Secondary header info (headteacher, website, pupil count, trust,
contact, area) isn't needed above the fold on phones/tablets, so it's
collapsed by default and revealed on demand via the "Show all details"
link — reclaiming the vertical space so the metrics surface sooner. */
.detailsToggle {
@@ -20,9 +20,8 @@ import { useEffect, useRef, useState, type ReactNode } from 'react';
import { useRouter } from 'next/navigation';
import { useComparison } from '@/hooks/useComparison';
import { SchoolHeroMap, type SchoolHeroMapHandle } from '../SchoolHeroMap';
import type { School, SchoolCensus } from '@/lib/types';
import { formatAgeRange, getPhaseStyle, isProposedToClose } from '@/lib/utils';
import { schoolFlags, schoolTypeLabel } from '@/lib/schoolFacts';
import type { School, SchoolResult, SchoolCensus } from '@/lib/types';
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';
@@ -42,12 +41,6 @@ export interface SchoolDetailShellProps {
children: ReactNode;
}
/** Equal ignoring case and punctuation: "TIFFIN SCHOOL" is Tiffin School. */
function sameName(a: string, b: string): boolean {
const key = (s: string) => s.toLowerCase().replace(/[^a-z0-9]/g, '');
return key(a) === key(b);
}
export function SchoolDetailShell({
schoolInfo, census, navItems, children,
}: SchoolDetailShellProps) {
@@ -68,7 +61,7 @@ export function SchoolDetailShell({
const heroMapRef = useRef<SchoolHeroMapHandle>(null);
// "All ▾" jump menu listing every section.
const [sectionsOpen, setSectionsOpen] = useState(false);
// Header details (headteacher, contact, trust, capacity) collapse behind a
// Header details (headteacher, contact, trust, area) collapse behind a
// "Show all details" link on mobile/tablet, where they're below the fold.
const [detailsOpen, setDetailsOpen] = useState(false);
@@ -132,28 +125,9 @@ export function SchoolDetailShell({
// composers; recomputing them here would duplicate that work for values
// this component never renders.
const phase = schoolInfo.phase ?? '';
const isAllThrough = phase.toLowerCase() === 'all-through';
const hasLocation = schoolInfo.latitude != null && schoolInfo.longitude != null;
// Header facts: each is one register value in a fixed slot (lib/schoolFacts).
const phasePill = getPhaseStyle(schoolInfo.phase);
// Never latestResults.total_pupils: that is the results cohort, which for a
// secondary is the GCSE year group alone.
const pupils = census?.total_pupils ?? schoolInfo.total_pupils ?? null;
const facts = [
formatAgeRange(schoolInfo.age_range),
schoolTypeLabel(schoolInfo),
pupils != null ? `${pupils.toLocaleString()} pupils` : null,
].filter((fact): fact is string => !!fact);
const flags = schoolFlags(schoolInfo);
// The council, unless the address already names it.
const council = schoolInfo.local_authority
&& !(schoolInfo.address ?? '').toLowerCase().includes(schoolInfo.local_authority.toLowerCase())
? schoolInfo.local_authority
: null;
// A single-academy trust carries the school's own name, which says nothing.
const trust = schoolInfo.trust_name && !sameName(schoolInfo.trust_name, schoolInfo.school_name)
? schoolInfo.trust_name
: null;
const singleSex = singleSexLabel(schoolInfo.gender);
const handleComparisonToggle = () => {
if (isInComparison) {
@@ -228,31 +202,27 @@ export function SchoolDetailShell({
<div className={styles.headerContent}>
<div className={styles.titleSection}>
<h1 className={styles.schoolName}>{schoolInfo.school_name}</h1>
{(phasePill.label || facts.length > 0) && (
<div className={styles.facts}>
{phasePill.label && (
<span className={`${styles.phasePill} ${styles[`phasePill${phasePill.key}`]}`}>
{phasePill.label}
</span>
)}
{facts.map((fact) => (
<span key={fact} className={styles.fact}>{fact}</span>
))}
</div>
)}
{flags.length > 0 && (
// role="list": list-style: none drops list semantics in Safari.
<ul role="list" className={styles.flags} aria-label="Admission and provision">
{flags.map((flag) => (
<li
key={flag.label}
className={flag.kind === 'condition' ? styles.flagCondition : styles.flagProvision}
>
{flag.label}
</li>
))}
</ul>
)}
<div className={styles.meta}>
{schoolInfo.local_authority && (
<span className={styles.metaItem}>{schoolInfo.local_authority}</span>
)}
{schoolInfo.school_type && (
<span className={styles.metaItem}>{schoolInfo.school_type}</span>
)}
{isAllThrough && (
<span className={styles.metaItem}>All-through (primary &amp; secondary)</span>
)}
{singleSex && <span className={styles.metaItem}>{singleSex}</span>}
{schoolInfo.age_range && (
<span className={styles.metaItem}>{formatAgeRange(schoolInfo.age_range)}</span>
)}
{hasNurseryClasses(schoolInfo.nursery_provision) && (
<span className={styles.metaItem}>Nursery</span>
)}
{schoolInfo.has_sixth_form && (
<span className={styles.metaItem}>Sixth form</span>
)}
</div>
{isProposedToClose(schoolInfo) && (
<div className={styles.closingStrip} role="note">
<strong>⚠ Proposed to close.</strong> Check with the local authority before
@@ -262,7 +232,6 @@ export function SchoolDetailShell({
{schoolInfo.address && (
<p className={styles.address}>
{schoolInfo.address}{schoolInfo.postcode && `, ${schoolInfo.postcode}`}
{council && ` · ${council}`}
{hasLocation && (
<>
{' · '}
@@ -309,6 +278,23 @@ export function SchoolDetailShell({
</a>
</span>
)}
{(() => {
// 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}>
<strong>Pupils:</strong> {total.toLocaleString()}
{schoolInfo.capacity != null && ` (capacity: ${schoolInfo.capacity})`}
</span>
);
})()}
{schoolInfo.trust_name && (
<span className={styles.headerDetail}>
Part of <strong>{schoolInfo.trust_name}</strong>
</span>
)}
{schoolInfo.telephone && (
<span className={styles.headerDetail}>
<strong>Phone:</strong>{' '}
@@ -317,14 +303,22 @@ export function SchoolDetailShell({
</a>
</span>
)}
{trust && (
{schoolInfo.religious_denomination && (
<span className={styles.headerDetail}>
<strong>Academy trust:</strong> {trust}
<strong>Religious character:</strong>{' '}
{['Does not apply', 'None'].includes(schoolInfo.religious_denomination)
? 'None'
: schoolInfo.religious_denomination}
</span>
)}
{schoolInfo.capacity != null && (
{schoolInfo.county && (
<span className={styles.headerDetail}>
<strong>Capacity:</strong> {schoolInfo.capacity.toLocaleString()}
<strong>County:</strong> {schoolInfo.county}
</span>
)}
{schoolInfo.parliamentary_constituency && (
<span className={styles.headerDetail}>
<strong>Constituency:</strong> {schoolInfo.parliamentary_constituency}
</span>
)}
</div>
-66
View File
@@ -1,66 +0,0 @@
/**
* The facts the school header and the search rows print, so a parent reads
* the same words in the list and on the page.
*
* Every value comes from one register field. Nothing is inferred or explained
* inline, and a missing field prints nothing.
*/
import type { School } from './types';
import { hasNurseryClasses, hasReligiousCharacter, singleSexLabel } from './utils';
/** The search filter's type groups (backend/school_groups.py), without its
* parenthesised notes: fees have a flag of their own. */
const TYPE_GROUP_LABELS: Record<string, string> = {
state: 'State school',
independent: 'Independent school',
special: 'Special school (SEND)',
post16: 'Sixth form or college',
alternative: 'Alternative provision',
};
/** The school's type in the search filter's terms, or the register's own name
* for a type in no group (secure units, online providers). */
export function schoolTypeLabel(school: Pick<School, 'type_group' | 'school_type'>): string | null {
const grouped = school.type_group ? TYPE_GROUP_LABELS[school.type_group] : undefined;
return grouped ?? (school.school_type?.trim() || null);
}
/** "condition": changes who can apply or what it costs.
* "provision": what the school offers. */
export type SchoolFlagKind = 'condition' | 'provision';
export interface SchoolFlag {
label: string;
kind: SchoolFlagKind;
}
type FlagFields = Pick<
School,
'type_group' | 'admissions_policy' | 'gender' | 'religious_denomination'
| 'nursery_provision' | 'has_sixth_form' | 'phase'
>;
/**
* Flags for what applies, in a fixed order: conditions first, then provision.
*
* Selective needs the exact value. The register files a partly selective
* school as "Non-selective", so that value is never printed.
*/
export function schoolFlags(school: FlagFields): SchoolFlag[] {
const flags: SchoolFlag[] = [];
const condition = (label: string) => flags.push({ label, kind: 'condition' });
const provision = (label: string) => flags.push({ label, kind: 'provision' });
const phase = school.phase?.trim().toLowerCase();
if (school.type_group === 'independent') condition('Fee-paying');
if (school.admissions_policy?.trim().toLowerCase() === 'selective') condition('Selective');
const singleSex = singleSexLabel(school.gender);
if (singleSex) condition(singleSex);
if (hasReligiousCharacter(school.religious_denomination)) {
condition(school.religious_denomination!.trim());
}
if (hasNurseryClasses(school.nursery_provision) && phase !== 'nursery') provision('Nursery class');
if (school.has_sixth_form && phase !== '16 plus') provision('Sixth form');
return flags;
}
-2
View File
@@ -17,8 +17,6 @@ export interface School {
local_authority_code: number | null;
school_type: string | null;
school_type_code: string | null;
/** Search-filter type group ("state", "independent"…), null for a type in none. */
type_group?: string | null;
religious_denomination: string | null;
age_range: string | null;
has_sixth_form?: boolean | null;