From 5ec4f3f7cd4cc5a2946afd57e67db9e45d55a2a3 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sat, 18 Jul 2026 21:14:57 +0100 Subject: [PATCH 1/2] fix(list/map): badge report-card schools as Report Card, not their carried-forward grade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search-result cards and map pins keyed report-card detection off ofsted_framework === 'ReportCard', but the API sets ofsted_framework to the raw event grouping ('Schools - S5'); worse, ofsted_grade (the carried-forward legacy grade) was checked first and won. So report-card schools were badged by their old grade — Barclay's pin/card read 'Outstanding · 2021' instead of 'Report Card · 2026'. Same root cause as the detail-page fix, different surface. Expose ofsted_rc_date on the list serialization (the report-card inspection date, non-null only for report cards) and make both badge builders treat a present rc-date as winning over any grade, using its year. Removes the dead framework === 'ReportCard' branches. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- backend/data_loader.py | 8 +++++++- backend/schemas.py | 1 + e2e/tests/journeys.spec.ts | 20 ++++++++++++++++++++ nextjs-app/__tests__/lib/utils.test.ts | 20 +++++++++++++++++--- nextjs-app/components/LeafletMapInner.tsx | 10 +++++++--- nextjs-app/lib/types.ts | 3 +++ nextjs-app/lib/utils.ts | 13 +++++++++---- 7 files changed, 64 insertions(+), 11 deletions(-) diff --git a/backend/data_loader.py b/backend/data_loader.py index 3cdba13..41cb0df 100644 --- a/backend/data_loader.py +++ b/backend/data_loader.py @@ -172,6 +172,7 @@ _MAIN_QUERY = text(""" foi.ofsted_grade, foi.ofsted_date, foi.ofsted_framework, + foi.ofsted_rc_date, l.local_authority_name AS local_authority, l.local_authority_code, l.address_line1 AS address1, @@ -256,7 +257,12 @@ _MAIN_QUERY = text(""" -- Fall back to the ungraded-inspection grade when no graded grade exists. COALESCE(overall_effectiveness, ungraded_grade) AS ofsted_grade, inspection_date AS ofsted_date, - framework AS ofsted_framework + framework AS ofsted_framework, + -- Report-card signal for list/map badges: non-null only when the + -- latest inspection carries report-card grades. framework is the + -- raw event grouping ("Schools - S5"), never "ReportCard", so it + -- can't be used to detect report cards. + rc_inspection_date AS ofsted_rc_date FROM marts.fact_ofsted_inspection ORDER BY urn, inspection_date DESC NULLS LAST ) foi ON s.urn = foi.urn diff --git a/backend/schemas.py b/backend/schemas.py index 8144116..14853f0 100644 --- a/backend/schemas.py +++ b/backend/schemas.py @@ -550,6 +550,7 @@ SCHOOL_COLUMNS = [ "ofsted_grade", "ofsted_date", "ofsted_framework", + "ofsted_rc_date", "latitude", "longitude", ] diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 751f47c..ce2ae08 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -80,6 +80,26 @@ test('searching by postcode returns nearby schools', async ({ page }) => { await expect(schoolLinks(page).first()).toBeVisible({ timeout: 15_000 }); }); +test('a report-card school shows a Report Card badge in search results, not its old grade', async ({ page }) => { + // List/map badges keyed off ofsted_grade (the carried-forward legacy grade) + // and never reached the report-card branch, so report-card schools were + // labelled by their old grade (e.g. "Outstanding · 2021"). The list now + // carries ofsted_rc_date and the badge treats a report card as winning. + const RC_URN = 138690; // Barclay Primary — has a Nov-2025+ report card + const res = await page.request.get(`/api/schools?search=Barclay%20Primary&per_page=5`); + expect(res.ok()).toBeTruthy(); + const barclay = ((await res.json()).schools ?? []).find( + (s: { urn: number }) => s.urn === RC_URN, + ); + test.skip(!barclay?.ofsted_rc_date, 'precondition: the list must expose ofsted_rc_date for a report-card school'); + + await searchByName(page, 'Barclay Primary'); + // The Barclay row must be present… + await expect(page.locator(`a[href*="${RC_URN}"]`).first()).toBeVisible({ timeout: 15_000 }); + // …badged as a Report Card, not its carried-forward "Outstanding" grade. + await expect(page.getByText(/Report Card ·/).first()).toBeVisible(); +}); + test('school detail page renders name and performance data', async ({ page }) => { await searchByName(page, 'primary'); const firstSchool = schoolLinks(page).first(); diff --git a/nextjs-app/__tests__/lib/utils.test.ts b/nextjs-app/__tests__/lib/utils.test.ts index 9fb9fba..7a6a406 100644 --- a/nextjs-app/__tests__/lib/utils.test.ts +++ b/nextjs-app/__tests__/lib/utils.test.ts @@ -130,9 +130,23 @@ describe('buildOfstedListBadge', () => { expect(badge.cssClass).toBe('ofsted2'); }); - it('returns Report Card badge when framework is ReportCard', () => { - const badge = buildOfstedListBadge({ ofsted_grade: null, ofsted_date: '2025-11-01', ofsted_framework: 'ReportCard' }); - expect(badge.label).toBe('Report Card · 2025'); + it('returns a Report Card badge when ofsted_rc_date is present', () => { + const badge = buildOfstedListBadge({ ofsted_grade: null, ofsted_rc_date: '2026-02-03' }); + expect(badge.label).toBe('Report Card · 2026'); + expect(badge.cssClass).toBe('ofstedRc'); + }); + + it('a report card wins over a carried-forward legacy grade', () => { + // The production bug: a report-card school (e.g. Barclay) also carries a + // carried-forward legacy grade (ofsted_grade), which used to win and label + // the pin "Outstanding · 2021" instead of "Report Card · 2026". + const badge = buildOfstedListBadge({ + ofsted_grade: 1, + ofsted_date: '2021-10-07', + ofsted_framework: 'Schools - S5', + ofsted_rc_date: '2026-02-03', + }); + expect(badge.label).toBe('Report Card · 2026'); expect(badge.cssClass).toBe('ofstedRc'); }); diff --git a/nextjs-app/components/LeafletMapInner.tsx b/nextjs-app/components/LeafletMapInner.tsx index a16e91f..b88d1a5 100644 --- a/nextjs-app/components/LeafletMapInner.tsx +++ b/nextjs-app/components/LeafletMapInner.tsx @@ -43,6 +43,13 @@ interface PopupBadge { } function buildPopupBadge(school: School): PopupBadge { + // A report card wins over any carried-forward legacy grade — its presence is + // signalled by ofsted_rc_date (the list has no full report_card object, and + // ofsted_framework is the raw event grouping, never "ReportCard"). + if (school.ofsted_rc_date) { + const rcYear = new Date(school.ofsted_rc_date).getFullYear(); + return { label: `Report Card · ${rcYear}`, style: 'background:#5a3a6e;color:#fff' }; + } const year = school.ofsted_date ? new Date(school.ofsted_date).getFullYear() : null; const yearStr = year ? ` · ${year}` : ''; if (school.ofsted_grade) { @@ -55,9 +62,6 @@ function buildPopupBadge(school: School): PopupBadge { }; return { label: `${labels[school.ofsted_grade]}${yearStr}`, style: colours[school.ofsted_grade] }; } - if (school.ofsted_framework === 'ReportCard') { - return { label: `Report Card${yearStr}`, style: 'background:#5a3a6e;color:#fff' }; - } return { label: 'Not yet inspected', style: 'background:#e0e0e0;color:#666' }; } diff --git a/nextjs-app/lib/types.ts b/nextjs-app/lib/types.ts index 7533e72..21d751a 100644 --- a/nextjs-app/lib/types.ts +++ b/nextjs-app/lib/types.ts @@ -68,6 +68,9 @@ export interface School { // Ofsted (for list view — summary only) ofsted_grade?: 1 | 2 | 3 | 4 | null; + /** Report-card inspection date (Nov 2025+); non-null identifies a report + * card in the list/map, where the full report_card object isn't available. */ + ofsted_rc_date?: string | null; ofsted_date?: string | null; ofsted_framework?: string | null; } diff --git a/nextjs-app/lib/utils.ts b/nextjs-app/lib/utils.ts index 928ac74..f62a8ca 100644 --- a/nextjs-app/lib/utils.ts +++ b/nextjs-app/lib/utils.ts @@ -704,7 +704,16 @@ export function buildOfstedListBadge(school: { ofsted_grade?: 1 | 2 | 3 | 4 | null; ofsted_date?: string | null; ofsted_framework?: string | null; + ofsted_rc_date?: string | null; }): OfstedListBadge { + // A report card wins over any carried-forward legacy grade — signalled by + // ofsted_rc_date. ofsted_framework is the raw event grouping ("Schools - + // S5"), never "ReportCard", so it can't detect report cards. + if (school.ofsted_rc_date) { + const rcYear = new Date(school.ofsted_rc_date).getFullYear(); + return { label: `Report Card · ${rcYear}`, cssClass: 'ofstedRc' }; + } + const year = school.ofsted_date ? new Date(school.ofsted_date).getFullYear() : null; @@ -723,10 +732,6 @@ export function buildOfstedListBadge(school: { }; } - if (school.ofsted_framework === 'ReportCard') { - return { label: `Report Card${yearStr}`, cssClass: 'ofstedRc' }; - } - // An inspection is on record (date or framework present) but carries no // overall grade — a post-Sept-2024 OEIF inspection. Distinct from a school // that has genuinely never been inspected. From b2b2cad5acf534ae7a667d3fb2be15efff37c4a7 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sat, 18 Jul 2026 21:24:09 +0100 Subject: [PATCH 2/2] test/docs: harden report-card list e2e + correct badge docstring Review fixes on the list/map report-card PR: - e2e precondition now hard-asserts ofsted_rc_date instead of test.skip, so the backend dropping the field fails loudly (that's the regression under test), not silently skips. - Use page_size=5 (the real backend param); per_page was ignored and fell back to the default page size. - Update buildOfstedListBadge docstring to describe the ofsted_rc_date-based, report-card-wins-first detection instead of the removed framework check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- e2e/tests/journeys.spec.ts | 11 +++++++++-- nextjs-app/lib/utils.ts | 7 +++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index ce2ae08..548a060 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -86,12 +86,19 @@ test('a report-card school shows a Report Card badge in search results, not its // labelled by their old grade (e.g. "Outstanding · 2021"). The list now // carries ofsted_rc_date and the badge treats a report card as winning. const RC_URN = 138690; // Barclay Primary — has a Nov-2025+ report card - const res = await page.request.get(`/api/schools?search=Barclay%20Primary&per_page=5`); + const res = await page.request.get(`/api/schools?search=Barclay%20Primary&page_size=5`); expect(res.ok()).toBeTruthy(); const barclay = ((await res.json()).schools ?? []).find( (s: { urn: number }) => s.urn === RC_URN, ); - test.skip(!barclay?.ofsted_rc_date, 'precondition: the list must expose ofsted_rc_date for a report-card school'); + // Hard assertions, not test.skip: if the backend stops exposing + // ofsted_rc_date for this report-card school, that IS the regression this + // test exists to catch, so it must fail loudly rather than skip. + expect(barclay, 'Barclay must appear in the search results').toBeTruthy(); + expect( + barclay.ofsted_rc_date, + 'the list must expose ofsted_rc_date for a report-card school', + ).toBeTruthy(); await searchByName(page, 'Barclay Primary'); // The Barclay row must be present… diff --git a/nextjs-app/lib/utils.ts b/nextjs-app/lib/utils.ts index f62a8ca..febe5d1 100644 --- a/nextjs-app/lib/utils.ts +++ b/nextjs-app/lib/utils.ts @@ -691,9 +691,12 @@ export interface OfstedListBadge { /** * Build the Ofsted badge for a school card in the list/map view. - * Three states: + * States, in priority order: + * - Report Card school (ofsted_rc_date set): "Report Card · YYYY" in purple. + * Checked FIRST so it wins over any carried-forward legacy grade — the + * list has no full report_card object, and ofsted_framework is the raw + * event grouping ("Schools - S5"), never "ReportCard". * - OEIF school (ofsted_grade set): grade word + year, colour-keyed - * - ReportCard school (ofsted_framework === 'ReportCard'): "Report Card · YYYY" in purple * - Inspected without an overall grade (OEIF post-Sept-2024, where Ofsted no * longer issues an overall judgement): "Inspected · YYYY" — mirrors the * detail page's hero chip so a school never reads as both inspected and