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 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
10 changed files with 218 additions and 17 deletions

No files matched your search

+46
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.
@@ -3341,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,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();
});
});
+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();
});
});
+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]);
+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';
@@ -120,13 +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 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) {
@@ -211,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 && (
+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
+20
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
// ============================================================================