From 6fc7fce94840b738b70743c09bc3397f5de1ed2b Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 8 Sep 2026 17:12:25 +0100 Subject: [PATCH] feat(flags): put /about and /blog behind flags, dark by default Both features ship dark. Neither is reachable in an environment where its flag is off, and every flag in this system starts off, so a deploy of this commit makes both disappear until someone turns them on deliberately. Two independent flags rather than one, which makes blog-on-about-off a reachable state. That state is the whole reason the change is larger than four notFound() calls: the blog leans on the About page for its author identity. The Person entity is anchored at /about#tudor, and that URL 404s while about_page is dark, so a post published in that state would claim an author resolving to nothing. Worse than having no named author. Both bylines fall back to unlinked text and the BlogPosting attributes to the publisher instead, so every combination of the two flags renders something correct. Gated: /about, /blog, /blog/[slug], the RSS feed, both footer links, and the matching content-sitemap entries. A sitemap must never advertise a URL that 404s. With both dark it emits a valid empty urlset rather than a 404, because robots.txt names it unconditionally. Not gated: /admin. Posts have to be writable before the blog is worth switching on, so flagging the panel would make the flag unflippable. getFlags takes a revalidate rather than always using the 300s constant. Reading a flag pins the calling route to the lowest revalidate among its fetches, and the footer links live in the root layout, so a naive gate there would have dropped every school and place page from a weekly cache to a 5-minute one. The layout passes 604800, the floor those routes already declare, and the build confirms all four SSG route families still prerender. The cost is one-way latency: pages follow a flip in minutes, footer links within a week. The e2e journeys follow the existing paired shape from the admission_distance flag: a lit journey and a dark one for each flag, reading state from whether /about and /blog respond rather than from /api/flags, which another journey asserts is not publicly reachable. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DXnXQKnPpZBBP61fBQiFkq --- backend/flags.py | 17 ++++++ e2e/tests/journeys.spec.ts | 57 ++++++++++++++++++- nextjs-app/__tests__/app/blogMetadata.test.ts | 17 +++++- .../__tests__/components/Footer.test.tsx | 47 +++++++++++++++ nextjs-app/__tests__/lib/flags.test.ts | 26 ++++++++- nextjs-app/app/(frontend)/about/page.tsx | 15 ++++- .../app/(frontend)/blog/[slug]/page.tsx | 24 +++++++- nextjs-app/app/(frontend)/blog/page.tsx | 11 +++- .../app/(frontend)/blog/rss.xml/route.ts | 6 ++ .../(frontend)/content-sitemap.xml/route.ts | 36 ++++++++---- nextjs-app/app/(frontend)/layout.tsx | 25 +++++++- nextjs-app/components/Footer.tsx | 43 ++++++++++---- nextjs-app/docs/PUBLISHING.md | 20 +++++++ nextjs-app/lib/flags.ts | 16 +++++- nextjs-app/lib/jsonld.ts | 16 +++++- 15 files changed, 338 insertions(+), 38 deletions(-) create mode 100644 nextjs-app/__tests__/components/Footer.test.tsx diff --git a/backend/flags.py b/backend/flags.py index a06e93e..663ade9 100644 --- a/backend/flags.py +++ b/backend/flags.py @@ -57,6 +57,23 @@ REGISTRY: dict[str, Flag] = { ), added=date(2026, 8, 26), ), + Flag( + name="about_page", + description=( + "The /about page, its footer link, its sitemap entry, and the " + "named-author byline on every blog post." + ), + added=date(2026, 9, 8), + ), + Flag( + name="blog", + description=( + "The /blog index, post pages, the RSS feed, their footer link " + "and their sitemap entries. Not /admin: posts must be " + "writable before the blog is readable." + ), + added=date(2026, 9, 8), + ), ) } diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 314fc07..cfb614f 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -2560,8 +2560,56 @@ test('the destinations section never claims a pupil stayed at this school', asyn * These journeys assert the load-bearing parts of that — a name, a face, the * honesty claim, and a resolvable Person entity — rather than exact copy, * which will be edited. + * + * Both are behind flags (about_page, blog), so each has a lit journey and a + * dark one. Flag state is read from the observable effect rather than from + * /api/flags, which the public proxy denies on purpose — the same approach + * distanceFeatureIsOn() takes above. */ +async function aboutPageIsOn(page: Page): Promise { + return (await page.request.get('/about')).ok(); +} + +async function blogIsOn(page: Page): Promise { + return (await page.request.get('/blog')).ok(); +} + +test('with the about page off, it is absent rather than empty', async ({ page }) => { + test.skip(await aboutPageIsOn(page), 'the about_page flag is on in this environment'); + + // Dark means the URL does not exist, not that it renders empty: a 404 is + // what stops a crawler keeping the page in its index. + expect((await page.request.get('/about')).status()).toBe(404); + + // A footer link into a 404 is the failure this flag has to avoid. + await page.goto('/'); + await expect(page.locator('footer a[href="/about"]')).toHaveCount(0); + + // And a sitemap must never advertise a URL that 404s. + const sitemap = await page.request.get('/content-sitemap.xml'); + expect(await sitemap.text()).not.toContain('/about'); +}); + +test('with the blog off, it is absent rather than empty', async ({ page }) => { + test.skip(await blogIsOn(page), 'the blog flag is on in this environment'); + + expect((await page.request.get('/blog')).status()).toBe(404); + expect((await page.request.get('/blog/rss.xml')).status()).toBe(404); + + await page.goto('/'); + await expect(page.locator('footer a[href="/blog"]')).toHaveCount(0); + + const sitemap = await page.request.get('/content-sitemap.xml'); + expect(await sitemap.text()).not.toContain('/blog'); + + // The admin panel is deliberately NOT flagged: posts have to be writable + // before the blog is readable, or there is nothing to turn on. + expect((await page.request.get('/admin')).status()).not.toBe(404); +}); + test('the about page names a human author and is reachable from the footer', async ({ page }) => { + test.skip(!(await aboutPageIsOn(page)), 'the about_page flag is off in this environment'); + await page.goto('/'); const aboutLink = page.locator('footer a[href="/about"]'); await expect(aboutLink).toBeVisible(); @@ -2585,6 +2633,8 @@ test('the about page names a human author and is reachable from the footer', asy }); test('the blog lists posts and each one renders with a byline', async ({ page }) => { + test.skip(!(await blogIsOn(page)), 'the blog flag is off in this environment'); + await page.goto('/blog'); await expect(page.getByRole('heading', { level: 1 })).toBeVisible(); @@ -2612,8 +2662,13 @@ test('the admin panel is not indexable', async ({ page }) => { test('the content sitemap lists the about page and is advertised in robots', async ({ page }) => { const sitemap = await page.request.get('/content-sitemap.xml'); + // Served whatever the flags say: robots.txt names it unconditionally, and + // with both dark it is a valid empty urlset rather than a 404. expect(sitemap.ok()).toBeTruthy(); - expect(await sitemap.text()).toContain('/about'); + + if (await aboutPageIsOn(page)) { + expect(await sitemap.text()).toContain('/about'); + } // The school corpus sitemap is proxied from FastAPI; this one is Next's. // robots.txt must advertise both or the blog never gets discovered. diff --git a/nextjs-app/__tests__/app/blogMetadata.test.ts b/nextjs-app/__tests__/app/blogMetadata.test.ts index ac3ee41..1f74c7f 100644 --- a/nextjs-app/__tests__/app/blogMetadata.test.ts +++ b/nextjs-app/__tests__/app/blogMetadata.test.ts @@ -27,14 +27,27 @@ describe('BlogPosting structured data', () => { it('names the same Person entity the about page declares', () => { // By @id, not by repeating the person: search engines must resolve every // post and the about page to one author entity, or the site has several. - const ld = blogPostingJsonLd(post); + const ld = blogPostingJsonLd(post, { namedAuthor: true }); expect(ld['@type']).toBe('BlogPosting'); expect(ld.author['@id']).toBe('https://www.schoolcompare.co.uk/about#tudor'); expect(ld.publisher['@id']).toBe('https://www.schoolcompare.co.uk#organization'); }); + it('attributes to the organization when the about page is dark', () => { + /* + * The two flags are independent, so blog-on-about-off is a reachable + * state. The Person entity lives at /about#tudor and that URL 404s while + * the flag is dark, so claiming it would declare an author that resolves + * to nothing — worse for the blog's credibility than having no named + * author at all. Attribute to the publisher instead. + */ + const ld = blogPostingJsonLd(post, { namedAuthor: false }); + expect(ld.author['@id']).toBe('https://www.schoolcompare.co.uk#organization'); + expect(JSON.stringify(ld)).not.toContain('/about'); + }); + it('carries a self-referencing canonical url and the publish date', () => { - const ld = blogPostingJsonLd(post); + const ld = blogPostingJsonLd(post, { namedAuthor: true }); expect(ld.url).toBe( 'https://www.schoolcompare.co.uk/blog/what-the-data-cannot-tell-you', ); diff --git a/nextjs-app/__tests__/components/Footer.test.tsx b/nextjs-app/__tests__/components/Footer.test.tsx new file mode 100644 index 0000000..6c7b95d --- /dev/null +++ b/nextjs-app/__tests__/components/Footer.test.tsx @@ -0,0 +1,47 @@ +/** + * The footer is the only navigational route to /about and /blog, so it is + * where a dark flag would otherwise leave a link into a 404. + * + * Both props default to false. A caller that forgets to pass them hides the + * links, which is the direction that cannot break a page — the same reasoning + * as backend/flags.py's "every flag defaults to False". + */ +import { render, screen } from '@testing-library/react'; +import { Footer } from '@/components/Footer'; + +describe('footer feature links', () => { + it('links to both when both flags are on', () => { + render(