Compare commits
20
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d1358cc00f | ||
|
|
865a69b54d | ||
|
|
9cc87c41bb | ||
|
|
8967966eef | ||
|
|
4a9a5c734b | ||
|
|
4e82e6c916 | ||
|
|
d4340a8fdd | ||
|
|
bb2f7a5841 | ||
|
|
1cb5314c53 | ||
|
|
4cea26b813 | ||
|
|
dbb74d9b60 | ||
|
|
9545aec7f4 | ||
|
|
3365ebcb3a | ||
|
|
6d79bd3331 | ||
|
|
24e114dee7 | ||
|
|
6c5db0c266 | ||
|
|
6f749ed21f | ||
|
|
d423826840 | ||
|
|
d3c63ccc6d | ||
|
|
b93eb3a691 |
No files matched your search
+52
-11
@@ -209,12 +209,27 @@ def _place_url(place) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]:
|
def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]:
|
||||||
return [
|
"""A <url> per place, plus a phase variant wherever that phase clears the
|
||||||
_url_element(BASE_URL + _place_url(p))
|
threshold on its own.
|
||||||
for p in sorted(get_place_registry().values(),
|
|
||||||
key=lambda p: (p.kind, p.slug))
|
Phase is part of the query — "primary schools in beccles" — so each
|
||||||
if p.kind in kinds
|
variant is its own indexable page. Submitting only the bare place URL left
|
||||||
]
|
~950 of them reachable by nothing: absent from every sitemap, and not
|
||||||
|
linked from the place page either.
|
||||||
|
"""
|
||||||
|
rows: list[str] = []
|
||||||
|
for p in sorted(get_place_registry().values(), key=lambda p: (p.kind, p.slug)):
|
||||||
|
if p.kind not in kinds:
|
||||||
|
continue
|
||||||
|
rows.append(_url_element(BASE_URL + _place_url(p)))
|
||||||
|
# Which phases a place publishes is the registry's decision alone —
|
||||||
|
# outcodes report none, because the spec gives them no phase route.
|
||||||
|
# Repeating that rule here was how the page and the sitemap came to
|
||||||
|
# disagree about which URLs exist.
|
||||||
|
for phase in ("primary", "secondary"):
|
||||||
|
if p.publishes_phase(phase):
|
||||||
|
rows.append(_url_element(f"{BASE_URL}{_place_url(p)}/{phase}"))
|
||||||
|
return rows
|
||||||
|
|
||||||
|
|
||||||
def build_sitemaps() -> dict[str, str]:
|
def build_sitemaps() -> dict[str, str]:
|
||||||
@@ -1185,7 +1200,8 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
if kind not in VALID_PLACE_KINDS:
|
if kind not in VALID_PLACE_KINDS:
|
||||||
raise HTTPException(status_code=404, detail="No such place")
|
raise HTTPException(status_code=404, detail="No such place")
|
||||||
|
|
||||||
place = get_place_registry().get(f"{kind}:{slug}")
|
registry = get_place_registry()
|
||||||
|
place = registry.get(f"{kind}:{slug}")
|
||||||
if place is None:
|
if place is None:
|
||||||
raise HTTPException(status_code=404, detail="No such place")
|
raise HTTPException(status_code=404, detail="No such place")
|
||||||
|
|
||||||
@@ -1197,10 +1213,15 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
if wanted and "phase" in rows.columns:
|
if wanted and "phase" in rows.columns:
|
||||||
rows = rows[rows["phase"].fillna("").str.lower().isin(wanted)]
|
rows = rows[rows["phase"].fillna("").str.lower().isin(wanted)]
|
||||||
|
|
||||||
# The metric the page ranks on, which is also the one it averages.
|
# The metric the page shows, and averages.
|
||||||
metric = "attainment_8_score" if phase == "secondary" else "rwm_expected_pct"
|
metric = "attainment_8_score" if phase == "secondary" else "rwm_expected_pct"
|
||||||
if metric in rows.columns:
|
|
||||||
rows = rows.sort_values(metric, ascending=False, na_position="last")
|
# Alphabetical, not by score. A place page is read by someone looking for
|
||||||
|
# a school they can name, and scanning for it is what the order should
|
||||||
|
# serve. /rankings is where the league-table ordering lives, and it keeps
|
||||||
|
# sorting by metric.
|
||||||
|
if "school_name" in rows.columns:
|
||||||
|
rows = rows.sort_values("school_name", key=lambda c: c.str.lower())
|
||||||
|
|
||||||
averages = {
|
averages = {
|
||||||
m: (None if m not in rows.columns or rows[m].dropna().empty
|
m: (None if m not in rows.columns or rows[m].dropna().empty
|
||||||
@@ -1216,7 +1237,27 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
return {
|
return {
|
||||||
"place": {"kind": place.kind, "slug": place.slug, "name": place.name,
|
"place": {"kind": place.kind, "slug": place.slug, "name": place.name,
|
||||||
"count": len(place.urns),
|
"count": len(place.urns),
|
||||||
"parent_authority": place.parent_authority},
|
"parent_authority": place.parent_authority,
|
||||||
|
# Every authority the place meaningfully sits in. SW19 is
|
||||||
|
# mostly Merton but partly Wandsworth; naming one asserts
|
||||||
|
# something false.
|
||||||
|
#
|
||||||
|
# The slug is null where that authority has no page of its
|
||||||
|
# own: City of London and the Isles of Scilly hold fewer
|
||||||
|
# schools than the threshold. Naming them is still right;
|
||||||
|
# linking them would be a 404.
|
||||||
|
"authorities": [
|
||||||
|
{"name": name,
|
||||||
|
"slug": (_slugify(name)
|
||||||
|
if f"authority:{_slugify(name)}" in registry
|
||||||
|
else None),
|
||||||
|
"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")
|
||||||
|
if place.publishes_phase(ph)]},
|
||||||
"schools": clean_for_json(rows[cols]),
|
"schools": clean_for_json(rows[cols]),
|
||||||
"averages": averages,
|
"averages": averages,
|
||||||
}
|
}
|
||||||
|
|||||||
+12
-5
@@ -20,6 +20,18 @@ A locality whose outcodes hold fewer than MIN_SCHOOLS schools is not
|
|||||||
published, so a typo produces no page rather than an empty one. Places that
|
published, so a typo produces no page rather than an empty one. Places that
|
||||||
fail that check are logged at startup, because a locality you meant to publish
|
fail that check are logged at startup, because a locality you meant to publish
|
||||||
quietly not appearing is the failure worth hearing about.
|
quietly not appearing is the failure worth hearing about.
|
||||||
|
|
||||||
|
Two rules for anything added here.
|
||||||
|
|
||||||
|
**Sub-borough districts only.** A London borough is a local authority and
|
||||||
|
already has a page at /schools/authority/[la] covering all of its schools; a
|
||||||
|
locality defined by two or three outcodes would be a partial, near-duplicate
|
||||||
|
subset of it. Hackney, Islington, Greenwich and Ealing were all in the first
|
||||||
|
draft for that reason and have been removed.
|
||||||
|
|
||||||
|
**The slug must not match a GIAS town.** "Richmond" did — GIAS has a Richmond
|
||||||
|
in North Yorkshire with 37 schools — so the London one could never publish.
|
||||||
|
The registry skips any locality that collides and logs it.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
# slug -> (display name, outcodes)
|
# slug -> (display name, outcodes)
|
||||||
@@ -30,16 +42,11 @@ LOCALITY_OUTCODES: dict[str, tuple[str, tuple[str, ...]]] = {
|
|||||||
"shoreditch": ("Shoreditch", ("EC2A", "E1")),
|
"shoreditch": ("Shoreditch", ("EC2A", "E1")),
|
||||||
"peckham": ("Peckham", ("SE15",)),
|
"peckham": ("Peckham", ("SE15",)),
|
||||||
"brixton": ("Brixton", ("SW2", "SW9")),
|
"brixton": ("Brixton", ("SW2", "SW9")),
|
||||||
"hackney": ("Hackney", ("E5", "E8", "E9")),
|
|
||||||
"islington": ("Islington", ("N1", "N5", "N7")),
|
|
||||||
"camden-town": ("Camden Town", ("NW1",)),
|
"camden-town": ("Camden Town", ("NW1",)),
|
||||||
"greenwich": ("Greenwich", ("SE10",)),
|
|
||||||
"wimbledon": ("Wimbledon", ("SW19",)),
|
"wimbledon": ("Wimbledon", ("SW19",)),
|
||||||
"putney": ("Putney", ("SW15",)),
|
"putney": ("Putney", ("SW15",)),
|
||||||
"fulham": ("Fulham", ("SW6",)),
|
"fulham": ("Fulham", ("SW6",)),
|
||||||
"chiswick": ("Chiswick", ("W4",)),
|
"chiswick": ("Chiswick", ("W4",)),
|
||||||
"ealing": ("Ealing", ("W5", "W13")),
|
|
||||||
"richmond": ("Richmond", ("TW9", "TW10")),
|
|
||||||
"stratford": ("Stratford", ("E15",)),
|
"stratford": ("Stratford", ("E15",)),
|
||||||
"walthamstow": ("Walthamstow", ("E17",)),
|
"walthamstow": ("Walthamstow", ("E17",)),
|
||||||
"tooting": ("Tooting", ("SW17",)),
|
"tooting": ("Tooting", ("SW17",)),
|
||||||
|
|||||||
+170
-23
@@ -14,7 +14,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import re
|
import re
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass, field
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -30,6 +30,19 @@ class Place:
|
|||||||
name: str
|
name: str
|
||||||
urns: tuple[int, ...]
|
urns: tuple[int, ...]
|
||||||
parent_authority: str | None # authority NAME, for the 301 target
|
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.
|
||||||
|
phase_urns: dict[str, tuple[int, ...]] = field(default_factory=dict)
|
||||||
|
|
||||||
|
def publishes_phase(self, phase: str) -> bool:
|
||||||
|
return len(self.phase_urns.get(phase, ())) >= MIN_SCHOOLS
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def key(self) -> str:
|
def key(self) -> str:
|
||||||
@@ -46,39 +59,152 @@ def _publishable_urns(df) -> set[int]:
|
|||||||
return set(df.loc[df[cols].notna().any(axis=1), "urn"].astype(int))
|
return set(df.loc[df[cols].notna().any(axis=1), "urn"].astype(int))
|
||||||
|
|
||||||
|
|
||||||
def _parent_authority(group) -> str | None:
|
# The measure a phase page is built around. A page with no results in this
|
||||||
"""The most common authority in a group — the useful 301 target.
|
# column has nothing a list of school names does not already give.
|
||||||
|
_PHASE_METRIC = {
|
||||||
|
"primary": "rwm_expected_pct",
|
||||||
|
"secondary": "attainment_8_score",
|
||||||
|
}
|
||||||
|
|
||||||
A town spanning several authorities has no single parent, so the mode is
|
|
||||||
the honest answer rather than an arbitrary first row.
|
def _phase_urns(group, publishable: set[int]) -> dict[str, tuple[int, ...]]:
|
||||||
|
"""URNs per phase, counting only schools with a result for that phase.
|
||||||
|
|
||||||
|
Not merely "publishable". A school with an Ofsted grade and no results is
|
||||||
|
worth a page of its own and belongs in the place list, but it cannot
|
||||||
|
populate a phase page's results column — and the threshold is there to ask
|
||||||
|
whether that column will have anything in it.
|
||||||
|
|
||||||
|
Counting publishable schools instead let /schools/kent/primary publish
|
||||||
|
with none of its five rows carrying a result, and left 44 phase pages
|
||||||
|
majority-blank. It is the same rule as "no page without a local average",
|
||||||
|
which was never extended per phase.
|
||||||
|
|
||||||
|
All-through schools count toward both phases, matching the PHASE_GROUPS
|
||||||
|
mapping the search filters already use.
|
||||||
"""
|
"""
|
||||||
|
from backend.app import PHASE_GROUPS
|
||||||
|
|
||||||
|
if "phase" not in group.columns:
|
||||||
|
return {}
|
||||||
|
lowered = group["phase"].fillna("").str.lower()
|
||||||
|
|
||||||
|
out: dict[str, tuple[int, ...]] = {}
|
||||||
|
for phase in ("primary", "secondary"):
|
||||||
|
wanted = PHASE_GROUPS.get(phase, set())
|
||||||
|
subset = group[lowered.isin(wanted)]
|
||||||
|
|
||||||
|
# The page lists every school of the phase; the threshold counts only
|
||||||
|
# those carrying a result, so a mostly-empty table never publishes.
|
||||||
|
metric = _PHASE_METRIC[phase]
|
||||||
|
with_result = (
|
||||||
|
{int(u) for u in subset.loc[subset[metric].notna(), "urn"]}
|
||||||
|
if metric in subset.columns else set()
|
||||||
|
)
|
||||||
|
if len(with_result & publishable) < MIN_SCHOOLS:
|
||||||
|
continue
|
||||||
|
|
||||||
|
urns = tuple(sorted({int(u) for u in subset["urn"]} & publishable))
|
||||||
|
if urns:
|
||||||
|
out[phase] = urns
|
||||||
|
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.
|
||||||
|
# There is deliberately no cap on how many are named. An earlier cut stopped
|
||||||
|
# at three, which silently dropped the fourth in exactly the case where the
|
||||||
|
# information matters most — a genuinely fragmented place. The share rule is
|
||||||
|
# the only limit, and it already bounds the list at ten.
|
||||||
|
_AUTHORITY_MIN_SHARE = 0.10
|
||||||
|
_AUTHORITY_MIN_SCHOOLS = 2
|
||||||
|
|
||||||
|
|
||||||
|
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:
|
if "local_authority" not in group.columns:
|
||||||
return None
|
return ()
|
||||||
top = group["local_authority"].dropna()
|
counts = group["local_authority"].dropna().value_counts()
|
||||||
return str(top.mode().iloc[0]) if not top.empty else None
|
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)
|
||||||
|
|
||||||
|
|
||||||
|
def _parent_authority(authorities: tuple[tuple[str, int], ...]) -> str | None:
|
||||||
|
"""The 301 target: the largest authority a place sits in.
|
||||||
|
|
||||||
|
Derived from `authorities` rather than computed separately. The first cut
|
||||||
|
used `mode()` here while `authorities` used `value_counts()`, and on an
|
||||||
|
exact tie pandas does not guarantee the two pick the same name — so the
|
||||||
|
redirect could have pointed somewhere other than the authority the page
|
||||||
|
named first. One computation, one answer.
|
||||||
|
|
||||||
|
Deriving it also inherits the sentinel filter, so a place can no longer
|
||||||
|
redirect to /schools/authority/does-not-apply.
|
||||||
|
"""
|
||||||
|
return authorities[0][0] if authorities else None
|
||||||
|
|
||||||
|
|
||||||
def _group(df, column: str, kind: str, publishable: set[int]) -> dict[str, Place]:
|
def _group(df, column: str, kind: str, publishable: set[int]) -> dict[str, Place]:
|
||||||
"""One Place per distinct value of `column` that clears the threshold."""
|
"""One Place per distinct SLUG in `column` that clears the threshold.
|
||||||
|
|
||||||
|
Grouped by slug, not by raw value, because GIAS spells the same place
|
||||||
|
several ways and they all resolve to one URL. Five town slugs come from
|
||||||
|
more than one spelling: "London" (1,819 schools) and "LONDON" (12) both
|
||||||
|
slugify to `london`; Weston-super-Mare is split 14/19 across two
|
||||||
|
spellings; Newcastle-under-Lyme across three.
|
||||||
|
|
||||||
|
Grouping by raw value meant the later group simply overwrote the earlier
|
||||||
|
one in this dict — so /schools/london could have shown twelve schools
|
||||||
|
instead of 1,819, silently and depending on row order.
|
||||||
|
|
||||||
|
The display name is the most common spelling, which is the one a reader
|
||||||
|
expects to see.
|
||||||
|
"""
|
||||||
from backend.app import _slugify
|
from backend.app import _slugify
|
||||||
|
|
||||||
if column not in df.columns:
|
if column not in df.columns:
|
||||||
return {}
|
return {}
|
||||||
|
|
||||||
|
working = df.assign(_slug=df[column].map(
|
||||||
|
lambda v: _slugify(str(v).strip()) if isinstance(v, str) and v.strip() else None))
|
||||||
|
working = working[working["_slug"].notna() & (working["_slug"] != "")]
|
||||||
|
|
||||||
out: dict[str, Place] = {}
|
out: dict[str, Place] = {}
|
||||||
for name, group in df.groupby(column, dropna=True):
|
for slug, group in working.groupby("_slug"):
|
||||||
name = str(name).strip()
|
slug = str(slug)
|
||||||
if not name:
|
|
||||||
continue
|
|
||||||
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
||||||
if len(urns) < MIN_SCHOOLS:
|
if len(urns) < MIN_SCHOOLS:
|
||||||
continue
|
continue
|
||||||
slug = _slugify(name)
|
spellings = group[column].dropna().value_counts()
|
||||||
if not slug:
|
if spellings.empty:
|
||||||
continue
|
continue
|
||||||
|
name = str(spellings.index[0]).strip()
|
||||||
|
authorities = () if kind == "authority" else _authorities(group)
|
||||||
place = Place(
|
place = Place(
|
||||||
kind=kind, slug=slug, name=name, urns=urns,
|
kind=kind, slug=slug, name=name, urns=urns,
|
||||||
parent_authority=_parent_authority(group) if kind == "town" else None,
|
parent_authority=_parent_authority(authorities),
|
||||||
|
authorities=authorities,
|
||||||
|
phase_urns=_phase_urns(group, publishable),
|
||||||
)
|
)
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
@@ -98,7 +224,14 @@ def _outcode(postcode) -> str | None:
|
|||||||
def _outcode_places(df, publishable: set[int]) -> dict[str, Place]:
|
def _outcode_places(df, publishable: set[int]) -> dict[str, Place]:
|
||||||
"""One Place per postcode district clearing the threshold.
|
"""One Place per postcode district clearing the threshold.
|
||||||
|
|
||||||
These carry no phase variants: nobody searches "primary schools in SW11".
|
These carry no phase variants: nobody searches "primary schools in SW11",
|
||||||
|
so the spec gives them no /primary or /secondary route. `phase_urns` is
|
||||||
|
left empty rather than computed and then filtered downstream — the
|
||||||
|
registry is the one place that decides which phases a place publishes,
|
||||||
|
and the page links whatever it reports.
|
||||||
|
|
||||||
|
Computing them here put a link to a route that does not exist on every one
|
||||||
|
of the 1,720 outcode pages.
|
||||||
"""
|
"""
|
||||||
if "postcode" not in df.columns:
|
if "postcode" not in df.columns:
|
||||||
return {}
|
return {}
|
||||||
@@ -110,8 +243,10 @@ def _outcode_places(df, publishable: set[int]) -> dict[str, Place]:
|
|||||||
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
||||||
if len(urns) < MIN_SCHOOLS:
|
if len(urns) < MIN_SCHOOLS:
|
||||||
continue
|
continue
|
||||||
|
authorities = _authorities(group)
|
||||||
place = Place(kind="outcode", slug=str(oc).lower(), name=str(oc),
|
place = Place(kind="outcode", slug=str(oc).lower(), name=str(oc),
|
||||||
urns=urns, parent_authority=_parent_authority(group))
|
urns=urns, parent_authority=_parent_authority(authorities),
|
||||||
|
authorities=authorities)
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
|
|
||||||
@@ -128,10 +263,19 @@ def _locality_places(df, publishable: set[int],
|
|||||||
out: dict[str, Place] = {}
|
out: dict[str, Place] = {}
|
||||||
for slug, (name, outcodes) in LOCALITY_OUTCODES.items():
|
for slug, (name, outcodes) in LOCALITY_OUTCODES.items():
|
||||||
if slug in town_slugs:
|
if slug in town_slugs:
|
||||||
raise ValueError(
|
# Skip, do not raise. The guard exists so a locality never
|
||||||
f"locality {slug!r} collides with a published town of the same "
|
# silently shadows a town — skipping achieves that, and the error
|
||||||
"slug; publishing both would shadow the town silently"
|
# log makes it loud.
|
||||||
)
|
#
|
||||||
|
# Raising here took down sitemap generation for all 25,000 school
|
||||||
|
# pages when "richmond" met the GIAS town Richmond in North
|
||||||
|
# Yorkshire. Worse, GIAS town names change without any code change,
|
||||||
|
# so a raise means curated data can break the site spontaneously.
|
||||||
|
# A curation mistake must cost one page, not the sitemap.
|
||||||
|
logger.error(
|
||||||
|
"locality %r collides with the published town of the same "
|
||||||
|
"slug and has been skipped; rename it or remove it", slug)
|
||||||
|
continue
|
||||||
group = working[working["_oc"].isin(outcodes)]
|
group = working[working["_oc"].isin(outcodes)]
|
||||||
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
||||||
if len(urns) < MIN_SCHOOLS:
|
if len(urns) < MIN_SCHOOLS:
|
||||||
@@ -143,8 +287,11 @@ def _locality_places(df, publishable: set[int],
|
|||||||
"threshold of %d - not published",
|
"threshold of %d - not published",
|
||||||
slug, ", ".join(outcodes), len(urns), MIN_SCHOOLS)
|
slug, ", ".join(outcodes), len(urns), MIN_SCHOOLS)
|
||||||
continue
|
continue
|
||||||
|
authorities = _authorities(group)
|
||||||
place = Place(kind="locality", slug=slug, name=name, urns=urns,
|
place = Place(kind="locality", slug=slug, name=name, urns=urns,
|
||||||
parent_authority=_parent_authority(group))
|
parent_authority=_parent_authority(authorities),
|
||||||
|
authorities=authorities,
|
||||||
|
phase_urns=_phase_urns(group, publishable))
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|||||||
@@ -108,16 +108,42 @@ def test_locality_below_the_threshold_is_not_published(monkeypatch):
|
|||||||
assert "locality:nowhere" not in reg
|
assert "locality:nowhere" not in reg
|
||||||
|
|
||||||
|
|
||||||
def test_a_locality_may_not_shadow_a_viable_town(monkeypatch):
|
def test_a_locality_may_not_shadow_a_viable_town(monkeypatch, caplog):
|
||||||
# Silently shadowing a town would lose a page carrying real demand.
|
"""A colliding locality is skipped loudly, and the town survives.
|
||||||
|
|
||||||
|
This used to raise, which took down sitemap generation for all 25,000
|
||||||
|
school pages the first time a curated slug met a real GIAS town. Curated
|
||||||
|
data must not be able to break the site — and GIAS town names change with
|
||||||
|
no code change at all, so the raise could fire spontaneously.
|
||||||
|
"""
|
||||||
|
import logging
|
||||||
|
|
||||||
from backend import localities
|
from backend import localities
|
||||||
monkeypatch.setattr(localities, "LOCALITY_OUTCODES",
|
monkeypatch.setattr(localities, "LOCALITY_OUTCODES",
|
||||||
{"brentwood": ("Brentwood", ("CM13",))})
|
{"brentwood": ("Brentwood", ("CM13",))})
|
||||||
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
||||||
for r in rows:
|
for r in rows:
|
||||||
r["postcode"] = "CM13 1AA"
|
r["postcode"] = "CM13 1AA"
|
||||||
with pytest.raises(ValueError, match="brentwood"):
|
|
||||||
build_place_registry(_df(rows))
|
with caplog.at_level(logging.ERROR):
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
|
||||||
|
assert "locality:brentwood" not in reg # skipped
|
||||||
|
assert "town:brentwood" in reg # the town is untouched
|
||||||
|
assert "brentwood" in caplog.text # and it was loud about it
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_locality_collision_does_not_break_the_rest_of_the_registry(monkeypatch):
|
||||||
|
# The whole point of skipping rather than raising.
|
||||||
|
from backend import localities
|
||||||
|
monkeypatch.setattr(localities, "LOCALITY_OUTCODES",
|
||||||
|
{"brentwood": ("Brentwood", ("CM13",))})
|
||||||
|
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
||||||
|
for r in rows:
|
||||||
|
r["postcode"] = "CM13 1AA"
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert "authority:essex" in reg
|
||||||
|
assert "outcode:cm13" in reg
|
||||||
|
|
||||||
|
|
||||||
def test_outcode_places_are_built_from_postcodes():
|
def test_outcode_places_are_built_from_postcodes():
|
||||||
@@ -174,3 +200,221 @@ def test_the_pipeline_seed_mirrors_the_canonical_module():
|
|||||||
for row in csv.DictReader(seed_path.open())
|
for row in csv.DictReader(seed_path.open())
|
||||||
}
|
}
|
||||||
assert seed == LOCALITY_OUTCODES
|
assert seed == LOCALITY_OUTCODES
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_curated_locality_names_a_london_borough():
|
||||||
|
"""Boroughs are authorities and already have a page.
|
||||||
|
|
||||||
|
A locality defined by two or three outcodes inside a borough would be a
|
||||||
|
partial, near-duplicate subset of that authority page — the exact
|
||||||
|
thin-content failure the two-namespace design exists to avoid. Hackney,
|
||||||
|
Islington, Greenwich and Ealing were all in the first draft.
|
||||||
|
|
||||||
|
Hardcoded rather than read from the corpus because this must fail in CI,
|
||||||
|
where there is no database.
|
||||||
|
"""
|
||||||
|
from backend.localities import LOCALITY_OUTCODES
|
||||||
|
|
||||||
|
boroughs = {
|
||||||
|
"barking-and-dagenham", "barnet", "bexley", "brent", "bromley",
|
||||||
|
"camden", "croydon", "ealing", "enfield", "greenwich", "hackney",
|
||||||
|
"hammersmith-and-fulham", "haringey", "harrow", "havering",
|
||||||
|
"hillingdon", "hounslow", "islington", "kensington-and-chelsea",
|
||||||
|
"kingston-upon-thames", "lambeth", "lewisham", "merton", "newham",
|
||||||
|
"redbridge", "richmond-upon-thames", "southwark", "sutton",
|
||||||
|
"tower-hamlets", "waltham-forest", "wandsworth", "westminster",
|
||||||
|
}
|
||||||
|
named = boroughs & set(LOCALITY_OUTCODES)
|
||||||
|
assert not named, (
|
||||||
|
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
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_qualifying_authority_is_named_with_no_cap():
|
||||||
|
"""An earlier cut stopped at three, dropping the fourth silently.
|
||||||
|
|
||||||
|
That truncation bit exactly where the information matters most — a
|
||||||
|
genuinely fragmented place — and nothing recorded it.
|
||||||
|
"""
|
||||||
|
rows = []
|
||||||
|
for i, la in enumerate(["Hackney", "Lambeth", "Westminster", "Lewisham"]):
|
||||||
|
rows += _town(3, "Fourway", la, start=300000 + i * 100)
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert len(reg["town:fourway"].authorities) == 4
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_redirect_target_is_the_authority_named_first():
|
||||||
|
"""They were computed separately — mode() against value_counts() — and on
|
||||||
|
an exact tie pandas does not guarantee the two agree."""
|
||||||
|
rows = (_town(26, "London", "Merton", start=300000)
|
||||||
|
+ _town(7, "London", "Wandsworth", start=400000))
|
||||||
|
for r in rows:
|
||||||
|
r["postcode"] = "SW19 1AA"
|
||||||
|
place = build_place_registry(_df(rows))["outcode:sw19"]
|
||||||
|
assert place.parent_authority == place.authorities[0][0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_place_never_redirects_to_a_sentinel_authority():
|
||||||
|
# Deriving the parent from `authorities` inherits its sentinel filter.
|
||||||
|
rows = (_town(6, "Someplace", "Does not apply", start=300000)
|
||||||
|
+ _town(5, "Someplace", "Essex", start=400000))
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert reg["town:someplace"].parent_authority == "Essex"
|
||||||
|
|
||||||
|
|
||||||
|
def test_spellings_of_one_place_are_merged_not_overwritten():
|
||||||
|
"""GIAS spells the same place several ways, and they share a URL.
|
||||||
|
|
||||||
|
"London" (1,819 schools) and "LONDON" (12) both slugify to `london`.
|
||||||
|
Grouping by raw value let the later group overwrite the earlier one, so
|
||||||
|
the page could have shown twelve schools instead of 1,819 — silently, and
|
||||||
|
depending on row order.
|
||||||
|
"""
|
||||||
|
rows = (_town(6, "Weston-super-Mare", "North Somerset", start=300000)
|
||||||
|
+ _town(5, "Weston-Super-Mare", "North Somerset", start=400000))
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert len(reg["town:weston-super-mare"].urns) == 11
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_merged_place_takes_its_most_common_spelling():
|
||||||
|
rows = (_town(9, "Newcastle-under-Lyme", "Staffordshire", start=300000)
|
||||||
|
+ _town(5, "NEWCASTLE-UNDER-LYME", "Staffordshire", start=400000))
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert reg["town:newcastle-under-lyme"].name == "Newcastle-under-Lyme"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_phase_page_needs_results_not_merely_publishable_schools():
|
||||||
|
"""/schools/kent/primary published with none of its five rows scored.
|
||||||
|
|
||||||
|
The threshold counted schools that were publishable — a result OR an
|
||||||
|
Ofsted grade — while the page exists for its results column. Forty-four
|
||||||
|
phase pages were majority-blank; one had no results at all.
|
||||||
|
"""
|
||||||
|
rows = _town(MIN_SCHOOLS, "Kent", "Kent")
|
||||||
|
for r in rows:
|
||||||
|
r["rwm_expected_pct"] = np.nan # Ofsted only, no results
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
|
||||||
|
assert "town:kent" in reg # the place still publishes
|
||||||
|
assert not reg["town:kent"].publishes_phase("primary")
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_phase_page_publishes_once_enough_schools_carry_a_result():
|
||||||
|
rows = _town(MIN_SCHOOLS, "Beccles", "Suffolk")
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert reg["town:beccles"].publishes_phase("primary")
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_publishing_phase_page_still_lists_its_unscored_schools():
|
||||||
|
"""The threshold gates whether the page exists; it does not filter rows.
|
||||||
|
|
||||||
|
A parent looking up a school by name has to find it whether or not it
|
||||||
|
published results.
|
||||||
|
"""
|
||||||
|
scored = _town(MIN_SCHOOLS, "Beccles", "Suffolk", start=300000)
|
||||||
|
unscored = _town(2, "Beccles", "Suffolk", start=400000)
|
||||||
|
for r in unscored:
|
||||||
|
r["rwm_expected_pct"] = np.nan
|
||||||
|
reg = build_place_registry(_df(scored + unscored))
|
||||||
|
|
||||||
|
place = reg["town:beccles"]
|
||||||
|
assert place.publishes_phase("primary")
|
||||||
|
assert len(place.phase_urns["primary"]) == MIN_SCHOOLS + 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_secondary_threshold_counts_its_own_metric():
|
||||||
|
# A town full of scored primaries must not thereby publish a secondary page.
|
||||||
|
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
assert not reg["town:brentwood"].publishes_phase("secondary")
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_outcode_publishes_no_phase_variants():
|
||||||
|
"""There is no /schools/near/[outcode]/[phase] route, by design.
|
||||||
|
|
||||||
|
Nobody searches "primary schools in SW11", so the spec gives outcodes no
|
||||||
|
phase variants. The registry computed them anyway, and the place page —
|
||||||
|
which links whatever phases the registry reports — put two 404s on every
|
||||||
|
outcode page in the site.
|
||||||
|
|
||||||
|
This is the single rule now: a kind with no phase route reports no phases,
|
||||||
|
so neither the page nor the sitemap can offer one.
|
||||||
|
"""
|
||||||
|
rows = [{"urn": 500000 + i, "school_name": f"SW11 School {i}",
|
||||||
|
"town": "London", "local_authority": "Wandsworth",
|
||||||
|
"postcode": "SW11 1AA"} for i in range(MIN_SCHOOLS + 3)]
|
||||||
|
reg = build_place_registry(_df(rows))
|
||||||
|
|
||||||
|
place = reg["outcode:sw11"]
|
||||||
|
assert place.phase_urns == {}
|
||||||
|
assert not place.publishes_phase("primary")
|
||||||
|
assert not place.publishes_phase("secondary")
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_authority_still_publishes_phase_variants():
|
||||||
|
"""Authorities keep theirs — "primary schools in Kent" is a real query,
|
||||||
|
and /schools/authority/[la]/[phase] is the route that serves it."""
|
||||||
|
reg = build_place_registry(_df(_town(MIN_SCHOOLS, "Maidstone", "Kent")))
|
||||||
|
assert reg["authority:kent"].publishes_phase("primary")
|
||||||
@@ -45,10 +45,30 @@ def test_registry_carries_a_count_per_place(client):
|
|||||||
assert town["count"] == 6
|
assert town["count"] == 6
|
||||||
|
|
||||||
|
|
||||||
def test_place_detail_returns_its_schools_ranked(client):
|
def test_place_detail_returns_its_schools_alphabetically(client):
|
||||||
|
"""A place page is read by someone looking for a school they can name.
|
||||||
|
|
||||||
|
Scanning for it is what the order should serve, so the list is A-Z.
|
||||||
|
/api/rankings is where the league-table ordering lives.
|
||||||
|
"""
|
||||||
body = client.get("/api/places/town/brentwood").json()
|
body = client.get("/api/places/town/brentwood").json()
|
||||||
assert body["place"]["name"] == "Brentwood"
|
assert body["place"]["name"] == "Brentwood"
|
||||||
scores = [s["rwm_expected_pct"] for s in body["schools"]]
|
names = [s["school_name"] for s in body["schools"]]
|
||||||
|
assert names == sorted(names, key=str.lower)
|
||||||
|
|
||||||
|
|
||||||
|
def test_place_ordering_ignores_case(client):
|
||||||
|
body = client.get("/api/places/town/brentwood").json()
|
||||||
|
names = [s["school_name"] for s in body["schools"]]
|
||||||
|
# A capitalised name must not sort ahead of every lowercase one.
|
||||||
|
assert names == sorted(names, key=str.lower)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_rankings_endpoint_still_ranks_by_metric(client):
|
||||||
|
# Alphabetical is a place-page decision, not a site-wide one.
|
||||||
|
body = client.get("/api/rankings?metric=rwm_expected_pct&phase=primary").json()
|
||||||
|
scores = [r["rwm_expected_pct"] for r in body.get("rankings", [])
|
||||||
|
if r.get("rwm_expected_pct") is not None]
|
||||||
assert scores == sorted(scores, reverse=True)
|
assert scores == sorted(scores, reverse=True)
|
||||||
|
|
||||||
|
|
||||||
@@ -69,3 +89,51 @@ def test_unknown_place_404s(client):
|
|||||||
|
|
||||||
def test_unknown_kind_404s(client):
|
def test_unknown_kind_404s(client):
|
||||||
assert client.get("/api/places/planet/mars").status_code == 404
|
assert client.get("/api/places/planet/mars").status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def _straddling_df() -> pd.DataFrame:
|
||||||
|
"""Eight schools in CM13: six in Essex, which has a page, and two in an
|
||||||
|
authority too small to have one.
|
||||||
|
|
||||||
|
Two, not one: the registry ignores an authority holding a single school in
|
||||||
|
a place, because GIAS carries occasional postcode errors."""
|
||||||
|
df = _schools_df()
|
||||||
|
extra = df.iloc[:2].copy()
|
||||||
|
extra["urn"] = [200000, 200001]
|
||||||
|
extra["school_name"] = ["Scilly School 0", "Scilly School 1"]
|
||||||
|
extra["local_authority"] = "Isles Of Scilly"
|
||||||
|
return pd.concat([df, extra], ignore_index=True)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def straddling_client(monkeypatch):
|
||||||
|
from backend import app as app_module
|
||||||
|
|
||||||
|
monkeypatch.setattr(app_module, "load_school_data", _straddling_df)
|
||||||
|
monkeypatch.setattr(app_module, "load_latest_school_data", _straddling_df)
|
||||||
|
monkeypatch.setattr(app_module, "_place_registry", None)
|
||||||
|
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_outcode_reports_no_phases_because_it_has_no_phase_route(client):
|
||||||
|
body = client.get("/api/places/outcode/cm13").json()
|
||||||
|
assert body["place"]["phases"] == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_authority_reports_the_phases_it_publishes(client):
|
||||||
|
body = client.get("/api/places/authority/essex").json()
|
||||||
|
assert body["place"]["phases"] == ["primary"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_authority_without_a_page_is_named_but_carries_no_slug(straddling_client):
|
||||||
|
"""Two English authorities — City of London and the Isles of Scilly — hold
|
||||||
|
fewer than the five schools a page needs, so they have no page.
|
||||||
|
|
||||||
|
Naming them is still right: the page says where the place is. Linking them
|
||||||
|
would not be. A null slug is what tells the page to print the name plainly
|
||||||
|
rather than invent a URL that 404s.
|
||||||
|
"""
|
||||||
|
body = straddling_client.get("/api/places/outcode/cm13").json()
|
||||||
|
by_name = {a["name"]: a for a in body["place"]["authorities"]}
|
||||||
|
assert by_name["Essex"]["slug"] == "essex"
|
||||||
|
assert by_name["Isles Of Scilly"]["slug"] is None
|
||||||
@@ -268,3 +268,39 @@ def test_place_urls_carry_no_priority_or_changefreq(place_sitemaps):
|
|||||||
for name in ("places-1.xml", "outcodes-1.xml"):
|
for name in ("places-1.xml", "outcodes-1.xml"):
|
||||||
assert "<priority>" not in place_sitemaps[name]
|
assert "<priority>" not in place_sitemaps[name]
|
||||||
assert "<changefreq>" not in place_sitemaps[name]
|
assert "<changefreq>" not in place_sitemaps[name]
|
||||||
|
|
||||||
|
|
||||||
|
def test_phase_variants_are_submitted_where_the_phase_clears_the_threshold(place_sitemaps):
|
||||||
|
# "primary schools in beccles" is the query shape the baseline showed, so
|
||||||
|
# each variant is its own page and has to be submitted. Emitting only the
|
||||||
|
# bare place URL left ~950 of them reachable by nothing.
|
||||||
|
xml = place_sitemaps["places-1.xml"]
|
||||||
|
assert "<loc>https://www.schoolcompare.co.uk/schools/brentwood/primary</loc>" in xml
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_phase_below_its_own_threshold_is_not_submitted(place_sitemaps):
|
||||||
|
# The fixture is six primaries and no secondaries.
|
||||||
|
xml = place_sitemaps["places-1.xml"]
|
||||||
|
assert "/schools/brentwood/secondary" not in xml
|
||||||
|
|
||||||
|
|
||||||
|
def test_outcodes_get_no_phase_variants(place_sitemaps):
|
||||||
|
# Nobody searches "primary schools in CM13"; the routes do not exist.
|
||||||
|
xml = place_sitemaps["outcodes-1.xml"]
|
||||||
|
assert "/primary" not in xml and "/secondary" not in xml
|
||||||
|
|
||||||
|
|
||||||
|
def test_authority_phase_variants_are_submitted_in_their_own_namespace(place_sitemaps):
|
||||||
|
"""302 of these were already in the sitemap, and every one 404'd.
|
||||||
|
|
||||||
|
The spec gives authorities a phase route; the plan built the bare
|
||||||
|
authority route and dropped it. Nothing noticed because the sitemap was
|
||||||
|
written from the registry, which was right, while the routes were written
|
||||||
|
by hand. This test fails if the URL ever leaves the sitemap; the e2e
|
||||||
|
journey fails if the route ever leaves the app.
|
||||||
|
"""
|
||||||
|
xml = place_sitemaps["places-1.xml"]
|
||||||
|
assert ("<loc>https://www.schoolcompare.co.uk"
|
||||||
|
"/schools/authority/essex/primary</loc>") in xml
|
||||||
|
# And never in the town namespace, which is a different set of schools.
|
||||||
|
assert "/schools/essex/primary" not in xml
|
||||||
+198
-3
@@ -1649,13 +1649,29 @@ const CANONICAL_ROUTES: Array<[string, string]> = [
|
|||||||
['/admissions', 'https://www.schoolcompare.co.uk/admissions'],
|
['/admissions', 'https://www.schoolcompare.co.uk/admissions'],
|
||||||
];
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Next normalises canonical URLs against `trailingSlash: false`, so the root
|
||||||
|
* ships as `https://www.schoolcompare.co.uk` with no slash while every other
|
||||||
|
* route keeps its path. Both forms address the same document, and which one
|
||||||
|
* Next emits is its business, not something worth pinning a test to.
|
||||||
|
*
|
||||||
|
* The first cut hardcoded the slash and failed only on the homepage — the
|
||||||
|
* same gap as the doubled brand: it asserted the metadata object rather than
|
||||||
|
* what the page actually renders.
|
||||||
|
*/
|
||||||
|
function sameUrl(a: string | null, b: string): boolean {
|
||||||
|
const strip = (u: string) => u.replace(/\/+$/, '');
|
||||||
|
return strip(a ?? '') === strip(b);
|
||||||
|
}
|
||||||
|
|
||||||
for (const [path, expected] of CANONICAL_ROUTES) {
|
for (const [path, expected] of CANONICAL_ROUTES) {
|
||||||
test(`${path} declares exactly one canonical, on the www host`, async ({ page }) => {
|
test(`${path} declares exactly one canonical, on the www host`, async ({ page }) => {
|
||||||
await page.goto(path);
|
await page.goto(path);
|
||||||
const hrefs = await page.locator('link[rel="canonical"]').evaluateAll(
|
const hrefs = await page.locator('link[rel="canonical"]').evaluateAll(
|
||||||
(els) => els.map((e) => e.getAttribute('href')));
|
(els) => els.map((e) => e.getAttribute('href')));
|
||||||
expect(hrefs, `${path} should declare one canonical`).toHaveLength(1);
|
expect(hrefs, `${path} should declare one canonical`).toHaveLength(1);
|
||||||
expect(hrefs[0]).toBe(expected);
|
expect(sameUrl(hrefs[0], expected),
|
||||||
|
`${path} canonical was ${hrefs[0]}, expected ${expected}`).toBe(true);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1663,7 +1679,8 @@ test('a filtered homepage still canonicalises to the bare root', async ({ page }
|
|||||||
await page.goto('/?search=primary&phase=primary&sort=name&page=2');
|
await page.goto('/?search=primary&phase=primary&sort=name&page=2');
|
||||||
const href = await page.locator('link[rel="canonical"]').first()
|
const href = await page.locator('link[rel="canonical"]').first()
|
||||||
.getAttribute('href');
|
.getAttribute('href');
|
||||||
expect(href).toBe('https://www.schoolcompare.co.uk/');
|
expect(sameUrl(href, 'https://www.schoolcompare.co.uk/'),
|
||||||
|
`filtered homepage canonical was ${href}`).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
test('a school page canonicalises to its own slug on the www host', async ({ page }) => {
|
test('a school page canonicalises to its own slug on the www host', async ({ page }) => {
|
||||||
@@ -1714,10 +1731,33 @@ test('staging answers noindex, and stays crawlable so the noindex is seen', asyn
|
|||||||
// The other half, and the reason this is one test rather than two: a
|
// The other half, and the reason this is one test rather than two: a
|
||||||
// Disallow would stop Google fetching the page at all, so it would never
|
// Disallow would stop Google fetching the page at all, so it would never
|
||||||
// see the noindex above. The two only work together.
|
// see the noindex above. The two only work together.
|
||||||
|
//
|
||||||
|
// Scoped to the `*` group. The first cut matched `Disallow: /` anywhere in
|
||||||
|
// the file and tripped over the AI-crawler groups Cloudflare injects —
|
||||||
|
// ClaudeBot, GPTBot, Amazonbot and friends all carry a blanket disallow,
|
||||||
|
// deliberately, and none of them is Googlebot.
|
||||||
const robots = await (await page.request.get('/robots.txt')).text();
|
const robots = await (await page.request.get('/robots.txt')).text();
|
||||||
expect(robots).not.toMatch(/^\s*Disallow:\s*\/\s*$/mi);
|
expect(blocksEverything(robots, '*'),
|
||||||
|
'the * group must not disallow the whole site, or the noindex is never seen')
|
||||||
|
.toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/** True when `agent`'s group in a robots.txt disallows the entire site. */
|
||||||
|
function blocksEverything(robots: string, agent: string): boolean {
|
||||||
|
let current: string | null = null;
|
||||||
|
let blocked = false;
|
||||||
|
for (const raw of robots.split('\n')) {
|
||||||
|
const line = raw.split('#')[0].trim();
|
||||||
|
if (!line) continue;
|
||||||
|
const [key, ...rest] = line.split(':');
|
||||||
|
const value = rest.join(':').trim();
|
||||||
|
const k = key.trim().toLowerCase();
|
||||||
|
if (k === 'user-agent') current = value;
|
||||||
|
else if (current === agent && k === 'disallow' && value === '/') blocked = true;
|
||||||
|
}
|
||||||
|
return blocked;
|
||||||
|
}
|
||||||
|
|
||||||
test('a school page on staging is noindexed too, not just the homepage', async ({ page }) => {
|
test('a school page on staging is noindexed too, not just the homepage', async ({ page }) => {
|
||||||
const list = await page.request.get('/api/schools?search=primary&per_page=1');
|
const list = await page.request.get('/api/schools?search=primary&per_page=1');
|
||||||
const [first] = (await list.json()).schools ?? [];
|
const [first] = (await list.json()).schools ?? [];
|
||||||
@@ -1842,3 +1882,158 @@ test('a place page states the local average against England', async ({ page }) =
|
|||||||
await page.goto(`/schools/${place.slug}`);
|
await page.goto(`/schools/${place.slug}`);
|
||||||
await expect(page.getByTestId('local-vs-england')).toContainText(/across England/i);
|
await expect(page.getByTestId('local-vs-england')).toContainText(/across England/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('a place page links its phase variants, and they resolve', async ({ page }) => {
|
||||||
|
// "primary schools in beccles" is the query shape the baseline showed. The
|
||||||
|
// first cut submitted only the bare place URL and linked nothing, leaving
|
||||||
|
// ~950 variant pages reachable by nothing at all.
|
||||||
|
const res = await page.request.get('/api/places');
|
||||||
|
const { places } = await res.json();
|
||||||
|
const town = places.find((p: { kind: string }) => p.kind === 'town');
|
||||||
|
expect(town).toBeTruthy();
|
||||||
|
|
||||||
|
const detail = await (await page.request.get(`/api/places/town/${town.slug}`)).json();
|
||||||
|
test.skip(!(detail.place.phases ?? []).length, 'no phase clears the threshold here');
|
||||||
|
|
||||||
|
await page.goto(`/schools/${town.slug}`);
|
||||||
|
const phase = detail.place.phases[0];
|
||||||
|
const link = page.locator(`a[href="/schools/${town.slug}/${phase}"]`).first();
|
||||||
|
await expect(link).toBeVisible();
|
||||||
|
|
||||||
|
await link.click();
|
||||||
|
await expect(page.locator('h1')).toContainText(new RegExp(`${phase} schools in`, 'i'));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('phase variants are submitted in the places sitemap', async ({ page }) => {
|
||||||
|
const xml = await (await page.request.get('/sitemaps/places-1.xml')).text();
|
||||||
|
expect(xml).toMatch(/\/schools\/[a-z0-9-]+\/primary</);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('authority phase variants are submitted, and in their own namespace', async ({ page }) => {
|
||||||
|
// 302 of these were in the sitemap for weeks and every one 404'd: the spec
|
||||||
|
// called for the route, the plan built the bare authority page and dropped
|
||||||
|
// it, and the sitemap — written from the registry — kept submitting them.
|
||||||
|
const xml = await (await page.request.get('/sitemaps/places-1.xml')).text();
|
||||||
|
expect(xml).toMatch(/\/schools\/authority\/[a-z0-9-]+\/primary</);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('every place link a place page emits resolves', async ({ page }) => {
|
||||||
|
/*
|
||||||
|
* The guard that was missing. Each family built its own links, so a URL
|
||||||
|
* shape belonging to one namespace was used by all four: an authority page
|
||||||
|
* offered "Primary schools in Barnet" pointing at /schools/barnet/primary,
|
||||||
|
* the *town*. For 87 of 151 authorities that 404'd; for the other 64 it
|
||||||
|
* quietly served a different set of schools under the same name.
|
||||||
|
*
|
||||||
|
* Only /schools links are followed. The per-school links are the same
|
||||||
|
* component the school-page journeys already cover, and there are hundreds
|
||||||
|
* of them on a page.
|
||||||
|
*/
|
||||||
|
for (const kind of ['town', 'authority', 'outcode'] as const) {
|
||||||
|
const place = await firstPlaceOfKind(page, kind);
|
||||||
|
const prefix = kind === 'authority' ? '/schools/authority/'
|
||||||
|
: kind === 'outcode' ? '/schools/near/' : '/schools/';
|
||||||
|
await page.goto(`${prefix}${place.slug}`);
|
||||||
|
|
||||||
|
const hrefs = [...new Set(
|
||||||
|
await page.locator('a[href^="/schools"]').evaluateAll(
|
||||||
|
(els) => els.map((e) => e.getAttribute('href')!)))];
|
||||||
|
expect(hrefs.length, `${kind} page links no other place`).toBeGreaterThan(0);
|
||||||
|
|
||||||
|
for (const href of hrefs) {
|
||||||
|
const res = await page.request.get(href);
|
||||||
|
expect(res.status(), `${kind} page links ${href}`).toBe(200);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an outcode page offers no phase link, because no such page exists', async ({ page }) => {
|
||||||
|
// Nobody searches "primary schools in SW11", so the spec gives outcodes no
|
||||||
|
// phase route. The registry computed the variants anyway and the page
|
||||||
|
// linked them, putting two 404s on each of 1,720 outcode pages.
|
||||||
|
const place = await firstPlaceOfKind(page, 'outcode');
|
||||||
|
const detail = await (await page.request.get(
|
||||||
|
`/api/places/outcode/${place.slug}`)).json();
|
||||||
|
expect(detail.place.phases).toEqual([]);
|
||||||
|
|
||||||
|
await page.goto(`/schools/near/${place.slug}`);
|
||||||
|
await expect(page.getByRole('navigation', { name: 'By phase' })).toHaveCount(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('no page title repeats the brand', async ({ page }) => {
|
||||||
|
// The root layout appends '| schoolcompare' to a plain-string title. Any
|
||||||
|
// route whose title already carries the brand must opt out with
|
||||||
|
// `absolute`, or it ships '... | schoolcompare | schoolcompare' — which is
|
||||||
|
// how ~2,600 place pages first went out.
|
||||||
|
const res = await page.request.get('/api/places');
|
||||||
|
const { places } = await res.json();
|
||||||
|
const town = places.find((p: { kind: string }) => p.kind === 'town');
|
||||||
|
|
||||||
|
for (const path of ['/', '/rankings', '/admissions', `/schools/${town.slug}`]) {
|
||||||
|
await page.goto(path);
|
||||||
|
const title = await page.title();
|
||||||
|
const brands = (title.match(/schoolcompare/gi) ?? []).length;
|
||||||
|
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) {
|
||||||
|
if (a.slug) {
|
||||||
|
await expect(page.locator(`a[href="/schools/authority/${a.slug}"]`).first())
|
||||||
|
.toBeVisible();
|
||||||
|
} else {
|
||||||
|
// No page of its own — City of London and the Isles of Scilly are
|
||||||
|
// under the threshold. Named, deliberately not linked.
|
||||||
|
await expect(page.locator('header p')).toContainText(a.name);
|
||||||
|
await expect(page.getByRole('link', { name: a.name })).toHaveCount(0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a place page lists its schools alphabetically', async ({ page }) => {
|
||||||
|
// Someone on a place page is usually looking for a school they can name,
|
||||||
|
// so the order should serve scanning for it. /rankings is where the
|
||||||
|
// league-table ordering lives.
|
||||||
|
const { places } = await (await page.request.get('/api/places')).json();
|
||||||
|
const town = places.find((p: { kind: string; count: number }) =>
|
||||||
|
p.kind === 'town' && p.count >= 5);
|
||||||
|
expect(town).toBeTruthy();
|
||||||
|
|
||||||
|
await page.goto(`/schools/${town.slug}`);
|
||||||
|
const names = await page.locator('a[href^="/school/"]').allTextContents();
|
||||||
|
expect(names.length).toBeGreaterThan(1);
|
||||||
|
|
||||||
|
const sorted = [...names].sort((a, b) =>
|
||||||
|
a.toLowerCase().localeCompare(b.toLowerCase()));
|
||||||
|
expect(names).toEqual(sorted);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('the rankings page still orders by score, not name', async ({ page }) => {
|
||||||
|
// Alphabetical is a place-page decision, not a site-wide one.
|
||||||
|
const res = await page.request.get('/api/rankings?metric=rwm_expected_pct&phase=primary');
|
||||||
|
expect(res.ok()).toBeTruthy();
|
||||||
|
const scores = ((await res.json()).rankings ?? [])
|
||||||
|
.map((r: { rwm_expected_pct: number | null }) => r.rwm_expected_pct)
|
||||||
|
.filter((v: number | null) => v != null);
|
||||||
|
expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a));
|
||||||
|
});
|
||||||
@@ -14,7 +14,7 @@ jest.mock('@/lib/places', () => ({
|
|||||||
describe('place page metadata', () => {
|
describe('place page metadata', () => {
|
||||||
it('titles the page the way the place is searched', async () => {
|
it('titles the page the way the place is searched', async () => {
|
||||||
const m = await placeMeta({ params: Promise.resolve({ place: 'brentwood' }) });
|
const m = await placeMeta({ params: Promise.resolve({ place: 'brentwood' }) });
|
||||||
expect(m.title).toMatch(/schools in brentwood/i);
|
expect((m.title as { absolute: string }).absolute).toMatch(/schools in brentwood/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('canonicalises to its own path on the www host', async () => {
|
it('canonicalises to its own path on the www host', async () => {
|
||||||
@@ -23,6 +23,18 @@ describe('place page metadata', () => {
|
|||||||
.toBe('https://www.schoolcompare.co.uk/schools/brentwood');
|
.toBe('https://www.schoolcompare.co.uk/schools/brentwood');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('opts out of the layout template, which would double the brand', () => {
|
||||||
|
// The root layout appends '| schoolcompare' to a plain string title, and
|
||||||
|
// these titles already carry it — every place page shipped reading
|
||||||
|
// '... | schoolcompare | schoolcompare' until this was made absolute.
|
||||||
|
return placeMeta({ params: Promise.resolve({ place: 'brentwood' }) })
|
||||||
|
.then((m) => {
|
||||||
|
expect(typeof m.title).toBe('object');
|
||||||
|
expect((m.title as { absolute: string }).absolute)
|
||||||
|
.not.toMatch(/schoolcompare.*schoolcompare/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it('an unknown place gets a not-found title rather than inventing one', async () => {
|
it('an unknown place gets a not-found title rather than inventing one', async () => {
|
||||||
const m = await placeMeta({ params: Promise.resolve({ place: 'atlantis' }) });
|
const m = await placeMeta({ params: Promise.resolve({ place: 'atlantis' }) });
|
||||||
expect(m.title).toMatch(/not found/i);
|
expect(m.title).toMatch(/not found/i);
|
||||||
|
|||||||
@@ -4,12 +4,12 @@ import type { PlaceDetail } from '@/lib/places';
|
|||||||
|
|
||||||
const detail: PlaceDetail = {
|
const detail: PlaceDetail = {
|
||||||
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 29,
|
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 29,
|
||||||
parent_authority: 'Essex' },
|
parent_authority: 'Essex', phases: ['primary'] },
|
||||||
schools: [
|
schools: [
|
||||||
{ urn: 1, school_name: 'Alpha Primary', rwm_expected_pct: 82,
|
{ urn: 1, school_name: 'Alpha Primary', rwm_expected_pct: 82,
|
||||||
ofsted_grade: 1 } as never,
|
ofsted_grade: 1, phase: 'Primary' } as never,
|
||||||
{ urn: 2, school_name: 'Beta Primary', rwm_expected_pct: 44,
|
{ urn: 2, school_name: 'Beta Primary', rwm_expected_pct: 44,
|
||||||
ofsted_grade: 3 } as never,
|
ofsted_grade: 3, phase: 'Primary' } as never,
|
||||||
],
|
],
|
||||||
averages: { rwm_expected_pct: 63, attainment_8_score: null },
|
averages: { rwm_expected_pct: 63, attainment_8_score: null },
|
||||||
};
|
};
|
||||||
@@ -91,3 +91,258 @@ describe('PlaceView structured data', () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('PlaceView phase variants', () => {
|
||||||
|
it('links the phase variants that exist', () => {
|
||||||
|
render(<PlaceView detail={detail} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('link', { name: /Primary schools in Brentwood/i }))
|
||||||
|
.toHaveAttribute('href', '/schools/brentwood/primary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('links no variant for a phase below its own threshold', () => {
|
||||||
|
render(<PlaceView detail={detail} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.queryByRole('link', { name: /Secondary schools in Brentwood/i }))
|
||||||
|
.not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not link sideways from a variant page to itself', () => {
|
||||||
|
render(<PlaceView detail={detail} phase="primary" englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
expect(screen.queryByRole('link', { name: /Primary schools in Brentwood/i }))
|
||||||
|
.not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PlaceView presentation', () => {
|
||||||
|
// /schools/brentwood shipped with 8 of 27 rows blank: an unphased page shows
|
||||||
|
// one primary-only measure for a list that also holds secondaries.
|
||||||
|
const mixed: PlaceDetail = {
|
||||||
|
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 4,
|
||||||
|
parent_authority: 'Essex', phases: ['primary', 'secondary'] },
|
||||||
|
schools: [
|
||||||
|
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
|
||||||
|
rwm_expected_pct: 82, attainment_8_score: null } as never,
|
||||||
|
{ urn: 2, school_name: 'Beta High', phase: 'Secondary',
|
||||||
|
rwm_expected_pct: null, attainment_8_score: 47 } as never,
|
||||||
|
],
|
||||||
|
averages: { rwm_expected_pct: 63, attainment_8_score: 45 },
|
||||||
|
};
|
||||||
|
|
||||||
|
it('gives each phase its own table rather than one column of blanks', () => {
|
||||||
|
render(<PlaceView detail={mixed} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('heading', { name: /^Primary schools/ })).toBeInTheDocument();
|
||||||
|
expect(screen.getByRole('heading', { name: /^Secondary schools/ })).toBeInTheDocument();
|
||||||
|
expect(screen.getByText('82%')).toBeInTheDocument();
|
||||||
|
expect(screen.getByText('47')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names the measure in plain words, not jargon', () => {
|
||||||
|
// The first cut said "RWM expected", which appears nowhere else on the site.
|
||||||
|
render(<PlaceView detail={mixed} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByText('Reading, writing & maths')).toBeInTheDocument();
|
||||||
|
expect(screen.getByText('Attainment 8')).toBeInTheDocument();
|
||||||
|
expect(screen.queryByText(/RWM expected/i)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('says a missing result is unpublished rather than showing a bare dash', () => {
|
||||||
|
const noResult: PlaceDetail = {
|
||||||
|
...mixed,
|
||||||
|
schools: [{ urn: 3, school_name: 'New Primary', phase: 'Primary',
|
||||||
|
rwm_expected_pct: null, attainment_8_score: null } as never],
|
||||||
|
};
|
||||||
|
render(<PlaceView detail={noResult} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByText('Not published')).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('styles school links to the site convention rather than browser default', () => {
|
||||||
|
const { container } = render(<PlaceView detail={mixed} englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
const link = container.querySelector('a[href^="/school/"]');
|
||||||
|
expect(link?.className).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('a phased page shows one table and no phase headings', () => {
|
||||||
|
render(<PlaceView detail={mixed} phase="primary" englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
expect(screen.queryByRole('heading', { name: /^Secondary schools/ }))
|
||||||
|
.not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PlaceView table alignment', () => {
|
||||||
|
const aligned: PlaceDetail = {
|
||||||
|
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 2,
|
||||||
|
parent_authority: 'Essex', phases: ['primary'] },
|
||||||
|
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('aligns the measure heading and its values with the same class', () => {
|
||||||
|
// They were aligned by two different selectors whose specificity did not
|
||||||
|
// match: `.table th:last-child` (0,2,1) won and went right, while `.num`
|
||||||
|
// (0,1,0) lost to `.table td` (0,1,1) and stayed left. Sharing one class
|
||||||
|
// is what makes them impossible to drift apart.
|
||||||
|
const { container } = render(<PlaceView detail={aligned} englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
const th = container.querySelectorAll('th')[1];
|
||||||
|
const td = container.querySelectorAll('tbody td')[1];
|
||||||
|
expect(th.className).toBeTruthy();
|
||||||
|
expect(td.className).toBe(th.className);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the school-name column unclassed so it takes the spare width', () => {
|
||||||
|
const { container } = render(<PlaceView detail={aligned} englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
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(<PlaceView detail={straddling} englandAverage={61} neighbours={[]} />);
|
||||||
|
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(<PlaceView detail={straddling}
|
||||||
|
englandAverage={61} neighbours={[]} />);
|
||||||
|
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(<PlaceView detail={legacy} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('link', { name: 'Merton' })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PlaceView list ordering', () => {
|
||||||
|
const detail3: PlaceDetail = {
|
||||||
|
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 2,
|
||||||
|
parent_authority: 'Essex', phases: ['primary'] },
|
||||||
|
schools: [
|
||||||
|
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
|
||||||
|
rwm_expected_pct: 40, attainment_8_score: null } as never,
|
||||||
|
{ urn: 2, school_name: 'Beta Primary', phase: 'Primary',
|
||||||
|
rwm_expected_pct: 90, attainment_8_score: null } as never,
|
||||||
|
],
|
||||||
|
averages: { rwm_expected_pct: 65, attainment_8_score: null },
|
||||||
|
};
|
||||||
|
|
||||||
|
it('renders schools in the order the API sent them, not by score', () => {
|
||||||
|
// The API sorts alphabetically now; the component must not re-sort.
|
||||||
|
render(<PlaceView detail={detail3} englandAverage={61} neighbours={[]} />);
|
||||||
|
const links = screen.getAllByRole('link', { name: /Primary$/ });
|
||||||
|
expect(links.map((l) => l.textContent))
|
||||||
|
.toEqual(['Alpha Primary', 'Beta Primary']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('declares the list as ascending rather than implying a ranking', () => {
|
||||||
|
// An ItemList carrying `position` reads as a ranking unless it says
|
||||||
|
// otherwise, and the table is A-Z.
|
||||||
|
const { container } = render(<PlaceView detail={detail3} englandAverage={61}
|
||||||
|
neighbours={[]} />);
|
||||||
|
const ld = JSON.parse(
|
||||||
|
container.querySelector('script[type="application/ld+json"]')!.textContent!);
|
||||||
|
const list = ld['@graph'].find((n: { '@type': string }) => n['@type'] === 'ItemList');
|
||||||
|
expect(list.itemListOrder).toBe('https://schema.org/ItemListOrderAscending');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PlaceView phase links', () => {
|
||||||
|
const authority: PlaceDetail = {
|
||||||
|
place: { kind: 'authority', slug: 'barnet', name: 'Barnet', count: 156,
|
||||||
|
parent_authority: null, phases: ['primary', 'secondary'] },
|
||||||
|
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('keeps an authority phase link in the authority namespace', () => {
|
||||||
|
// The link was built as `/schools/${slug}/${phase}` for every kind, so an
|
||||||
|
// authority page pointed into the town namespace. For 87 of 151
|
||||||
|
// authorities that 404'd; for the other 64 it silently landed on the town
|
||||||
|
// page of the same name — a different set of schools, and exactly the
|
||||||
|
// duplicate the two namespaces exist to prevent. Barnet is one of the 64.
|
||||||
|
render(<PlaceView detail={authority} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('link', { name: /^Primary schools in Barnet$/ }))
|
||||||
|
.toHaveAttribute('href', '/schools/authority/barnet/primary');
|
||||||
|
expect(screen.getByRole('link', { name: /^Secondary schools in Barnet$/ }))
|
||||||
|
.toHaveAttribute('href', '/schools/authority/barnet/secondary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('still uses the bare namespace for a town', () => {
|
||||||
|
render(<PlaceView detail={detail} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('link', { name: /^Primary schools in Brentwood$/ }))
|
||||||
|
.toHaveAttribute('href', '/schools/brentwood/primary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('offers no phase link when the place publishes none', () => {
|
||||||
|
// Outcodes are the case: no phase route exists for them, so the registry
|
||||||
|
// reports no phases and the nav does not render.
|
||||||
|
const outcode = { ...detail,
|
||||||
|
place: { ...detail.place, kind: 'outcode', slug: 'cm13', name: 'CM13',
|
||||||
|
phases: [] } };
|
||||||
|
render(<PlaceView detail={outcode} englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.queryByRole('navigation', { name: 'By phase' }))
|
||||||
|
.not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PlaceView unlinkable authorities', () => {
|
||||||
|
const withUnpublished: PlaceDetail = {
|
||||||
|
place: { kind: 'outcode', slug: 'tr21', name: 'TR21', count: 8,
|
||||||
|
parent_authority: 'Cornwall', phases: [],
|
||||||
|
authorities: [
|
||||||
|
{ name: 'Cornwall', slug: 'cornwall', count: 6 },
|
||||||
|
{ name: 'Isles Of Scilly', slug: null, count: 2 },
|
||||||
|
] },
|
||||||
|
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 an authority with no page without linking it', () => {
|
||||||
|
// City of London and the Isles of Scilly hold fewer schools than a page
|
||||||
|
// needs. Saying where the place is stays right; linking there would 404.
|
||||||
|
const { container } = render(<PlaceView detail={withUnpublished}
|
||||||
|
englandAverage={61} neighbours={[]} />);
|
||||||
|
expect(screen.getByRole('link', { name: 'Cornwall' })).toBeInTheDocument();
|
||||||
|
expect(screen.queryByRole('link', { name: 'Isles Of Scilly' }))
|
||||||
|
.not.toBeInTheDocument();
|
||||||
|
expect(container.querySelector('header p')?.textContent)
|
||||||
|
.toContain('Isles Of Scilly');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -37,10 +37,12 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
const word = phase === 'secondary' ? 'Secondary' : 'Primary';
|
const word = phase === 'secondary' ? 'Secondary' : 'Primary';
|
||||||
const { name } = detail.place;
|
const { name } = detail.place;
|
||||||
return {
|
return {
|
||||||
title: `${word} Schools in ${name} — Ranked | schoolcompare`,
|
// Not "Ranked": the table is alphabetical, so the word would be a claim
|
||||||
|
// the page does not keep.
|
||||||
|
title: { absolute: `${word} Schools in ${name} | schoolcompare` },
|
||||||
description:
|
description:
|
||||||
`Every ${phase} school in ${name} ranked by results, with Ofsted grades and `
|
`Every ${phase} school in ${name}, with results, Ofsted grades and the local `
|
||||||
+ `the local average against England.`,
|
+ `average against England.`,
|
||||||
alternates: { canonical: absoluteUrl(`/schools/${slug}/${phase}`) },
|
alternates: { canonical: absoluteUrl(`/schools/${slug}/${phase}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -53,10 +53,13 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
|
|
||||||
const { name, count } = detail.place;
|
const { name, count } = detail.place;
|
||||||
return {
|
return {
|
||||||
title: `Schools in ${name} — Compare ${count} Schools | schoolcompare`,
|
// absolute: the root layout's template appends '| schoolcompare' to a
|
||||||
|
// plain string, and this title already carries it. Without this every
|
||||||
|
// place title read '... | schoolcompare | schoolcompare'.
|
||||||
|
title: { absolute: `Schools in ${name} — Compare ${count} Schools | schoolcompare` },
|
||||||
description:
|
description:
|
||||||
`Every school in ${name} ranked by SATs and GCSE results, with Ofsted grades, `
|
`Every school in ${name}, with SATs and GCSE results, Ofsted grades, the local `
|
||||||
+ `the local average against England, and how close you had to live to get a place.`,
|
+ `average against England, and how close you had to live to get a place.`,
|
||||||
alternates: { canonical: absoluteUrl(`/schools/${slug}`) },
|
alternates: { canonical: absoluteUrl(`/schools/${slug}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -71,9 +74,14 @@ export default async function PlacePage({ params }: Props) {
|
|||||||
// defers to its authority rather than publishing a thin page.
|
// defers to its authority rather than publishing a thin page.
|
||||||
if (detail.averages.rwm_expected_pct == null
|
if (detail.averages.rwm_expected_pct == null
|
||||||
&& detail.averages.attainment_8_score == null) {
|
&& detail.averages.attainment_8_score == null) {
|
||||||
if (detail.place.parent_authority) {
|
// The API's own slug, which is null when that authority is itself under
|
||||||
redirect(`/schools/authority/${authoritySlug(detail.place.parent_authority)}`);
|
// the threshold and has no page. Re-slugifying the name here would send
|
||||||
}
|
// the reader to a 404 instead of telling them the place has no page.
|
||||||
|
const target = detail.place.authorities?.[0]?.slug
|
||||||
|
?? (detail.place.parent_authority
|
||||||
|
? authoritySlug(detail.place.parent_authority)
|
||||||
|
: null);
|
||||||
|
if (target) redirect(`/schools/authority/${target}`);
|
||||||
notFound();
|
notFound();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
/**
|
||||||
|
* Phase variants of an authority page.
|
||||||
|
*
|
||||||
|
* The spec called for these; the plan built the bare authority route and
|
||||||
|
* dropped them. Nothing caught it, because the sitemap is written from the
|
||||||
|
* place registry — which was right about them all along — while the routes
|
||||||
|
* were written by hand. 302 authority phase URLs were submitted to Google and
|
||||||
|
* every one 404'd, and every authority page linked to a phase page in the
|
||||||
|
* *town* namespace, which is a different set of schools entirely.
|
||||||
|
*
|
||||||
|
* "Primary schools in Kent" is the query these serve, and it is a real one:
|
||||||
|
* admissions are authority-run, so the authority is the unit a parent thinks
|
||||||
|
* in when they have not settled on a town.
|
||||||
|
*/
|
||||||
|
import { notFound } from 'next/navigation';
|
||||||
|
import type { Metadata } from 'next';
|
||||||
|
import { fetchPlace } from '@/lib/places';
|
||||||
|
import { fetchNationalAverages } from '@/lib/api';
|
||||||
|
import { PlaceView } from '@/components/places/PlaceView';
|
||||||
|
import { absoluteUrl } from '@/lib/site';
|
||||||
|
|
||||||
|
interface Props { params: Promise<{ la: string; phase: string }> }
|
||||||
|
|
||||||
|
export const revalidate = 604800;
|
||||||
|
export const dynamicParams = true;
|
||||||
|
|
||||||
|
const PHASES = ['primary', 'secondary'] as const;
|
||||||
|
type Phase = (typeof PHASES)[number];
|
||||||
|
|
||||||
|
const isPhase = (v: string): v is Phase => (PHASES as readonly string[]).includes(v);
|
||||||
|
|
||||||
|
export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
||||||
|
const { la, phase } = await params;
|
||||||
|
if (!isPhase(phase)) return { title: 'Place Not Found' };
|
||||||
|
const detail = await fetchPlace('authority', la, phase);
|
||||||
|
if (!detail || detail.schools.length === 0) return { title: 'Place Not Found' };
|
||||||
|
|
||||||
|
const word = phase === 'secondary' ? 'Secondary' : 'Primary';
|
||||||
|
const { name } = detail.place;
|
||||||
|
return {
|
||||||
|
// "Local Authority" stays in the title for the same reason it is on the
|
||||||
|
// bare authority page: 67 town names collide with an authority name, and
|
||||||
|
// a reader landing on both needs to know which set each covers.
|
||||||
|
title: { absolute: `${word} Schools in ${name} — Local Authority | schoolcompare` },
|
||||||
|
description:
|
||||||
|
`Every ${phase} school in the ${name} local authority, with results, Ofsted `
|
||||||
|
+ `grades and the authority average against England.`,
|
||||||
|
alternates: { canonical: absoluteUrl(`/schools/authority/${la}/${phase}`) },
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
export default async function AuthorityPhasePage({ params }: Props) {
|
||||||
|
const { la, phase } = await params;
|
||||||
|
if (!isPhase(phase)) notFound();
|
||||||
|
const detail = await fetchPlace('authority', la, phase);
|
||||||
|
if (!detail || detail.schools.length === 0) notFound();
|
||||||
|
|
||||||
|
const national = await fetchNationalAverages().catch(() => null);
|
||||||
|
const englandAverage = phase === 'secondary'
|
||||||
|
? national?.secondary?.attainment_8_score ?? null
|
||||||
|
: national?.primary?.rwm_expected_pct ?? null;
|
||||||
|
|
||||||
|
return <PlaceView detail={detail} phase={phase}
|
||||||
|
englandAverage={englandAverage} neighbours={[]} />;
|
||||||
|
}
|
||||||
@@ -43,10 +43,10 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
|
|
||||||
const { name, count } = detail.place;
|
const { name, count } = detail.place;
|
||||||
return {
|
return {
|
||||||
title: `Schools in ${name} — Local Authority | schoolcompare`,
|
title: { absolute: `Schools in ${name} — Local Authority | schoolcompare` },
|
||||||
description:
|
description:
|
||||||
`All ${count} schools in the ${name} local authority, ranked by SATs and GCSE `
|
`All ${count} schools in the ${name} local authority, with SATs and GCSE results, `
|
||||||
+ `results, with Ofsted grades and the authority average against England.`,
|
+ `Ofsted grades and the authority average against England.`,
|
||||||
alternates: { canonical: absoluteUrl(`/schools/authority/${la}`) },
|
alternates: { canonical: absoluteUrl(`/schools/authority/${la}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -38,10 +38,10 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
|
|
||||||
const { name, count } = detail.place;
|
const { name, count } = detail.place;
|
||||||
return {
|
return {
|
||||||
title: `Schools near ${name} | schoolcompare`,
|
title: { absolute: `Schools near ${name} | schoolcompare` },
|
||||||
description:
|
description:
|
||||||
`${count} schools in the ${name} postcode district, ranked by results, with `
|
`${count} schools in the ${name} postcode district, with results, Ofsted grades `
|
||||||
+ `Ofsted grades and how close you had to live to get a place.`,
|
+ `and how close you had to live to get a place.`,
|
||||||
alternates: { canonical: absoluteUrl(`/schools/near/${outcode}`) },
|
alternates: { canonical: absoluteUrl(`/schools/near/${outcode}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
/* Tokens only — see globals.css. Matches RankingsView's conventions. */
|
/* Tokens only — see globals.css. Follows RankingsView's conventions, and in
|
||||||
|
particular its link treatment: table links take --text-primary with no
|
||||||
|
underline and a brand-coloured hover, not the browser default. The first
|
||||||
|
cut used bare <Link> with no class at all, which rendered as default blue
|
||||||
|
underlined links and read as unstyled beside the rest of the site. */
|
||||||
.container {
|
.container {
|
||||||
width: 100%;
|
width: 100%;
|
||||||
min-width: 0;
|
min-width: 0;
|
||||||
@@ -24,6 +28,44 @@
|
|||||||
line-height: 1.6;
|
line-height: 1.6;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Links in running copy: brand colour, underline on hover only. */
|
||||||
|
.inlineLink {
|
||||||
|
color: var(--brand);
|
||||||
|
text-decoration: none;
|
||||||
|
transition: color 0.2s ease;
|
||||||
|
}
|
||||||
|
|
||||||
|
.inlineLink:hover {
|
||||||
|
color: var(--brand-strong);
|
||||||
|
text-decoration: underline;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Phase variants are separate indexable pages, so the bare place page has to
|
||||||
|
link them — a sitemap entry alone leaves them with no internal path in. */
|
||||||
|
.phaseLinks {
|
||||||
|
display: flex;
|
||||||
|
flex-wrap: wrap;
|
||||||
|
gap: 0.5rem 0.75rem;
|
||||||
|
margin: 0 0 1.25rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.phaseLink {
|
||||||
|
display: inline-block;
|
||||||
|
padding: 0.4rem 0.875rem;
|
||||||
|
border: 1px solid var(--border-strong);
|
||||||
|
border-radius: 999px;
|
||||||
|
font-size: 0.875rem;
|
||||||
|
font-weight: 500;
|
||||||
|
color: var(--text-primary);
|
||||||
|
text-decoration: none;
|
||||||
|
transition: border-color 0.2s ease, color 0.2s ease;
|
||||||
|
}
|
||||||
|
|
||||||
|
.phaseLink:hover {
|
||||||
|
border-color: var(--brand);
|
||||||
|
color: var(--brand-strong);
|
||||||
|
}
|
||||||
|
|
||||||
/* The one number a list cannot give you, so it gets its own band. */
|
/* The one number a list cannot give you, so it gets its own band. */
|
||||||
.compare {
|
.compare {
|
||||||
background: var(--bg-secondary);
|
background: var(--bg-secondary);
|
||||||
@@ -46,6 +88,30 @@
|
|||||||
color: var(--text-secondary);
|
color: var(--text-secondary);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.group {
|
||||||
|
margin-bottom: 2rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.groupHeading {
|
||||||
|
display: flex;
|
||||||
|
align-items: baseline;
|
||||||
|
gap: 0.625rem;
|
||||||
|
font-size: 1.25rem;
|
||||||
|
font-weight: 600;
|
||||||
|
color: var(--text-primary);
|
||||||
|
font-family: var(--font-display);
|
||||||
|
margin: 0 0 0.75rem;
|
||||||
|
}
|
||||||
|
|
||||||
|
.groupCount {
|
||||||
|
font-size: 0.8125rem;
|
||||||
|
font-weight: 500;
|
||||||
|
color: var(--text-secondary);
|
||||||
|
background: var(--bg-secondary);
|
||||||
|
border-radius: 999px;
|
||||||
|
padding: 0.125rem 0.5rem;
|
||||||
|
}
|
||||||
|
|
||||||
/* Wide content scrolls in its own container so the page body never does. */
|
/* Wide content scrolls in its own container so the page body never does. */
|
||||||
.tableWrap {
|
.tableWrap {
|
||||||
overflow-x: auto;
|
overflow-x: auto;
|
||||||
@@ -72,18 +138,54 @@
|
|||||||
color: var(--text-secondary);
|
color: var(--text-secondary);
|
||||||
font-weight: 600;
|
font-weight: 600;
|
||||||
font-size: 0.8125rem;
|
font-size: 0.8125rem;
|
||||||
text-transform: uppercase;
|
|
||||||
letter-spacing: 0.04em;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
.table tbody tr:last-child td {
|
.table tbody tr:last-child td {
|
||||||
border-bottom: none;
|
border-bottom: none;
|
||||||
}
|
}
|
||||||
|
|
||||||
.table th:last-child,
|
/*
|
||||||
.num {
|
* Header and value share one class and one rule, so they cannot drift apart.
|
||||||
|
*
|
||||||
|
* The first cut aligned them with two different selectors: `.table th:last-child`
|
||||||
|
* at (0,2,1) beat the element rule and went right, while `.num` at (0,1,0) lost
|
||||||
|
* to `.table td` at (0,1,1) and stayed left. The heading and its numbers sat on
|
||||||
|
* opposite edges of the column.
|
||||||
|
*
|
||||||
|
* width:1% with nowrap makes the measure column hug its content so the school
|
||||||
|
* name takes the remaining width — without it the two columns split evenly and
|
||||||
|
* the gap between heading and value reads as misalignment on a wide screen.
|
||||||
|
*/
|
||||||
|
.table th.num,
|
||||||
|
.table td.num {
|
||||||
text-align: right;
|
text-align: right;
|
||||||
font-variant-numeric: tabular-nums;
|
font-variant-numeric: tabular-nums;
|
||||||
|
width: 1%;
|
||||||
|
white-space: nowrap;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* The measure is spelled out; the tooltip carries the definition. */
|
||||||
|
.metricHead {
|
||||||
|
text-decoration: none;
|
||||||
|
cursor: help;
|
||||||
|
border-bottom: 1px dotted var(--border-strong);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Table links: site convention is body colour, brand on hover. */
|
||||||
|
.schoolLink {
|
||||||
|
color: var(--text-primary);
|
||||||
|
text-decoration: none;
|
||||||
|
transition: color 0.2s ease;
|
||||||
|
}
|
||||||
|
|
||||||
|
.schoolLink:hover {
|
||||||
|
color: var(--brand-strong);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* "Not published" is a fact about the school, not an error. */
|
||||||
|
.noData {
|
||||||
|
color: var(--text-muted);
|
||||||
|
font-size: 0.8125rem;
|
||||||
}
|
}
|
||||||
|
|
||||||
.neighbours {
|
.neighbours {
|
||||||
|
|||||||
@@ -12,6 +12,7 @@
|
|||||||
import Link from 'next/link';
|
import Link from 'next/link';
|
||||||
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
|
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
|
||||||
import { placeUrl, authoritySlug } from '@/lib/places';
|
import { placeUrl, authoritySlug } from '@/lib/places';
|
||||||
|
import type { School } from '@/lib/types';
|
||||||
import { schoolUrl } from '@/lib/utils';
|
import { schoolUrl } from '@/lib/utils';
|
||||||
import { absoluteUrl } from '@/lib/site';
|
import { absoluteUrl } from '@/lib/site';
|
||||||
import styles from './PlaceView.module.css';
|
import styles from './PlaceView.module.css';
|
||||||
@@ -30,19 +31,110 @@ const OFSTED_LABELS: Array<[number, string]> = [
|
|||||||
[3, 'Requires improvement'], [4, 'Inadequate'],
|
[3, 'Requires improvement'], [4, 'Inadequate'],
|
||||||
];
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Column headings, taken from the site's own metric dictionary rather than
|
||||||
|
* invented here — see METRIC_DEFINITIONS in backend/schemas.py, surfaced at
|
||||||
|
* /api/metrics. The first cut said "RWM expected", which is jargon that
|
||||||
|
* appears nowhere else on the site.
|
||||||
|
*/
|
||||||
|
const METRICS = {
|
||||||
|
primary: {
|
||||||
|
key: 'rwm_expected_pct' as const,
|
||||||
|
heading: 'Reading, writing & maths',
|
||||||
|
hint: '% meeting the expected standard in reading, writing and maths',
|
||||||
|
unit: '%',
|
||||||
|
},
|
||||||
|
secondary: {
|
||||||
|
key: 'attainment_8_score' as const,
|
||||||
|
heading: 'Attainment 8',
|
||||||
|
hint: "Average grade across a pupil's best 8 GCSEs, including English and maths",
|
||||||
|
unit: '',
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
type PhaseKey = keyof typeof METRICS;
|
||||||
|
|
||||||
|
/** All-through schools sit in both phases, matching the search filters. */
|
||||||
|
function isPhase(school: School, phase: PhaseKey): boolean {
|
||||||
|
const p = (school.phase ?? '').toLowerCase();
|
||||||
|
if (p === 'all-through') return true;
|
||||||
|
return phase === 'secondary'
|
||||||
|
? p.includes('secondary') || p === '16 plus'
|
||||||
|
: p.includes('primary') || p.includes('middle');
|
||||||
|
}
|
||||||
|
|
||||||
|
function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) {
|
||||||
|
const metric = METRICS[phase];
|
||||||
|
return (
|
||||||
|
<div className={styles.tableWrap}>
|
||||||
|
<table className={styles.table}>
|
||||||
|
<thead>
|
||||||
|
<tr>
|
||||||
|
<th scope="col">School</th>
|
||||||
|
{/* Same class as the value cell below: one rule aligns both, so
|
||||||
|
they cannot drift apart. */}
|
||||||
|
<th scope="col" className={styles.num}>
|
||||||
|
<abbr className={styles.metricHead} title={metric.hint}>
|
||||||
|
{metric.heading}
|
||||||
|
</abbr>
|
||||||
|
</th>
|
||||||
|
</tr>
|
||||||
|
</thead>
|
||||||
|
<tbody>
|
||||||
|
{schools.map((s) => {
|
||||||
|
const value = s[metric.key];
|
||||||
|
return (
|
||||||
|
<tr key={s.urn}>
|
||||||
|
<td>
|
||||||
|
<Link href={schoolUrl(s.urn, s.school_name)} className={styles.schoolLink}>
|
||||||
|
{s.school_name}
|
||||||
|
</Link>
|
||||||
|
</td>
|
||||||
|
<td className={styles.num}>
|
||||||
|
{value == null
|
||||||
|
? <span className={styles.noData}>Not published</span>
|
||||||
|
: `${Math.round(Number(value))}${metric.unit}`}
|
||||||
|
</td>
|
||||||
|
</tr>
|
||||||
|
);
|
||||||
|
})}
|
||||||
|
</tbody>
|
||||||
|
</table>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {
|
export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {
|
||||||
const { place, schools, averages } = detail;
|
const { place, schools, averages } = detail;
|
||||||
const metric = phase === 'secondary' ? 'attainment_8_score' : 'rwm_expected_pct';
|
// Fall back to the single parent when the API predates the authorities
|
||||||
const local = averages[metric];
|
// 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'
|
const phaseWord = phase === 'secondary' ? 'Secondary schools'
|
||||||
: phase === 'primary' ? 'Primary schools' : 'Schools';
|
: phase === 'primary' ? 'Primary schools' : 'Schools';
|
||||||
const graded = OFSTED_LABELS
|
const graded = OFSTED_LABELS
|
||||||
.map(([grade, label]) => [label, schools.filter((s) => s.ofsted_grade === grade).length] as const)
|
.map(([grade, label]) => [label, schools.filter((s) => s.ofsted_grade === grade).length] as const)
|
||||||
.filter(([, n]) => n > 0);
|
.filter(([, n]) => n > 0);
|
||||||
|
|
||||||
// ItemList tells Google this page is a ranked set rather than prose;
|
/*
|
||||||
// BreadcrumbList puts the place in a hierarchy. Capped at 20 because that
|
* An unphased page holds both primaries and secondaries, and they are
|
||||||
// is what the page shows above the fold and what the markup should mirror.
|
* scored on different measures — a percentage and a 0-90 score. Showing one
|
||||||
|
* column for both left 30% of rows blank on /schools/brentwood and put two
|
||||||
|
* incomparable scales in one column when it did not.
|
||||||
|
*
|
||||||
|
* So the phases get a table each. A blank cell inside one now means the
|
||||||
|
* school genuinely has no published result, which is worth saying.
|
||||||
|
*/
|
||||||
|
const groups: Array<[PhaseKey, School[]]> = phase
|
||||||
|
? [[phase, schools]]
|
||||||
|
: (['primary', 'secondary'] as PhaseKey[])
|
||||||
|
.map((p) => [p, schools.filter((s) => isPhase(s, p))] as [PhaseKey, School[]])
|
||||||
|
.filter(([, list]) => list.length > 0);
|
||||||
|
|
||||||
const jsonLd = {
|
const jsonLd = {
|
||||||
'@context': 'https://schema.org',
|
'@context': 'https://schema.org',
|
||||||
'@graph': [
|
'@graph': [
|
||||||
@@ -50,6 +142,10 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
'@type': 'ItemList',
|
'@type': 'ItemList',
|
||||||
name: `${phaseWord} in ${place.name}`,
|
name: `${phaseWord} in ${place.name}`,
|
||||||
numberOfItems: schools.length,
|
numberOfItems: schools.length,
|
||||||
|
// Alphabetical, and said so. Without this an ItemList carrying
|
||||||
|
// `position` reads as a ranking, which would be a claim the page
|
||||||
|
// stopped making when the table became A-Z.
|
||||||
|
itemListOrder: 'https://schema.org/ItemListOrderAscending',
|
||||||
itemListElement: schools.slice(0, 20).map((s, i) => ({
|
itemListElement: schools.slice(0, 20).map((s, i) => ({
|
||||||
'@type': 'ListItem',
|
'@type': 'ListItem',
|
||||||
position: i + 1,
|
position: i + 1,
|
||||||
@@ -60,8 +156,7 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
{
|
{
|
||||||
'@type': 'BreadcrumbList',
|
'@type': 'BreadcrumbList',
|
||||||
itemListElement: [
|
itemListElement: [
|
||||||
{ '@type': 'ListItem', position: 1, name: 'Schools',
|
{ '@type': 'ListItem', position: 1, name: 'Schools', item: absoluteUrl('/') },
|
||||||
item: absoluteUrl('/') },
|
|
||||||
{ '@type': 'ListItem', position: 2, name: place.name },
|
{ '@type': 'ListItem', position: 2, name: place.name },
|
||||||
],
|
],
|
||||||
},
|
},
|
||||||
@@ -74,21 +169,51 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
type="application/ld+json"
|
type="application/ld+json"
|
||||||
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }}
|
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }}
|
||||||
/>
|
/>
|
||||||
|
|
||||||
<header className={styles.header}>
|
<header className={styles.header}>
|
||||||
<h1>{phaseWord} in {place.name}</h1>
|
<h1>{phaseWord} in {place.name}</h1>
|
||||||
<p className={styles.summary}>
|
<p className={styles.summary}>
|
||||||
{place.count} schools
|
{place.count} schools
|
||||||
{place.parent_authority && (
|
{authorities.length > 0 && (
|
||||||
<>
|
<>
|
||||||
{' · '}
|
{' · '}
|
||||||
<Link href={`/schools/authority/${authoritySlug(place.parent_authority)}`}>
|
{/* Every authority, not just the largest. A quarter of outcodes
|
||||||
{place.parent_authority}
|
and a third of towns cross a boundary: SW19 is mostly Merton
|
||||||
</Link>
|
but partly Wandsworth, and naming one asserts otherwise. */}
|
||||||
|
{authorities.map((a, i) => (
|
||||||
|
<span key={a.name}>
|
||||||
|
{i > 0 && (i === authorities.length - 1 ? ' and ' : ', ')}
|
||||||
|
{/* No slug means no page: City of London and the Isles of
|
||||||
|
Scilly hold too few schools for one. Saying where the
|
||||||
|
place is stays right; linking there would 404. */}
|
||||||
|
{a.slug
|
||||||
|
? (
|
||||||
|
<Link href={`/schools/authority/${a.slug}`} className={styles.inlineLink}>
|
||||||
|
{a.name}
|
||||||
|
</Link>
|
||||||
|
)
|
||||||
|
: a.name}
|
||||||
|
</span>
|
||||||
|
))}
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
</p>
|
</p>
|
||||||
</header>
|
</header>
|
||||||
|
|
||||||
|
{!phase && (place.phases ?? []).length > 0 && (
|
||||||
|
<nav className={styles.phaseLinks} aria-label="By phase">
|
||||||
|
{(place.phases ?? []).map((ph) => (
|
||||||
|
/* placeUrl, not a template: the bare `/schools/[slug]/[phase]`
|
||||||
|
shape belongs to towns alone, and using it everywhere sent
|
||||||
|
every authority page into the town namespace. */
|
||||||
|
<Link key={ph} href={placeUrl(place.kind, place.slug, ph)}
|
||||||
|
className={styles.phaseLink}>
|
||||||
|
{ph === 'secondary' ? 'Secondary schools' : 'Primary schools'} in {place.name}
|
||||||
|
</Link>
|
||||||
|
))}
|
||||||
|
</nav>
|
||||||
|
)}
|
||||||
|
|
||||||
{local != null && englandAverage != null && (
|
{local != null && englandAverage != null && (
|
||||||
<p className={styles.compare} data-testid="local-vs-england">
|
<p className={styles.compare} data-testid="local-vs-england">
|
||||||
{place.name} averages <strong>{Math.round(local)}</strong> against{' '}
|
{place.name} averages <strong>{Math.round(local)}</strong> against{' '}
|
||||||
@@ -104,28 +229,17 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
</ul>
|
</ul>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
<div className={styles.tableWrap}>
|
{groups.map(([p, list]) => (
|
||||||
<table className={styles.table}>
|
<section key={p} className={styles.group}>
|
||||||
<thead>
|
{groups.length > 1 && (
|
||||||
<tr>
|
<h2 className={styles.groupHeading}>
|
||||||
<th>School</th>
|
{p === 'secondary' ? 'Secondary schools' : 'Primary schools'}
|
||||||
<th>{phase === 'secondary' ? 'Attainment 8' : 'RWM expected'}</th>
|
<span className={styles.groupCount}>{list.length}</span>
|
||||||
</tr>
|
</h2>
|
||||||
</thead>
|
)}
|
||||||
<tbody>
|
<SchoolTable schools={list} phase={p} />
|
||||||
{schools.map((s) => (
|
</section>
|
||||||
<tr key={s.urn}>
|
))}
|
||||||
<td>
|
|
||||||
<Link href={schoolUrl(s.urn, s.school_name)}>{s.school_name}</Link>
|
|
||||||
</td>
|
|
||||||
<td className={styles.num}>
|
|
||||||
{s[metric] == null ? '—' : Math.round(Number(s[metric]))}
|
|
||||||
</td>
|
|
||||||
</tr>
|
|
||||||
))}
|
|
||||||
</tbody>
|
|
||||||
</table>
|
|
||||||
</div>
|
|
||||||
|
|
||||||
{neighbours.length > 0 && (
|
{neighbours.length > 0 && (
|
||||||
<nav className={styles.neighbours} aria-label="Nearby places">
|
<nav className={styles.neighbours} aria-label="Nearby places">
|
||||||
@@ -133,7 +247,7 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
<ul>
|
<ul>
|
||||||
{neighbours.map((n) => (
|
{neighbours.map((n) => (
|
||||||
<li key={n.kind + n.slug}>
|
<li key={n.kind + n.slug}>
|
||||||
<Link href={placeUrl(n.kind, n.slug)}>{n.name}</Link>
|
<Link href={placeUrl(n.kind, n.slug)} className={styles.inlineLink}>{n.name}</Link>
|
||||||
</li>
|
</li>
|
||||||
))}
|
))}
|
||||||
</ul>
|
</ul>
|
||||||
|
|||||||
@@ -13,10 +13,26 @@ export interface PlaceSummary {
|
|||||||
slug: string;
|
slug: string;
|
||||||
name: string;
|
name: string;
|
||||||
count: number;
|
count: number;
|
||||||
|
/** Phases that clear the threshold on their own, so the page links
|
||||||
|
* variants that exist rather than 404s. Absent on the registry listing. */
|
||||||
|
phases?: string[];
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface PlaceAuthority {
|
||||||
|
name: string;
|
||||||
|
/** null when that authority has no page of its own — two English
|
||||||
|
* authorities hold fewer schools than the threshold. */
|
||||||
|
slug: string | null;
|
||||||
|
count: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface PlaceDetail {
|
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[];
|
schools: School[];
|
||||||
averages: {
|
averages: {
|
||||||
rwm_expected_pct: number | null;
|
rwm_expected_pct: number | null;
|
||||||
@@ -32,9 +48,20 @@ export function placeUrl(kind: string, slug: string, phase?: string): string {
|
|||||||
return phase ? `${base}/${phase}` : base;
|
return phase ? `${base}/${phase}` : base;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** An authority name as it appears in a URL. */
|
/**
|
||||||
|
* An authority name as it appears in a URL.
|
||||||
|
*
|
||||||
|
* Only a fallback: the API sends the slug it built, and that is what should
|
||||||
|
* be used. This mirrors `_slugify` in backend/app.py, collapsed runs and
|
||||||
|
* trimmed hyphens included, so the two cannot disagree about a name like
|
||||||
|
* "Bristol, City of".
|
||||||
|
*/
|
||||||
export function authoritySlug(name: string): string {
|
export function authoritySlug(name: string): string {
|
||||||
return name.toLowerCase().trim().replace(/[^\w\s-]/g, '').replace(/\s+/g, '-');
|
return name.toLowerCase().trim()
|
||||||
|
.replace(/[^\w\s-]/g, '')
|
||||||
|
.replace(/\s+/g, '-')
|
||||||
|
.replace(/-+/g, '-')
|
||||||
|
.replace(/^-|-$/g, '');
|
||||||
}
|
}
|
||||||
|
|
||||||
const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL
|
const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL
|
||||||
|
|||||||
@@ -5,16 +5,11 @@ clapham,Clapham,SW4,London
|
|||||||
shoreditch,Shoreditch,EC2A|E1,London
|
shoreditch,Shoreditch,EC2A|E1,London
|
||||||
peckham,Peckham,SE15,London
|
peckham,Peckham,SE15,London
|
||||||
brixton,Brixton,SW2|SW9,London
|
brixton,Brixton,SW2|SW9,London
|
||||||
hackney,Hackney,E5|E8|E9,London
|
|
||||||
islington,Islington,N1|N5|N7,London
|
|
||||||
camden-town,Camden Town,NW1,London
|
camden-town,Camden Town,NW1,London
|
||||||
greenwich,Greenwich,SE10,London
|
|
||||||
wimbledon,Wimbledon,SW19,London
|
wimbledon,Wimbledon,SW19,London
|
||||||
putney,Putney,SW15,London
|
putney,Putney,SW15,London
|
||||||
fulham,Fulham,SW6,London
|
fulham,Fulham,SW6,London
|
||||||
chiswick,Chiswick,W4,London
|
chiswick,Chiswick,W4,London
|
||||||
ealing,Ealing,W5|W13,London
|
|
||||||
richmond,Richmond,TW9|TW10,London
|
|
||||||
stratford,Stratford,E15,London
|
stratford,Stratford,E15,London
|
||||||
walthamstow,Walthamstow,E17,London
|
walthamstow,Walthamstow,E17,London
|
||||||
tooting,Tooting,SW17,London
|
tooting,Tooting,SW17,London
|
||||||
|
|||||||
|
Reference in new issue
Block a user