diff --git a/backend/app.py b/backend/app.py index 1eb0837..4cba056 100644 --- a/backend/app.py +++ b/backend/app.py @@ -1232,6 +1232,13 @@ async def get_place(request: Request, kind: str, slug: str, "place": {"kind": place.kind, "slug": place.slug, "name": place.name, "count": len(place.urns), "parent_authority": place.parent_authority, + # Every authority the place meaningfully sits in. SW19 is + # mostly Merton but partly Wandsworth; naming one asserts + # something false. + "authorities": [ + {"name": name, "slug": _slugify(name), "count": n} + for name, n in place.authorities + ], # Only phases that clear the threshold, so the page links # variants that exist rather than 404s. "phases": [ph for ph in ("primary", "secondary") diff --git a/backend/places.py b/backend/places.py index a9bb3e6..443acdd 100644 --- a/backend/places.py +++ b/backend/places.py @@ -30,6 +30,12 @@ class Place: name: str urns: tuple[int, ...] parent_authority: str | None # authority NAME, for the 301 target + # Every authority the place meaningfully sits in, largest first. A quarter + # of outcodes and a third of towns straddle a boundary — SW19 is mostly + # Merton but partly Wandsworth — so naming only one asserts something + # false. parent_authority stays single because a redirect needs one + # target; this is what the page shows. + authorities: tuple[tuple[str, int], ...] = () # URNs per phase, so the per-phase threshold can be applied without # re-querying. A place with 30 primaries and 2 secondaries publishes a # primary variant and no secondary one. @@ -71,6 +77,42 @@ def _phase_urns(group, publishable: set[int]) -> dict[str, tuple[int, ...]]: return out +# A place is described by an authority when it holds at least a tenth of the +# schools, and at least two. GIAS carries occasional postcode errors — EN6 +# lists two Shropshire schools among fourteen in Hertfordshire — and a bare +# "any authority present" rule would print those as though they were real. +_AUTHORITY_MIN_SHARE = 0.10 +_AUTHORITY_MIN_SCHOOLS = 2 +_AUTHORITY_MAX_SHOWN = 3 + + +def _authorities(group) -> tuple[tuple[str, int], ...]: + """Authorities this place meaningfully sits in, largest first.""" + from backend.app import EXCLUDED_FILTER_VALUES + + if "local_authority" not in group.columns: + return () + counts = group["local_authority"].dropna().value_counts() + total = int(counts.sum()) + if not total: + return () + + kept = [ + (str(name), int(n)) for name, n in counts.items() + if str(name) not in EXCLUDED_FILTER_VALUES + and n >= _AUTHORITY_MIN_SCHOOLS + and n / total >= _AUTHORITY_MIN_SHARE + ] + # A place too small or too fragmented for the share rule still names its + # largest authority, or the page would say nothing about where it is. + if not kept: + for name, n in counts.items(): + if str(name) not in EXCLUDED_FILTER_VALUES: + return ((str(name), int(n)),) + return () + return tuple(kept[:_AUTHORITY_MAX_SHOWN]) + + def _parent_authority(group) -> str | None: """The most common authority in a group — the useful 301 target. @@ -104,6 +146,7 @@ def _group(df, column: str, kind: str, publishable: set[int]) -> dict[str, Place place = Place( kind=kind, slug=slug, name=name, urns=urns, parent_authority=_parent_authority(group) if kind == "town" else None, + authorities=() if kind == "authority" else _authorities(group), phase_urns=_phase_urns(group, publishable), ) out[place.key] = place @@ -138,6 +181,7 @@ def _outcode_places(df, publishable: set[int]) -> dict[str, Place]: continue place = Place(kind="outcode", slug=str(oc).lower(), name=str(oc), urns=urns, parent_authority=_parent_authority(group), + authorities=_authorities(group), phase_urns=_phase_urns(group, publishable)) out[place.key] = place return out @@ -181,6 +225,7 @@ def _locality_places(df, publishable: set[int], continue place = Place(kind="locality", slug=slug, name=name, urns=urns, parent_authority=_parent_authority(group), + authorities=_authorities(group), phase_urns=_phase_urns(group, publishable)) out[place.key] = place return out diff --git a/backend/tests/test_places.py b/backend/tests/test_places.py index af14671..96283f4 100644 --- a/backend/tests/test_places.py +++ b/backend/tests/test_places.py @@ -229,3 +229,64 @@ def test_no_curated_locality_names_a_london_borough(): f"these are boroughs, not districts: {sorted(named)} - they already " "have an authority page covering every school" ) + + +def test_a_place_names_every_authority_it_straddles(): + """SW19 is mostly Merton but partly Wandsworth. + + A quarter of viable outcodes and a third of viable towns cross an + authority boundary, so naming only the largest asserts something false. + """ + rows = (_town(26, "London", "Merton", start=300000) + + _town(7, "London", "Wandsworth", start=400000)) + for r in rows: + r["postcode"] = "SW19 1AA" + reg = build_place_registry(_df(rows)) + + names = [n for n, _ in reg["outcode:sw19"].authorities] + assert names == ["Merton", "Wandsworth"] # largest first + assert dict(reg["outcode:sw19"].authorities)["Wandsworth"] == 7 + + +def test_the_redirect_target_stays_a_single_authority(): + # parent_authority and authorities do different jobs: a 301 needs one + # target, the page needs the truth. + rows = (_town(26, "London", "Merton", start=300000) + + _town(7, "London", "Wandsworth", start=400000)) + for r in rows: + r["postcode"] = "SW19 1AA" + reg = build_place_registry(_df(rows)) + assert reg["outcode:sw19"].parent_authority == "Merton" + + +def test_a_stray_authority_below_the_share_threshold_is_not_named(): + # GIAS carries postcode errors — EN6 lists two Shropshire schools among + # fourteen in Hertfordshire. Printing those as though real would be worse + # than omitting them. + rows = (_town(30, "Barnet", "Hertfordshire", start=300000) + + _town(1, "Barnet", "Shropshire", start=400000)) + for r in rows: + r["postcode"] = "EN6 1AA" + reg = build_place_registry(_df(rows)) + assert [n for n, _ in reg["outcode:en6"].authorities] == ["Hertfordshire"] + + +def test_a_sentinel_authority_is_never_named(): + rows = (_town(20, "London", "Merton", start=300000) + + _town(6, "London", "Does not apply", start=400000)) + for r in rows: + r["postcode"] = "SW19 1AA" + reg = build_place_registry(_df(rows)) + assert [n for n, _ in reg["outcode:sw19"].authorities] == ["Merton"] + + +def test_a_place_always_names_at_least_one_authority(): + # Even when every authority is below the share threshold, the page has to + # say where the place is. + rows = [] + for i, la in enumerate(["A", "B", "C", "D", "E", "F", "G"]): + rows += _town(1, "Fragmented", la, start=300000 + i * 100) + reg = build_place_registry(_df(rows)) + place = reg.get("town:fragmented") + assert place is not None + assert len(place.authorities) == 1 diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 35b6049..60b561d 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1885,3 +1885,29 @@ test('no page title repeats the brand', async ({ page }) => { expect(brands, `${path} repeats the brand: ${title}`).toBeLessThanOrEqual(1); } }); + +test('a place straddling a boundary names every authority it sits in', async ({ page }) => { + // A quarter of outcodes and a third of towns cross an authority boundary — + // SW19 is mostly Merton but partly Wandsworth. Naming only the largest + // asserts something false about the place. + const { places } = await (await page.request.get('/api/places')).json(); + const outcode = places.find((p: { kind: string }) => p.kind === 'outcode'); + expect(outcode).toBeTruthy(); + + // Find any place the registry reports as straddling. + let straddling: { kind: string; slug: string } | null = null; + for (const p of places.filter((p: { kind: string }) => p.kind === 'outcode').slice(0, 40)) { + const d = await (await page.request.get(`/api/places/outcode/${p.slug}`)).json(); + if ((d.place.authorities ?? []).length > 1) { straddling = p; break; } + } + test.skip(!straddling, 'no straddling outcode found in the sample'); + + const detail = await (await page.request.get( + `/api/places/outcode/${straddling!.slug}`)).json(); + await page.goto(`/schools/near/${straddling!.slug}`); + + for (const a of detail.place.authorities) { + await expect(page.locator(`a[href="/schools/authority/${a.slug}"]`).first()) + .toBeVisible(); + } +}); diff --git a/nextjs-app/__tests__/components/PlaceView.test.tsx b/nextjs-app/__tests__/components/PlaceView.test.tsx index aeec09e..f85b624 100644 --- a/nextjs-app/__tests__/components/PlaceView.test.tsx +++ b/nextjs-app/__tests__/components/PlaceView.test.tsx @@ -199,3 +199,47 @@ describe('PlaceView table alignment', () => { expect(container.querySelectorAll('th')[0].className).toBe(''); }); }); + +describe('PlaceView authorities', () => { + const straddling: PlaceDetail = { + place: { kind: 'outcode', slug: 'sw19', name: 'SW19', count: 33, + parent_authority: 'Merton', phases: ['primary'], + authorities: [ + { name: 'Merton', slug: 'merton', count: 26 }, + { name: 'Wandsworth', slug: 'wandsworth', count: 7 }, + ] }, + schools: [ + { urn: 1, school_name: 'Alpha Primary', phase: 'Primary', + rwm_expected_pct: 82, attainment_8_score: null } as never, + ], + averages: { rwm_expected_pct: 63, attainment_8_score: null }, + }; + + it('names every authority the place straddles, not just the largest', () => { + // SW19 is mostly Merton but partly Wandsworth. Naming one asserts + // something false about a quarter of outcodes. + render(); + expect(screen.getByRole('link', { name: 'Merton' })) + .toHaveAttribute('href', '/schools/authority/merton'); + expect(screen.getByRole('link', { name: 'Wandsworth' })) + .toHaveAttribute('href', '/schools/authority/wandsworth'); + }); + + it('joins them readably rather than as a bare list', () => { + // Asserted on the summary line's whole text: a loose /and/ matcher also + // hits "Wandsworth". + const { container } = render(); + const summary = container.querySelector('header p'); + expect(summary?.textContent).toContain('Merton and Wandsworth'); + }); + + it('falls back to the single parent when the field is absent', () => { + // A cached API response predating the authorities field must not blank + // the line entirely. + const legacy = { ...straddling, + place: { ...straddling.place, authorities: undefined } }; + render(); + expect(screen.getByRole('link', { name: 'Merton' })).toBeInTheDocument(); + }); +}); diff --git a/nextjs-app/components/places/PlaceView.tsx b/nextjs-app/components/places/PlaceView.tsx index a8efc5d..4a3ea5e 100644 --- a/nextjs-app/components/places/PlaceView.tsx +++ b/nextjs-app/components/places/PlaceView.tsx @@ -106,6 +106,13 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) { const { place, schools, averages } = detail; + // Fall back to the single parent when the API predates the authorities + // field, so a stale cache never blanks the line entirely. + const authorities = place.authorities?.length + ? place.authorities + : place.parent_authority + ? [{ name: place.parent_authority, slug: authoritySlug(place.parent_authority), count: 0 }] + : []; const local = averages[METRICS[phase ?? 'primary'].key]; const phaseWord = phase === 'secondary' ? 'Secondary schools' : phase === 'primary' ? 'Primary schools' : 'Schools'; @@ -163,13 +170,20 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)

{phaseWord} in {place.name}

{place.count} schools - {place.parent_authority && ( + {authorities.length > 0 && ( <> {' · '} - - {place.parent_authority} - + {/* Every authority, not just the largest. A quarter of outcodes + and a third of towns cross a boundary: SW19 is mostly Merton + but partly Wandsworth, and naming one asserts otherwise. */} + {authorities.map((a, i) => ( + + {i > 0 && (i === authorities.length - 1 ? ' and ' : ', ')} + + {a.name} + + + ))} )}

diff --git a/nextjs-app/lib/places.ts b/nextjs-app/lib/places.ts index d1be648..183efb2 100644 --- a/nextjs-app/lib/places.ts +++ b/nextjs-app/lib/places.ts @@ -18,8 +18,19 @@ export interface PlaceSummary { phases?: string[]; } +export interface PlaceAuthority { + name: string; + slug: string; + count: number; +} + export interface PlaceDetail { - place: PlaceSummary & { parent_authority: string | null }; + place: PlaceSummary & { + parent_authority: string | null; + /** Every authority the place meaningfully sits in, largest first. SW19 is + * mostly Merton but partly Wandsworth. */ + authorities?: PlaceAuthority[]; + }; schools: School[]; averages: { rwm_expected_pct: number | null;