fix(api): filter by every GIAS phase, not just the grouped ones
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m13s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 20s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m20s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 16s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m13s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 20s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m20s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 16s
/api/schools only recognised primary, secondary and all-through. Any other phase the search page offers (nursery, 16 plus, middle deemed primary/secondary) fell through to no filter, so "Nursery" returned the whole result set, mostly primaries. Ungrouped phases now match exactly, and an unknown phase returns nothing rather than everything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0c414680fd
commit
bd7c8593d9
3 files changed
+104
-4
No files matched your search
+6
-4
@@ -736,7 +736,7 @@ async def get_schools(
|
||||
None, description="Filter by local authority", max_length=100
|
||||
),
|
||||
school_type: Optional[str] = Query(None, description="Filter by school type", max_length=100),
|
||||
phase: Optional[str] = Query(None, description="Filter by phase: primary, secondary, all-through", max_length=50),
|
||||
phase: Optional[str] = Query(None, description="Filter by phase: primary or secondary (grouped), or any GIAS phase name (exact)", max_length=50),
|
||||
postcode: Optional[str] = Query(None, description="Search near postcode", max_length=10),
|
||||
radius: float = Query(5.0, ge=0.1, le=5, description="Search radius in miles"),
|
||||
page: int = Query(1, ge=1, le=1000, description="Page number"),
|
||||
@@ -771,11 +771,13 @@ async def get_schools(
|
||||
|
||||
# Phase filter — uses PHASE_GROUPS so all-through/middle schools appear
|
||||
# in the correct phase(s) rather than being invisible to both filters.
|
||||
# Any other GIAS phase (nursery, 16 plus, middle deemed ...) is an exact
|
||||
# match. It must never fall through to no filter: the search page offers
|
||||
# every phase, and "Nursery" used to return the whole result set.
|
||||
if phase:
|
||||
phase_lower = phase.lower().replace("_", "-")
|
||||
allowed = PHASE_GROUPS.get(phase_lower)
|
||||
if allowed:
|
||||
df_latest = df_latest[df_latest["phase"].str.lower().isin(allowed)]
|
||||
allowed = PHASE_GROUPS.get(phase_lower, {phase_lower})
|
||||
df_latest = df_latest[df_latest["phase"].fillna("").str.lower().isin(allowed)]
|
||||
|
||||
# Secondary-specific filters (after phase filter)
|
||||
if gender:
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
"""The /api/schools phase filter.
|
||||
|
||||
The search page offers every GIAS phase, but the filter only knew the three
|
||||
grouped ones (primary, secondary, all-through). Anything else — nursery,
|
||||
16 plus, the middle-deemed phases — fell through to no filter at all, so
|
||||
"Nursery" returned the whole result set, mostly primaries.
|
||||
"""
|
||||
|
||||
import numpy as np
|
||||
import pandas as pd
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
PHASES = {
|
||||
100001: "Nursery",
|
||||
100002: "Primary",
|
||||
100003: "Middle deemed primary",
|
||||
100004: "Secondary",
|
||||
100005: "Middle deemed secondary",
|
||||
100006: "16 plus",
|
||||
100007: "All-through",
|
||||
}
|
||||
|
||||
|
||||
def _schools_df() -> pd.DataFrame:
|
||||
base = {
|
||||
"local_authority": "Testshire",
|
||||
"school_type": "Academy",
|
||||
"address": "1 Test Street",
|
||||
"town": "Testtown",
|
||||
"postcode": "TS1 1AA",
|
||||
"religious_denomination": None,
|
||||
"age_range": "4-11",
|
||||
"has_sixth_form": None,
|
||||
"gender": "Mixed",
|
||||
"admissions_policy": None,
|
||||
"ofsted_grade": np.nan,
|
||||
"ofsted_date": None,
|
||||
"ofsted_framework": None,
|
||||
"latitude": 51.5,
|
||||
"longitude": -0.1,
|
||||
"year": 202425,
|
||||
"total_pupils": 300,
|
||||
"rwm_expected_pct": np.nan,
|
||||
"attainment_8_score": np.nan,
|
||||
}
|
||||
return pd.DataFrame([
|
||||
{**base, "urn": urn, "school_name": f"{phase} School", "phase": phase}
|
||||
for urn, phase in PHASES.items()
|
||||
])
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(monkeypatch):
|
||||
from backend import app as app_module
|
||||
|
||||
monkeypatch.setattr(app_module, "load_latest_school_data", _schools_df)
|
||||
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
|
||||
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
def _urns(client, phase):
|
||||
resp = client.get("/api/schools", params={"phase": phase})
|
||||
assert resp.status_code == 200, resp.text
|
||||
return sorted(s["urn"] for s in resp.json()["schools"])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("phase, urn", [
|
||||
("nursery", 100001),
|
||||
("16 plus", 100006),
|
||||
("middle deemed primary", 100003),
|
||||
("middle deemed secondary", 100005),
|
||||
])
|
||||
def test_an_ungrouped_phase_matches_exactly(client, phase, urn):
|
||||
assert _urns(client, phase) == [urn]
|
||||
|
||||
|
||||
def test_grouped_phases_still_take_in_their_related_phases(client):
|
||||
assert _urns(client, "primary") == [100002, 100003, 100007]
|
||||
assert _urns(client, "secondary") == [100004, 100005, 100006, 100007]
|
||||
|
||||
|
||||
def test_an_unknown_phase_returns_nothing_rather_than_everything(client):
|
||||
assert _urns(client, "kindergarten") == []
|
||||
Reference in new issue
Block a user