fix(compare): expert sign-off must-fixes M1-M3 #53

Merged
tudor merged 1 commits from fix/expert-signoff-mustfixes into main 2026-07-17 12:02:56 +00:00
11 changed files with 300 additions and 11 deletions
Showing only changes of commit 5944d88f0b - Show all commits
+3
View File
@@ -740,6 +740,9 @@ async def compare_schools(
"religious_denomination": convert_to_native(latest.get("religious_denomination")),
"age_range": convert_to_native(latest.get("age_range")),
"gender": convert_to_native(latest.get("gender")),
# Needed by the admissions "What this means" copy: selective
# schools get entrance-test framing, never the distance template.
"admissions_policy": convert_to_native(latest.get("admissions_policy")),
"has_sixth_form": convert_to_native(latest.get("has_sixth_form")),
"capacity": convert_to_native(latest.get("capacity")),
"gias_total_pupils": convert_to_native(latest.get("gias_total_pupils")),
+5
View File
@@ -237,6 +237,11 @@ test('comparing two secondary schools renders the secondary sections', async ({
// A KS4 measure proves the secondary academics variant rendered.
await expect(page.getByText(/Attainment 8/i).first()).toBeVisible();
await expect(page.getByText(/No primary schools in your comparison/)).toHaveCount(0);
// The admissions template must be phase-aware: the primaries' distance
// copy ("non-faith primaries") must never appear on a secondary comparison
// (expert sign-off must-fix M3).
await expect(page.getByText(/non-faith primaries/)).toHaveCount(0);
});
test('opening a different compare link after a previous comparison still renders', async ({ page }) => {
@@ -0,0 +1,140 @@
/**
* Getting a place — phase and school-type correctness (expert sign-off
* must-fixes M1/M3):
* - an all-through school's Year 7 round must never render on the primary
* tab as if it were Reception odds;
* - selective schools get entrance-test framing, and the secondary tab
* never shows the primaries' distance template.
*/
import { render, screen } from '@testing-library/react';
import { CompareAdmissions } from '@/components/compare/CompareAdmissions';
import type { ComparisonData, School, SchoolAdmissions } from '@/lib/types';
function school(urn: number, name: string, extra: Partial<School> = {}): School {
return { urn, school_name: name, ...extra } as School;
}
function admissions(partial: Partial<SchoolAdmissions>): SchoolAdmissions {
return {
year: 202627,
school_phase: 'Secondary',
places_offered: 173,
total_applications: 433,
first_preference_offer_pct: 83,
oversubscribed: true,
...partial,
} as SchoolAdmissions;
}
function entry(info: School, a: SchoolAdmissions | null): ComparisonData {
return {
school_info: info,
yearly_data: [],
ofsted: null,
census: null,
admissions: a,
admissions_history: a ? [a] : [],
deprivation: null,
};
}
describe('CompareAdmissions', () => {
it("does not show an all-through school's Year 7 round on the primary tab", () => {
// The real M1 scenario: an all-through school (Year 7 round only) beside
// a primary with a Reception round.
const allThrough = school(137306, 'Hessle High and Penshurst Primary');
const primary = school(138690, 'Barclay Primary School');
const data = {
'137306': entry(allThrough, admissions({ school_phase: 'Secondary' })),
'138690': entry(
primary,
admissions({
school_phase: 'Primary',
total_applications: 300,
places_offered: 120,
first_preference_offer_pct: 96,
}),
),
};
render(<CompareAdmissions schools={[allThrough, primary]} data={data} isSecondary={false} />);
// Hessle's Year 7 figures must not appear…
expect(screen.queryByText('433')).toBeNull();
expect(
screen.getByText(/We don't hold Reception admissions data for this school/),
).toBeInTheDocument();
// …while Barclay's Reception round renders normally.
expect(screen.getByText('300')).toBeInTheDocument();
});
it('phase-labels the section empty state when no matching round exists at all', () => {
const allThrough = school(137306, 'Hessle High and Penshurst Primary');
const data = { '137306': entry(allThrough, admissions({ school_phase: 'Secondary' })) };
render(<CompareAdmissions schools={[allThrough]} data={data} isSecondary={false} />);
expect(
screen.getByText(/No Reception admissions data is available for these schools yet/),
).toBeInTheDocument();
expect(screen.queryByText('433')).toBeNull();
});
it('shows the Year 7 round on the secondary tab', () => {
const allThrough = school(137306, 'Hessle High and Penshurst Primary');
const data = { '137306': entry(allThrough, admissions({ school_phase: 'Secondary' })) };
render(<CompareAdmissions schools={[allThrough]} data={data} isSecondary={true} />);
expect(screen.getByText('433')).toBeInTheDocument();
expect(screen.getByText('173')).toBeInTheDocument();
});
it('gives selective schools entrance-test framing, never the distance template', () => {
const grammar = school(136276, 'Watford Grammar School for Boys', {
admissions_policy: 'Selective',
religious_denomination: 'Church of England',
});
const data = {
'136276': entry(grammar, admissions({ first_preference_offer_pct: 43.7 })),
};
render(<CompareAdmissions schools={[grammar]} data={data} isSecondary={true} />);
expect(
screen.getByText(/Entry is by entrance test — the school is selective/),
).toBeInTheDocument();
expect(screen.queryByText(/non-faith primaries/)).toBeNull();
});
it('secondary faith school gets faith-aware copy, not the primaries template', () => {
const faithSchool = school(102052, "Bishop Stopford's School", {
admissions_policy: 'Non-selective',
religious_denomination: 'Church of England',
});
const data = {
'102052': entry(faithSchool, admissions({ first_preference_offer_pct: 68 })),
};
render(<CompareAdmissions schools={[faithSchool]} data={data} isSecondary={true} />);
expect(screen.getByText(/faith-based criteria may apply/)).toBeInTheDocument();
expect(screen.queryByText(/non-faith primaries/)).toBeNull();
});
it('keeps the reviewed distance copy for oversubscribed non-faith primaries', () => {
const primary = school(100140, 'Plumcroft Primary School');
const data = {
'100140': entry(
primary,
admissions({ school_phase: 'Primary', first_preference_offer_pct: 73.4 }),
),
};
render(<CompareAdmissions schools={[primary]} data={data} isSecondary={false} />);
expect(screen.getByText(/for most non-faith primaries, distance decides/)).toBeInTheDocument();
});
});
@@ -96,6 +96,31 @@ describe('CompareOfsted', () => {
expect(links[0]).toHaveAttribute('href', 'https://reports.ofsted.gov.uk/provider/21/1');
});
it('never renders Ofsted sentinel codes (9 = not applicable) as judgement chips', () => {
const sentinelSchool = school(6, 'Sentinel School');
const sentinelData: Record<string, ComparisonData> = {
'6': {
school_info: sentinelSchool,
yearly_data: [],
ofsted: ofsted({
overall_effectiveness: 2,
grade_source: 'graded',
quality_of_education: 1,
early_years_provision: 9,
sixth_form_provision: 2,
}),
},
};
render(<CompareOfsted schools={[sentinelSchool]} data={sentinelData} />);
// Real grades render…
expect(screen.getByText('Quality of education')).toBeInTheDocument();
// …the applicable sixth-form judgement renders (was previously dropped)…
expect(screen.getByText('Sixth form provision')).toBeInTheDocument();
// …and the not-applicable sentinel never appears, neither as area nor code.
expect(screen.queryByText('Early years provision')).toBeNull();
expect(screen.queryByText('9')).toBeNull();
});
it('dates a report card with the report-card inspection date, never the legacy date', () => {
const cardSchool = school(4, 'Dated Card School');
const cardData: Record<string, ComparisonData> = {
@@ -7,6 +7,7 @@
import {
OFSTED_LEGACY_GRADES,
admissionsForPhase,
ofstedDisplay,
progressBand,
rcAreaLabel,
@@ -188,6 +189,44 @@ describe('summariseAdmissions', () => {
});
});
describe('admissionsForPhase', () => {
const row = (year: number, school_phase: string | null): SchoolAdmissions =>
({ year, school_phase, places_offered: 100, total_applications: 200, first_preference_offer_pct: 80 }) as SchoolAdmissions;
it('returns the latest round matching the active phase', () => {
const data = {
admissions: row(202627, 'Secondary'),
admissions_history: [row(202526, 'Secondary'), row(202526, 'Primary'), row(202425, 'Primary')],
};
expect(admissionsForPhase(data, true)?.year).toBe(202627);
expect(admissionsForPhase(data, false)?.year).toBe(202526);
expect(admissionsForPhase(data, false)?.school_phase).toBe('Primary');
});
it("never substitutes the other phase's round (all-through with Year 7 data only)", () => {
const data = {
admissions: row(202627, 'Secondary'),
admissions_history: [row(202526, 'Secondary')],
};
expect(admissionsForPhase(data, false)).toBeNull();
expect(admissionsForPhase(data, true)?.year).toBe(202627);
});
it('uses untagged legacy rows only when no row carries a phase', () => {
const untagged = { admissions: row(202627, null), admissions_history: [row(202526, null)] };
expect(admissionsForPhase(untagged, false)?.year).toBe(202627);
expect(admissionsForPhase(untagged, true)?.year).toBe(202627);
const mixed = { admissions: row(202627, 'Secondary'), admissions_history: [row(202526, null)] };
expect(admissionsForPhase(mixed, false)).toBeNull();
});
it('handles missing data', () => {
expect(admissionsForPhase(null, false)).toBeNull();
expect(admissionsForPhase({ admissions: null, admissions_history: [] }, true)).toBeNull();
});
});
describe('progressBand', () => {
it('CI entirely above zero → above', () => {
expect(progressBand(1.2, 0.4, 2.0)).toBe('above');
+5 -1
View File
@@ -422,7 +422,11 @@ export function ComparisonView({
benchmarks={benchmarks}
isSecondary={!isPrimary}
/>
<CompareAdmissions schools={activeSchools} data={activeComparisonData} />
<CompareAdmissions
schools={activeSchools}
data={activeComparisonData}
isSecondary={!isPrimary}
/>
<CompareCommunity
schools={activeSchools}
data={activeComparisonData}
@@ -7,7 +7,7 @@
'use client';
import { summariseAdmissions } from '@/lib/compareLogic';
import { admissionsForPhase, summariseAdmissions } from '@/lib/compareLogic';
import type { ComparisonData, School } from '@/lib/types';
import { CHART_COLORS } from '@/lib/utils';
import { Cell, Chip, Measure, Section, SectionGrid, sectionStyles as s } from './sectionShared';
@@ -15,11 +15,17 @@ import { Cell, Chip, Measure, Section, SectionGrid, sectionStyles as s } from '.
export function CompareAdmissions({
schools,
data,
isSecondary = false,
}: {
schools: School[];
data: Record<string, ComparisonData>;
isSecondary?: boolean;
}) {
const rows = schools.map((school) => data[String(school.urn)]?.admissions ?? null);
// Admissions rounds are phase-specific: an all-through school's Year 7
// round must never stand in for Reception on the primary tab (and vice
// versa) — beside pure primaries it reads as Reception odds.
const rows = schools.map((school) => admissionsForPhase(data[String(school.urn)], isSecondary));
const roundLabel = isSecondary ? 'Year 7' : 'Reception';
const anyData = rows.some(Boolean);
const entryYear = rows.find(Boolean)?.year;
const entryLabel = entryYear
@@ -28,7 +34,10 @@ export function CompareAdmissions({
if (!anyData) {
return (
<Section title="Getting a place" how="No admissions data is available for these schools yet.">
<Section
title="Getting a place"
how={`No ${roundLabel} admissions data is available for these schools yet.`}
>
<></>
</Section>
);
@@ -63,7 +72,9 @@ export function CompareAdmissions({
<strong>{a.places_offered.toLocaleString('en-GB')}</strong> places
</>
) : (
<span className={s.small}>No data</span>
<span className={s.small}>
We don&apos;t hold {roundLabel} admissions data for this school
</span>
)}
</Cell>
);
@@ -104,15 +115,28 @@ export function CompareAdmissions({
{schools.map((school, i) => {
const a = rows[i];
const summary = summariseAdmissions(a);
const info = data[String(school.urn)]?.school_info;
const selective = (info?.admissions_policy ?? '').toLowerCase() === 'selective';
const faith =
!!info?.religious_denomination &&
!/^(none|does not apply|not applicable)$/i.test(info.religious_denomination);
let text: string | null = null;
if (summary.firstPrefPct != null) {
if (summary.firstPrefPct >= 100) {
if (selective) {
// Selective schools: the entrance test decides, whatever the
// offer percentage looks like — never the distance template.
text =
'Entry is by entrance test — the school is selective; distance and preference rank dont decide places.';
} else if (summary.firstPrefPct >= 100) {
text = `Every family who put ${school.school_name} first got a place.`;
} else if (summary.firstPrefPct >= 90) {
text = `Nearly every family who put ${school.school_name} first got a place.`;
} else if (a?.oversubscribed) {
text =
'More first-choice applications than places — check the schools admission criteria (for most non-faith primaries, distance decides).';
text = isSecondary
? faith
? 'More first-choice applications than places — check the schools admission criteria (faith-based criteria may apply).'
: 'More first-choice applications than places — check the schools admission criteria (catchment or distance often decides, but criteria vary).'
: 'More first-choice applications than places — check the schools admission criteria (for most non-faith primaries, distance decides).';
} else {
text = `${summary.firstPrefPct}% of first-choice families received an offer.`;
}
@@ -8,6 +8,7 @@
'use client';
import {
admissionsForPhase,
latestValues,
ofstedDisplay,
summariseAdmissions,
@@ -145,7 +146,11 @@ export function CompareAtAGlance({
<Measure label="Getting a place">
{schools.map((school, i) => {
const summary = summariseAdmissions(data[String(school.urn)]?.admissions);
// Phase-matched round only — an all-through school's Year 7 round
// must not masquerade as Reception odds on the primary tab.
const summary = summariseAdmissions(
admissionsForPhase(data[String(school.urn)], isSecondary),
);
return (
<Cell key={school.urn} school={school} index={i}>
{summary.chip ? (
@@ -108,14 +108,21 @@ function JudgementDetailCell({
);
}
const legacyAreas: Array<[string, number | null]> = [
const legacyAreas: Array<[string, number | null | undefined]> = [
['Quality of education', ofsted.quality_of_education],
['Behaviour & attitudes', ofsted.behaviour_attitudes],
['Personal development', ofsted.personal_development],
['Leadership & management', ofsted.leadership_management],
['Early years provision', ofsted.early_years_provision],
['Sixth form provision', ofsted.sixth_form_provision],
];
const published = legacyAreas.filter(([, grade]) => grade != null);
// Only real Ofsted grades (14) are judgements. The MI file uses sentinel
// codes for "not applicable / no judgement" (9, and 0/8 variants) — those
// must never render as a rating chip.
const published = legacyAreas.filter(
(entry): entry is [string, number] =>
entry[1] != null && entry[1] >= 1 && entry[1] <= 4,
);
if (published.length === 0) {
return (
+35
View File
@@ -143,6 +143,41 @@ export interface AdmissionsSummary {
interest: string | null;
}
/**
* Pick the admissions round for the ACTIVE phase tab. An all-through school
* can carry only a Year 7 (Secondary) round — rendering that beside pure
* primaries' Reception rounds made 433-forms-for-173-places read as
* Reception odds. Rows matching the target phase win (latest year first);
* rows tagged with the OTHER phase are never substituted. Untagged rows
* (legacy data, no school_phase) are used only when no row carries a phase.
*/
export function admissionsForPhase(
data:
| { admissions?: SchoolAdmissions | null; admissions_history?: SchoolAdmissions[] }
| null
| undefined,
isSecondary: boolean,
): SchoolAdmissions | null {
if (!data) return null;
const rows: SchoolAdmissions[] = [
...(data.admissions_history ?? []),
...(data.admissions ? [data.admissions] : []),
];
if (rows.length === 0) return null;
const target = isSecondary ? 'secondary' : 'primary';
const byYearDesc = (a: SchoolAdmissions, b: SchoolAdmissions) => (b.year ?? 0) - (a.year ?? 0);
const matching = rows
.filter((r) => r.school_phase?.toLowerCase() === target)
.sort(byYearDesc);
if (matching.length > 0) return matching[0];
const tagged = rows.some((r) => r.school_phase != null);
if (!tagged) return [...rows].sort(byYearDesc)[0];
return null;
}
export function summariseAdmissions(
a: SchoolAdmissions | null | undefined,
): AdmissionsSummary {
+2
View File
@@ -87,6 +87,8 @@ export interface OfstedInspection {
quality_of_education: number | null;
behaviour_attitudes: number | null;
personal_development: number | null;
/** Sixth-form judgement where applicable; sentinel 9 = not applicable. */
sixth_form_provision?: number | null;
leadership_management: number | null;
early_years_provision: number | null;
previous_overall: number | null;