fix(api): school detail 500s for schools with no performance rows #20
@@ -51,11 +51,14 @@ jobs:
|
|||||||
python-version: "3.12"
|
python-version: "3.12"
|
||||||
|
|
||||||
- name: Install dependencies
|
- name: Install dependencies
|
||||||
run: pip install -r requirements.txt
|
run: pip install -r requirements.txt pytest "httpx<0.28"
|
||||||
|
|
||||||
- name: Import smoke test
|
- name: Import smoke test
|
||||||
run: python -c "from backend.app import app; print('backend imports OK')"
|
run: python -c "from backend.app import app; print('backend imports OK')"
|
||||||
|
|
||||||
|
- name: Backend unit tests
|
||||||
|
run: python -m pytest backend/tests -q
|
||||||
|
|
||||||
build-backend:
|
build-backend:
|
||||||
name: Build Backend (no push)
|
name: Build Backend (no push)
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
|||||||
+1
-1
@@ -1,2 +1,2 @@
|
|||||||
venv
|
venv
|
||||||
backend/__pycache__
|
__pycache__/
|
||||||
|
|||||||
+13
-4
@@ -33,7 +33,7 @@ from .data_loader import (
|
|||||||
)
|
)
|
||||||
from .data_loader import get_data_info as get_db_info
|
from .data_loader import get_data_info as get_db_info
|
||||||
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
from .schemas import METRIC_DEFINITIONS, RANKING_COLUMNS, SCHOOL_COLUMNS
|
||||||
from .utils import clean_for_json
|
from .utils import clean_for_json, convert_to_native
|
||||||
|
|
||||||
# Values to exclude from filter dropdowns (empty strings, non-applicable labels)
|
# Values to exclude from filter dropdowns (empty strings, non-applicable labels)
|
||||||
EXCLUDED_FILTER_VALUES = {"", "Not applicable", "Does not apply"}
|
EXCLUDED_FILTER_VALUES = {"", "Not applicable", "Does not apply"}
|
||||||
@@ -582,8 +582,13 @@ async def get_school_details(request: Request, urn: int):
|
|||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
return {
|
# Schools with no performance rows (post-16 institutions, PRUs, new
|
||||||
"school_info": {
|
# schools) carry NaN in every LEFT-JOINed numeric column; NaN reaching
|
||||||
|
# JSONResponse raises ValueError, so school_info needs the same
|
||||||
|
# conversion yearly_data gets from clean_for_json.
|
||||||
|
school_info = {
|
||||||
|
k: convert_to_native(v)
|
||||||
|
for k, v in {
|
||||||
"urn": urn,
|
"urn": urn,
|
||||||
"school_name": latest.get("school_name", ""),
|
"school_name": latest.get("school_name", ""),
|
||||||
"local_authority": latest.get("local_authority", ""),
|
"local_authority": latest.get("local_authority", ""),
|
||||||
@@ -601,7 +606,11 @@ async def get_school_details(request: Request, urn: int):
|
|||||||
"total_pupils": latest.get("gias_total_pupils"),
|
"total_pupils": latest.get("gias_total_pupils"),
|
||||||
"trust_name": latest.get("trust_name"),
|
"trust_name": latest.get("trust_name"),
|
||||||
"gender": latest.get("gender"),
|
"gender": latest.get("gender"),
|
||||||
},
|
}.items()
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
"school_info": school_info,
|
||||||
"yearly_data": clean_for_json(school_data),
|
"yearly_data": clean_for_json(school_data),
|
||||||
# Supplementary data (null if not yet populated by Kestra)
|
# Supplementary data (null if not yet populated by Kestra)
|
||||||
"ofsted": supplementary.get("ofsted"),
|
"ofsted": supplementary.get("ofsted"),
|
||||||
|
|||||||
@@ -0,0 +1,71 @@
|
|||||||
|
"""Regression tests for GET /api/schools/{urn}.
|
||||||
|
|
||||||
|
Schools with no performance rows (special post-16 institutions, sixth-form
|
||||||
|
centres, PRUs, brand-new schools) come back from the marts LEFT JOIN with
|
||||||
|
NaN in every numeric column. The endpoint must still serialize them — a NaN
|
||||||
|
that reaches Starlette's JSONResponse raises ValueError (allow_nan=False)
|
||||||
|
and the route 500s, which the frontend then renders as a 404.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import numpy as np
|
||||||
|
import pandas as pd
|
||||||
|
import pytest
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
|
def _no_results_school_df() -> pd.DataFrame:
|
||||||
|
"""One school row as produced by the marts query for a school with no
|
||||||
|
performance data: GIAS/location fields partly populated, every
|
||||||
|
results-linked column NaN (including year)."""
|
||||||
|
return 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,
|
||||||
|
}
|
||||||
|
]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def client(monkeypatch):
|
||||||
|
from backend import app as app_module
|
||||||
|
|
||||||
|
monkeypatch.setattr(app_module, "load_school_data", _no_results_school_df)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
app_module, "get_supplementary_data", lambda db, urn: {}
|
||||||
|
)
|
||||||
|
return TestClient(app_module.app, raise_server_exceptions=False)
|
||||||
|
|
||||||
|
|
||||||
|
def test_school_without_performance_rows_returns_200(client):
|
||||||
|
resp = client.get("/api/schools/150275")
|
||||||
|
assert resp.status_code == 200, resp.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_nan_gias_fields_serialize_as_null(client):
|
||||||
|
info = client.get("/api/schools/150275").json()["school_info"]
|
||||||
|
assert info["capacity"] is None
|
||||||
|
assert info["total_pupils"] is None
|
||||||
|
assert info["school_name"] == "West London Performing Arts Academy"
|
||||||
@@ -60,6 +60,34 @@ test('school detail page renders name and performance data', async ({ page }) =>
|
|||||||
await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
|
await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('school with no performance data still gets a working detail page', async ({ page }) => {
|
||||||
|
// Schools without KS2/KS4 results (special post-16 institutions, sixth-form
|
||||||
|
// centres, PRUs) used to 500 in the API — NaN GIAS fields broke JSON
|
||||||
|
// serialization — which the frontend rendered as a 404 on every such SEO
|
||||||
|
// landing page. Find one via the search API (year === null marks "no
|
||||||
|
// performance rows") and assert its page renders.
|
||||||
|
const candidates: number[] = [];
|
||||||
|
for (const q of ['post 16', 'specialist college', 'sixth form']) {
|
||||||
|
const resp = await page.request.get(
|
||||||
|
`/api/schools?search=${encodeURIComponent(q)}&per_page=20`
|
||||||
|
);
|
||||||
|
if (!resp.ok()) continue;
|
||||||
|
const body = await resp.json();
|
||||||
|
for (const s of body.schools ?? []) {
|
||||||
|
if (s.year === null && s.urn) candidates.push(s.urn);
|
||||||
|
}
|
||||||
|
if (candidates.length) break;
|
||||||
|
}
|
||||||
|
test.skip(candidates.length === 0, 'no results-less school in this dataset');
|
||||||
|
|
||||||
|
const detail = await page.request.get(`/api/schools/${candidates[0]}`);
|
||||||
|
expect(detail.status(), 'detail API must not 500 for a results-less school').toBe(200);
|
||||||
|
|
||||||
|
await page.goto(`/school/${candidates[0]}`);
|
||||||
|
await page.waitForURL(/\/school\/\d+-/); // redirected to canonical slug
|
||||||
|
await expect(page.locator('h1').first()).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
test('school hero map opens fullscreen on mobile without the Fullscreen API', async ({ page }) => {
|
test('school hero map opens fullscreen on mobile without the Fullscreen API', async ({ page }) => {
|
||||||
// iOS Safari has no Element.requestFullscreen; the map must fall back to a
|
// iOS Safari has no Element.requestFullscreen; the map must fall back to a
|
||||||
// CSS overlay. Simulate that by removing the API before any page script runs.
|
// CSS overlay. Simulate that by removing the API before any page script runs.
|
||||||
|
|||||||
Reference in New Issue
Block a user