diff --git a/backend/app.py b/backend/app.py index 20d9570..2b22d81 100644 --- a/backend/app.py +++ b/backend/app.py @@ -7,6 +7,7 @@ Uses real data from UK Government Compare School Performance downloads. import hashlib import re from contextlib import asynccontextmanager +from datetime import datetime, timezone from typing import Optional import numpy as np @@ -48,11 +49,14 @@ PHASE_GROUPS: dict[str, set[str]] = { "all-through": {"all-through"}, } -BASE_URL = "https://schoolcompare.co.uk" +# Must match SITE_URL in nextjs-app/lib/site.ts. The apex 301s to www, and a +# sitemap that redirects wastes a crawl on every URL it lists. +BASE_URL = "https://www.schoolcompare.co.uk" MAX_SLUG_LENGTH = 60 -# In-memory sitemap cache -_sitemap_xml: str | None = None +# In-memory sitemap cache: name -> XML. Populated on startup and by the admin +# regenerate endpoint after a pipeline run. +_sitemaps: dict[str, str] | None = None def _slugify(text: str) -> str: @@ -70,43 +74,128 @@ def _school_url(urn: int, school_name: str) -> str: return f"/school/{urn}-{slug}" -def build_sitemap() -> str: - """Generate sitemap XML from in-memory school data. Returns the XML string.""" +# Routes worth submitting that are not a school page. /admissions was missing +# from the sitemap entirely despite being a static, indexable guide. +STATIC_SITEMAP_PATHS = ("/", "/rankings", "/compare", "/admissions") + + +def _has_publishable_data(row) -> bool: + """True when a school page has something a search result could state. + + A school with no results in any year and no Ofsted grade renders an empty + page. Submitting it spends crawl budget and drags the corpus-wide quality + signal down, so it stays out of the sitemap. The page itself still resolves + for anyone who has the URL. + """ + for field in ("rwm_expected_pct", "attainment_8_score", "ofsted_grade"): + value = row.get(field) + if value is not None and not pd.isna(value): + return True + return False + + +def _url_element(loc: str, lastmod: str | None = None) -> str: + """One entry. No priority or changefreq — Google ignores both.""" + body = f"{loc}" + if lastmod: + body += f"{lastmod}" + return f" {body}" + + +def _school_sitemap_rows(df) -> list[str]: + """A element per school that has something to show. + + lastmod comes from the school's Ofsted date where there is one and is + omitted otherwise. An always-now lastmod is a claim Google learns to + distrust; an absent one honestly means "unknown". + """ + if df.empty or "urn" not in df.columns or "school_name" not in df.columns: + return [] + + rows: list[str] = [] + seen: set[int] = set() + + # Latest row per URN first, so a school's most recent Ofsted date wins. + ordered = df.sort_values("year", ascending=False) if "year" in df.columns else df + + for _, row in ordered.iterrows(): + urn = int(row["urn"]) + if urn in seen: + continue + seen.add(urn) + if not _has_publishable_data(row): + continue + + lastmod = None + ofsted_date = row.get("ofsted_date") + if ofsted_date is not None and not pd.isna(ofsted_date): + lastmod = pd.Timestamp(ofsted_date).date().isoformat() + + rows.append(_url_element( + BASE_URL + _school_url(urn, str(row["school_name"])), lastmod)) + + return rows + + +# Sitemaps cap at 50,000 URLs per file. 10,000 keeps a child small enough to +# scan by eye in Search Console, which is the point of splitting at all: +# coverage is reported per submitted sitemap, so one file per page family is +# what makes an indexation problem attributable to a family. +SITEMAP_CHUNK_SIZE = 10_000 + +# Children are served under /sitemaps/ because Next.js only treats a whole +# bracketed path segment as dynamic — a route folder named "sitemap-[...parts]" +# is read as a literal static segment and never matches. +SITEMAP_CHILD_PREFIX = "/sitemaps" + + +def _urlset(rows: list[str]) -> str: + return "\n".join([ + '', + '', + *rows, + "", + ]) + + +def build_sitemaps() -> dict[str, str]: + """Build the sitemap index and every child, keyed by name.""" df = load_school_data() - static_urls = [ - (BASE_URL + "/", "daily", "1.0"), - (BASE_URL + "/rankings", "weekly", "0.8"), - (BASE_URL + "/compare", "weekly", "0.8"), + children: dict[str, str] = { + "static.xml": _urlset( + [_url_element(BASE_URL + path) for path in STATIC_SITEMAP_PATHS]), + } + + school_rows = _school_sitemap_rows(df) + # Always emit at least one school child, so the index shape is stable even + # on an empty database. + chunks = [school_rows[i:i + SITEMAP_CHUNK_SIZE] + for i in range(0, len(school_rows), SITEMAP_CHUNK_SIZE)] or [[]] + for n, chunk in enumerate(chunks, start=1): + children[f"schools-{n}.xml"] = _urlset(chunk) + + # On a sitemap index, lastmod means "when this sitemap file last changed", + # so generation time is the correct value here — unlike on a , where + # it would be a claim about content we cannot support. + generated = datetime.now(timezone.utc).date().isoformat() + index_rows = [ + f" {BASE_URL}{SITEMAP_CHILD_PREFIX}/{name}" + f"{generated}" + for name in children ] + index = "\n".join([ + '', + '', + *index_rows, + "", + ]) + return {**children, "sitemap.xml": index} - lines = ['', - ''] - for url, freq, priority in static_urls: - lines.append( - f" {url}" - f"{freq}" - f"{priority}" - ) - - if not df.empty and "urn" in df.columns and "school_name" in df.columns: - seen = set() - for _, row in df[["urn", "school_name"]].drop_duplicates(subset="urn").iterrows(): - urn = int(row["urn"]) - name = str(row["school_name"]) - if urn in seen: - continue - seen.add(urn) - path = _school_url(urn, name) - lines.append( - f" {BASE_URL}{path}" - f"monthly" - f"0.6" - ) - - lines.append("") - return "\n".join(lines) +def build_sitemap() -> str: + """The sitemap index. Kept for `lifespan` and the admin endpoint.""" + return build_sitemaps()["sitemap.xml"] def clean_filter_values(series: pd.Series) -> list[str]: @@ -284,7 +373,7 @@ def validate_postcode(postcode: Optional[str]) -> Optional[str]: @asynccontextmanager async def lifespan(app: FastAPI): """Application lifespan - startup and shutdown events.""" - global _sitemap_xml + global _sitemaps print("Loading school data from marts...") df = load_school_data() if df.empty: @@ -294,9 +383,9 @@ async def lifespan(app: FastAPI): # Pre-compute the latest-year snapshot so the first search request is fast await asyncio.to_thread(load_latest_school_data) try: - _sitemap_xml = build_sitemap() - n = _sitemap_xml.count("") - print(f"Sitemap built: {n} URLs.") + _sitemaps = build_sitemaps() + n = sum(x.count("") for x in _sitemaps.values()) + print(f"Sitemaps built: {len(_sitemaps)} files, {n} URLs.") except Exception as e: print(f"Warning: sitemap build failed on startup: {e}") @@ -1087,16 +1176,28 @@ async def robots_txt(): return FileResponse(settings.frontend_dir / "robots.txt", media_type="text/plain") -@app.get("/sitemap.xml") -async def sitemap_xml(): - """Serve sitemap.xml for search engine indexing.""" - global _sitemap_xml - if _sitemap_xml is None: +def _serve_sitemap(name: str) -> Response: + global _sitemaps + if _sitemaps is None: try: - _sitemap_xml = build_sitemap() + _sitemaps = build_sitemaps() except Exception as e: raise HTTPException(status_code=503, detail=f"Sitemap unavailable: {e}") - return Response(content=_sitemap_xml, media_type="application/xml") + if name not in _sitemaps: + raise HTTPException(status_code=404, detail="No such sitemap") + return Response(content=_sitemaps[name], media_type="application/xml") + + +@app.get("/sitemap.xml") +async def sitemap_xml(): + """Serve the sitemap index.""" + return _serve_sitemap("sitemap.xml") + + +@app.get("/sitemaps/{name}") +async def sitemap_child(name: str): + """Serve a child sitemap (static.xml, or schools-N.xml).""" + return _serve_sitemap(name) @app.post("/api/admin/regenerate-sitemap") @@ -1106,10 +1207,10 @@ async def regenerate_sitemap( _: bool = Depends(verify_admin_api_key), ): """Rebuild and cache the sitemap from current school data. Called by Airflow after data updates.""" - global _sitemap_xml - _sitemap_xml = build_sitemap() - n = _sitemap_xml.count("") - return {"status": "ok", "urls": n} + global _sitemaps + _sitemaps = build_sitemaps() + n = sum(x.count("") for x in _sitemaps.values()) + return {"status": "ok", "urls": n, "sitemaps": len(_sitemaps)} # Mount static files directly (must be after all routes to avoid catching API calls) diff --git a/backend/tests/test_sitemap.py b/backend/tests/test_sitemap.py new file mode 100644 index 0000000..eb030b1 --- /dev/null +++ b/backend/tests/test_sitemap.py @@ -0,0 +1,178 @@ +"""Tests for sitemap generation (spec 2026-08-20, workstream W1). + +The sitemap is built from the in-memory school DataFrame, so these inject a +small frame via monkeypatch rather than touching a database. +""" + +import numpy as np +import pandas as pd +import pytest + + +def _schools_df() -> pd.DataFrame: + """Two schools: one with results, one with neither results nor Ofsted.""" + base = { + "local_authority": "Testshire", + "school_type": "Academy", + "phase": "Primary", + "year": 202425, + "ofsted_date": None, + } + return pd.DataFrame( + [ + {**base, "urn": 100001, "school_name": "Alpha Primary", + "rwm_expected_pct": 62.0, "attainment_8_score": np.nan, + "ofsted_grade": 2.0}, + {**base, "urn": 100002, "school_name": "Ghost Primary", + "rwm_expected_pct": np.nan, "attainment_8_score": np.nan, + "ofsted_grade": np.nan}, + ] + ) + + +@pytest.fixture() +def sitemap(monkeypatch) -> str: + """The sitemap index.""" + from backend import app as app_module + + monkeypatch.setattr(app_module, "load_school_data", _schools_df) + return app_module.build_sitemap() + + +@pytest.fixture() +def schools_child(monkeypatch) -> str: + """The first school child sitemap, where school URLs actually live.""" + from backend import app as app_module + + monkeypatch.setattr(app_module, "load_school_data", _schools_df) + return app_module.build_sitemaps()["schools-1.xml"] + + +@pytest.fixture() +def static_child(monkeypatch) -> str: + from backend import app as app_module + + monkeypatch.setattr(app_module, "load_school_data", _schools_df) + return app_module.build_sitemaps()["static.xml"] + + +def test_every_loc_uses_the_www_host(sitemaps): + # The apex 301s to www. A that redirects burns a crawl per URL. + # Checked across every file, index included, not just one. + for name, xml in sitemaps.items(): + assert "https://www.schoolcompare.co.uk" in xml, name + assert "https://schoolcompare.co.uk" not in xml, name + + +def test_school_with_results_is_listed(schools_child): + assert "/school/100001-alpha-primary" in schools_child + + +def test_school_with_no_results_and_no_ofsted_is_omitted(schools_child): + # Nothing for a search result to say about it. Submitting it spends crawl + # budget and drags the corpus-wide quality signal down. + # + # Asserted against the child, not the index: the index carries no school + # URLs at all, so it would pass this trivially and prove nothing. + assert "/school/100002" not in schools_child + + +def test_no_invented_priority_or_changefreq(sitemaps): + # Google ignores both. They were noise dressed as signal. + for name, xml in sitemaps.items(): + assert "" not in xml, name + assert "" not in xml, name + + +def test_ofsted_date_becomes_lastmod(monkeypatch): + from backend import app as app_module + import datetime + + def _df(): + base = _schools_df() + base.loc[base["urn"] == 100001, "ofsted_date"] = datetime.date(2024, 3, 14) + return base + + monkeypatch.setattr(app_module, "load_school_data", _df) + xml = app_module.build_sitemaps()["schools-1.xml"] + assert "2024-03-14" in xml + + +def test_no_lastmod_invented_when_date_unknown(monkeypatch): + # An always-now lastmod is a claim Google learns to distrust. Absent + # honestly means unknown. + from backend import app as app_module + + def _df(): + df = _schools_df() + df["ofsted_date"] = None + return df + + monkeypatch.setattr(app_module, "load_school_data", _df) + # The child only. The index legitimately carries a lastmod, because there + # it means "when this sitemap file changed", which we do know. + xml = app_module.build_sitemaps()["schools-1.xml"] + assert "" not in xml + + +def test_static_routes_are_listed(static_child): + for path in ("/", "/rankings", "/compare", "/admissions"): + assert f"https://www.schoolcompare.co.uk{path}" in static_child + + +@pytest.fixture() +def sitemaps(monkeypatch) -> dict: + from backend import app as app_module + + monkeypatch.setattr(app_module, "load_school_data", _schools_df) + return app_module.build_sitemaps() + + +def test_index_lists_each_child(sitemaps): + index = sitemaps["sitemap.xml"] + assert " entries only; mixing in is invalid. + assert "" not in sitemaps["sitemap.xml"] + + +def test_index_does_not_list_itself(sitemaps): + assert "https://www.schoolcompare.co.uk/sitemap.xml" not in sitemaps["sitemap.xml"] + + +def test_static_child_holds_the_static_routes(sitemaps): + static = sitemaps["static.xml"] + for path in ("/", "/rankings", "/compare", "/admissions"): + assert f"https://www.schoolcompare.co.uk{path}" in static + + +def test_school_child_holds_the_schools(sitemaps): + assert "/school/100001-alpha-primary" in sitemaps["schools-1.xml"] + + +def test_children_are_chunked_under_the_limit(monkeypatch): + # Sitemaps cap at 50,000 URLs per file. Chunk at 10,000 so a child stays + # small enough to eyeball in Search Console. + from backend import app as app_module + import pandas as _pd + + rows = [ + {"urn": 200000 + i, "school_name": f"School {i}", "year": 202425, + "rwm_expected_pct": 60.0, "attainment_8_score": None, + "ofsted_grade": 2.0, "ofsted_date": None} + for i in range(10_001) + ] + monkeypatch.setattr(app_module, "load_school_data", lambda: _pd.DataFrame(rows)) + + maps = app_module.build_sitemaps() + assert maps["schools-1.xml"].count("") == 10_000 + assert maps["schools-2.xml"].count("") == 1 + + +def test_build_sitemap_still_returns_the_index(sitemap): + # lifespan and the admin endpoint call build_sitemap(); keep it working. + assert " { +async function sitemapChildren(page: Page): Promise { const res = await page.request.get('/sitemap.xml'); expect(res.ok()).toBeTruthy(); - const xml = await res.text(); + const index = await res.text(); + expect(index).toContain('([^<]+)<\/loc>/g)].map((m) => m[1]); +} - const urlCount = (xml.match(//g) ?? []).length; - expect(urlCount, 'sitemap looks empty or truncated').toBeGreaterThan(1000); +test('the sitemap index names children that all resolve', async ({ page }) => { + const index = await (await page.request.get('/sitemap.xml')).text(); + // An index holds entries only; mixing in is invalid. + expect(index).not.toContain(''); - // 401559 (Cardiff) and 402426 (ACT Schools, Cardiff) were both submitted - // before the England-only filter landed. - expect(xml).not.toContain('/school/401559'); - expect(xml).not.toContain('/school/402426'); + const locs = await sitemapChildren(page); + expect(locs.length).toBeGreaterThanOrEqual(2); + + for (const loc of locs) { + expect(loc.startsWith('https://www.schoolcompare.co.uk/sitemaps/')).toBeTruthy(); + const child = await page.request.get(new URL(loc).pathname); + expect(child.ok(), `${loc} should resolve`).toBeTruthy(); + expect(await child.text()).toContain(' { + const locs = await sitemapChildren(page); + + let total = 0; + for (const loc of locs) { + const xml = await (await page.request.get(new URL(loc).pathname)).text(); + total += (xml.match(//g) ?? []).length; + // 401559 (Adamsdown, Cardiff) and 402426 (ACT Schools, Cardiff) were both + // submitted before the England-only filter landed. + expect(xml).not.toContain('/school/401559'); + expect(xml).not.toContain('/school/402426'); + } + expect(total, 'sitemap looks empty or truncated').toBeGreaterThan(1000); +}); + +test('the sitemap invents no priority or changefreq', async ({ page }) => { + const [first] = await sitemapChildren(page); + expect(first).toBeTruthy(); + + const xml = await (await page.request.get(new URL(first).pathname)).text(); + // Google ignores both. They were noise dressed as signal. + expect(xml).not.toContain(''); + expect(xml).not.toContain(''); +}); + +/* + * Canonical URLs (spec 2026-08-20, W1). + * + * Every indexable route declares exactly one canonical, on the www host, with + * no query string. The homepage's eleven search params filter a result set + * rather than making a new document, so they all collapse onto "/". + */ +const CANONICAL_ROUTES: Array<[string, string]> = [ + ['/', 'https://www.schoolcompare.co.uk/'], + ['/rankings', 'https://www.schoolcompare.co.uk/rankings'], + ['/admissions', 'https://www.schoolcompare.co.uk/admissions'], +]; + +for (const [path, expected] of CANONICAL_ROUTES) { + test(`${path} declares exactly one canonical, on the www host`, async ({ page }) => { + await page.goto(path); + const hrefs = await page.locator('link[rel="canonical"]').evaluateAll( + (els) => els.map((e) => e.getAttribute('href'))); + expect(hrefs, `${path} should declare one canonical`).toHaveLength(1); + expect(hrefs[0]).toBe(expected); + }); +} + +test('a filtered homepage still canonicalises to the bare root', async ({ page }) => { + await page.goto('/?search=primary&phase=primary&sort=name&page=2'); + const href = await page.locator('link[rel="canonical"]').first() + .getAttribute('href'); + expect(href).toBe('https://www.schoolcompare.co.uk/'); +}); + +test('a school page canonicalises to its own slug on the www host', async ({ page }) => { + const res = await page.request.get('/api/schools?search=primary&per_page=1'); + expect(res.ok()).toBeTruthy(); + const [first] = (await res.json()).schools ?? []; + expect(first, 'no school available').toBeTruthy(); + + await page.goto(`/school/${first.urn}-x`); + const href = await page.locator('link[rel="canonical"]').first() + .getAttribute('href'); + expect(href).toMatch(/^https:\/\/www\.schoolcompare\.co\.uk\/school\/\d+-/); +}); + +test('a bare /compare is indexable, a parameterised one is not', async ({ page }) => { + await page.goto('/compare'); + await expect(page.locator('meta[name="robots"]')).toHaveCount(0); + + const [a, b] = await twoPrimaryUrns(page); + await page.goto(`/compare?urns=${a},${b}`); + const robots = await page.locator('meta[name="robots"]').first() + .getAttribute('content'); + expect(robots).toContain('noindex'); + expect(robots).toContain('follow'); + + // noindex but follow: the links out to each school page still count, so the + // canonical must still be present and point at the bare path. + const canonical = await page.locator('link[rel="canonical"]').first() + .getAttribute('href'); + expect(canonical).toBe('https://www.schoolcompare.co.uk/compare'); }); diff --git a/nextjs-app/__tests__/app/metadata.test.ts b/nextjs-app/__tests__/app/metadata.test.ts new file mode 100644 index 0000000..e173201 --- /dev/null +++ b/nextjs-app/__tests__/app/metadata.test.ts @@ -0,0 +1,53 @@ +import { metadata as homeMetadata } from '@/app/page'; +import { metadata as rankingsMetadata } from '@/app/rankings/page'; +import { metadata as admissionsMetadata } from '@/app/admissions/page'; +import { generateMetadata as compareMetadata } from '@/app/compare/page'; + +describe('canonical URLs', () => { + it('the homepage canonicalises to the bare root', () => { + // page.tsx reads eleven search params. Without this, every filter + // combination is a crawlable near-duplicate of the one page we want to + // rank for "compare schools". + expect(homeMetadata.alternates?.canonical) + .toBe('https://www.schoolcompare.co.uk/'); + }); + + it('rankings canonicalises to the bare path', () => { + expect(rankingsMetadata.alternates?.canonical) + .toBe('https://www.schoolcompare.co.uk/rankings'); + }); + + it('admissions canonicalises to the bare path', () => { + expect(admissionsMetadata.alternates?.canonical) + .toBe('https://www.schoolcompare.co.uk/admissions'); + }); +}); + +describe('/compare indexability', () => { + it('the bare compare page is indexable and canonical to itself', async () => { + // This is the landing page for the "compare schools" head term. + const meta = await compareMetadata({ searchParams: Promise.resolve({}) }); + expect(meta.alternates?.canonical) + .toBe('https://www.schoolcompare.co.uk/compare'); + expect(meta.robots).toBeUndefined(); + }); + + it('a comparison of specific schools is noindex, follow', async () => { + // ~317 million pairs before triples. Indexing the parameter space would + // swamp everything else in the corpus. + const meta = await compareMetadata({ + searchParams: Promise.resolve({ urns: '100001,100002' }), + }); + expect(meta.robots).toEqual({ index: false, follow: true }); + }); + + it('a parameterised comparison still canonicalises to the bare path', async () => { + // follow:true plus a canonical means the outbound links to each school + // page still pass value even though this URL is not indexed. + const meta = await compareMetadata({ + searchParams: Promise.resolve({ urns: '100001,100002' }), + }); + expect(meta.alternates?.canonical) + .toBe('https://www.schoolcompare.co.uk/compare'); + }); +}); diff --git a/nextjs-app/__tests__/lib/site.test.ts b/nextjs-app/__tests__/lib/site.test.ts new file mode 100644 index 0000000..2003959 --- /dev/null +++ b/nextjs-app/__tests__/lib/site.test.ts @@ -0,0 +1,27 @@ +import { SITE_URL, absoluteUrl } from '@/lib/site'; + +describe('SITE_URL', () => { + it('is the www host, which is the one that serves a 200', () => { + // The apex 301s to www at Cloudflare. A canonical pointing at a redirect + // is a wasted signal, so every absolute URL we emit must already be www. + expect(SITE_URL).toBe('https://www.schoolcompare.co.uk'); + }); + + it('has no trailing slash, so joins never double up', () => { + expect(SITE_URL.endsWith('/')).toBe(false); + }); +}); + +describe('absoluteUrl', () => { + it('joins a rooted path', () => { + expect(absoluteUrl('/rankings')).toBe('https://www.schoolcompare.co.uk/rankings'); + }); + + it('joins a path missing its leading slash', () => { + expect(absoluteUrl('rankings')).toBe('https://www.schoolcompare.co.uk/rankings'); + }); + + it('maps the site root to a bare trailing slash', () => { + expect(absoluteUrl('/')).toBe('https://www.schoolcompare.co.uk/'); + }); +}); diff --git a/nextjs-app/app/admissions/page.tsx b/nextjs-app/app/admissions/page.tsx index c4cad97..e3d2a67 100644 --- a/nextjs-app/app/admissions/page.tsx +++ b/nextjs-app/app/admissions/page.tsx @@ -1,3 +1,4 @@ +import { absoluteUrl } from '@/lib/site'; import type { Metadata } from 'next'; import { AdmissionsView } from '@/components/AdmissionsView'; @@ -7,6 +8,7 @@ export const metadata: Metadata = { title: 'School Admissions Guide', description: 'Understand the Primary and Secondary school admissions process in England, with live countdowns to every key deadline and National Offer Day.', + alternates: { canonical: absoluteUrl('/admissions') }, }; export default function AdmissionsPage() { diff --git a/nextjs-app/app/compare/page.tsx b/nextjs-app/app/compare/page.tsx index 0b2b4e4..2d8fb22 100644 --- a/nextjs-app/app/compare/page.tsx +++ b/nextjs-app/app/compare/page.tsx @@ -5,6 +5,7 @@ import { fetchComparison, fetchMetrics } from '@/lib/api'; import { ComparisonView } from '@/components/ComparisonView'; +import { absoluteUrl } from '@/lib/site'; import type { Metadata } from 'next'; interface ComparePageProps { @@ -14,13 +15,33 @@ interface ComparePageProps { }>; } -export const metadata: Metadata = { - title: 'Compare Schools', - description: - 'Compare schools in England side by side — Ofsted inspections, KS2 and GCSE results against the England average, admissions odds and school community.', - keywords: - 'school comparison, compare schools, Ofsted comparison, school admissions, KS2 comparison, primary school performance', -}; +/** + * Indexability depends on the query string, so this cannot be a static export. + * + * Bare /compare is the landing page for the "compare schools" head term and + * stays indexable. /compare?urns=… is an unbounded parameter space — 25,193 + * schools make ~317 million pairs — so it goes noindex. It stays `follow` and + * keeps a canonical to the bare path, so the links out to each school page + * still count. + */ +export async function generateMetadata( + { searchParams }: ComparePageProps, +): Promise { + const { urns } = await searchParams; + + const base: Metadata = { + title: 'Compare Schools', + description: + 'Compare schools in England side by side — Ofsted inspections, KS2 and GCSE results against the England average, admissions odds and school community.', + keywords: + 'school comparison, compare schools, Ofsted comparison, school admissions, KS2 comparison, primary school performance', + alternates: { canonical: absoluteUrl('/compare') }, + }; + + if (!urns) return base; + + return { ...base, robots: { index: false, follow: true } }; +} // Dynamic via searchParams; remove force-dynamic so internal data fetches // can still use Next.js's per-call revalidate cache. diff --git a/nextjs-app/app/layout.tsx b/nextjs-app/app/layout.tsx index ebf9b67..20eb8d7 100644 --- a/nextjs-app/app/layout.tsx +++ b/nextjs-app/app/layout.tsx @@ -5,6 +5,7 @@ import { Navigation } from '@/components/Navigation'; import { Footer } from '@/components/Footer'; import { ComparisonToast } from '@/components/ComparisonToast'; import { ComparisonProvider } from '@/context/ComparisonProvider'; +import { SITE_URL } from '@/lib/site'; import './globals.css'; // Manrope carries headings and key messaging — the guideline's "friendly, @@ -57,12 +58,12 @@ export const metadata: Metadata = { // No `icons` key on purpose: setting it here would override the file // conventions. app/icon.svg and app/apple-icon.tsx are the source, and // app/opengraph-image.tsx supplies og:image and twitter:image. - metadataBase: new URL('https://schoolcompare.co.uk'), + metadataBase: new URL(SITE_URL), openGraph: { type: 'website', title: 'schoolcompare | Compare School Performance', description: 'Compare primary and secondary school SATs and GCSE performance across England', - url: 'https://schoolcompare.co.uk', + url: SITE_URL, siteName: 'schoolcompare', }, twitter: { diff --git a/nextjs-app/app/page.tsx b/nextjs-app/app/page.tsx index e9d89b6..750ca6e 100644 --- a/nextjs-app/app/page.tsx +++ b/nextjs-app/app/page.tsx @@ -3,6 +3,7 @@ * Main landing page with school search and browsing */ +import { absoluteUrl } from '@/lib/site'; import type { Metadata } from 'next'; import { fetchSchools, fetchFilters, fetchDataInfo } from '@/lib/api'; import { formatAcademicYear } from '@/lib/utils'; @@ -35,6 +36,10 @@ interface HomePageProps { export const metadata: Metadata = { title: { absolute: 'schoolcompare | Compare every school in England' }, description: 'Search and compare school performance across England', + // This page reads eleven search params. They filter a result set; they do + // not make a new document. Collapsing every combination onto "/" stops the + // homepage competing with itself for its own head terms. + alternates: { canonical: absoluteUrl('/') }, }; // The page reads searchParams, which makes rendering dynamic by default. diff --git a/nextjs-app/app/rankings/page.tsx b/nextjs-app/app/rankings/page.tsx index 6078cd3..708893a 100644 --- a/nextjs-app/app/rankings/page.tsx +++ b/nextjs-app/app/rankings/page.tsx @@ -5,6 +5,7 @@ import { fetchRankings, fetchFilters, fetchMetrics } from '@/lib/api'; import { RankingsView } from '@/components/RankingsView'; +import { absoluteUrl } from '@/lib/site'; import type { Metadata } from 'next'; interface RankingsPageProps { @@ -20,6 +21,9 @@ export const metadata: Metadata = { title: 'School Rankings', description: 'Top-ranked schools by SATs and GCSE performance across England', keywords: 'school rankings, top schools, best schools, KS2 rankings, KS4 rankings, school league tables', + // Param forms (?metric=&local_authority=&year=&phase=) collapse here for + // now. W3 replaces them with real indexable paths. + alternates: { canonical: absoluteUrl('/rankings') }, }; // Dynamic via searchParams; remove force-dynamic so internal data fetches diff --git a/nextjs-app/app/robots.ts b/nextjs-app/app/robots.ts index 8ab3bbc..e2cb38e 100644 --- a/nextjs-app/app/robots.ts +++ b/nextjs-app/app/robots.ts @@ -4,6 +4,7 @@ */ import { MetadataRoute } from 'next'; +import { absoluteUrl } from '@/lib/site'; export default function robots(): MetadataRoute.Robots { return { @@ -14,6 +15,6 @@ export default function robots(): MetadataRoute.Robots { disallow: ['/api/', '/_next/'], }, ], - sitemap: 'https://schoolcompare.co.uk/sitemap.xml', + sitemap: absoluteUrl('/sitemap.xml'), }; } diff --git a/nextjs-app/app/school/[slug]/page.tsx b/nextjs-app/app/school/[slug]/page.tsx index db2c229..4e9f6aa 100644 --- a/nextjs-app/app/school/[slug]/page.tsx +++ b/nextjs-app/app/school/[slug]/page.tsx @@ -15,6 +15,7 @@ import { } from '@/lib/schoolSections'; import { parseSchoolSlug, schoolUrl } from '@/lib/utils'; import type { NationalAverages } from '@/lib/types'; +import { absoluteUrl } from '@/lib/site'; import type { Metadata } from 'next'; /** @@ -97,7 +98,7 @@ export async function generateMetadata({ params }: SchoolPageProps): Promise }, +) { + const { parts } = await params; + const name = parts.join('/'); + if (!CHILD.test(name)) { + return new NextResponse('Not found', { status: 404 }); + } + return proxySitemap(`/sitemaps/${name}`); +} diff --git a/nextjs-app/lib/site.ts b/nextjs-app/lib/site.ts new file mode 100644 index 0000000..3d8c356 --- /dev/null +++ b/nextjs-app/lib/site.ts @@ -0,0 +1,19 @@ +/** + * The one place the site's absolute origin is written down. + * + * The apex domain 301s to www at Cloudflare, so www is the host that actually + * serves a 200. Canonicals, og:url, sitemap entries and the robots.txt + * Sitemap: line must all agree with it — a canonical pointing at a redirect + * makes Google resolve the hop before it can consolidate the signal. + * + * backend/app.py holds the same value as BASE_URL for the sitemap. The two are + * asserted against each other by the e2e journeys rather than shared at build + * time, because the backend and frontend ship as separate images. + */ +export const SITE_URL = 'https://www.schoolcompare.co.uk'; + +/** Absolute URL for a site-relative path. Tolerates a missing leading slash. */ +export function absoluteUrl(path: string): string { + const rooted = path.startsWith('/') ? path : `/${path}`; + return `${SITE_URL}${rooted}`; +} diff --git a/nextjs-app/lib/sitemapProxy.ts b/nextjs-app/lib/sitemapProxy.ts new file mode 100644 index 0000000..dab6359 --- /dev/null +++ b/nextjs-app/lib/sitemapProxy.ts @@ -0,0 +1,29 @@ +/** + * Runtime proxy for the sitemap family → the FastAPI backend. + * + * Like the /api/* proxy, this reads FASTAPI_URL at request time rather than + * baking the backend host into the build, so one image works in every + * environment. robots.ts points crawlers at /sitemap.xml, which is the index; + * the index names children under /sitemaps/, which land on the same proxy. + */ +import { NextResponse } from 'next/server'; + +function backendOrigin(): string { + const base = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000/api'; + return base.replace(/\/api$/, ''); +} + +export async function proxySitemap(path: string): Promise { + let upstream: Response; + try { + upstream = await fetch(`${backendOrigin()}${path}`, { cache: 'no-store' }); + } catch { + return new NextResponse('Sitemap temporarily unavailable', { status: 502 }); + } + + const body = await upstream.text(); + return new NextResponse(body, { + status: upstream.status, + headers: { 'content-type': upstream.headers.get('content-type') || 'application/xml' }, + }); +}