Merge pull request 'fix(search): offer every filter option, not only those in the results' (#169) from fix/filter-options-not-from-results into main

Reviewed-on: #169

docs/LEGACY_CODE.md

@@ -44,6 +44,7 @@ Their previous implementations remain recoverable from Git history.
 | `nextjs-app/components/SchoolCard.tsx` and its CSS | Imported by its own tests, not application code. HomeView uses SchoolRow/SecondarySchoolRow. | Decide whether to retire the card design; if removed, remove its dedicated tests as well. Passing tests do not establish runtime use. |
 | `backend/database.py: get_db`, `get_db_session` | No remaining callers after removing the importer. Current code creates SessionLocal directly. | Either adopt these helpers during session-lifecycle cleanup or remove them; do not rewrite active sessions in a documentation change. |
 | `backend/schemas.py: COLUMN_MAPPINGS`, `NULL_VALUES`, `LA_CODE_TO_NAME` | No remaining Python consumers found after importer removal. Other constants in this module are active. | Remove individual constants after checking external data utilities; retain the module. |
+| `backend/app.py: result_filters` keys `school_types`, `phases`, `genders`, `admissions_policies` | Since 2026-10-02 FilterBar offers these from `/api/filters`, because options scoped to the results left only the chosen value on offer. Only `local_authorities` is still read. | Stop computing the four keys in a focused API change; keep `local_authorities`. |
 | `backend/config.py: data_dir`, `max_page_size`, `rate_limit_burst` | No active consumers found. `default_page_size` appears only in a branch that expects None, although the route supplies a concrete default. | Reconcile settings with route validation in a focused API change. |
 
 ## Legacy/manual paths requiring operational verification

e2e/tests/journeys.spec.ts

@@ -286,6 +286,25 @@ test('the phase filter switches straight from secondary to primary', async ({ pa
   await expect(phase).toHaveValue('primary');
 });
 
+test('school type and gender switch straight to another value', async ({ page }) => {
+  // Their options came from the result set, which the filter had already
+  // narrowed, so with one value chosen it was the only one on offer.
+  await page.goto('/?search=school&gender=girls');
+  // An applied gender filter opens More filters by itself.
+  const gender = page.getByRole('combobox', { name: 'Gender', exact: true });
+  await expect(gender).toHaveValue('girls', { timeout: 15_000 });
+  await gender.selectOption('boys');
+  await expect(page).toHaveURL(/[?&]gender=boys(&|$)/);
+
+  // Whatever types the data holds, choosing one leaves the same list on offer.
+  const type = page.getByRole('combobox', { name: 'School type', exact: true });
+  const offered = await type.locator('option').allTextContents();
+  expect(offered.length, 'no school type to choose').toBeGreaterThan(1);
+  await type.selectOption(offered[1]);
+  await expect(page).toHaveURL(/[?&]school_type=/);
+  await expect.poll(() => type.locator('option').allTextContents()).toEqual(offered);
+});
+
 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.

nextjs-app/__tests__/components/FilterBarOptions.test.tsx

@@ -0,0 +1,125 @@
+import { fireEvent, render, screen, within } from '@testing-library/react';
+import { FilterBar } from '@/components/FilterBar';
+import type { ResultFilters } from '@/lib/types';
+
+/*
+ * A filter's options must not come from the results it is filtering, or
+ * choosing one leaves only that one on offer: pick "Girls" and "Boys" is gone
+ * until the filter is cleared. School type, gender and admissions offer the
+ * full lists, as phase already did. Local authority stays scoped to the
+ * results, so a postcode search offers the councils nearby rather than 153.
+ *
+ * Gender, sixth form and admissions show unless the phase chosen is a primary
+ * one, so what the results happen to contain never decides which filters
+ * there are.
+ */
+
+let params = new URLSearchParams('postcode=SW196AR&radius=1');
+const push = jest.fn();
+jest.mock('next/navigation', () => ({
+  useSearchParams: () => params,
+  usePathname: () => '/',
+  useRouter: () => ({ push, replace: jest.fn(), prefetch: jest.fn() }),
+}));
+jest.mock('@/lib/analytics', () => ({ track: jest.fn() }));
+
+const filters = {
+  local_authorities: ['Merton', 'Wandsworth'],
+  school_types: ['Academy converter', 'Community school'], years: [],
+  phases: ['Middle deemed primary', 'Nursery', 'Primary', 'Secondary', 'All-through'],
+  genders: ['Boys', 'Girls', 'Mixed'],
+  admissions_policies: ['Non-selective', 'Selective'],
+};
+
+// What the results came back with once narrowed by the chosen filters.
+const narrowed: ResultFilters = {
+  local_authorities: ['Wandsworth'], school_types: ['Community school'],
+  phases: ['Secondary'], genders: ['Girls'], admissions_policies: ['Non-selective'],
+};
+
+const pushedParams = () => new URLSearchParams(push.mock.calls.at(-1)![0].split('?')[1]);
+
+const openSheet = () => {
+  fireEvent.click(screen.getByRole('button', { name: /^Filters/ }));
+  return screen.getByRole('dialog', { name: 'Filters' });
+};
+
+const optionsOf = (sheet: HTMLElement, name: string) =>
+  within(within(sheet).getByRole('combobox', { name }))
+    .getAllByRole('option').map((o) => o.textContent);
+
+beforeEach(() => {
+  params = new URLSearchParams('postcode=SW196AR&radius=1');
+  push.mockClear();
+});
+
+describe('filter options', () => {
+  it('offer every school type, gender and admissions policy, whatever the results hold', () => {
+    params = new URLSearchParams('postcode=SW196AR&radius=1&school_type=Community+school&gender=girls');
+    render(<FilterBar filters={filters} resultFilters={narrowed} />);
+    const sheet = openSheet();
+    expect(optionsOf(sheet, 'School type')).toEqual(['Any school type', 'Academy converter', 'Community school']);
+    expect(optionsOf(sheet, 'Gender')).toEqual(['Boys, Girls & Mixed', 'Boys', 'Girls', 'Mixed']);
+    expect(optionsOf(sheet, 'Admissions')).toEqual(['All admissions types', 'Non-selective', 'Selective']);
+  });
+
+  it('keep local authority to the councils in the results', () => {
+    render(<FilterBar filters={filters} resultFilters={narrowed} />);
+    expect(optionsOf(openSheet(), 'Local authority')).toEqual(['All Local Authorities', 'Wandsworth']);
+  });
+
+  it('offer the full school types in the desktop row too', () => {
+    render(<FilterBar filters={filters} resultFilters={narrowed} />);
+    const row = screen.getByRole('group', { name: 'Filters' });
+    expect(within(within(row).getByRole('combobox', { name: 'School type' }))
+      .getAllByRole('option')).toHaveLength(3);
+  });
+});
+
+describe('the secondary-only filters', () => {
+  const secondaryOnly = ['Gender', 'Sixth form', 'Admissions'];
+  const shown = (sheet: HTMLElement) =>
+    secondaryOnly.filter((name) => within(sheet).queryByRole('combobox', { name }));
+
+  it('show with any phase, even when no secondary school is in the results', () => {
+    const primariesOnly = { ...narrowed, phases: ['Primary'], genders: [], admissions_policies: [] };
+    render(<FilterBar filters={filters} resultFilters={primariesOnly} />);
+    expect(shown(openSheet())).toEqual(secondaryOnly);
+  });
+
+  it.each(['primary', 'nursery', 'middle deemed primary', 'Middle-deemed  Primary'])(
+    'hide for the %s phase', (phase) => {
+    params = new URLSearchParams(`postcode=SW196AR&radius=1&phase=${encodeURIComponent(phase)}`);
+    render(<FilterBar filters={filters} />);
+    expect(shown(openSheet())).toEqual([]);
+  });
+
+  it('leave out a filter with no options, rather than show only its "any"', () => {
+    render(<FilterBar filters={{ ...filters, genders: [], admissions_policies: [] }} />);
+    expect(shown(openSheet())).toEqual(['Sixth form']);
+  });
+
+  it.each(['secondary', 'all-through', 'middle deemed secondary', '16 plus'])('show for the %s phase', (phase) => {
+    params = new URLSearchParams(`postcode=SW196AR&radius=1&phase=${phase}`);
+    render(<FilterBar filters={filters} />);
+    expect(shown(openSheet())).toEqual(secondaryOnly);
+  });
+
+  it('are cleared by choosing a primary phase, rather than left applied and hidden', () => {
+    params = new URLSearchParams(
+      'postcode=SW196AR&radius=1&phase=secondary&gender=girls&has_sixth_form=yes&admissions_policy=selective&school_type=Community+school');
+    render(<FilterBar filters={filters} />);
+    fireEvent.change(within(openSheet()).getByRole('combobox', { name: 'Phase' }), { target: { value: 'primary' } });
+    const next = pushedParams();
+    expect(next.get('phase')).toBe('primary');
+    for (const key of ['gender', 'has_sixth_form', 'admissions_policy']) expect(next.get(key)).toBeNull();
+    expect(next.get('school_type')).toBe('Community school');
+  });
+
+  it('are kept when the new phase still has them', () => {
+    params = new URLSearchParams('postcode=SW196AR&radius=1&phase=secondary&gender=girls');
+    render(<FilterBar filters={filters} />);
+    fireEvent.change(within(openSheet()).getByRole('combobox', { name: 'Phase' }), { target: { value: 'all-through' } });
+    expect(pushedParams().get('gender')).toBe('girls');
+  });
+});

nextjs-app/components/FilterBar.tsx

@@ -66,6 +66,23 @@ const FILTER_KEYS = [
 ] as const;
 type FilterKey = (typeof FILTER_KEYS)[number];
 
+/** Filters that only mean something for schools teaching beyond primary. */
+const SECONDARY_ONLY_KEYS = ["gender", "has_sixth_form", "admissions_policy"] as const;
+
+/**
+ * False for a phase with no secondary-age pupils, for which those filters are
+ * hidden: Primary, Nursery and Middle deemed primary. Matched on the words,
+ * not exact labels, so a change of case, hyphen or spacing in the GIAS label
+ * cannot leave them showing. The same reading of "primary" as
+ * compareGroups in lib/compareLogic.ts. The API's PHASE_GROUPS answers a
+ * different question (which phases a phase filter returns: primary includes
+ * all-through), so it is not this list.
+ */
+function hasSecondaryFilters(phase: string): boolean {
+  const p = phase.toLowerCase().replace(/[^a-z]+/g, " ").trim();
+  return !(p === "nursery" || (p.includes("primary") && !p.includes("secondary")));
+}
+
 function SlidersIcon() {
   return (
     <svg
@@ -326,7 +343,13 @@ export function FilterBar({
   };
 
   const handleFilterChange = (key: string, value: string) => {
-    updateURL({ [key]: value });
+    // A primary phase hides the secondary-only filters, so it clears them
+    // too: left applied, they would empty the list with no control showing.
+    const cleared =
+      key === "phase" && !hasSecondaryFilters(value)
+        ? Object.fromEntries(SECONDARY_ONLY_KEYS.map((k) => [k, ""]))
+        : {};
+    updateURL({ ...cleared, [key]: value });
   };
 
   // Every filter at once, keeping the search and its distance: what "Clear
@@ -353,20 +376,24 @@ export function FilterBar({
     currentAdmissionsPolicy ||
     currentHasSixthForm;
 
-  // Use result-scoped filter values when available, fall back to global
+  /*
+   * A filter's options come from the full lists, not from the results: the
+   * results have already been narrowed by that filter, so scoping to them
+   * would leave only the chosen value on offer, and switching (Girls to Boys,
+   * one school type to another) would need clearing first.
+   *
+   * Local authority is the exception, scoped to the results so a postcode
+   * search offers the councils nearby rather than all of England's.
+   */
   const laOptions =
     resultFilters?.local_authorities ?? filters.local_authorities;
-  const typeOptions = resultFilters?.school_types ?? filters.school_types;
+  const typeOptions = filters.school_types;
-  // 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 genderOptions = filters.genders ?? [];
-  const admissionsPolicyOptions =
+  const admissionsPolicyOptions = filters.admissions_policies ?? [];
-    resultFilters?.admissions_policies ?? filters.admissions_policies ?? [];
 
-  const isSecondaryMode =
+  // Set by the phase chosen, never by what the results happen to contain.
-    currentPhase === "secondary" || genderOptions.length > 0;
+  const isSecondaryMode = hasSecondaryFilters(currentPhase);
 
   // A select that is narrowing the results carries a sage tint; the class is
   // only ever additive, so the control's behaviour is untouched.
@@ -409,11 +436,8 @@ export function FilterBar({
     // rest are the names themselves.
     const named: Partial<Record<FilterKey, string[]>> = {
       phase: phaseOptions,
-      gender: [...genderOptions, ...(filters.genders ?? [])],
+      gender: genderOptions,
-      admissions_policy: [
+      admissions_policy: admissionsPolicyOptions,
-        ...admissionsPolicyOptions,
-        ...(filters.admissions_policies ?? []),
-      ],
     };
     return (named[key] ?? []).find((o) => o.toLowerCase() === value.toLowerCase()) ?? value;
   };