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