From e3f21a5bc78150702b1caa96a4723cfec2621828 Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 22:16:49 +0100 Subject: [PATCH] fix(school): address review on the header chip fixes - 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, and match the single-sex labels exactly instead of any span ending in "school". Co-Authored-By: Claude Opus 5.5 --- e2e/tests/journeys.spec.ts | 5 ++++- nextjs-app/__tests__/components/schoolDetailHeader.test.tsx | 5 +++-- nextjs-app/__tests__/lib/utils.test.ts | 5 +++++ nextjs-app/lib/utils.ts | 5 +++-- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 2d8a721..1abb796 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -3293,11 +3293,14 @@ for (const width of [360, 390, 430]) { * 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'); + 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 }) }); diff --git a/nextjs-app/__tests__/components/schoolDetailHeader.test.tsx b/nextjs-app/__tests__/components/schoolDetailHeader.test.tsx index 07ea030..3fc3b6a 100644 --- a/nextjs-app/__tests__/components/schoolDetailHeader.test.tsx +++ b/nextjs-app/__tests__/components/schoolDetailHeader.test.tsx @@ -7,6 +7,7 @@ */ import { screen } from '@testing-library/react'; +import type { School } from '@/lib/types'; import { primaryFixture, secondaryFixture } from '../support/schoolFixtures'; import { renderSchoolDetail, renderSecondarySchoolDetail } from '../support/renderSchoolDetail'; @@ -30,7 +31,7 @@ jest.mock('@/components/SchoolHeroMap', () => ({ __esModule: true, })); -function withSchool(fixture: T, info: object): T { +function withSchool(fixture: T, info: Partial): T { return { ...fixture, schoolInfo: { ...fixture.schoolInfo, ...info } }; } @@ -61,6 +62,6 @@ describe('school header single-sex chip', () => { it('says nothing for a mixed school', () => { renderSecondarySchoolDetail(withSchool(secondaryFixture, { gender: 'Mixed' })); - expect(screen.queryByText(/school$/, { selector: 'span' })).not.toBeInTheDocument(); + expect(screen.queryByText(/^(Girls|Boys|Mixed)'s? school$/)).not.toBeInTheDocument(); }); }); diff --git a/nextjs-app/__tests__/lib/utils.test.ts b/nextjs-app/__tests__/lib/utils.test.ts index 0798090..9fb5042 100644 --- a/nextjs-app/__tests__/lib/utils.test.ts +++ b/nextjs-app/__tests__/lib/utils.test.ts @@ -366,6 +366,11 @@ describe('singleSexLabel', () => { 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(); diff --git a/nextjs-app/lib/utils.ts b/nextjs-app/lib/utils.ts index eab673e..f794260 100644 --- a/nextjs-app/lib/utils.ts +++ b/nextjs-app/lib/utils.ts @@ -113,8 +113,9 @@ export function hasNurseryClasses(value: string | null | undefined): boolean { * 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`; + const g = gender?.trim().toLowerCase(); + if (g === 'girls') return "Girls' school"; + if (g === 'boys') return "Boys' school"; return null; }