From 59ea8a4bdd550dc62bd325859048e275cb79d41a Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 2 Oct 2026 23:20:14 +0100 Subject: [PATCH] fix(search): stop replaying a failed LA-averages request forever The search page fetched LA averages with cache: 'force-cache', which serves any stored response, however old, without asking the server. One failed request (a staging deploy restart; the July proxy outage) was stored and replayed on every later visit, and the error was swallowed, so the "vs LA avg" delta silently vanished from every secondary row in that browser. A Playwright profile still held a 500 dated 5 July. The default cache mode honours the API's Cache-Control (five minutes), so a good answer is still reused and an error never is. Browsers holding a stored failure recover on their next visit. A journey now checks that a mainstream secondary's row shows the comparison: nothing did, which is how it could go missing unnoticed. Co-Authored-By: Claude Opus 5.5 --- e2e/tests/journeys.spec.ts | 23 +++++++++++++++++++ .../components/HomeView.staleFetch.test.tsx | 22 +++++++++++++++++- nextjs-app/components/HomeView.tsx | 8 +++++-- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index cbe4360..9d18903 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -410,6 +410,29 @@ test('search and the school page agree on how many pupils a secondary has', asyn expect(school.total_pupils).toBe(detail.school_info.total_pupils); }); +test('a secondary search row compares its Attainment 8 with the LA average', async ({ page }) => { + // The comparison vanished unnoticed: the averages were fetched with + // force-cache, so one stored failure hid it in that browser for good. + // Playwright disables the HTTP cache when it intercepts requests, so this + // guards the comparison itself; the unit test pins the cache mode. + const la = await (await page.request.get('/api/la-averages')).json(); + const averages: Record = la.secondary?.attainment_8_by_la ?? {}; + const res = await page.request.get('/api/schools?search=school&phase=secondary&page_size=50'); + expect(res.ok()).toBeTruthy(); + const school = ((await res.json()).schools ?? []).find( + (s: { attainment_8_score?: number | null; local_authority?: string; school_type?: string }) => + s.attainment_8_score != null && s.local_authority != null && averages[s.local_authority] != null + && !/special|pupil referral|alternative provision/i.test(s.school_type ?? '')); + test.skip(!school, 'no mainstream secondary with an LA average here'); + + await searchByName(page, school.school_name); + const link = page.locator(`a[href^="/school/${school.urn}-"]`).first(); + await expect(link).toBeVisible({ timeout: 15_000 }); + const stats = link.locator('xpath=ancestor::div[contains(@class, "__rowContent")][1]') + .locator('[class*="__line3"]'); + await expect(stats.getByText(/vs LA avg/)).toBeVisible(); +}); + 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. diff --git a/nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx b/nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx index b8d788a..8725831 100644 --- a/nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx +++ b/nextjs-app/__tests__/components/HomeView.staleFetch.test.tsx @@ -1,6 +1,6 @@ import { act, fireEvent, render, screen } from '@testing-library/react'; import { HomeView } from '@/components/HomeView'; -import { fetchSchools } from '@/lib/api'; +import { fetchLAaverages, fetchSchools } from '@/lib/api'; import { primaryFixture } from '../support/schoolFixtures'; import type { SchoolsResponse, School } from '@/lib/types'; @@ -84,3 +84,23 @@ test('failed map requests can be retried by reopening the map', async () => { expect(fetchSchools).toHaveBeenCalledTimes(2); expect(screen.getByTestId('map')).toHaveTextContent('Retry result'); }); + +test('LA averages are not fetched with force-cache, so one failure is not replayed for good', async () => { + // force-cache serves any stored response, however old, without asking the + // server. A request that failed once (a staging deploy restart, the July + // proxy outage) was stored and replayed on every later visit, and the + // "vs LA avg" delta vanished from every secondary row in that browser. + // The default mode honours the API's Cache-Control and never reuses an + // error. + params = new URLSearchParams('search=high'); + const secondary: SchoolsResponse = { + ...response('Alpha High'), + schools: [{ ...primaryFixture.schoolInfo, school_name: 'Alpha High', phase: 'Secondary', attainment_8_score: 50 }], + }; + render(); + await act(async () => {}); + expect(fetchLAaverages).toHaveBeenCalled(); + for (const [options] of jest.mocked(fetchLAaverages).mock.calls) { + expect(options?.cache).not.toBe('force-cache'); + } +}); diff --git a/nextjs-app/components/HomeView.tsx b/nextjs-app/components/HomeView.tsx index 2f46f8a..363d9e1 100644 --- a/nextjs-app/components/HomeView.tsx +++ b/nextjs-app/components/HomeView.tsx @@ -358,10 +358,14 @@ export function HomeView({ initialSchools, filters, totalSchools, howItWorks, ed return () => controller.abort(); }, [resultsView, searchParams, initialSchools.schools]); - // Fetch LA averages when secondary or mixed schools are visible + // Fetch LA averages when secondary or mixed schools are visible. Default + // cache mode, never force-cache: force-cache replays any stored response + // without asking the server, so one failed request (a deploy restart) hid + // every "vs LA avg" delta in that browser for good. The API's Cache-Control + // already lets the browser reuse a good answer for five minutes. useEffect(() => { if (!isSecondaryView && !isMixedView) return; - fetchLAaverages({ cache: 'force-cache' }) + fetchLAaverages() .then(data => setLaAverages(data.secondary.attainment_8_by_la)) .catch(() => {}); }, [isSecondaryView, isMixedView]);