Compare commits

..
Author SHA1 Message Date
TudorandClaude Opus 5.5 bd7c8593d9 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
/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>
2026-10-01 14:45:16 +01:00
tudor 0c414680fd Merge pull request 'fix(search): offer every phase while a phase filter is applied' (#162) from fix/phase-filter-global into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 20s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m25s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 29s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 3m0s
Reviewed-on: #162
2026-10-01 11:03:20 +00:00
TudorandClaude Opus 5.5 e211e1376d fix(search): offer every phase while a phase filter is applied
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m12s
PR Checks / Backend Smoke (pull_request) Successful in 10s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
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 15s
The phase select read its options from the result-scoped filters, which
the backend computes after applying the phase filter. With secondary
chosen only secondary and all-through were offered, so switching to
primary meant going back to "Any phase" first. Read the global phase
list instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 11:06:04 +01:00
tudor 74418ca6b9 Merge pull request 'fix(search): keep the map list's count and sort on one line' (#161) from fix/map-pane-header into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 21s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 1m26s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 28s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 3m1s
Reviewed-on: #161
2026-10-01 09:25:17 +00:00
TudorandClaude Opus 5.5 5df8c93420 fix(search): keep the map list's count and sort on one line
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m12s
PR Checks / Backend Smoke (pull_request) Successful in 10s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m21s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 18s
Beside the map, "19 schools within 1.0 miles of SW196AR" (359px) and the
sort menu (281px, as wide as its longest option) could not share the
pane's 427px, so the sort dropped to a second line.

In the map view the heading now reads "19 schools within 1 mile": the
postcode is already in the search box just above, and the radius reads
as the toolbar's does. The sort menu is capped at 11rem, truncating a
long choice, and the header no longer wraps; on a narrow pane the count
truncates rather than pushing the sort down. List view keeps the full
heading. Tried on staging by injecting the change: 243px + 176px, one
line at 1440.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 10:18:26 +01:00
8 changed files with 203 additions and 11 deletions

No files matched your search

+6 -4
View File
@@ -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:
+84
View File
@@ -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") == []
+31
View File
@@ -272,6 +272,31 @@ test('searching by postcode returns nearby schools', async ({ page }) => {
await expect(schoolLinks(page).first()).toBeVisible({ timeout: 15_000 });
});
test('the phase filter switches straight from secondary to primary', async ({ page }) => {
// The phase options once came from the result set, which the phase filter
// had already narrowed — so with secondary chosen, primary was not offered.
await page.goto('/?search=school&phase=secondary');
const phase = page.getByRole('combobox', { name: 'Phase' });
await expect(phase).toHaveValue('secondary', { timeout: 15_000 });
await phase.selectOption('primary');
await expect(page).toHaveURL(/[?&]phase=primary(&|$)/);
await expect(phase).toHaveValue('primary');
});
test('a phase outside primary/secondary filters to that phase, not to everything', async ({ page }) => {
// The search page offers every GIAS phase, but the API only knew the grouped
// ones and silently dropped the rest — so "Nursery" returned primaries.
for (const phase of ['Nursery', '16 plus']) {
const res = await page.request.get(
`/api/schools?phase=${encodeURIComponent(phase.toLowerCase())}&page_size=50`);
expect(res.ok()).toBeTruthy();
const phases = new Set(
((await res.json()).schools ?? []).map((s: { phase?: string }) => s.phase));
expect([...phases], `phase=${phase} returned other phases`)
.toEqual(phases.size ? [phase] : []);
}
});
test('a report-card school shows a Report Card badge in search results, not its old grade', async ({ page }) => {
// List/map badges keyed off ofsted_grade (the carried-forward legacy grade)
// and never reached the report-card branch, so report-card schools were
@@ -567,6 +592,12 @@ test('a desktop postcode search opens on the map with the list beside it', async
await expect(card.getByRole('button', { name: /Compar/ })).toBeVisible();
await expect(page.locator('.sc-pin').first()).toBeVisible({ timeout: 15_000 });
// The count and the sort share one line at the top of the list.
const heading = await pane.getByRole('heading', { level: 2 }).boundingBox();
const sort = await pane.getByRole('combobox').boundingBox();
expect(Math.abs((heading!.y + heading!.height / 2) - (sort!.y + sort!.height / 2)))
.toBeLessThan(8);
// The split runs to the bottom of the screen rather than stopping short.
const split = await page.locator('[class*="mapViewContainer"]').boundingBox();
expect(split!.y + split!.height).toBeGreaterThan(800 - 40);
@@ -0,0 +1,40 @@
import { render, screen, within } from '@testing-library/react';
import { FilterBar } from '@/components/FilterBar';
let searchParams = new URLSearchParams();
jest.mock('next/navigation', () => ({
useRouter: () => ({ push: jest.fn(), replace: jest.fn(), prefetch: jest.fn() }),
usePathname: () => '/',
useSearchParams: () => searchParams,
}));
const FILTERS = {
local_authorities: [], school_types: [], years: [],
phases: ['Primary', 'Secondary', 'All-through'],
genders: [], admissions_policies: [],
};
/**
* The phase options must not come from the result set. The backend scopes its
* result filters to the schools it returns, and it applies the phase filter
* first — so with "secondary" chosen the scoped list holds only secondary-ish
* phases, and switching to primary meant going back to "Any phase" first.
*/
describe('FilterBar phase options', () => {
it('offers every phase while a phase filter narrows the results', () => {
searchParams = new URLSearchParams('search=hampton&phase=secondary');
render(
<FilterBar
filters={FILTERS}
resultFilters={{
local_authorities: [], school_types: [],
phases: ['Secondary', 'All-through'],
genders: [], admissions_policies: [],
}}
/>,
);
const phase = screen.getByRole('combobox', { name: 'Phase' });
expect(within(phase).getByRole('option', { name: 'Primary' })).toBeInTheDocument();
expect(phase).toHaveValue('secondary');
});
});
@@ -94,7 +94,8 @@ it('lists a name search, which has no map', async () => {
it('puts the count and the sort in the list beside the map, once', async () => {
await renderMap();
expect(screen.getAllByRole('heading', { name: /3 schools within 1.0 miles of SW196AR/ })).toHaveLength(1);
// Short beside the map, so it shares one line with the sort.
expect(screen.getAllByRole('heading', { name: '3 schools within 1 mile' })).toHaveLength(1);
expect(screen.getAllByRole('combobox')).toHaveLength(1);
});
@@ -172,3 +173,9 @@ it('lets a keyboard pick a pin from the list, with a real button', async () => {
expect(screen.getByTestId('map')).toHaveAttribute('data-selected', '2');
expect(show).toHaveAttribute('aria-pressed', 'true');
});
it('keeps the postcode in the heading in list view, where there is room', async () => {
await renderMap();
fireEvent.click(screen.getByRole('button', { name: 'List' }));
expect(screen.getByRole('heading', { name: '3 schools within 1.0 miles of SW196AR' })).toBeInTheDocument();
});
+4 -1
View File
@@ -295,7 +295,10 @@ export function FilterBar({
const laOptions =
resultFilters?.local_authorities ?? filters.local_authorities;
const typeOptions = resultFilters?.school_types ?? filters.school_types;
const phaseOptions = resultFilters?.phases ?? filters.phases ?? [];
// Phase is the exception: always the full list. The result set has already
// been narrowed by the phase filter, so scoping to it would leave only the
// chosen phase on offer and switching phase would need "Any phase" first.
const phaseOptions = filters.phases ?? [];
const genderOptions = resultFilters?.genders ?? filters.genders ?? [];
const admissionsPolicyOptions =
resultFilters?.admissions_policies ?? filters.admissions_policies ?? [];
+21 -2
View File
@@ -608,13 +608,32 @@
border-right: 1px solid var(--border);
}
/* One line: the count takes what the sort leaves, and truncates rather than
wrap if a narrow pane cannot fit both. */
.mapListPane .resultsHeader {
gap: 0.5rem;
flex-wrap: nowrap;
gap: 0.75rem;
padding: 0.875rem 1rem 0.75rem;
}
.mapListPane .resultsHeader h2 {
font-size: var(--step-1);
flex: 1;
min-width: 0;
font-size: var(--step-0);
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
.mapListPane .resultsHeaderActions {
flex-shrink: 0;
}
/* A select is as wide as its longest option ("Highest Reading, Writing &
Maths %", 281px), more than half the pane. Cap it; a long choice truncates. */
.mapListPane .sortSelect {
max-width: 11rem;
text-overflow: ellipsis;
}
.mapContainer {
+9 -3
View File
@@ -583,14 +583,20 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
)
);
const schoolCount = `${initialSchools.total.toLocaleString()} school${initialSchools.total !== 1 ? 's' : ''}`;
// The count and the sort. Above the list in list view; at the top of the
// list pane, beside the map, in map view.
const resultsHeader = (
<div className={styles.resultsHeader}>
<h2 aria-live="polite" aria-atomic="true">
{isLocationSearch && initialSchools.location_info
? `${initialSchools.total.toLocaleString()} school${initialSchools.total !== 1 ? 's' : ''} within ${(initialSchools.location_info.radius / 1.60934).toFixed(1)} miles of ${initialSchools.location_info.postcode}`
: `${initialSchools.total.toLocaleString()} school${initialSchools.total !== 1 ? 's' : ''} found`
{/* Beside the map the heading shares one line with the sort, so it
drops the postcode, which the search box just above already shows. */}
{isMapView && radiusMiles != null
? `${schoolCount} within ${radiusMiles} mile${radiusMiles === 1 ? '' : 's'}`
: isLocationSearch && initialSchools.location_info
? `${schoolCount} within ${(initialSchools.location_info.radius / 1.60934).toFixed(1)} miles of ${initialSchools.location_info.postcode}`
: `${schoolCount} found`
}
</h2>
<div className={styles.resultsHeaderActions}>