refactor(detail): drop dead derived state from the shell
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m12s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 9m28s

Leftover from the mechanical extraction: the shell still called
computeSchoolFlags() and derived isReportCard / ofstedInspectedDate /
oeifAreas / oeifAllSameGrade / deprivationDesc / primaryAvg / secondaryAvg on
every render, duplicating work page.tsx already does. None of those values
were referenced in its JSX anymore -- that logic moved to the section
composers.

The chrome needs only four locally-derived values (latestResults, phase,
isAllThrough, hasLocation), all one-liners over props it already owns.

Removing them made seven props dead, which TypeScript caught at both call
sites: absenceData, ofsted, admissions, admissionsHistory, deprivation,
finance and nationalAvg now go straight to the section composers and never
reach the client component. The shell's surface is down to schoolInfo,
yearlyData, census, navItems and children.

No behaviour change: 155 tests pass and the characterization tests remain
byte-identical to the commit that introduced them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tudor
2026-08-02 21:54:59 +01:00
co-authored by Claude Opus 5
parent ec2d12478e
commit 803e68970c
3 changed files with 23 additions and 85 deletions
@@ -37,8 +37,9 @@ export function renderSchoolDetail(fixture: any) {
return render(
withProviders(
<SchoolDetailShell
{...fixture}
nationalAvg={nationalAveragesFixture}
schoolInfo={fixture.schoolInfo}
yearlyData={fixture.yearlyData}
census={fixture.census}
navItems={navItems}
>
<PrimarySchoolSections
@@ -62,8 +63,9 @@ export function renderSecondarySchoolDetail(fixture: any) {
return render(
withProviders(
<SchoolDetailShell
{...fixture}
nationalAvg={nationalAveragesFixture}
schoolInfo={fixture.schoolInfo}
yearlyData={fixture.yearlyData}
census={fixture.census}
navItems={navItems}
>
<SecondarySchoolSections
-14
View File
@@ -218,14 +218,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
<SchoolDetailShell
schoolInfo={school_info}
yearlyData={yearly_data}
absenceData={absence_data}
ofsted={ofsted ?? null}
census={census ?? null}
admissions={admissions ?? null}
admissionsHistory={admissions_history ?? []}
deprivation={deprivation ?? null}
finance={finance ?? null}
nationalAvg={nationalAvg}
navItems={secondaryNavItems}
>
<SecondarySchoolSections
@@ -245,14 +238,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
<SchoolDetailShell
schoolInfo={school_info}
yearlyData={yearly_data}
absenceData={absence_data}
ofsted={ofsted ?? null}
census={census ?? null}
admissions={admissions ?? null}
admissionsHistory={admissions_history ?? []}
deprivation={deprivation ?? null}
finance={finance ?? null}
nationalAvg={nationalAvg}
navItems={primaryNavItems}
>
<PrimarySchoolSections
@@ -20,34 +20,23 @@ import { useEffect, useRef, useState, type ReactNode } from 'react';
import { useRouter } from 'next/navigation';
import { useComparison } from '@/hooks/useComparison';
import { SchoolHeroMap, type SchoolHeroMapHandle } from '../SchoolHeroMap';
import type {
School, SchoolResult, AbsenceData,
OfstedInspection, SchoolCensus,
SchoolAdmissions,
SchoolDeprivation, SchoolFinance, NationalAverages,
} from '@/lib/types';
import {
formatPercentage, formatProgress, formatAcademicYear, formatAgeRange, isProposedToClose, ofstedLegacyAreas,
} from '@/lib/utils';
import { computeSchoolFlags, type NavItem } from '@/lib/schoolSections';
import type { School, SchoolResult, SchoolCensus } from '@/lib/types';
import { formatAgeRange, isProposedToClose } from '@/lib/utils';
import type { NavItem } from '@/lib/schoolSections';
import { track, getNavigationSource } from '@/lib/analytics';
import styles from './SchoolDetailShell.module.css';
/**
* Only what the chrome itself renders. Everything the sections need — Ofsted,
* admissions, deprivation, finance, national averages — goes straight to the
* section composers in page.tsx and never reaches the client.
*/
export interface SchoolDetailShellProps {
schoolInfo: School;
/** Only for the header's pupil-count fallback. */
yearlyData: SchoolResult[];
absenceData: AbsenceData | null;
ofsted: OfstedInspection | null;
census: SchoolCensus | null;
admissions: SchoolAdmissions | null;
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;
/** Section list for the sticky nav, computed on the server. */
navItems: NavItem[];
/** The server-rendered sections. */
@@ -55,9 +44,7 @@ export interface SchoolDetailShellProps {
}
export function SchoolDetailShell({
schoolInfo, yearlyData, absenceData,
ofsted, census, admissions, admissionsHistory, deprivation, finance,
nationalAvg, navItems, children,
schoolInfo, yearlyData, census, navItems, children,
}: SchoolDetailShellProps) {
const router = useRouter();
const { addSchool, removeSchool, isSelected } = useComparison();
@@ -135,21 +122,14 @@ export function SchoolDetailShell({
return () => window.removeEventListener('keydown', onKey);
}, [sectionsOpen]);
// Derived data-shape logic lives in lib/schoolSections so the server route
// can compute the section list without importing this client component.
const flags = computeSchoolFlags({
schoolInfo, yearlyData, absenceData, census, deprivation, finance,
});
const {
latestResults, isAllThrough, isSecondary, isPrimary,
hasGenderSplit, hasInclusionData, hasSchoolLife, hasDeprivation,
hasFinance, hasLocation, hasKS2Results, hasKS4Results, hasAnyResults,
isSpecial, ks2Placeholder, suppressKs2Comparison, suppressKs4Comparison,
} = flags;
// The chrome needs only these four. 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.
const latestResults = yearlyData.length > 0 ? yearlyData[yearlyData.length - 1] : null;
const phase = schoolInfo.phase ?? '';
const primaryAvg = nationalAvg?.primary ?? {};
const secondaryAvg = nationalAvg?.secondary ?? {};
const isAllThrough = phase.toLowerCase() === 'all-through';
const hasLocation = schoolInfo.latitude != null && schoolInfo.longitude != null;
const handleComparisonToggle = () => {
if (isInComparison) {
@@ -172,13 +152,6 @@ export function SchoolDetailShell({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [schoolInfo.urn]);
const deprivationDesc = (decile: number) => {
if (decile <= 3) return `This school is in one of England's most deprived areas (decile ${decile}/10). Many pupils may face additional challenges at home.`;
if (decile <= 7) return `This school is in an area with average levels of deprivation (decile ${decile}/10).`;
return `This school is in one of England's less deprived areas (decile ${decile}/10).`;
};
// Track active section as user scrolls
useEffect(() => {
const ids = navItems.map(n => n.id);
@@ -211,29 +184,6 @@ export function SchoolDetailShell({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [navItems.map(n => n.id).join(',')]);
// A report card is identified by the presence of report-card area
// judgements, NOT by `framework` — the API sets `framework` to the raw
// event grouping (e.g. "Schools - S5") even for report-card schools, so
// the old `framework === 'ReportCard'` test never matched and report cards
// were rendered as legacy ratings dated to a pre-Nov-2025 inspection.
const isReportCard = !!(
ofsted?.report_card && Object.keys(ofsted.report_card).length > 0
);
// A report card is dated by its own inspection (rc_inspection_date); the
// legacy inspection_date belongs to an older inspection and must never
// date a report card (report cards exist only from Nov 2025).
const ofstedInspectedDate = isReportCard
? ofsted?.rc_inspection_date ?? null
: ofsted?.inspection_date ?? null;
// ── Ofsted: detect if all OEIF sub-grades match the overall ───────────
const oeifAreas = ofsted ? ofstedLegacyAreas(ofsted) : [];
const oeifAllSameGrade =
!!ofsted &&
!isReportCard &&
oeifAreas.length >= 3 &&
oeifAreas.every((a) => a.value === ofsted.overall_effectiveness);
// Label shown in the mobile "section" menu button — the section in view.
const activeNavLabel = (navItems.find((n) => n.id === activeSection) ?? navItems[0])?.label ?? '';