Compare commits
18
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
59265f78b6 | ||
|
|
e953ee7c5f | ||
|
|
413d86cc3c | ||
|
|
4f01fbdedb | ||
|
|
c3ba7aae0d | ||
|
|
54a30de0d8 | ||
|
|
c30ad1db07 | ||
|
|
7424cef7c6 | ||
|
|
01ccbb8e82 | ||
|
|
c339c2f1a1 | ||
|
|
e2ca3d79f9 | ||
|
|
c2364bf09e | ||
|
|
43e0621728 | ||
|
|
d1358cc00f | ||
|
|
865a69b54d | ||
|
|
9cc87c41bb | ||
|
|
8967966eef | ||
|
|
4a9a5c734b |
No files matched your search
+45
-10
@@ -35,6 +35,7 @@ from .data_loader import (
|
|||||||
search_schools_typesense,
|
search_schools_typesense,
|
||||||
)
|
)
|
||||||
from .data_loader import get_data_info as get_db_info
|
from .data_loader import get_data_info as get_db_info
|
||||||
|
from . import flags
|
||||||
from .places import build_place_registry
|
from .places import build_place_registry
|
||||||
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
||||||
from .utils import clean_for_json, convert_to_native
|
from .utils import clean_for_json, convert_to_native
|
||||||
@@ -222,10 +223,10 @@ def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]:
|
|||||||
if p.kind not in kinds:
|
if p.kind not in kinds:
|
||||||
continue
|
continue
|
||||||
rows.append(_url_element(BASE_URL + _place_url(p)))
|
rows.append(_url_element(BASE_URL + _place_url(p)))
|
||||||
# Outcodes carry no phase variants: nobody searches "primary schools
|
# Which phases a place publishes is the registry's decision alone —
|
||||||
# in SW11", so the routes do not exist to submit.
|
# outcodes report none, because the spec gives them no phase route.
|
||||||
if p.kind == "outcode":
|
# Repeating that rule here was how the page and the sitemap came to
|
||||||
continue
|
# disagree about which URLs exist.
|
||||||
for phase in ("primary", "secondary"):
|
for phase in ("primary", "secondary"):
|
||||||
if p.publishes_phase(phase):
|
if p.publishes_phase(phase):
|
||||||
rows.append(_url_element(f"{BASE_URL}{_place_url(p)}/{phase}"))
|
rows.append(_url_element(f"{BASE_URL}{_place_url(p)}/{phase}"))
|
||||||
@@ -459,6 +460,7 @@ def validate_postcode(postcode: Optional[str]) -> Optional[str]:
|
|||||||
async def lifespan(app: FastAPI):
|
async def lifespan(app: FastAPI):
|
||||||
"""Application lifespan - startup and shutdown events."""
|
"""Application lifespan - startup and shutdown events."""
|
||||||
global _sitemaps
|
global _sitemaps
|
||||||
|
flags.init()
|
||||||
print("Loading school data from marts...")
|
print("Loading school data from marts...")
|
||||||
df = load_school_data()
|
df = load_school_data()
|
||||||
if df.empty:
|
if df.empty:
|
||||||
@@ -806,7 +808,13 @@ async def get_school_details(request: Request, urn: int):
|
|||||||
"census": supplementary.get("census"),
|
"census": supplementary.get("census"),
|
||||||
"admissions": supplementary.get("admissions"),
|
"admissions": supplementary.get("admissions"),
|
||||||
"admissions_history": supplementary.get("admissions_history") or [],
|
"admissions_history": supplementary.get("admissions_history") or [],
|
||||||
"admission_distance": supplementary.get("admission_distance"),
|
# Behind a flag, and withheld at the source rather than rendered-but-
|
||||||
|
# hidden: this endpoint is public and unauthenticated, so a field left
|
||||||
|
# in the payload is a published field. The key is absent, not null —
|
||||||
|
# null would state that this school has no cut-off, which is a
|
||||||
|
# different claim from "we are not publishing cut-offs".
|
||||||
|
**({"admission_distance": supplementary.get("admission_distance")}
|
||||||
|
if flags.is_enabled("admission_distance") else {}),
|
||||||
"sen_detail": supplementary.get("sen_detail"),
|
"sen_detail": supplementary.get("sen_detail"),
|
||||||
"phonics": supplementary.get("phonics"),
|
"phonics": supplementary.get("phonics"),
|
||||||
"deprivation": supplementary.get("deprivation"),
|
"deprivation": supplementary.get("deprivation"),
|
||||||
@@ -1200,7 +1208,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")
|
||||||
|
|
||||||
@@ -1212,10 +1221,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
|
||||||
@@ -1235,8 +1249,17 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
# Every authority the place meaningfully sits in. SW19 is
|
# Every authority the place meaningfully sits in. SW19 is
|
||||||
# mostly Merton but partly Wandsworth; naming one asserts
|
# mostly Merton but partly Wandsworth; naming one asserts
|
||||||
# something false.
|
# 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": [
|
"authorities": [
|
||||||
{"name": name, "slug": _slugify(name), "count": n}
|
{"name": name,
|
||||||
|
"slug": (_slugify(name)
|
||||||
|
if f"authority:{_slugify(name)}" in registry
|
||||||
|
else None),
|
||||||
|
"count": n}
|
||||||
for name, n in place.authorities
|
for name, n in place.authorities
|
||||||
],
|
],
|
||||||
# Only phases that clear the threshold, so the page links
|
# Only phases that clear the threshold, so the page links
|
||||||
@@ -1248,6 +1271,18 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@app.get("/api/flags")
|
||||||
|
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
||||||
|
async def get_feature_flags(request: Request):
|
||||||
|
"""Every declared flag and its current value.
|
||||||
|
|
||||||
|
Internal only. The Next proxy denies this path, because the response names
|
||||||
|
every unreleased feature the codebase knows about — which is exactly what
|
||||||
|
shipping dark is meant to keep quiet.
|
||||||
|
"""
|
||||||
|
return flags.all_flags()
|
||||||
|
|
||||||
|
|
||||||
@app.get("/api/data-info")
|
@app.get("/api/data-info")
|
||||||
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
||||||
async def get_data_info(request: Request):
|
async def get_data_info(request: Request):
|
||||||
|
|||||||
@@ -42,6 +42,16 @@ class Settings(BaseSettings):
|
|||||||
typesense_url: str = "http://localhost:8108"
|
typesense_url: str = "http://localhost:8108"
|
||||||
typesense_api_key: str = ""
|
typesense_api_key: str = ""
|
||||||
|
|
||||||
|
# Feature flags (Unleash). An empty unleash_url disables flags entirely and
|
||||||
|
# every flag evaluates False — the correct behaviour for local development
|
||||||
|
# and CI, and the reason no test needs a running Unleash.
|
||||||
|
unleash_url: str = ""
|
||||||
|
unleash_api_token: str = ""
|
||||||
|
unleash_app_name: str = "schoolcompare-backend"
|
||||||
|
# On a named volume, so a restart during an Unleash outage keeps
|
||||||
|
# last-known state instead of reverting a released feature to dark.
|
||||||
|
unleash_cache_directory: str = "/app/.unleash"
|
||||||
|
|
||||||
# Analytics
|
# Analytics
|
||||||
ga_measurement_id: Optional[str] = "G-J0PCVT14NY" # Google Analytics 4 Measurement ID
|
ga_measurement_id: Optional[str] = "G-J0PCVT14NY" # Google Analytics 4 Measurement ID
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,111 @@
|
|||||||
|
"""Feature flags: what can be switched, and what is switched right now.
|
||||||
|
|
||||||
|
Ship-dark, not a kill switch. Flags let work merge and deploy without becoming
|
||||||
|
visible; they are expected to flip about monthly, by a person, deliberately.
|
||||||
|
Nothing here does percentage rollouts or user targeting — the site has no user
|
||||||
|
identity to target.
|
||||||
|
|
||||||
|
Unleash holds the state. It does not hold the list. REGISTRY below is that
|
||||||
|
list, and it exists for three reasons: the SDK evaluates an unknown flag to
|
||||||
|
False, so without a registry that is an *undeclared* False, indistinguishable
|
||||||
|
from a typo; /api/flags needs a key set to return when Unleash is unreachable;
|
||||||
|
and a flag in the UI but not in the registry is orphaned and should be visibly
|
||||||
|
so rather than quietly authoritative.
|
||||||
|
|
||||||
|
Every flag defaults to False. There is no per-flag default, because a flag that
|
||||||
|
defaults on is a kill switch, and this is not one.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import logging
|
||||||
|
from dataclasses import dataclass
|
||||||
|
from datetime import date
|
||||||
|
|
||||||
|
from .config import settings
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# A flag is temporary scaffolding. See test_a_flag_older_than_the_limit.
|
||||||
|
MAX_FLAG_AGE_DAYS = 90
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class Flag:
|
||||||
|
# One string: the registry key, the Unleash flag name, and the JSON key in
|
||||||
|
# /api/flags. snake_case, matching the API's existing convention. No case
|
||||||
|
# transformation anywhere, so there is no mapping layer to get wrong.
|
||||||
|
name: str
|
||||||
|
description: str # one line: what turning this on reveals
|
||||||
|
added: date # for the staleness tripwire
|
||||||
|
|
||||||
|
|
||||||
|
REGISTRY: dict[str, Flag] = {
|
||||||
|
f.name: f for f in (
|
||||||
|
Flag(
|
||||||
|
name="admission_distance",
|
||||||
|
description=(
|
||||||
|
"The last-distance-offered figure on the Admissions tile and "
|
||||||
|
"the 'How far away are you?' section on school pages."
|
||||||
|
),
|
||||||
|
added=date(2026, 8, 23),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
_client = None
|
||||||
|
|
||||||
|
|
||||||
|
def init() -> None:
|
||||||
|
"""Start the Unleash client, or log why flags are all off.
|
||||||
|
|
||||||
|
Called once from the app lifespan. Never raises: a flag system that can
|
||||||
|
stop the API from booting is worse than one that is switched off.
|
||||||
|
"""
|
||||||
|
global _client
|
||||||
|
if not settings.unleash_url or not settings.unleash_api_token:
|
||||||
|
logger.warning(
|
||||||
|
"Unleash is not configured (UNLEASH_URL / UNLEASH_API_TOKEN); "
|
||||||
|
"every feature flag evaluates to False.")
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
from UnleashClient import UnleashClient
|
||||||
|
|
||||||
|
_client = UnleashClient(
|
||||||
|
url=settings.unleash_url,
|
||||||
|
app_name=settings.unleash_app_name,
|
||||||
|
custom_headers={"Authorization": settings.unleash_api_token},
|
||||||
|
cache_directory=settings.unleash_cache_directory,
|
||||||
|
refresh_interval=15,
|
||||||
|
)
|
||||||
|
_client.initialize_client()
|
||||||
|
logger.info("Unleash client initialised against %s", settings.unleash_url)
|
||||||
|
except Exception:
|
||||||
|
# Fail closed and keep serving. The SDK also evaluates everything False
|
||||||
|
# until its first successful sync, so this is the same direction.
|
||||||
|
_client = None
|
||||||
|
logger.exception("Unleash client failed to start; flags are all False.")
|
||||||
|
|
||||||
|
|
||||||
|
def is_enabled(name: str) -> bool:
|
||||||
|
"""Whether `name` is on. False for anything unknown, unreachable or broken."""
|
||||||
|
if name not in REGISTRY:
|
||||||
|
logger.error(
|
||||||
|
"undeclared feature flag %r was evaluated; returning False. "
|
||||||
|
"Add it to backend/flags.py REGISTRY or fix the name.", name)
|
||||||
|
return False
|
||||||
|
if _client is None:
|
||||||
|
return False
|
||||||
|
try:
|
||||||
|
return bool(_client.is_enabled(
|
||||||
|
name, fallback_function=lambda feature_name, context: False))
|
||||||
|
except Exception:
|
||||||
|
logger.exception("flag %r failed to evaluate; returning False", name)
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def all_flags() -> dict[str, bool]:
|
||||||
|
"""Every declared flag and its current value. Serves /api/flags."""
|
||||||
|
return {name: is_enabled(name) for name in REGISTRY}
|
||||||
+44
-5
@@ -59,18 +59,51 @@ 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))
|
||||||
|
|
||||||
|
|
||||||
|
# The measure a phase page is built around. A page with no results in this
|
||||||
|
# column has nothing a list of school names does not already give.
|
||||||
|
_PHASE_METRIC = {
|
||||||
|
"primary": "rwm_expected_pct",
|
||||||
|
"secondary": "attainment_8_score",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def _phase_urns(group, publishable: set[int]) -> dict[str, tuple[int, ...]]:
|
def _phase_urns(group, publishable: set[int]) -> dict[str, tuple[int, ...]]:
|
||||||
"""URNs per phase. All-through schools count toward both, matching the
|
"""URNs per phase, counting only schools with a result for that phase.
|
||||||
PHASE_GROUPS mapping the search filters already use."""
|
|
||||||
|
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
|
from backend.app import PHASE_GROUPS
|
||||||
|
|
||||||
if "phase" not in group.columns:
|
if "phase" not in group.columns:
|
||||||
return {}
|
return {}
|
||||||
lowered = group["phase"].fillna("").str.lower()
|
lowered = group["phase"].fillna("").str.lower()
|
||||||
|
|
||||||
out: dict[str, tuple[int, ...]] = {}
|
out: dict[str, tuple[int, ...]] = {}
|
||||||
for phase in ("primary", "secondary"):
|
for phase in ("primary", "secondary"):
|
||||||
wanted = PHASE_GROUPS.get(phase, set())
|
wanted = PHASE_GROUPS.get(phase, set())
|
||||||
subset = group[lowered.isin(wanted)]
|
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))
|
urns = tuple(sorted({int(u) for u in subset["urn"]} & publishable))
|
||||||
if urns:
|
if urns:
|
||||||
out[phase] = urns
|
out[phase] = urns
|
||||||
@@ -191,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 {}
|
||||||
@@ -206,8 +246,7 @@ def _outcode_places(df, publishable: set[int]) -> dict[str, Place]:
|
|||||||
authorities = _authorities(group)
|
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(authorities),
|
urns=urns, parent_authority=_parent_authority(authorities),
|
||||||
authorities=authorities,
|
authorities=authorities)
|
||||||
phase_urns=_phase_urns(group, publishable))
|
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,163 @@
|
|||||||
|
"""Tests for the feature flag layer (spec 2026-08-23).
|
||||||
|
|
||||||
|
None of these need a running Unleash. That is the point: an unset UNLEASH_URL
|
||||||
|
means every flag is False, which is what local development and CI get.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from datetime import date, timedelta
|
||||||
|
|
||||||
|
from backend import flags
|
||||||
|
|
||||||
|
|
||||||
|
def test_every_declared_flag_is_keyed_by_its_own_name():
|
||||||
|
# One string is the registry key, the Unleash flag name and the JSON key.
|
||||||
|
# A mismatch here would mean the UI toggles a flag the code never reads.
|
||||||
|
for key, flag in flags.REGISTRY.items():
|
||||||
|
assert key == flag.name
|
||||||
|
|
||||||
|
|
||||||
|
def test_flag_names_are_snake_case():
|
||||||
|
# Matches the API's existing convention (admission_distance,
|
||||||
|
# rwm_expected_pct) so no case transformation exists to get wrong.
|
||||||
|
for name in flags.REGISTRY:
|
||||||
|
assert name == name.lower()
|
||||||
|
assert "-" not in name and " " not in name
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_unconfigured_client_evaluates_every_flag_false(monkeypatch):
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
for name in flags.REGISTRY:
|
||||||
|
assert flags.is_enabled(name) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_undeclared_flag_is_false_rather_than_an_error(monkeypatch):
|
||||||
|
# A typo'd flag name must not raise in a request path. It is logged as an
|
||||||
|
# error, because an undeclared flag is always a bug.
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
assert flags.is_enabled("no_such_flag") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_exploding_client_is_false_rather_than_a_500(monkeypatch):
|
||||||
|
class Boom:
|
||||||
|
def is_enabled(self, *a, **kw):
|
||||||
|
raise RuntimeError("unleash is on fire")
|
||||||
|
|
||||||
|
monkeypatch.setattr(flags, "_client", Boom())
|
||||||
|
name = next(iter(flags.REGISTRY))
|
||||||
|
assert flags.is_enabled(name) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_all_flags_reports_every_declared_flag(monkeypatch):
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
assert set(flags.all_flags()) == set(flags.REGISTRY)
|
||||||
|
assert all(v is False for v in flags.all_flags().values())
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_flag_older_than_the_limit_fails_this_test():
|
||||||
|
"""A tripwire, not an assertion about correctness.
|
||||||
|
|
||||||
|
Flags are temporary scaffolding and the failure mode of every flag system
|
||||||
|
is accumulation. This fails on the day a flag turns 90, on whatever PR
|
||||||
|
happens to be open — which is the point: someone has to decide.
|
||||||
|
|
||||||
|
To fix: delete the flag and the branches that read it, or, if it genuinely
|
||||||
|
still needs to exist, move its `added` date and say why in the commit.
|
||||||
|
"""
|
||||||
|
stale = [
|
||||||
|
f.name for f in flags.REGISTRY.values()
|
||||||
|
if date.today() - f.added > timedelta(days=flags.MAX_FLAG_AGE_DAYS)
|
||||||
|
]
|
||||||
|
assert not stale, (
|
||||||
|
f"Flags older than {flags.MAX_FLAG_AGE_DAYS} days: {stale}. "
|
||||||
|
"Remove the flag and the code branches it guards, or move its `added` "
|
||||||
|
"date deliberately."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _client():
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from backend import app as app_module
|
||||||
|
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_flags_endpoint_lists_every_declared_flag(monkeypatch):
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
body = _client().get("/api/flags").json()
|
||||||
|
assert set(body) == set(flags.REGISTRY)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_flags_endpoint_answers_false_when_unleash_is_unreachable(monkeypatch):
|
||||||
|
# The endpoint must still answer. A frontend that cannot read flags renders
|
||||||
|
# everything dark, which is right; one that gets a 500 renders nothing.
|
||||||
|
monkeypatch.setattr(flags, "_client", None)
|
||||||
|
res = _client().get("/api/flags")
|
||||||
|
assert res.status_code == 200
|
||||||
|
assert all(v is False for v in res.json().values())
|
||||||
|
|
||||||
|
|
||||||
|
def _school_payload(monkeypatch, *, flag_on: bool):
|
||||||
|
"""Fetch one school's payload with the distance flag forced on or off.
|
||||||
|
|
||||||
|
The DataFrame shape is copied from test_school_details.py rather than
|
||||||
|
minimised: the endpoint reads a wide set of GIAS columns, and a trimmed
|
||||||
|
frame fails for reasons that have nothing to do with flags.
|
||||||
|
"""
|
||||||
|
import numpy as np
|
||||||
|
import pandas as pd
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from backend import app as app_module
|
||||||
|
|
||||||
|
df = pd.DataFrame([{
|
||||||
|
"urn": 150275,
|
||||||
|
"school_name": "West London Performing Arts Academy",
|
||||||
|
"phase": "Secondary",
|
||||||
|
"school_type": "Special post 16 institution",
|
||||||
|
"trust_name": None,
|
||||||
|
"religious_denomination": "Does not apply",
|
||||||
|
"gender": None,
|
||||||
|
"age_range": "16-25",
|
||||||
|
"admissions_policy": None,
|
||||||
|
"capacity": np.nan,
|
||||||
|
"gias_total_pupils": np.nan,
|
||||||
|
"headteacher_name": None,
|
||||||
|
"website": None,
|
||||||
|
"ofsted_grade": np.nan,
|
||||||
|
"local_authority": "Ealing",
|
||||||
|
"address": "268 Northfield Avenue, London, W5 4UB",
|
||||||
|
"postcode": "W5 4UB",
|
||||||
|
"latitude": 51.4986,
|
||||||
|
"longitude": -0.3148,
|
||||||
|
"year": np.nan,
|
||||||
|
"total_pupils": np.nan,
|
||||||
|
"eligible_pupils": np.nan,
|
||||||
|
"rwm_expected_pct": np.nan,
|
||||||
|
}])
|
||||||
|
|
||||||
|
monkeypatch.setattr(app_module, "load_school_data", lambda: df)
|
||||||
|
# Two arguments: get_supplementary_data(db, urn). See backend/app.py.
|
||||||
|
monkeypatch.setattr(
|
||||||
|
app_module, "get_supplementary_data",
|
||||||
|
lambda db, urn: {"admission_distance": {"distance_m": 772.49,
|
||||||
|
"year": 2024}})
|
||||||
|
monkeypatch.setattr(flags, "is_enabled", lambda name: flag_on)
|
||||||
|
|
||||||
|
client = TestClient(app_module.app, raise_server_exceptions=False)
|
||||||
|
res = client.get("/api/schools/150275")
|
||||||
|
assert res.status_code == 200, res.text
|
||||||
|
return res.json()
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_distance_field_is_absent_when_the_flag_is_off(monkeypatch):
|
||||||
|
"""Absent, not null, and withheld at the source.
|
||||||
|
|
||||||
|
/api/schools/ is public and unauthenticated. Leaving a withheld field in
|
||||||
|
the payload while declining to render it hands the record to anyone who
|
||||||
|
opens the network tab — the reasoning already recorded in c9a1892.
|
||||||
|
"""
|
||||||
|
body = _school_payload(monkeypatch, flag_on=False)
|
||||||
|
assert "admission_distance" not in body
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_distance_field_is_present_when_the_flag_is_on(monkeypatch):
|
||||||
|
body = _school_payload(monkeypatch, flag_on=True)
|
||||||
|
assert body["admission_distance"]["distance_m"] == 772.49
|
||||||
@@ -343,3 +343,78 @@ def test_the_merged_place_takes_its_most_common_spelling():
|
|||||||
+ _town(5, "NEWCASTLE-UNDER-LYME", "Staffordshire", start=400000))
|
+ _town(5, "NEWCASTLE-UNDER-LYME", "Staffordshire", start=400000))
|
||||||
reg = build_place_registry(_df(rows))
|
reg = build_place_registry(_df(rows))
|
||||||
assert reg["town:newcastle-under-lyme"].name == "Newcastle-under-Lyme"
|
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
|
||||||
@@ -288,3 +288,19 @@ def test_outcodes_get_no_phase_variants(place_sitemaps):
|
|||||||
# Nobody searches "primary schools in CM13"; the routes do not exist.
|
# Nobody searches "primary schools in CM13"; the routes do not exist.
|
||||||
xml = place_sitemaps["outcodes-1.xml"]
|
xml = place_sitemaps["outcodes-1.xml"]
|
||||||
assert "/primary" not in xml and "/secondary" not in 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
|
||||||
@@ -16,6 +16,8 @@
|
|||||||
# ADMIN_API_KEY — Backend admin API key
|
# ADMIN_API_KEY — Backend admin API key
|
||||||
# TYPESENSE_API_KEY — Typesense admin API key
|
# TYPESENSE_API_KEY — Typesense admin API key
|
||||||
# TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend)
|
# TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend)
|
||||||
|
# UNLEASH_URL — http://<unleash-ip>:4242/api (empty = all flags off)
|
||||||
|
# UNLEASH_API_TOKEN — Unleash *client* token, environment: development
|
||||||
# AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs)
|
# AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs)
|
||||||
# STAGING_DB_IP — macvlan IP for staging Postgres (default 10.0.1.190)
|
# STAGING_DB_IP — macvlan IP for staging Postgres (default 10.0.1.190)
|
||||||
# STAGING_FRONTEND_IP — macvlan IP for staging frontend (default 10.0.1.151)
|
# STAGING_FRONTEND_IP — macvlan IP for staging frontend (default 10.0.1.151)
|
||||||
@@ -55,6 +57,12 @@ services:
|
|||||||
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
||||||
TYPESENSE_URL: http://typesense:8108
|
TYPESENSE_URL: http://typesense:8108
|
||||||
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
||||||
|
# Unset means every feature flag is False — the correct dark state for an
|
||||||
|
# environment with no Unleash, not a failure.
|
||||||
|
UNLEASH_URL: ${UNLEASH_URL:-}
|
||||||
|
UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-}
|
||||||
|
volumes:
|
||||||
|
- unleash_cache:/app/.unleash
|
||||||
depends_on:
|
depends_on:
|
||||||
sc_database:
|
sc_database:
|
||||||
condition: service_healthy
|
condition: service_healthy
|
||||||
@@ -212,3 +220,4 @@ volumes:
|
|||||||
postgres_data:
|
postgres_data:
|
||||||
typesense_data:
|
typesense_data:
|
||||||
airflow_logs:
|
airflow_logs:
|
||||||
|
unleash_cache:
|
||||||
@@ -0,0 +1,73 @@
|
|||||||
|
# Portainer Stack Definition for School Compare — UNLEASH (feature flags)
|
||||||
|
#
|
||||||
|
# Deploy as a *separate* Portainer stack ("schoolcompare-unleash"), alongside
|
||||||
|
# the production and staging stacks. It deliberately belongs to neither: a
|
||||||
|
# staging redeploy must not be able to disturb production's flag state, and a
|
||||||
|
# production redeploy must not disturb staging's.
|
||||||
|
#
|
||||||
|
# One instance serves both environments. Open-source Unleash ships with
|
||||||
|
# `development` and `production` environments and environment-scoped client
|
||||||
|
# tokens, so the same flag holds independent state in each — which is what
|
||||||
|
# lets a feature be on in staging, where the E2E journeys exercise it, while
|
||||||
|
# production stays dark.
|
||||||
|
#
|
||||||
|
# Portainer environment variables (set in Portainer UI -> Stack -> Environment):
|
||||||
|
# UNLEASH_DB_PASSWORD — PostgreSQL password for the Unleash database
|
||||||
|
# UNLEASH_ADMIN_PASSWORD — initial admin password for the Unleash UI
|
||||||
|
# UNLEASH_IP — macvlan IP for the Unleash server (default 10.0.1.152)
|
||||||
|
|
||||||
|
services:
|
||||||
|
|
||||||
|
# ── PostgreSQL (Unleash's own; nothing else uses it) ──────────────────
|
||||||
|
unleash_db:
|
||||||
|
container_name: sc_unleash_postgres
|
||||||
|
image: postgres:16-alpine
|
||||||
|
environment:
|
||||||
|
POSTGRES_USER: unleash
|
||||||
|
POSTGRES_PASSWORD: ${UNLEASH_DB_PASSWORD}
|
||||||
|
POSTGRES_DB: unleash
|
||||||
|
volumes:
|
||||||
|
- unleash_postgres_data:/var/lib/postgresql/data
|
||||||
|
networks:
|
||||||
|
- unleash
|
||||||
|
healthcheck:
|
||||||
|
test: ["CMD-SHELL", "pg_isready -U unleash"]
|
||||||
|
interval: 10s
|
||||||
|
timeout: 5s
|
||||||
|
retries: 5
|
||||||
|
start_period: 10s
|
||||||
|
restart: unless-stopped
|
||||||
|
|
||||||
|
# ── Unleash server (UI + client API on 4242) ──────────────────────────
|
||||||
|
unleash:
|
||||||
|
container_name: sc_unleash
|
||||||
|
image: unleashorg/unleash-server:6
|
||||||
|
environment:
|
||||||
|
DATABASE_URL: postgres://unleash:${UNLEASH_DB_PASSWORD}@unleash_db:5432/unleash
|
||||||
|
DATABASE_SSL: "false"
|
||||||
|
INIT_ADMIN_API_TOKENS: ""
|
||||||
|
UNLEASH_DEFAULT_ADMIN_PASSWORD: ${UNLEASH_ADMIN_PASSWORD}
|
||||||
|
depends_on:
|
||||||
|
unleash_db:
|
||||||
|
condition: service_healthy
|
||||||
|
networks:
|
||||||
|
unleash: {}
|
||||||
|
macvlan:
|
||||||
|
ipv4_address: ${UNLEASH_IP:-10.0.1.152}
|
||||||
|
healthcheck:
|
||||||
|
test: ["CMD-SHELL", "wget -qO- http://localhost:4242/health || exit 1"]
|
||||||
|
interval: 30s
|
||||||
|
timeout: 10s
|
||||||
|
retries: 3
|
||||||
|
start_period: 30s
|
||||||
|
restart: unless-stopped
|
||||||
|
|
||||||
|
networks:
|
||||||
|
unleash:
|
||||||
|
driver: bridge
|
||||||
|
macvlan:
|
||||||
|
external:
|
||||||
|
name: macvlan
|
||||||
|
|
||||||
|
volumes:
|
||||||
|
unleash_postgres_data:
|
||||||
@@ -7,6 +7,8 @@
|
|||||||
# ADMIN_API_KEY — Backend admin API key
|
# ADMIN_API_KEY — Backend admin API key
|
||||||
# TYPESENSE_API_KEY — Typesense admin API key
|
# TYPESENSE_API_KEY — Typesense admin API key
|
||||||
# TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend)
|
# TYPESENSE_SEARCH_KEY — Typesense search-only key (exposed to frontend)
|
||||||
|
# UNLEASH_URL — http://<unleash-ip>:4242/api (empty = all flags off)
|
||||||
|
# UNLEASH_API_TOKEN — Unleash *client* token, environment: production
|
||||||
# AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs)
|
# AIRFLOW_ADMIN_USER — Airflow admin username (password auto-generated, see api-server logs)
|
||||||
|
|
||||||
services:
|
services:
|
||||||
@@ -44,6 +46,12 @@ services:
|
|||||||
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
||||||
TYPESENSE_URL: http://typesense:8108
|
TYPESENSE_URL: http://typesense:8108
|
||||||
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
||||||
|
# Unset means every feature flag is False — the correct dark state for an
|
||||||
|
# environment with no Unleash, not a failure.
|
||||||
|
UNLEASH_URL: ${UNLEASH_URL:-}
|
||||||
|
UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-}
|
||||||
|
volumes:
|
||||||
|
- unleash_cache:/app/.unleash
|
||||||
depends_on:
|
depends_on:
|
||||||
sc_database:
|
sc_database:
|
||||||
condition: service_healthy
|
condition: service_healthy
|
||||||
@@ -201,3 +209,4 @@ volumes:
|
|||||||
postgres_data:
|
postgres_data:
|
||||||
typesense_data:
|
typesense_data:
|
||||||
airflow_logs:
|
airflow_logs:
|
||||||
|
unleash_cache:
|
||||||
@@ -36,6 +36,10 @@ services:
|
|||||||
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
ADMIN_API_KEY: ${ADMIN_API_KEY:-changeme}
|
||||||
TYPESENSE_URL: http://typesense:8108
|
TYPESENSE_URL: http://typesense:8108
|
||||||
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
TYPESENSE_API_KEY: ${TYPESENSE_API_KEY:-changeme}
|
||||||
|
# Unset means every feature flag is False — the correct dark state for an
|
||||||
|
# environment with no Unleash, not a failure.
|
||||||
|
UNLEASH_URL: ${UNLEASH_URL:-}
|
||||||
|
UNLEASH_API_TOKEN: ${UNLEASH_API_TOKEN:-}
|
||||||
volumes:
|
volumes:
|
||||||
- ./data:/app/data:ro
|
- ./data:/app/data:ro
|
||||||
depends_on:
|
depends_on:
|
||||||
|
|||||||
@@ -150,3 +150,73 @@ token Gitea Actions provides automatically (`secrets.GITEA_TOKEN` — no setup
|
|||||||
needed), and fails the check only when a finding is rated
|
needed), and fails the check only when a finding is rated
|
||||||
**severe** (would break prod, leak data, or corrupt data). Minor findings are
|
**severe** (would break prod, leak data, or corrupt data). Minor findings are
|
||||||
informational and never block a merge.
|
informational and never block a merge.
|
||||||
|
|
||||||
|
## Feature flags (Unleash)
|
||||||
|
|
||||||
|
Flag state lives in a self-hosted Unleash instance, deployed as its own
|
||||||
|
Portainer stack from `docker-compose.portainer.unleash.yml`. It is separate
|
||||||
|
from the application stacks on purpose — redeploying staging must not be able
|
||||||
|
to disturb production's flags.
|
||||||
|
|
||||||
|
The flags themselves are declared in `backend/flags.py`. Unleash holds the
|
||||||
|
state; the registry holds the list. A flag in the UI that is not in the
|
||||||
|
registry is orphaned and nothing reads it.
|
||||||
|
|
||||||
|
### First-time setup
|
||||||
|
|
||||||
|
1. Deploy the stack in Portainer. Set `UNLEASH_DB_PASSWORD`,
|
||||||
|
`UNLEASH_ADMIN_PASSWORD` and (optionally) `UNLEASH_IP`.
|
||||||
|
2. Log in to the UI at `http://<UNLEASH_IP>:4242` as `admin`.
|
||||||
|
3. Create one **client** API token per environment:
|
||||||
|
- `schoolcompare-staging`, environment **development**
|
||||||
|
- `schoolcompare-prod`, environment **production**
|
||||||
|
|
||||||
|
Client tokens, not admin tokens — the backend only reads.
|
||||||
|
4. Put each token in the matching Portainer stack's `UNLEASH_API_TOKEN`
|
||||||
|
variable, and set `UNLEASH_URL` to `http://<UNLEASH_IP>:4242/api`.
|
||||||
|
5. Redeploy the application stacks.
|
||||||
|
|
||||||
|
### Adding a flag to Unleash
|
||||||
|
|
||||||
|
**Unleash does not create flags by itself.** The SDK reads definitions from the
|
||||||
|
server and never registers anything, and metrics for a flag the server has
|
||||||
|
never heard of are discarded. So a flag declared in `backend/flags.py` will be
|
||||||
|
evaluated on every request, stay `False` forever, and never appear in the UI
|
||||||
|
until someone creates it there by hand.
|
||||||
|
|
||||||
|
For each flag in the registry, create one in Unleash with:
|
||||||
|
|
||||||
|
- **Name** — character for character what `backend/flags.py` declares.
|
||||||
|
snake_case, no hyphens or spaces. A typo produces a flag that looks correct
|
||||||
|
in the UI and is read by nothing.
|
||||||
|
- **Type** — Release. No strategies, constraints or variants: these are plain
|
||||||
|
on/off switches, by design.
|
||||||
|
|
||||||
|
### Turning a feature on
|
||||||
|
|
||||||
|
Toggle the flag in the environment matching the stack you mean: **development**
|
||||||
|
for staging, **production** for prod. The token in each stack is scoped to one
|
||||||
|
environment, so toggling the other one has no visible effect.
|
||||||
|
|
||||||
|
The SDK refreshes every 15 seconds, so the API reflects the change almost at
|
||||||
|
once; the pages follow on their own schedule, below.
|
||||||
|
|
||||||
|
A flip reaches school pages within about five minutes and place pages within
|
||||||
|
the hour. Next's ISR does the propagating — it revalidates a route at the
|
||||||
|
*lowest* `revalidate` among that route's fetches, which is 300s for
|
||||||
|
`/school/[slug]` and 3600s for the place pages. There is no webhook, and
|
||||||
|
adding one would only be worth it if flips ever needed to be instant.
|
||||||
|
|
||||||
|
### When Unleash is unreachable
|
||||||
|
|
||||||
|
Every flag evaluates to `False` and the site serves as though nothing were
|
||||||
|
switched on. That is deliberate — an unfinished feature staying hidden is the
|
||||||
|
safe direction — but it means a *released* feature disappears if a backend
|
||||||
|
container cold-starts with an empty cache while Unleash is down. The SDK's
|
||||||
|
disk cache is on a named volume so restarts keep last-known state, and flags
|
||||||
|
are removed from the code within 90 days (enforced by a test), which bounds
|
||||||
|
how long any feature is exposed to this.
|
||||||
|
|
||||||
|
If `UNLEASH_URL` is unset, every flag is `False` and no connection is
|
||||||
|
attempted. That is the correct behaviour for local development and CI, and it
|
||||||
|
means the test suites need no flag server.
|
||||||
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,294 @@
|
|||||||
|
# Feature Flags — Design
|
||||||
|
|
||||||
|
**Date:** 2026-08-23
|
||||||
|
**Status:** approved for planning
|
||||||
|
**First consumer:** the last-distance-offered feature (`admission_distance`)
|
||||||
|
|
||||||
|
## Goal
|
||||||
|
|
||||||
|
Let work merge to `main` and deploy to production without becoming visible,
|
||||||
|
so that releasing a feature stops being the same event as deploying it.
|
||||||
|
|
||||||
|
The site has no way to do this today. A feature is either on `main` and live,
|
||||||
|
or it is on a branch. That forces long-lived branches for anything not ready,
|
||||||
|
and it makes every promotion to production an all-or-nothing decision about
|
||||||
|
everything queued behind it.
|
||||||
|
|
||||||
|
This is a **ship-dark** capability, not a kill switch. Flags are expected to
|
||||||
|
flip on the order of once a month, by a person, deliberately. Nothing here is
|
||||||
|
designed for flipping something off in seconds under pressure, and nothing
|
||||||
|
here does percentage rollouts, user targeting or A/B tests — the site has no
|
||||||
|
user identity to target.
|
||||||
|
|
||||||
|
## Decision: Unleash
|
||||||
|
|
||||||
|
Flag state is held in a self-hosted [Unleash](https://www.getunleash.io/)
|
||||||
|
instance (Apache-2.0), not in the repository.
|
||||||
|
|
||||||
|
A lighter option was considered and rejected by the project owner: a typed
|
||||||
|
registry in each runtime with environment-variable overrides set in the
|
||||||
|
Portainer stack files, which would have needed no new container and kept flag
|
||||||
|
state in git. The argument for Unleash is that it provides a UI and an audit
|
||||||
|
log without a deploy, and that flags are expected to become an ongoing
|
||||||
|
operational tool rather than an occasional one.
|
||||||
|
|
||||||
|
Two consequences follow from choosing a service, and this design exists mostly
|
||||||
|
to handle them:
|
||||||
|
|
||||||
|
1. **Flag state lives outside the repository.** `main` is no longer the whole
|
||||||
|
truth about what is switched on. The registry in §2 exists to bound that.
|
||||||
|
2. **A flag can change without a deploy**, so nothing else clears the caches
|
||||||
|
that a deploy would have cleared. §4 establishes how long a flip takes to
|
||||||
|
become visible, and why that is short enough to need no extra mechanism.
|
||||||
|
|
||||||
|
Also considered: Flagsmith (heavier — Django, Postgres and Redis), GrowthBook
|
||||||
|
(requires MongoDB), and Flipt v2 (the closest conceptual fit, git-native, but
|
||||||
|
now under the Fair Core Licence — source-available, not OSI open source).
|
||||||
|
|
||||||
|
## 1. Topology
|
||||||
|
|
||||||
|
A third Portainer stack, `docker-compose.portainer.unleash.yml`, holding
|
||||||
|
`unleashorg/unleash-server` and its own PostgreSQL 16. It is on the macvlan so
|
||||||
|
both application stacks can reach it, and it belongs to neither of them — a
|
||||||
|
staging redeploy must not be able to disturb production's flag state, and vice
|
||||||
|
versa.
|
||||||
|
|
||||||
|
One instance serves both environments. Open-source Unleash ships with
|
||||||
|
`development` and `production` environments and environment-scoped client
|
||||||
|
tokens, so the same flag holds independent state in each: staging's FastAPI
|
||||||
|
carries a `development` token, production's carries a `production` one.
|
||||||
|
|
||||||
|
That property is what makes ship-dark testable. A feature can be **on in
|
||||||
|
staging and off in production** for as long as it takes, which means the `e2e/`
|
||||||
|
journeys exercise it against staging while production stays unchanged.
|
||||||
|
|
||||||
|
## 2. The registry
|
||||||
|
|
||||||
|
Unleash supplies flag *state* and the toggle UI. It does not supply the list of
|
||||||
|
flags. `backend/flags.py` declares every flag the code knows about:
|
||||||
|
|
||||||
|
```python
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class Flag:
|
||||||
|
name: str # identical in the registry, in Unleash, and in JSON
|
||||||
|
description: str # one line: what turning this on reveals
|
||||||
|
added: date # for the staleness test in §8
|
||||||
|
```
|
||||||
|
|
||||||
|
**Every flag defaults to `False`.** There is no per-flag default field, because
|
||||||
|
a flag that defaults on is not a ship-dark flag — it is a kill switch, and this
|
||||||
|
design does not offer one. A single unconditional default also means the
|
||||||
|
fallback path has no branching to get wrong.
|
||||||
|
|
||||||
|
Three reasons the registry is not optional:
|
||||||
|
|
||||||
|
- The Unleash SDK evaluates an unknown flag to `False`. Without a registry that
|
||||||
|
is an *undeclared* false — indistinguishable from a typo in a flag name.
|
||||||
|
- `/api/flags` needs a key set to return when Unleash is unreachable. It cannot
|
||||||
|
enumerate flags it has never heard of.
|
||||||
|
- A flag present in the Unleash UI but absent from the registry is orphaned,
|
||||||
|
and should be visibly so rather than quietly authoritative.
|
||||||
|
|
||||||
|
**Naming.** One string, used unchanged as the registry key, the Unleash flag
|
||||||
|
name, and the JSON key in `/api/flags`. It is snake_case, matching the API's
|
||||||
|
existing convention (`admission_distance`, `rwm_expected_pct`) and the mirrored
|
||||||
|
types in `nextjs-app/lib/types.ts`. No case transformation anywhere, so there
|
||||||
|
is no mapping layer to get wrong.
|
||||||
|
|
||||||
|
## 3. Read paths
|
||||||
|
|
||||||
|
### Backend
|
||||||
|
|
||||||
|
`backend/flags.py` wraps `UnleashClient` behind `is_enabled(name: str) -> bool`.
|
||||||
|
|
||||||
|
Fail-closed is the default rather than something added: the Python SDK
|
||||||
|
evaluates every flag to `False` until it has synchronised with the server. An
|
||||||
|
unfinished feature therefore stays hidden when Unleash is unreachable, which is
|
||||||
|
the correct direction for ship-dark.
|
||||||
|
|
||||||
|
The SDK's fcache directory is mounted on a named volume so a container restart
|
||||||
|
during an Unleash outage keeps last-known state rather than reverting a
|
||||||
|
released feature to dark. The registry default remains `False`, so the worst
|
||||||
|
case is a feature disappearing, never one appearing.
|
||||||
|
|
||||||
|
### Frontend
|
||||||
|
|
||||||
|
`nextjs-app/lib/flags.ts` exposes `getFlags(): Promise<Flags>`, a single
|
||||||
|
server-side fetch of `/api/flags` returning a typed record. Server components
|
||||||
|
only — no flag value reaches the browser bundle, and `package.json` gains no
|
||||||
|
Unleash dependency. The Unleash client library stays entirely inside the
|
||||||
|
service that already owns every other piece of data the frontend renders.
|
||||||
|
|
||||||
|
The cost, named plainly: a purely front-end flag must still be declared in a
|
||||||
|
Python file. It is a flat data edit rather than programming, and the return is
|
||||||
|
one list, so nobody has to ask which service knows about a given flag.
|
||||||
|
|
||||||
|
### `/api/flags` must not be publicly reachable
|
||||||
|
|
||||||
|
`nextjs-app/app/api/[...path]/route.ts` proxies **everything** under `/api/` to
|
||||||
|
FastAPI. Left alone, `https://www.schoolcompare.co.uk/api/flags` would return
|
||||||
|
`{"admission_distance": false, ...}` — publishing the name and state of every
|
||||||
|
unreleased feature, which defeats the purpose of shipping dark.
|
||||||
|
|
||||||
|
The proxy therefore gains a denylist, and `flags` is on it: a request for a
|
||||||
|
denied path returns 404 rather than being forwarded. Next's own `getFlags()` is
|
||||||
|
unaffected because it calls `FASTAPI_URL` directly across the Docker network
|
||||||
|
and never transits the public proxy.
|
||||||
|
|
||||||
|
This is a general hole rather than a flags-specific one — the proxy will
|
||||||
|
forward any future internal endpoint too — so the denylist is written as a
|
||||||
|
named constant with a comment saying what belongs on it.
|
||||||
|
|
||||||
|
## 4. Propagation
|
||||||
|
|
||||||
|
**Time-based revalidation is sufficient. There is no webhook.**
|
||||||
|
|
||||||
|
An earlier draft of this section specified two Unleash webhooks and a
|
||||||
|
`revalidateTag('flags')` purge, on the premise that pages cache for seven days.
|
||||||
|
That premise was wrong, and checking it removed the most complex part of the
|
||||||
|
design.
|
||||||
|
|
||||||
|
Next uses the **lowest** `revalidate` among a route's fetches to set the
|
||||||
|
revalidation frequency of the whole route — the segment-level
|
||||||
|
`export const revalidate` does not override a lower value inside it. Measured
|
||||||
|
against this codebase:
|
||||||
|
|
||||||
|
| Page family | Segment | Lowest fetch | Effective |
|
||||||
|
|---|---|---|---|
|
||||||
|
| `/school/[slug]` | 604800 | `fetchSchoolDetails` at 300 | **5 minutes** |
|
||||||
|
| `/schools/*` | 604800 | `fetchNationalAverages` at 3600 | **1 hour** |
|
||||||
|
|
||||||
|
The Unleash SDK polls every 15 seconds, so a flip reaches school pages within
|
||||||
|
about five minutes and place pages within the hour, unaided. Flags flip
|
||||||
|
monthly, by hand, deliberately. That is fast enough.
|
||||||
|
|
||||||
|
What this removes: two webhook integrations, a `/api/revalidate-flags` route, a
|
||||||
|
shared-secret-in-a-query-string scheme, an idempotency requirement against
|
||||||
|
duplicate and out-of-order delivery, and a rule that every fetch in
|
||||||
|
`nextjs-app/lib/` carry a cache tag. None of it has to be built, maintained, or
|
||||||
|
kept correct as new fetches are added.
|
||||||
|
|
||||||
|
**If instant flips are ever wanted**, the webhook is the way to add them, and it
|
||||||
|
is purely additive — nothing in this design has to change first.
|
||||||
|
|
||||||
|
### Two constraints this leaves behind
|
||||||
|
|
||||||
|
**Never flag content on a `force-static` page.** `app/admissions/page.tsx`
|
||||||
|
declares `export const dynamic = 'force-static'`, so it is baked at build time
|
||||||
|
and never revalidates. A flag gating anything on such a page would not take
|
||||||
|
effect until the next deploy, silently. If a flag ever needs to reach one, that
|
||||||
|
page must first move to ISR.
|
||||||
|
|
||||||
|
**A route-family flag still needs the sitemap rebuilt.** The sitemap is held in
|
||||||
|
memory and rebuilt only at startup or via `POST /api/admin/regenerate-sitemap`.
|
||||||
|
No flag in scope touches the sitemap (§6), so this is deferred with the route
|
||||||
|
case rather than solved now — but a route flag must not ship without it, or the
|
||||||
|
sitemap will advertise URLs that `notFound()`.
|
||||||
|
|
||||||
|
## 5. What "off" means, per surface
|
||||||
|
|
||||||
|
| Surface | Off |
|
||||||
|
|---|---|
|
||||||
|
| Route | `notFound()`, **and** absent from the sitemap, **and** absent from nav |
|
||||||
|
| UI element | Not rendered; surrounding page byte-identical to today |
|
||||||
|
| API field | Key **absent**, not `null` |
|
||||||
|
| API endpoint | 404, not 403 |
|
||||||
|
|
||||||
|
The three parts of the route rule move together or not at all. Submitting URLs
|
||||||
|
to Google that return 404 is the bug fixed in PR #124, and a flag is a new way
|
||||||
|
to reintroduce it.
|
||||||
|
|
||||||
|
An API field is withheld **at the source**, never rendered-but-hidden. The
|
||||||
|
precedent is already set in this codebase by commit `c9a1892`: `/api/schools/`
|
||||||
|
is public and unauthenticated, so leaving a withheld field in the payload hands
|
||||||
|
the record to anyone who opens the network tab.
|
||||||
|
|
||||||
|
## 6. First consumer: `admission_distance`
|
||||||
|
|
||||||
|
The last-distance-offered feature is merged to `main` and live on staging.
|
||||||
|
Production has never received it: `/api/schools/100010` on production carries
|
||||||
|
no `admission_distance` key, and no Distance section renders.
|
||||||
|
|
||||||
|
It needs **exactly one gate** — `backend/app.py:809`, where the field is
|
||||||
|
attached to the school payload:
|
||||||
|
|
||||||
|
```python
|
||||||
|
"admission_distance": (
|
||||||
|
supplementary.get("admission_distance")
|
||||||
|
if flags.is_enabled("admission_distance") else None
|
||||||
|
),
|
||||||
|
```
|
||||||
|
|
||||||
|
The frontend follows with no change. `DistanceSection` already returns `null`
|
||||||
|
when `admission_distance?.distance_m == null`, and `PrimarySchoolSections`
|
||||||
|
already conditions the admissions block on `(admissions || admissionDistance)`.
|
||||||
|
The off-state is the commonest state on the site — only 57 local authorities
|
||||||
|
publish cut-off distances at all — so it is well covered by construction.
|
||||||
|
|
||||||
|
The flag does not touch the sitemap: school pages exist either way.
|
||||||
|
|
||||||
|
Intended lifecycle: default off, so production receives the code dark on the
|
||||||
|
next promotion; on in the `development` environment so staging keeps testing
|
||||||
|
it; flipped on in `production` when the owner chooses.
|
||||||
|
|
||||||
|
**This flag exercises two of the three surfaces** in §5 — API field and UI
|
||||||
|
element. No route case ships with it. The route rule is specified but unproven
|
||||||
|
until a route-shaped flag exists, and should be treated as such.
|
||||||
|
|
||||||
|
## 7. Testing
|
||||||
|
|
||||||
|
**Backend unit.** The registry is well-formed; an unknown flag evaluates
|
||||||
|
`False`; `/api/flags` returns every declared flag with its default when the
|
||||||
|
SDK is unreachable; `admission_distance` is absent from the school payload when
|
||||||
|
the flag is off and present when on.
|
||||||
|
|
||||||
|
**Frontend unit.** `getFlags()` returns declared defaults when `/api/flags`
|
||||||
|
fails, rather than throwing and taking the page with it.
|
||||||
|
|
||||||
|
**E2E.** Journeys read `/api/flags` and gate flag-dependent assertions on it,
|
||||||
|
matching the `test.skip` shape the suite already uses.
|
||||||
|
|
||||||
|
One trap to avoid, worth stating because the existing distance journeys walk
|
||||||
|
straight into it: they already skip when no school has a published figure, so
|
||||||
|
with the flag off they would skip silently and the suite would go green. The
|
||||||
|
gate must be explicit — **if `/api/flags` reports `admission_distance` on, then
|
||||||
|
a school with a cut-off must be found**, converting a silent skip into a real
|
||||||
|
assertion.
|
||||||
|
|
||||||
|
## 8. Lifecycle
|
||||||
|
|
||||||
|
A flag is temporary scaffolding, and the failure mode of every flag system is
|
||||||
|
accumulation.
|
||||||
|
|
||||||
|
The registry records the date each flag was added, and a backend test fails any
|
||||||
|
flag older than **90 days**. Removing a flag means deleting the registry entry,
|
||||||
|
the branches that read it, and the flag in the Unleash UI.
|
||||||
|
|
||||||
|
Unleash SDK usage metrics stay enabled, so the UI shows which flags are still
|
||||||
|
being evaluated — the evidence needed to retire one safely.
|
||||||
|
|
||||||
|
## 9. Risks
|
||||||
|
|
||||||
|
**Production gains a homelab dependency.** If Unleash is unreachable when a
|
||||||
|
production container cold-starts with an empty cache, every flag evaluates
|
||||||
|
`False` and any feature currently switched on disappears. The fcache volume
|
||||||
|
covers restarts; the 90-day lifecycle rule bounds how long any feature is
|
||||||
|
exposed to this. It is a real regression risk and the reason flags must be
|
||||||
|
retired rather than left on indefinitely.
|
||||||
|
|
||||||
|
**Flag state is not in git.** `main` no longer tells you what production is
|
||||||
|
showing. The registry lists what *can* be flagged; only the Unleash UI says
|
||||||
|
what *is*. This is inherent to the choice of a service.
|
||||||
|
|
||||||
|
**A large promotion backlog exists.** Production is running the
|
||||||
|
pre-SEO-programme build — no place pages, and a sitemap still declaring the
|
||||||
|
apex host. The first promotion after this work ships that entire backlog. The
|
||||||
|
flag isolates the distance feature from it and nothing else.
|
||||||
|
|
||||||
|
## Out of scope
|
||||||
|
|
||||||
|
- Percentage rollouts, user targeting, A/B testing, and Unleash strategies
|
||||||
|
beyond simple on/off. Flags are booleans.
|
||||||
|
- Pipeline and dbt flags. Airflow and dbt are not flag consumers.
|
||||||
|
- Client-side flag evaluation. Flags are server-side only.
|
||||||
|
- Automatic flag removal. The staleness test reports; a person deletes.
|
||||||
+162
-2
@@ -1226,6 +1226,27 @@ const CUTOFF_CANDIDATE_URNS = [
|
|||||||
101099, 100553, 102574, 100769, // mixed
|
101099, 100553, 102574, 100769, // mixed
|
||||||
];
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the last-distance-offered feature is switched on here.
|
||||||
|
*
|
||||||
|
* Read from the data rather than from /api/flags, which the public proxy
|
||||||
|
* denies on purpose — the endpoint names unreleased features. The observable
|
||||||
|
* effect is the field's presence: the flag is off iff no candidate school
|
||||||
|
* carries an `admission_distance` key at all.
|
||||||
|
*
|
||||||
|
* The distinction that matters: `admission_distance: null` means this school
|
||||||
|
* has no published cut-off, and the key being ABSENT means cut-offs are not
|
||||||
|
* being published at all.
|
||||||
|
*/
|
||||||
|
async function distanceFeatureIsOn(page: Page): Promise<boolean> {
|
||||||
|
for (const urn of CUTOFF_CANDIDATE_URNS) {
|
||||||
|
const res = await page.request.get(`/api/schools/${urn}`);
|
||||||
|
if (!res.ok()) continue;
|
||||||
|
if ('admission_distance' in (await res.json())) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
async function schoolWithCutoff(page: Page) {
|
async function schoolWithCutoff(page: Page) {
|
||||||
for (const urn of CUTOFF_CANDIDATE_URNS) {
|
for (const urn of CUTOFF_CANDIDATE_URNS) {
|
||||||
const res = await page.request.get(`/api/schools/${urn}`);
|
const res = await page.request.get(`/api/schools/${urn}`);
|
||||||
@@ -1237,6 +1258,59 @@ async function schoolWithCutoff(page: Page) {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test('when the distance feature is on, a school with a cut-off is findable', async ({ page }) => {
|
||||||
|
/*
|
||||||
|
* The gate that stops the other distance journeys passing vacuously.
|
||||||
|
*
|
||||||
|
* They all skip when schoolWithCutoff() finds nothing, which is right when
|
||||||
|
* the feature is off — but it means a feature that is *supposed* to be on
|
||||||
|
* and is silently broken shows up as a green run full of skips. This test
|
||||||
|
* fails in that case.
|
||||||
|
*/
|
||||||
|
test.skip(!(await distanceFeatureIsOn(page)),
|
||||||
|
'the admission_distance flag is off in this environment');
|
||||||
|
|
||||||
|
expect(await schoolWithCutoff(page),
|
||||||
|
'the distance feature is on, but no candidate school has a cut-off — '
|
||||||
|
+ 'the flag is on and the data or the query behind it is broken')
|
||||||
|
.not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('with the distance feature off, the section is absent rather than empty', async ({ page }) => {
|
||||||
|
// Shipping dark means the page renders as it did before the feature existed,
|
||||||
|
// not as a feature with its content removed.
|
||||||
|
test.skip(await distanceFeatureIsOn(page),
|
||||||
|
'the admission_distance flag is on in this environment');
|
||||||
|
|
||||||
|
// A school that exists, found rather than hardcoded — a 404 page would
|
||||||
|
// satisfy the absent-heading assertion without proving anything.
|
||||||
|
//
|
||||||
|
// A plain loop, not Array.find: find's predicate is synchronous, so an async
|
||||||
|
// one returns a Promise, every Promise is truthy, and it would always hand
|
||||||
|
// back the first URN whether or not that school exists.
|
||||||
|
let urn: number | null = null;
|
||||||
|
for (const candidate of CUTOFF_CANDIDATE_URNS) {
|
||||||
|
if ((await page.request.get(`/api/schools/${candidate}`)).ok()) {
|
||||||
|
urn = candidate;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
expect(urn, 'no candidate school resolves in this environment').not.toBeNull();
|
||||||
|
|
||||||
|
await page.goto(`/school/${urn}`);
|
||||||
|
await expect(page.locator('h1')).toBeVisible();
|
||||||
|
|
||||||
|
await expect(page.getByRole('heading', { name: /How far away are you\?/ }))
|
||||||
|
.toHaveCount(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('/api/flags is not reachable from the public internet', async ({ page }) => {
|
||||||
|
// It names every unreleased feature and whether it is on. Next reads it
|
||||||
|
// server-side over the Docker network; the public proxy must deny it.
|
||||||
|
const res = await page.request.get('/api/flags');
|
||||||
|
expect(res.status()).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
test('a published cut-off distance is shown with the year it belongs to', async ({ page }) => {
|
test('a published cut-off distance is shown with the year it belongs to', async ({ page }) => {
|
||||||
const found = await schoolWithCutoff(page);
|
const found = await schoolWithCutoff(page);
|
||||||
test.skip(found === null, 'no school in the sample has a published cut-off distance yet');
|
test.skip(found === null, 'no school in the sample has a published cut-off distance yet');
|
||||||
@@ -1909,6 +1983,57 @@ test('phase variants are submitted in the places sitemap', async ({ page }) => {
|
|||||||
expect(xml).toMatch(/\/schools\/[a-z0-9-]+\/primary</);
|
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 }) => {
|
test('no page title repeats the brand', async ({ page }) => {
|
||||||
// The root layout appends '| schoolcompare' to a plain-string title. Any
|
// The root layout appends '| schoolcompare' to a plain-string title. Any
|
||||||
// route whose title already carries the brand must opt out with
|
// route whose title already carries the brand must opt out with
|
||||||
@@ -1947,7 +2072,42 @@ test('a place straddling a boundary names every authority it sits in', async ({
|
|||||||
await page.goto(`/schools/near/${straddling!.slug}`);
|
await page.goto(`/schools/near/${straddling!.slug}`);
|
||||||
|
|
||||||
for (const a of detail.place.authorities) {
|
for (const a of detail.place.authorities) {
|
||||||
await expect(page.locator(`a[href="/schools/authority/${a.slug}"]`).first())
|
if (a.slug) {
|
||||||
.toBeVisible();
|
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));
|
||||||
|
});
|
||||||
@@ -0,0 +1,31 @@
|
|||||||
|
/**
|
||||||
|
* The /api/* proxy is public. Anything it forwards is on the internet.
|
||||||
|
*
|
||||||
|
* @jest-environment node
|
||||||
|
*/
|
||||||
|
// The docblock above is load-bearing. jest.config.js sets jsdom globally, and
|
||||||
|
// NextRequest/NextResponse need the Web Fetch API globals that only the node
|
||||||
|
// environment provides — under jsdom this suite fails on import, not on an
|
||||||
|
// assertion.
|
||||||
|
import { NextRequest } from 'next/server';
|
||||||
|
import { GET } from '@/app/api/[...path]/route';
|
||||||
|
|
||||||
|
function request(path: string) {
|
||||||
|
return new NextRequest(`http://localhost:3000/api/${path}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('public API proxy', () => {
|
||||||
|
it('refuses to forward internal-only paths', async () => {
|
||||||
|
// /api/flags names every unreleased feature and its state. Forwarding it
|
||||||
|
// publishes the thing shipping dark exists to keep quiet.
|
||||||
|
const res = await GET(request('flags'), { params: Promise.resolve({ path: ['flags'] }) });
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not deny a path that merely starts with the same letters', async () => {
|
||||||
|
// A prefix match would take /api/flagship down with /api/flags.
|
||||||
|
const res = await GET(
|
||||||
|
request('flagship'), { params: Promise.resolve({ path: ['flagship'] }) });
|
||||||
|
expect(res.status).not.toBe(404);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -243,3 +243,106 @@ describe('PlaceView authorities', () => {
|
|||||||
expect(screen.getByRole('link', { name: 'Merton' })).toBeInTheDocument();
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,34 @@
|
|||||||
|
import { getFlags } from '@/lib/flags';
|
||||||
|
|
||||||
|
// jsdom provides no global fetch, so there is nothing for jest.spyOn to attach
|
||||||
|
// to — assign it and restore the original afterwards. This is the first test
|
||||||
|
// here to mock fetch; later ones should follow this shape.
|
||||||
|
const realFetch = global.fetch;
|
||||||
|
|
||||||
|
function mockFetch(impl: () => Promise<unknown>) {
|
||||||
|
global.fetch = jest.fn(impl) as unknown as typeof fetch;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('getFlags', () => {
|
||||||
|
afterEach(() => { global.fetch = realFetch; });
|
||||||
|
|
||||||
|
it('returns the flags the API reports', async () => {
|
||||||
|
mockFetch(async () => ({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ admission_distance: true }),
|
||||||
|
}));
|
||||||
|
await expect(getFlags()).resolves.toEqual({ admission_distance: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns no flags rather than throwing when the API is down', async () => {
|
||||||
|
// A page that cannot read flags must render everything dark, not 500.
|
||||||
|
// Fail-closed is the same direction as the backend's default.
|
||||||
|
mockFetch(async () => { throw new Error('ECONNREFUSED'); });
|
||||||
|
await expect(getFlags()).resolves.toEqual({});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns no flags rather than throwing on a non-200', async () => {
|
||||||
|
mockFetch(async () => ({ ok: false, status: 503 }));
|
||||||
|
await expect(getFlags()).resolves.toEqual({});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -26,8 +26,27 @@ function backendBase(): string {
|
|||||||
const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection'];
|
const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection'];
|
||||||
const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
|
const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* API paths this public proxy must not forward.
|
||||||
|
*
|
||||||
|
* Matched on the first segment, exactly — a prefix match would take
|
||||||
|
* /api/flagship down with /api/flags.
|
||||||
|
*
|
||||||
|
* `flags` is here because GET /api/flags names every unreleased feature the
|
||||||
|
* codebase knows about, along with whether it is on. Publishing that defeats
|
||||||
|
* the point of shipping dark. Next reads it server-side via FASTAPI_URL, on
|
||||||
|
* the Docker network, which never transits this route.
|
||||||
|
*
|
||||||
|
* Anything else internal-only belongs here too.
|
||||||
|
*/
|
||||||
|
const INTERNAL_ONLY_SEGMENTS = new Set(['flags']);
|
||||||
|
|
||||||
async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) {
|
async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) {
|
||||||
const { path } = await ctx.params;
|
const { path } = await ctx.params;
|
||||||
|
if (INTERNAL_ONLY_SEGMENTS.has(path[0])) {
|
||||||
|
return NextResponse.json({ detail: 'Not Found' }, { status: 404 });
|
||||||
|
}
|
||||||
|
|
||||||
const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`;
|
const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`;
|
||||||
|
|
||||||
const headers = new Headers(req.headers);
|
const headers = new Headers(req.headers);
|
||||||
|
|||||||
@@ -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: { absolute: `${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}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -58,8 +58,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
// place title read '... | schoolcompare | schoolcompare'.
|
// place title read '... | schoolcompare | schoolcompare'.
|
||||||
title: { absolute: `Schools in ${name} — Compare ${count} Schools | 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}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -74,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={[]} />;
|
||||||
|
}
|
||||||
@@ -45,8 +45,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
return {
|
return {
|
||||||
title: { absolute: `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}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -40,8 +40,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
|
|||||||
return {
|
return {
|
||||||
title: { absolute: `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}`) },
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -142,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,
|
||||||
@@ -177,11 +181,18 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
and a third of towns cross a boundary: SW19 is mostly Merton
|
and a third of towns cross a boundary: SW19 is mostly Merton
|
||||||
but partly Wandsworth, and naming one asserts otherwise. */}
|
but partly Wandsworth, and naming one asserts otherwise. */}
|
||||||
{authorities.map((a, i) => (
|
{authorities.map((a, i) => (
|
||||||
<span key={a.slug}>
|
<span key={a.name}>
|
||||||
{i > 0 && (i === authorities.length - 1 ? ' and ' : ', ')}
|
{i > 0 && (i === authorities.length - 1 ? ' and ' : ', ')}
|
||||||
<Link href={`/schools/authority/${a.slug}`} className={styles.inlineLink}>
|
{/* No slug means no page: City of London and the Isles of
|
||||||
{a.name}
|
Scilly hold too few schools for one. Saying where the
|
||||||
</Link>
|
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>
|
</span>
|
||||||
))}
|
))}
|
||||||
</>
|
</>
|
||||||
@@ -192,7 +203,11 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
{!phase && (place.phases ?? []).length > 0 && (
|
{!phase && (place.phases ?? []).length > 0 && (
|
||||||
<nav className={styles.phaseLinks} aria-label="By phase">
|
<nav className={styles.phaseLinks} aria-label="By phase">
|
||||||
{(place.phases ?? []).map((ph) => (
|
{(place.phases ?? []).map((ph) => (
|
||||||
<Link key={ph} href={`/schools/${place.slug}/${ph}`} className={styles.phaseLink}>
|
/* 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}
|
{ph === 'secondary' ? 'Secondary schools' : 'Primary schools'} in {place.name}
|
||||||
</Link>
|
</Link>
|
||||||
))}
|
))}
|
||||||
|
|||||||
@@ -12,6 +12,12 @@ jest.mock('next/navigation', () => ({
|
|||||||
useSearchParams: () => new URLSearchParams(),
|
useSearchParams: () => new URLSearchParams(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// Everything below this line is browser furniture, and this file runs for
|
||||||
|
// every suite — including the ones that declare `@jest-environment node` to
|
||||||
|
// test route handlers, where NextRequest needs Fetch API globals jsdom does
|
||||||
|
// not provide. There is no `window` there, so guard rather than assume one.
|
||||||
|
if (typeof window !== 'undefined') {
|
||||||
|
|
||||||
// Mock window.matchMedia
|
// Mock window.matchMedia
|
||||||
Object.defineProperty(window, 'matchMedia', {
|
Object.defineProperty(window, 'matchMedia', {
|
||||||
writable: true,
|
writable: true,
|
||||||
@@ -52,3 +58,5 @@ const localStorageMock = {
|
|||||||
clear: jest.fn(),
|
clear: jest.fn(),
|
||||||
};
|
};
|
||||||
global.localStorage = localStorageMock;
|
global.localStorage = localStorageMock;
|
||||||
|
|
||||||
|
} // end: browser-only globals
|
||||||
@@ -0,0 +1,40 @@
|
|||||||
|
/**
|
||||||
|
* Reading feature flags.
|
||||||
|
*
|
||||||
|
* Server-side only. No flag value reaches the browser bundle, and there is no
|
||||||
|
* Unleash dependency in package.json — the SDK lives in FastAPI, which already
|
||||||
|
* owns every other piece of data this app renders.
|
||||||
|
*
|
||||||
|
* Flags are declared in backend/flags.py. A purely front-end flag still has to
|
||||||
|
* be declared there; it is a flat data edit, and the return is that one list
|
||||||
|
* answers "what flags exist" for the whole system.
|
||||||
|
*/
|
||||||
|
|
||||||
|
export type Flags = Record<string, boolean>;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Reading flags pins the calling route to this ISR floor: Next uses the LOWEST
|
||||||
|
* revalidate among a route's fetches to set the whole route's revalidation
|
||||||
|
* frequency. 300s matches what /school/[slug] already sits at, so a page that
|
||||||
|
* reads flags is no more dynamic than a school page already is.
|
||||||
|
*
|
||||||
|
* It is also what makes a flip propagate without a webhook: five minutes on
|
||||||
|
* school pages, an hour on place pages, against flags that flip monthly.
|
||||||
|
*/
|
||||||
|
export const FLAGS_REVALIDATE = 300;
|
||||||
|
|
||||||
|
const API = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL
|
||||||
|
|| 'http://localhost:8000/api';
|
||||||
|
|
||||||
|
/** Every flag and its value. Never throws: an unreadable flag is a dark one. */
|
||||||
|
export async function getFlags(): Promise<Flags> {
|
||||||
|
try {
|
||||||
|
const res = await fetch(`${API}/flags`, {
|
||||||
|
next: { revalidate: FLAGS_REVALIDATE },
|
||||||
|
});
|
||||||
|
if (!res.ok) return {};
|
||||||
|
return await res.json();
|
||||||
|
} catch {
|
||||||
|
return {};
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -20,7 +20,9 @@ export interface PlaceSummary {
|
|||||||
|
|
||||||
export interface PlaceAuthority {
|
export interface PlaceAuthority {
|
||||||
name: string;
|
name: string;
|
||||||
slug: 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;
|
count: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -46,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
|
||||||
|
|||||||
@@ -361,7 +361,12 @@ export interface SchoolDetailsResponse {
|
|||||||
* held back as a paid feature and are not part of this public payload — see
|
* held back as a paid feature and are not part of this public payload — see
|
||||||
* data_loader._admission_distance.
|
* data_loader._admission_distance.
|
||||||
*/
|
*/
|
||||||
admission_distance: SchoolAdmissionDistance | null;
|
/**
|
||||||
|
* Absent — not null — when the admission_distance flag is off. Null means
|
||||||
|
* "this school has no published cut-off"; absent means "cut-offs are not
|
||||||
|
* being published at all". They are different claims and the type says so.
|
||||||
|
*/
|
||||||
|
admission_distance?: SchoolAdmissionDistance | null;
|
||||||
deprivation: SchoolDeprivation | null;
|
deprivation: SchoolDeprivation | null;
|
||||||
finance: SchoolFinance | null;
|
finance: SchoolFinance | null;
|
||||||
}
|
}
|
||||||
|
|||||||
+1
-1
@@ -12,4 +12,4 @@ slowapi==0.1.9
|
|||||||
secure==0.3.0
|
secure==0.3.0
|
||||||
typesense==0.21.0
|
typesense==0.21.0
|
||||||
numpy==1.26.4
|
numpy==1.26.4
|
||||||
|
UnleashClient==6.0.1
|
||||||
Reference in new issue
Block a user