fix(review): accurate PRU/AP copy, unbundle same-school trend, de-dupe note CSS
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m2s
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 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 12s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m38s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m2s
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 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 12s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m38s
Three review points on the special-schools change: 1. Copy accuracy — the context note said "Its pupils have special educational needs" for every isSpecialSchool() match, but the helper also matches pupil referral units and alternative provision, whose pupils are educated outside a mainstream setting (e.g. after exclusion) and are not necessarily SEND. Extracted a shared <SpecialSchoolNote> with type-aware copy: SEND wording only for genuine special schools; PRUs/AP get their own accurate wording. 2. Same-school trend was conflated with the England comparison — SchoolRow's year-over-year trend arrow (and the school's own figure) were gated on the same flag that drops the vs-England delta, hiding a still-meaningful trend for special schools with real data. Split the two: the school's OWN RWM figure + trend show whenever there's a real value (special schools included; only a placeholder all-zero row is hidden); only the vs-England delta is additionally dropped for special/PRU/AP. Mirrored in SecondarySchoolRow (own Attainment 8 shown; only the vs-LA delta dropped). 3. De-duplicated the .specialNote CSS (was copy-pasted between the two detail view module files) into SpecialSchoolNote.module.css, owned by the shared component so it can't drift. New SpecialSchoolNote unit tests assert SEND wording for special schools and NOT for PRUs/AP. tsc clean; 112/112 unit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
This commit is contained in:
@@ -0,0 +1,33 @@
|
|||||||
|
/**
|
||||||
|
* SpecialSchoolNote Component Tests
|
||||||
|
*/
|
||||||
|
|
||||||
|
import '@testing-library/jest-dom';
|
||||||
|
import { render, screen } from '@testing-library/react';
|
||||||
|
import { SpecialSchoolNote } from '@/components/SpecialSchoolNote';
|
||||||
|
|
||||||
|
describe('SpecialSchoolNote', () => {
|
||||||
|
it('renders nothing for a mainstream school', () => {
|
||||||
|
const { container } = render(<SpecialSchoolNote school={{ school_type: 'Academy converter' }} />);
|
||||||
|
expect(container).toBeEmptyDOMElement();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('describes a special school as teaching pupils with SEND', () => {
|
||||||
|
render(<SpecialSchoolNote school={{ school_type: 'Community special school' }} />);
|
||||||
|
expect(screen.getByText(/This is a special school/i)).toBeInTheDocument();
|
||||||
|
expect(screen.getByText(/special educational needs/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT call PRU pupils SEND — they are educated outside a mainstream school', () => {
|
||||||
|
render(<SpecialSchoolNote school={{ school_type: 'Pupil referral unit' }} />);
|
||||||
|
expect(screen.getByText(/This is a pupil referral unit/i)).toBeInTheDocument();
|
||||||
|
expect(screen.getByText(/educated outside a mainstream school/i)).toBeInTheDocument();
|
||||||
|
expect(screen.queryByText(/special educational needs/i)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT call alternative-provision pupils SEND', () => {
|
||||||
|
render(<SpecialSchoolNote school={{ school_type: 'Academy alternative provision converter' }} />);
|
||||||
|
expect(screen.getByText(/This is an alternative provision setting/i)).toBeInTheDocument();
|
||||||
|
expect(screen.queryByText(/special educational needs/i)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1558,21 +1558,3 @@
|
|||||||
.closingStrip strong {
|
.closingStrip strong {
|
||||||
color: #8a6200;
|
color: #8a6200;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Special-school context note: soft, informational (teal), not a warning. It
|
|
||||||
explains why the mainstream attainment measures and England comparison are
|
|
||||||
dropped for special schools / PRUs / AP. */
|
|
||||||
.specialNote {
|
|
||||||
background: var(--bg-secondary, #f3ede4);
|
|
||||||
border-left: 4px solid var(--accent-teal, #2d7d7d);
|
|
||||||
border-radius: 0 6px 6px 0;
|
|
||||||
padding: 0.7rem 1rem;
|
|
||||||
margin: 0 0 1rem;
|
|
||||||
font-size: 0.9rem;
|
|
||||||
line-height: 1.5;
|
|
||||||
color: var(--text-secondary, #5c564d);
|
|
||||||
max-width: 72ch;
|
|
||||||
}
|
|
||||||
.specialNote strong {
|
|
||||||
color: var(--text-primary, #1a1612);
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import {
|
|||||||
formatPercentage, formatProgress, formatAcademicYear, isProposedToClose, ofstedLegacyAreas, isSpecialSchool,
|
formatPercentage, formatProgress, formatAcademicYear, isProposedToClose, ofstedLegacyAreas, isSpecialSchool,
|
||||||
} from '@/lib/utils';
|
} from '@/lib/utils';
|
||||||
import { DeltaChip } from './DeltaChip';
|
import { DeltaChip } from './DeltaChip';
|
||||||
|
import { SpecialSchoolNote } from './SpecialSchoolNote';
|
||||||
import { summariseAdmissions } from '@/lib/compareLogic';
|
import { summariseAdmissions } from '@/lib/compareLogic';
|
||||||
|
|
||||||
const PerformanceChart = dynamic(
|
const PerformanceChart = dynamic(
|
||||||
@@ -656,20 +657,10 @@ export function SchoolDetailView({
|
|||||||
: 'End-of-primary-school tests taken by Year 6 pupils. England averages shown for comparison.'}
|
: 'End-of-primary-school tests taken by Year 6 pupils. England averages shown for comparison.'}
|
||||||
</p>
|
</p>
|
||||||
|
|
||||||
{/* Special schools sit the same national assessments but teach pupils
|
{/* Explains up front why the England comparison is dropped below, so
|
||||||
with SEND, so very few reach the mainstream "expected standard".
|
a 0% headline never reads as a failing grade against a benchmark
|
||||||
Explain that up front and drop the England comparison below, so a
|
that doesn't fit. Type-aware copy (special vs PRU vs AP). */}
|
||||||
0% headline never reads as a failing grade against a benchmark
|
<SpecialSchoolNote school={schoolInfo} />
|
||||||
that doesn't fit the school. */}
|
|
||||||
{isSpecial && (
|
|
||||||
<div className={styles.specialNote} role="note">
|
|
||||||
<strong>This is a special school.</strong> Its pupils have special educational
|
|
||||||
needs and work towards individual targets. They sit the same national tests, but
|
|
||||||
very few reach the mainstream “expected standard” these measures report,
|
|
||||||
so a comparison with the England average isn’t a meaningful guide to the
|
|
||||||
school. Where available, the progress pupils make (below) is a fairer measure.
|
|
||||||
</div>
|
|
||||||
)}
|
|
||||||
|
|
||||||
{/* ── Primary / KS2 content ── */}
|
{/* ── Primary / KS2 content ── */}
|
||||||
{hasKS2Results && (
|
{hasKS2Results && (
|
||||||
|
|||||||
@@ -38,24 +38,22 @@ export function SchoolRow({
|
|||||||
school.religious_denomination &&
|
school.religious_denomination &&
|
||||||
school.religious_denomination !== 'Does not apply';
|
school.religious_denomination !== 'Does not apply';
|
||||||
|
|
||||||
// Special schools / PRUs / AP: the mainstream RWM measure and its England
|
// A placeholder all-zero row (every subject 0 — a special/suppressed
|
||||||
// comparison aren't a fair judgement (their pupils have SEND), so a "0% ·
|
// signature, matching SchoolDetailView's ks2Placeholder) isn't a real score,
|
||||||
// −62 vs national" row misrepresents them. Also guard a placeholder all-zero
|
// so its figure is hidden. A genuine 0% combined (some pupils met individual
|
||||||
// row (every subject 0 — a special/suppressed signature), matching
|
// subjects but not all three) is NOT all-zero and stays shown.
|
||||||
// SchoolDetailView's ks2Placeholder. A genuine 0% combined (some pupils met
|
|
||||||
// individual subjects but not all three) is NOT all-zero, so it stays
|
|
||||||
// comparable and shows its real figure.
|
|
||||||
const rwmPlaceholder =
|
const rwmPlaceholder =
|
||||||
school.rwm_expected_pct === 0 &&
|
school.rwm_expected_pct === 0 &&
|
||||||
(school.reading_expected_pct ?? 0) === 0 &&
|
(school.reading_expected_pct ?? 0) === 0 &&
|
||||||
(school.writing_expected_pct ?? 0) === 0 &&
|
(school.writing_expected_pct ?? 0) === 0 &&
|
||||||
(school.maths_expected_pct ?? 0) === 0;
|
(school.maths_expected_pct ?? 0) === 0;
|
||||||
const rwmComparable =
|
// The school's OWN figure and its year-over-year trend are same-school
|
||||||
school.rwm_expected_pct != null && !rwmPlaceholder && !isSpecialSchool(school);
|
// measures — shown whenever there's a real value, special schools included.
|
||||||
|
const showRwmValue = school.rwm_expected_pct != null && !rwmPlaceholder;
|
||||||
// vs-national delta
|
// The vs-England delta is a mainstream benchmark: additionally dropped for
|
||||||
|
// special schools / PRUs / AP, whose pupils aren't measured against it fairly.
|
||||||
const rwmDelta =
|
const rwmDelta =
|
||||||
rwmComparable && nationalAvgRwm != null
|
showRwmValue && !isSpecialSchool(school) && nationalAvgRwm != null
|
||||||
? Math.round((school.rwm_expected_pct as number) - nationalAvgRwm)
|
? Math.round((school.rwm_expected_pct as number) - nationalAvgRwm)
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
@@ -102,9 +100,9 @@ export function SchoolRow({
|
|||||||
<div className={styles.line3}>
|
<div className={styles.line3}>
|
||||||
<span className={styles.stat}>
|
<span className={styles.stat}>
|
||||||
<strong className={styles.statValue}>
|
<strong className={styles.statValue}>
|
||||||
{rwmComparable ? formatPercentage(school.rwm_expected_pct, 0) : '—'}
|
{showRwmValue ? formatPercentage(school.rwm_expected_pct, 0) : '—'}
|
||||||
</strong>
|
</strong>
|
||||||
{rwmComparable && school.prev_rwm_expected_pct != null && (
|
{showRwmValue && school.prev_rwm_expected_pct != null && (
|
||||||
<span
|
<span
|
||||||
className={`${styles.trend} ${styles[`trend${trend.charAt(0).toUpperCase() + trend.slice(1)}`]}`}
|
className={`${styles.trend} ${styles[`trend${trend.charAt(0).toUpperCase() + trend.slice(1)}`]}`}
|
||||||
title={`Previous year: ${formatPercentage(school.prev_rwm_expected_pct)}`}
|
title={`Previous year: ${formatPercentage(school.prev_rwm_expected_pct)}`}
|
||||||
|
|||||||
@@ -395,22 +395,6 @@
|
|||||||
line-height: 1.5;
|
line-height: 1.5;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Special-school context note: soft, informational (teal), not a warning. */
|
|
||||||
.specialNote {
|
|
||||||
background: var(--bg-secondary, #f3ede4);
|
|
||||||
border-left: 4px solid var(--accent-teal, #2d7d7d);
|
|
||||||
border-radius: 0 6px 6px 0;
|
|
||||||
padding: 0.7rem 1rem;
|
|
||||||
margin: 0 0 1rem;
|
|
||||||
font-size: 0.9rem;
|
|
||||||
line-height: 1.5;
|
|
||||||
color: var(--text-secondary, #5c564d);
|
|
||||||
max-width: 72ch;
|
|
||||||
}
|
|
||||||
.specialNote strong {
|
|
||||||
color: var(--text-primary, #1a1612);
|
|
||||||
}
|
|
||||||
|
|
||||||
/* ── Metrics Grid & Cards ────────────────────────────── */
|
/* ── Metrics Grid & Cards ────────────────────────────── */
|
||||||
.metricsGrid {
|
.metricsGrid {
|
||||||
display: grid;
|
display: grid;
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ import type {
|
|||||||
} from '@/lib/types';
|
} from '@/lib/types';
|
||||||
import { formatPercentage, formatProgress, formatAcademicYear, formatAgeRange, isProposedToClose, ofstedLegacyAreas, isSpecialSchool } from '@/lib/utils';
|
import { formatPercentage, formatProgress, formatAcademicYear, formatAgeRange, isProposedToClose, ofstedLegacyAreas, isSpecialSchool } from '@/lib/utils';
|
||||||
import { DeltaChip } from './DeltaChip';
|
import { DeltaChip } from './DeltaChip';
|
||||||
|
import { SpecialSchoolNote } from './SpecialSchoolNote';
|
||||||
import { track, getNavigationSource } from '@/lib/analytics';
|
import { track, getNavigationSource } from '@/lib/analytics';
|
||||||
import styles from './SecondarySchoolDetailView.module.css';
|
import styles from './SecondarySchoolDetailView.module.css';
|
||||||
|
|
||||||
@@ -465,14 +466,7 @@ export function SecondarySchoolDetailView({
|
|||||||
GCSE results for Year 11 pupils.{!suppressComparison && ' England averages shown for comparison.'}
|
GCSE results for Year 11 pupils.{!suppressComparison && ' England averages shown for comparison.'}
|
||||||
</p>
|
</p>
|
||||||
|
|
||||||
{isSpecial && (
|
<SpecialSchoolNote school={schoolInfo} />
|
||||||
<div className={styles.specialNote} role="note">
|
|
||||||
<strong>This is a special school.</strong> Its pupils have special educational
|
|
||||||
needs and work towards individual targets. They sit the same GCSEs, but their
|
|
||||||
headline attainment is far below the mainstream average by design, so a comparison
|
|
||||||
with the England average isn’t a meaningful guide to the school.
|
|
||||||
</div>
|
|
||||||
)}
|
|
||||||
|
|
||||||
{p8Suspended && (
|
{p8Suspended && (
|
||||||
<div className={styles.p8Banner}>
|
<div className={styles.p8Banner}>
|
||||||
|
|||||||
@@ -55,15 +55,14 @@ export function SecondarySchoolRow({
|
|||||||
const ofstedBadge = buildOfstedListBadge(school);
|
const ofstedBadge = buildOfstedListBadge(school);
|
||||||
const phase = getPhaseStyle(school.phase);
|
const phase = getPhaseStyle(school.phase);
|
||||||
const att8 = school.attainment_8_score;
|
const att8 = school.attainment_8_score;
|
||||||
// Special schools / PRUs / AP: Attainment 8 vs the LA average isn't a fair
|
// The school's own Attainment 8 is a same-school figure — shown whenever it
|
||||||
// comparison (their pupils have SEND), so drop the delta and the number
|
// exists (special schools included; their type tag on line 2 gives context).
|
||||||
// rather than show them trailing "the average" by design. Attainment 8 is a
|
// Only the vs-LA-average delta, a benchmark comparison, is dropped for
|
||||||
// single 0–80 score with no subject breakdown to test for a placeholder, so
|
// special schools / PRUs / AP, whose pupils aren't measured against it fairly.
|
||||||
// this keys off establishment type only — a genuine (if extreme) 0.0 at a
|
|
||||||
// mainstream school still shows its real value.
|
|
||||||
const att8Comparable = att8 != null && !isSpecialSchool(school);
|
|
||||||
const laDelta =
|
const laDelta =
|
||||||
att8Comparable && laAvgAttainment8 != null ? (att8 as number) - laAvgAttainment8 : null;
|
att8 != null && !isSpecialSchool(school) && laAvgAttainment8 != null
|
||||||
|
? att8 - laAvgAttainment8
|
||||||
|
: null;
|
||||||
|
|
||||||
const admissionsTag = detectAdmissionsTag(school);
|
const admissionsTag = detectAdmissionsTag(school);
|
||||||
const sixthForm = hasSixthForm(school);
|
const sixthForm = hasSixthForm(school);
|
||||||
@@ -113,7 +112,7 @@ export function SecondarySchoolRow({
|
|||||||
<div className={styles.line3}>
|
<div className={styles.line3}>
|
||||||
<span className={styles.stat}>
|
<span className={styles.stat}>
|
||||||
<strong className={styles.statValueLarge}>
|
<strong className={styles.statValueLarge}>
|
||||||
{att8Comparable ? (att8 as number).toFixed(1) : '—'}
|
{att8 != null ? att8.toFixed(1) : '—'}
|
||||||
</strong>
|
</strong>
|
||||||
<span className={styles.statLabel}>Attainment 8</span>
|
<span className={styles.statLabel}>Attainment 8</span>
|
||||||
</span>
|
</span>
|
||||||
|
|||||||
@@ -0,0 +1,16 @@
|
|||||||
|
/* Special-school / PRU / AP context note: soft, informational (teal), not a
|
||||||
|
warning. Shared by both detail views so the styling can't drift. */
|
||||||
|
.note {
|
||||||
|
background: var(--bg-secondary, #f3ede4);
|
||||||
|
border-left: 4px solid var(--accent-teal, #2d7d7d);
|
||||||
|
border-radius: 0 6px 6px 0;
|
||||||
|
padding: 0.7rem 1rem;
|
||||||
|
margin: 0 0 1rem;
|
||||||
|
font-size: 0.9rem;
|
||||||
|
line-height: 1.5;
|
||||||
|
color: var(--text-secondary, #5c564d);
|
||||||
|
max-width: 72ch;
|
||||||
|
}
|
||||||
|
.note strong {
|
||||||
|
color: var(--text-primary, #1a1612);
|
||||||
|
}
|
||||||
@@ -0,0 +1,58 @@
|
|||||||
|
/**
|
||||||
|
* SpecialSchoolNote — the context note shown on special-school / PRU / AP
|
||||||
|
* detail pages explaining why the mainstream England-average comparison is
|
||||||
|
* dropped. Renders nothing for mainstream schools.
|
||||||
|
*
|
||||||
|
* The copy is type-aware: only genuine special schools have pupils with special
|
||||||
|
* educational needs. Pupil referral units and alternative provision teach
|
||||||
|
* pupils educated outside a mainstream setting (e.g. after exclusion, or for
|
||||||
|
* medical reasons) who are not necessarily SEND — so their note says so rather
|
||||||
|
* than mischaracterising them. Suppressing the England comparison is reasonable
|
||||||
|
* for all three.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { isSpecialSchool } from '@/lib/utils';
|
||||||
|
import styles from './SpecialSchoolNote.module.css';
|
||||||
|
|
||||||
|
type SpecialKind = 'special' | 'pru' | 'ap';
|
||||||
|
|
||||||
|
function specialKind(schoolType: string | null | undefined): SpecialKind {
|
||||||
|
const t = (schoolType ?? '').toLowerCase();
|
||||||
|
if (/pupil referral/.test(t)) return 'pru';
|
||||||
|
if (/alternative provision/.test(t)) return 'ap';
|
||||||
|
return 'special';
|
||||||
|
}
|
||||||
|
|
||||||
|
export function SpecialSchoolNote({ school }: { school: { school_type?: string | null } }) {
|
||||||
|
if (!isSpecialSchool(school)) return null;
|
||||||
|
const kind = specialKind(school.school_type);
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div className={styles.note} role="note">
|
||||||
|
{kind === 'special' && (
|
||||||
|
<>
|
||||||
|
<strong>This is a special school.</strong> Its pupils have special educational needs and
|
||||||
|
work towards individual targets. They sit the same national assessments, but very few
|
||||||
|
reach the mainstream “expected standard” these measures report — so a
|
||||||
|
comparison with the England average isn’t a meaningful guide to the school.
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
{kind === 'pru' && (
|
||||||
|
<>
|
||||||
|
<strong>This is a pupil referral unit.</strong> It teaches pupils educated outside a
|
||||||
|
mainstream school — for example after exclusion, or for medical or behavioural reasons.
|
||||||
|
The mainstream “expected standard” and the England-average comparison
|
||||||
|
aren’t a meaningful guide to the school.
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
{kind === 'ap' && (
|
||||||
|
<>
|
||||||
|
<strong>This is an alternative provision setting.</strong> It teaches pupils educated
|
||||||
|
outside a mainstream school. The mainstream “expected standard” and the
|
||||||
|
England-average comparison aren’t a meaningful guide to the school.
|
||||||
|
</>
|
||||||
|
)}{' '}
|
||||||
|
Where available, the progress its pupils make is a fairer measure.
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user