diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index be8783a..36a067b 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -51,11 +51,14 @@ jobs: python-version: "3.12" - name: Install dependencies - run: pip install -r requirements.txt + run: pip install -r requirements.txt pytest "httpx<0.28" - name: Import smoke test 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: name: Build Backend (no push) runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 100ba7f..81b56a7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,2 @@ venv -backend/__pycache__ +__pycache__/ diff --git a/backend/app.py b/backend/app.py index 9a68e21..a146f19 100644 --- a/backend/app.py +++ b/backend/app.py @@ -33,7 +33,7 @@ from .data_loader import ( ) from .data_loader import get_data_info as get_db_info 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) EXCLUDED_FILTER_VALUES = {"", "Not applicable", "Does not apply"} @@ -582,8 +582,13 @@ async def get_school_details(request: Request, urn: int): except Exception: pass - return { - "school_info": { + # Schools with no performance rows (post-16 institutions, PRUs, new + # 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, "school_name": latest.get("school_name", ""), "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"), "trust_name": latest.get("trust_name"), "gender": latest.get("gender"), - }, + }.items() + } + + return { + "school_info": school_info, "yearly_data": clean_for_json(school_data), # Supplementary data (null if not yet populated by Kestra) "ofsted": supplementary.get("ofsted"), diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/test_school_details.py b/backend/tests/test_school_details.py new file mode 100644 index 0000000..f2a20ac --- /dev/null +++ b/backend/tests/test_school_details.py @@ -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" diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index dbb8923..cf68899 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -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 }); }); +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 }) => { // 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.