Compare commits

..
Author SHA1 Message Date
TudorandClaude Opus 5 7a16b1b52f fix(admissions): flag-off pages must not speak for the council
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m8s
The secondary admissions section words the absence of a cut-off distance:
"<LA> has not published a cut-off distance for this school." That sentence
is true when the authority publishes nothing. It is false when the
authority does publish and the admission_distance flag is simply off — and
off is the current state, so every secondary page with an EES admissions
row has been making a claim about a council on our behalf.

The backend already draws the distinction the copy needs. /api/schools/{urn}
omits the admission_distance key entirely while the flag is dark rather than
sending null, precisely so that "we are not publishing cut-offs" stays
distinguishable from "this school has no cut-off"; lib/types.ts says so in
as many words. The page then collapsed the two with `?? null` before the
section ever saw them.

So stop collapsing it: thread the raw field to SecondarySchoolSections and
word the absence only when the feature is on. Null still gets the sentence
naming the authority — that case is unchanged and still tested.

Primary pages are unaffected: AdmissionsSection carries no absence copy and
renders nothing when there is no figure. DistanceSection already treated
absent and null alike; only its type widens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 21:17:16 +01:00
tudor cf9d41b476 Merge pull request 'fix(analytics): the funnel source read a referrer that never changes' (#134) from fix/navigation-source-soft-nav into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 52s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 1m40s
Reviewed-on: #134
2026-08-27 08:26:45 +00:00
TudorandClaude Opus 5 e820e7fecd fix(analytics): the funnel source read a referrer that never changes
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m5s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m13s
The staging E2E gate has been red since #132 merged (run 1064, and
1066 after it): "a school reached from a location page is attributed
to it, not to direct" expects `place`, receives `direct`.

#132 fixed a real bug — `/schools/` had no case and fell through to
`direct` — but the mechanism underneath it never worked.
getNavigationSource read document.referrer, which the browser writes
only when a *document* loads. Every internal navigation here is an App
Router soft navigation: history.pushState, no new document, so
document.referrer goes on naming whatever opened the tab for the whole
session.

Verified on staging: load /schools/brentwood, click a school, the URL
becomes /school/… and document.referrer is still "".

So `from` reported `direct` for essentially every in-app journey, not
just the ones through the location layer — search, rankings, compare
and detail were all being counted as "typed the URL". The unit suite
passed throughout because every case set document.referrer directly,
which only happens on a full page load.

The fix is a module-level trail written by RouteTrail, a render-nothing
client component in the root layout. Its lifetime is exactly right: it
survives soft navigation, and it dies on a real document load — which
is precisely when document.referrer becomes meaningful again, so the
two cover each other with no overlap.

Reading it skips entries equal to the current path rather than taking
the second-to-last. That makes the answer independent of whether the
layout effect or the page effect ran first — React orders those by
tree position, which is not a contract worth resting a measurement on
— and it gives the right answer both when the user returns to a page
they came from and on a hard load of a school page.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 09:22:37 +01:00
tudor 4fdeb70a93 Merge pull request 'feat(places): say what each school is, not only how it scored' (#133) from feat/place-school-attributes into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 19s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 50s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 1m42s
Reviewed-on: #133
2026-08-27 07:59:10 +00:00
TudorandClaude Opus 5 9a1f56c431 feat(places): say what each school is, not only how it scored
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m44s
The location tables carried one column: a percentage. A parent
shortlisting from a town page is asking a different question first —
does it take my child's age, is it a faith school, does it have a
nursery — and the page could not answer any of it.

Primary tables gain Ages, Religious character, Nursery and
Constituency; secondary tables the same minus Nursery, which is a
question about a different intake. An all-through school renders in
both groups, so its nursery shows under primary alone.

The measure moves to the second column rather than the last. Six
columns overflow a phone and .tableWrap turns that into a horizontal
swipe; with the measure last, the one number the page exists for is
the one scrolled off the screen.

Cell rules are the ones the school page already uses, so the two
surfaces cannot disagree about the same school: "Does not apply",
"None" and "Not applicable" all read as no religious character, and
the en-dash age normalisation moves into formatAgeSpan, which
formatAgeRange now delegates to.

Backend: nursery_provision and parliamentary_constituency were not in
the place response. Both are optional GIAS mart columns that
data_loader degrades to NULL, and the `in rows.columns` guard keeps a
mart the pipeline has not rebuilt working.

Also fixes a live bug on the same line: SCHOOL_COLUMNS already ends
with latitude and longitude, and the endpoint concatenated them again,
so pandas dropped one of every duplicated pair and warned "columns are
not unique" on each request. Ordered de-duplication removes the
warning and the silent drop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FuPUioHpxtaiDNagQvjxyM
2026-08-27 08:49:14 +01:00
tudor ade9dbb3ba Merge pull request 'feat(analytics): measure the location layer, and stop calling it direct' (#132) from feat/place-analytics into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 51s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 12s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 1m44s
2026-08-27 07:29:48 +00:00
18 changed files with 658 additions and 23 deletions

No files matched your search

+16 -3
View File
@@ -1337,9 +1337,22 @@ async def get_place(request: Request, kind: str, slug: str,
for m in ("rwm_expected_pct", "attainment_8_score")
}
cols = [c for c in SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
"rwm_expected_pct", "attainment_8_score",
"total_pupils"]
# dict.fromkeys, not a list: SCHOOL_COLUMNS already ends with latitude and
# longitude, so concatenating them again selected each twice and pandas
# dropped one of every duplicated pair with a "columns are not unique"
# warning. Ordered de-duplication keeps the column order and the warning
# cannot come back.
#
# nursery_provision and parliamentary_constituency are not in
# SCHOOL_COLUMNS and the place table shows both. The `in rows.columns`
# guard is what keeps a mart the pipeline has not rebuilt working: those
# two are the optional GIAS columns data_loader degrades to NULL.
cols = [c for c in dict.fromkeys(
SCHOOL_COLUMNS + ["latitude", "longitude", "phase",
"nursery_provision",
"parliamentary_constituency",
"rwm_expected_pct", "attainment_8_score",
"total_pupils"])
if c in rows.columns]
return {
+46
View File
@@ -137,3 +137,49 @@ def test_an_authority_without_a_page_is_named_but_carries_no_slug(straddling_cli
by_name = {a["name"]: a for a in body["place"]["authorities"]}
assert by_name["Essex"]["slug"] == "essex"
assert by_name["Isles Of Scilly"]["slug"] is None
def _attributed_df() -> pd.DataFrame:
"""The same town, with the four attributes the place table now shows."""
df = _schools_df()
df["age_range"] = "4-11"
df["religious_denomination"] = "Church of England"
df["nursery_provision"] = True
df["parliamentary_constituency"] = "Brentwood and Ongar"
return df
@pytest.fixture()
def attributed_client(monkeypatch):
from backend import app as app_module
monkeypatch.setattr(app_module, "load_school_data", _attributed_df)
monkeypatch.setattr(app_module, "load_latest_school_data", _attributed_df)
monkeypatch.setattr(app_module, "_place_registry", None)
return TestClient(app_module.app, raise_server_exceptions=False)
def test_place_detail_carries_the_attributes_the_table_shows(attributed_client):
"""age_range and religious_denomination ride in on SCHOOL_COLUMNS.
nursery_provision and parliamentary_constituency do not, and the place
table needs all four — a column the response cannot fill is a column of
dashes on ~3,900 pages.
"""
body = attributed_client.get("/api/places/town/brentwood").json()
school = body["schools"][0]
assert school["age_range"] == "4-11"
assert school["religious_denomination"] == "Church of England"
assert school["nursery_provision"] is True
assert school["parliamentary_constituency"] == "Brentwood and Ongar"
def test_place_detail_survives_a_mart_without_the_optional_columns(client):
"""The base fixture has neither column, as an unrebuilt mart does not.
data_loader degrades those to NULL rather than failing the load, so the
endpoint must not assume they are present.
"""
res = client.get("/api/places/town/brentwood")
assert res.status_code == 200
assert "nursery_provision" not in res.json()["schools"][0]
+99
View File
@@ -1304,6 +1304,52 @@ test('with the distance feature off, the section is absent rather than empty', a
.toHaveCount(0);
});
/**
* A secondary school carrying an EES admissions row, which is what makes its
* Admissions section render while the distance feature is dark.
*/
async function secondarySchoolWithAdmissions(page: Page) {
const list = await page.request.get('/api/schools?phase=secondary&page_size=40');
if (!list.ok()) return null;
const body = await list.json();
for (const s of (body?.schools ?? []).slice(0, 25)) {
const res = await page.request.get(`/api/schools/${s.urn}`);
if (!res.ok()) continue;
const detail = await res.json();
if (detail?.admissions == null) continue;
return { urn: s.urn as number };
}
return null;
}
test('with the distance feature off, a secondary page makes no claim about publication', async ({ page }) => {
/*
* Shipping dark must not put words in the council's mouth. The secondary
* template is the only one that words the absence, and "X has not published
* a cut-off distance for this school" is false wherever X does publish and
* we are simply withholding it.
*
* This is why the API omits the key rather than sending null: absent means
* "cut-offs are not published at all", null means "this school has none".
* Only the second is a fact about the school, and only the second is sayable.
*/
test.skip(await distanceFeatureIsOn(page),
'the admission_distance flag is on in this environment');
const found = await secondarySchoolWithAdmissions(page);
test.skip(found === null, 'no secondary school in the sample has an admissions row');
await page.goto(`/school/${found!.urn}`);
await expect(page.locator('h1').first()).toBeVisible({ timeout: 15_000 });
// The Admissions section is still there — this is not a test that the whole
// section vanished, which would pass for the wrong reason.
await expect(page.locator('#admissions')).toHaveCount(1);
await expect(page.getByText(/has not published a cut-off distance/)).toHaveCount(0);
await expect(page.getByText(/Contact the admissions authority/)).toHaveCount(0);
});
test('/api/flags is not reachable from the public internet', async ({ page }) => {
// It names every unreleased feature and whether it is on. Next reads it
// server-side over the Docker network; the public proxy must deny it.
@@ -1978,6 +2024,59 @@ test('a place page links its phase variants, and they resolve', async ({ page })
await expect(page.locator('h1')).toContainText(new RegExp(`${phase} schools in`, 'i'));
});
/*
* The table shipped with one column of scores. A parent shortlisting from a
* town page needs to know whether a school takes their child's age, whether
* it is a faith school, and — for a primary — whether it has a nursery,
* before a percentage means anything.
*
* These assert the column headings rather than the values: nursery_provision
* and parliamentary_constituency are optional mart columns, and on an
* environment whose pipeline has not rebuilt them the API degrades them to
* absent. A value assertion would then fail for a data reason, not a code one.
*/
async function phasedPlace(page: Page, phase: 'primary' | 'secondary') {
const place = await firstPlaceOfKind(page, 'town');
const detail = await (await page.request.get(`/api/places/town/${place.slug}`)).json();
test.skip(!(detail.place.phases ?? []).includes(phase),
`no ${phase} page clears the threshold here`);
return place;
}
test('a primary place page names each school as well as scoring it', async ({ page }) => {
const place = await phasedPlace(page, 'primary');
await page.goto(`/schools/${place.slug}/primary`);
for (const heading of ['Ages', 'Religious character', 'Nursery', 'Constituency']) {
await expect(page.getByRole('columnheader', { name: heading, exact: true }))
.toBeVisible();
}
// age_range rides in on SCHOOL_COLUMNS and predates the optional columns,
// so it is the one attribute safe to assert a value for anywhere.
await expect(page.locator('table tbody td').filter({ hasText: /^\d+–\d+$/ }).first())
.toBeVisible();
});
test('a secondary place page does not ask about nurseries', async ({ page }) => {
const place = await phasedPlace(page, 'secondary');
await page.goto(`/schools/${place.slug}/secondary`);
await expect(page.getByRole('columnheader', { name: 'Ages', exact: true }))
.toBeVisible();
await expect(page.getByRole('columnheader', { name: 'Nursery', exact: true }))
.toHaveCount(0);
});
test('the measure stays beside the school name, not behind a swipe', async ({ page }) => {
// Six columns overflow a phone; .tableWrap turns that into a horizontal
// scroll. With the measure last, the number the page exists for is the one
// off the screen.
const place = await phasedPlace(page, 'primary');
await page.setViewportSize({ width: 390, height: 844 });
await page.goto(`/schools/${place.slug}/primary`);
const second = page.locator('table thead th').nth(1);
await expect(second).toContainText(/reading, writing/i);
await expect(second).toBeInViewport();
});
test('phase variants are submitted in the places sitemap', async ({ page }) => {
const xml = await (await page.request.get('/sitemaps/places-1.xml')).text();
expect(xml).toMatch(/\/schools\/[a-z0-9-]+\/primary</);
@@ -346,3 +346,134 @@ describe('PlaceView unlinkable authorities', () => {
.toContain('Isles Of Scilly');
});
});
describe('PlaceView school attributes', () => {
/*
* The table shipped with one column of scores, which answers "how did they
* do" and nothing about whether the school is one a family could use. Age
* range, faith, nursery and constituency are the four facts a parent
* filters on before they look at a number at all.
*/
const withAttributes: PlaceDetail = {
place: { kind: 'town', slug: 'chelmsford', name: 'Chelmsford', count: 3,
parent_authority: 'Essex', phases: ['primary', 'secondary'] },
schools: [
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
rwm_expected_pct: 82, attainment_8_score: null,
age_range: '4-11', religious_denomination: 'Church of England',
nursery_provision: true,
parliamentary_constituency: 'Chelmsford' } as never,
{ urn: 2, school_name: 'Beta High', phase: 'Secondary',
rwm_expected_pct: null, attainment_8_score: 47,
age_range: '11-16', religious_denomination: 'Does not apply',
nursery_provision: false,
parliamentary_constituency: 'Witham' } as never,
],
averages: { rwm_expected_pct: 63, attainment_8_score: 45 },
};
function headings(container: HTMLElement, table = 0): string[] {
return Array.from(container.querySelectorAll('table')[table]
.querySelectorAll('thead th')).map((th) => th.textContent ?? '');
}
it('heads a primary table with all four attributes', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
expect(headings(container)).toEqual([
'School', 'Reading, writing & maths',
'Ages', 'Religious character', 'Nursery', 'Constituency',
]);
});
it('omits nursery from a secondary table, where it does not apply', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
expect(headings(container, 1)).toEqual([
'School', 'Attainment 8', 'Ages', 'Religious character', 'Constituency',
]);
});
it('keeps the measure beside the school name, where a phone can see it', () => {
// Six columns overflow a phone and .tableWrap turns that into a swipe.
// With the measure last, the one number the page exists for is the one
// scrolled off the screen.
const { container } = render(<PlaceView detail={withAttributes}
phase="primary" englandAverage={61} neighbours={[]} />);
expect(headings(container)[1]).toBe('Reading, writing & maths');
});
it('shows the age range without repeating the column heading', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('4–11')).toBeInTheDocument();
expect(screen.queryByText('Ages 4–11')).not.toBeInTheDocument();
});
it('names the faith of a faith school', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('Church of England')).toBeInTheDocument();
});
it('reads "Does not apply" as no religious character, not as a value', () => {
// GIAS spells the absence of a faith as "Does not apply", which is a
// database answer rather than an English one. The school page already
// suppresses it; the two must not disagree about the same school.
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
const secondary = container.querySelectorAll('table')[1]
.querySelectorAll('tbody td');
expect(secondary[3].textContent).toBe('—');
expect(screen.queryByText(/Does not apply/)).not.toBeInTheDocument();
});
it('marks a nursery as such and a school without one as not', () => {
const { container } = render(<PlaceView detail={withAttributes}
englandAverage={61} neighbours={[]} />);
const cells = container.querySelectorAll('table')[0]
.querySelectorAll('tbody td');
expect(cells[4].textContent).toBe('Yes');
});
it('names the constituency of each school', () => {
render(<PlaceView detail={withAttributes} englandAverage={61}
neighbours={[]} />);
expect(screen.getByText('Chelmsford', { selector: 'td' })).toBeInTheDocument();
expect(screen.getByText('Witham', { selector: 'td' })).toBeInTheDocument();
});
it('dashes an attribute the data does not carry', () => {
// nursery_provision and parliamentary_constituency are absent from marts
// the pipeline has not rebuilt, and the API degrades them to null rather
// than failing. A row must survive that.
const bare: PlaceDetail = {
...withAttributes,
schools: [{ urn: 3, school_name: 'Gamma Primary', phase: 'Primary',
rwm_expected_pct: 70 } as never],
};
const { container } = render(<PlaceView detail={bare} phase="primary"
englandAverage={61} neighbours={[]} />);
const cells = Array.from(container.querySelectorAll('tbody td'))
.map((td) => td.textContent);
expect(cells.slice(2)).toEqual(['—', '—', '—', '—']);
});
it('gives an all-through school its nursery under primary only', () => {
// All-through schools render in both groups. Nursery belongs to the
// primary reading of the same school, not the secondary one.
const allThrough: PlaceDetail = {
...withAttributes,
schools: [{ urn: 4, school_name: 'Delta Academy', phase: 'All-through',
rwm_expected_pct: 66, attainment_8_score: 51,
age_range: '4-18', religious_denomination: 'None',
nursery_provision: true,
parliamentary_constituency: 'Chelmsford' } as never],
};
const { container } = render(<PlaceView detail={allThrough}
englandAverage={61} neighbours={[]} />);
const tables = container.querySelectorAll('table');
expect(tables[0].textContent).toContain('Yes');
expect(tables[1].textContent).not.toContain('Yes');
});
});
@@ -0,0 +1,38 @@
/**
* The trail has to be written by something, and it has to be written on every
* route — not only the ones that happen to track an event.
*/
import { render } from '@testing-library/react';
const recordVisitedPath = jest.fn();
let pathname = '/schools/brentwood';
jest.mock('next/navigation', () => ({ usePathname: () => pathname }));
jest.mock('@/lib/analytics', () => ({
recordVisitedPath: (p: string) => recordVisitedPath(p),
}));
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { RouteTrail } = require('@/components/RouteTrail');
describe('RouteTrail', () => {
beforeEach(() => recordVisitedPath.mockClear());
it('records the page it is mounted on', () => {
render(<RouteTrail />);
expect(recordVisitedPath).toHaveBeenCalledWith('/schools/brentwood');
});
it('records each new route as the user moves through the app', () => {
const { rerender } = render(<RouteTrail />);
pathname = '/school/115429-brentwood-school';
rerender(<RouteTrail />);
expect(recordVisitedPath).toHaveBeenLastCalledWith(
'/school/115429-brentwood-school');
});
it('renders nothing, so it can sit anywhere in the layout', () => {
const { container } = render(<RouteTrail />);
expect(container).toBeEmptyDOMElement();
});
});
@@ -98,6 +98,17 @@ describe('secondary detail page', () => {
expect(screen.getByText(/has not published a cut-off distance/)).toBeInTheDocument();
});
it('makes no claim about publication when the feature is switched off', () => {
// Absent, not null. The API omits the key entirely while the
// admission_distance flag is off, and "Islington has not published a
// cut-off distance" is then a statement about us, not about Islington —
// false wherever the authority does publish one.
renderSecondarySchoolDetail({ ...secondaryFixture, admissionDistance: undefined });
expect(screen.queryByText(/has not published a cut-off distance/)).not.toBeInTheDocument();
expect(screen.queryByText(/Contact the admissions authority/)).not.toBeInTheDocument();
});
});
// ── The Distance section ───────────────────────────────────────────────
@@ -62,3 +62,97 @@ describe('getNavigationSource', () => {
expect(getNavigationSource()).toBe('direct');
});
});
/*
* The defect the existing suite could not see.
*
* Every test above sets document.referrer, which the browser writes only when
* a *document* loads. Every internal navigation in this app is an App Router
* soft navigation — history.pushState, no new document — so document.referrer
* keeps naming whatever opened the tab for the whole session. Verified on
* staging: /schools/brentwood → click a school → URL changes to /school/…
* and document.referrer is still "".
*
* So `from` reported 'direct' for essentially every in-app journey, and the
* suite passed because it only ever exercised the full-page-load path.
*/
function freshAnalytics() {
let mod!: typeof import('@/lib/analytics');
jest.isolateModules(() => {
mod = require('@/lib/analytics');
});
return mod;
}
function at(path: string) {
window.history.pushState({}, '', path);
}
describe('getNavigationSource across a soft navigation', () => {
afterEach(() => {
referrer('');
at('/');
});
it('attributes a school view to the place page the user actually came from', () => {
const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
at('/schools/brentwood');
recordVisitedPath('/schools/brentwood');
at('/school/115429-brentwood-school');
recordVisitedPath('/school/115429-brentwood-school');
expect(source()).toBe('place');
});
it('does not depend on whether the new path was recorded first', () => {
// The trail is written by a layout-level effect and read by a page-level
// one. React orders those by tree position, which is not a contract worth
// resting a measurement on, so the answer must be the same either way.
const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
recordVisitedPath('/rankings');
at('/school/115429-brentwood-school');
expect(source()).toBe('rankings');
});
it('names the previous page, not the current one, when both are schools', () => {
const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
at('/school/100010-brecknock-primary-school');
recordVisitedPath('/school/100010-brecknock-primary-school');
at('/school/115429-brentwood-school');
recordVisitedPath('/school/115429-brentwood-school');
expect(source()).toBe('detail');
});
it('looks past a return visit to the page the user came back from', () => {
const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
for (const p of ['/schools/brentwood', '/school/115429-brentwood-school',
'/schools/brentwood']) {
at(p);
recordVisitedPath(p);
}
expect(source()).toBe('detail');
});
it('falls back to the referrer on a real document load, where it is true', () => {
// A fresh module is a fresh document: nothing has been recorded, and
// document.referrer is meaningful again.
const { getNavigationSource: source } = freshAnalytics();
at('/school/115429-brentwood-school');
referrer(`${ORIGIN}/schools/barnet`);
expect(source()).toBe('place');
});
it('still reads an arrival from outside as direct', () => {
const { recordVisitedPath, getNavigationSource: source } = freshAnalytics();
at('/schools/brentwood');
recordVisitedPath('/schools/brentwood');
referrer('https://www.google.com/search?q=schools+in+brentwood');
expect(source()).toBe('direct');
});
});
+26
View File
@@ -13,6 +13,8 @@ import {
metricKind,
shortName,
computeYBounds,
formatAgeRange,
formatAgeSpan,
} from '@/lib/utils';
describe('formatPercentage', () => {
@@ -320,3 +322,27 @@ describe('shortName', () => {
expect(shortName('A'.repeat(30), 10)).toBe('AAAAAAAAA…');
});
});
describe('formatAgeSpan', () => {
it('normalises a hyphenated range to an en dash, without a label', () => {
// The place table carries "Ages" in the column heading, so repeating it
// in every cell is noise. formatAgeRange keeps the label for the contexts
// that have no heading to hang it on.
expect(formatAgeSpan('4-11')).toBe('4–11');
});
it('leaves a range it does not recognise alone rather than mangling it', () => {
expect(formatAgeSpan('3-19 (SEN)')).toBe('3-19 (SEN)');
});
it('returns an empty string for a missing range', () => {
expect(formatAgeSpan(null)).toBe('');
expect(formatAgeSpan(undefined)).toBe('');
});
});
describe('formatAgeRange', () => {
it('keeps its label, so the two helpers stay distinguishable', () => {
expect(formatAgeRange('4-11')).toBe('Ages 4–11');
});
});
+5
View File
@@ -4,6 +4,7 @@ import Script from 'next/script';
import { Navigation } from '@/components/Navigation';
import { Footer } from '@/components/Footer';
import { ComparisonToast } from '@/components/ComparisonToast';
import { RouteTrail } from '@/components/RouteTrail';
import { ComparisonProvider } from '@/context/ComparisonProvider';
import { SITE_URL } from '@/lib/site';
import './globals.css';
@@ -114,6 +115,10 @@ export default function RootLayout({
/>
</head>
<body>
{/* Records every route so funnel attribution has a previous page to
name. document.referrer cannot: a soft navigation creates no
document, so the browser never updates it. */}
<RouteTrail />
<ComparisonProvider>
<a href="#main-content" className="skip-link">Skip to main content</a>
<Navigation />
+1 -1
View File
@@ -232,7 +232,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) {
census={census ?? null}
admissions={admissions ?? null}
admissionsHistory={admissions_history ?? []}
admissionDistance={admission_distance ?? null}
admissionDistance={admission_distance}
deprivation={deprivation ?? null}
finance={finance ?? null}
nationalAvg={nationalAvg}
+27
View File
@@ -0,0 +1,27 @@
/**
* Writes the in-app navigation trail that funnel attribution reads.
*
* Renders nothing. It exists because document.referrer cannot answer "which
* page did they come from" in an App Router app: a soft navigation creates no
* document, so the browser never updates it. See the trail comment in
* lib/analytics.ts.
*
* Mounted once in the root layout, so every route is recorded — including the
* ones that fire no event of their own, which are still somebody else's
* previous page.
*/
'use client';
import { useEffect } from 'react';
import { usePathname } from 'next/navigation';
import { recordVisitedPath } from '@/lib/analytics';
export function RouteTrail() {
const pathname = usePathname();
useEffect(() => {
recordVisitedPath(pathname);
}, [pathname]);
return null;
}
@@ -164,6 +164,34 @@
white-space: nowrap;
}
/*
* Attribute columns. Muted, because they qualify the row rather than compete
* with the measure for it, and hugging their content so the school name keeps
* the spare width — the same width:1% trick as .num, which is what stops six
* columns from splitting evenly and squeezing the names into two lines each.
*
* .attr never wraps: "4–11" and "Yes" broken across lines read as two values.
* .attrWide may — "Church of England" and some constituency names are long
* enough that forcing one line would push the measure off a phone screen.
*/
.table th.attr,
.table td.attr,
.table th.attrWide,
.table td.attrWide {
color: var(--text-secondary);
width: 1%;
}
.table th.attr,
.table td.attr {
white-space: nowrap;
}
.table th.attrWide,
.table td.attrWide {
min-width: 8rem;
}
/* The measure is spelled out; the tooltip carries the definition. */
.metricHead {
text-decoration: none;
+44 -1
View File
@@ -13,7 +13,7 @@ import Link from 'next/link';
import type { PlaceDetail, PlaceSummary } from '@/lib/places';
import { placeUrl, authoritySlug } from '@/lib/places';
import type { School } from '@/lib/types';
import { schoolUrl } from '@/lib/utils';
import { schoolUrl, formatAgeSpan } from '@/lib/utils';
import { absoluteUrl } from '@/lib/site';
import { TrackPlaceView } from './TrackPlaceView';
import styles from './PlaceView.module.css';
@@ -64,8 +64,31 @@ function isPhase(school: School, phase: PhaseKey): boolean {
: p.includes('primary') || p.includes('middle');
}
/*
* GIAS spells the absence of a faith as "Does not apply", and sometimes
* "None" or "Not applicable" — database answers, not English ones. The school
* page and the comparison already suppress all three; this is the same rule,
* so the two surfaces cannot disagree about the same school.
*/
const NO_FAITH = /^(none|does not apply|not applicable)$/i;
/** An attribute the data does not carry. Distinct from the measure's "Not
* published": four of those per row would drown the row it qualifies. */
const NO_VALUE = '—';
function faithOf(school: School): string {
const denom = school.religious_denomination ?? '';
return denom && !NO_FAITH.test(denom) ? denom : NO_VALUE;
}
function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey }) {
const metric = METRICS[phase];
/*
* Nursery is a primary question. An all-through school renders in both
* groups, and its nursery belongs to the primary reading of it — under
* "Secondary schools" the column would be a fact about a different intake.
*/
const showNursery = phase === 'primary';
return (
<div className={styles.tableWrap}>
<table className={styles.table}>
@@ -79,6 +102,13 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
{metric.heading}
</abbr>
</th>
{/* The measure sits second, not last. Six columns overflow a
phone and .tableWrap turns that into a swipe; last would put
the one number the page exists for off the screen. */}
<th scope="col" className={styles.attr}>Ages</th>
<th scope="col" className={styles.attrWide}>Religious character</th>
{showNursery && <th scope="col" className={styles.attr}>Nursery</th>}
<th scope="col" className={styles.attrWide}>Constituency</th>
</tr>
</thead>
<tbody>
@@ -96,6 +126,19 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
? <span className={styles.noData}>Not published</span>
: `${Math.round(Number(value))}${metric.unit}`}
</td>
<td className={styles.attr}>{formatAgeSpan(s.age_range) || NO_VALUE}</td>
<td className={styles.attrWide}>{faithOf(s)}</td>
{showNursery && (
<td className={styles.attr}>
{/* Undefined is a mart the pipeline has not rebuilt, and
false is a school without one. Neither is a "Yes", and
neither is worth two different words. */}
{s.nursery_provision ? 'Yes' : NO_VALUE}
</td>
)}
<td className={styles.attrWide}>
{s.parliamentary_constituency || NO_VALUE}
</td>
</tr>
);
})}
@@ -24,7 +24,7 @@ export function DistanceSection({
admissionDistance,
schoolInfo,
}: {
admissionDistance: SchoolAdmissionDistance | null;
admissionDistance: SchoolAdmissionDistance | null | undefined;
schoolInfo: School;
}) {
// Without a figure there is nothing to compare against, and without
@@ -21,11 +21,17 @@ export function SecondaryAdmissionsSection({
published cut-off and no EES admissions row. */
admissions: SchoolAdmissions | null;
admissionsHistory: SchoolAdmissions[];
admissionDistance: SchoolAdmissionDistance | null;
admissionDistance: SchoolAdmissionDistance | null | undefined;
schoolInfo: School;
hasSixthForm: boolean;
}) {
const cutoff = describeCutoff(admissionDistance);
/* Absent means cut-offs are not being published at all; null means this
school has no published cut-off. Only the second is a fact about the
school, and only the second can be stated. Saying "X has not published a
cut-off" while the feature is dark describes us, and is false wherever the
authority does publish one. */
const featureOn = admissionDistance !== undefined;
// Moved with this section from SecondarySchoolDetailView, its only consumer.
const admissionsTag = (() => {
const policy = schoolInfo.admissions_policy?.toLowerCase() ?? '';
@@ -102,7 +108,7 @@ export function SecondaryAdmissionsSection({
{CUTOFF_NOTE} {CUTOFF_MEASUREMENT_NOTE}
{cutoff.routeNote && <> {cutoff.routeNote}</>}
</p>
) : (
) : featureOn ? (
<p className={styles.sectionSubtitle} style={{ marginTop: '1rem' }}>
{describeCutoffAbsence({
localAuthority: schoolInfo.local_authority,
@@ -110,7 +116,7 @@ export function SecondaryAdmissionsSection({
admissionsHistory,
})}
</p>
)}
) : null}
{hasSixthForm && (
<div className={styles.sixthFormNote}>
@@ -36,7 +36,10 @@ export interface SecondarySchoolSectionsProps {
/** Needed to tell a year with no published cut-off apart from a year the
* school simply was not oversubscribed. */
admissionsHistory: SchoolAdmissions[];
admissionDistance: SchoolAdmissionDistance | null;
/** Absent — not null — while the admission_distance flag is off. The two
* mean different things to the reader and must stay distinguishable:
* see SecondaryAdmissionsSection, which words the absence. */
admissionDistance: SchoolAdmissionDistance | null | undefined;
deprivation: SchoolDeprivation | null;
finance: SchoolFinance | null;
nationalAvg: NationalAverages | null;
+66 -11
View File
@@ -60,22 +60,77 @@ export function track(name: EventName, data?: Payload): void {
export type NavigationSource =
'search' | 'rankings' | 'compare' | 'detail' | 'place' | 'direct';
/*
* The in-app trail.
*
* document.referrer is written by the browser only when a *document* loads.
* Every internal navigation here is an App Router soft navigation —
* history.pushState, no new document — so document.referrer goes on naming
* whatever opened the tab (usually nothing, or a search engine) for the whole
* session. Reading it to answer "which page did they come from" therefore
* returned 'direct' for essentially every in-app journey, including the one
* the location layer exists to produce.
*
* Verified on staging: /schools/brentwood, click a school, the URL becomes
* /school/… and document.referrer is still "".
*
* A module-level trail is the counterpart with exactly the right lifetime. It
* survives soft navigation, and it dies on a real document load — which is
* precisely when document.referrer becomes meaningful again, so the two cover
* each other with no overlap.
*/
const TRAIL_LIMIT = 4;
const trail: string[] = [];
/** Record a path the user is now on. Called by RouteTrail on every route. */
export function recordVisitedPath(path: string): void {
if (trail[trail.length - 1] === path) return;
trail.push(path);
if (trail.length > TRAIL_LIMIT) trail.shift();
}
/**
* The most recent path that is not the one being viewed.
*
* Skipping the current path rather than taking trail[length - 2] is what
* makes the answer independent of ordering: the trail is written by a
* layout-level effect and read by a page-level one, and React orders those by
* tree position — not a contract worth resting a measurement on. It also
* gives the right answer when the user goes back to a page they came from.
*/
function previousInAppPath(): string | null {
if (typeof window === 'undefined') return null;
const current = window.location.pathname;
for (let i = trail.length - 1; i >= 0; i -= 1) {
if (trail[i] !== current) return trail[i];
}
return null;
}
function classifyPath(p: string): NavigationSource {
if (p === '/' || p === '') return 'search';
if (p.startsWith('/rankings')) return 'rankings';
if (p.startsWith('/compare')) return 'compare';
// `/schools/` before `/school/`: they differ by one letter and mean
// different things — the location layer versus a single school. Checked
// first so the narrower-looking prefix cannot shadow it if either string
// is ever edited.
if (p.startsWith('/schools/')) return 'place';
if (p.startsWith('/school/')) return 'detail';
return 'direct';
}
export function getNavigationSource(): NavigationSource {
const internal = previousInAppPath();
if (internal) return classifyPath(internal);
// No trail means this is the first page of the document, so the referrer is
// the only witness — and an honest one.
if (typeof window === 'undefined' || !document.referrer) return 'direct';
try {
const ref = new URL(document.referrer);
if (ref.origin !== window.location.origin) return 'direct';
const p = ref.pathname;
if (p === '/' || p === '') return 'search';
if (p.startsWith('/rankings')) return 'rankings';
if (p.startsWith('/compare')) return 'compare';
// `/schools/` before `/school/`: they differ by one letter and mean
// different things — the location layer versus a single school. Checked
// first so the narrower-looking prefix cannot shadow it if either string
// is ever edited.
if (p.startsWith('/schools/')) return 'place';
if (p.startsWith('/school/')) return 'detail';
return 'direct';
return classifyPath(ref.pathname);
} catch {
return 'direct';
}
+12 -2
View File
@@ -82,11 +82,21 @@ export function shortName(name: string, maxLength = 32): string {
* Display-only — leaves the raw `age_range` field (used for sixth-form
* detection) untouched. Falls back to the raw value if it's not a plain range.
*/
export function formatAgeRange(ageRange: string | null | undefined): string {
export function formatAgeSpan(ageRange: string | null | undefined): string {
if (!ageRange) return '';
const match = ageRange.match(/^\s*(\d+)\s*[-–]\s*(\d+)\s*$/);
if (!match) return ageRange;
return `Ages ${match[1]}–${match[2]}`;
return `${match[1]}–${match[2]}`;
}
/**
* The same span, labelled — for the places that show it with no column
* heading to carry the word "Ages". Delegates so the en-dash normalisation
* lives in one place.
*/
export function formatAgeRange(ageRange: string | null | undefined): string {
const span = formatAgeSpan(ageRange);
return /^\d+–\d+$/.test(span) ? `Ages ${span}` : span;
}
// ============================================================================