Merge branch 'fix/search-row-facts' into feat/header-facts-and-flags

nextjs-app/__tests__/components/SchoolRow.test.tsx

@@ -0,0 +1,36 @@
+/**
+ * SchoolRow (primary search results): line 2 prints the religious character
+ * only when the school has one. The register's "None" was printed as a chip.
+ */
+
+import '@testing-library/jest-dom';
+import { render, screen } from '@testing-library/react';
+import { SchoolRow } from '@/components/SchoolRow';
+import type { School } from '@/lib/types';
+
+const base = {
+  urn: 100001,
+  school_name: 'Alpha Primary School',
+  local_authority: 'Testshire',
+  school_type: 'Free schools',
+  phase: 'Primary',
+  gender: 'Mixed',
+  age_range: '4-11',
+  rwm_expected_pct: 70,
+} as unknown as School;
+
+describe('SchoolRow religious character', () => {
+  it.each(['None', 'Does not apply', ''])(
+    'prints nothing when the register says %p',
+    (religious_denomination) => {
+      render(<SchoolRow school={{ ...base, religious_denomination }} />);
+      expect(screen.queryByText('None')).not.toBeInTheDocument();
+      expect(screen.queryByText('Does not apply')).not.toBeInTheDocument();
+    },
+  );
+
+  it('prints a religious character the school has', () => {
+    render(<SchoolRow school={{ ...base, religious_denomination: 'Church of England' }} />);
+    expect(screen.getByText('Church of England')).toBeInTheDocument();
+  });
+});

nextjs-app/__tests__/components/SecondarySchoolRow.test.tsx

@@ -65,3 +65,38 @@ describe('SecondarySchoolRow proposed-to-close tag', () => {
     expect(screen.queryByText(/Proposed to close/)).not.toBeInTheDocument();
   });
 });
+
+describe('SecondarySchoolRow admissions tag', () => {
+  it('tags a selective school', () => {
+    render(<SecondarySchoolRow school={{ ...base, admissions_policy: 'Selective' }} />);
+    expect(screen.getByText('Selective')).toBeInTheDocument();
+  });
+
+  it('does not tag a non-selective school as selective', () => {
+    // "Non-selective" contains "selective": a substring test tagged every
+    // comprehensive (Burntwood, Graveney) as Selective.
+    render(<SecondarySchoolRow school={{ ...base, admissions_policy: 'Non-selective' }} />);
+    expect(screen.queryByText('Selective')).not.toBeInTheDocument();
+  });
+
+  it.each(['None', 'Does not apply', '', null])(
+    'gives no faith tag when the religious character is %p',
+    (religious_denomination) => {
+      render(
+        <SecondarySchoolRow
+          school={{ ...base, admissions_policy: 'Not applicable', religious_denomination }}
+        />,
+      );
+      expect(screen.queryByText('Faith priority')).not.toBeInTheDocument();
+    },
+  );
+
+  it('tags a school with a religious character', () => {
+    render(
+      <SecondarySchoolRow
+        school={{ ...base, admissions_policy: 'Not applicable', religious_denomination: 'Church of England' }}
+      />,
+    );
+    expect(screen.getByText('Faith priority')).toBeInTheDocument();
+  });
+});

nextjs-app/__tests__/components/schoolPupilCount.test.tsx

@@ -0,0 +1,56 @@
+/**
+ * "Pupils" on a school page is the size of the school.
+ *
+ * A year's results row carries the cohort its figures were measured on. For a
+ * secondary that is the GCSE year group alone (Burntwood: 245, against 1,462
+ * on roll), so it must never stand in for the whole-school count.
+ */
+
+import { screen } from '@testing-library/react';
+import { secondaryFixture } from '../support/schoolFixtures';
+import { renderSecondarySchoolDetail } from '../support/renderSchoolDetail';
+
+jest.mock('@/lib/analytics', () => ({
+  track: jest.fn(),
+  getNavigationSource: () => 'direct',
+}));
+jest.mock('@/components/PerformanceChart', () => ({
+  PerformanceChart: () => <div data-testid="performance-chart" />,
+}));
+jest.mock('@/components/SatsChart', () => ({
+  __esModule: true,
+  default: () => <div data-testid="sats-chart" />,
+}));
+jest.mock('@/components/AdmissionsTrendChart', () => ({
+  __esModule: true,
+  default: () => <div data-testid="admissions-trend-chart" />,
+}));
+jest.mock('@/components/SchoolHeroMap', () => ({
+  SchoolHeroMap: () => <div data-testid="hero-map" />,
+  __esModule: true,
+}));
+
+function withoutCensus(schoolTotal: number | null) {
+  const yearlyData = secondaryFixture.yearlyData.map((r) => ({ ...r, total_pupils: 245 }));
+  return {
+    ...secondaryFixture,
+    census: null,
+    yearlyData,
+    schoolInfo: { ...secondaryFixture.schoolInfo, total_pupils: schoolTotal },
+  };
+}
+
+describe('pupil count without a census record', () => {
+  it('uses the register count in the header, not the results cohort', () => {
+    renderSecondarySchoolDetail(withoutCensus(1462));
+    const pupils = screen.getByText('Pupils:').parentElement!;
+    expect(pupils).toHaveTextContent('1,462');
+    expect(pupils).not.toHaveTextContent('245');
+  });
+
+  it('shows no count rather than the results cohort when the register has none', () => {
+    renderSecondarySchoolDetail(withoutCensus(null));
+    expect(screen.queryByText('Pupils:')).not.toBeInTheDocument();
+    expect(screen.queryByText('Total pupils')).not.toBeInTheDocument();
+  });
+});

nextjs-app/__tests__/support/renderSchoolDetail.tsx

@@ -39,7 +39,6 @@ export function renderSchoolDetail(fixture: any) {
     withProviders(
       <SchoolDetailShell
         schoolInfo={fixture.schoolInfo}
-        yearlyData={fixture.yearlyData}
         census={fixture.census}
         navItems={navItems}
       >
@@ -67,7 +66,6 @@ export function renderSecondarySchoolDetail(fixture: any) {
     withProviders(
       <SchoolDetailShell
         schoolInfo={fixture.schoolInfo}
-        yearlyData={fixture.yearlyData}
         census={fixture.census}
         navItems={navItems}
       >

nextjs-app/app/(frontend)/school/[slug]/page.tsx

@@ -249,7 +249,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
       {isSecondary ? (
         <SchoolDetailShell
           schoolInfo={school_info}
-          yearlyData={yearly_data}
           census={census ?? null}
           navItems={secondaryNavItems}
         >
@@ -273,7 +272,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
       ) : (
         <SchoolDetailShell
           schoolInfo={school_info}
-          yearlyData={yearly_data}
           census={census ?? null}
           navItems={primaryNavItems}
         >

nextjs-app/components/SchoolRow.tsx

@@ -9,7 +9,7 @@
  */
 
 import type { School } from '@/lib/types';
-import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
+import { formatPercentage, calculateTrend, getPhaseStyle, schoolUrl, buildOfstedListBadge, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool, listRwmValue } from '@/lib/utils';
 import styles from './SchoolRow.module.css';
 
 interface SchoolRowProps {
@@ -34,9 +34,7 @@ export function SchoolRow({
   const ofstedBadge = buildOfstedListBadge(school);
 
   const showGender = school.gender && school.gender.toLowerCase() !== 'mixed';
-  const showDenomination =
-    school.religious_denomination &&
-    school.religious_denomination !== 'Does not apply';
+  const showDenomination = hasReligiousCharacter(school.religious_denomination);
 
   // The school's OWN figure and its year-over-year trend are same-school
   // measures — shown whenever there's a real value (not the all-zero

nextjs-app/components/SecondarySchoolRow.tsx

@@ -11,14 +11,14 @@
 'use client';
 
 import type { School } from '@/lib/types';
-import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, isProposedToClose, isSpecialSchool } from '@/lib/utils';
+import { buildOfstedListBadge, getPhaseStyle, schoolUrl, formatAgeRange, hasReligiousCharacter, isProposedToClose, isSpecialSchool } from '@/lib/utils';
 import styles from './SecondarySchoolRow.module.css';
 
 function detectAdmissionsTag(school: School): string | null {
-  const policy = school.admissions_policy?.toLowerCase() ?? '';
-  if (policy.includes('selective')) return 'Selective';
-  const denom = school.religious_denomination ?? '';
-  if (denom && denom !== 'Does not apply') return 'Faith priority';
+  // Exact match: "Non-selective" contains "selective", so a substring test
+  // tagged every comprehensive as Selective.
+  if (school.admissions_policy?.trim().toLowerCase() === 'selective') return 'Selective';
+  if (hasReligiousCharacter(school.religious_denomination)) return 'Faith priority';
   return null;
 }
 

nextjs-app/components/school/SchoolDetailShell.tsx

@@ -34,8 +34,6 @@ import styles from './SchoolDetailShell.module.css';
  */
 export interface SchoolDetailShellProps {
   schoolInfo: School;
-  /** Only for the header's pupil-count fallback. */
-  yearlyData: SchoolResult[];
   census: SchoolCensus | null;
   /** Section list for the sticky nav, computed on the server. */
   navItems: NavItem[];
@@ -44,7 +42,7 @@ export interface SchoolDetailShellProps {
 }
 
 export function SchoolDetailShell({
-  schoolInfo, yearlyData, census, navItems, children,
+  schoolInfo, census, navItems, children,
 }: SchoolDetailShellProps) {
   const router = useRouter();
   const { addSchool, removeSchool, isSelected } = useComparison();
@@ -126,7 +124,6 @@ export function SchoolDetailShell({
   // 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 isAllThrough = phase.toLowerCase() === 'all-through';
   const hasLocation = schoolInfo.latitude != null && schoolInfo.longitude != null;
@@ -282,7 +279,9 @@ export function SchoolDetailShell({
                 </span>
               )}
               {(() => {
-                const total = census?.total_pupils ?? latestResults?.total_pupils ?? null;
+                // Never latestResults.total_pupils: that is the results
+                // cohort, which for a secondary is the GCSE year group alone.
+                const total = census?.total_pupils ?? schoolInfo.total_pupils ?? null;
                 if (total == null) return null;
                 return (
                   <span className={styles.headerDetail}>

nextjs-app/components/school/WellbeingSection.tsx

@@ -54,7 +54,8 @@ export function WellbeingSection({
               </div>
             )}
             {(() => {
-              const total = census?.total_pupils ?? schoolInfo.total_pupils ?? latestResults?.total_pupils ?? null;
+              // Not latestResults.total_pupils: that is the GCSE year group.
+              const total = census?.total_pupils ?? schoolInfo.total_pupils ?? null;
               if (total == null) return null;
               const female = census?.female_pupils ?? null;
               const male = census?.male_pupils ?? null;

nextjs-app/lib/utils.ts

@@ -922,6 +922,15 @@ export function isSpecialSchool(school: { school_type?: string | null }): boolea
   return /\bspecial\b/.test(t) || /pupil referral/.test(t) || /alternative provision/.test(t);
 }
 
+/**
+ * Whether GIAS records a religious character. "None" and "Does not apply" are
+ * the register's two ways of saying it has none, and neither is a faith.
+ */
+export function hasReligiousCharacter(value: string | null | undefined): boolean {
+  const v = value?.trim().toLowerCase() ?? '';
+  return v !== '' && v !== 'none' && v !== 'does not apply';
+}
+
 /**
  * The school's combined Reading, Writing & Maths figure, or null when there is
  * no real one to show.