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"]')
|
.locator('[aria-label="Schools in this comparison"]')
|
||||||
.evaluate((el) => getComputedStyle(el).gridTemplateColumns);
|
.evaluate((el) => getComputedStyle(el).gridTemplateColumns);
|
||||||
expect(barTemplate).toMatch(/^200px /);
|
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
|
// Ofsted linkout goes to the school's provider page, never a report deep-link
|
||||||
const ofstedLink = page.getByRole('link', { name: /Ofsted page/i }).first();
|
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);
|
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 }) => {
|
test('compare chart on mobile shows school chips with tap-to-focus', async ({ page }) => {
|
||||||
await page.setViewportSize({ width: 390, height: 844 });
|
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.getAllByText('Gamma High').length).toBeGreaterThan(0);
|
||||||
expect(screen.queryByText(/No primary schools in your comparison/)).toBeNull();
|
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();
|
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;
|
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
|
/* Desktop (matches the sections' 761px breakpoint): the bar adopts the same
|
||||||
grid template as compareSections' .grid — a 200px row-label rail plus one
|
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.
|
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) {
|
@media (min-width: 761px) {
|
||||||
.schoolBar {
|
.schoolBar {
|
||||||
display: grid;
|
display: grid;
|
||||||
@@ -164,8 +170,29 @@
|
|||||||
min-width: 0;
|
min-width: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
.schoolChip:first-child {
|
.barCaption {
|
||||||
grid-column: 2;
|
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);
|
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(',');
|
const urnKey = selectedSchools.map((s) => s.urn).join(',');
|
||||||
|
|
||||||
@@ -128,8 +130,18 @@ export function ComparisonView({
|
|||||||
const covered = urnKey.split(',').every((urn) => have[urn] != null);
|
const covered = urnKey.split(',').every((urn) => have[urn] != null);
|
||||||
if (covered) return;
|
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' })
|
fetchComparison(urnKey, { cache: 'no-store' })
|
||||||
.then((data) => {
|
.then((data) => {
|
||||||
|
if (cancelled) return;
|
||||||
setComparisonData(data.comparison);
|
setComparisonData(data.comparison);
|
||||||
setNationalAverages(data.national_averages);
|
setNationalAverages(data.national_averages);
|
||||||
setBenchmarks(data.benchmarks);
|
setBenchmarks(data.benchmarks);
|
||||||
@@ -140,6 +152,9 @@ export function ComparisonView({
|
|||||||
// destroy a working comparison the user is looking at.
|
// destroy a working comparison the user is looking at.
|
||||||
console.error('Failed to fetch comparison:', err);
|
console.error('Failed to fetch comparison:', err);
|
||||||
});
|
});
|
||||||
|
return () => {
|
||||||
|
cancelled = true;
|
||||||
|
};
|
||||||
}, [urnKey, isInitialized]);
|
}, [urnKey, isInitialized]);
|
||||||
|
|
||||||
const primarySchools = selectedSchools.filter((school) => {
|
const primarySchools = selectedSchools.filter((school) => {
|
||||||
@@ -350,6 +365,14 @@ export function ComparisonView({
|
|||||||
style={{ '--school-count': activeSchools.length } as CSSProperties}
|
style={{ '--school-count': activeSchools.length } as CSSProperties}
|
||||||
aria-label="Schools in this comparison"
|
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) => (
|
{activeSchools.map((school, index) => (
|
||||||
<div
|
<div
|
||||||
key={school.urn}
|
key={school.urn}
|
||||||
|
|||||||
Reference in New Issue
Block a user