feat(places): list schools alphabetically on place pages #123

Merged
tudor merged 2 commits from feat/place-alphabetical-sort into main 2026-08-21 23:22:49 +00:00
9 changed files with 106 additions and 14 deletions
Showing only changes of commit 8967966eef - Show all commits

No files matched your search

+8 -3
View File
@@ -1212,10 +1212,15 @@ async def get_place(request: Request, kind: str, slug: str,
if wanted and "phase" in rows.columns:
rows = rows[rows["phase"].fillna("").str.lower().isin(wanted)]
# The metric the page ranks on, which is also the one it averages.
# The metric the page shows, and averages.
metric = "attainment_8_score" if phase == "secondary" else "rwm_expected_pct"
if metric in rows.columns:
rows = rows.sort_values(metric, ascending=False, na_position="last")
# Alphabetical, not by score. A place page is read by someone looking for
# a school they can name, and scanning for it is what the order should
# serve. /rankings is where the league-table ordering lives, and it keeps
# sorting by metric.
if "school_name" in rows.columns:
rows = rows.sort_values("school_name", key=lambda c: c.str.lower())
averages = {
m: (None if m not in rows.columns or rows[m].dropna().empty
+22 -2
View File
@@ -45,10 +45,30 @@ def test_registry_carries_a_count_per_place(client):
assert town["count"] == 6
def test_place_detail_returns_its_schools_ranked(client):
def test_place_detail_returns_its_schools_alphabetically(client):
"""A place page is read by someone looking for a school they can name.
Scanning for it is what the order should serve, so the list is A-Z.
/api/rankings is where the league-table ordering lives.
"""
body = client.get("/api/places/town/brentwood").json()
assert body["place"]["name"] == "Brentwood"
scores = [s["rwm_expected_pct"] for s in body["schools"]]
names = [s["school_name"] for s in body["schools"]]
assert names == sorted(names, key=str.lower)
def test_place_ordering_ignores_case(client):
body = client.get("/api/places/town/brentwood").json()
names = [s["school_name"] for s in body["schools"]]
# A capitalised name must not sort ahead of every lowercase one.
assert names == sorted(names, key=str.lower)
def test_the_rankings_endpoint_still_ranks_by_metric(client):
# Alphabetical is a place-page decision, not a site-wide one.
body = client.get("/api/rankings?metric=rwm_expected_pct&phase=primary").json()
scores = [r["rwm_expected_pct"] for r in body.get("rankings", [])
if r.get("rwm_expected_pct") is not None]
assert scores == sorted(scores, reverse=True)
+28
View File
@@ -1951,3 +1951,31 @@ test('a place straddling a boundary names every authority it sits in', async ({
.toBeVisible();
}
});
test('a place page lists its schools alphabetically', async ({ page }) => {
// Someone on a place page is usually looking for a school they can name,
// so the order should serve scanning for it. /rankings is where the
// league-table ordering lives.
const { places } = await (await page.request.get('/api/places')).json();
const town = places.find((p: { kind: string; count: number }) =>
p.kind === 'town' && p.count >= 5);
expect(town).toBeTruthy();
await page.goto(`/schools/${town.slug}`);
const names = await page.locator('a[href^="/school/"]').allTextContents();
expect(names.length).toBeGreaterThan(1);
const sorted = [...names].sort((a, b) =>
a.toLowerCase().localeCompare(b.toLowerCase()));
expect(names).toEqual(sorted);
});
test('the rankings page still orders by score, not name', async ({ page }) => {
// Alphabetical is a place-page decision, not a site-wide one.
const res = await page.request.get('/api/rankings?metric=rwm_expected_pct&phase=primary');
expect(res.ok()).toBeTruthy();
const scores = ((await res.json()).rankings ?? [])
.map((r: { rwm_expected_pct: number | null }) => r.rwm_expected_pct)
.filter((v: number | null) => v != null);
expect(scores).toEqual([...scores].sort((a: number, b: number) => b - a));
});
@@ -243,3 +243,36 @@ describe('PlaceView authorities', () => {
expect(screen.getByRole('link', { name: 'Merton' })).toBeInTheDocument();
});
});
describe('PlaceView list ordering', () => {
const detail3: PlaceDetail = {
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 2,
parent_authority: 'Essex', phases: ['primary'] },
schools: [
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
rwm_expected_pct: 40, attainment_8_score: null } as never,
{ urn: 2, school_name: 'Beta Primary', phase: 'Primary',
rwm_expected_pct: 90, attainment_8_score: null } as never,
],
averages: { rwm_expected_pct: 65, attainment_8_score: null },
};
it('renders schools in the order the API sent them, not by score', () => {
// The API sorts alphabetically now; the component must not re-sort.
render(<PlaceView detail={detail3} englandAverage={61} neighbours={[]} />);
const links = screen.getAllByRole('link', { name: /Primary$/ });
expect(links.map((l) => l.textContent))
.toEqual(['Alpha Primary', 'Beta Primary']);
});
it('declares the list as ascending rather than implying a ranking', () => {
// An ItemList carrying `position` reads as a ranking unless it says
// otherwise, and the table is A-Z.
const { container } = render(<PlaceView detail={detail3} englandAverage={61}
neighbours={[]} />);
const ld = JSON.parse(
container.querySelector('script[type="application/ld+json"]')!.textContent!);
const list = ld['@graph'].find((n: { '@type': string }) => n['@type'] === 'ItemList');
expect(list.itemListOrder).toBe('https://schema.org/ItemListOrderAscending');
});
});
@@ -37,10 +37,12 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
const word = phase === 'secondary' ? 'Secondary' : 'Primary';
const { name } = detail.place;
return {
title: { absolute: `${word} Schools in ${name} — Ranked | schoolcompare` },
// Not "Ranked": the table is alphabetical, so the word would be a claim
// the page does not keep.
title: { absolute: `${word} Schools in ${name} | schoolcompare` },
description:
`Every ${phase} school in ${name} ranked by results, with Ofsted grades and `
+ `the local average against England.`,
`Every ${phase} school in ${name}, with results, Ofsted grades and the local `
+ `average against England.`,
alternates: { canonical: absoluteUrl(`/schools/${slug}/${phase}`) },
};
}
+2 -2
View File
@@ -58,8 +58,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
// place title read '... | schoolcompare | schoolcompare'.
title: { absolute: `Schools in ${name} — Compare ${count} Schools | schoolcompare` },
description:
`Every school in ${name} ranked by SATs and GCSE results, with Ofsted grades, `
+ `the local average against England, and how close you had to live to get a place.`,
`Every school in ${name}, with SATs and GCSE results, Ofsted grades, the local `
+ `average against England, and how close you had to live to get a place.`,
alternates: { canonical: absoluteUrl(`/schools/${slug}`) },
};
}
@@ -45,8 +45,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
return {
title: { absolute: `Schools in ${name} — Local Authority | schoolcompare` },
description:
`All ${count} schools in the ${name} local authority, ranked by SATs and GCSE `
+ `results, with Ofsted grades and the authority average against England.`,
`All ${count} schools in the ${name} local authority, with SATs and GCSE results, `
+ `Ofsted grades and the authority average against England.`,
alternates: { canonical: absoluteUrl(`/schools/authority/${la}`) },
};
}
@@ -40,8 +40,8 @@ export async function generateMetadata({ params }: Props): Promise<Metadata> {
return {
title: { absolute: `Schools near ${name} | schoolcompare` },
description:
`${count} schools in the ${name} postcode district, ranked by results, with `
+ `Ofsted grades and how close you had to live to get a place.`,
`${count} schools in the ${name} postcode district, with results, Ofsted grades `
+ `and how close you had to live to get a place.`,
alternates: { canonical: absoluteUrl(`/schools/near/${outcode}`) },
};
}
@@ -142,6 +142,10 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
'@type': 'ItemList',
name: `${phaseWord} in ${place.name}`,
numberOfItems: schools.length,
// Alphabetical, and said so. Without this an ItemList carrying
// `position` reads as a ranking, which would be a claim the page
// stopped making when the table became A-Z.
itemListOrder: 'https://schema.org/ItemListOrderAscending',
itemListElement: schools.slice(0, 20).map((s, i) => ({
'@type': 'ListItem',
position: i + 1,