Compare commits
25
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
9a1f56c431 | ||
|
|
ade9dbb3ba | ||
|
|
d1a8596208 | ||
|
|
a3c09d9b67 | ||
|
|
a7f4c86464 | ||
|
|
0804566736 | ||
|
|
55363cbd18 | ||
|
|
868eb344f5 | ||
|
|
0b15497c09 | ||
|
|
d55f6cce23 | ||
|
|
3236efa846 | ||
|
|
d5a6db289d | ||
|
|
d8ccb5b733 | ||
|
|
0fa1a292c7 | ||
|
|
c3f044bd65 | ||
|
|
d2115364ae | ||
|
|
28cf0a342c | ||
|
|
d88e77f459 | ||
|
|
06eb433db5 | ||
|
|
1a6d349dad | ||
|
|
75d3534d82 | ||
|
|
ff041544f2 | ||
|
|
6e0a278340 | ||
|
|
e651dd0d65 | ||
|
|
22c113fc29 |
No files matched your search
+151
-6
@@ -6,6 +6,7 @@ Uses real data from UK Government Compare School Performance downloads.
|
||||
|
||||
import hashlib
|
||||
import re
|
||||
import time
|
||||
from contextlib import asynccontextmanager
|
||||
from datetime import datetime, timezone
|
||||
from typing import Optional
|
||||
@@ -15,7 +16,7 @@ import pandas as pd
|
||||
from fastapi import FastAPI, HTTPException, Query, Request, Depends, Header
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from fastapi.middleware.gzip import GZipMiddleware
|
||||
from fastapi.responses import FileResponse, Response
|
||||
from fastapi.responses import FileResponse, JSONResponse, Response
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from slowapi import Limiter, _rate_limit_exceeded_handler
|
||||
from slowapi.util import get_remote_address
|
||||
@@ -33,6 +34,7 @@ from .data_loader import (
|
||||
get_supplementary_data,
|
||||
get_supplementary_data_batch,
|
||||
search_schools_typesense,
|
||||
suggest_schools_typesense,
|
||||
)
|
||||
from .data_loader import get_data_info as get_db_info
|
||||
from . import flags
|
||||
@@ -296,8 +298,101 @@ def clean_filter_values(series: pd.Series) -> list[str]:
|
||||
# SECURITY MIDDLEWARE & HELPERS
|
||||
# =============================================================================
|
||||
|
||||
# Rate limiter
|
||||
limiter = Limiter(key_func=get_remote_address)
|
||||
def client_key(request: Request) -> str:
|
||||
"""The rate-limit bucket: the real caller, not the proxy in front of them.
|
||||
|
||||
`get_remote_address` reads request.client.host. In staging and production
|
||||
the backend has no published ports and sits on the internal network, so its
|
||||
only caller is the Next proxy — meaning every browser user on the site
|
||||
shared one bucket. Measured before this fix: 70 concurrent requests to
|
||||
/api/schools returned 60 OK and 10 refused.
|
||||
|
||||
CF-Connecting-IP first, because Cloudflare (in front of both environments)
|
||||
sets it on every origin request and *overwrites* any client-supplied value,
|
||||
which a parsed X-Forwarded-For chain does not guarantee. The XFF fallback is
|
||||
forgeable, but only by a caller already inside the Docker network, which is
|
||||
the one place nothing untrusted can reach.
|
||||
"""
|
||||
cf = request.headers.get("cf-connecting-ip")
|
||||
if cf:
|
||||
return cf.strip()
|
||||
xff = request.headers.get("x-forwarded-for")
|
||||
if xff:
|
||||
return xff.split(",")[0].strip()
|
||||
return get_remote_address(request)
|
||||
|
||||
|
||||
# Per-client limiter. Paired with the global ceiling below — the two do
|
||||
# different jobs and neither substitutes for the other.
|
||||
limiter = Limiter(key_func=client_key)
|
||||
|
||||
|
||||
# --- The ceiling no header can raise ----------------------------------------
|
||||
#
|
||||
# client_key trusts CF-Connecting-IP, and nothing in this process can tell an
|
||||
# edge-set header from an attacker-set one. That distinction can only be made
|
||||
# at Cloudflare, with Authenticated Origin Pulls or an origin firewall. A
|
||||
# caller reaching the origin directly could otherwise mint a fresh rate-limit
|
||||
# bucket per request and evade per-client limits entirely — which would make
|
||||
# correct keying a net regression against abuse, since the single shared bucket
|
||||
# it replaced at least capped everyone at 60/minute together.
|
||||
#
|
||||
# So per-client limits give fairness, and this gives the origin a hard total.
|
||||
# It does not make the header trustworthy; it bounds what trusting it can cost.
|
||||
# The header problem itself is closed at Cloudflare, not here.
|
||||
#
|
||||
# [window_start_monotonic, count], or None before the first request. A fixed
|
||||
# window is crude, which is right for a backstop: it has to be obviously
|
||||
# correct rather than fair.
|
||||
_global_window: Optional[list] = None
|
||||
|
||||
# The container healthcheck runs `curl http://localhost:80/api/data-info` from
|
||||
# inside the container. Starving it would fail the check, restart the
|
||||
# container, and turn a load spike into an outage loop — the ceiling exists to
|
||||
# protect the origin, not to kill it.
|
||||
_LOCAL_HOSTS = frozenset({"127.0.0.1", "::1", "localhost"})
|
||||
|
||||
|
||||
def exempt_from_ceiling(request: Request) -> bool:
|
||||
"""Whether the ceiling should ignore this request.
|
||||
|
||||
Its own function so the rule is testable without standing up a server —
|
||||
and so the healthcheck exemption is somewhere a reader can find it.
|
||||
"""
|
||||
if not request.url.path.startswith("/api/"):
|
||||
return True
|
||||
# The peer address, never the Host header: Host is set by the caller and
|
||||
# would hand every attacker an exemption.
|
||||
return (request.client.host if request.client else "") in _LOCAL_HOSTS
|
||||
|
||||
|
||||
class GlobalRateLimitMiddleware(BaseHTTPMiddleware):
|
||||
"""A cap on total /api/ traffic, independent of any client identity."""
|
||||
|
||||
async def dispatch(self, request: Request, call_next):
|
||||
global _global_window
|
||||
|
||||
if exempt_from_ceiling(request):
|
||||
return await call_next(request)
|
||||
|
||||
now = time.monotonic()
|
||||
# One event loop, and no await between the read and the write, so this
|
||||
# sequence is atomic without a lock.
|
||||
if _global_window is None or now - _global_window[0] >= 60:
|
||||
_global_window = [now, 0]
|
||||
_global_window[1] += 1
|
||||
|
||||
if _global_window[1] > settings.global_rate_limit_per_minute:
|
||||
return JSONResponse(
|
||||
# Distinguishable from slowapi's per-client 429: an operator
|
||||
# reading logs has to be able to tell "one noisy client" from
|
||||
# "the origin is saturated".
|
||||
{"detail": "The service is at capacity. Please retry shortly."},
|
||||
status_code=429,
|
||||
headers={"Retry-After":
|
||||
str(max(1, int(60 - (now - _global_window[0]))))},
|
||||
)
|
||||
return await call_next(request)
|
||||
|
||||
|
||||
class SecurityHeadersMiddleware(BaseHTTPMiddleware):
|
||||
@@ -355,6 +450,7 @@ CACHE_RULES: list[tuple[str, tuple[int, int, int]]] = [
|
||||
("/api/schools/", (300, 3600, 86400)), # /api/schools/{urn}
|
||||
("/api/rankings", (60, 600, 3600)),
|
||||
("/api/compare", (60, 600, 3600)),
|
||||
("/api/suggest", (60, 3600, 86400)), # autosuggest
|
||||
("/api/schools", (30, 300, 1800)), # search list
|
||||
]
|
||||
|
||||
@@ -502,6 +598,10 @@ app.add_middleware(CacheAndETagMiddleware)
|
||||
app.add_middleware(SecurityHeadersMiddleware)
|
||||
app.add_middleware(RequestSizeLimitMiddleware)
|
||||
app.add_middleware(GZipMiddleware, minimum_size=512)
|
||||
# Added last, so it is outermost and refuses before anything downstream does
|
||||
# work. A ceiling that only applies after the expensive part has run is not a
|
||||
# ceiling.
|
||||
app.add_middleware(GlobalRateLimitMiddleware)
|
||||
|
||||
# CORS middleware - restricted for production
|
||||
app.add_middleware(
|
||||
@@ -1237,9 +1337,22 @@ async def get_place(request: Request, kind: str, slug: str,
|
||||
for m in ("rwm_expected_pct", "attainment_8_score")
|
||||
}
|
||||
|
||||
cols = [c for c in SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
|
||||
"rwm_expected_pct", "attainment_8_score",
|
||||
"total_pupils"]
|
||||
# dict.fromkeys, not a list: SCHOOL_COLUMNS already ends with latitude and
|
||||
# longitude, so concatenating them again selected each twice and pandas
|
||||
# dropped one of every duplicated pair with a "columns are not unique"
|
||||
# warning. Ordered de-duplication keeps the column order and the warning
|
||||
# cannot come back.
|
||||
#
|
||||
# nursery_provision and parliamentary_constituency are not in
|
||||
# SCHOOL_COLUMNS and the place table shows both. The `in rows.columns`
|
||||
# guard is what keeps a mart the pipeline has not rebuilt working: those
|
||||
# two are the optional GIAS columns data_loader degrades to NULL.
|
||||
cols = [c for c in dict.fromkeys(
|
||||
SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
|
||||
"nursery_provision",
|
||||
"parliamentary_constituency",
|
||||
"rwm_expected_pct", "attainment_8_score",
|
||||
"total_pupils"])
|
||||
if c in rows.columns]
|
||||
|
||||
return {
|
||||
@@ -1271,6 +1384,38 @@ async def get_place(request: Request, kind: str, slug: str,
|
||||
}
|
||||
|
||||
|
||||
# Two characters. One is not a query — it matches thousands of schools and the
|
||||
# response is useless, so it is not worth a round trip.
|
||||
SUGGEST_MIN_QUERY = 2
|
||||
|
||||
|
||||
@app.get("/api/suggest")
|
||||
@limiter.limit("120/minute")
|
||||
async def suggest_schools(
|
||||
request: Request,
|
||||
q: str = Query("", max_length=100),
|
||||
limit: int = Query(8, ge=1, le=20),
|
||||
):
|
||||
"""School name suggestions, from Typesense alone.
|
||||
|
||||
Deliberately not a mode of /api/schools: that path filters and sorts the
|
||||
full in-memory DataFrame, which is far too expensive to run per keystroke.
|
||||
|
||||
Nothing here returns an error for ordinary input. A short query, no
|
||||
matches, or Typesense being unreachable are all 200 with an empty list —
|
||||
a dropdown that quietly does not appear is the right failure for a
|
||||
keystroke path, and there is no DataFrame fallback because the 25,000-row
|
||||
substring scan is precisely what this endpoint exists to avoid.
|
||||
|
||||
120/minute rather than the default 60: a 200 ms debounce makes typing
|
||||
legitimately bursty.
|
||||
"""
|
||||
query = q.strip()
|
||||
if len(query) < SUGGEST_MIN_QUERY:
|
||||
return {"suggestions": []}
|
||||
return {"suggestions": suggest_schools_typesense(query, limit)}
|
||||
|
||||
|
||||
@app.get("/api/flags")
|
||||
@limiter.limit(f"{settings.rate_limit_per_minute}/minute")
|
||||
async def get_feature_flags(request: Request):
|
||||
|
||||
@@ -35,6 +35,11 @@ class Settings(BaseSettings):
|
||||
# Security
|
||||
admin_api_key: str = Field(default_factory=lambda: secrets.token_urlsafe(32))
|
||||
rate_limit_per_minute: int = 60 # Requests per minute per IP
|
||||
# A ceiling on total /api/ traffic, independent of any client identity.
|
||||
# client_key trusts headers only Cloudflare can vouch for, so a caller
|
||||
# reaching the origin directly could otherwise mint a fresh bucket per
|
||||
# request. See GlobalRateLimitMiddleware in backend/app.py.
|
||||
global_rate_limit_per_minute: int = 3000
|
||||
rate_limit_burst: int = 10 # Allow burst of requests
|
||||
max_request_size: int = 1024 * 1024 # 1MB max request size
|
||||
|
||||
|
||||
@@ -100,6 +100,58 @@ def search_schools_typesense(query: str, limit: int = 250) -> List[int]:
|
||||
return []
|
||||
|
||||
|
||||
# The most a public endpoint will return in one response.
|
||||
SUGGEST_MAX_LIMIT = 20
|
||||
|
||||
# Fields a suggestion row carries, and the default when the document omits an
|
||||
# optional one. phase and school_type are optional in the Typesense schema.
|
||||
_SUGGEST_FIELDS = ("school_name", "local_authority", "postcode",
|
||||
"phase", "school_type")
|
||||
|
||||
|
||||
def suggest_schools_typesense(query: str, limit: int = 8) -> List[dict]:
|
||||
"""Autosuggest rows straight from Typesense. Never raises.
|
||||
|
||||
Returns documents rather than URNs, unlike search_schools_typesense, so the
|
||||
caller needs no DataFrame. Every field below is already in the index — see
|
||||
pipeline/scripts/sync_typesense.py — which is what makes this cheap enough
|
||||
to run per keystroke.
|
||||
"""
|
||||
client = _get_typesense_client()
|
||||
if client is None:
|
||||
return []
|
||||
try:
|
||||
result = client.collections["schools"].documents.search({
|
||||
"q": query,
|
||||
"query_by": "school_name,local_authority",
|
||||
"per_page": max(1, min(limit, SUGGEST_MAX_LIMIT)),
|
||||
"typo_tokens_threshold": 1,
|
||||
})
|
||||
except Exception:
|
||||
# A dropdown that quietly stops appearing is the right failure here.
|
||||
return []
|
||||
|
||||
rows = []
|
||||
for hit in result.get("hits", []) or []:
|
||||
doc = (hit or {}).get("document") or {}
|
||||
try:
|
||||
urn = int(doc["urn"])
|
||||
except (KeyError, TypeError, ValueError):
|
||||
# Skip the row, keep the rest. Typesense declares urn as int32 so
|
||||
# this should be unreachable, but the index is a separate system
|
||||
# that something other than this code can reindex — and "never
|
||||
# raises" is a promise the keystroke path actually depends on.
|
||||
# Dropping one malformed document is right; blanking the whole
|
||||
# dropdown, or serving a suggestion pointing at /school/0, is not.
|
||||
logging.getLogger(__name__).warning(
|
||||
"skipping malformed suggestion document: %r", doc)
|
||||
continue
|
||||
row = {"urn": urn}
|
||||
row.update({f: str(doc.get(f, "") or "") for f in _SUGGEST_FIELDS})
|
||||
rows.append(row)
|
||||
return rows
|
||||
|
||||
|
||||
def normalize_school_type(school_type: Optional[str]) -> Optional[str]:
|
||||
"""Convert cryptic school type codes to user-friendly names."""
|
||||
if not school_type:
|
||||
|
||||
@@ -50,6 +50,13 @@ REGISTRY: dict[str, Flag] = {
|
||||
),
|
||||
added=date(2026, 8, 23),
|
||||
),
|
||||
Flag(
|
||||
name="school_autosuggest",
|
||||
description=(
|
||||
"School name suggestions as you type in the main search box."
|
||||
),
|
||||
added=date(2026, 8, 26),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -137,3 +137,49 @@ def test_an_authority_without_a_page_is_named_but_carries_no_slug(straddling_cli
|
||||
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
|
||||
|
||||
|
||||
def _attributed_df() -> pd.DataFrame:
|
||||
"""The same town, with the four attributes the place table now shows."""
|
||||
df = _schools_df()
|
||||
df["age_range"] = "4-11"
|
||||
df["religious_denomination"] = "Church of England"
|
||||
df["nursery_provision"] = True
|
||||
df["parliamentary_constituency"] = "Brentwood and Ongar"
|
||||
return df
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def attributed_client(monkeypatch):
|
||||
from backend import app as app_module
|
||||
|
||||
monkeypatch.setattr(app_module, "load_school_data", _attributed_df)
|
||||
monkeypatch.setattr(app_module, "load_latest_school_data", _attributed_df)
|
||||
monkeypatch.setattr(app_module, "_place_registry", None)
|
||||
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
def test_place_detail_carries_the_attributes_the_table_shows(attributed_client):
|
||||
"""age_range and religious_denomination ride in on SCHOOL_COLUMNS.
|
||||
|
||||
nursery_provision and parliamentary_constituency do not, and the place
|
||||
table needs all four — a column the response cannot fill is a column of
|
||||
dashes on ~3,900 pages.
|
||||
"""
|
||||
body = attributed_client.get("/api/places/town/brentwood").json()
|
||||
school = body["schools"][0]
|
||||
assert school["age_range"] == "4-11"
|
||||
assert school["religious_denomination"] == "Church of England"
|
||||
assert school["nursery_provision"] is True
|
||||
assert school["parliamentary_constituency"] == "Brentwood and Ongar"
|
||||
|
||||
|
||||
def test_place_detail_survives_a_mart_without_the_optional_columns(client):
|
||||
"""The base fixture has neither column, as an unrebuilt mart does not.
|
||||
|
||||
data_loader degrades those to NULL rather than failing the load, so the
|
||||
endpoint must not assume they are present.
|
||||
"""
|
||||
res = client.get("/api/places/town/brentwood")
|
||||
assert res.status_code == 200
|
||||
assert "nursery_provision" not in res.json()["schools"][0]
|
||||
@@ -0,0 +1,147 @@
|
||||
"""The rate-limit bucket must be the caller, not the proxy in front of them.
|
||||
|
||||
`get_remote_address` reads request.client.host. In staging and production the
|
||||
backend has no published ports and its only caller is the Next proxy, so that
|
||||
host is the Next container — one bucket for every browser user on the site.
|
||||
Measured before this fix: 70 concurrent requests, 60 served and 10 refused.
|
||||
"""
|
||||
|
||||
from starlette.datastructures import Headers
|
||||
|
||||
from backend.app import client_key
|
||||
|
||||
|
||||
class _Req:
|
||||
"""Enough of a Request for the key function: headers and a client host."""
|
||||
|
||||
def __init__(self, headers: dict, host: str = "10.0.0.9"):
|
||||
self.headers = Headers(headers)
|
||||
self.client = type("C", (), {"host": host})()
|
||||
self.scope = {"type": "http", "client": (host, 0),
|
||||
"headers": [(k.lower().encode(), v.encode())
|
||||
for k, v in headers.items()]}
|
||||
|
||||
|
||||
def test_cloudflare_header_wins():
|
||||
# Cloudflare sets CF-Connecting-IP and overwrites any client-supplied
|
||||
# value, so it is trustworthy in a way a parsed XFF chain is not.
|
||||
assert client_key(_Req({"cf-connecting-ip": "203.0.113.7"})) == "203.0.113.7"
|
||||
|
||||
|
||||
def test_forwarded_for_is_the_fallback_and_takes_the_first_entry():
|
||||
# Left-most is the original client; everything after it is proxies.
|
||||
assert client_key(
|
||||
_Req({"x-forwarded-for": "203.0.113.7, 10.0.0.2"})) == "203.0.113.7"
|
||||
|
||||
|
||||
def test_remote_address_is_the_last_resort():
|
||||
assert client_key(_Req({}, host="10.0.0.9")) == "10.0.0.9"
|
||||
|
||||
|
||||
def test_cloudflare_header_beats_forwarded_for():
|
||||
key = client_key(_Req({"cf-connecting-ip": "203.0.113.7",
|
||||
"x-forwarded-for": "198.51.100.1"}))
|
||||
assert key == "203.0.113.7"
|
||||
|
||||
|
||||
def test_two_callers_get_two_buckets():
|
||||
# The whole point: one user exhausting their limit must not refuse another.
|
||||
a = client_key(_Req({"cf-connecting-ip": "203.0.113.7"}))
|
||||
b = client_key(_Req({"cf-connecting-ip": "203.0.113.8"}))
|
||||
assert a != b
|
||||
|
||||
|
||||
def test_whitespace_is_stripped():
|
||||
# "a, b" split on comma leaves a leading space on every entry but the
|
||||
# first; an unstripped key silently creates a second bucket per client.
|
||||
assert client_key(_Req({"x-forwarded-for": " 203.0.113.7 ,10.0.0.2"})) \
|
||||
== "203.0.113.7"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The ceiling that header rotation cannot raise.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def api(monkeypatch):
|
||||
from backend import app as app_module
|
||||
from backend.config import settings
|
||||
|
||||
monkeypatch.setattr(settings, "global_rate_limit_per_minute", 5)
|
||||
monkeypatch.setattr(app_module, "_global_window", None)
|
||||
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
|
||||
def _ceiling_req(path: str, host: str):
|
||||
"""Enough of a Request for exempt_from_ceiling: a path and a peer host."""
|
||||
return type("R", (), {
|
||||
"url": type("U", (), {"path": path})(),
|
||||
"client": type("C", (), {"host": host})(),
|
||||
})()
|
||||
|
||||
|
||||
def _get(client, path="/api/flags", cf=None):
|
||||
headers = {"cf-connecting-ip": cf} if cf else {}
|
||||
return client.get(path, headers=headers)
|
||||
|
||||
|
||||
def test_rotating_the_cloudflare_header_cannot_buy_unlimited_requests(api):
|
||||
"""The attack the per-client keying opened up.
|
||||
|
||||
client_key trusts CF-Connecting-IP, and nothing in this process can tell an
|
||||
edge-set header from an attacker-set one — that distinction can only be
|
||||
made at Cloudflare, with Authenticated Origin Pulls or an origin firewall.
|
||||
A caller reaching the origin directly can therefore mint a fresh
|
||||
rate-limit bucket per request and evade per-client limits entirely.
|
||||
|
||||
Per-client fairness is still the right default; this is the backstop that
|
||||
bounds what evading it can achieve. Without it, correct keying would be a
|
||||
net regression against abuse compared with the shared bucket it replaced.
|
||||
"""
|
||||
codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(8)]
|
||||
assert codes.count(200) == 5
|
||||
assert codes.count(429) == 3
|
||||
|
||||
|
||||
def test_the_ceiling_says_which_limit_was_hit(api):
|
||||
# Distinguishable from slowapi's per-client 429, or an operator reading
|
||||
# logs cannot tell "one noisy client" from "the origin is saturated".
|
||||
for i in range(5):
|
||||
_get(api, cf=f"203.0.113.{i}")
|
||||
refused = _get(api, cf="203.0.113.99")
|
||||
assert refused.status_code == 429
|
||||
assert "capacity" in refused.json()["detail"].lower()
|
||||
assert refused.headers.get("retry-after")
|
||||
|
||||
|
||||
def test_traffic_below_the_ceiling_is_untouched(api):
|
||||
codes = [_get(api, cf=f"203.0.113.{i}").status_code for i in range(5)]
|
||||
assert codes == [200] * 5
|
||||
|
||||
|
||||
def test_the_container_healthcheck_is_exempt(api):
|
||||
"""The healthcheck runs `curl http://localhost:80/api/data-info` inside the
|
||||
container. If the ceiling could starve it, saturation would fail the
|
||||
healthcheck, restart the container, and turn a load spike into an outage
|
||||
loop — the ceiling has to protect the origin, not kill it.
|
||||
"""
|
||||
from backend.app import exempt_from_ceiling
|
||||
|
||||
assert exempt_from_ceiling(_ceiling_req("/api/data-info", "127.0.0.1"))
|
||||
assert exempt_from_ceiling(_ceiling_req("/api/data-info", "::1"))
|
||||
# Everyone else is counted.
|
||||
assert not exempt_from_ceiling(_ceiling_req("/api/data-info", "10.0.0.9"))
|
||||
|
||||
|
||||
def test_the_ceiling_ignores_non_api_paths():
|
||||
# Sitemaps and robots.txt are served by this app too, and a crawler
|
||||
# fetching them must not be refused because the API is busy.
|
||||
from backend.app import exempt_from_ceiling
|
||||
|
||||
assert exempt_from_ceiling(_ceiling_req("/sitemap.xml", "10.0.0.9"))
|
||||
assert exempt_from_ceiling(_ceiling_req("/robots.txt", "10.0.0.9"))
|
||||
@@ -0,0 +1,157 @@
|
||||
"""Tests for school autosuggest (spec 2026-08-26)."""
|
||||
|
||||
from backend import data_loader
|
||||
|
||||
|
||||
class _FakeDocs:
|
||||
def __init__(self, hits, explode=False):
|
||||
self._hits = hits
|
||||
self._explode = explode
|
||||
self.last_params = None
|
||||
|
||||
def search(self, params):
|
||||
self.last_params = params
|
||||
if self._explode:
|
||||
raise RuntimeError("typesense is down")
|
||||
return {"hits": [{"document": d} for d in self._hits]}
|
||||
|
||||
|
||||
class _FakeClient:
|
||||
def __init__(self, hits, explode=False):
|
||||
self.docs = _FakeDocs(hits, explode)
|
||||
self.collections = {"schools": type("C", (), {"documents": self.docs})()}
|
||||
|
||||
|
||||
_HIT = {
|
||||
"urn": 100010, "school_name": "Brecknock Primary School",
|
||||
"local_authority": "Camden", "postcode": "NW1 1AA",
|
||||
"phase": "Primary", "school_type": "Community school",
|
||||
}
|
||||
|
||||
|
||||
def _use(monkeypatch, client):
|
||||
monkeypatch.setattr(data_loader, "_get_typesense_client", lambda: client)
|
||||
|
||||
|
||||
def test_returns_the_fields_a_suggestion_needs(monkeypatch):
|
||||
# Local authority is not decoration: there are many schools called
|
||||
# "St Mary's", and a list without it cannot be chosen between.
|
||||
_use(monkeypatch, _FakeClient([_HIT]))
|
||||
out = data_loader.suggest_schools_typesense("breck")
|
||||
assert out == [{
|
||||
"urn": 100010, "school_name": "Brecknock Primary School",
|
||||
"local_authority": "Camden", "postcode": "NW1 1AA",
|
||||
"phase": "Primary", "school_type": "Community school",
|
||||
}]
|
||||
|
||||
|
||||
def test_a_missing_optional_field_becomes_an_empty_string(monkeypatch):
|
||||
# phase and school_type are optional in the Typesense schema. A missing
|
||||
# key must not KeyError in the keystroke path.
|
||||
_use(monkeypatch, _FakeClient([{"urn": 1, "school_name": "X",
|
||||
"local_authority": "Y", "postcode": "Z"}]))
|
||||
out = data_loader.suggest_schools_typesense("x")
|
||||
assert out[0]["phase"] == "" and out[0]["school_type"] == ""
|
||||
|
||||
|
||||
def test_typesense_unavailable_gives_no_suggestions_rather_than_raising(monkeypatch):
|
||||
_use(monkeypatch, None)
|
||||
assert data_loader.suggest_schools_typesense("anything") == []
|
||||
|
||||
|
||||
def test_a_typesense_error_gives_no_suggestions_rather_than_raising(monkeypatch):
|
||||
_use(monkeypatch, _FakeClient([], explode=True))
|
||||
assert data_loader.suggest_schools_typesense("anything") == []
|
||||
|
||||
|
||||
def test_the_limit_is_passed_through_and_clamped(monkeypatch):
|
||||
client = _FakeClient([])
|
||||
_use(monkeypatch, client)
|
||||
data_loader.suggest_schools_typesense("x", limit=500)
|
||||
assert client.docs.last_params["per_page"] == 20
|
||||
|
||||
|
||||
def _client(monkeypatch, rows, *, blow_up_dataframe=False):
|
||||
from fastapi.testclient import TestClient
|
||||
from backend import app as app_module
|
||||
|
||||
monkeypatch.setattr(app_module, "suggest_schools_typesense",
|
||||
lambda q, limit=8: rows)
|
||||
if blow_up_dataframe:
|
||||
def _boom():
|
||||
raise AssertionError("the suggest path must not load the DataFrame")
|
||||
monkeypatch.setattr(app_module, "load_school_data", _boom)
|
||||
monkeypatch.setattr(app_module, "load_latest_school_data", _boom)
|
||||
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
def test_the_endpoint_returns_suggestions(monkeypatch):
|
||||
body = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck").json()
|
||||
assert body["suggestions"][0]["school_name"] == "Brecknock Primary School"
|
||||
|
||||
|
||||
def test_the_endpoint_never_touches_the_dataframe(monkeypatch):
|
||||
"""The whole reason this is not a mode of /api/schools.
|
||||
|
||||
That endpoint filters and sorts 25,000 rows of pandas per query, holding
|
||||
the GIL. Per keystroke, that is the cost this endpoint exists to avoid.
|
||||
"""
|
||||
res = _client(monkeypatch, [_HIT], blow_up_dataframe=True).get("/api/suggest?q=breck")
|
||||
assert res.status_code == 200
|
||||
assert res.json()["suggestions"]
|
||||
|
||||
|
||||
def test_a_one_character_query_returns_nothing_and_does_not_error(monkeypatch):
|
||||
# The keystroke path never errors on ordinary input.
|
||||
res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=b")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"suggestions": []}
|
||||
|
||||
|
||||
def test_a_blank_query_returns_nothing_and_does_not_error(monkeypatch):
|
||||
res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"suggestions": []}
|
||||
|
||||
|
||||
def test_typesense_down_is_an_empty_list_not_a_500(monkeypatch):
|
||||
res = _client(monkeypatch, []).get("/api/suggest?q=breck")
|
||||
assert res.status_code == 200
|
||||
assert res.json() == {"suggestions": []}
|
||||
|
||||
|
||||
def test_the_response_is_cacheable(monkeypatch):
|
||||
# Prefix queries repeat enormously across users, and school names change
|
||||
# once a year. Without this the endpoint pays full price every keystroke.
|
||||
res = _client(monkeypatch, [_HIT]).get("/api/suggest?q=breck")
|
||||
assert "s-maxage" in res.headers.get("cache-control", "")
|
||||
assert res.headers.get("etag")
|
||||
|
||||
|
||||
def test_a_malformed_urn_does_not_raise(monkeypatch):
|
||||
"""The docstring promises "never raises"; the parsing loop sat outside the
|
||||
try, so int(None) or int("abc") would have turned a keystroke into a 500.
|
||||
|
||||
Typesense declares urn as int32, so this should be unreachable — but the
|
||||
contract is what the caller relies on, and a search index is a separate
|
||||
system that can be reindexed by something other than this code.
|
||||
"""
|
||||
_use(monkeypatch, _FakeClient([{"urn": None, "school_name": "X",
|
||||
"local_authority": "Y", "postcode": "Z"}]))
|
||||
assert data_loader.suggest_schools_typesense("x") == []
|
||||
|
||||
|
||||
def test_a_malformed_row_does_not_discard_the_good_ones(monkeypatch):
|
||||
# One bad document must not blank the whole dropdown.
|
||||
_use(monkeypatch, _FakeClient([
|
||||
{"urn": "not-a-number", "school_name": "Bad", "local_authority": "Y",
|
||||
"postcode": "Z"},
|
||||
_HIT,
|
||||
]))
|
||||
out = data_loader.suggest_schools_typesense("x")
|
||||
assert [r["urn"] for r in out] == [100010]
|
||||
|
||||
|
||||
def test_a_hit_with_no_document_does_not_raise(monkeypatch):
|
||||
_use(monkeypatch, _FakeClient([{}]))
|
||||
assert data_loader.suggest_schools_typesense("x") == []
|
||||
@@ -151,6 +151,36 @@ needed), and fails the check only when a finding is rated
|
||||
**severe** (would break prod, leak data, or corrupt data). Minor findings are
|
||||
informational and never block a merge.
|
||||
|
||||
## Rate limiting, and the Cloudflare gap
|
||||
|
||||
Two independent limits protect the API:
|
||||
|
||||
- **Per client**, via slowapi, keyed on `CF-Connecting-IP` (falling back to
|
||||
`X-Forwarded-For`, then the peer address). 60/minute by default;
|
||||
`/api/suggest` gets 120/minute because typing is bursty.
|
||||
- **Globally**, via `GlobalRateLimitMiddleware`: a fixed 60-second window over
|
||||
all `/api/` traffic, `GLOBAL_RATE_LIMIT_PER_MINUTE` (default 3000),
|
||||
independent of any client identity. Requests from `127.0.0.1` are exempt so
|
||||
the container healthcheck cannot be starved into a restart loop.
|
||||
|
||||
### Open: the origin must only accept Cloudflare
|
||||
|
||||
`CF-Connecting-IP` is only meaningful for requests that actually reached the
|
||||
origin through Cloudflare, and **the application cannot verify that they did**.
|
||||
Anything able to reach the origin directly can set that header freely and, by
|
||||
rotating it, mint a fresh rate-limit bucket per request — defeating per-client
|
||||
limits on every endpoint.
|
||||
|
||||
The global ceiling bounds the damage to total origin capacity. It does not fix
|
||||
the underlying gap, and nothing in the code can. Closing it needs one of:
|
||||
|
||||
- **Authenticated Origin Pulls** — Cloudflare presents a client certificate the
|
||||
origin requires, so non-Cloudflare traffic is refused at TLS.
|
||||
- **An origin firewall** restricted to Cloudflare's published IP ranges.
|
||||
|
||||
Until one is in place, treat per-client limits as protection against accidents
|
||||
and ordinary load, not against a determined caller.
|
||||
|
||||
## Feature flags (Unleash)
|
||||
|
||||
Flag state lives in a self-hosted Unleash instance, deployed as its own
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,294 @@
|
||||
# School Autosuggest — Design
|
||||
|
||||
**Date:** 2026-08-26
|
||||
**Status:** approved for planning
|
||||
**Depends on:** the feature-flag layer (PR #125, merged)
|
||||
|
||||
## Goal
|
||||
|
||||
Suggest schools by name as someone types in the site's main search box, so a
|
||||
parent who knows the school they want reaches it in one step instead of
|
||||
searching, scanning a result list, and clicking.
|
||||
|
||||
Scope is **schools only**. Places and postcodes were considered and excluded —
|
||||
see *Out of scope*.
|
||||
|
||||
## The finding that shapes everything
|
||||
|
||||
The site's rate limiter does not do what it looks like it does.
|
||||
|
||||
`limiter = Limiter(key_func=get_remote_address)` with `60/minute` reads
|
||||
`request.client.host`. In staging and production the backend has no published
|
||||
ports and sits on the internal `backend` network, so its only caller is the
|
||||
Next proxy — and `request.client.host` is therefore **the Next container**, for
|
||||
every browser user on the site.
|
||||
|
||||
Measured against staging: 70 concurrent requests to `/api/schools` returned
|
||||
**60 × 200 and 10 × 429**. One machine consumed the whole site's budget for
|
||||
that minute.
|
||||
|
||||
Autosuggest is the worst possible feature to build on that. One person typing
|
||||
"st marys primary" produces six to eight debounced requests; **eight concurrent
|
||||
searchers would 429 the site.** The compare modal's search-as-you-type already
|
||||
shares this bucket, so the exposure exists today — autosuggest makes it
|
||||
certain.
|
||||
|
||||
Fixing the keying is therefore part of this work, not a follow-up.
|
||||
|
||||
## 1. Rate-limit keying
|
||||
|
||||
Both environments sit behind Cloudflare (`server: cloudflare`, `cf-ray` present
|
||||
on staging and production). Cloudflare sets `CF-Connecting-IP` on every request
|
||||
to the origin and **overwrites any client-supplied value**, which makes it
|
||||
trustworthy in a way a parsed `X-Forwarded-For` chain is not.
|
||||
|
||||
```python
|
||||
def client_key(request: Request) -> str:
|
||||
"""Rate-limit bucket: the real caller, not the proxy in front of them."""
|
||||
cf = request.headers.get("cf-connecting-ip")
|
||||
if cf:
|
||||
return cf.strip()
|
||||
xff = request.headers.get("x-forwarded-for")
|
||||
if xff:
|
||||
return xff.split(",")[0].strip()
|
||||
return get_remote_address(request)
|
||||
```
|
||||
|
||||
`nextjs-app/app/api/[...path]/route.ts` already forwards every inbound header
|
||||
except `host` and `connection`, so `CF-Connecting-IP` reaches the backend with
|
||||
no proxy change.
|
||||
|
||||
**This header is trustworthy only for traffic that actually passed through
|
||||
Cloudflare, and nothing in the application can verify that it did.** An earlier
|
||||
draft of this section claimed Cloudflare "replaces the header, so a browser
|
||||
cannot forge it", and that only the `X-Forwarded-For` fallback was forgeable.
|
||||
That was wrong. Cloudflare does overwrite the header *on requests it handles* —
|
||||
but a caller reaching the origin directly sets whatever it likes, and this
|
||||
process cannot distinguish an edge-set header from an attacker-set one. Both
|
||||
headers are equally forgeable in that scenario.
|
||||
|
||||
The consequence is sharper than a weakened defence. An attacker rotating
|
||||
`CF-Connecting-IP` per request mints a fresh rate-limit bucket every time and
|
||||
evades per-client limits entirely — including on the DataFrame-heavy
|
||||
`/api/schools`. Against abuse that is *worse* than the shared bucket it
|
||||
replaced, which at least capped everyone at 60/minute together.
|
||||
|
||||
Two mitigations, and they are not interchangeable:
|
||||
|
||||
1. **The real fix is at Cloudflare** — Authenticated Origin Pulls, or an origin
|
||||
firewall that refuses connections not from Cloudflare's ranges. Only the
|
||||
edge can vouch for its own header. This is infrastructure work and is not
|
||||
part of this change; it is the thing that makes the header mean anything.
|
||||
2. **The ceiling in §1.1 bounds what evading the keying can achieve** while
|
||||
that remains open. It does not make the header trustworthy — it makes
|
||||
trusting it survivable.
|
||||
|
||||
The backend being unreachable from outside the Docker network is a real second
|
||||
layer, but it depends on the ingress path in front of the frontend, which this
|
||||
design does not control and should not assume.
|
||||
|
||||
### 1.1 The ceiling, which is back
|
||||
|
||||
The shared bucket was acting as an accidental global throttle on a
|
||||
single-process uvicorn backend that filters a 25,000-row DataFrame in-process.
|
||||
Correct per-user keying removes it: the origin becomes reachable at 60/min *per
|
||||
user* rather than 60/min in total, and — per above — at an unbounded rate by
|
||||
anyone willing to rotate a header.
|
||||
|
||||
An earlier draft dropped the in-app ceiling, arguing it belonged at Cloudflare.
|
||||
That argument assumed the keying was sound. It is not, so the ceiling is
|
||||
load-bearing rather than redundant, and it ships here:
|
||||
|
||||
`GlobalRateLimitMiddleware` counts all `/api/` requests in a fixed 60-second
|
||||
window against `global_rate_limit_per_minute` (3000), independent of any client
|
||||
identity, and refuses with a 429 that names capacity rather than the client —
|
||||
an operator has to be able to tell "one noisy client" from "the origin is
|
||||
saturated". It is registered last so it is outermost: a ceiling that applies
|
||||
after the expensive work has run is not a ceiling.
|
||||
|
||||
slowapi cannot express this. `default_limits` and `application_limits` are both
|
||||
evaluated with the same `key_func`, making them per-client rather than global,
|
||||
and `application_limits` only apply with `SlowAPIMiddleware` installed, which
|
||||
this app does not use. Hence the explicit middleware — about thirty lines, and
|
||||
obviously correct, which is what a backstop needs to be.
|
||||
|
||||
Requests from `127.0.0.1` are exempt. The container healthcheck runs
|
||||
`curl http://localhost:80/api/data-info` from inside the container, and
|
||||
starving it would fail the check, restart the container, and turn a load spike
|
||||
into an outage loop. The exemption keys on the peer address, never the `Host`
|
||||
header, which the caller sets.
|
||||
|
||||
3000/minute is an estimate, not a measurement, and worth revisiting against
|
||||
real traffic.
|
||||
|
||||
### Per-user limits
|
||||
|
||||
Per-user fairness and origin protection are different jobs, and this design now
|
||||
does both separately: the ceiling above for the origin, and per-route limits
|
||||
for fairness. Conflating them is what produced the original behaviour, where
|
||||
one bucket served the whole internet.
|
||||
|
||||
The existing 60/minute default is unchanged, and `/api/suggest` gets
|
||||
120/minute. Both are estimates rather than measurements, and are a starting
|
||||
point to revisit once the keying is correct enough for real per-user traffic to
|
||||
be visible — which it was not before, because everyone shared one bucket.
|
||||
|
||||
## 2. `GET /api/suggest`
|
||||
|
||||
A dedicated endpoint, not a mode of `/api/schools`.
|
||||
|
||||
The existing search path calls Typesense for URNs and then filters, ranks and
|
||||
sorts the full in-memory DataFrame — a pandas pass per keystroke, holding the
|
||||
GIL and blocking other requests in the same worker. Suggestions need none of
|
||||
it: `urn`, `school_name`, `phase`, `school_type`, `local_authority`,
|
||||
`postcode` and `ofsted_rating` are all already in the Typesense document
|
||||
(`pipeline/scripts/sync_typesense.py`).
|
||||
|
||||
```
|
||||
GET /api/suggest?q=<query>&limit=8
|
||||
→ 200 {"suggestions": [
|
||||
{"urn": 100010, "school_name": "Brecknock Primary School",
|
||||
"local_authority": "Camden", "postcode": "NW1 1AA",
|
||||
"phase": "Primary", "school_type": "Community school"}
|
||||
]}
|
||||
```
|
||||
|
||||
- **Under two characters** returns `{"suggestions": []}` with 200. The
|
||||
keystroke path never returns an error for ordinary input.
|
||||
- **Typesense unavailable** returns `{"suggestions": []}` with 200. There is
|
||||
deliberately **no DataFrame fallback**: the substring scan `/api/schools`
|
||||
falls back to is precisely the cost this endpoint exists to avoid, and a
|
||||
silent 25,000-row scan per keystroke is worse than no suggestions.
|
||||
- **`limit` is clamped** to 20. It is a public endpoint.
|
||||
- **Rate limit `120/minute`** per client, not the default 60. A 200 ms
|
||||
debounce tops out near 5 requests/second while someone is actively typing,
|
||||
but averages far below that across a real search; 120 leaves headroom for
|
||||
bursts while still bounding one client.
|
||||
- **Local authority is part of the payload, not decoration.** There are many
|
||||
schools called "St Mary's"; a suggestion list without the authority is
|
||||
unusable for exactly the queries autosuggest is meant to serve.
|
||||
|
||||
### Caching
|
||||
|
||||
`CACHE_RULES` gains `("/api/suggest", (60, 3600, 86400))`. Prefix queries
|
||||
repeat enormously across users and school names change once a year.
|
||||
|
||||
The client fetch must **not** use `cache: "no-store"`. The compare modal does,
|
||||
and copying that pattern would throw away both the browser cache and the ETag
|
||||
304s the existing `CacheAndETagMiddleware` already provides.
|
||||
|
||||
Both environments currently report `cf-cache-status: DYNAMIC` — Cloudflare
|
||||
ignores the `Cache-Control` the API already sends, because it does not cache
|
||||
dynamic paths by default. **A Cloudflare Cache Rule for `/api/suggest*` would
|
||||
let the edge absorb most of this traffic and never reach the origin.** That is
|
||||
a dashboard change, it is optional, and nothing here depends on it.
|
||||
|
||||
## 3. The combobox
|
||||
|
||||
This is an ARIA combobox, not a text input with a list underneath.
|
||||
|
||||
**Files.** `FilterBar.tsx` is already long. The work splits three ways:
|
||||
`hooks/useSchoolSuggest.ts` owns fetching, debouncing and cancellation;
|
||||
`components/SuggestList.tsx` owns rendering and ARIA; `FilterBar.tsx` wires
|
||||
them to the existing input and form.
|
||||
|
||||
**Fetching.** 200 ms debounce; minimum two characters; an `AbortController`
|
||||
cancels the superseded request on every keystroke. Cancellation is not an
|
||||
optimisation — without it, a slow response for `"st"` can land after the fast
|
||||
one for `"st marys"` and replace a correct list with a stale one.
|
||||
|
||||
**Suppressed during postcode entry.** The box takes a school name *or* a
|
||||
postcode, and `isValidPostcode` already distinguishes them. Suggestions do not
|
||||
appear once the value parses as a postcode.
|
||||
|
||||
**Keyboard.** `ArrowDown`/`ArrowUp` move the active option, `Escape` closes and
|
||||
keeps the typed text, `Tab` closes. `Enter` **with an option active** navigates
|
||||
to that school's page. `Enter` **with none active** submits the free-text
|
||||
search exactly as it does today — the existing behaviour is preserved, not
|
||||
replaced.
|
||||
|
||||
**ARIA.** `role="combobox"` with `aria-expanded` and `aria-controls` on the
|
||||
input, `aria-activedescendant` pointing at the active option, `role="listbox"`
|
||||
on the list and `role="option"` on each row.
|
||||
|
||||
**Both instances get it.** `HomeView` renders `FilterBar` twice — hero and
|
||||
sticky — from one component, so there is one implementation.
|
||||
|
||||
## 4. Behind a flag
|
||||
|
||||
Flag `school_autosuggest`, declared in `backend/flags.py`, default off.
|
||||
|
||||
This is the most-used control on the site and the first change to it in a
|
||||
while. `app/page.tsx` is an async server component, so it reads the flag and
|
||||
threads it to `FilterBar` through `HomeView` — two prop hops, explicit, no
|
||||
client-side flag read.
|
||||
|
||||
Off means the input behaves exactly as it does today: no listener, no fetch, no
|
||||
markup. Not a rendered-then-hidden dropdown.
|
||||
|
||||
The rate-limit keying is **not** flagged. It is a correctness fix that should
|
||||
apply whether or not autosuggest is on, and flagging it would mean shipping a
|
||||
known-wrong limiter into production deliberately.
|
||||
|
||||
## 5. Analytics
|
||||
|
||||
`search_submitted` already carries `via: 'input'`. Selecting a suggestion fires
|
||||
it with `via: 'suggestion'` plus the chosen `urn`, so the obvious question —
|
||||
does this actually help, or do people ignore it — has an answer in the data
|
||||
rather than an opinion.
|
||||
|
||||
## 6. Testing
|
||||
|
||||
**Backend.** `client_key` prefers `CF-Connecting-IP`, falls back through
|
||||
`X-Forwarded-For` to the remote address, and two different values get two
|
||||
different buckets. `/api/suggest` returns matches, returns empty below two
|
||||
characters, returns empty and 200 when Typesense is unavailable, and clamps
|
||||
`limit`. That it never touches the DataFrame is asserted by making
|
||||
`load_school_data` raise and requiring the endpoint to answer anyway.
|
||||
|
||||
**Frontend.** The hook debounces, aborts superseded requests, and drops a
|
||||
late-arriving response for a stale query. The list renders the ARIA
|
||||
attributes. Keyboard navigation moves the active option; `Enter` on an option
|
||||
navigates; `Enter` on none submits the search.
|
||||
|
||||
**E2E.** With the flag on, typing a known school name shows it and selecting it
|
||||
lands on that school's page. With the flag off, no combobox markup exists.
|
||||
Gated on the flag the same way the distance journeys are — read the observable
|
||||
effect, since `/api/flags` is denied to the public.
|
||||
|
||||
## 7. Risks
|
||||
|
||||
**Removing the accidental throttle.** Covered in §1. Correct per-user keying
|
||||
means the origin is reachable at 60/minute *per user* where it was 60/minute
|
||||
in total, and no in-app global cap replaces it — that job goes to Cloudflare,
|
||||
which is not done as part of this change. Until it is, a determined caller
|
||||
with many source addresses can put more load on a single-process origin than
|
||||
they can today. Against this site's traffic that is a theoretical risk rather
|
||||
than a live one, but it is a real one and it is the price of the fix.
|
||||
|
||||
**Cloudflare bypass — the open one.** If the origin is reachable without
|
||||
passing through Cloudflare, `CF-Connecting-IP` is attacker-controlled, and
|
||||
rotating it per request defeats per-client limits on every endpoint. The
|
||||
ceiling in §1.1 bounds the damage to the origin's total capacity; it does not
|
||||
restore per-client fairness under attack, and it cannot. Closing this properly
|
||||
means Authenticated Origin Pulls or an origin firewall restricted to
|
||||
Cloudflare's published ranges — infrastructure work, outside this change, and
|
||||
the single most valuable follow-up here.
|
||||
|
||||
**Typesense becomes user-visible.** Today a Typesense outage degrades search to
|
||||
a slow substring match. With autosuggest it also means the dropdown silently
|
||||
stops appearing. That is the correct failure — quiet, not broken — but it makes
|
||||
Typesense health worth monitoring in a way it was not before.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **Place suggestions.** The 2,646 town, authority and outcode pages are a
|
||||
strong candidate and would route people onto the pages W2 built, but they
|
||||
live in the place registry rather than Typesense, so it is a second index and
|
||||
a ranking rule for comparing two kinds of result. Worth its own change.
|
||||
- **Postcode completion.** Would put postcodes.io in the keystroke path, with
|
||||
its own latency and rate limits.
|
||||
- **The compare modal.** It already has search-as-you-type. Converting it to
|
||||
this component is a reasonable follow-up, not part of this.
|
||||
- **Recent or popular searches.** No storage for either, and no evidence yet
|
||||
that they are wanted.
|
||||
+260
-5
@@ -1978,6 +1978,59 @@ test('a place page links its phase variants, and they resolve', async ({ page })
|
||||
await expect(page.locator('h1')).toContainText(new RegExp(`${phase} schools in`, 'i'));
|
||||
});
|
||||
|
||||
/*
|
||||
* The table shipped with one column of scores. A parent shortlisting from a
|
||||
* town page needs to know whether a school takes their child's age, whether
|
||||
* it is a faith school, and — for a primary — whether it has a nursery,
|
||||
* before a percentage means anything.
|
||||
*
|
||||
* These assert the column headings rather than the values: nursery_provision
|
||||
* and parliamentary_constituency are optional mart columns, and on an
|
||||
* environment whose pipeline has not rebuilt them the API degrades them to
|
||||
* absent. A value assertion would then fail for a data reason, not a code one.
|
||||
*/
|
||||
async function phasedPlace(page: Page, phase: 'primary' | 'secondary') {
|
||||
const place = await firstPlaceOfKind(page, 'town');
|
||||
const detail = await (await page.request.get(`/api/places/town/${place.slug}`)).json();
|
||||
test.skip(!(detail.place.phases ?? []).includes(phase),
|
||||
`no ${phase} page clears the threshold here`);
|
||||
return place;
|
||||
}
|
||||
|
||||
test('a primary place page names each school as well as scoring it', async ({ page }) => {
|
||||
const place = await phasedPlace(page, 'primary');
|
||||
await page.goto(`/schools/${place.slug}/primary`);
|
||||
for (const heading of ['Ages', 'Religious character', 'Nursery', 'Constituency']) {
|
||||
await expect(page.getByRole('columnheader', { name: heading, exact: true }))
|
||||
.toBeVisible();
|
||||
}
|
||||
// age_range rides in on SCHOOL_COLUMNS and predates the optional columns,
|
||||
// so it is the one attribute safe to assert a value for anywhere.
|
||||
await expect(page.locator('table tbody td').filter({ hasText: /^\d+–\d+$/ }).first())
|
||||
.toBeVisible();
|
||||
});
|
||||
|
||||
test('a secondary place page does not ask about nurseries', async ({ page }) => {
|
||||
const place = await phasedPlace(page, 'secondary');
|
||||
await page.goto(`/schools/${place.slug}/secondary`);
|
||||
await expect(page.getByRole('columnheader', { name: 'Ages', exact: true }))
|
||||
.toBeVisible();
|
||||
await expect(page.getByRole('columnheader', { name: 'Nursery', exact: true }))
|
||||
.toHaveCount(0);
|
||||
});
|
||||
|
||||
test('the measure stays beside the school name, not behind a swipe', async ({ page }) => {
|
||||
// Six columns overflow a phone; .tableWrap turns that into a horizontal
|
||||
// scroll. With the measure last, the number the page exists for is the one
|
||||
// off the screen.
|
||||
const place = await phasedPlace(page, 'primary');
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
await page.goto(`/schools/${place.slug}/primary`);
|
||||
const second = page.locator('table thead th').nth(1);
|
||||
await expect(second).toContainText(/reading, writing/i);
|
||||
await expect(second).toBeInViewport();
|
||||
});
|
||||
|
||||
test('phase variants are submitted in the places sitemap', async ({ page }) => {
|
||||
const xml = await (await page.request.get('/sitemaps/places-1.xml')).text();
|
||||
expect(xml).toMatch(/\/schools\/[a-z0-9-]+\/primary</);
|
||||
@@ -2094,12 +2147,32 @@ test('a place page lists its schools alphabetically', async ({ page }) => {
|
||||
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);
|
||||
/*
|
||||
* Per table, not per page.
|
||||
*
|
||||
* An unphased place page renders one table per phase, and an all-through
|
||||
* school legitimately appears in both — so the page's school links are not
|
||||
* one alphabetical run and never were. This assertion used to collect them
|
||||
* all together and only passed because no town it picked happened to hold an
|
||||
* all-through school; when the data gave Abbots Langley one, Breakspeare
|
||||
* School showed up in the primary table and again in the secondary, and the
|
||||
* test failed on correct behaviour.
|
||||
*/
|
||||
const tables = page.locator('table');
|
||||
const tableCount = await tables.count();
|
||||
expect(tableCount).toBeGreaterThan(0);
|
||||
|
||||
let checked = 0;
|
||||
for (let i = 0; i < tableCount; i++) {
|
||||
const names = await tables.nth(i).locator('a[href^="/school/"]').allTextContents();
|
||||
if (names.length < 2) continue; // a one-row table says nothing about order
|
||||
const sorted = [...names].sort((a, b) =>
|
||||
a.toLowerCase().localeCompare(b.toLowerCase()));
|
||||
expect(names, `table ${i + 1} is not alphabetical`).toEqual(sorted);
|
||||
checked++;
|
||||
}
|
||||
expect(checked, 'no table had enough rows to check the ordering').toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('the rankings page still orders by score, not name', async ({ page }) => {
|
||||
@@ -2111,3 +2184,185 @@ test('the rankings page still orders by score, not name', async ({ page }) => {
|
||||
.filter((v: number | null) => v != null);
|
||||
expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a));
|
||||
});
|
||||
|
||||
/*
|
||||
* Analytics on the location layer.
|
||||
*
|
||||
* Umami counts a pageview for every one of these URLs already. What it cannot
|
||||
* say is which *kind* of location page earns engagement, because all four
|
||||
* families share the /schools/ prefix — and that is the question that decides
|
||||
* whether to keep investing in them.
|
||||
*/
|
||||
|
||||
/** Capture Umami events, with the real script blocked so it cannot clobber
|
||||
* the stub. Must be called before the first navigation. */
|
||||
async function captureEvents(page: Page) {
|
||||
const events: Array<{ name: string; data: Record<string, unknown> }> = [];
|
||||
await page.route('**/analytics.schoolcompare.co.uk/**', (route) => route.abort());
|
||||
await page.exposeFunction('__capture',
|
||||
(name: string, data: Record<string, unknown>) => { events.push({ name, data }); });
|
||||
await page.addInitScript(() => {
|
||||
(window as unknown as { umami: unknown }).umami = {
|
||||
track: (name: string, data: unknown) =>
|
||||
(window as unknown as { __capture: (n: string, d: unknown) => void })
|
||||
.__capture(name, data),
|
||||
};
|
||||
});
|
||||
return events;
|
||||
}
|
||||
|
||||
test('a location page reports which kind of place it is', async ({ page }) => {
|
||||
const events = await captureEvents(page);
|
||||
const place = await firstPlaceOfKind(page, 'authority');
|
||||
|
||||
await page.goto(`/schools/authority/${place.slug}`);
|
||||
await expect.poll(() => events.find((e) => e.name === 'place_viewed'),
|
||||
{ timeout: 10_000 }).toBeTruthy();
|
||||
|
||||
const event = events.find((e) => e.name === 'place_viewed')!;
|
||||
expect(event.data.kind).toBe('authority');
|
||||
expect(event.data.slug).toBe(place.slug);
|
||||
expect(event.data.phase).toBe('all');
|
||||
});
|
||||
|
||||
test('a school reached from a location page is attributed to it, not to direct', async ({ page }) => {
|
||||
/*
|
||||
* The defect this was written for. getNavigationSource had no case for
|
||||
* /schools/, so every school view that came through the location layer was
|
||||
* filed as 'direct' — the bucket you read as "typed the URL". The one
|
||||
* measurement that says whether ~3,900 SEO pages work was reporting the
|
||||
* wrong answer, confidently.
|
||||
*/
|
||||
const events = await captureEvents(page);
|
||||
const place = await firstPlaceOfKind(page, 'town');
|
||||
|
||||
await page.goto(`/schools/${place.slug}`);
|
||||
await page.locator('a[href^="/school/"]').first().click();
|
||||
await page.waitForURL(/\/school\//);
|
||||
|
||||
await expect.poll(() => events.find((e) => e.name === 'school_viewed'),
|
||||
{ timeout: 10_000 }).toBeTruthy();
|
||||
expect(events.find((e) => e.name === 'school_viewed')!.data.from).toBe('place');
|
||||
});
|
||||
|
||||
/*
|
||||
* School autosuggest (spec 2026-08-26).
|
||||
*/
|
||||
async function autosuggestIsOn(page: Page): Promise<boolean> {
|
||||
await page.goto('/');
|
||||
return (await page.getByRole('combobox').count()) > 0;
|
||||
}
|
||||
|
||||
test('the suggest endpoint answers from Typesense', async ({ page }) => {
|
||||
// Not flagged — the endpoint is live even while the UI is dark, so it can
|
||||
// be smoke-tested before the feature is switched on.
|
||||
const res = await page.request.get('/api/suggest?q=brecknock');
|
||||
expect(res.ok()).toBeTruthy();
|
||||
const { suggestions } = await res.json();
|
||||
expect(Array.isArray(suggestions)).toBeTruthy();
|
||||
if (suggestions.length) {
|
||||
// Local authority is what tells two "St Mary's" apart.
|
||||
expect(suggestions[0]).toHaveProperty('school_name');
|
||||
expect(suggestions[0]).toHaveProperty('local_authority');
|
||||
}
|
||||
});
|
||||
|
||||
test('a one-character query is answered, not rejected', async ({ page }) => {
|
||||
// The keystroke path never errors on ordinary input.
|
||||
const res = await page.request.get('/api/suggest?q=b');
|
||||
expect(res.status()).toBe(200);
|
||||
expect((await res.json()).suggestions).toEqual([]);
|
||||
});
|
||||
|
||||
test('the suggest response is cacheable', async ({ page }) => {
|
||||
const res = await page.request.get('/api/suggest?q=brecknock');
|
||||
expect(res.headers()['cache-control'] ?? '').toContain('s-maxage');
|
||||
});
|
||||
|
||||
test('typing a school name suggests it, and choosing it opens that school', async ({ page }) => {
|
||||
test.skip(!(await autosuggestIsOn(page)),
|
||||
'the school_autosuggest flag is off in this environment');
|
||||
|
||||
// A school certain to exist in any environment with data.
|
||||
const { schools } = await (await page.request.get('/api/schools?page_size=1')).json();
|
||||
test.skip(!schools?.length, 'no schools in this environment');
|
||||
const name = schools[0].school_name as string;
|
||||
|
||||
await page.goto('/');
|
||||
await page.getByRole('combobox').first().fill(name.slice(0, 12));
|
||||
const option = page.getByRole('option').first();
|
||||
await expect(option).toBeVisible();
|
||||
await option.click();
|
||||
await expect(page).toHaveURL(/\/school\/\d+/);
|
||||
});
|
||||
|
||||
test('the whole dropdown is reachable, not clipped by the hero', async ({ page }) => {
|
||||
/*
|
||||
* The hero panel had overflow: hidden to clip its artwork to the rounded
|
||||
* corners, and it clipped the dropdown too — 320px of list against 145px of
|
||||
* panel below the input, so roughly half was cut off with nothing to say so.
|
||||
*
|
||||
* toBeVisible() does not catch this: it checks the box is non-empty and not
|
||||
* visibility:hidden, and an ancestor's overflow clips neither. The invariant
|
||||
* that does catch it is that the LAST option is the thing actually painted
|
||||
* at its own coordinates — which fails for clipping and for occlusion alike.
|
||||
*/
|
||||
test.skip(!(await autosuggestIsOn(page)),
|
||||
'the school_autosuggest flag is off in this environment');
|
||||
|
||||
const { schools } = await (await page.request.get('/api/schools?page_size=1')).json();
|
||||
test.skip(!schools?.length, 'no schools in this environment');
|
||||
|
||||
await page.goto('/');
|
||||
await page.getByRole('combobox').first().fill(
|
||||
(schools[0].school_name as string).slice(0, 6));
|
||||
|
||||
const options = page.getByRole('option');
|
||||
await expect(options.first()).toBeVisible();
|
||||
const count = await options.count();
|
||||
|
||||
const painted = await options.nth(count - 1).evaluate((el) => {
|
||||
const r = el.getBoundingClientRect();
|
||||
const hit = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
|
||||
return { inside: el.contains(hit) || el === hit, bottom: Math.round(r.bottom) };
|
||||
});
|
||||
expect(painted.inside,
|
||||
`the last option is not painted at its own coordinates (bottom ${painted.bottom}) `
|
||||
+ '— an ancestor is clipping or covering the dropdown').toBeTruthy();
|
||||
});
|
||||
|
||||
test('the dropdown does not survive into the results it produced', async ({ page }) => {
|
||||
/*
|
||||
* The bug that took the staging gate down, and it was not a test problem:
|
||||
* after a search the results-page bar still holds the term, so the dropdown
|
||||
* reopened on top of the results and swallowed the click on the first one.
|
||||
* Playwright reported it as "<li role=option> intercepts pointer events"; a
|
||||
* reader would simply have found their first result unclickable.
|
||||
*/
|
||||
test.skip(!(await autosuggestIsOn(page)),
|
||||
'the school_autosuggest flag is off in this environment');
|
||||
|
||||
await page.goto('/');
|
||||
await page.getByRole('combobox').first().fill('school');
|
||||
await expect(page.getByRole('option').first()).toBeVisible();
|
||||
|
||||
await page.getByRole('button', { name: /Search/i }).first().click();
|
||||
await page.waitForURL(/search=school/);
|
||||
|
||||
await expect(page.getByRole('listbox')).toHaveCount(0);
|
||||
// And the results underneath are actually reachable, which is the point.
|
||||
await page.locator('a[href^="/school/"]').first().click({ timeout: 15_000 });
|
||||
await expect(page).toHaveURL(/\/school\//);
|
||||
});
|
||||
|
||||
test('with autosuggest off, the search box is a plain input', async ({ page }) => {
|
||||
test.skip(await autosuggestIsOn(page),
|
||||
'the school_autosuggest flag is on in this environment');
|
||||
|
||||
await page.goto('/');
|
||||
await expect(page.getByRole('combobox')).toHaveCount(0);
|
||||
// And the box still works: the existing search must be untouched.
|
||||
await page.getByPlaceholder(/School name or postcode/i).first().fill('abbey');
|
||||
await page.getByRole('button', { name: /Search/i }).first().click();
|
||||
await expect(page).toHaveURL(/search=abbey/);
|
||||
});
|
||||
@@ -0,0 +1,110 @@
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { FilterBar } from '@/components/FilterBar';
|
||||
|
||||
const push = jest.fn();
|
||||
let searchParams = new URLSearchParams();
|
||||
jest.mock('next/navigation', () => ({
|
||||
useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
|
||||
usePathname: () => '/',
|
||||
useSearchParams: () => searchParams,
|
||||
}));
|
||||
|
||||
const FILTERS = {
|
||||
local_authorities: [], school_types: [], years: [], phases: [],
|
||||
genders: [], admissions_policies: [],
|
||||
};
|
||||
|
||||
const realFetch = global.fetch;
|
||||
beforeEach(() => {
|
||||
global.fetch = jest.fn(async () => ({
|
||||
ok: true,
|
||||
json: async () => ({ suggestions: [{
|
||||
urn: 100010, school_name: 'Brecknock Primary School',
|
||||
local_authority: 'Camden', postcode: 'NW1 1AA',
|
||||
phase: 'Primary', school_type: 'Community school' }] }),
|
||||
})) as unknown as typeof fetch;
|
||||
push.mockClear();
|
||||
searchParams = new URLSearchParams();
|
||||
});
|
||||
afterEach(() => { global.fetch = realFetch; });
|
||||
|
||||
describe('FilterBar autosuggest', () => {
|
||||
it('is a combobox only when the flag is on', () => {
|
||||
const { rerender } = render(<FilterBar filters={FILTERS} autosuggest={false} />);
|
||||
expect(screen.queryByRole('combobox')).not.toBeInTheDocument();
|
||||
rerender(<FilterBar filters={FILTERS} autosuggest />);
|
||||
expect(screen.getByRole('combobox')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('makes no request while the flag is off', async () => {
|
||||
// Off means off: no listener, no fetch, no markup.
|
||||
render(<FilterBar filters={FILTERS} autosuggest={false} />);
|
||||
await userEvent.type(screen.getByPlaceholderText(/School name or postcode/i),
|
||||
'brecknock');
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('shows suggestions and navigates when one is chosen', async () => {
|
||||
render(<FilterBar filters={FILTERS} autosuggest />);
|
||||
await userEvent.type(screen.getByRole('combobox'), 'brecknock');
|
||||
const option = await screen.findByRole('option', { name: /Brecknock/ });
|
||||
await userEvent.click(option);
|
||||
expect(push).toHaveBeenCalledWith(
|
||||
expect.stringContaining('/school/100010'));
|
||||
});
|
||||
|
||||
it('suppresses suggestions once the value is a postcode', async () => {
|
||||
// The box takes a name OR a postcode; suggestions must get out of the way.
|
||||
//
|
||||
// fireEvent.change, not userEvent.type: typing sets "N", "NW", "NW1"... and
|
||||
// "NW1" is not a postcode, so a request for it is correct behaviour. Only
|
||||
// the settled value is the assertion, so set it in one go.
|
||||
render(<FilterBar filters={FILTERS} autosuggest />);
|
||||
fireEvent.change(screen.getByRole('combobox'), { target: { value: 'NW1 1AA' } });
|
||||
await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('Enter with no active option still submits the free-text search', async () => {
|
||||
// The existing behaviour is preserved, not replaced.
|
||||
render(<FilterBar filters={FILTERS} autosuggest />);
|
||||
const input = screen.getByRole('combobox');
|
||||
await userEvent.type(input, 'brecknock{Enter}');
|
||||
// updateURL pushes inside startTransition, so the call is not synchronous.
|
||||
await waitFor(() => expect(push).toHaveBeenCalledWith(
|
||||
expect.stringContaining('search=brecknock')));
|
||||
});
|
||||
});
|
||||
|
||||
describe('FilterBar autosuggest does not reopen over results', () => {
|
||||
it('stays shut when the input arrives pre-filled from the URL', async () => {
|
||||
/*
|
||||
* The results-page bar renders with the search term already in the input.
|
||||
* Opening on that would drop the dropdown on top of the results the search
|
||||
* just produced — which is exactly what happened: the first result became
|
||||
* unclickable, because the list sat over it and swallowed the pointer.
|
||||
*
|
||||
* Suggestions answer typing, not the presence of a value.
|
||||
*/
|
||||
searchParams = new URLSearchParams('search=brecknock');
|
||||
render(<FilterBar filters={FILTERS} autosuggest />);
|
||||
|
||||
expect(screen.getByRole('combobox')).toHaveValue('brecknock');
|
||||
await new Promise((r) => setTimeout(r, 300)); // past the 200ms debounce
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
expect(screen.queryByRole('listbox')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('closes the dropdown when the search is submitted', async () => {
|
||||
render(<FilterBar filters={FILTERS} autosuggest />);
|
||||
const input = screen.getByRole('combobox');
|
||||
|
||||
await userEvent.type(input, 'brecknock');
|
||||
expect(await screen.findByRole('listbox')).toBeInTheDocument();
|
||||
|
||||
await userEvent.type(input, '{Enter}');
|
||||
await waitFor(() =>
|
||||
expect(screen.queryByRole('listbox')).not.toBeInTheDocument());
|
||||
});
|
||||
});
|
||||
@@ -346,3 +346,134 @@ describe('PlaceView unlinkable authorities', () => {
|
||||
.toContain('Isles Of Scilly');
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlaceView school attributes', () => {
|
||||
/*
|
||||
* The table shipped with one column of scores, which answers "how did they
|
||||
* do" and nothing about whether the school is one a family could use. Age
|
||||
* range, faith, nursery and constituency are the four facts a parent
|
||||
* filters on before they look at a number at all.
|
||||
*/
|
||||
const withAttributes: PlaceDetail = {
|
||||
place: { kind: 'town', slug: 'chelmsford', name: 'Chelmsford', count: 3,
|
||||
parent_authority: 'Essex', phases: ['primary', 'secondary'] },
|
||||
schools: [
|
||||
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
|
||||
rwm_expected_pct: 82, attainment_8_score: null,
|
||||
age_range: '4-11', religious_denomination: 'Church of England',
|
||||
nursery_provision: true,
|
||||
parliamentary_constituency: 'Chelmsford' } as never,
|
||||
{ urn: 2, school_name: 'Beta High', phase: 'Secondary',
|
||||
rwm_expected_pct: null, attainment_8_score: 47,
|
||||
age_range: '11-16', religious_denomination: 'Does not apply',
|
||||
nursery_provision: false,
|
||||
parliamentary_constituency: 'Witham' } as never,
|
||||
],
|
||||
averages: { rwm_expected_pct: 63, attainment_8_score: 45 },
|
||||
};
|
||||
|
||||
function headings(container: HTMLElement, table = 0): string[] {
|
||||
return Array.from(container.querySelectorAll('table')[table]
|
||||
.querySelectorAll('thead th')).map((th) => th.textContent ?? '');
|
||||
}
|
||||
|
||||
it('heads a primary table with all four attributes', () => {
|
||||
const { container } = render(<PlaceView detail={withAttributes}
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
expect(headings(container)).toEqual([
|
||||
'School', 'Reading, writing & maths',
|
||||
'Ages', 'Religious character', 'Nursery', 'Constituency',
|
||||
]);
|
||||
});
|
||||
|
||||
it('omits nursery from a secondary table, where it does not apply', () => {
|
||||
const { container } = render(<PlaceView detail={withAttributes}
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
expect(headings(container, 1)).toEqual([
|
||||
'School', 'Attainment 8', 'Ages', 'Religious character', 'Constituency',
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps the measure beside the school name, where a phone can see it', () => {
|
||||
// Six columns overflow a phone and .tableWrap turns that into a swipe.
|
||||
// With the measure last, the one number the page exists for is the one
|
||||
// scrolled off the screen.
|
||||
const { container } = render(<PlaceView detail={withAttributes}
|
||||
phase="primary" englandAverage={61} neighbours={[]} />);
|
||||
expect(headings(container)[1]).toBe('Reading, writing & maths');
|
||||
});
|
||||
|
||||
it('shows the age range without repeating the column heading', () => {
|
||||
render(<PlaceView detail={withAttributes} englandAverage={61}
|
||||
neighbours={[]} />);
|
||||
expect(screen.getByText('4–11')).toBeInTheDocument();
|
||||
expect(screen.queryByText('Ages 4–11')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('names the faith of a faith school', () => {
|
||||
render(<PlaceView detail={withAttributes} englandAverage={61}
|
||||
neighbours={[]} />);
|
||||
expect(screen.getByText('Church of England')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('reads "Does not apply" as no religious character, not as a value', () => {
|
||||
// GIAS spells the absence of a faith as "Does not apply", which is a
|
||||
// database answer rather than an English one. The school page already
|
||||
// suppresses it; the two must not disagree about the same school.
|
||||
const { container } = render(<PlaceView detail={withAttributes}
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
const secondary = container.querySelectorAll('table')[1]
|
||||
.querySelectorAll('tbody td');
|
||||
expect(secondary[3].textContent).toBe('—');
|
||||
expect(screen.queryByText(/Does not apply/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('marks a nursery as such and a school without one as not', () => {
|
||||
const { container } = render(<PlaceView detail={withAttributes}
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
const cells = container.querySelectorAll('table')[0]
|
||||
.querySelectorAll('tbody td');
|
||||
expect(cells[4].textContent).toBe('Yes');
|
||||
});
|
||||
|
||||
it('names the constituency of each school', () => {
|
||||
render(<PlaceView detail={withAttributes} englandAverage={61}
|
||||
neighbours={[]} />);
|
||||
expect(screen.getByText('Chelmsford', { selector: 'td' })).toBeInTheDocument();
|
||||
expect(screen.getByText('Witham', { selector: 'td' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('dashes an attribute the data does not carry', () => {
|
||||
// nursery_provision and parliamentary_constituency are absent from marts
|
||||
// the pipeline has not rebuilt, and the API degrades them to null rather
|
||||
// than failing. A row must survive that.
|
||||
const bare: PlaceDetail = {
|
||||
...withAttributes,
|
||||
schools: [{ urn: 3, school_name: 'Gamma Primary', phase: 'Primary',
|
||||
rwm_expected_pct: 70 } as never],
|
||||
};
|
||||
const { container } = render(<PlaceView detail={bare} phase="primary"
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
const cells = Array.from(container.querySelectorAll('tbody td'))
|
||||
.map((td) => td.textContent);
|
||||
expect(cells.slice(2)).toEqual(['—', '—', '—', '—']);
|
||||
});
|
||||
|
||||
it('gives an all-through school its nursery under primary only', () => {
|
||||
// All-through schools render in both groups. Nursery belongs to the
|
||||
// primary reading of the same school, not the secondary one.
|
||||
const allThrough: PlaceDetail = {
|
||||
...withAttributes,
|
||||
schools: [{ urn: 4, school_name: 'Delta Academy', phase: 'All-through',
|
||||
rwm_expected_pct: 66, attainment_8_score: 51,
|
||||
age_range: '4-18', religious_denomination: 'None',
|
||||
nursery_provision: true,
|
||||
parliamentary_constituency: 'Chelmsford' } as never],
|
||||
};
|
||||
const { container } = render(<PlaceView detail={allThrough}
|
||||
englandAverage={61} neighbours={[]} />);
|
||||
const tables = container.querySelectorAll('table');
|
||||
expect(tables[0].textContent).toContain('Yes');
|
||||
expect(tables[1].textContent).not.toContain('Yes');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,50 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { SuggestList, suggestOptionId } from '@/components/SuggestList';
|
||||
|
||||
const ROWS = [
|
||||
{ urn: 1, school_name: "St Mary's Primary", local_authority: 'Camden',
|
||||
postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Voluntary aided school' },
|
||||
{ urn: 2, school_name: "St Mary's Primary", local_authority: 'Barnet',
|
||||
postcode: 'EN5 2AA', phase: 'Primary', school_type: 'Community school' },
|
||||
];
|
||||
|
||||
describe('SuggestList', () => {
|
||||
it('is a listbox of options', () => {
|
||||
render(<SuggestList id="s" suggestions={ROWS} activeIndex={-1}
|
||||
onPick={() => {}} onHover={() => {}} />);
|
||||
expect(screen.getByRole('listbox')).toBeInTheDocument();
|
||||
expect(screen.getAllByRole('option')).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('shows the local authority, which is what tells two schools apart', () => {
|
||||
// Both rows are "St Mary's Primary". Without the authority the list is
|
||||
// unusable for exactly the query autosuggest exists to serve.
|
||||
render(<SuggestList id="s" suggestions={ROWS} activeIndex={-1}
|
||||
onPick={() => {}} onHover={() => {}} />);
|
||||
expect(screen.getByText('Camden')).toBeInTheDocument();
|
||||
expect(screen.getByText('Barnet')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('marks only the active option selected', () => {
|
||||
render(<SuggestList id="s" suggestions={ROWS} activeIndex={1}
|
||||
onPick={() => {}} onHover={() => {}} />);
|
||||
const options = screen.getAllByRole('option');
|
||||
expect(options[0]).toHaveAttribute('aria-selected', 'false');
|
||||
expect(options[1]).toHaveAttribute('aria-selected', 'true');
|
||||
});
|
||||
|
||||
it('gives each option the id the input will point at', () => {
|
||||
// aria-activedescendant on the input has to name a real element id, or
|
||||
// a screen reader announces nothing as the user arrows through.
|
||||
render(<SuggestList id="s" suggestions={ROWS} activeIndex={0}
|
||||
onPick={() => {}} onHover={() => {}} />);
|
||||
expect(screen.getAllByRole('option')[0]).toHaveAttribute(
|
||||
'id', suggestOptionId('s', 0));
|
||||
});
|
||||
|
||||
it('renders nothing when there is nothing to suggest', () => {
|
||||
const { container } = render(<SuggestList id="s" suggestions={[]}
|
||||
activeIndex={-1} onPick={() => {}} onHover={() => {}} />);
|
||||
expect(container).toBeEmptyDOMElement();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,45 @@
|
||||
import { render } from '@testing-library/react';
|
||||
import { TrackPlaceView } from '@/components/places/TrackPlaceView';
|
||||
|
||||
const trackMock = jest.fn();
|
||||
jest.mock('@/lib/analytics', () => ({
|
||||
track: (...args: unknown[]) => trackMock(...args),
|
||||
getNavigationSource: () => 'search',
|
||||
}));
|
||||
|
||||
describe('TrackPlaceView', () => {
|
||||
beforeEach(() => trackMock.mockClear());
|
||||
|
||||
it('reports which kind of location page was viewed', () => {
|
||||
/*
|
||||
* `kind` is the reason this event exists. Whether to keep investing in the
|
||||
* location layer turns on which *sort* of page earns engagement — towns,
|
||||
* authorities or postcode districts — and a bare pageview cannot say,
|
||||
* because all four families share the /schools/ prefix.
|
||||
*/
|
||||
render(<TrackPlaceView kind="authority" slug="kent" count={412} />);
|
||||
expect(trackMock).toHaveBeenCalledWith('place_viewed', {
|
||||
kind: 'authority', slug: 'kent', phase: 'all',
|
||||
school_count: 412, from: 'search',
|
||||
});
|
||||
});
|
||||
|
||||
it('names the phase when the page is a phase variant', () => {
|
||||
render(<TrackPlaceView kind="town" slug="brentwood" count={29} phase="primary" />);
|
||||
expect(trackMock).toHaveBeenCalledWith('place_viewed',
|
||||
expect.objectContaining({ phase: 'primary' }));
|
||||
});
|
||||
|
||||
it('fires once, not once per render', () => {
|
||||
const { rerender } = render(
|
||||
<TrackPlaceView kind="town" slug="brentwood" count={29} />);
|
||||
rerender(<TrackPlaceView kind="town" slug="brentwood" count={29} />);
|
||||
expect(trackMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('renders nothing', () => {
|
||||
const { container } = render(
|
||||
<TrackPlaceView kind="town" slug="brentwood" count={29} />);
|
||||
expect(container).toBeEmptyDOMElement();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,78 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
|
||||
/**
|
||||
* Guards against light-theme-only CSS.
|
||||
*
|
||||
* The site themes entirely through tokens redefined under
|
||||
* `@media (prefers-color-scheme: dark)`. A hardcoded colour therefore does not
|
||||
* fail loudly — it renders perfectly in the theme it was written for and
|
||||
* quietly wrongly in the other, which nobody sees unless they happen to be in
|
||||
* dark mode when they look.
|
||||
*
|
||||
* Both rules below are drawn from real defects in SchoolHeroMap.module.css,
|
||||
* found by eye rather than by any test:
|
||||
*
|
||||
* - the map's fade to the header ramped through hardcoded white and landed on
|
||||
* `var(--bg-card)`. Invisible in light; a bright band across the full width
|
||||
* of a near-black card in dark.
|
||||
* - the controls floating over the map paired a hardcoded white background
|
||||
* with `color: var(--text-primary)`, which resolves to #E9EEF0 in dark —
|
||||
* near-white text on a near-white button.
|
||||
*/
|
||||
|
||||
const COMPONENTS = path.join(__dirname, '..', '..', 'components');
|
||||
|
||||
function stylesheets(dir: string): string[] {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) return stylesheets(full);
|
||||
return entry.name.endsWith('.module.css') ? [full] : [];
|
||||
});
|
||||
}
|
||||
|
||||
/** Innermost `selector { body }` pairs. Nested at-rules never match as rules,
|
||||
* because their body contains braces. */
|
||||
function rules(css: string): Array<{ selector: string; body: string }> {
|
||||
return Array.from(css.matchAll(/([^{}]+)\{([^{}]*)\}/g), (m) => ({
|
||||
selector: m[1].trim().split('\n').pop()!.trim(),
|
||||
body: m[2],
|
||||
}));
|
||||
}
|
||||
|
||||
const HARDCODED_WHITE_BG = /background[^;]*(?:255,\s*255,\s*255|#fff\b|#ffffff\b)/i;
|
||||
const THEMED_COLOR = /(?:^|[^-])color:\s*var\(--/;
|
||||
|
||||
const files = stylesheets(COMPONENTS);
|
||||
|
||||
describe('dark-theme safety', () => {
|
||||
it('finds stylesheets to check', () => {
|
||||
expect(files.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('never pairs a hardcoded white background with a themed text colour', () => {
|
||||
const offenders = files.flatMap((file) =>
|
||||
rules(fs.readFileSync(file, 'utf8'))
|
||||
.filter((r) => HARDCODED_WHITE_BG.test(r.body) && THEMED_COLOR.test(r.body))
|
||||
.map((r) => `${path.relative(COMPONENTS, file)} ${r.selector}`));
|
||||
|
||||
// Either the surface follows the theme and so should the text, or it does
|
||||
// not and the text must be literal too. Mixing them is how near-white text
|
||||
// ends up on a near-white button.
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
it('never fades to a themed colour through a hardcoded one', () => {
|
||||
const offenders = files.flatMap((file) =>
|
||||
rules(fs.readFileSync(file, 'utf8'))
|
||||
.filter((r) => /linear-gradient/.test(r.body)
|
||||
&& /var\(--bg-(card|primary|secondary)\)/.test(r.body)
|
||||
&& /255,\s*255,\s*255|#fff\b/i.test(r.body))
|
||||
.map((r) => `${path.relative(COMPONENTS, file)} ${r.selector}`));
|
||||
|
||||
// A gradient that lands on a token has to be made of that token, or the
|
||||
// ramp and its destination disagree in one theme. Use the matching
|
||||
// `--*-rgb` token for the transparent stops.
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,108 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
|
||||
/**
|
||||
* The hero search and the results filter bar are the same component in two
|
||||
* costumes. `.filterBar` is the card — background, border, shadow, padding —
|
||||
* and `.heroMode` strips all of it so the search sits directly on the hero
|
||||
* panel.
|
||||
*
|
||||
* Both selectors have specificity (0,1,0), so **source order decides**, and
|
||||
* `.heroMode` only wins because it is declared immediately after. Any later
|
||||
* bare `.filterBar` rule — which in practice means one inside a media query —
|
||||
* silently wins instead, and the hero grows a card's padding back.
|
||||
*
|
||||
* That is exactly what happened: `@media (max-width: 768px) { .filterBar {
|
||||
* padding: 0.875rem } }` re-added 14px in hero mode, indenting the search box,
|
||||
* the hint and the location link 14px past the headline above them and costing
|
||||
* the search field 28px of width on a 390px screen. The two rules directly
|
||||
* below it in the same block were correctly written as
|
||||
* `.filterBar:not(.heroMode)`; this one was missed, and nothing caught it
|
||||
* because the result is a plausible-looking layout rather than a broken one.
|
||||
*/
|
||||
|
||||
const CSS = path.join(__dirname, '..', '..', 'components', 'FilterBar.module.css');
|
||||
|
||||
/** Properties `.heroMode` resets. A later bare `.filterBar` rule setting any
|
||||
* of these puts the card back on the hero. */
|
||||
const RESET_BY_HERO_MODE = [
|
||||
'background', 'border', 'border-radius', 'box-shadow', 'padding',
|
||||
];
|
||||
|
||||
/**
|
||||
* Comments are stripped before anything is parsed.
|
||||
*
|
||||
* A `{` or `}` inside a comment would otherwise desynchronise the brace walk
|
||||
* below and the rule regex alike, and the selector text captured for each rule
|
||||
* would carry the preceding comment along with it.
|
||||
*/
|
||||
function withoutComments(css: string): string {
|
||||
return css.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
}
|
||||
|
||||
/**
|
||||
* The individual selectors in a rule's prelude.
|
||||
*
|
||||
* Split on commas, because a selector list is a list: `.filterBar, .other { }`
|
||||
* applies to `.filterBar` just as surely as `.filterBar { }` does, and an
|
||||
* earlier version of this guard compared the whole prelude against the literal
|
||||
* string '.filterBar' — so writing the regression as a comma list, or across
|
||||
* two lines, would have walked straight past it.
|
||||
*/
|
||||
function selectorsOf(prelude: string): string[] {
|
||||
return prelude.split(',').map((sel) => sel.trim().replace(/\s+/g, ' '))
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
function mediaQueryBodies(css: string): string[] {
|
||||
const bodies: string[] = [];
|
||||
const re = /@media[^{]*\{/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = re.exec(css)) !== null) {
|
||||
// Walk braces from the opening one to find this at-rule's whole body.
|
||||
let depth = 1;
|
||||
let i = m.index + m[0].length;
|
||||
const start = i;
|
||||
while (i < css.length && depth > 0) {
|
||||
if (css[i] === '{') depth++;
|
||||
else if (css[i] === '}') depth--;
|
||||
i++;
|
||||
}
|
||||
bodies.push(css.slice(start, i - 1));
|
||||
}
|
||||
return bodies;
|
||||
}
|
||||
|
||||
describe('FilterBar hero-mode scoping', () => {
|
||||
const css = withoutComments(fs.readFileSync(CSS, 'utf8'));
|
||||
|
||||
it('confirms heroMode still resets the card, which is what makes this matter', () => {
|
||||
const hero = css.match(/\.heroMode\s*\{([^}]*)\}/);
|
||||
expect(hero).not.toBeNull();
|
||||
expect(hero![1]).toMatch(/padding:\s*0/);
|
||||
});
|
||||
|
||||
it('never re-applies card styling to the hero from inside a media query', () => {
|
||||
const offenders: string[] = [];
|
||||
|
||||
for (const body of mediaQueryBodies(css)) {
|
||||
for (const rule of body.matchAll(/([^{}]+)\{([^{}]*)\}/g)) {
|
||||
// Only a *bare* .filterBar is dangerous, and it is dangerous wherever
|
||||
// it appears in a selector list. Scoped variants
|
||||
// (`.filterBar:not(.heroMode)`) and descendants are fine.
|
||||
const selectors = selectorsOf(rule[1]);
|
||||
if (!selectors.includes('.filterBar')) continue;
|
||||
|
||||
for (const prop of RESET_BY_HERO_MODE) {
|
||||
if (new RegExp(`(^|[;\\s])${prop}\\s*:`).test(rule[2])) {
|
||||
offenders.push(`${rule[1].trim()} sets ${prop}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Fix by scoping the rule as `.filterBar:not(.heroMode)`, the way the
|
||||
// neighbouring rules in the same block already are.
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,73 @@
|
||||
import { renderHook, act, waitFor } from '@testing-library/react';
|
||||
import { useSchoolSuggest } from '@/hooks/useSchoolSuggest';
|
||||
|
||||
const realFetch = global.fetch;
|
||||
|
||||
function mockFetch(rows: unknown[], delayMs = 0) {
|
||||
global.fetch = jest.fn(async (_url: unknown, init?: { signal?: AbortSignal }) => {
|
||||
if (delayMs) {
|
||||
await new Promise((resolve, reject) => {
|
||||
const t = setTimeout(resolve, delayMs);
|
||||
init?.signal?.addEventListener('abort', () => {
|
||||
clearTimeout(t);
|
||||
reject(Object.assign(new Error('aborted'), { name: 'AbortError' }));
|
||||
});
|
||||
});
|
||||
}
|
||||
return { ok: true, json: async () => ({ suggestions: rows }) };
|
||||
}) as unknown as typeof fetch;
|
||||
}
|
||||
|
||||
const ROW = {
|
||||
urn: 1, school_name: 'Brecknock Primary School', local_authority: 'Camden',
|
||||
postcode: 'NW1 1AA', phase: 'Primary', school_type: 'Community school',
|
||||
};
|
||||
|
||||
describe('useSchoolSuggest', () => {
|
||||
beforeEach(() => { jest.useFakeTimers(); });
|
||||
afterEach(() => { jest.useRealTimers(); global.fetch = realFetch; });
|
||||
|
||||
it('does not fetch below the minimum query length', () => {
|
||||
mockFetch([ROW]);
|
||||
renderHook(() => useSchoolSuggest('b', true));
|
||||
act(() => { jest.advanceTimersByTime(500); });
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not fetch at all when disabled', () => {
|
||||
// The flag being off must mean no request, not a hidden dropdown.
|
||||
mockFetch([ROW]);
|
||||
renderHook(() => useSchoolSuggest('brecknock', false));
|
||||
act(() => { jest.advanceTimersByTime(500); });
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('debounces rather than firing per keystroke', () => {
|
||||
mockFetch([ROW]);
|
||||
const { rerender } = renderHook(
|
||||
({ q }) => useSchoolSuggest(q, true), { initialProps: { q: 'br' } });
|
||||
rerender({ q: 'bre' });
|
||||
rerender({ q: 'brec' });
|
||||
act(() => { jest.advanceTimersByTime(199); });
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
act(() => { jest.advanceTimersByTime(2); });
|
||||
expect(global.fetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('opens with results once they arrive', async () => {
|
||||
mockFetch([ROW]);
|
||||
const { result } = renderHook(() => useSchoolSuggest('brecknock', true));
|
||||
act(() => { jest.advanceTimersByTime(200); });
|
||||
await waitFor(() => expect(result.current.suggestions).toHaveLength(1));
|
||||
expect(result.current.open).toBe(true);
|
||||
});
|
||||
|
||||
it('close() hides the list without clearing the query', async () => {
|
||||
mockFetch([ROW]);
|
||||
const { result } = renderHook(() => useSchoolSuggest('brecknock', true));
|
||||
act(() => { jest.advanceTimersByTime(200); });
|
||||
await waitFor(() => expect(result.current.open).toBe(true));
|
||||
act(() => { result.current.close(); });
|
||||
expect(result.current.open).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
import { getNavigationSource } from '@/lib/analytics';
|
||||
|
||||
/** jsdom's document.referrer is read-only; redefining it is the way in. */
|
||||
function referrer(url: string) {
|
||||
Object.defineProperty(document, 'referrer', { value: url, configurable: true });
|
||||
}
|
||||
|
||||
const ORIGIN = 'http://localhost';
|
||||
|
||||
describe('getNavigationSource', () => {
|
||||
afterEach(() => referrer(''));
|
||||
|
||||
it('attributes a visit from a location page to the place layer', () => {
|
||||
/*
|
||||
* The one this was added for.
|
||||
*
|
||||
* W2 published ~3,900 location pages whose entire purpose is to funnel
|
||||
* search traffic onto school pages. Before this case existed they fell
|
||||
* through to 'direct' — so the location layer's contribution was not
|
||||
* merely missing from the funnel, it was being counted in the bucket you
|
||||
* read as "typed the URL". The measurement that decides whether W2 worked
|
||||
* was confidently reporting the wrong answer.
|
||||
*/
|
||||
referrer(`${ORIGIN}/schools/barnet`);
|
||||
expect(getNavigationSource()).toBe('place');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['/schools/authority/kent', 'authority'],
|
||||
['/schools/near/sw11', 'outcode'],
|
||||
['/schools/brentwood/primary', 'phase variant'],
|
||||
])('covers %s (%s)', (path) => {
|
||||
referrer(`${ORIGIN}${path}`);
|
||||
expect(getNavigationSource()).toBe('place');
|
||||
});
|
||||
|
||||
it('still calls a school page "detail", one character away', () => {
|
||||
// /school/ and /schools/ differ by one letter and mean different things.
|
||||
// A prefix test written in the wrong order silently merges them.
|
||||
referrer(`${ORIGIN}/school/100010-brecknock-primary-school`);
|
||||
expect(getNavigationSource()).toBe('detail');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['/', 'search'],
|
||||
['/rankings', 'rankings'],
|
||||
['/compare?urns=1,2', 'compare'],
|
||||
])('leaves %s attributed as %s', (path, expected) => {
|
||||
referrer(`${ORIGIN}${path}`);
|
||||
expect(getNavigationSource()).toBe(expected);
|
||||
});
|
||||
|
||||
it('treats an external referrer as direct', () => {
|
||||
// Umami records the real referrer on the pageview; this field is only
|
||||
// about internal navigation.
|
||||
referrer('https://www.google.com/search?q=schools+in+barnet');
|
||||
expect(getNavigationSource()).toBe('direct');
|
||||
});
|
||||
|
||||
it('treats no referrer as direct', () => {
|
||||
referrer('');
|
||||
expect(getNavigationSource()).toBe('direct');
|
||||
});
|
||||
});
|
||||
@@ -13,6 +13,8 @@ import {
|
||||
metricKind,
|
||||
shortName,
|
||||
computeYBounds,
|
||||
formatAgeRange,
|
||||
formatAgeSpan,
|
||||
} from '@/lib/utils';
|
||||
|
||||
describe('formatPercentage', () => {
|
||||
@@ -320,3 +322,27 @@ describe('shortName', () => {
|
||||
expect(shortName('A'.repeat(30), 10)).toBe('AAAAAAAAA…');
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatAgeSpan', () => {
|
||||
it('normalises a hyphenated range to an en dash, without a label', () => {
|
||||
// The place table carries "Ages" in the column heading, so repeating it
|
||||
// in every cell is noise. formatAgeRange keeps the label for the contexts
|
||||
// that have no heading to hang it on.
|
||||
expect(formatAgeSpan('4-11')).toBe('4–11');
|
||||
});
|
||||
|
||||
it('leaves a range it does not recognise alone rather than mangling it', () => {
|
||||
expect(formatAgeSpan('3-19 (SEN)')).toBe('3-19 (SEN)');
|
||||
});
|
||||
|
||||
it('returns an empty string for a missing range', () => {
|
||||
expect(formatAgeSpan(null)).toBe('');
|
||||
expect(formatAgeSpan(undefined)).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatAgeRange', () => {
|
||||
it('keeps its label, so the two helpers stay distinguishable', () => {
|
||||
expect(formatAgeRange('4-11')).toBe('Ages 4–11');
|
||||
});
|
||||
});
|
||||
@@ -28,6 +28,9 @@
|
||||
--bg-primary: #FAFAF8; /* Warm White */
|
||||
--bg-secondary: #F5EFE6; /* Sand — hero panels, sunken rows */
|
||||
--bg-card: #FFFFFF;
|
||||
/* For gradients that have to fade to the card colour. A hardcoded white
|
||||
ramp reads as a bright band against a dark card. */
|
||||
--bg-card-rgb: 255, 255, 255;
|
||||
--surface-inverse: #0F766E;
|
||||
|
||||
/* ── Ink ────────────────────────────────────────────────────────── */
|
||||
@@ -234,6 +237,7 @@
|
||||
--bg-primary: #111A20;
|
||||
--bg-secondary: #16222A;
|
||||
--bg-card: #18242C;
|
||||
--bg-card-rgb: 24, 36, 44;
|
||||
--surface-inverse: #E9EEF0;
|
||||
|
||||
--text-primary: #E9EEF0;
|
||||
|
||||
@@ -8,6 +8,7 @@ import type { Metadata } from 'next';
|
||||
import { fetchSchools, fetchFilters, fetchDataInfo } from '@/lib/api';
|
||||
import { formatAcademicYear } from '@/lib/utils';
|
||||
import { HomeView } from '@/components/HomeView';
|
||||
import { getFlags } from '@/lib/flags';
|
||||
import { HowItWorksSection } from '@/components/HowItWorksSection';
|
||||
import { EditorialSection } from '@/components/EditorialSection';
|
||||
|
||||
@@ -63,6 +64,11 @@ export default async function HomePage({ searchParams }: HomePageProps) {
|
||||
// Await search params (Next.js 15 requirement)
|
||||
const params = await searchParams;
|
||||
|
||||
// Server-read: no flag value reaches the browser bundle. Threaded down to
|
||||
// both FilterBar instances via HomeView.
|
||||
const flags = await getFlags();
|
||||
const autosuggest = flags.school_autosuggest === true;
|
||||
|
||||
// Parse search params
|
||||
const page = parseInt(params.page || '1');
|
||||
const radius = params.radius ? parseFloat(params.radius) : undefined;
|
||||
@@ -111,6 +117,7 @@ export default async function HomePage({ searchParams }: HomePageProps) {
|
||||
const years = dataInfo?.years_available ?? [];
|
||||
return (
|
||||
<HomeView
|
||||
autosuggest={autosuggest}
|
||||
initialSchools={schoolsData}
|
||||
filters={resolvedFilters}
|
||||
totalSchools={total}
|
||||
@@ -131,6 +138,7 @@ export default async function HomePage({ searchParams }: HomePageProps) {
|
||||
const emptyFilters = { local_authorities: [], school_types: [], years: [], phases: [], genders: [], admissions_policies: [] };
|
||||
return (
|
||||
<HomeView
|
||||
autosuggest={autosuggest}
|
||||
initialSchools={{ schools: [], page: 1, page_size: 50, total: 0, total_pages: 0 }}
|
||||
filters={emptyFilters}
|
||||
totalSchools={null}
|
||||
|
||||
@@ -48,6 +48,8 @@
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 0.5rem;
|
||||
/* The suggestion dropdown is absolutely positioned against this box. */
|
||||
position: relative;
|
||||
}
|
||||
|
||||
/* The hero pill: hairline, soft corner, everything else sits inside it. */
|
||||
@@ -411,7 +413,17 @@
|
||||
/* ── Narrow ───────────────────────────────────────────────────────── */
|
||||
|
||||
@media (max-width: 768px) {
|
||||
.filterBar {
|
||||
/*
|
||||
* Scoped, like the two rules below it.
|
||||
*
|
||||
* The results filter bar is a card — background, border, shadow — and needs
|
||||
* inner padding. The hero's search is not a card: .heroMode zeroes the
|
||||
* padding, border and background so the search sits directly on the panel.
|
||||
* Unscoped, this rule put 14px back, which indented the search box, the hint
|
||||
* and the location link 14px past the headline they sit under, and cost the
|
||||
* search field 28px of width on a 390px screen.
|
||||
*/
|
||||
.filterBar:not(.heroMode) {
|
||||
padding: 0.875rem;
|
||||
}
|
||||
|
||||
@@ -455,6 +467,14 @@
|
||||
align-items: flex-start;
|
||||
}
|
||||
|
||||
/* Optical alignment: the button's own 6px of padding is what makes its
|
||||
label start further right than the hint above it, even once both boxes
|
||||
share a left edge. Pulling the padding back off lines the text up while
|
||||
keeping the tap target. */
|
||||
.heroMode .nearMeBtn {
|
||||
margin-left: -0.375rem;
|
||||
}
|
||||
|
||||
.geoError {
|
||||
text-align: left;
|
||||
}
|
||||
|
||||
@@ -3,8 +3,11 @@
|
||||
import { useState, useCallback, useTransition, useRef, useEffect } from "react";
|
||||
import type { ReactNode } from "react";
|
||||
import { useRouter, useSearchParams, usePathname } from "next/navigation";
|
||||
import { isValidPostcode } from "@/lib/utils";
|
||||
import { isValidPostcode, schoolUrl } from "@/lib/utils";
|
||||
import { track } from "@/lib/analytics";
|
||||
import { useSchoolSuggest } from "@/hooks/useSchoolSuggest";
|
||||
import { SuggestList, suggestOptionId } from "./SuggestList";
|
||||
import type { Suggestion } from "@/lib/suggest";
|
||||
import type { Filters, ResultFilters } from "@/lib/types";
|
||||
import styles from "./FilterBar.module.css";
|
||||
|
||||
@@ -17,6 +20,8 @@ interface FilterBarProps {
|
||||
onNearMe?: () => void;
|
||||
geoState?: "idle" | "requesting" | "error";
|
||||
geoError?: string | null;
|
||||
/** Server-read feature flag. Off means no listener, no fetch, no markup. */
|
||||
autosuggest?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -48,6 +53,7 @@ export function FilterBar({
|
||||
onNearMe,
|
||||
geoState = "idle",
|
||||
geoError,
|
||||
autosuggest = false,
|
||||
}: FilterBarProps) {
|
||||
const router = useRouter();
|
||||
const pathname = usePathname();
|
||||
@@ -62,6 +68,59 @@ export function FilterBar({
|
||||
|
||||
const [omniValue, setOmniValue] = useState(initialOmniValue);
|
||||
|
||||
const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`;
|
||||
|
||||
/*
|
||||
* Suggestions answer typing, not the mere presence of a value.
|
||||
*
|
||||
* Without this the results-page bar reopened the dropdown over the results:
|
||||
* after a search the input still holds the term, so on every render the
|
||||
* query was >= 2 characters and the list opened again — on top of the very
|
||||
* results the search had just produced, swallowing the click on the first
|
||||
* one. The E2E gate caught it as "<li role=option> intercepts pointer
|
||||
* events", but a reader would just have found the page unclickable.
|
||||
*/
|
||||
const [hasTyped, setHasTyped] = useState(false);
|
||||
|
||||
// Suppressed once the value parses as a postcode: the box takes a school
|
||||
// name OR a postcode, and suggesting schools during postcode entry fights
|
||||
// the user rather than helping them.
|
||||
const suggestEnabled = autosuggest && hasTyped && !isValidPostcode(omniValue);
|
||||
const { suggestions, open, activeIndex, setActiveIndex, close } =
|
||||
useSchoolSuggest(omniValue, suggestEnabled);
|
||||
|
||||
const pickSuggestion = (s: Suggestion) => {
|
||||
setHasTyped(false);
|
||||
close();
|
||||
track('search_submitted', {
|
||||
query: s.school_name.toLowerCase(),
|
||||
via: 'suggestion',
|
||||
urn: s.urn,
|
||||
has_postcode: false,
|
||||
filters_active: '',
|
||||
filters_count: 0,
|
||||
});
|
||||
router.push(schoolUrl(s.urn, s.school_name));
|
||||
};
|
||||
|
||||
const handleOmniKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
|
||||
if (!open) return;
|
||||
if (e.key === "ArrowDown") {
|
||||
e.preventDefault();
|
||||
setActiveIndex(activeIndex + 1 >= suggestions.length ? 0 : activeIndex + 1);
|
||||
} else if (e.key === "ArrowUp") {
|
||||
e.preventDefault();
|
||||
setActiveIndex(activeIndex <= 0 ? suggestions.length - 1 : activeIndex - 1);
|
||||
} else if (e.key === "Escape") {
|
||||
close();
|
||||
} else if (e.key === "Enter" && activeIndex >= 0) {
|
||||
// Only when an option is active. With none, the event falls through to
|
||||
// the form's submit handler and searches the typed text, as it does now.
|
||||
e.preventDefault();
|
||||
pickSuggestion(suggestions[activeIndex]);
|
||||
}
|
||||
};
|
||||
|
||||
const currentLA = searchParams.get("local_authority") || "";
|
||||
const currentType = searchParams.get("school_type") || "";
|
||||
const currentPhase = searchParams.get("phase") || "";
|
||||
@@ -124,6 +183,9 @@ export function FilterBar({
|
||||
|
||||
const handleSearchSubmit = (e: React.FormEvent) => {
|
||||
e.preventDefault();
|
||||
// The search has been made; the suggestions that led to it are spent.
|
||||
setHasTyped(false);
|
||||
close();
|
||||
if (!omniValue.trim()) {
|
||||
updateURL({ search: "", postcode: "", radius: "" });
|
||||
return;
|
||||
@@ -226,9 +288,20 @@ export function FilterBar({
|
||||
ref={inputRef}
|
||||
type="search"
|
||||
value={omniValue}
|
||||
onChange={(e) => setOmniValue(e.target.value)}
|
||||
onChange={(e) => { setOmniValue(e.target.value); setHasTyped(true); }}
|
||||
onKeyDown={handleOmniKeyDown}
|
||||
onBlur={close}
|
||||
placeholder="School name or postcode"
|
||||
className={styles.omniInput}
|
||||
{...(autosuggest ? {
|
||||
role: "combobox",
|
||||
"aria-expanded": open,
|
||||
"aria-controls": suggestId,
|
||||
"aria-autocomplete": "list" as const,
|
||||
"aria-activedescendant":
|
||||
activeIndex >= 0 ? suggestOptionId(suggestId, activeIndex) : undefined,
|
||||
autoComplete: "off",
|
||||
} : {})}
|
||||
/>
|
||||
<button
|
||||
type="submit"
|
||||
@@ -237,6 +310,15 @@ export function FilterBar({
|
||||
>
|
||||
{isPending ? <div className={styles.spinner}></div> : isHero ? "Search schools" : "Search"}
|
||||
</button>
|
||||
{autosuggest && open && (
|
||||
<SuggestList
|
||||
id={suggestId}
|
||||
suggestions={suggestions}
|
||||
activeIndex={activeIndex}
|
||||
onPick={pickSuggestion}
|
||||
onHover={setActiveIndex}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
{isHero && (
|
||||
<>
|
||||
|
||||
@@ -90,7 +90,18 @@
|
||||
isolation: isolate;
|
||||
background: var(--hero-ground);
|
||||
border-radius: var(--radius-xl);
|
||||
overflow: hidden;
|
||||
/*
|
||||
* Deliberately NOT overflow: hidden.
|
||||
*
|
||||
* It used to be, to clip the artwork and the scrim to the rounded corners —
|
||||
* and it also clipped the search box's suggestion dropdown, which is 320px
|
||||
* tall against 145px of panel below the input. Roughly half the list was cut
|
||||
* off with no indication anything was missing.
|
||||
*
|
||||
* The two things that actually needed clipping round themselves instead, so
|
||||
* the panel can let a dropdown out. Anything absolutely positioned inside
|
||||
* this panel and taller than the space below it depends on this.
|
||||
*/
|
||||
}
|
||||
|
||||
.heroContent {
|
||||
@@ -107,6 +118,10 @@
|
||||
position: absolute;
|
||||
inset: 0;
|
||||
z-index: 0;
|
||||
/* Rounds itself, because the panel no longer clips it. inset: 0 makes this
|
||||
exactly the panel's own corners. */
|
||||
border-radius: inherit;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.heroArt picture,
|
||||
@@ -143,6 +158,9 @@
|
||||
inset: 0;
|
||||
z-index: 1;
|
||||
pointer-events: none;
|
||||
/* Same reason as .heroArt: the panel stopped clipping, so the scrim keeps
|
||||
its own corners rather than squaring off over the panel's. */
|
||||
border-radius: inherit;
|
||||
background: linear-gradient(
|
||||
to right,
|
||||
var(--hero-ground) 0%,
|
||||
@@ -331,6 +349,10 @@
|
||||
position: static;
|
||||
order: -1;
|
||||
height: 13rem;
|
||||
/* Top corners only. Here the artwork is a band flush with the top of the
|
||||
panel, not a layer covering it — inheriting all four would leave it
|
||||
floating with rounded bottom corners against the copy below. */
|
||||
border-radius: var(--radius-xl) var(--radius-xl) 0 0;
|
||||
}
|
||||
/* The band crop puts the schoolhouse at 73% across — reported by
|
||||
scripts/build-hero-images.js, which derives it from the crop box rather
|
||||
|
||||
@@ -29,6 +29,8 @@ interface HomeViewProps {
|
||||
// show (e.g. an active search).
|
||||
howItWorks?: React.ReactNode;
|
||||
editorial?: React.ReactNode;
|
||||
/** Server-read feature flag, threaded to both FilterBar instances. */
|
||||
autosuggest?: boolean;
|
||||
}
|
||||
|
||||
function daysUntil(month: number, day: number): number {
|
||||
@@ -193,7 +195,7 @@ const VALUE_PROPS: ValueProp[] = [
|
||||
},
|
||||
];
|
||||
|
||||
export function HomeView({ initialSchools, filters, totalSchools, howItWorks, editorial }: HomeViewProps) {
|
||||
export function HomeView({ initialSchools, filters, totalSchools, howItWorks, editorial, autosuggest = false }: HomeViewProps) {
|
||||
const searchParams = useSearchParams();
|
||||
const router = useRouter();
|
||||
const pathname = usePathname();
|
||||
@@ -462,6 +464,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
|
||||
onNearMe={handleNearMe}
|
||||
geoState={geoState}
|
||||
geoError={geoError}
|
||||
autosuggest={autosuggest}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -500,6 +503,7 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
|
||||
onNearMe={handleNearMe}
|
||||
geoState={geoState}
|
||||
geoError={geoError}
|
||||
autosuggest={autosuggest}
|
||||
/>
|
||||
)}
|
||||
|
||||
|
||||
@@ -47,7 +47,13 @@
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
background:
|
||||
linear-gradient(100deg, rgba(255, 255, 255, 0) 40%, rgba(255, 255, 255, .5) 50%, rgba(255, 255, 255, 0) 60%) var(--bg-secondary);
|
||||
/* Sweeps toward the card colour, which is a shade lighter than this
|
||||
ground in both themes. Hardcoded white was a bright flash across a
|
||||
dark page every 1.4s while the tiles loaded. */
|
||||
linear-gradient(100deg,
|
||||
rgba(var(--bg-card-rgb), 0) 40%,
|
||||
rgba(var(--bg-card-rgb), .5) 50%,
|
||||
rgba(var(--bg-card-rgb), 0) 60%) var(--bg-secondary);
|
||||
background-size: 200% 100%;
|
||||
animation: shimmer 1.4s infinite;
|
||||
}
|
||||
@@ -76,6 +82,15 @@
|
||||
justify-content: center;
|
||||
}
|
||||
|
||||
/*
|
||||
* Controls that float ON the map.
|
||||
*
|
||||
* The map tiles are light in both themes, so these deliberately do NOT follow
|
||||
* the theme — they follow the map. The literal ink below is the point: paired
|
||||
* with a hardcoded white background, `color: var(--text-primary)` resolved to
|
||||
* #E9EEF0 in the dark theme and put near-white text on a near-white button.
|
||||
* A themed token is the wrong tool for a surface that never changes.
|
||||
*/
|
||||
.openHint {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
@@ -85,7 +100,8 @@
|
||||
border-radius: 999px;
|
||||
font-size: 13px;
|
||||
font-weight: 600;
|
||||
color: var(--text-primary);
|
||||
/* See "Controls that float ON the map" above. */
|
||||
color: #1C2731;
|
||||
background: rgba(255, 255, 255, .85);
|
||||
-webkit-backdrop-filter: blur(6px);
|
||||
backdrop-filter: blur(6px);
|
||||
@@ -113,11 +129,18 @@
|
||||
on top of the blend. */
|
||||
z-index: 450;
|
||||
pointer-events: none;
|
||||
/* The card colour, not white.
|
||||
This ramp was hardcoded white and ended at var(--bg-card). In the light
|
||||
theme that is white into white and invisible, as intended. In the dark
|
||||
theme it climbed to 95% WHITE and then met a near-black card — a bright
|
||||
band across the full width, right where the map is supposed to dissolve
|
||||
into the header. Fading to the same colour the gradient lands on is the
|
||||
whole trick, and it only works if that colour is a token. */
|
||||
background: linear-gradient(to bottom,
|
||||
rgba(255, 255, 255, 0) 0%,
|
||||
rgba(255, 255, 255, .35) 35%,
|
||||
rgba(255, 255, 255, .75) 62%,
|
||||
rgba(255, 255, 255, .95) 82%,
|
||||
rgba(var(--bg-card-rgb), 0) 0%,
|
||||
rgba(var(--bg-card-rgb), .35) 35%,
|
||||
rgba(var(--bg-card-rgb), .75) 62%,
|
||||
rgba(var(--bg-card-rgb), .95) 82%,
|
||||
var(--bg-card) 100%);
|
||||
}
|
||||
|
||||
@@ -134,7 +157,8 @@
|
||||
border: none;
|
||||
border-radius: 8px;
|
||||
background: rgba(255, 255, 255, .92);
|
||||
color: var(--text-primary);
|
||||
/* See "Controls that float ON the map" above. */
|
||||
color: #1C2731;
|
||||
cursor: pointer;
|
||||
box-shadow: 0 2px 10px rgba(var(--shadow-rgb), .2);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
/*
|
||||
* Anchored to .omniBoxContainer, which is position: relative for this reason.
|
||||
*
|
||||
* Every colour is a token, so the dropdown follows the theme. The dark theme
|
||||
* redefines --bg-card, --border, --text-muted and --shadow-soft, and this
|
||||
* inherits all four without a second rule.
|
||||
*/
|
||||
.list {
|
||||
position: absolute;
|
||||
top: calc(100% + 4px);
|
||||
left: 0;
|
||||
right: 0;
|
||||
/* Above the sticky filter bar (10) and the hero layers (0–2), below the
|
||||
skip-link (10000) and the modal overlay (1000). */
|
||||
z-index: 40;
|
||||
margin: 0;
|
||||
padding: 4px;
|
||||
list-style: none;
|
||||
max-height: 320px;
|
||||
overflow-y: auto;
|
||||
background: var(--bg-card);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-md);
|
||||
box-shadow: var(--shadow-soft);
|
||||
}
|
||||
|
||||
.option {
|
||||
display: flex;
|
||||
align-items: baseline;
|
||||
justify-content: space-between;
|
||||
gap: 12px;
|
||||
padding: 10px 12px;
|
||||
border-radius: var(--radius-sm);
|
||||
cursor: pointer;
|
||||
color: var(--text-primary);
|
||||
}
|
||||
|
||||
/* Hover and keyboard share one style: the active option is the active option
|
||||
however it became active. Two rules would drift. */
|
||||
.option:hover,
|
||||
.active {
|
||||
background: var(--bg-secondary);
|
||||
}
|
||||
|
||||
.name {
|
||||
font-weight: 500;
|
||||
}
|
||||
|
||||
.meta {
|
||||
font-size: 0.85em;
|
||||
color: var(--text-muted);
|
||||
white-space: nowrap;
|
||||
}
|
||||
@@ -0,0 +1,53 @@
|
||||
'use client';
|
||||
|
||||
/**
|
||||
* The autosuggest dropdown. Presentational only — it fetches nothing and owns
|
||||
* no state, so the fetching rules and the ARIA rules can be read separately.
|
||||
*/
|
||||
|
||||
import type { Suggestion } from '@/lib/suggest';
|
||||
import styles from './SuggestList.module.css';
|
||||
|
||||
/** The id the input's aria-activedescendant points at. */
|
||||
export function suggestOptionId(id: string, index: number): string {
|
||||
return `${id}-option-${index}`;
|
||||
}
|
||||
|
||||
interface Props {
|
||||
/** Shared with the input's aria-controls. */
|
||||
id: string;
|
||||
suggestions: Suggestion[];
|
||||
activeIndex: number;
|
||||
onPick: (s: Suggestion) => void;
|
||||
onHover: (index: number) => void;
|
||||
}
|
||||
|
||||
export function SuggestList({ id, suggestions, activeIndex, onPick, onHover }: Props) {
|
||||
if (suggestions.length === 0) return null;
|
||||
|
||||
return (
|
||||
<ul className={styles.list} id={id} role="listbox">
|
||||
{suggestions.map((s, i) => (
|
||||
<li
|
||||
key={s.urn}
|
||||
id={suggestOptionId(id, i)}
|
||||
role="option"
|
||||
aria-selected={i === activeIndex}
|
||||
className={`${styles.option} ${i === activeIndex ? styles.active : ''}`}
|
||||
/*
|
||||
* onMouseDown, not onClick: the input's blur handler closes the list,
|
||||
* and blur fires before click — so a click handler never runs. This
|
||||
* is the classic autosuggest bug where the dropdown is unclickable
|
||||
* with a mouse while working perfectly with a keyboard.
|
||||
*/
|
||||
onMouseDown={(e) => { e.preventDefault(); onPick(s); }}
|
||||
onMouseEnter={() => onHover(i)}
|
||||
>
|
||||
<span className={styles.name}>{s.school_name}</span>
|
||||
{/* Not decoration: there are many "St Mary's". */}
|
||||
<span className={styles.meta}>{s.local_authority}</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
);
|
||||
}
|
||||
@@ -164,6 +164,34 @@
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
/*
|
||||
* Attribute columns. Muted, because they qualify the row rather than compete
|
||||
* with the measure for it, and hugging their content so the school name keeps
|
||||
* the spare width — the same width:1% trick as .num, which is what stops six
|
||||
* columns from splitting evenly and squeezing the names into two lines each.
|
||||
*
|
||||
* .attr never wraps: "4–11" and "Yes" broken across lines read as two values.
|
||||
* .attrWide may — "Church of England" and some constituency names are long
|
||||
* enough that forcing one line would push the measure off a phone screen.
|
||||
*/
|
||||
.table th.attr,
|
||||
.table td.attr,
|
||||
.table th.attrWide,
|
||||
.table td.attrWide {
|
||||
color: var(--text-secondary);
|
||||
width: 1%;
|
||||
}
|
||||
|
||||
.table th.attr,
|
||||
.table td.attr {
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.table th.attrWide,
|
||||
.table td.attrWide {
|
||||
min-width: 8rem;
|
||||
}
|
||||
|
||||
/* The measure is spelled out; the tooltip carries the definition. */
|
||||
.metricHead {
|
||||
text-decoration: none;
|
||||
|
||||
@@ -13,8 +13,9 @@ import Link from 'next/link';
|
||||
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
|
||||
import { placeUrl, authoritySlug } from '@/lib/places';
|
||||
import type { School } from '@/lib/types';
|
||||
import { schoolUrl } from '@/lib/utils';
|
||||
import { schoolUrl, formatAgeSpan } from '@/lib/utils';
|
||||
import { absoluteUrl } from '@/lib/site';
|
||||
import { TrackPlaceView } from './TrackPlaceView';
|
||||
import styles from './PlaceView.module.css';
|
||||
|
||||
interface Props {
|
||||
@@ -63,8 +64,31 @@ function isPhase(school: School, phase: PhaseKey): boolean {
|
||||
: p.includes('primary') || p.includes('middle');
|
||||
}
|
||||
|
||||
/*
|
||||
* GIAS spells the absence of a faith as "Does not apply", and sometimes
|
||||
* "None" or "Not applicable" — database answers, not English ones. The school
|
||||
* page and the comparison already suppress all three; this is the same rule,
|
||||
* so the two surfaces cannot disagree about the same school.
|
||||
*/
|
||||
const NO_FAITH = /^(none|does not apply|not applicable)$/i;
|
||||
|
||||
/** An attribute the data does not carry. Distinct from the measure's "Not
|
||||
* published": four of those per row would drown the row it qualifies. */
|
||||
const NO_VALUE = '—';
|
||||
|
||||
function faithOf(school: School): string {
|
||||
const denom = school.religious_denomination ?? '';
|
||||
return denom && !NO_FAITH.test(denom) ? denom : NO_VALUE;
|
||||
}
|
||||
|
||||
function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) {
|
||||
const metric = METRICS[phase];
|
||||
/*
|
||||
* Nursery is a primary question. An all-through school renders in both
|
||||
* groups, and its nursery belongs to the primary reading of it — under
|
||||
* "Secondary schools" the column would be a fact about a different intake.
|
||||
*/
|
||||
const showNursery = phase === 'primary';
|
||||
return (
|
||||
<div className={styles.tableWrap}>
|
||||
<table className={styles.table}>
|
||||
@@ -78,6 +102,13 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
|
||||
{metric.heading}
|
||||
</abbr>
|
||||
</th>
|
||||
{/* The measure sits second, not last. Six columns overflow a
|
||||
phone and .tableWrap turns that into a swipe; last would put
|
||||
the one number the page exists for off the screen. */}
|
||||
<th scope="col" className={styles.attr}>Ages</th>
|
||||
<th scope="col" className={styles.attrWide}>Religious character</th>
|
||||
{showNursery && <th scope="col" className={styles.attr}>Nursery</th>}
|
||||
<th scope="col" className={styles.attrWide}>Constituency</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
@@ -95,6 +126,19 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
|
||||
? <span className={styles.noData}>Not published</span>
|
||||
: `${Math.round(Number(value))}${metric.unit}`}
|
||||
</td>
|
||||
<td className={styles.attr}>{formatAgeSpan(s.age_range) || NO_VALUE}</td>
|
||||
<td className={styles.attrWide}>{faithOf(s)}</td>
|
||||
{showNursery && (
|
||||
<td className={styles.attr}>
|
||||
{/* Undefined is a mart the pipeline has not rebuilt, and
|
||||
false is a school without one. Neither is a "Yes", and
|
||||
neither is worth two different words. */}
|
||||
{s.nursery_provision ? 'Yes' : NO_VALUE}
|
||||
</td>
|
||||
)}
|
||||
<td className={styles.attrWide}>
|
||||
{s.parliamentary_constituency || NO_VALUE}
|
||||
</td>
|
||||
</tr>
|
||||
);
|
||||
})}
|
||||
@@ -165,6 +209,11 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
||||
|
||||
return (
|
||||
<div className={styles.container}>
|
||||
{/* One line, and all four place families are measured, because they all
|
||||
render through this component. */}
|
||||
<TrackPlaceView kind={place.kind} slug={place.slug}
|
||||
count={place.count} phase={phase} />
|
||||
|
||||
<script
|
||||
type="application/ld+json"
|
||||
dangerouslySetInnerHTML={{ __html: JSON.stringify(jsonLd) }}
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
'use client';
|
||||
|
||||
/**
|
||||
* Fires `place_viewed` once per location page.
|
||||
*
|
||||
* A separate client component because PlaceView is a server component and
|
||||
* cannot call into the browser. It renders nothing — its whole job is the
|
||||
* effect, which keeps the page itself server-rendered.
|
||||
*
|
||||
* Umami already counts a pageview for every one of these URLs, so this is not
|
||||
* about traffic. It is about `kind`: whether to keep investing in the location
|
||||
* layer turns on which *sort* of page earns engagement — towns, authorities,
|
||||
* London localities or postcode districts — and a pageview cannot say, because
|
||||
* all four families share the /schools/ prefix and only the registry knows
|
||||
* which is which.
|
||||
*/
|
||||
|
||||
import { useEffect } from 'react';
|
||||
import { track, getNavigationSource } from '@/lib/analytics';
|
||||
|
||||
interface Props {
|
||||
kind: string;
|
||||
slug: string;
|
||||
count: number;
|
||||
phase?: 'primary' | 'secondary';
|
||||
}
|
||||
|
||||
export function TrackPlaceView({ kind, slug, count, phase }: Props) {
|
||||
useEffect(() => {
|
||||
track('place_viewed', {
|
||||
kind,
|
||||
slug,
|
||||
// "all" rather than omitting it, so the unphased page is a value in the
|
||||
// same field rather than a gap that has to be interpreted.
|
||||
phase: phase ?? 'all',
|
||||
school_count: count,
|
||||
// Internal navigation only. An arrival from Google reads as 'direct'
|
||||
// here; Umami's own pageview referrer is where external attribution
|
||||
// lives, and these pages exist to be arrived at externally.
|
||||
from: getNavigationSource(),
|
||||
});
|
||||
// Once per place, not once per render.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [kind, slug, phase]);
|
||||
|
||||
return null;
|
||||
}
|
||||
@@ -0,0 +1,58 @@
|
||||
'use client';
|
||||
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { fetchSuggestions, SUGGEST_MIN_QUERY, type Suggestion } from '@/lib/suggest';
|
||||
|
||||
/*
|
||||
* Long enough that a fast typist does not fire a request per character, short
|
||||
* enough that the list feels attached to the keyboard.
|
||||
*/
|
||||
const DEBOUNCE_MS = 200;
|
||||
|
||||
export function useSchoolSuggest(query: string, enabled: boolean) {
|
||||
const [suggestions, setSuggestions] = useState<Suggestion[]>([]);
|
||||
const [open, setOpen] = useState(false);
|
||||
const [activeIndex, setActiveIndex] = useState(-1);
|
||||
// Set when the user dismisses the list, so a re-render does not reopen it.
|
||||
const dismissed = useRef('');
|
||||
|
||||
useEffect(() => {
|
||||
const q = query.trim();
|
||||
if (!enabled || q.length < SUGGEST_MIN_QUERY || dismissed.current === q) {
|
||||
setSuggestions([]);
|
||||
setOpen(false);
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* Abort the superseded request on every keystroke. This is correctness,
|
||||
* not economy: without it a slow response for "st" can land after the fast
|
||||
* one for "st marys" and replace a correct list with a stale one.
|
||||
*/
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(async () => {
|
||||
const rows = await fetchSuggestions(q, controller.signal);
|
||||
if (controller.signal.aborted) return;
|
||||
setSuggestions(rows);
|
||||
setActiveIndex(-1);
|
||||
setOpen(rows.length > 0);
|
||||
}, DEBOUNCE_MS);
|
||||
|
||||
return () => {
|
||||
clearTimeout(timer);
|
||||
controller.abort();
|
||||
};
|
||||
}, [query, enabled]);
|
||||
|
||||
return {
|
||||
suggestions,
|
||||
open,
|
||||
activeIndex,
|
||||
setActiveIndex,
|
||||
close: () => {
|
||||
dismissed.current = query.trim();
|
||||
setOpen(false);
|
||||
setActiveIndex(-1);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -19,6 +19,7 @@ export type EventName =
|
||||
| 'empty_results'
|
||||
// Engagement
|
||||
| 'school_viewed'
|
||||
| 'place_viewed'
|
||||
| 'section_nav_used'
|
||||
| 'chart_metric_changed'
|
||||
| 'metric_compared_in_rankings'
|
||||
@@ -56,7 +57,10 @@ export function track(name: EventName, data?: Payload): void {
|
||||
* Categorise where the user navigated from, for funnel attribution
|
||||
* (mostly used on school_viewed). Only checks same-origin referrers.
|
||||
*/
|
||||
export function getNavigationSource(): 'search' | 'rankings' | 'compare' | 'detail' | 'direct' {
|
||||
export type NavigationSource =
|
||||
'search' | 'rankings' | 'compare' | 'detail' | 'place' | 'direct';
|
||||
|
||||
export function getNavigationSource(): NavigationSource {
|
||||
if (typeof window === 'undefined' || !document.referrer) return 'direct';
|
||||
try {
|
||||
const ref = new URL(document.referrer);
|
||||
@@ -65,6 +69,11 @@ export function getNavigationSource(): 'search' | 'rankings' | 'compare' | 'deta
|
||||
if (p === '/' || p === '') return 'search';
|
||||
if (p.startsWith('/rankings')) return 'rankings';
|
||||
if (p.startsWith('/compare')) return 'compare';
|
||||
// `/schools/` before `/school/`: they differ by one letter and mean
|
||||
// different things — the location layer versus a single school. Checked
|
||||
// first so the narrower-looking prefix cannot shadow it if either string
|
||||
// is ever edited.
|
||||
if (p.startsWith('/schools/')) return 'place';
|
||||
if (p.startsWith('/school/')) return 'detail';
|
||||
return 'direct';
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
/**
|
||||
* Client for /api/suggest.
|
||||
*
|
||||
* No `cache: "no-store"`. The compare modal's search uses it, and copying that
|
||||
* here would discard both the browser cache and the ETag 304s the backend's
|
||||
* CacheAndETagMiddleware already provides — on the one endpoint where prefix
|
||||
* queries repeat most.
|
||||
*/
|
||||
|
||||
export interface Suggestion {
|
||||
urn: number;
|
||||
school_name: string;
|
||||
local_authority: string;
|
||||
postcode: string;
|
||||
phase: string;
|
||||
school_type: string;
|
||||
}
|
||||
|
||||
/** Below this the response is thousands of schools and worth no round trip. */
|
||||
export const SUGGEST_MIN_QUERY = 2;
|
||||
|
||||
const API = process.env.NEXT_PUBLIC_API_URL || '/api';
|
||||
|
||||
/** Suggestions for `q`. Never throws: no suggestions is a fine outcome. */
|
||||
export async function fetchSuggestions(
|
||||
q: string, signal?: AbortSignal,
|
||||
): Promise<Suggestion[]> {
|
||||
if (q.trim().length < SUGGEST_MIN_QUERY) return [];
|
||||
try {
|
||||
const res = await fetch(`${API}/suggest?q=${encodeURIComponent(q.trim())}`,
|
||||
{ signal });
|
||||
if (!res.ok) return [];
|
||||
const body = await res.json();
|
||||
return body.suggestions ?? [];
|
||||
} catch {
|
||||
// Includes AbortError, which is the normal path on every keystroke.
|
||||
return [];
|
||||
}
|
||||
}
|
||||
+12
-2
@@ -82,11 +82,21 @@ export function shortName(name: string, maxLength = 32): string {
|
||||
* Display-only — leaves the raw `age_range` field (used for sixth-form
|
||||
* detection) untouched. Falls back to the raw value if it's not a plain range.
|
||||
*/
|
||||
export function formatAgeRange(ageRange: string | null | undefined): string {
|
||||
export function formatAgeSpan(ageRange: string | null | undefined): string {
|
||||
if (!ageRange) return '';
|
||||
const match = ageRange.match(/^\s*(\d+)\s*[-–]\s*(\d+)\s*$/);
|
||||
if (!match) return ageRange;
|
||||
return `Ages ${match[1]}–${match[2]}`;
|
||||
return `${match[1]}–${match[2]}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* The same span, labelled — for the places that show it with no column
|
||||
* heading to carry the word "Ages". Delegates so the en-dash normalisation
|
||||
* lives in one place.
|
||||
*/
|
||||
export function formatAgeRange(ageRange: string | null | undefined): string {
|
||||
const span = formatAgeSpan(ageRange);
|
||||
return /^\d+–\d+$/.test(span) ? `Ages ${span}` : span;
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
|
||||
Reference in new issue
Block a user