fix(compare): stale-basket fetch blanking comparisons; rail caption #52
@@ -204,6 +204,8 @@ test('comparing two schools shows the parent-first sections side by side', async
|
||||
.locator('[aria-label="Schools in this comparison"]')
|
||||
.evaluate((el) => getComputedStyle(el).gridTemplateColumns);
|
||||
expect(barTemplate).toMatch(/^200px /);
|
||||
// ...and its label rail carries the comparison caption.
|
||||
await expect(page.getByText(/^\d+ (primary|secondary) schools?$/)).toBeVisible();
|
||||
|
||||
// Ofsted linkout goes to the school's provider page, never a report deep-link
|
||||
const ofstedLink = page.getByRole('link', { name: /Ofsted page/i }).first();
|
||||
@@ -237,6 +239,31 @@ test('comparing two secondary schools renders the secondary sections', async ({
|
||||
await expect(page.getByText(/No primary schools in your comparison/)).toHaveCount(0);
|
||||
});
|
||||
|
||||
test('opening a different compare link after a previous comparison still renders', async ({ page }) => {
|
||||
// Regression: the first visit stores a basket in localStorage; opening a
|
||||
// link for a DIFFERENT school set then raced a stale fetch for the stored
|
||||
// basket against the new SSR data, blanking every section (including the
|
||||
// trends chart) until a hard refresh.
|
||||
const [s0, s1] = await twoSecondaryUrns(page);
|
||||
const [p0, p1] = await twoPrimaryUrns(page);
|
||||
|
||||
await page.goto(`/compare?urns=${s0},${s1}`);
|
||||
await expect(page.getByRole('heading', { name: 'At a glance' }).first()).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
|
||||
await page.goto(`/compare?urns=${p0},${p1}`);
|
||||
await expect(page.getByRole('heading', { name: 'At a glance' }).first()).toBeVisible({
|
||||
timeout: 15_000,
|
||||
});
|
||||
// Give any straggling stale response time to land, then confirm the new
|
||||
// comparison is still on screen.
|
||||
await page.waitForTimeout(1500);
|
||||
await expect(page.getByRole('heading', { name: 'At a glance' }).first()).toBeVisible();
|
||||
await expect(page.getByRole('heading', { name: 'Explore trends' }).first()).toBeVisible();
|
||||
await expect(page.locator(`a[href*="${p0}"]`).first()).toBeVisible();
|
||||
});
|
||||
|
||||
test('compare chart on mobile shows school chips with tap-to-focus', async ({ page }) => {
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
|
||||
|
||||
@@ -72,5 +72,7 @@ test('an all-secondary comparison renders the sections, not an empty primary tab
|
||||
});
|
||||
expect(screen.getAllByText('Gamma High').length).toBeGreaterThan(0);
|
||||
expect(screen.queryByText(/No primary schools in your comparison/)).toBeNull();
|
||||
// The sticky bar's rail caption reflects the active phase and count.
|
||||
expect(screen.getByText('2 secondary schools')).toBeInTheDocument();
|
||||
expect(fetchComparison).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
/**
|
||||
* Regression: opening a compare link while a DIFFERENT basket is stored must
|
||||
* not blank the page.
|
||||
*
|
||||
* The basket hydrates from localStorage first, which can fire a fetch for the
|
||||
* OLD school set; the URL-seed effect then replaces the basket with the URL's
|
||||
* schools (already covered by SSR data, so no new fetch). When the stale
|
||||
* response for the old set finally lands, it must not clobber the fresh SSR
|
||||
* data — that left every section (including the trends chart) empty until a
|
||||
* hard refresh.
|
||||
*/
|
||||
|
||||
import { act, render, screen, waitFor } from '@testing-library/react';
|
||||
|
||||
import { ComparisonView } from '@/components/ComparisonView';
|
||||
import { ComparisonProvider } from '@/context/ComparisonProvider';
|
||||
import type { ComparisonData, School } from '@/lib/types';
|
||||
|
||||
const fetchComparison = jest.fn();
|
||||
jest.mock('@/lib/api', () => ({
|
||||
fetchComparison: (...args: unknown[]) => fetchComparison(...args),
|
||||
}));
|
||||
jest.mock('@/lib/analytics', () => ({ track: jest.fn() }));
|
||||
|
||||
function school(urn: number, name: string): School {
|
||||
return {
|
||||
urn,
|
||||
school_name: name,
|
||||
local_authority: 'Testshire',
|
||||
school_type: 'Community school',
|
||||
rwm_expected_pct: 80,
|
||||
phase: 'Primary',
|
||||
} as School;
|
||||
}
|
||||
|
||||
function data(urn: number, name: string): ComparisonData {
|
||||
return {
|
||||
school_info: school(urn, name),
|
||||
yearly_data: [{ year: 202425, rwm_expected_pct: 80 }] as ComparisonData['yearly_data'],
|
||||
ofsted: null,
|
||||
census: null,
|
||||
admissions: null,
|
||||
admissions_history: [],
|
||||
deprivation: null,
|
||||
};
|
||||
}
|
||||
|
||||
// The visitor's previously stored basket (a different school entirely).
|
||||
const STORED_SCHOOL = school(900, 'Old Stored School');
|
||||
|
||||
// The comparison the URL (and SSR) actually asked for.
|
||||
const URL_DATA = {
|
||||
'100': data(100, 'Alpha Primary'),
|
||||
'200': data(200, 'Beta Primary'),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
fetchComparison.mockReset();
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
test('a stale fetch for the previously stored basket does not clobber the URL comparison', async () => {
|
||||
localStorage.setItem('selectedSchools', JSON.stringify([STORED_SCHOOL]));
|
||||
|
||||
const pending: Array<(v: unknown) => void> = [];
|
||||
fetchComparison.mockImplementation(() => new Promise((resolve) => pending.push(resolve)));
|
||||
|
||||
render(
|
||||
<ComparisonProvider>
|
||||
<ComparisonView
|
||||
initialData={URL_DATA}
|
||||
initialNationalAverages={{
|
||||
year: 202425,
|
||||
primary: { rwm_expected_pct: 62 },
|
||||
secondary: {},
|
||||
by_year: [],
|
||||
}}
|
||||
initialBenchmarks={undefined}
|
||||
initialUrns={[100, 200]}
|
||||
metrics={[]}
|
||||
selectedMetric="rwm_expected_pct"
|
||||
/>
|
||||
</ComparisonProvider>,
|
||||
);
|
||||
|
||||
// The URL's schools render from SSR data once the basket is reseeded.
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('heading', { name: 'At a glance' })).toBeInTheDocument();
|
||||
});
|
||||
expect(screen.getAllByText('Alpha Primary').length).toBeGreaterThan(0);
|
||||
|
||||
// The transient stored-basket fetch (for school 900) resolves LATE, after
|
||||
// the basket has moved on to the URL's schools.
|
||||
await act(async () => {
|
||||
for (const resolve of pending) {
|
||||
resolve({
|
||||
comparison: { '900': data(900, 'Old Stored School') },
|
||||
national_averages: { year: 202425, primary: {}, secondary: {}, by_year: [] },
|
||||
benchmarks: undefined,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// The page must still show the URL comparison — not go blank.
|
||||
expect(screen.getByRole('heading', { name: 'At a glance' })).toBeInTheDocument();
|
||||
expect(screen.getAllByText('Alpha Primary').length).toBeGreaterThan(0);
|
||||
});
|
||||
@@ -148,10 +148,16 @@
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
/* Caption filling the label rail on desktop ("Comparing / 3 primary
|
||||
schools"). Hidden on mobile, where the bar is a row of compact pills. */
|
||||
.barCaption {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* Desktop (matches the sections' 761px breakpoint): the bar adopts the same
|
||||
grid template as compareSections' .grid — a 200px row-label rail plus one
|
||||
column per school — so each chip sits exactly over the column it labels.
|
||||
The first chip starts after the empty label rail. */
|
||||
The caption occupies the rail; chips flow into the school columns. */
|
||||
@media (min-width: 761px) {
|
||||
.schoolBar {
|
||||
display: grid;
|
||||
@@ -164,8 +170,29 @@
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.schoolChip:first-child {
|
||||
grid-column: 2;
|
||||
.barCaption {
|
||||
grid-column: 1;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
justify-content: center;
|
||||
gap: 0.1rem;
|
||||
padding-right: 0.5rem;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.barCaptionEyebrow {
|
||||
font-size: 0.72rem;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
color: var(--text-muted, #6d685f);
|
||||
}
|
||||
|
||||
.barCaptionCount {
|
||||
font-size: 0.95rem;
|
||||
font-weight: 600;
|
||||
line-height: 1.3;
|
||||
color: var(--text-primary, #1a1612);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -85,7 +85,9 @@ export function ComparisonView({
|
||||
replaceSchools(urlSchools);
|
||||
}
|
||||
}
|
||||
}, [isInitialized]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
// Re-seed when a client-side navigation lands on a different ?urns= set
|
||||
// (initialUrns/initialData are new props on the same component instance).
|
||||
}, [isInitialized, initialUrns.join(',')]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
|
||||
const urnKey = selectedSchools.map((s) => s.urn).join(',');
|
||||
|
||||
@@ -128,8 +130,18 @@ export function ComparisonView({
|
||||
const covered = urnKey.split(',').every((urn) => have[urn] != null);
|
||||
if (covered) return;
|
||||
|
||||
// Guard against out-of-order responses: while the basket hydrates from
|
||||
// localStorage it can transiently hold a DIFFERENT school set than the
|
||||
// URL, firing a fetch for schools the user is no longer comparing. That
|
||||
// stale response must not replace data for the current set — it blanked
|
||||
// every section until a hard refresh. Cleanup marks the run cancelled
|
||||
// when urnKey moves on, so only the current selection's response is
|
||||
// applied (replacing the map keeps it bounded and guarantees a re-added
|
||||
// school is refetched fresh rather than served a lingering old entry).
|
||||
let cancelled = false;
|
||||
fetchComparison(urnKey, { cache: 'no-store' })
|
||||
.then((data) => {
|
||||
if (cancelled) return;
|
||||
setComparisonData(data.comparison);
|
||||
setNationalAverages(data.national_averages);
|
||||
setBenchmarks(data.benchmarks);
|
||||
@@ -140,6 +152,9 @@ export function ComparisonView({
|
||||
// destroy a working comparison the user is looking at.
|
||||
console.error('Failed to fetch comparison:', err);
|
||||
});
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [urnKey, isInitialized]);
|
||||
|
||||
const primarySchools = selectedSchools.filter((school) => {
|
||||
@@ -350,6 +365,14 @@ export function ComparisonView({
|
||||
style={{ '--school-count': activeSchools.length } as CSSProperties}
|
||||
aria-label="Schools in this comparison"
|
||||
>
|
||||
{/* Fills the 200px label rail on desktop (hidden on mobile). */}
|
||||
<div className={styles.barCaption}>
|
||||
<span className={styles.barCaptionEyebrow}>Comparing</span>
|
||||
<span className={styles.barCaptionCount}>
|
||||
{activeSchools.length} {comparePhase} school
|
||||
{activeSchools.length === 1 ? '' : 's'}
|
||||
</span>
|
||||
</div>
|
||||
{activeSchools.map((school, index) => (
|
||||
<div
|
||||
key={school.urn}
|
||||
|
||||
Reference in New Issue
Block a user