Compare commits
14
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3236efa846 | ||
|
|
d8ccb5b733 | ||
|
|
0fa1a292c7 | ||
|
|
c3f044bd65 | ||
|
|
d2115364ae | ||
|
|
28cf0a342c | ||
|
|
d88e77f459 | ||
|
|
06eb433db5 | ||
|
|
1a6d349dad | ||
|
|
75d3534d82 | ||
|
|
ff041544f2 | ||
|
|
6e0a278340 | ||
|
|
e651dd0d65 | ||
|
|
22c113fc29 |
No files matched your search
+135
-3
@@ -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(
|
||||
@@ -1271,6 +1371,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),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
@@ -2111,3 +2111,66 @@ 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));
|
||||
});
|
||||
|
||||
/*
|
||||
* 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('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,76 @@
|
||||
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();
|
||||
jest.mock('next/navigation', () => ({
|
||||
useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
|
||||
usePathname: () => '/',
|
||||
useSearchParams: () => new URLSearchParams(),
|
||||
}));
|
||||
|
||||
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();
|
||||
});
|
||||
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')));
|
||||
});
|
||||
});
|
||||
@@ -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,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,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);
|
||||
});
|
||||
});
|
||||
@@ -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. */
|
||||
|
||||
@@ -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,45 @@ export function FilterBar({
|
||||
|
||||
const [omniValue, setOmniValue] = useState(initialOmniValue);
|
||||
|
||||
const suggestId = `school-suggest-${isHero ? "hero" : "bar"}`;
|
||||
// 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 && !isValidPostcode(omniValue);
|
||||
const { suggestions, open, activeIndex, setActiveIndex, close } =
|
||||
useSchoolSuggest(omniValue, suggestEnabled);
|
||||
|
||||
const pickSuggestion = (s: Suggestion) => {
|
||||
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") || "";
|
||||
@@ -227,8 +272,19 @@ export function FilterBar({
|
||||
type="search"
|
||||
value={omniValue}
|
||||
onChange={(e) => setOmniValue(e.target.value)}
|
||||
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 +293,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 && (
|
||||
<>
|
||||
|
||||
@@ -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>
|
||||
);
|
||||
}
|
||||
@@ -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);
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -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 [];
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user