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>
This commit is contained in:
TudorandClaude Opus 5.5 committed 2026-10-02 22:16:49 +01:00
1 parent dea435a906
commit e3f21a5bc7
4 files changed
+15 -5

No files matched your search

+4 -1
View File
@@ -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 }) });
@@ -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<T extends { schoolInfo: object }>(fixture: T, info: object): T {
function withSchool<T extends { schoolInfo: School }>(fixture: T, info: Partial<School>): 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();
});
});
+5
View File
@@ -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();
+3 -2
View File
@@ -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;
}