From dea435a90653281ebfe286ffac00aee83e26992e Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 22:02:32 +0100 Subject: [PATCH] fix(school): show Nursery only for nursery classes, and say Girls' school 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 --- e2e/tests/journeys.spec.ts | 20 ++++++ .../__tests__/components/PlaceView.test.tsx | 18 ++++- .../components/schoolDetailHeader.test.tsx | 66 +++++++++++++++++++ nextjs-app/__tests__/lib/utils.test.ts | 26 ++++++++ nextjs-app/components/places/PlaceView.tsx | 9 +-- .../components/school/SchoolDetailShell.tsx | 11 ++-- nextjs-app/lib/types.ts | 3 +- nextjs-app/lib/utils.ts | 19 ++++++ 8 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 nextjs-app/__tests__/components/schoolDetailHeader.test.tsx diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 13ebecf..2d8a721 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -3286,3 +3286,23 @@ 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&per_page=1'); + expect(res.ok()).toBeTruthy(); + const [school] = (await res.json()).schools ?? []; + test.skip(!school, 'no girls\' secondary in this environment'); + const detail = await (await page.request.get(`/api/schools/${school.urn}`)).json(); + + 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); +}); diff --git a/nextjs-app/__tests__/components/PlaceView.test.tsx b/nextjs-app/__tests__/components/PlaceView.test.tsx index 2ef75dd..ff1a680 100644 --- a/nextjs-app/__tests__/components/PlaceView.test.tsx +++ b/nextjs-app/__tests__/components/PlaceView.test.tsx @@ -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(); + 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( ({ + track: jest.fn(), + getNavigationSource: () => 'direct', +})); +jest.mock('@/components/PerformanceChart', () => ({ + PerformanceChart: () =>
, +})); +jest.mock('@/components/SatsChart', () => ({ + __esModule: true, + default: () =>
, +})); +jest.mock('@/components/AdmissionsTrendChart', () => ({ + __esModule: true, + default: () =>
, +})); +jest.mock('@/components/SchoolHeroMap', () => ({ + SchoolHeroMap: () =>
, + __esModule: true, +})); + +function withSchool(fixture: T, info: object): 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(/school$/, { selector: 'span' })).not.toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/__tests__/lib/utils.test.ts b/nextjs-app/__tests__/lib/utils.test.ts index b5f74a4..0798090 100644 --- a/nextjs-app/__tests__/lib/utils.test.ts +++ b/nextjs-app/__tests__/lib/utils.test.ts @@ -15,6 +15,8 @@ import { computeYBounds, formatAgeRange, formatAgeSpan, + hasNurseryClasses, + singleSexLabel, } from '@/lib/utils'; describe('formatPercentage', () => { @@ -346,3 +348,27 @@ 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('returns null for a mixed or unknown school', () => { + expect(singleSexLabel('Mixed')).toBeNull(); + expect(singleSexLabel(null)).toBeNull(); + expect(singleSexLabel(undefined)).toBeNull(); + }); +}); diff --git a/nextjs-app/components/places/PlaceView.tsx b/nextjs-app/components/places/PlaceView.tsx index b27c935..1012864 100644 --- a/nextjs-app/components/places/PlaceView.tsx +++ b/nextjs-app/components/places/PlaceView.tsx @@ -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 && ( {/* 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} )} diff --git a/nextjs-app/components/school/SchoolDetailShell.tsx b/nextjs-app/components/school/SchoolDetailShell.tsx index ff031ca..fe9b7ab 100644 --- a/nextjs-app/components/school/SchoolDetailShell.tsx +++ b/nextjs-app/components/school/SchoolDetailShell.tsx @@ -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'; @@ -122,7 +122,7 @@ 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. @@ -130,6 +130,7 @@ export function SchoolDetailShell({ 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 +215,11 @@ export function SchoolDetailShell({ {isAllThrough && ( All-through (primary & secondary) )} - {schoolInfo.gender && schoolInfo.gender !== 'Mixed' && ( - {schoolInfo.gender}'s school - )} + {singleSex && {singleSex}} {schoolInfo.age_range && ( {formatAgeRange(schoolInfo.age_range)} )} - {schoolInfo.nursery_provision && ( + {hasNurseryClasses(schoolInfo.nursery_provision) && ( Nursery )} {schoolInfo.has_sixth_form && ( diff --git a/nextjs-app/lib/types.ts b/nextjs-app/lib/types.ts index 47fa1c9..93d31a8 100644 --- a/nextjs-app/lib/types.ts +++ b/nextjs-app/lib/types.ts @@ -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 diff --git a/nextjs-app/lib/utils.ts b/nextjs-app/lib/utils.ts index 3422edb..eab673e 100644 --- a/nextjs-app/lib/utils.ts +++ b/nextjs-app/lib/utils.ts @@ -99,6 +99,25 @@ 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(); + if (g === 'Girls' || g === 'Boys') return `${g}' school`; + return null; +} + // ============================================================================ // Number Formatting // ============================================================================