diff --git a/backend/app.py b/backend/app.py index f819bf6..20d9570 100644 --- a/backend/app.py +++ b/backend/app.py @@ -633,7 +633,6 @@ async def get_school_details(request: Request, urn: int): "admissions": supplementary.get("admissions"), "admissions_history": supplementary.get("admissions_history") or [], "admission_distance": supplementary.get("admission_distance"), - "admission_distance_history": supplementary.get("admission_distance_history") or [], "sen_detail": supplementary.get("sen_detail"), "phonics": supplementary.get("phonics"), "deprivation": supplementary.get("deprivation"), diff --git a/backend/data_loader.py b/backend/data_loader.py index b27873a..62aa4a5 100644 --- a/backend/data_loader.py +++ b/backend/data_loader.py @@ -771,7 +771,6 @@ def _empty_supplementary() -> dict: "admissions": None, "admissions_history": [], "admission_distance": None, - "admission_distance_history": [], "sen_detail": None, "phonics": None, "deprivation": None, @@ -850,29 +849,30 @@ def get_supplementary_data_batch(db: Session, urns: list[int]) -> dict: result[urn]["admissions"] = rows_for_urn[-1] if rows_for_urn else None _safe(_admissions) - # Last distance offered — all years per URN, oldest first, with the latest - # also exposed on its own. + # Last distance offered — the latest year per URN, and only that. # - # The history was deliberately withheld at first: coverage is ragged (a - # school may have 2021 and 2026 and nothing between), and a plain series - # would draw a trend line straight through gaps that are absences of - # publication rather than absences of a cut-off. It is served now because - # the client distinguishes those gaps explicitly — see cutoffYearRows in - # lastDistanceOffered.ts, which classifies every year in the range and - # breaks the line rather than interpolating across it. + # The mart holds every published year and the DAG keeps loading them; what + # changed is what leaves this process. Earlier years are being held back as + # a paid feature, and this API is public and unauthenticated — serving the + # history here would hand it to anyone who opened the network tab, whatever + # the page chose to render. Withholding it in the client would have been + # decoration, not a decision. + # + # Restoring it for entitled callers is a change to this function, not to + # the pipeline: fact_admission_distance is untouched and complete. def _admission_distance(): rows = ( db.query(FactAdmissionDistance) .filter(FactAdmissionDistance.urn.in_(urns)) - .order_by(FactAdmissionDistance.urn, FactAdmissionDistance.year.asc()) + .order_by(FactAdmissionDistance.urn, FactAdmissionDistance.year.desc()) .all() ) - history: dict = {urn: [] for urn in urns} + seen = set() for d in rows: - history[d.urn].append(_admission_distance_dict(d)) - for urn, rows_for_urn in history.items(): - result[urn]["admission_distance_history"] = rows_for_urn - result[urn]["admission_distance"] = rows_for_urn[-1] if rows_for_urn else None + if d.urn in seen: + continue + seen.add(d.urn) + result[d.urn]["admission_distance"] = _admission_distance_dict(d) _safe(_admission_distance) # Deprivation — one row per URN. diff --git a/backend/tests/test_supplementary_batch.py b/backend/tests/test_supplementary_batch.py index c48d11f..5fb6cdd 100644 --- a/backend/tests/test_supplementary_batch.py +++ b/backend/tests/test_supplementary_batch.py @@ -151,11 +151,13 @@ def test_one_query_per_table_and_latest_row_per_urn(): assert out[1]["admissions"]["year"] == 202627 assert out[2]["admissions_history"] == [{**out[2]["admissions_history"][0]}] - # Cut-off distance: full history oldest-first, with the newest year also - # exposed on its own. The fixture rows are deliberately out of order, so - # this only passes if the query's ORDER BY is doing the work. - assert [r["year"] for r in out[1]["admission_distance_history"]] == [2024, 2025, 2026] + # Cut-off distance: the latest year only. Earlier years stay in the mart + # but are held back as a paid feature, and this API is public — serving + # them here would hand them to anyone reading the response. The fixture + # rows are deliberately out of order, so "latest" only comes out right if + # the query's ORDER BY is doing the work. assert out[1]["admission_distance"]["year"] == 2026 + assert "admission_distance_history" not in out[1] assert out[1]["admission_distance"]["distance_m"] == 529.47 # route_count travels with the figure — the page needs it to say the # distance is the furthest of several bands rather than the only one. @@ -171,4 +173,4 @@ def test_single_wrapper_matches_batch(monkeypatch): assert single["ofsted"]["overall_effectiveness"] == 2 assert single["admissions_history"] == [] assert single["admission_distance"] is None - assert single["admission_distance_history"] == [] + assert "admission_distance_history" not in single diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 165ad0d..70d48d2 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1226,21 +1226,13 @@ const CUTOFF_CANDIDATE_URNS = [ 101099, 100553, 102574, 100769, // mixed ]; -async function schoolWithCutoff(page: Page, minPublishedYears = 1) { +async function schoolWithCutoff(page: Page) { for (const urn of CUTOFF_CANDIDATE_URNS) { const res = await page.request.get(`/api/schools/${urn}`); if (!res.ok()) continue; const body = await res.json(); if (body?.admission_distance?.distance_m == null) continue; - const published = (body.admission_distance_history ?? []) - .filter((h: { distance_m?: number | null }) => h.distance_m != null); - if (published.length < minPublishedYears) continue; - return { - urn, - distance: body.admission_distance, - history: published, - phase: body.school_info?.phase, - }; + return { urn, distance: body.admission_distance, phase: body.school_info?.phase }; } return null; } @@ -1317,91 +1309,90 @@ test('no school page shows an implausible cut-off distance', async ({ page }) => // ── The Distance view, map and postcode check ──────────────────────────── -test('a school with several published years gets the Distance view', async ({ page }) => { - const found = await schoolWithCutoff(page, 2); - test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); +test('the public API serves the latest cut-off only', async ({ page }) => { + /* + * Earlier years are held back as a paid feature. This endpoint is public and + * unauthenticated, so withholding them in the UI alone would be decoration: + * anyone could read the history out of the network tab. The mart still holds + * every year — this asserts what leaves the process, not what was collected. + */ + let checked = 0; + for (const urn of CUTOFF_CANDIDATE_URNS) { + const res = await page.request.get(`/api/schools/${urn}`); + if (!res.ok()) continue; + const body = await res.json(); + if (body?.admission_distance?.distance_m == null) continue; + checked += 1; + expect(body, `URN ${urn} still exposes a cut-off history`) + .not.toHaveProperty('admission_distance_history'); + } + test.skip(checked === 0, 'no cut-off distances available to check yet'); +}); + +test('a school page shows no year-by-year cut-off record', async ({ page }) => { + const found = await schoolWithCutoff(page); + test.skip(found === null, 'no school in the sample has a published cut-off yet'); await page.goto(`/school/${found!.urn}`); await expect(page.locator('#admissions')).toBeVisible({ timeout: 15_000 }); - // Its own section on both templates, not a third tab inside Admissions — - // stacking a view that tall in the admissions viewport sized the whole card - // to it and left the default view mostly blank. - await expect(page.locator('#distance')).toBeVisible(); + // The render side of the same rule, so a future component cannot put the + // history back without the API. + await expect(page.getByText(/Last distance offered, by year/)).toHaveCount(0); + await expect(page.getByText(/too few to read as a trend/)).toHaveCount(0); await expect(page.getByRole('button', { name: 'Distance' })).toHaveCount(0); - - // Every published year must appear as a row. - for (const h of found!.history as { year: number }[]) { - await expect(page.getByRole('rowheader', { name: String(h.year) })).toBeVisible(); - } }); -test('the year table never leaves a gap unexplained', async ({ page }) => { - // A blank row would tell a parent nothing; each gap must name its reason. - const found = await schoolWithCutoff(page, 2); - test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); +test('the postcode check answers for the published year, and names it', async ({ page }) => { + const found = await schoolWithCutoff(page); + test.skip(found === null, 'no school in the sample has a published cut-off yet'); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); + const section = page.locator('#distance'); + // Needs coordinates as well as a figure; not every school has both. + test.skip(await section.count() === 0, 'this school has no distance section'); + await expect(section).toBeVisible({ timeout: 15_000 }); - const statuses = await page.locator('[class*="cutoffPill"]').allTextContents(); - expect(statuses.length).toBeGreaterThan(0); - for (const s of statuses) { - expect(s.trim(), 'every row carries a status').not.toBe(''); - } - // The stronger claim the data cannot support must never appear. - await expect(page.getByText(/every applicant was offered/i)).toHaveCount(0); -}); - -test('the postcode check answers with a distance and per-year verdicts', async ({ page }) => { - const found = await schoolWithCutoff(page, 2); - test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); - - await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); - - const input = page.getByLabel('Your postcode'); - await expect(input).toBeVisible(); - await input.fill('SW1A 1AA'); + await page.getByLabel('Your postcode').fill('SW1A 1AA'); await page.getByRole('button', { name: 'Check' }).click(); - // Either a verdict or a plain error — never a silent no-op. const result = page.getByRole('status'); const error = page.getByRole('alert'); await expect(result.or(error)).toBeVisible({ timeout: 20_000 }); if (await result.isVisible()) { - await expect(result).toContainText(/mile|m\b/); + // A verdict without its year is a number a parent cannot place. + await expect(result).toContainText(new RegExp(`September ${found!.distance.year}`)); + await expect(result).toContainText(/inside|beyond|too close/); } }); test('the postcode check states its limits before it is used', async ({ page }) => { - // The caveat is not revealed with the answer — it is there while the parent - // is deciding whether to trust the answer at all. - const found = await schoolWithCutoff(page, 2); - test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); + const found = await schoolWithCutoff(page); + test.skip(found === null, 'no school in the sample has a published cut-off yet'); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); + const section = page.locator('#distance'); + test.skip(await section.count() === 0, 'this school has no distance section'); + await expect(section).toBeVisible({ timeout: 15_000 }); const caveat = page.getByText(/Distance is the last criterion applied/); await expect(caveat).toBeVisible(); await expect(caveat).toContainText(/not a catchment boundary/); await expect(caveat).toContainText(/walking route/); - // One caveat for the section, not the three paragraphs it replaced. await expect(caveat).toHaveCount(1); }); test('the distance section never scrolls the page sideways', async ({ page }) => { - const found = await schoolWithCutoff(page, 2); - test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); + const found = await schoolWithCutoff(page); + test.skip(found === null, 'no school in the sample has a published cut-off yet'); await page.setViewportSize({ width: 390, height: 844 }); await page.goto(`/school/${found!.urn}`); - await expect(page.locator('#distance')).toBeVisible({ timeout: 15_000 }); + const section = page.locator('#distance'); + test.skip(await section.count() === 0, 'this school has no distance section'); + await expect(section).toBeVisible({ timeout: 15_000 }); - // The year table is deliberately wider than a phone; its own wrapper has to - // absorb that, or the whole page slides under the reader's thumb. const overflow = await page.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth); expect(overflow, 'page must not scroll horizontally').toBeLessThanOrEqual(1); @@ -1422,7 +1413,7 @@ test('no single section dominates the height of a school page', async ({ page }) * a reader actually sees is one section wildly out of proportion with its * neighbours, so that is what this asserts. */ - const found = await schoolWithCutoff(page, 2); + const found = await schoolWithCutoff(page); test.skip(found === null, 'no school in the sample has 2+ published cut-off years yet'); await page.goto(`/school/${found!.urn}`); diff --git a/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx b/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx index aabb4ca..e37759f 100644 --- a/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx +++ b/nextjs-app/__tests__/components/CutoffMapPanel.test.tsx @@ -3,17 +3,17 @@ * * This is the one place on the site that answers a question about a specific * family rather than about a school, so the tests here are mostly about what it - * refuses to say: no verdict without a published figure, and no verdict at all - * when the margin is inside the error of a postcode centroid. + * refuses to say — and that matters more now than it did, because there is only + * one year to answer with. A run of years used to soften a single close call; + * nothing does now, so the "too close to call" band is the whole safety margin. */ import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { CutoffMapPanel } from '@/components/school/CutoffMapPanel'; -import { cutoffYearRows, CUTOFF_UNCERTAINTY_M } from '@/components/school/lastDistanceOffered'; +import { CUTOFF_UNCERTAINTY_M } from '@/components/school/lastDistanceOffered'; import type { School, SchoolAdmissionDistance } from '@/lib/types'; // Leaflet needs a real layout box and network tiles; neither exists in jsdom. -// The panel's logic is independent of it, so the map is stubbed out. jest.mock('@/components/LeafletCutoffMapInner', () => ({ __esModule: true, default: () =>
, @@ -27,7 +27,7 @@ jest.mock('@/lib/api', () => ({ const SCHOOL = { urn: 100010, school_name: 'Test Primary', latitude: 51.5, longitude: -0.12 } as School; -const d = (year: number, distance_m: number): SchoolAdmissionDistance => ({ +const cutoff = (distance_m: number | null, year = 2026): SchoolAdmissionDistance => ({ year, distance_m, route_count: 1, la_name: 'Camden', distance_unit_raw: 'miles', }); @@ -36,11 +36,8 @@ function northOf(metres: number) { return { latitude: SCHOOL.latitude! + metres / 111_320, longitude: SCHOOL.longitude! }; } -function renderPanel(history: SchoolAdmissionDistance[], admissions: { year: number; oversubscribed?: boolean }[] = []) { - return render( - , - ); -} +const renderPanel = (c = cutoff(800)) => + render(); async function check(postcode: string) { fireEvent.change(screen.getByLabelText('Your postcode'), { target: { value: postcode } }); @@ -50,8 +47,8 @@ async function check(postcode: string) { beforeEach(() => mockGeocode.mockReset()); describe('CutoffMapPanel', () => { - it('renders nothing without a published figure to draw', () => { - const { container } = renderPanel([]); + it('renders nothing without a figure to compare against', () => { + const { container } = renderPanel(cutoff(null)); expect(container).toBeEmptyDOMElement(); }); @@ -59,61 +56,58 @@ describe('CutoffMapPanel', () => { const { container } = render( , ); expect(container).toBeEmptyDOMElement(); }); it('rejects a malformed postcode without calling the geocoder', async () => { - renderPanel([d(2024, 800)]); + renderPanel(); await check('not a postcode'); expect(await screen.findByRole('alert')).toHaveTextContent(/does not look like a UK postcode/); expect(mockGeocode).not.toHaveBeenCalled(); }); - it('reports a home clearly inside every published year', async () => { + it('names the year in the verdict, so the figure is never free-floating', async () => { mockGeocode.mockResolvedValue(northOf(200)); - renderPanel([d(2022, 1000), d(2024, 800)]); + renderPanel(cutoff(800, 2026)); await check('SE23 3NA'); - expect(await screen.findByRole('status')).toHaveTextContent(/inside the cut-off in all 2 years/); + const result = await screen.findByRole('status'); + expect(result).toHaveTextContent(/inside the/); + expect(result).toHaveTextContent(/September 2026/); }); - it('reports a home clearly outside every published year', async () => { + it('reports a home clearly beyond the cut-off', async () => { mockGeocode.mockResolvedValue(northOf(5000)); - renderPanel([d(2022, 1000), d(2024, 800)]); + renderPanel(cutoff(800)); await check('SE23 3NA'); - expect(await screen.findByRole('status')).toHaveTextContent(/outside the cut-off in every year/); + expect(await screen.findByRole('status')).toHaveTextContent(/beyond the/); }); it('declines to call a result that sits inside the measurement error', async () => { - // The home is nominally inside 2024's 800 m cut-off, but only by half the - // uncertainty band — which a postcode centroid cannot resolve. + // Nominally inside the 800 m cut-off, but by half the uncertainty band — + // which a postcode centroid cannot resolve. With only one year published + // there is nothing else to fall back on, so this must not read as a pass. mockGeocode.mockResolvedValue(northOf(800 - CUTOFF_UNCERTAINTY_M / 2)); - renderPanel([d(2024, 800)]); + renderPanel(cutoff(800)); await check('SE23 3NA'); const result = await screen.findByRole('status'); - expect(result).toHaveTextContent(/too close to call/); - expect(result).not.toHaveTextContent(/inside the cut-off in all/); - }); - - it('counts an unpublished year as unknown rather than as a pass', async () => { - mockGeocode.mockResolvedValue(northOf(200)); - renderPanel([d(2022, 1000), d(2024, 800)], [{ year: 2023, oversubscribed: false }]); - await check('SE23 3NA'); - - const result = await screen.findByRole('status'); - expect(result).toHaveTextContent(/inside the cut-off in all 2 years/); - expect(result).toHaveTextContent(/no published figure/); + expect(result).toHaveTextContent(/too close/); + expect(result).toHaveTextContent(/measurement error/); + // Explanation is supporting text, not part of the bold verdict line. + expect(result.querySelector('[class*="cutoffCheckHeadline"]')!.textContent) + .not.toMatch(/measurement error/); + expect(result).not.toHaveTextContent(/^\S+ away — inside/); }); it('surfaces a postcode the geocoder cannot find', async () => { mockGeocode.mockResolvedValue(null); - renderPanel([d(2024, 800)]); + renderPanel(); await check('ZZ99 9ZZ'); expect(await screen.findByRole('alert')).toHaveTextContent(/could not find that postcode/); @@ -121,7 +115,7 @@ describe('CutoffMapPanel', () => { it('recovers from a geocoder failure instead of leaving a stale verdict', async () => { mockGeocode.mockResolvedValue(northOf(200)); - renderPanel([d(2024, 800)]); + renderPanel(); await check('SE23 3NA'); await screen.findByRole('status'); @@ -132,21 +126,10 @@ describe('CutoffMapPanel', () => { expect(screen.getByRole('alert')).toHaveTextContent(/Something went wrong/); }); - it('does not carry the caveat itself', () => { - // It moved to CutoffDistanceDetail so it renders once per section, and so - // it still appears for a school with no coordinates (no map, no check). - // Asserting its absence here is what stops the old three-paragraph stutter - // creeping back. - renderPanel([d(2024, 800)]); - expect(screen.queryByText(/Distance is the last criterion/)).not.toBeInTheDocument(); - }); - it('keeps the map behind a request until there is a reason to show it', async () => { - renderPanel([d(2024, 800)]); + renderPanel(); expect(screen.queryByTestId('cutoff-map')).not.toBeInTheDocument(); - // A successful check is that reason: the rings only answer a question once - // there is a home to sit beside them. mockGeocode.mockResolvedValue(northOf(200)); await check('SE23 3NA'); await screen.findByRole('status'); @@ -154,8 +137,16 @@ describe('CutoffMapPanel', () => { }); it('can also show the map without a postcode, on request', () => { - renderPanel([d(2024, 800)]); - fireEvent.click(screen.getByRole('button', { name: /Show these distances on a map/ })); + renderPanel(); + fireEvent.click(screen.getByRole('button', { name: /Show this distance on a map/ })); expect(screen.getByTestId('cutoff-map')).toBeInTheDocument(); }); + + it('states its limits before it is used, not with the answer', () => { + renderPanel(); + const caveat = screen.getByText(/Distance is the last criterion applied/); + expect(caveat).toBeInTheDocument(); + expect(caveat).toHaveTextContent(/not a catchment boundary/); + expect(caveat).toHaveTextContent(/walking route/); + }); }); diff --git a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx index c3c13bd..0c30e16 100644 --- a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx +++ b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx @@ -89,128 +89,68 @@ describe('secondary detail page', () => { }); }); -// ── The Distance view ────────────────────────────────────────────────── +// ── The Distance section ─────────────────────────────────────────────── -const history = (pts: [number, number][]): SchoolAdmissionDistance[] => - pts.map(([year, distance_m]) => ({ - year, distance_m, route_count: 1, la_name: 'Camden', distance_unit_raw: 'miles', - })); - -describe('primary Distance section', () => { - it('appears as its own section once there are two or more published years', () => { - // Not a third tab inside Admissions: stacking a 1402px view in the - // admissions viewport sized the whole card to it and left the default view - // as four tiles in ~1080px of blank card. +describe('Distance section', () => { + it('appears for a school with a figure and coordinates', () => { const { container } = renderSchoolDetail({ ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2021, 1000], [2022, 900], [2023, 800], [2024, 700]]), + schoolInfo: { ...primaryFixture.schoolInfo, latitude: 51.5, longitude: -0.12 }, + admissionDistance: cutoff({ distance_m: 700, year: 2026 }), }); expect(container.querySelector('#distance')).toBeInTheDocument(); - expect(screen.getByText('How far the last place went')).toBeInTheDocument(); - // And it did not come back as a tab. - expect(screen.queryByRole('button', { name: 'Distance' })).not.toBeInTheDocument(); + expect(screen.getByText('How far away are you?')).toBeInTheDocument(); }); - it('stays away on a single published year', () => { - // One point is a fact, not a history; a chart of it invites a trend reading - // that is not there. + it('stays away when the school has no coordinates to measure from', () => { const { container } = renderSchoolDetail({ ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2024, 700]]), + schoolInfo: { ...primaryFixture.schoolInfo, latitude: null, longitude: null }, + admissionDistance: cutoff({ distance_m: 700, year: 2026 }), }); expect(container.querySelector('#distance')).not.toBeInTheDocument(); }); - it('lists every year in the span, including the ones with no figure', () => { - renderSchoolDetail({ + it('stays away when no figure has been published', () => { + const { container } = renderSchoolDetail({ ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2021, 1000], [2024, 700]]), + schoolInfo: { ...primaryFixture.schoolInfo, latitude: 51.5, longitude: -0.12 }, + admissionDistance: null, }); - // 2022 and 2023 were never published but must still appear as rows, or the - // gap in the chart has nothing explaining it. - for (const year of ['2021', '2022', '2023', '2024']) { - expect(screen.getByRole('rowheader', { name: year })).toBeInTheDocument(); - } - expect(screen.getAllByText('Not published').length).toBe(2); + expect(container.querySelector('#distance')).not.toBeInTheDocument(); }); - it('never claims everyone was offered a place', () => { - // fact_admissions.oversubscribed is about first preferences only, so the - // stronger claim is not supported by the data behind it. - renderSchoolDetail({ - ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2021, 1000], [2024, 700]]), - admissionsHistory: [ - ...primaryFixture.admissionsHistory, - { year: 2022, oversubscribed: false, places_offered: 60 }, - ], - }); - - expect(screen.getByText(/Places available on first preferences/)).toBeInTheDocument(); - expect(screen.queryByText(/every applicant/i)).not.toBeInTheDocument(); - expect(screen.queryByText(/all offered/i)).not.toBeInTheDocument(); - }); - - it('draws the chart only once it will also state a direction', () => { - const four = renderSchoolDetail({ - ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2021, 1000], [2022, 900], [2023, 800], [2024, 700]]), - }); - expect(screen.getByText(/Last distance offered, by year/)).toBeInTheDocument(); - four.unmount(); - - renderSchoolDetail({ - ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2023, 800], [2024, 700]]), - }); - expect(screen.queryByText(/Last distance offered, by year/)).not.toBeInTheDocument(); - }); - - it('states the caveat exactly once, even though the check also renders', () => { + it('shows no year-by-year record — that is held back as a paid feature', () => { + // The page must not leak the history through a table, a chart or a strip of + // per-year verdicts. The API no longer sends it either; this guards the + // render side so a future component cannot quietly put it back. renderSchoolDetail({ ...primaryFixture, schoolInfo: { ...primaryFixture.schoolInfo, latitude: 51.5, longitude: -0.12 }, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2021, 1000], [2024, 700]]), + admissionDistance: cutoff({ distance_m: 700, year: 2026 }), }); - expect(screen.getAllByText(/Distance is the last criterion/)).toHaveLength(1); - }); - - it('withholds a trend reading while the record is thin', () => { - renderSchoolDetail({ - ...primaryFixture, - admissionDistance: cutoff({ distance_m: 700, year: 2024 }), - admissionDistanceHistory: history([[2023, 800], [2024, 700]]), - }); - - expect(screen.getByText(/too few to read as a trend/)).toBeInTheDocument(); - expect(screen.queryByText(/Across \d+ published years/)).not.toBeInTheDocument(); + // Scoped to the section: the page has other tables (the history section's). + const section = document.querySelector('#distance')!; + expect(section.querySelector('table')).toBeNull(); + expect(screen.queryByText(/Last distance offered, by year/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Not published/)).not.toBeInTheDocument(); + expect(screen.queryByText(/too few to read as a trend/)).not.toBeInTheDocument(); }); }); describe('secondary Distance section', () => { - it('renders as its own section once there are two published years', () => { - renderSecondarySchoolDetail({ + it('renders on the secondary template too', () => { + const { container } = renderSecondarySchoolDetail({ ...secondaryFixture, - admissionDistance: cutoff({ distance_m: 3472.96 }), - admissionDistanceHistory: history([[2023, 3800], [2024, 3472.96]]), + schoolInfo: { ...secondaryFixture.schoolInfo, latitude: 51.5, longitude: -0.12 }, + admissionDistance: cutoff({ distance_m: 3472.96, year: 2026 }), }); - expect(screen.getByRole('rowheader', { name: '2023' })).toBeInTheDocument(); - // Two points is below the threshold that lets us state a direction, so no - // chart is drawn — a line through three points asserts a trend the - // summary underneath would refuse to. - expect(screen.queryByText(/Last distance offered, by year/)).not.toBeInTheDocument(); + expect(container.querySelector('#distance')).toBeInTheDocument(); }); it('explains a selective school by how it admits rather than as missing data', () => { @@ -218,7 +158,6 @@ describe('secondary Distance section', () => { ...secondaryFixture, schoolInfo: { ...secondaryFixture.schoolInfo, admissions_policy: 'Selective' }, admissionDistance: null, - admissionDistanceHistory: [], }); expect(screen.getByText(/ranked by the entrance test/)).toBeInTheDocument(); @@ -229,7 +168,6 @@ describe('secondary Distance section', () => { renderSecondarySchoolDetail({ ...secondaryFixture, admissionDistance: null, - admissionDistanceHistory: [], admissionsHistory: [ { year: 2022, oversubscribed: false }, { year: 2023, oversubscribed: false }, diff --git a/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts b/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts index 0059303..09278ae 100644 --- a/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts +++ b/nextjs-app/__tests__/lib/lastDistanceOffered.test.ts @@ -9,8 +9,7 @@ import { formatCutoffDistance, formatEntryYear } from '@/lib/utils'; import { - describeCutoff, cutoffYearRows, cutoffTrendSummary, cutoffCoverageNote, - describeCutoffAbsence, compareToCutoffs, entryYearOf, CUTOFF_UNCERTAINTY_M, + describeCutoff, describeCutoffAbsence, compareToCutoff, CUTOFF_UNCERTAINTY_M, } from '@/components/school/lastDistanceOffered'; import type { SchoolAdmissionDistance } from '@/lib/types'; @@ -82,97 +81,42 @@ describe('describeCutoff', () => { }); }); -// ── Year-by-year history ─────────────────────────────────────────────── +// ── "Would we have got in?" ──────────────────────────────────────────── -const d = (year: number, distance_m: number | null, route_count = 1): SchoolAdmissionDistance => ({ - year, distance_m, route_count, la_name: 'Camden', distance_unit_raw: 'miles', -}); - -describe('entryYearOf', () => { - it('reduces EES academic codes and plain entry years to one key', () => { - // Without this the two histories never join and every year looks unpublished. - expect(entryYearOf(202425)).toBe(2024); - expect(entryYearOf(2024)).toBe(2024); - }); -}); - -describe('cutoffYearRows', () => { - it('emits a row for every year in the span, including the empty ones', () => { - const rows = cutoffYearRows([d(2021, 900), d(2024, 700)]); - expect(rows.map((r) => r.year)).toEqual([2024, 2023, 2022, 2021]); - expect(rows.map((r) => r.status)).toEqual([ - 'published', 'not-published', 'not-published', 'published', - ]); +describe('compareToCutoff', () => { + it('calls a clearly nearer home inside, and names the year', () => { + const r = compareToCutoff(300, 800, 2026); + expect(r.verdict).toBe('inside'); + expect(r.headline).toContain('September 2026'); }); - it('separates "nothing published" from "was not oversubscribed"', () => { - // The two look identical in the distance data and mean opposite things. - const rows = cutoffYearRows( - [d(2021, 900), d(2023, 700)], - [{ year: 2022, oversubscribed: false }], - ); - expect(rows.find((r) => r.year === 2022)!.status).toBe('not-oversubscribed'); + it('calls a clearly further home beyond', () => { + expect(compareToCutoff(4000, 800, 2026).verdict).toBe('outside'); }); - it('matches EES six-digit years against plain cut-off years', () => { - const rows = cutoffYearRows( - [d(2023, 700)], - [{ year: 202324, places_offered: 60, oversubscribed: true }], - ); - expect(rows.find((r) => r.year === 2023)!.placesOffered).toBe(60); + it('refuses to call a result inside the measurement error, either way', () => { + // A postcode centroid covers several addresses, so a margin this fine is + // noise. With one published year there is no other year to fall back on, + // which makes this band the only thing standing between a parent and a + // place they do not have. + expect(compareToCutoff(800 - CUTOFF_UNCERTAINTY_M / 2, 800, 2026).verdict).toBe('too-close'); + expect(compareToCutoff(800 + CUTOFF_UNCERTAINTY_M / 2, 800, 2026).verdict).toBe('too-close'); + expect(compareToCutoff(800, 800, 2026).detail).toContain('measurement error'); + // The explanation is not welded to the headline, so it does not run at + // headline weight in the result block. + expect(compareToCutoff(800, 800, 2026).headline).not.toContain('measurement error'); + expect(compareToCutoff(300, 800, 2026).detail).toBeNull(); }); - it('does not stretch the span back over admissions years with no cut-offs', () => { - // EES reaches back further than councils publish; padding the chart with a - // decade of blanks would bury the years that carry a figure. - const rows = cutoffYearRows( - [d(2024, 700)], - [{ year: 2015, oversubscribed: true }, { year: 2024, oversubscribed: true }], - ); - expect(rows.map((r) => r.year)).toEqual([2024]); - }); - - it('is safe when the backend does not send the field at all', () => { - expect(cutoffYearRows(undefined as never)).toEqual([]); - expect(cutoffYearRows([], [])).toEqual([]); - }); -}); - -describe('cutoffTrendSummary', () => { - const rowsFor = (pts: [number, number][]) => cutoffYearRows(pts.map(([y, m]) => d(y, m))); - - it('says nothing below four published points', () => { - expect(cutoffTrendSummary(rowsFor([[2022, 900], [2023, 800], [2024, 700]]))).toBeNull(); - }); - - it('names both endpoints and their years rather than passing a verdict', () => { - const s = cutoffTrendSummary(rowsFor([ - [2021, 1000], [2022, 900], [2023, 800], [2024, 500], - ])); - expect(s).toContain('2021'); - expect(s).toContain('2024'); - expect(s).toContain('tightened'); - }); - - it('does not call a small wobble a direction', () => { - const s = cutoffTrendSummary(rowsFor([ - [2021, 1000], [2022, 1010], [2023, 990], [2024, 1030], - ])); - expect(s).toContain('stayed broadly the same'); - }); -}); - -describe('cutoffCoverageNote', () => { - it('warns while the record is too thin to read as a trend', () => { - expect(cutoffCoverageNote(cutoffYearRows([d(2024, 700)]))).toContain('Only 1 year'); - expect(cutoffCoverageNote(cutoffYearRows([ - d(2021, 1000), d(2022, 900), d(2023, 800), d(2024, 700), - ]))).toBeNull(); + it('treats the band as exclusive at its edge', () => { + // Exactly on the boundary is still too close; one metre past it is not. + expect(compareToCutoff(800 - CUTOFF_UNCERTAINTY_M, 800, 2026).verdict).toBe('too-close'); + expect(compareToCutoff(800 - CUTOFF_UNCERTAINTY_M - 1, 800, 2026).verdict).toBe('inside'); }); }); describe('describeCutoffAbsence', () => { - it('explains a selective school by how it admits, not by missing data', () => { + it('explains a selective school by how it admits, not as missing data', () => { const s = describeCutoffAbsence({ localAuthority: 'Kent', admissionsPolicy: 'Selective' }); expect(s).toContain('entrance test'); expect(s).not.toContain('has not published'); @@ -195,37 +139,3 @@ describe('describeCutoffAbsence', () => { .toContain('Camden has not published'); }); }); - -describe('compareToCutoffs', () => { - const rows = cutoffYearRows( - [d(2022, 1000), d(2024, 800)], - [{ year: 2023, oversubscribed: false }], - ); - - it('marks a clearly nearer home as inside every comparable year', () => { - const r = compareToCutoffs(300, rows); - expect(r.insideCount).toBe(2); - expect(r.comparableCount).toBe(2); - expect(r.headline).toContain('inside the cut-off in all 2 years'); - }); - - it('marks a clearly further home as outside', () => { - const r = compareToCutoffs(4000, rows); - expect(r.insideCount).toBe(0); - expect(r.headline).toContain('outside the cut-off in every year'); - }); - - it('refuses to call a result inside the measurement error', () => { - // A postcode centroid covers several addresses; claiming a place on a 20 m - // margin would be inventing precision the inputs do not have. - const r = compareToCutoffs(800 - CUTOFF_UNCERTAINTY_M / 2, rows); - expect(r.years.find((y) => y.year === 2024)!.verdict).toBe('too-close'); - expect(r.detail).toContain('too close to call'); - }); - - it('never counts an unpublished year as a pass or a fail', () => { - const r = compareToCutoffs(300, rows); - expect(r.years.find((y) => y.year === 2023)!.verdict).toBe('unknown'); - expect(r.detail).toContain('no published figure'); - }); -}); diff --git a/nextjs-app/app/school/[slug]/page.tsx b/nextjs-app/app/school/[slug]/page.tsx index 547e70a..db2c229 100644 --- a/nextjs-app/app/school/[slug]/page.tsx +++ b/nextjs-app/app/school/[slug]/page.tsx @@ -147,7 +147,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { notFound(); } - const { school_info, yearly_data, absence_data, ofsted, census, admissions, admissions_history, admission_distance, admission_distance_history, deprivation, finance } = data; + const { school_info, yearly_data, absence_data, ofsted, census, admissions, admissions_history, admission_distance, deprivation, finance } = data; // Redirect bare URN to canonical slug URL const canonicalSlug = schoolUrl(urn, school_info.school_name).replace('/school/', ''); @@ -177,8 +177,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { ofsted: ofsted ?? null, admissions: admissions ?? null, admissionDistance: admission_distance ?? null, - admissionDistanceHistory: admission_distance_history ?? [], - admissionsHistory: admissions_history ?? [], + hasLocation: school_info.latitude != null && school_info.longitude != null, yearlyDataLength: yearly_data.length, }; const primaryNavItems = buildNavItems(primaryFlags, navInput); @@ -233,7 +232,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) { admissions={admissions ?? null} admissionsHistory={admissions_history ?? []} admissionDistance={admission_distance ?? null} - admissionDistanceHistory={admission_distance_history ?? []} deprivation={deprivation ?? null} finance={finance ?? null} nationalAvg={nationalAvg} @@ -256,7 +254,6 @@ export default async function SchoolPage({ params }: SchoolPageProps) { admissions={admissions ?? null} admissionsHistory={admissions_history ?? []} admissionDistance={admission_distance ?? null} - admissionDistanceHistory={admission_distance_history ?? []} deprivation={deprivation ?? null} finance={finance ?? null} nationalAvg={nationalAvg} diff --git a/nextjs-app/components/CutoffTrendChart.tsx b/nextjs-app/components/CutoffTrendChart.tsx deleted file mode 100644 index cf22a4b..0000000 --- a/nextjs-app/components/CutoffTrendChart.tsx +++ /dev/null @@ -1,123 +0,0 @@ -'use client'; - -/** - * CutoffTrendChart - * The last distance offered, year by year. - * - * Two things this chart deliberately does not do. - * - * It does not join across gaps (`spanGaps: false`). A missing year is usually a - * year the authority published nothing, and a line drawn through it would - * assert a cut-off that was never measured. - * - * It does not plot the non-distance outcomes. A year in which the school was - * not oversubscribed has no distance — that is the point of it — and giving it - * a y-position would put a number on the axis that does not exist. The mockup - * floated such years at the top of the plot; here they are a gap in the line - * and a labelled row in the table underneath, which is where a reason can be - * stated in words rather than implied by a coordinate. - */ - -import { Line } from 'react-chartjs-2'; -import { ChartOptions } from 'chart.js'; -import '@/lib/chartSetup'; -import { useThemeTokens, alpha } from '@/lib/theme'; -import type { CutoffYearRow } from './school/lastDistanceOffered'; -import styles from './AdmissionsTrendChart.module.css'; - -const METRES_PER_MILE = 1609.344; - -export default function CutoffTrendChart({ rows }: { rows: CutoffYearRow[] }) { - // rows arrive newest-first; a time axis reads oldest-first. - const axis = [...rows].reverse(); - const published = axis.filter((r) => r.distanceM != null); - if (published.length < 2) return null; - - const labels = axis.map((r) => String(r.year)); - const values: (number | null)[] = axis.map((r) => - r.distanceM != null ? r.distanceM / METRES_PER_MILE : null, - ); - - const present = values.map((v, i) => (v != null ? i : -1)).filter((i) => i >= 0); - const lastIdx = present[present.length - 1]; - - const numeric = values.filter((v): v is number => v != null); - const lo = Math.min(...numeric); - const hi = Math.max(...numeric); - // Headroom proportional to the spread, with a floor so a flat series does not - // collapse onto a single gridline and read as more precise than it is. - const pad = Math.max(0.05, (hi - lo) * 0.25); - const yMin = Math.max(0, lo - pad); - const yMax = hi + pad; - - const [cBrand, cGrid, cInverse, cInverseText, cCard] = useThemeTokens( - '--brand', '--chart-grid', '--surface-inverse', '--text-inverse', '--bg-card', - ); - - const options: ChartOptions<'line'> = { - responsive: true, - maintainAspectRatio: false, - interaction: { mode: 'index', intersect: false }, - layout: { padding: { top: 8 } }, - plugins: { - legend: { display: false }, - title: { display: false }, - tooltip: { - backgroundColor: cInverse, - titleColor: cInverseText, - bodyColor: cInverseText, - padding: 10, - titleFont: { size: 12 }, - bodyFont: { size: 12 }, - callbacks: { - label: (ctx) => - ctx.parsed.y == null ? '' : `Last distance offered: ${ctx.parsed.y.toFixed(2)} miles`, - }, - }, - }, - scales: { - y: { - min: yMin, - max: yMax, - grid: { color: cGrid }, - ticks: { - font: { size: 11 }, - maxTicksLimit: 5, - callback: (v) => `${Number(v).toFixed(2)} mi`, - }, - }, - x: { - grid: { display: false }, - ticks: { font: { size: 11 }, autoSkip: true, maxRotation: 0, autoSkipPadding: 16 }, - }, - }, - }; - - const data = { - labels, - datasets: [ - { - label: 'Last distance offered', - data: values, - clip: false as const, - spanGaps: false, - borderColor: cBrand, - backgroundColor: alpha('--brand', 0.10), - borderWidth: 2.5, - tension: 0.3, - fill: true, - pointRadius: values.map((v, i) => (v == null ? 0 : i === lastIdx ? 5 : 3)), - pointBackgroundColor: cBrand, - pointBorderColor: cCard, - pointBorderWidth: values.map((_, i) => (i === lastIdx ? 2 : 0)), - pointHoverRadius: 6, - }, - ], - }; - - return ( -
- -
- ); -} diff --git a/nextjs-app/components/school/CutoffDistanceDetail.tsx b/nextjs-app/components/school/CutoffDistanceDetail.tsx deleted file mode 100644 index f44514e..0000000 --- a/nextjs-app/components/school/CutoffDistanceDetail.tsx +++ /dev/null @@ -1,75 +0,0 @@ -/** - * CutoffDistanceDetail — the full year-by-year cut-off story. - * - * Chart, table, map and postcode check, in that order: the shape of the trend, - * then what happened in each year, then the same numbers over real streets, - * then the reader's own address measured against them. - * - * Shared by both templates because the content is identical, but placed - * differently by each. The primary page has a segmented control and gives this - * its own "Distance" tab; the secondary page is one flat panel by design, so it - * renders inline beneath the metric cards. Extracting it keeps the caveats and - * the ordering from drifting apart between the two. - * - * Server component — the map and postcode form are the only client parts, and - * they carry their own boundary. - */ - -import type { School } from '@/lib/types'; -import { sectionStyles as styles } from './sectionShared'; -import { CutoffTrendChart } from './charts'; -import { CutoffYearTable } from './CutoffYearTable'; -import { CutoffMapPanel } from './CutoffMapPanel'; -import { - cutoffTrendSummary, cutoffCoverageNote, CUTOFF_CHECK_CAVEAT, - type CutoffYearRow, -} from './lastDistanceOffered'; - -export function CutoffDistanceDetail({ - rows, - schoolInfo, -}: { - rows: CutoffYearRow[]; - schoolInfo: School; -}) { - const publishedYears = rows.filter((r) => r.status === 'published').length; - if (publishedYears < 2) return null; - - const trendSummary = cutoffTrendSummary(rows); - const coverageNote = cutoffCoverageNote(rows); - /* - * The chart appears at the same four points that let cutoffTrendSummary - * state a direction. Below that we already refuse to call the series a - * trend, and drawing a trend line under that refusal contradicts it — three - * points joined by a line say "look, it is falling" whatever the sentence - * beneath admits. The table carries every one of those years anyway, with - * the reasons a line cannot show, so nothing is lost by leaving it out. - */ - const showChart = publishedYears >= 4; - - return ( - <> - {showChart && ( - <> -
Last distance offered, by year
- -

- Gaps are years with no published figure — the line is never drawn - across one. The table says what happened in each. -

- - )} - {trendSummary &&

{trendSummary}

} - - - {coverageNote &&

{coverageNote}

} - - - - {/* The single caveat for the whole section. It lives here rather than - inside the check, so it still renders for a school with no - coordinates — where there is a table but no map and no check. */} -

{CUTOFF_CHECK_CAVEAT}

- - ); -} diff --git a/nextjs-app/components/school/CutoffMapPanel.tsx b/nextjs-app/components/school/CutoffMapPanel.tsx index 71928f5..ac70a99 100644 --- a/nextjs-app/components/school/CutoffMapPanel.tsx +++ b/nextjs-app/components/school/CutoffMapPanel.tsx @@ -1,33 +1,36 @@ 'use client'; /** - * CutoffMapPanel — "Where the last place went". + * CutoffMapPanel — "How far away are you?" * - * The rings and the postcode check live in one component because they are one - * question asked twice: the form answers "how far are we?" and the map shows - * that answer against the cut-offs. Entering a postcode drops a pin on the same - * rings rather than producing a separate verdict elsewhere on the page. + * Measures a postcode against the one cut-off we publish, and will draw that + * cut-off as a ring around the school on request. * - * The map is not rendered until asked for. Before a postcode is entered it is a + * It used to compare against every published year and show a set of shrinking + * rings. Earlier years are now held back as a paid feature and no longer leave + * the API, so this answers one question about one year — which makes the + * verdict sharper to state, and puts more weight on qualifying it properly, + * since there is no run of years left to soften a single close call. + * + * The map is not rendered until asked for: before a postcode is entered it is a * circle drawn round a school, and it costs a Leaflet bundle and 240px of - * section height to say that; a successful check opens it automatically, - * because that is the point at which it starts answering something. + * height to say that. A successful check opens it automatically, because that + * is the point at which it starts answering something. * * The postcode never leaves the browser except to postcodes.io for a lat/long, * and nothing is stored — this is a client-side measurement, not a lookup * against the family. */ -import { useState, useMemo, type FormEvent } from 'react'; +import { useState, type FormEvent } from 'react'; import dynamic from 'next/dynamic'; -import type { School } from '@/lib/types'; +import type { School, SchoolAdmissionDistance } from '@/lib/types'; import { geocodePostcode, calculateDistance } from '@/lib/api'; import { isValidPostcode } from '@/lib/utils'; import { - compareToCutoffs, - type CutoffYearRow, type CutoffCheckResult, type CutoffVerdict, + compareToCutoff, CUTOFF_CHECK_CAVEAT, + type CutoffCheckResult, type CutoffVerdict, } from './lastDistanceOffered'; -import type { CutoffRing } from '../LeafletCutoffMapInner'; import styles from './schoolSections.module.css'; const CutoffMap = dynamic(() => import('../LeafletCutoffMapInner'), { @@ -35,65 +38,35 @@ const CutoffMap = dynamic(() => import('../LeafletCutoffMapInner'), { loading: () =>