Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
090d5f7bec | ||
|
|
abc03a0dd3 | ||
|
|
43a2c4a6bc |
@@ -5,6 +5,13 @@ on:
|
||||
branches:
|
||||
- main
|
||||
|
||||
# Cancel superseded runs: pushing a new commit to a PR (or an empty
|
||||
# re-trigger) aborts the previous still-running checks instead of running
|
||||
# a second full matrix alongside them.
|
||||
concurrency:
|
||||
group: pr-checks-${{ gitea.event.pull_request.number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
env:
|
||||
REGISTRY: privaterepo.sitaru.org
|
||||
BACKEND_IMAGE_NAME: ${{ gitea.repository }}-backend
|
||||
@@ -23,12 +30,22 @@ jobs:
|
||||
uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: 22
|
||||
cache: npm
|
||||
cache-dependency-path: nextjs-app/package-lock.json
|
||||
|
||||
# Cache the resolved node_modules (452 MB / 460 packages) keyed on the
|
||||
# lockfile. On a hit — the common case, since deps change rarely — the
|
||||
# whole `npm ci` step is skipped, not just its download phase. The key
|
||||
# pins OS + node major so we never restore incompatible native binaries.
|
||||
- name: Cache node_modules
|
||||
id: node-modules-cache
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: nextjs-app/node_modules
|
||||
key: nextjs-node-modules-${{ runner.os }}-node22-${{ hashFiles('nextjs-app/package-lock.json') }}
|
||||
|
||||
- name: Install dependencies
|
||||
if: steps.node-modules-cache.outputs.cache-hit != 'true'
|
||||
working-directory: nextjs-app
|
||||
run: npm ci
|
||||
run: npm ci --prefer-offline --no-audit --no-fund
|
||||
|
||||
- name: Typecheck
|
||||
working-directory: nextjs-app
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* Regression: on refresh, the compare page must show the SSR-rendered data.
|
||||
*
|
||||
* The basket hydrates from the URL a beat after mount (selectedSchools is
|
||||
* empty for the first render), so the fetch effect must not blank the
|
||||
* SSR payload during that window — and must not refetch data the server
|
||||
* already provided.
|
||||
*/
|
||||
|
||||
import { 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,
|
||||
};
|
||||
}
|
||||
|
||||
const INITIAL_DATA = {
|
||||
'100': data(100, 'Alpha Primary'),
|
||||
'200': data(200, 'Beta Primary'),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
fetchComparison.mockReset();
|
||||
});
|
||||
|
||||
test('renders SSR data on refresh without wiping it or refetching', async () => {
|
||||
render(
|
||||
<ComparisonProvider>
|
||||
<ComparisonView
|
||||
initialData={INITIAL_DATA}
|
||||
initialNationalAverages={{
|
||||
year: 202425,
|
||||
primary: { rwm_expected_pct: 62 },
|
||||
secondary: {},
|
||||
by_year: [],
|
||||
}}
|
||||
initialBenchmarks={undefined}
|
||||
initialUrns={[100, 200]}
|
||||
metrics={[]}
|
||||
selectedMetric="rwm_expected_pct"
|
||||
/>
|
||||
</ComparisonProvider>,
|
||||
);
|
||||
|
||||
// Both SSR-provided schools appear (data was not blanked during hydration)
|
||||
await waitFor(() => {
|
||||
expect(screen.getAllByText('Alpha Primary').length).toBeGreaterThan(0);
|
||||
});
|
||||
expect(screen.getAllByText('Beta Primary').length).toBeGreaterThan(0);
|
||||
expect(screen.getByRole('heading', { name: 'At a glance' })).toBeInTheDocument();
|
||||
|
||||
// …and the client never refetched data the server already rendered.
|
||||
expect(fetchComparison).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -107,24 +107,26 @@ export function ComparisonView({
|
||||
router.replace(newUrl, { scroll: false });
|
||||
}, [urnKey, selectedMetric, pathname, searchParams, router]);
|
||||
|
||||
// Fetch only when the school set changes. The very first run is skipped
|
||||
// when the SSR payload already covers the current set — no double-fetch
|
||||
// of data the server just rendered.
|
||||
const firstFetchRef = useRef(true);
|
||||
useEffect(() => {
|
||||
if (!urnKey) {
|
||||
setComparisonData(null);
|
||||
setNationalAverages(undefined);
|
||||
setBenchmarks(undefined);
|
||||
return;
|
||||
}
|
||||
// Fetch when the school set changes, but only for schools we don't already
|
||||
// have data for. This skips the refetch of SSR-rendered data on load AND
|
||||
// avoids a network call when a school is merely removed. A ref holds the
|
||||
// latest data so the effect can read it without re-running on every fetch.
|
||||
//
|
||||
// Correctness note: we must NOT null the data on a transient empty urnKey.
|
||||
// On mount the basket is empty for a beat before it hydrates from the URL,
|
||||
// and blanking here (then skipping the refetch because SSR "covers" the set)
|
||||
// was leaving the page empty on refresh. The render already shows the empty
|
||||
// state whenever `selectedSchools` is empty, so stale data for deselected
|
||||
// schools is harmless — it's simply unused.
|
||||
const comparisonDataRef = useRef(comparisonData);
|
||||
comparisonDataRef.current = comparisonData;
|
||||
|
||||
if (firstFetchRef.current) {
|
||||
firstFetchRef.current = false;
|
||||
const ssrUrns = new Set(Object.keys(initialData ?? {}));
|
||||
const covered = urnKey.split(',').every((urn) => ssrUrns.has(urn));
|
||||
if (covered && ssrUrns.size > 0) return;
|
||||
}
|
||||
useEffect(() => {
|
||||
if (!isInitialized || !urnKey) return;
|
||||
|
||||
const have = comparisonDataRef.current ?? {};
|
||||
const covered = urnKey.split(',').every((urn) => have[urn] != null);
|
||||
if (covered) return;
|
||||
|
||||
fetchComparison(urnKey, { cache: 'no-store' })
|
||||
.then((data) => {
|
||||
@@ -138,8 +140,7 @@ export function ComparisonView({
|
||||
// destroy a working comparison the user is looking at.
|
||||
console.error('Failed to fetch comparison:', err);
|
||||
});
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [urnKey]);
|
||||
}, [urnKey, isInitialized]);
|
||||
|
||||
// Classify schools by phase using comparison data
|
||||
const classifySchool = (school: School): 'primary' | 'secondary' => {
|
||||
|
||||
@@ -1,50 +1,18 @@
|
||||
/**
|
||||
* Custom hook for managing school comparison state
|
||||
* Uses shared context for real-time updates across components
|
||||
* Custom hook for managing school comparison state.
|
||||
*
|
||||
* This hook is mounted on every page via the global Navigation and
|
||||
* ComparisonToast, so it must stay cheap — it exposes basket state only.
|
||||
* The compare page fetches `/api/compare` itself (ComparisonView); nothing
|
||||
* ever read the comparison payload from here, so the previous per-page SWR
|
||||
* fetch (which fired on every page whenever the basket was non-empty) was
|
||||
* dead weight and has been removed.
|
||||
*/
|
||||
|
||||
'use client';
|
||||
|
||||
import useSWR from 'swr';
|
||||
import { fetcher } from '@/lib/api';
|
||||
import { useComparisonContext } from '@/context/ComparisonContext';
|
||||
import type { ComparisonResponse } from '@/lib/types';
|
||||
|
||||
export function useComparison() {
|
||||
const {
|
||||
selectedSchools,
|
||||
addSchool,
|
||||
removeSchool,
|
||||
replaceSchools,
|
||||
clearAll,
|
||||
isSelected,
|
||||
canAddMore,
|
||||
isInitialized,
|
||||
} = useComparisonContext();
|
||||
|
||||
// Fetch comparison data for selected schools
|
||||
const urns = selectedSchools.map((s) => s.urn).join(',');
|
||||
const { data, error, isLoading, mutate } = useSWR<ComparisonResponse>(
|
||||
selectedSchools.length > 0 ? `/compare?urns=${urns}` : null,
|
||||
fetcher,
|
||||
{
|
||||
revalidateOnFocus: false,
|
||||
dedupingInterval: 10000,
|
||||
}
|
||||
);
|
||||
|
||||
return {
|
||||
selectedSchools,
|
||||
comparisonData: data?.comparison,
|
||||
isLoading,
|
||||
error,
|
||||
addSchool,
|
||||
removeSchool,
|
||||
replaceSchools,
|
||||
clearAll,
|
||||
isSelected,
|
||||
canAddMore,
|
||||
isInitialized,
|
||||
mutate,
|
||||
};
|
||||
return useComparisonContext();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user