Compare commits

..
Author SHA1 Message Date
TudorandClaude Fable 5 3adea73ee0 fix(e2e): compare-chips test must use schools in one phase
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m38s
PR Checks / Backend Smoke (pull_request) Successful in 5s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 50s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 9s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 37s
The test picked the first two /school/ links from a 'primary' search and
asserted exactly two mobile chips. But a 'primary' search can return
all-through schools (e.g. 'Hessle High School and Penshurst Primary')
that classify as secondary, so the two picks can split across phases —
the active phase then holds one school and the chips are correctly gated
out (they need ≥2 in the active phase), while the canvas still shows one
line. That's a test artefact, not a bug.

Pick three schools instead: across two phases the auto-selected majority
phase always holds ≥2, so the chip legend is guaranteed. Assert ≥2 chips
(the majority may be 2 or 3). Verified against staging.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-06 11:42:11 +01:00
5 changed files with 25 additions and 118 deletions
+9 -4
View File
@@ -100,15 +100,20 @@ test('compare chart on mobile shows school chips with tap-to-focus', async ({ pa
links.map((l) => (l as HTMLAnchorElement).getAttribute('href') || '')
);
const urns = [...new Set(hrefs.map((h) => h.match(/\/school\/(\d+)/)?.[1]).filter(Boolean))];
expect(urns.length).toBeGreaterThanOrEqual(2);
// Compare three schools, not two: a "primary" search can return all-through
// schools that classify as secondary, and the chips only appear for the
// active phase. With three schools across two phases, the auto-selected
// majority phase always holds ≥2, so the chip legend is guaranteed to render.
expect(urns.length).toBeGreaterThanOrEqual(3);
await page.goto(`/compare?urns=${urns[0]},${urns[1]}`);
await page.goto(`/compare?urns=${urns[0]},${urns[1]},${urns[2]}`);
await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 });
// The mobile chart legend renders one chip per school inside the chart card.
// The mobile chart legend renders one chip per school in the active phase.
const chipGroup = page.getByRole('group', { name: /highlight a school/i });
const chips = chipGroup.getByRole('button');
await expect(chips).toHaveCount(2);
await expect(chips.first()).toBeVisible({ timeout: 15_000 });
expect(await chips.count()).toBeGreaterThanOrEqual(2);
// Tapping a chip focuses that school's line; tapping again releases it.
await chips.first().click();
+1 -3
View File
@@ -22,9 +22,7 @@ COPY . .
ENV NEXT_TELEMETRY_DISABLED=1
ENV NODE_ENV=production
# Default backend URL for any server-side fetch during `next build`. The
# runtime /api proxy reads FASTAPI_URL per request (see app/api/[...path]),
# so the deployed container's env is what actually routes traffic.
# Build argument for FastAPI URL (used by Next.js rewrites at build time)
ARG FASTAPI_URL=http://backend:80/api
ENV FASTAPI_URL=${FASTAPI_URL}
-75
View File
@@ -1,75 +0,0 @@
/**
* Runtime proxy for /api/* → the FastAPI backend.
*
* This replaces the old next.config.js `rewrites()` proxy, whose destination
* was baked into the build (routes-manifest.json) from FASTAPI_URL at build
* time. Because one frontend image is promoted staging→prod, a baked hostname
* forced every environment to name the backend identically; a mismatch (e.g.
* a `backend_stg` service) produced `getaddrinfo ENOTFOUND backend`.
*
* A route handler reads process.env.FASTAPI_URL on each request, so the same
* image adapts to whatever the backend is called in each environment.
*/
import { type NextRequest, NextResponse } from 'next/server';
export const dynamic = 'force-dynamic';
export const runtime = 'nodejs';
// FASTAPI_URL already includes the `/api` suffix (e.g. http://backend:80/api).
function backendBase(): string {
return process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000/api';
}
// Hop-by-hop / length headers must not be copied across a proxy — undici has
// already decoded the body, so a stale content-encoding/length corrupts it.
const STRIPPED_RESPONSE_HEADERS = ['content-encoding', 'content-length', 'transfer-encoding', 'connection'];
const METHODS_WITH_BODY = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
async function handler(req: NextRequest, ctx: { params: Promise<{ path: string[] }> }) {
const { path } = await ctx.params;
const target = `${backendBase()}/${path.join('/')}${req.nextUrl.search}`;
const headers = new Headers(req.headers);
headers.delete('host');
headers.delete('connection');
const init: RequestInit & { duplex?: 'half' } = {
method: req.method,
headers,
redirect: 'manual',
cache: 'no-store',
};
if (METHODS_WITH_BODY.has(req.method)) {
init.body = req.body;
init.duplex = 'half';
}
let upstream: Response;
try {
upstream = await fetch(target, init);
} catch (err) {
// e.g. DNS failure or connection refused — surface a clean 502 instead of
// an opaque proxy crash so callers can degrade gracefully.
return NextResponse.json({ detail: 'Upstream request failed' }, { status: 502 });
}
const responseHeaders = new Headers(upstream.headers);
for (const h of STRIPPED_RESPONSE_HEADERS) responseHeaders.delete(h);
return new NextResponse(upstream.body, {
status: upstream.status,
statusText: upstream.statusText,
headers: responseHeaders,
});
}
export {
handler as GET,
handler as HEAD,
handler as POST,
handler as PUT,
handler as PATCH,
handler as DELETE,
handler as OPTIONS,
};
-32
View File
@@ -1,32 +0,0 @@
/**
* Runtime proxy for /sitemap.xml → the FastAPI backend's generated sitemap.
*
* 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 here.
*/
import { NextResponse } from 'next/server';
export const dynamic = 'force-dynamic';
export const runtime = 'nodejs';
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 GET() {
let upstream: Response;
try {
upstream = await fetch(`${backendOrigin()}/sitemap.xml`, { 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' },
});
}
+15 -4
View File
@@ -3,10 +3,21 @@ const nextConfig = {
// Enable standalone output for Docker
output: 'standalone',
// The /api/* and /sitemap.xml proxies to the FastAPI backend are route
// handlers (app/api/[...path]/route.ts, app/sitemap.xml/route.ts) rather
// than rewrites, so the backend host is read from FASTAPI_URL at runtime
// instead of being baked into the build.
// API Proxy to FastAPI backend
async rewrites() {
const apiUrl = process.env.FASTAPI_URL || 'http://localhost:8000/api';
const backendUrl = apiUrl.replace(/\/api$/, '');
return [
{
source: '/api/:path*',
destination: `${apiUrl}/:path*`,
},
{
source: '/sitemap.xml',
destination: `${backendUrl}/sitemap.xml`,
},
];
},
// Image optimization
images: {