Compare commits

..
Author SHA1 Message Date
TudorandClaude Opus 5.5 8ebe461435 fix(search): set the toolbar's line count by width, not by results
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 1m19s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 11s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 18s
The results toolbar wrapped wherever it ran out of room, and the List/Map
switch only appears when there are results, so the same search took two
lines with results and one without.

From 1340px the controls never wrap away from the search, which takes
what they leave (at least 12rem); phase and type chips cap at 11rem to
fit. Between 641px and 1339px the controls always take a full line of
their own. The switch now sits in FilterBar's row via a viewSwitch slot,
so that line runs the full width instead of stopping short of it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 15:27:14 +01:00
tudor 2002529137 Merge pull request 'fix(api): filter by every GIAS phase, not just the grouped ones' (#163) from fix/phase-filter-exact-match into main
Stage (build -> staging -> E2E gate) / prepare (push) Successful in 1s
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 22s
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 3m3s
Reviewed-on: #163
2026-10-01 14:07:56 +00:00
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
9 changed files with 279 additions and 34 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") == []
+57
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
@@ -547,6 +572,38 @@ test('the results toolbar stays pinned with its List/Map switch', async ({ page
.toHaveAttribute('aria-pressed', 'true');
});
/*
* The toolbar's line count is set by the screen width, never by the results.
* It once wrapped wherever it ran out of room, and the List/Map switch only
* appears when there are results — so the same search took two lines with
* results and one without.
*/
test('the results toolbar keeps its line count whether or not there are results', async ({ page }) => {
const withResults = '/?postcode=B1%201BB&radius=1';
// No school type matches this, so the same search returns nothing.
const without = `${withResults}&school_type=no-such-type`;
const lines = async (url: string) => {
await page.goto(url);
// By label: the input is a combobox when autosuggest is on.
const input = page.getByLabel('School name or postcode', { exact: true });
const filters = page.getByRole('group', { name: 'Filters' });
await expect(filters).toBeVisible({ timeout: 15_000 });
const a = (await input.boundingBox())!;
const b = (await filters.boundingBox())!;
return b.y >= a.y + a.height ? 2 : 1;
};
const view = page.getByRole('group', { name: 'Results view' });
for (const [width, expected] of [[1400, 1], [1100, 2]] as const) {
await page.setViewportSize({ width, height: 800 });
expect(await lines(withResults), `${width}px with results`).toBe(expected);
await expect(view).toBeVisible();
expect(await lines(without), `${width}px without results`).toBe(expected);
await expect(view).toHaveCount(0);
}
});
/*
* Desktop opens a postcode search on the map (mockup B): the list in a pane on
* the left, the map filling the rest of the screen, and a card on the map for
@@ -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');
});
});
@@ -18,7 +18,10 @@ jest.mock('@/lib/api', () => ({
fetchNationalAverages: jest.fn(async () => ({})),
fetchLAaverages: jest.fn(async () => ({ secondary: { attainment_8_by_la: {} } })),
}));
jest.mock('@/components/FilterBar', () => ({ FilterBar: () => null }));
// Renders only the List/Map switch HomeView hands it, which lives in its row.
jest.mock('@/components/FilterBar', () => ({
FilterBar: ({ viewSwitch }: { viewSwitch?: unknown }) => viewSwitch || null,
}));
jest.mock('@/components/SchoolRow', () => ({ SchoolRow: ({ school }: {school: School}) => <div>{school.school_name}</div> }));
jest.mock('@/components/SchoolMap', () => ({ SchoolMap: ({ schools }: {schools: School[]}) => <div data-testid="map">{schools.map(s => s.school_name).join(',')}</div> }));
@@ -24,7 +24,10 @@ jest.mock('@/lib/api', () => ({
fetchNationalAverages: jest.fn(),
fetchLAaverages: jest.fn(async () => ({ secondary: { attainment_8_by_la: {} } })),
}));
jest.mock('@/components/FilterBar', () => ({ FilterBar: () => null }));
// Renders only the List/Map switch HomeView hands it, which lives in its row.
jest.mock('@/components/FilterBar', () => ({
FilterBar: ({ viewSwitch }: { viewSwitch?: unknown }) => viewSwitch || null,
}));
jest.mock('@/components/SchoolMap', () => ({
SchoolMap: ({ selectedUrn, radiusMiles, onMarkerClick, schools }: {
selectedUrn: number | null; radiusMiles?: number;
+48 -5
View File
@@ -40,8 +40,19 @@
margin: 0 auto 1.5rem;
}
/* One row where it fits: the search takes what the controls leave, and the
"More filters" panel breaks onto its own line below both. */
/*
* One row on wide screens, two below 1340px — decided by the width alone,
* never by what the search returned.
*
* The row once wrapped wherever it ran out of room, and its contents change
* with the results: the List/Map switch beside it, the distance chip and Clear
* all come and go. So the same search folded onto two lines when it had
* results and sat on one when it had none. Now the controls never wrap away
* from the search on a wide screen; the search box takes what they leave, and
* 1340px is where the fullest toolbar (distance, phase, type, More filters,
* Clear and the switch) still leaves it 12rem. The "More filters" panel breaks
* onto its own line below both.
*/
.filterBar:not(.heroMode) {
display: flex;
flex-wrap: wrap;
@@ -56,10 +67,34 @@
}
.filterBar:not(.heroMode) .searchSection {
flex: 1 1 320px;
flex: 1 1 0;
min-width: 0;
}
.filterBar:not(.heroMode) .controlsRow {
flex: 0 0 auto;
flex-wrap: nowrap;
}
/* The List/Map switch closes the line; the "More filters" panel follows it. */
.viewSwitchSlot {
flex: 0 0 auto;
order: 1;
}
/* Below that, the switch stays up beside the search and the controls always
take a full-width line of their own, results or not. */
@media (min-width: 641px) and (max-width: 1339px) {
.viewSwitchSlot {
order: 0;
}
.filterBar:not(.heroMode) .controlsRow {
flex-basis: 100%;
flex-wrap: wrap;
}
}
/* Only phones fold the form away; see the 640px block. */
.searchSummary {
display: none;
@@ -324,8 +359,9 @@
font-weight: 500;
white-space: nowrap;
/* A select is as wide as its longest option, and a school type can run to
"Academy special sponsor led". Cap it; the chosen value truncates. */
max-width: 14rem;
"Academy special sponsor led". Cap it; the chosen value truncates. The cap
is part of the one-line budget above. */
max-width: 11rem;
text-overflow: ellipsis;
}
@@ -363,6 +399,7 @@
pushing the results off a short screen. The 3px gutter keeps the selects'
focus rings clear of the scroll clip. */
.filters {
order: 2;
flex-basis: 100%;
display: flex;
gap: 0.625rem;
@@ -579,6 +616,12 @@
display: none;
}
/* Phones switch views with the floating button (HomeView, .mobileDock). An
empty slot would still take a gap in this column. */
.viewSwitchSlot {
display: none;
}
/* Bleeds to the screen edge so a chip scrolls out from under it, rather than
being cut off at the toolbar's padding. The toolbar's inline padding is
1rem at this width (HomeView.module.css, .resultsToolbar). The 4px of
+14 -1
View File
@@ -22,6 +22,12 @@ interface FilterBarProps {
geoError?: string | null;
/** Server-read feature flag. Off means no listener, no fetch, no markup. */
autosuggest?: boolean;
/**
* The results page's List/Map switch. It sits in this bar's own row rather
* than beside it, so that when the bar takes two lines the filters' line
* runs the full width instead of stopping short of the switch.
*/
viewSwitch?: ReactNode;
}
/**
@@ -54,6 +60,7 @@ export function FilterBar({
geoState = "idle",
geoError,
autosuggest = false,
viewSwitch,
}: FilterBarProps) {
const router = useRouter();
const pathname = usePathname();
@@ -295,7 +302,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 ?? [];
@@ -449,6 +459,9 @@ export function FilterBar({
{!isHero && (
<>
{viewSwitch && (
<div className={styles.viewSwitchSlot}>{viewSwitch}</div>
)}
{/* Every control here is a real <select> or <button>, drawn as a
pill. On phones the row scrolls sideways rather than wrapping, so
the pinned toolbar stays two lines tall. */}
+22 -22
View File
@@ -713,29 +713,29 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed
geoState={geoState}
geoError={geoError}
autosuggest={autosuggest}
viewSwitch={hasViewSwitch && (
<div className={styles.viewSwitch} role="group" aria-label="Results view">
<button
type="button"
className={styles.viewSwitchBtn}
aria-pressed={resultsView === 'list'}
onClick={() => changeView('list', 'toolbar')}
>
<ListIcon />
List
</button>
<button
type="button"
className={styles.viewSwitchBtn}
aria-pressed={resultsView === 'map'}
onClick={() => changeView('map', 'toolbar')}
>
<MapIcon />
Map
</button>
</div>
)}
/>
{hasViewSwitch && (
<div className={styles.viewSwitch} role="group" aria-label="Results view">
<button
type="button"
className={styles.viewSwitchBtn}
aria-pressed={resultsView === 'list'}
onClick={() => changeView('list', 'toolbar')}
>
<ListIcon />
List
</button>
<button
type="button"
className={styles.viewSwitchBtn}
aria-pressed={resultsView === 'map'}
onClick={() => changeView('map', 'toolbar')}
>
<MapIcon />
Map
</button>
</div>
)}
</div>
)}