Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
dbb74d9b60 |
No files matched your search
@@ -1232,13 +1232,6 @@ async def get_place(request: Request, kind: str, slug: str,
|
|||||||
"place": {"kind": place.kind, "slug": place.slug, "name": place.name,
|
"place": {"kind": place.kind, "slug": place.slug, "name": place.name,
|
||||||
"count": len(place.urns),
|
"count": len(place.urns),
|
||||||
"parent_authority": place.parent_authority,
|
"parent_authority": place.parent_authority,
|
||||||
# Every authority the place meaningfully sits in. SW19 is
|
|
||||||
# mostly Merton but partly Wandsworth; naming one asserts
|
|
||||||
# something false.
|
|
||||||
"authorities": [
|
|
||||||
{"name": name, "slug": _slugify(name), "count": n}
|
|
||||||
for name, n in place.authorities
|
|
||||||
],
|
|
||||||
# Only phases that clear the threshold, so the page links
|
# Only phases that clear the threshold, so the page links
|
||||||
# variants that exist rather than 404s.
|
# variants that exist rather than 404s.
|
||||||
"phases": [ph for ph in ("primary", "secondary")
|
"phases": [ph for ph in ("primary", "secondary")
|
||||||
|
|||||||
@@ -30,12 +30,6 @@ class Place:
|
|||||||
name: str
|
name: str
|
||||||
urns: tuple[int, ...]
|
urns: tuple[int, ...]
|
||||||
parent_authority: str | None # authority NAME, for the 301 target
|
parent_authority: str | None # authority NAME, for the 301 target
|
||||||
# Every authority the place meaningfully sits in, largest first. A quarter
|
|
||||||
# of outcodes and a third of towns straddle a boundary — SW19 is mostly
|
|
||||||
# Merton but partly Wandsworth — so naming only one asserts something
|
|
||||||
# false. parent_authority stays single because a redirect needs one
|
|
||||||
# target; this is what the page shows.
|
|
||||||
authorities: tuple[tuple[str, int], ...] = ()
|
|
||||||
# URNs per phase, so the per-phase threshold can be applied without
|
# URNs per phase, so the per-phase threshold can be applied without
|
||||||
# re-querying. A place with 30 primaries and 2 secondaries publishes a
|
# re-querying. A place with 30 primaries and 2 secondaries publishes a
|
||||||
# primary variant and no secondary one.
|
# primary variant and no secondary one.
|
||||||
@@ -77,42 +71,6 @@ def _phase_urns(group, publishable: set[int]) -> dict[str, tuple[int, ...]]:
|
|||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
# A place is described by an authority when it holds at least a tenth of the
|
|
||||||
# schools, and at least two. GIAS carries occasional postcode errors — EN6
|
|
||||||
# lists two Shropshire schools among fourteen in Hertfordshire — and a bare
|
|
||||||
# "any authority present" rule would print those as though they were real.
|
|
||||||
_AUTHORITY_MIN_SHARE = 0.10
|
|
||||||
_AUTHORITY_MIN_SCHOOLS = 2
|
|
||||||
_AUTHORITY_MAX_SHOWN = 3
|
|
||||||
|
|
||||||
|
|
||||||
def _authorities(group) -> tuple[tuple[str, int], ...]:
|
|
||||||
"""Authorities this place meaningfully sits in, largest first."""
|
|
||||||
from backend.app import EXCLUDED_FILTER_VALUES
|
|
||||||
|
|
||||||
if "local_authority" not in group.columns:
|
|
||||||
return ()
|
|
||||||
counts = group["local_authority"].dropna().value_counts()
|
|
||||||
total = int(counts.sum())
|
|
||||||
if not total:
|
|
||||||
return ()
|
|
||||||
|
|
||||||
kept = [
|
|
||||||
(str(name), int(n)) for name, n in counts.items()
|
|
||||||
if str(name) not in EXCLUDED_FILTER_VALUES
|
|
||||||
and n >= _AUTHORITY_MIN_SCHOOLS
|
|
||||||
and n / total >= _AUTHORITY_MIN_SHARE
|
|
||||||
]
|
|
||||||
# A place too small or too fragmented for the share rule still names its
|
|
||||||
# largest authority, or the page would say nothing about where it is.
|
|
||||||
if not kept:
|
|
||||||
for name, n in counts.items():
|
|
||||||
if str(name) not in EXCLUDED_FILTER_VALUES:
|
|
||||||
return ((str(name), int(n)),)
|
|
||||||
return ()
|
|
||||||
return tuple(kept[:_AUTHORITY_MAX_SHOWN])
|
|
||||||
|
|
||||||
|
|
||||||
def _parent_authority(group) -> str | None:
|
def _parent_authority(group) -> str | None:
|
||||||
"""The most common authority in a group — the useful 301 target.
|
"""The most common authority in a group — the useful 301 target.
|
||||||
|
|
||||||
@@ -146,7 +104,6 @@ def _group(df, column: str, kind: str, publishable: set[int]) -> dict[str, Place
|
|||||||
place = Place(
|
place = Place(
|
||||||
kind=kind, slug=slug, name=name, urns=urns,
|
kind=kind, slug=slug, name=name, urns=urns,
|
||||||
parent_authority=_parent_authority(group) if kind == "town" else None,
|
parent_authority=_parent_authority(group) if kind == "town" else None,
|
||||||
authorities=() if kind == "authority" else _authorities(group),
|
|
||||||
phase_urns=_phase_urns(group, publishable),
|
phase_urns=_phase_urns(group, publishable),
|
||||||
)
|
)
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
@@ -181,7 +138,6 @@ def _outcode_places(df, publishable: set[int]) -> dict[str, Place]:
|
|||||||
continue
|
continue
|
||||||
place = Place(kind="outcode", slug=str(oc).lower(), name=str(oc),
|
place = Place(kind="outcode", slug=str(oc).lower(), name=str(oc),
|
||||||
urns=urns, parent_authority=_parent_authority(group),
|
urns=urns, parent_authority=_parent_authority(group),
|
||||||
authorities=_authorities(group),
|
|
||||||
phase_urns=_phase_urns(group, publishable))
|
phase_urns=_phase_urns(group, publishable))
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
@@ -225,7 +181,6 @@ def _locality_places(df, publishable: set[int],
|
|||||||
continue
|
continue
|
||||||
place = Place(kind="locality", slug=slug, name=name, urns=urns,
|
place = Place(kind="locality", slug=slug, name=name, urns=urns,
|
||||||
parent_authority=_parent_authority(group),
|
parent_authority=_parent_authority(group),
|
||||||
authorities=_authorities(group),
|
|
||||||
phase_urns=_phase_urns(group, publishable))
|
phase_urns=_phase_urns(group, publishable))
|
||||||
out[place.key] = place
|
out[place.key] = place
|
||||||
return out
|
return out
|
||||||
|
|||||||
@@ -229,64 +229,3 @@ def test_no_curated_locality_names_a_london_borough():
|
|||||||
f"these are boroughs, not districts: {sorted(named)} - they already "
|
f"these are boroughs, not districts: {sorted(named)} - they already "
|
||||||
"have an authority page covering every school"
|
"have an authority page covering every school"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_a_place_names_every_authority_it_straddles():
|
|
||||||
"""SW19 is mostly Merton but partly Wandsworth.
|
|
||||||
|
|
||||||
A quarter of viable outcodes and a third of viable towns cross an
|
|
||||||
authority boundary, so naming only the largest asserts something false.
|
|
||||||
"""
|
|
||||||
rows = (_town(26, "London", "Merton", start=300000)
|
|
||||||
+ _town(7, "London", "Wandsworth", start=400000))
|
|
||||||
for r in rows:
|
|
||||||
r["postcode"] = "SW19 1AA"
|
|
||||||
reg = build_place_registry(_df(rows))
|
|
||||||
|
|
||||||
names = [n for n, _ in reg["outcode:sw19"].authorities]
|
|
||||||
assert names == ["Merton", "Wandsworth"] # largest first
|
|
||||||
assert dict(reg["outcode:sw19"].authorities)["Wandsworth"] == 7
|
|
||||||
|
|
||||||
|
|
||||||
def test_the_redirect_target_stays_a_single_authority():
|
|
||||||
# parent_authority and authorities do different jobs: a 301 needs one
|
|
||||||
# target, the page needs the truth.
|
|
||||||
rows = (_town(26, "London", "Merton", start=300000)
|
|
||||||
+ _town(7, "London", "Wandsworth", start=400000))
|
|
||||||
for r in rows:
|
|
||||||
r["postcode"] = "SW19 1AA"
|
|
||||||
reg = build_place_registry(_df(rows))
|
|
||||||
assert reg["outcode:sw19"].parent_authority == "Merton"
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_stray_authority_below_the_share_threshold_is_not_named():
|
|
||||||
# GIAS carries postcode errors — EN6 lists two Shropshire schools among
|
|
||||||
# fourteen in Hertfordshire. Printing those as though real would be worse
|
|
||||||
# than omitting them.
|
|
||||||
rows = (_town(30, "Barnet", "Hertfordshire", start=300000)
|
|
||||||
+ _town(1, "Barnet", "Shropshire", start=400000))
|
|
||||||
for r in rows:
|
|
||||||
r["postcode"] = "EN6 1AA"
|
|
||||||
reg = build_place_registry(_df(rows))
|
|
||||||
assert [n for n, _ in reg["outcode:en6"].authorities] == ["Hertfordshire"]
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_sentinel_authority_is_never_named():
|
|
||||||
rows = (_town(20, "London", "Merton", start=300000)
|
|
||||||
+ _town(6, "London", "Does not apply", start=400000))
|
|
||||||
for r in rows:
|
|
||||||
r["postcode"] = "SW19 1AA"
|
|
||||||
reg = build_place_registry(_df(rows))
|
|
||||||
assert [n for n, _ in reg["outcode:sw19"].authorities] == ["Merton"]
|
|
||||||
|
|
||||||
|
|
||||||
def test_a_place_always_names_at_least_one_authority():
|
|
||||||
# Even when every authority is below the share threshold, the page has to
|
|
||||||
# say where the place is.
|
|
||||||
rows = []
|
|
||||||
for i, la in enumerate(["A", "B", "C", "D", "E", "F", "G"]):
|
|
||||||
rows += _town(1, "Fragmented", la, start=300000 + i * 100)
|
|
||||||
reg = build_place_registry(_df(rows))
|
|
||||||
place = reg.get("town:fragmented")
|
|
||||||
assert place is not None
|
|
||||||
assert len(place.authorities) == 1
|
|
||||||
@@ -1885,29 +1885,3 @@ test('no page title repeats the brand', async ({ page }) => {
|
|||||||
expect(brands, `${path} repeats the brand: ${title}`).toBeLessThanOrEqual(1);
|
expect(brands, `${path} repeats the brand: ${title}`).toBeLessThanOrEqual(1);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
test('a place straddling a boundary names every authority it sits in', async ({ page }) => {
|
|
||||||
// A quarter of outcodes and a third of towns cross an authority boundary —
|
|
||||||
// SW19 is mostly Merton but partly Wandsworth. Naming only the largest
|
|
||||||
// asserts something false about the place.
|
|
||||||
const { places } = await (await page.request.get('/api/places')).json();
|
|
||||||
const outcode = places.find((p: { kind: string }) => p.kind === 'outcode');
|
|
||||||
expect(outcode).toBeTruthy();
|
|
||||||
|
|
||||||
// Find any place the registry reports as straddling.
|
|
||||||
let straddling: { kind: string; slug: string } | null = null;
|
|
||||||
for (const p of places.filter((p: { kind: string }) => p.kind === 'outcode').slice(0, 40)) {
|
|
||||||
const d = await (await page.request.get(`/api/places/outcode/${p.slug}`)).json();
|
|
||||||
if ((d.place.authorities ?? []).length > 1) { straddling = p; break; }
|
|
||||||
}
|
|
||||||
test.skip(!straddling, 'no straddling outcode found in the sample');
|
|
||||||
|
|
||||||
const detail = await (await page.request.get(
|
|
||||||
`/api/places/outcode/${straddling!.slug}`)).json();
|
|
||||||
await page.goto(`/schools/near/${straddling!.slug}`);
|
|
||||||
|
|
||||||
for (const a of detail.place.authorities) {
|
|
||||||
await expect(page.locator(`a[href="/schools/authority/${a.slug}"]`).first())
|
|
||||||
.toBeVisible();
|
|
||||||
}
|
|
||||||
});
|
|
||||||
@@ -169,14 +169,10 @@ describe('PlaceView presentation', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('PlaceView authorities', () => {
|
describe('PlaceView table alignment', () => {
|
||||||
const straddling: PlaceDetail = {
|
const aligned: PlaceDetail = {
|
||||||
place: { kind: 'outcode', slug: 'sw19', name: 'SW19', count: 33,
|
place: { kind: 'town', slug: 'brentwood', name: 'Brentwood', count: 2,
|
||||||
parent_authority: 'Merton', phases: ['primary'],
|
parent_authority: 'Essex', phases: ['primary'] },
|
||||||
authorities: [
|
|
||||||
{ name: 'Merton', slug: 'merton', count: 26 },
|
|
||||||
{ name: 'Wandsworth', slug: 'wandsworth', count: 7 },
|
|
||||||
] },
|
|
||||||
schools: [
|
schools: [
|
||||||
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
|
{ urn: 1, school_name: 'Alpha Primary', phase: 'Primary',
|
||||||
rwm_expected_pct: 82, attainment_8_score: null } as never,
|
rwm_expected_pct: 82, attainment_8_score: null } as never,
|
||||||
@@ -184,31 +180,22 @@ describe('PlaceView authorities', () => {
|
|||||||
averages: { rwm_expected_pct: 63, attainment_8_score: null },
|
averages: { rwm_expected_pct: 63, attainment_8_score: null },
|
||||||
};
|
};
|
||||||
|
|
||||||
it('names every authority the place straddles, not just the largest', () => {
|
it('aligns the measure heading and its values with the same class', () => {
|
||||||
// SW19 is mostly Merton but partly Wandsworth. Naming one asserts
|
// They were aligned by two different selectors whose specificity did not
|
||||||
// something false about a quarter of outcodes.
|
// match: `.table th:last-child` (0,2,1) won and went right, while `.num`
|
||||||
render(<PlaceView detail={straddling} englandAverage={61} neighbours={[]} />);
|
// (0,1,0) lost to `.table td` (0,1,1) and stayed left. Sharing one class
|
||||||
expect(screen.getByRole('link', { name: 'Merton' }))
|
// is what makes them impossible to drift apart.
|
||||||
.toHaveAttribute('href', '/schools/authority/merton');
|
const { container } = render(<PlaceView detail={aligned} englandAverage={61}
|
||||||
expect(screen.getByRole('link', { name: 'Wandsworth' }))
|
neighbours={[]} />);
|
||||||
.toHaveAttribute('href', '/schools/authority/wandsworth');
|
const th = container.querySelectorAll('th')[1];
|
||||||
|
const td = container.querySelectorAll('tbody td')[1];
|
||||||
|
expect(th.className).toBeTruthy();
|
||||||
|
expect(td.className).toBe(th.className);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('joins them readably rather than as a bare list', () => {
|
it('leaves the school-name column unclassed so it takes the spare width', () => {
|
||||||
// Asserted on the summary line's whole text: a loose /and/ matcher also
|
const { container } = render(<PlaceView detail={aligned} englandAverage={61}
|
||||||
// hits "Wandsworth".
|
neighbours={[]} />);
|
||||||
const { container } = render(<PlaceView detail={straddling}
|
expect(container.querySelectorAll('th')[0].className).toBe('');
|
||||||
englandAverage={61} neighbours={[]} />);
|
|
||||||
const summary = container.querySelector('header p');
|
|
||||||
expect(summary?.textContent).toContain('Merton and Wandsworth');
|
|
||||||
});
|
|
||||||
|
|
||||||
it('falls back to the single parent when the field is absent', () => {
|
|
||||||
// A cached API response predating the authorities field must not blank
|
|
||||||
// the line entirely.
|
|
||||||
const legacy = { ...straddling,
|
|
||||||
place: { ...straddling.place, authorities: undefined } };
|
|
||||||
render(<PlaceView detail={legacy} englandAverage={61} neighbours={[]} />);
|
|
||||||
expect(screen.getByRole('link', { name: 'Merton' })).toBeInTheDocument();
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -144,10 +144,24 @@
|
|||||||
border-bottom: none;
|
border-bottom: none;
|
||||||
}
|
}
|
||||||
|
|
||||||
.table th:last-child,
|
/*
|
||||||
.num {
|
* Header and value share one class and one rule, so they cannot drift apart.
|
||||||
|
*
|
||||||
|
* The first cut aligned them with two different selectors: `.table th:last-child`
|
||||||
|
* at (0,2,1) beat the element rule and went right, while `.num` at (0,1,0) lost
|
||||||
|
* to `.table td` at (0,1,1) and stayed left. The heading and its numbers sat on
|
||||||
|
* opposite edges of the column.
|
||||||
|
*
|
||||||
|
* width:1% with nowrap makes the measure column hug its content so the school
|
||||||
|
* name takes the remaining width — without it the two columns split evenly and
|
||||||
|
* the gap between heading and value reads as misalignment on a wide screen.
|
||||||
|
*/
|
||||||
|
.table th.num,
|
||||||
|
.table td.num {
|
||||||
text-align: right;
|
text-align: right;
|
||||||
font-variant-numeric: tabular-nums;
|
font-variant-numeric: tabular-nums;
|
||||||
|
width: 1%;
|
||||||
|
white-space: nowrap;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* The measure is spelled out; the tooltip carries the definition. */
|
/* The measure is spelled out; the tooltip carries the definition. */
|
||||||
|
|||||||
@@ -71,7 +71,9 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
|
|||||||
<thead>
|
<thead>
|
||||||
<tr>
|
<tr>
|
||||||
<th scope="col">School</th>
|
<th scope="col">School</th>
|
||||||
<th scope="col">
|
{/* Same class as the value cell below: one rule aligns both, so
|
||||||
|
they cannot drift apart. */}
|
||||||
|
<th scope="col" className={styles.num}>
|
||||||
<abbr className={styles.metricHead} title={metric.hint}>
|
<abbr className={styles.metricHead} title={metric.hint}>
|
||||||
{metric.heading}
|
{metric.heading}
|
||||||
</abbr>
|
</abbr>
|
||||||
@@ -104,13 +106,6 @@ function SchoolTable({ schools, phase }: { schools: School[]; phase: PhaseKey })
|
|||||||
|
|
||||||
export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {
|
export function PlaceView({ detail, phase, englandAverage, neighbours }: Props) {
|
||||||
const { place, schools, averages } = detail;
|
const { place, schools, averages } = detail;
|
||||||
// Fall back to the single parent when the API predates the authorities
|
|
||||||
// field, so a stale cache never blanks the line entirely.
|
|
||||||
const authorities = place.authorities?.length
|
|
||||||
? place.authorities
|
|
||||||
: place.parent_authority
|
|
||||||
? [{ name: place.parent_authority, slug: authoritySlug(place.parent_authority), count: 0 }]
|
|
||||||
: [];
|
|
||||||
const local = averages[METRICS[phase ?? 'primary'].key];
|
const local = averages[METRICS[phase ?? 'primary'].key];
|
||||||
const phaseWord = phase === 'secondary' ? 'Secondary schools'
|
const phaseWord = phase === 'secondary' ? 'Secondary schools'
|
||||||
: phase === 'primary' ? 'Primary schools' : 'Schools';
|
: phase === 'primary' ? 'Primary schools' : 'Schools';
|
||||||
@@ -168,20 +163,13 @@ export function PlaceView({ detail, phase, englandAverage, neighbours }: Props)
|
|||||||
<h1>{phaseWord} in {place.name}</h1>
|
<h1>{phaseWord} in {place.name}</h1>
|
||||||
<p className={styles.summary}>
|
<p className={styles.summary}>
|
||||||
{place.count} schools
|
{place.count} schools
|
||||||
{authorities.length > 0 && (
|
{place.parent_authority && (
|
||||||
<>
|
<>
|
||||||
{' · '}
|
{' · '}
|
||||||
{/* Every authority, not just the largest. A quarter of outcodes
|
<Link href={`/schools/authority/${authoritySlug(place.parent_authority)}`}
|
||||||
and a third of towns cross a boundary: SW19 is mostly Merton
|
className={styles.inlineLink}>
|
||||||
but partly Wandsworth, and naming one asserts otherwise. */}
|
{place.parent_authority}
|
||||||
{authorities.map((a, i) => (
|
</Link>
|
||||||
<span key={a.slug}>
|
|
||||||
{i > 0 && (i === authorities.length - 1 ? ' and ' : ', ')}
|
|
||||||
<Link href={`/schools/authority/${a.slug}`} className={styles.inlineLink}>
|
|
||||||
{a.name}
|
|
||||||
</Link>
|
|
||||||
</span>
|
|
||||||
))}
|
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
</p>
|
</p>
|
||||||
|
|||||||
@@ -18,19 +18,8 @@ export interface PlaceSummary {
|
|||||||
phases?: string[];
|
phases?: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface PlaceAuthority {
|
|
||||||
name: string;
|
|
||||||
slug: string;
|
|
||||||
count: number;
|
|
||||||
}
|
|
||||||
|
|
||||||
export interface PlaceDetail {
|
export interface PlaceDetail {
|
||||||
place: PlaceSummary & {
|
place: PlaceSummary & { parent_authority: string | null };
|
||||||
parent_authority: string | null;
|
|
||||||
/** Every authority the place meaningfully sits in, largest first. SW19 is
|
|
||||||
* mostly Merton but partly Wandsworth. */
|
|
||||||
authorities?: PlaceAuthority[];
|
|
||||||
};
|
|
||||||
schools: School[];
|
schools: School[];
|
||||||
averages: {
|
averages: {
|
||||||
rwm_expected_pct: number | null;
|
rwm_expected_pct: number | null;
|
||||||
|
|||||||
Reference in new issue
Block a user