Self-review caught three defects in the plan. .omniBoxContainer, the wrapper the dropdown positions against, does not declare position: relative — without it the list anchors to the page. The postcode suppression test typed character by character, so it would have asserted no request while 'NW1' legitimately fires one; it now sets the value in one go. And the Enter-submits-search assertion needed waitFor, because updateURL pushes inside startTransition. Task 1 is the one to review hardest: it is the only unflagged change and it alters rate limiting for every endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
1354 lines
47 KiB
Markdown
1354 lines
47 KiB
Markdown
# School Autosuggest Implementation Plan
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**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.
|
||
|
||
**Architecture:** A dedicated `/api/suggest` endpoint answers from Typesense alone, touching no pandas. The frontend adds an ARIA combobox to the existing `FilterBar` omni-input, behind the `school_autosuggest` flag. First, the rate limiter is fixed to key on the real caller rather than the Next container — without that, autosuggest 429s the site.
|
||
|
||
**Tech Stack:** FastAPI, slowapi, Typesense, Next.js 15 App Router, React, Playwright, pytest, Jest.
|
||
|
||
**Spec:** `docs/superpowers/specs/2026-08-26-school-autosuggest-design.md`
|
||
|
||
## Global Constraints
|
||
|
||
- **The rate-limit fix is not flagged.** It is a correctness fix that applies whether or not autosuggest is on.
|
||
- **`/api/suggest` is not flagged either.** Only the UI is. A live endpoint with the UI dark is deliberate — it lets the endpoint be smoke-tested before the feature is switched on.
|
||
- **The keystroke path never returns an error for ordinary input.** Short query, no matches, Typesense down — all `200` with `{"suggestions": []}`.
|
||
- **No DataFrame fallback in `/api/suggest`.** The 25,000-row substring scan `/api/schools` falls back to is exactly the cost this endpoint exists to avoid.
|
||
- **`Enter` with no active option must still submit the free-text search**, exactly as today.
|
||
- **The client fetch must not use `cache: "no-store"`** — it would discard the browser cache and the ETag 304s the existing middleware already provides.
|
||
- **Every flag defaults to `False`**, declared in `backend/flags.py` with a `name`/`description`/`added` triple.
|
||
- Backend tests run via: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`
|
||
- Never push to `main`. Branch, PR, let checks pass.
|
||
- Do not start a local server to test the application; it does not work.
|
||
|
||
## File Structure
|
||
|
||
| File | Responsibility |
|
||
|---|---|
|
||
| `backend/app.py` (modify) | `client_key`, the `Limiter` construction, `GET /api/suggest`, one `CACHE_RULES` row. |
|
||
| `backend/data_loader.py` (modify) | `suggest_schools_typesense` — returns documents, not URNs. |
|
||
| `backend/flags.py` (modify) | Declare `school_autosuggest`. |
|
||
| `backend/tests/test_rate_limit_key.py` (new) | Keying precedence and bucket separation. |
|
||
| `backend/tests/test_suggest.py` (new) | Endpoint behaviour, including that it never touches the DataFrame. |
|
||
| `nextjs-app/lib/suggest.ts` (new) | `fetchSuggestions(q, signal)` — one typed fetch. |
|
||
| `nextjs-app/hooks/useSchoolSuggest.ts` (new) | Debounce, abort, stale-response rejection, open/active state. |
|
||
| `nextjs-app/components/SuggestList.tsx` (new) | Presentational list + ARIA. No fetching. |
|
||
| `nextjs-app/components/SuggestList.module.css` (new) | Dropdown styling. |
|
||
| `nextjs-app/components/FilterBar.tsx` (modify) | Wire hook + list to the existing input and form. |
|
||
| `nextjs-app/components/HomeView.tsx` (modify) | Pass `autosuggest` through to both `FilterBar` instances. |
|
||
| `nextjs-app/app/page.tsx` (modify) | Read the flag server-side, pass to both `HomeView` render sites. |
|
||
| `e2e/tests/journeys.spec.ts` (modify) | Flag-gated journeys. |
|
||
|
||
---
|
||
|
||
### Task 1: Key the rate limiter on the real caller
|
||
|
||
**Files:**
|
||
- Modify: `backend/app.py`
|
||
- Create: `backend/tests/test_rate_limit_key.py`
|
||
|
||
**Interfaces:**
|
||
- Produces: `backend.app.client_key(request: Request) -> str`
|
||
|
||
> Measured on staging before this change: 70 concurrent requests to
|
||
> `/api/schools` returned exactly 60 × 200 and 10 × 429. One machine consumed
|
||
> the whole site's budget, because `get_remote_address` returns the Next
|
||
> container's IP for every browser user.
|
||
|
||
- [ ] **Step 1: Write the failing test**
|
||
|
||
Create `backend/tests/test_rate_limit_key.py`:
|
||
|
||
```python
|
||
"""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"
|
||
```
|
||
|
||
- [ ] **Step 2: Run it to verify it fails**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_rate_limit_key.py -q`
|
||
|
||
Expected: FAIL — `ImportError: cannot import name 'client_key' from 'backend.app'`
|
||
|
||
- [ ] **Step 3: Implement the key function**
|
||
|
||
In `backend/app.py`, replace:
|
||
|
||
```python
|
||
# Rate limiter
|
||
limiter = Limiter(key_func=get_remote_address)
|
||
```
|
||
|
||
with:
|
||
|
||
```python
|
||
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)
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_rate_limit_key.py -q`
|
||
|
||
Expected: PASS, 6 tests.
|
||
|
||
- [ ] **Step 5: Run the full backend suite**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`
|
||
|
||
Expected: all pass.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add backend/app.py backend/tests/test_rate_limit_key.py
|
||
git commit -m "fix(api): rate-limit per caller, not per proxy"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 2: `suggest_schools_typesense`
|
||
|
||
**Files:**
|
||
- Modify: `backend/data_loader.py`
|
||
- Create: `backend/tests/test_suggest.py`
|
||
|
||
**Interfaces:**
|
||
- Produces: `backend.data_loader.suggest_schools_typesense(query: str, limit: int = 8) -> list[dict]`, each dict carrying `urn` (int), `school_name`, `local_authority`, `postcode`, `phase`, `school_type` (all str, `""` when the document omits an optional field).
|
||
|
||
> The existing `search_schools_typesense` returns URNs only, which forces the
|
||
> caller to hydrate from the DataFrame. Suggestions need the fields Typesense
|
||
> already holds — see the schema in `pipeline/scripts/sync_typesense.py`.
|
||
|
||
- [ ] **Step 1: Write the failing test**
|
||
|
||
Create `backend/tests/test_suggest.py`:
|
||
|
||
```python
|
||
"""Tests for school autosuggest (spec 2026-08-26)."""
|
||
|
||
import pytest
|
||
|
||
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
|
||
```
|
||
|
||
- [ ] **Step 2: Run it to verify it fails**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q`
|
||
|
||
Expected: FAIL — `AttributeError: module 'backend.data_loader' has no attribute 'suggest_schools_typesense'`
|
||
|
||
- [ ] **Step 3: Implement it**
|
||
|
||
In `backend/data_loader.py`, directly below `search_schools_typesense`:
|
||
|
||
```python
|
||
# 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
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q`
|
||
|
||
Expected: PASS, 5 tests.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/data_loader.py backend/tests/test_suggest.py
|
||
git commit -m "feat(suggest): Typesense rows for autosuggest, no DataFrame"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 3: `GET /api/suggest`
|
||
|
||
**Files:**
|
||
- Modify: `backend/app.py`
|
||
- Modify: `backend/tests/test_suggest.py`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `backend.data_loader.suggest_schools_typesense(query, limit) -> list[dict]`
|
||
- Produces: `GET /api/suggest?q=<str>&limit=<int>` → `200 {"suggestions": [...]}`
|
||
|
||
- [ ] **Step 1: Write the failing tests**
|
||
|
||
Append to `backend/tests/test_suggest.py`:
|
||
|
||
```python
|
||
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")
|
||
```
|
||
|
||
- [ ] **Step 2: Run them to verify they fail**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests/test_suggest.py -q`
|
||
|
||
Expected: FAIL — the endpoint 404s, so `body["suggestions"]` raises `KeyError`.
|
||
|
||
- [ ] **Step 3: Import the helper**
|
||
|
||
In `backend/app.py`, add `suggest_schools_typesense` to the existing
|
||
`from .data_loader import (...)` block, alongside `search_schools_typesense`.
|
||
|
||
- [ ] **Step 4: Add the cache rule**
|
||
|
||
In `backend/app.py`, add to `CACHE_RULES` — before the `/api/schools` row, so
|
||
the longest-prefix match is unaffected:
|
||
|
||
```python
|
||
("/api/suggest", (60, 3600, 86400)),
|
||
```
|
||
|
||
- [ ] **Step 5: Add the endpoint**
|
||
|
||
In `backend/app.py`, directly above `@app.get("/api/flags")`:
|
||
|
||
```python
|
||
# 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)}
|
||
```
|
||
|
||
- [ ] **Step 6: Run the tests to verify they pass**
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`
|
||
|
||
Expected: all pass.
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add backend/app.py backend/tests/test_suggest.py
|
||
git commit -m "feat(suggest): GET /api/suggest, cacheable and DataFrame-free"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 4: The client and the hook
|
||
|
||
**Files:**
|
||
- Create: `nextjs-app/lib/suggest.ts`
|
||
- Create: `nextjs-app/hooks/useSchoolSuggest.ts`
|
||
- Create: `nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx`
|
||
|
||
**Interfaces:**
|
||
- Produces:
|
||
- `export interface Suggestion { urn: number; school_name: string; local_authority: string; postcode: string; phase: string; school_type: string }`
|
||
- `fetchSuggestions(q: string, signal?: AbortSignal): Promise<Suggestion[]>`
|
||
- `useSchoolSuggest(query: string, enabled: boolean): { suggestions: Suggestion[]; open: boolean; activeIndex: number; setActiveIndex: (i: number) => void; close: () => void }`
|
||
|
||
- [ ] **Step 1: Write `lib/suggest.ts`**
|
||
|
||
```typescript
|
||
/**
|
||
* 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 [];
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Write the failing hook test**
|
||
|
||
Create `nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx`:
|
||
|
||
```tsx
|
||
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);
|
||
});
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Run it to verify it fails**
|
||
|
||
Run: `cd nextjs-app && npx jest __tests__/hooks/useSchoolSuggest.test.tsx`
|
||
|
||
Expected: FAIL — `Cannot find module '@/hooks/useSchoolSuggest'`
|
||
|
||
- [ ] **Step 4: Write the hook**
|
||
|
||
Create `nextjs-app/hooks/useSchoolSuggest.ts`:
|
||
|
||
```typescript
|
||
'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);
|
||
},
|
||
};
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 5: Run the tests to verify they pass**
|
||
|
||
Run: `cd nextjs-app && npx jest __tests__/hooks/useSchoolSuggest.test.tsx && npx tsc --noEmit`
|
||
|
||
Expected: PASS, 5 tests, typecheck clean.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add nextjs-app/lib/suggest.ts nextjs-app/hooks/useSchoolSuggest.ts nextjs-app/__tests__/hooks/useSchoolSuggest.test.tsx
|
||
git commit -m "feat(suggest): debounced, abortable suggestion hook"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 5: The list, with its ARIA
|
||
|
||
**Files:**
|
||
- Create: `nextjs-app/components/SuggestList.tsx`
|
||
- Create: `nextjs-app/components/SuggestList.module.css`
|
||
- Create: `nextjs-app/__tests__/components/SuggestList.test.tsx`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `Suggestion` from `@/lib/suggest`
|
||
- Produces: `<SuggestList id, suggestions, activeIndex, onPick, onHover />`, and `suggestOptionId(id: string, index: number): string`
|
||
|
||
- [ ] **Step 1: Write the failing test**
|
||
|
||
Create `nextjs-app/__tests__/components/SuggestList.test.tsx`:
|
||
|
||
```tsx
|
||
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();
|
||
});
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Run it to verify it fails**
|
||
|
||
Run: `cd nextjs-app && npx jest __tests__/components/SuggestList.test.tsx`
|
||
|
||
Expected: FAIL — `Cannot find module '@/components/SuggestList'`
|
||
|
||
- [ ] **Step 3: Write the component**
|
||
|
||
Create `nextjs-app/components/SuggestList.tsx`:
|
||
|
||
```tsx
|
||
'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>
|
||
);
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Write the stylesheet**
|
||
|
||
Create `nextjs-app/components/SuggestList.module.css`:
|
||
|
||
```css
|
||
/* Anchored to the search field's wrapper, which is position: relative. */
|
||
.list {
|
||
position: absolute;
|
||
top: calc(100% + 4px);
|
||
left: 0;
|
||
right: 0;
|
||
z-index: 40;
|
||
margin: 0;
|
||
padding: 4px;
|
||
list-style: none;
|
||
max-height: 320px;
|
||
overflow-y: auto;
|
||
background: var(--color-surface, #fff);
|
||
border: 1px solid var(--color-border, #d8d8d8);
|
||
border-radius: 10px;
|
||
box-shadow: 0 10px 30px rgb(0 0 0 / 12%);
|
||
}
|
||
|
||
.option {
|
||
display: flex;
|
||
align-items: baseline;
|
||
justify-content: space-between;
|
||
gap: 12px;
|
||
padding: 10px 12px;
|
||
border-radius: 6px;
|
||
cursor: pointer;
|
||
}
|
||
|
||
/* Hover and keyboard share one style: the active option is the active option
|
||
however it became active. */
|
||
.option:hover,
|
||
.active {
|
||
background: var(--color-surface-hover, #f1f1f1);
|
||
}
|
||
|
||
.name {
|
||
font-weight: 500;
|
||
}
|
||
|
||
.meta {
|
||
font-size: 0.85em;
|
||
color: var(--color-text-muted, #666);
|
||
white-space: nowrap;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 5: Run the tests to verify they pass**
|
||
|
||
Run: `cd nextjs-app && npx jest __tests__/components/SuggestList.test.tsx && npx tsc --noEmit`
|
||
|
||
Expected: PASS, 5 tests, typecheck clean.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add nextjs-app/components/SuggestList.tsx nextjs-app/components/SuggestList.module.css nextjs-app/__tests__/components/SuggestList.test.tsx
|
||
git commit -m "feat(suggest): the dropdown, with combobox ARIA"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 6: Wire it into the search box, behind the flag
|
||
|
||
**Files:**
|
||
- Modify: `backend/flags.py`
|
||
- Modify: `nextjs-app/app/page.tsx`
|
||
- Modify: `nextjs-app/components/HomeView.tsx`
|
||
- Modify: `nextjs-app/components/FilterBar.tsx`
|
||
- Modify: `nextjs-app/components/FilterBar.module.css`
|
||
- Create: `nextjs-app/__tests__/components/FilterBarSuggest.test.tsx`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `useSchoolSuggest`, `SuggestList`, `suggestOptionId`, `schoolUrl` from `@/lib/utils`, `getFlags` from `@/lib/flags`.
|
||
|
||
- [ ] **Step 1: Declare the flag**
|
||
|
||
In `backend/flags.py`, add to the `REGISTRY` tuple, after the
|
||
`admission_distance` entry:
|
||
|
||
```python
|
||
Flag(
|
||
name="school_autosuggest",
|
||
description=(
|
||
"School name suggestions as you type in the main search box."
|
||
),
|
||
added=date(2026, 8, 26),
|
||
),
|
||
```
|
||
|
||
- [ ] **Step 2: Write the failing test**
|
||
|
||
Create `nextjs-app/__tests__/components/FilterBarSuggest.test.tsx`:
|
||
|
||
```tsx
|
||
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')));
|
||
});
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 3: Run it to verify it fails**
|
||
|
||
Run: `cd nextjs-app && npx jest __tests__/components/FilterBarSuggest.test.tsx`
|
||
|
||
Expected: FAIL — `autosuggest` is not a prop, so no combobox is rendered.
|
||
|
||
- [ ] **Step 4: Wire the hook into FilterBar**
|
||
|
||
In `nextjs-app/components/FilterBar.tsx`:
|
||
|
||
Add to the imports:
|
||
|
||
```typescript
|
||
import { useSchoolSuggest } from "@/hooks/useSchoolSuggest";
|
||
import { SuggestList, suggestOptionId } from "./SuggestList";
|
||
import { schoolUrl } from "@/lib/utils";
|
||
import type { Suggestion } from "@/lib/suggest";
|
||
```
|
||
|
||
Add to `FilterBarProps`:
|
||
|
||
```typescript
|
||
/** Server-read feature flag. Off means no listener, no fetch, no markup. */
|
||
autosuggest?: boolean;
|
||
```
|
||
|
||
Add `autosuggest = false,` to the destructured parameters of `FilterBar`.
|
||
|
||
Inside the component, after the `omniValue` state declaration:
|
||
|
||
```typescript
|
||
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]);
|
||
}
|
||
};
|
||
```
|
||
|
||
- [ ] **Step 5: Wire the markup**
|
||
|
||
In `nextjs-app/components/FilterBar.tsx`, replace the existing omni `<input>`
|
||
element with:
|
||
|
||
```tsx
|
||
<input
|
||
ref={inputRef}
|
||
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",
|
||
} : {})}
|
||
/>
|
||
```
|
||
|
||
And directly after the closing `</button>` of the search submit button, still
|
||
inside the wrapper element that holds the input:
|
||
|
||
```tsx
|
||
{autosuggest && open && (
|
||
<SuggestList
|
||
id={suggestId}
|
||
suggestions={suggestions}
|
||
activeIndex={activeIndex}
|
||
onPick={pickSuggestion}
|
||
onHover={setActiveIndex}
|
||
/>
|
||
)}
|
||
```
|
||
|
||
- [ ] **Step 6: Anchor the dropdown**
|
||
|
||
The list is `position: absolute`, so its nearest positioned ancestor must be
|
||
the wrapper around the input and the submit button. That is
|
||
`.omniBoxContainer`, and it **does not currently declare `position: relative`**
|
||
— without this the dropdown anchors to the page and lands in the wrong place.
|
||
|
||
In `nextjs-app/components/FilterBar.module.css`, change:
|
||
|
||
```css
|
||
.omniBoxContainer {
|
||
display: flex;
|
||
align-items: center;
|
||
gap: 0.5rem;
|
||
}
|
||
```
|
||
|
||
to:
|
||
|
||
```css
|
||
.omniBoxContainer {
|
||
display: flex;
|
||
align-items: center;
|
||
gap: 0.5rem;
|
||
/* The suggestion dropdown is absolutely positioned against this box. */
|
||
position: relative;
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 7: Thread the flag from the server**
|
||
|
||
In `nextjs-app/app/page.tsx`, add the import:
|
||
|
||
```typescript
|
||
import { getFlags } from '@/lib/flags';
|
||
```
|
||
|
||
Inside `HomePage`, before the `try`:
|
||
|
||
```typescript
|
||
// Server-read: no flag value reaches the browser bundle.
|
||
const flags = await getFlags();
|
||
const autosuggest = flags.school_autosuggest === true;
|
||
```
|
||
|
||
Add `autosuggest={autosuggest}` to **both** `<HomeView ... />` render sites —
|
||
the success path and the `catch` fallback. Missing the second means the flag
|
||
silently does nothing whenever the API call fails.
|
||
|
||
In `nextjs-app/components/HomeView.tsx`, add `autosuggest?: boolean` to its
|
||
props interface, destructure it with a default of `false`, and pass
|
||
`autosuggest={autosuggest}` to **both** `<FilterBar ... />` instances — hero
|
||
and sticky.
|
||
|
||
- [ ] **Step 8: Run the tests to verify they pass**
|
||
|
||
Run: `cd nextjs-app && npx jest && npx tsc --noEmit && npm run build`
|
||
|
||
Expected: all pass, typecheck clean, build green.
|
||
|
||
- [ ] **Step 9: Run the backend suite**
|
||
|
||
The flag registry gained an entry, so the registry tests must still pass.
|
||
|
||
Run: `uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q`
|
||
|
||
Expected: all pass.
|
||
|
||
- [ ] **Step 10: Commit**
|
||
|
||
```bash
|
||
git add backend/flags.py nextjs-app/app/page.tsx nextjs-app/components/HomeView.tsx nextjs-app/components/FilterBar.tsx nextjs-app/components/FilterBar.module.css nextjs-app/__tests__/components/FilterBarSuggest.test.tsx
|
||
git commit -m "feat(suggest): wire autosuggest into the search box behind a flag"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 7: E2E journeys
|
||
|
||
**Files:**
|
||
- Modify: `e2e/tests/journeys.spec.ts`
|
||
|
||
- [ ] **Step 1: Write the journeys**
|
||
|
||
`/api/flags` is denied to the public, so feature state is read from its
|
||
observable effect — the same approach the distance journeys use.
|
||
|
||
Append to `e2e/tests/journeys.spec.ts`:
|
||
|
||
```typescript
|
||
/*
|
||
* 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/);
|
||
});
|
||
```
|
||
|
||
- [ ] **Step 2: Verify the spec compiles and the tests are collected**
|
||
|
||
Run: `cd e2e && npx playwright test --list`
|
||
|
||
Expected: the five new titles appear; total rises by 5.
|
||
|
||
- [ ] **Step 3: Commit**
|
||
|
||
```bash
|
||
git add e2e/tests/journeys.spec.ts
|
||
git commit -m "test(e2e): autosuggest journeys, gated on the flag"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 8: Verify and open the PR
|
||
|
||
- [ ] **Step 1: Run everything**
|
||
|
||
```bash
|
||
uv run --quiet --with-requirements requirements.txt --with pytest --with "httpx==0.27.0" python -m pytest backend/tests -q
|
||
cd nextjs-app && npx jest && npx tsc --noEmit && npm run build
|
||
cd ../e2e && npx playwright test --list
|
||
```
|
||
|
||
Expected: backend green, frontend green, typecheck clean, build green, e2e collects.
|
||
|
||
- [ ] **Step 2: Open the PR against `main`**
|
||
|
||
The body must state: the rate-limit keying change applies unflagged and
|
||
site-wide; autosuggest itself is off until toggled in Unleash; and that
|
||
`/api/suggest` is live regardless, deliberately.
|
||
|
||
## Notes for the executor
|
||
|
||
**Task 1 changes rate limiting for every endpoint.** It is the one change here
|
||
that is not behind a flag, and it is the one worth the most review attention.
|
||
Before it, everyone shares one 60/minute bucket; after it, each caller gets
|
||
their own. That is the intended fix, and it also removes an accidental global
|
||
throttle — the spec's §1 and Risks say so plainly. A global ceiling belongs at
|
||
Cloudflare and is deliberately not built here.
|
||
|
||
**Do not add an in-app global rate limit.** An earlier spec draft did. slowapi's
|
||
`default_limits` and `application_limits` are both keyed by `key_func`, so they
|
||
are per-client rather than global, and `application_limits` only apply with
|
||
`SlowAPIMiddleware` installed, which this app does not use.
|
||
|
||
**`onMouseDown`, not `onClick`, on the options.** The input's `onBlur` closes
|
||
the list and blur fires first, so a click handler never runs. This is the
|
||
classic bug where the dropdown works by keyboard and is dead to the mouse.
|
||
|
||
**Both render sites, twice over.** `page.tsx` renders `HomeView` in the success
|
||
path *and* the catch fallback; `HomeView` renders `FilterBar` as hero *and*
|
||
sticky. Missing any of the four makes the flag silently do nothing somewhere.
|