From d74cc950342ed2a3ec464175bd055e55c8347b3e Mon Sep 17 00:00:00 2001 From: Tudor Date: Sat, 1 Aug 2026 21:11:08 +0100 Subject: [PATCH] refactor(detail): fetch national averages on the server Both detail views fetched /api/national-averages in a useEffect, so the England-comparison deltas popped in after hydration and every section that uses them was pinned to the client. The page now fetches it in parallel with the school details (backend-cached 1h, degrades to null) and passes it down. Removes one client round-trip per detail page and unblocks the section extraction. Characterization tests pass unmodified; only the render helper changed. Co-Authored-By: Claude Opus 5 --- .../__tests__/support/renderSchoolDetail.tsx | 25 ++++++++----------- nextjs-app/app/school/[slug]/page.tsx | 17 ++++++++++--- nextjs-app/components/SchoolDetailView.tsx | 13 +++------- .../components/SecondarySchoolDetailView.tsx | 13 +++------- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/nextjs-app/__tests__/support/renderSchoolDetail.tsx b/nextjs-app/__tests__/support/renderSchoolDetail.tsx index c931303..38707ac 100644 --- a/nextjs-app/__tests__/support/renderSchoolDetail.tsx +++ b/nextjs-app/__tests__/support/renderSchoolDetail.tsx @@ -6,9 +6,8 @@ * suite may change — the characterization assertions passing unmodified across * that rewrite is the proof that behaviour was preserved. * - * National averages are currently fetched client-side via useEffect, so this - * helper stubs global.fetch. Once they arrive as a server-supplied prop the - * stub goes away; the tests use findBy* queries so they pass either way. + * National averages now arrive as a server-supplied prop rather than a client + * fetch, so no fetch stub is needed. */ import { render } from '@testing-library/react'; @@ -24,20 +23,16 @@ function withProviders(ui: ReactNode) { return {ui}; } -function stubNationalAveragesFetch() { - global.fetch = jest.fn((url: any) => - String(url).includes('national-averages') - ? Promise.resolve({ ok: true, json: () => Promise.resolve(nationalAveragesFixture) }) - : Promise.resolve({ ok: false, json: () => Promise.resolve({}) }), - ) as unknown as typeof fetch; -} - export function renderSchoolDetail(fixture: any) { - stubNationalAveragesFetch(); - return render(withProviders()); + return render( + withProviders(), + ); } export function renderSecondarySchoolDetail(fixture: any) { - stubNationalAveragesFetch(); - return render(withProviders()); + return render( + withProviders( + , + ), + ); } diff --git a/nextjs-app/app/school/[slug]/page.tsx b/nextjs-app/app/school/[slug]/page.tsx index 176c692..827e6d6 100644 --- a/nextjs-app/app/school/[slug]/page.tsx +++ b/nextjs-app/app/school/[slug]/page.tsx @@ -4,11 +4,12 @@ * URL format: /school/138267-school-name-here */ -import { fetchSchoolDetails, fetchSchools } from '@/lib/api'; +import { fetchSchoolDetails, fetchSchools, fetchNationalAverages } from '@/lib/api'; import { notFound, redirect } from 'next/navigation'; import { SchoolDetailView } from '@/components/SchoolDetailView'; import { SecondarySchoolDetailView } from '@/components/SecondarySchoolDetailView'; import { parseSchoolSlug, schoolUrl } from '@/lib/utils'; +import type { NationalAverages } from '@/lib/types'; import type { Metadata } from 'next'; /** @@ -124,10 +125,18 @@ export default async function SchoolPage({ params }: SchoolPageProps) { notFound(); } - // Fetch school data + // Fetch school data. National averages feed the England-comparison deltas + // across most sections; fetching them here rather than in a client effect + // keeps those sections server-renderable and puts the deltas in the initial + // HTML. They are supplementary, so they degrade to null rather than 404ing + // the page. let data; + let nationalAvg: NationalAverages | null = null; try { - data = await fetchSchoolDetails(urn); + [data, nationalAvg] = await Promise.all([ + fetchSchoolDetails(urn), + fetchNationalAverages().catch(() => null), + ]); } catch (error) { console.error(`Failed to fetch school ${urn}:`, error); notFound(); @@ -193,6 +202,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { admissions={admissions ?? null} deprivation={deprivation ?? null} finance={finance ?? null} + nationalAvg={nationalAvg} /> ) : ( )} diff --git a/nextjs-app/components/SchoolDetailView.tsx b/nextjs-app/components/SchoolDetailView.tsx index 3a14cc7..c14b67a 100644 --- a/nextjs-app/components/SchoolDetailView.tsx +++ b/nextjs-app/components/SchoolDetailView.tsx @@ -70,11 +70,15 @@ interface SchoolDetailViewProps { admissionsHistory: SchoolAdmissions[]; deprivation: SchoolDeprivation | null; finance: SchoolFinance | null; + /** Fetched on the server so the England-comparison deltas are in the + * initial HTML; null when the endpoint is unavailable. */ + nationalAvg: NationalAverages | null; } export function SchoolDetailView({ schoolInfo, yearlyData, absenceData, ofsted, census, admissions, admissionsHistory, deprivation, finance, + nationalAvg, }: SchoolDetailViewProps) { const router = useRouter(); const { addSchool, removeSchool, isSelected } = useComparison(); @@ -169,15 +173,6 @@ export function SchoolDetailView({ const isSecondary = phase.toLowerCase().includes('secondary') || isAllThrough; const isPrimary = !isSecondary; - // National averages (fetched dynamically so they stay current) - const [nationalAvg, setNationalAvg] = useState(null); - useEffect(() => { - fetch('/api/national-averages') - .then(r => r.ok ? r.json() : null) - .then(data => { if (data) setNationalAvg(data); }) - .catch(() => {}); - }, []); - const primaryAvg = nationalAvg?.primary ?? {}; const secondaryAvg = nationalAvg?.secondary ?? {}; diff --git a/nextjs-app/components/SecondarySchoolDetailView.tsx b/nextjs-app/components/SecondarySchoolDetailView.tsx index dedd252..2f1a537 100644 --- a/nextjs-app/components/SecondarySchoolDetailView.tsx +++ b/nextjs-app/components/SecondarySchoolDetailView.tsx @@ -70,11 +70,15 @@ interface SecondarySchoolDetailViewProps { admissions: SchoolAdmissions | null; deprivation: SchoolDeprivation | null; finance: SchoolFinance | null; + /** Fetched on the server so the England-comparison deltas are in the + * initial HTML; null when the endpoint is unavailable. */ + nationalAvg: NationalAverages | null; } export function SecondarySchoolDetailView({ schoolInfo, yearlyData, ofsted, census, admissions, deprivation, finance, absenceData, + nationalAvg, }: SecondarySchoolDetailViewProps) { const router = useRouter(); // Hero map — the "View on map" link opens its fullscreen view. @@ -88,15 +92,6 @@ export function SecondarySchoolDetailView({ const latestResults = yearlyData.length > 0 ? yearlyData[yearlyData.length - 1] : null; - const [nationalAvg, setNationalAvg] = useState(null); - - useEffect(() => { - fetch('/api/national-averages') - .then(r => r.ok ? r.json() : null) - .then(data => { if (data) setNationalAvg(data); }) - .catch(() => {}); - }, []); - const secondaryAvg = nationalAvg?.secondary ?? {}; // GIAS OfficialSixthForm flag; missing (pipeline not yet re-run) => false.