diff --git a/docs/development.md b/docs/development.md index 2456f36..c6b398b 100644 --- a/docs/development.md +++ b/docs/development.md @@ -98,8 +98,9 @@ The UI has a Playwright smoke lane: `npm run test:e2e` runs `tests/e2e/*.spec.js in Chromium against a dev server it starts (or reuses) itself. Every `/api/*` call and the panorama image are stubbed at the browser boundary (`tests/e2e/helpers.js`), so it needs no Redis, Neon, Mapillary, or `.env`. -It covers the region picker, the username modal, and one full round to the -reveal. Anything beyond those flows (real panoramas, real scoring round-trips) +It covers the region picker, the username modal, one full round to the +reveal, and the `/game/{slug}` routing contract (legacy `?region=` redirects, +unknown-region 404s). Anything beyond those flows (real panoramas, real scoring round-trips) is still manual: inform the user when work is complete. Do NOT start development servers - user handles manual testing themselves. diff --git a/docs/game-flow.md b/docs/game-flow.md index 031e832..cd4befd 100644 --- a/docs/game-flow.md +++ b/docs/game-flow.md @@ -23,6 +23,26 @@ - Regions with no usable panoramas are listed but disabled, with the reason shown. See the Coverage note in [project-overview.md](project-overview.md) +#### The game URL + +The region is a lowercase path segment: `/game/tphcm`, `/game/vn`, +`/game/hn-badinh`. Codes are uppercase inside the app (they key the region +tree); `regionSlug` / `regionFromSlug` in `src/lib/regions.js` own the +conversion, and are the only place the URL casing convention lives. + +- An unknown region is a 404. A real region with no imagery is not — it renders + and shows the coverage message, which is a better answer than a 404 +- An uppercase URL plays rather than redirecting. There is deliberately no + canonical-casing redirect: a redirect on a prerendered route gets cached as + that route's response, and nothing in the app generates an uppercase link +- `/game` with no region plays the whole country +- `?region=` and `?location=` still work; they redirect to `/game/{slug}` and + are kept for links and bookmarks already in the wild +- **API routes deliberately keep their query params.** `/api/new-game?region=`, + `/api/leaderboard?region=` and the debug routes are unchanged and should stay + that way — they are fetch calls, not navigations, so there is nothing for a + path segment to buy there + ### 3. Session Creation - Server generates unique UUID v4 session ID - Server stores the exact target location **and the district the panorama sits diff --git a/docs/project-structure.md b/docs/project-structure.md index 00dcfe7..b7db2fa 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -4,7 +4,9 @@ - `CLAUDE.md` - Project instructions and guidelines for Claude Code - `components.json` - shadcn/ui configuration - `package.json` - Dependencies and scripts -- `next.config.mjs` - Next.js configuration +- `next.config.mjs` - Next.js configuration. Deliberately has no `redirects()`: + a config redirect forwards the source query string to the destination, so the + legacy `/game?region=` links are redirected from `src/app/game/page.js` instead - `eslint.config.mjs` - ESLint configuration - `postcss.config.mjs` - PostCSS configuration - `jsconfig.json` - JavaScript project configuration @@ -26,7 +28,12 @@ Next.js 16 App Router structure: - `favicon.ico` - Site favicon #### Game Pages -- `game/page.js` - Main game interface +- `game/[region]/page.js` - The game screen for one region (`/game/tphcm`). + Server Component: validates the slug, prerenders one page per region, + 404s an unknown one +- `game/[region]/not-found.js` - The 404 for an unknown region code +- `game/page.js` - Redirects the legacy `?region=` / `?location=` links to + `/game/{slug}`; a region-less `/game` goes to the country round - `credits/page.js` - Data sources, licenses, and open-source credits - `debug/page.js` - Debug hub: lists every debug tool as a peer - `debug/layout.js` - Shared shell for all debug pages: app bar, DebugNav, theme @@ -84,7 +91,7 @@ the shadcn CLI when a screen needs them, rather than keeping unused ones around. Both directories are build output. Do not hand-edit; see *Rebuilding the generated region data* in [development.md](development.md). -- `regions/index.js` - The 67-node tree: code, name, parent, level, children, +- `regions/index.js` - The 85-node tree: code, name, parent, level, children, center, bbox, and coverage flags - `regions/counts.js` - Per-region panorama and cell tallies. **The one panorama-derived file a client component may import** - it carries counts diff --git a/src/app/components/GameClient.js b/src/app/components/GameClient.js index 9c2163a..4b820c8 100644 --- a/src/app/components/GameClient.js +++ b/src/app/components/GameClient.js @@ -1,7 +1,7 @@ "use client"; import { useEffect, useState, useRef, useCallback } from 'react'; -import { useSearchParams, useRouter } from 'next/navigation'; +import { useRouter } from 'next/navigation'; import { ArrowLeft, Beer } from 'lucide-react'; import PanoramaViewer from './PanoramaViewer'; import ThemeToggle from './ThemeToggle'; @@ -39,11 +39,18 @@ async function fetchNewRound(locationCode, currentSessionId) { return data; } -export default function GameClient() { - const searchParams = useSearchParams(); +/** + * The game screen. + * + * The region arrives as a prop from the /game/[region] page, which has already + * resolved and validated it -- so there is no query string to read and nothing + * to suspend on. + * @param {Object} props + * @param {string} props.region Validated, canonical-case region code to play. + */ +export default function GameClient({ region }) { const router = useRouter(); - const [location, setLocation] = useState('TPHCM'); const [imageData, setImageData] = useState(null); // Bumped once per applied round and used as the viewer's key. The image URL // is not enough: a small district can serve the same panorama twice in a @@ -83,14 +90,11 @@ export default function GameClient() { // ignores this flag entirely. const [mapExpanded, setMapExpanded] = useState(false); - // What the player picked, resolved through the tree. A bookmarked - // ?location=DL still works: DL is a district of Lam Dong now, and every node - // keeps an entry. - const pickedCode = location.toUpperCase(); - const pickedRegion = isRegion(pickedCode) ? getRegion(pickedCode) : null; - // Never echo the raw query string: an unknown value would render as the - // page's own label, and the API uppercases before resolving, so ?region=hn - // would otherwise show 'hn' while serving a Ha Noi round. + // What the player picked, resolved through the tree. The page validated the + // code before rendering, so the isRegion guard is belt-and-braces rather + // than the load-bearing check it used to be when the code came from a query + // string this component read itself. + const pickedRegion = isRegion(region) ? getRegion(region) : null; const regionName = pickedRegion ? pickedRegion.name : 'Vietnam'; const initializingRef = useRef(false); @@ -166,14 +170,10 @@ export default function GameClient() { useEffect(() => { if (initialized) return; - // ?region= is the current form; ?location= is what existing links carry. - const locationParam = - searchParams.get('region') || searchParams.get('location') || 'TPHCM'; - setLocation(locationParam); const existingUsername = getUsername(); setUsernameState(existingUsername || ''); - loadLibrariesAndInitialize(locationParam); - }, [searchParams, loadLibrariesAndInitialize, initialized]); + loadLibrariesAndInitialize(region); + }, [region, loadLibrariesAndInitialize, initialized]); const submitGameResult = async (guessCoords) => { if (!guessCoords || !sessionId) return null; @@ -293,7 +293,7 @@ export default function GameClient() { setSubmitting(false); setShowResult(true); - startPrefetch(location, currentSession); + startPrefetch(region, currentSession); }; // Everything a round accumulates. Both Next Round and Skip come through here, @@ -334,7 +334,7 @@ export default function GameClient() { } } - const loaded = await loadRound(location, currentSession, epoch); + const loaded = await loadRound(region, currentSession, epoch); if (!loaded) setRoundLoading(false); }; @@ -362,7 +362,7 @@ export default function GameClient() { // suppress the spinner and read as a hang while the new round loads. setLoadError(null); setRoundLoading(true); - const loaded = await loadRound(location, currentSession, roundEpochRef.current); + const loaded = await loadRound(region, currentSession, roundEpochRef.current); if (!loaded) setRoundLoading(false); }; @@ -370,7 +370,7 @@ export default function GameClient() { setLoadError(null); roundEpochRef.current += 1; setRoundLoading(true); - const loaded = await loadRound(location, null, roundEpochRef.current); + const loaded = await loadRound(region, null, roundEpochRef.current); if (!loaded) setRoundLoading(false); }; diff --git a/src/app/components/RegionPicker.js b/src/app/components/RegionPicker.js index 4d0a473..d86972b 100644 --- a/src/app/components/RegionPicker.js +++ b/src/app/components/RegionPicker.js @@ -20,6 +20,7 @@ import { isThin, isUnresolved, provinces, + regionSlug, } from '../../lib/regions'; /** Thousands separators, so 225,966 reads as a quantity rather than a code. */ @@ -49,7 +50,7 @@ function unavailableLabel(code) { */ function PlayRow({ code, label, emphasis, onPlayClick }) { const { panos } = coverageOf(code); - const href = `/game?region=${code}`; + const href = `/game/${regionSlug(code)}`; return ( +
+ {/* A real heading, not a styled

: a not-found page owns the whole + document, unlike GameClient's in-place error panel, which sits + inside a page that already has its own h1. */} +

No such region

+

+ That link points at a region that doesn't exist. It may have been + mistyped or cut short. +

+ +
+ + ); +} diff --git a/src/app/game/[region]/page.js b/src/app/game/[region]/page.js new file mode 100644 index 0000000..3aecbc1 --- /dev/null +++ b/src/app/game/[region]/page.js @@ -0,0 +1,42 @@ +import { notFound } from 'next/navigation'; +import GameClient from '../../components/GameClient'; +import { allRegions, regionFromSlug, regionSlug } from '../../../lib/regions'; + +// Every region gets a prerendered page. The region tree is the only data this +// needs -- never pano-index.js or pano-db.js, which reach the exact panorama +// coordinates. See the boundary note at the top of lib/regions.js. +// +// Deliberately allRegions() and not playableRegions(): a real region with no +// imagery still renders, so the player gets the coverage message the API +// already writes ("... has no street view coverage yet") instead of a 404, +// which would be a worse answer than the one already there. +export async function generateStaticParams() { + return allRegions().map((code) => ({ region: regionSlug(code) })); +} + +/** + * The game screen for one region. + * + * Validating here rather than in GameClient is what turns a typo'd URL into a + * real 404. Without it an unknown code renders under the page's own fallback + * label ('Vietnam') while the API resolves whatever it was handed. + * + * An unusual casing renders rather than redirecting to the canonical one -- + * see regionFromSlug for why. + */ +export default async function GameRegionPage({ params }) { + // params is a Promise in Next 16; destructuring it synchronously is the + // pre-16 shape and yields undefined here. + const { region } = await params; + const code = regionFromSlug(region); + + if (!code) notFound(); + + // Keyed by region so a region-to-region navigation remounts rather than + // handing the running game a new prop. GameClient initialises once behind an + // `initialized` guard, so a prop change alone would leave the previous + // region's round on screen. Nothing navigates that way today -- every path + // out goes through the menu -- but a path segment makes prop-change-without- + // unmount the normal App Router pattern, and this is one word. + return ; +} diff --git a/src/app/game/page.js b/src/app/game/page.js index d499094..36cea5b 100644 --- a/src/app/game/page.js +++ b/src/app/game/page.js @@ -1,23 +1,52 @@ -"use client"; +import { redirect } from 'next/navigation'; +import { COUNTRY_CODE, regionSlug } from '../../lib/regions'; -import { Suspense } from 'react'; -import GameClient from '../components/GameClient'; - -function GameLoadingFallback() { - return ( -
-
- -
- ); +/** + * One-line value read from a query string. + * + * URLSearchParams hands back an array when a key repeats, and an empty string + * for a present-but-empty key. Both must read as "no value" so ?region= falls + * through to the country rather than redirecting to a broken `/game/`. + * @param {string|string[]|undefined} value Raw search-param value. + * @returns {string} The value, or '' when there is not a usable one. + */ +function firstValue(value) { + const raw = Array.isArray(value) ? value[0] : value; + return typeof raw === 'string' ? raw : ''; } -export default function GamePage() { - return ( - }> - - - ); +/** + * The old game URL, kept alive for every link and bookmark still in the wild. + * + * The region lives in the path now (/game/{slug}), so this reads the legacy + * query and redirects. `region` wins over `location`, matching how the client + * used to resolve a URL carrying both. + * + * Redirecting from here rather than through next.config.mjs `redirects()` is + * deliberate: a config redirect forwards the original query string to the + * destination, so a visitor would land on `/game/tphcm?region=TPHCM` and keep + * the dead param forever. Building the destination here also applies the + * lowercase URL convention in the same hop, so a legacy `?region=HN` reaches + * `/game/hn` directly rather than through a second redirect. + * + * A region-less visit goes to the country. `resolveRegion()` has always read + * an absent region that way; the old TPHCM fallback was never a stated default, + * just where the query chain happened to end. + */ +export default async function GamePage({ searchParams }) { + // searchParams is a Promise in Next 16. + const query = await searchParams; + const legacy = firstValue(query.region) || firstValue(query.location); + const code = regionSlug(legacy || COUNTRY_CODE); + + // Encoded because `code` is whatever the query string carried -- this value + // goes into a Location header unvalidated. A raw CRLF makes Node throw + // (a 500 instead of the honest 404 below), and a raw '../' would be + // normalised by the browser into a different path entirely. A real region + // code round-trips through encodeURIComponent untouched, which + // tests/regions.test.js asserts, so this costs nothing on the happy path. + // + // An unknown legacy code lands on /game/{slug}, which 404s there. That is + // the honest answer -- it was never a playable link. + redirect(`/game/${encodeURIComponent(code)}`); } diff --git a/src/lib/regions.js b/src/lib/regions.js index 024614e..4b4c6b9 100644 --- a/src/lib/regions.js +++ b/src/lib/regions.js @@ -35,6 +35,41 @@ export function isRegion(code) { return Boolean(code && REGIONS[code]); } +/** + * The URL path segment for a region: `/game/tphcm-q7`. + * + * Codes are uppercase everywhere inside the app -- they are the keys of the + * generated tree -- but URLs are lowercase, which is the ordinary convention + * for a path a person might type or read aloud. + * + * The conversion lives here rather than at each call site because three of + * them build these URLs (the region picker, the route's generateStaticParams, + * and the legacy-query redirect), and a casing mismatch between any two would + * split one region across two analytics rows -- which is the whole reason the + * region is in the path at all. + * @param {string} code Region code, e.g. 'TPHCM-Q7'. + * @returns {string} Path segment, e.g. 'tphcm-q7'. + */ +export function regionSlug(code) { + return String(code).toLowerCase(); +} + +/** + * The region a URL path segment names, or null. + * + * Case-insensitive on purpose: lowercase is what the app links to, but a + * hand-typed or hand-edited `/game/TPHCM` should play rather than 404. There + * is deliberately no redirect to the canonical casing -- a redirect on a + * prerendered route gets cached as that route's response, and an uppercase URL + * is rare enough that one stray analytics row costs less than that mechanism. + * @param {string} slug Path segment from the URL. + * @returns {string|null} The canonical region code, or null when unknown. + */ +export function regionFromSlug(slug) { + const code = String(slug).toUpperCase(); + return isRegion(code) ? code : null; +} + /** * The node and every ancestor above it, ending at the country. * diff --git a/tests/e2e/game.spec.js b/tests/e2e/game.spec.js index 65043af..d154fac 100644 --- a/tests/e2e/game.spec.js +++ b/tests/e2e/game.spec.js @@ -11,7 +11,7 @@ test.beforeEach(async ({ page }) => { // Not the spec under test here, and the banner would sit over the panorama. await seedHintSeen(page); await stubGameApis(page, 'e2e-player'); - await page.goto('/game?region=TPHCM'); + await page.goto('/game/tphcm'); }); test('plays a round to the reveal and into the next one', async ({ page }) => { diff --git a/tests/e2e/home.spec.js b/tests/e2e/home.spec.js index 81f3804..ade97ea 100644 --- a/tests/e2e/home.spec.js +++ b/tests/e2e/home.spec.js @@ -23,7 +23,7 @@ test('expands a province to playable district links', async ({ page }) => { await expect(page.getByRole('link', { name: /Play anywhere in Ho Chi Minh/ })).toBeVisible(); const district7 = page.getByRole('link', { name: /District 7/ }); await expect(district7).toBeVisible(); - await expect(district7).toHaveAttribute('href', '/game?region=TPHCM-Q7'); + await expect(district7).toHaveAttribute('href', '/game/tphcm-q7'); }); test('lists an uncovered district as disabled, with the reason', async ({ page }) => { @@ -38,13 +38,22 @@ test('lists an uncovered district as disabled, with the reason', async ({ page } test('shows the build commit in the debug footer and copies it on click', async ({ page, context }) => { // The dev server resolves the sha from git, so this asserts the real wiring: // config env -> layout -> footer -> clipboard. - const stamp = page.getByRole('button', { name: 'Copy build commit' }); - await expect(stamp).toBeVisible(); - await expect(stamp).toHaveText(/^[0-9a-f]{7}$/); + // + // The sha and the copy control are two elements, not one: the label links to + // the commit on GitHub and the button beside it copies the full sha. Both + // halves are asserted here. + const shaLink = page.getByRole('link', { name: /^View build commit [0-9a-f]{40} on GitHub$/ }); + await expect(shaLink).toBeVisible(); + // The label is the short form; the href carries the full one. + await expect(shaLink).toHaveText(/^[0-9a-f]{7}$/); + await expect(shaLink).toHaveAttribute('href', /\/commit\/[0-9a-f]{40}$/); await context.grantPermissions(['clipboard-read', 'clipboard-write']); - await stamp.click(); - await expect(stamp).toHaveText('copied!'); + await page.getByRole('button', { name: 'Copy build commit' }).click(); + + // The glyph swap is decorative (aria-hidden); the accessible name is what + // announces the state, so that is what gets asserted. + await expect(page.getByRole('button', { name: 'Build commit copied' })).toBeVisible(); const copied = await page.evaluate(() => navigator.clipboard.readText()); expect(copied).toMatch(/^[0-9a-f]{40}$/); }); diff --git a/tests/e2e/routing.spec.js b/tests/e2e/routing.spec.js new file mode 100644 index 0000000..53b000d --- /dev/null +++ b/tests/e2e/routing.spec.js @@ -0,0 +1,124 @@ +import { test, expect } from '@playwright/test'; +import { stubGameApis, seedUsername, seedHintSeen } from './helpers.js'; + +// The region lives in the path (/game/tphcm), and every URL shape that used to +// work still has to. A dead bookmark produces no error anyone sees, so these +// assertions are the only thing that would notice a broken redirect. +// +// URLs are asserted exactly, not by partial match: an unanchored /\/game\/VN/ +// would also pass for /game/vnsomething. + +/** + * The path a navigation actually settled on, query string included. + * + * Compared as one string rather than pathname alone: proving the query is + * GONE is half the point of these tests -- a config-level redirect would have + * carried ?region= onto the destination. Host-free so the spec does not + * duplicate playwright.config.js's baseURL. + * @param {import('@playwright/test').Page} page Playwright page. + * @returns {string} e.g. '/game/TPHCM'. + */ +function landedAt(page) { + const url = new URL(page.url()); + return `${url.pathname}${url.search}`; +} + +test.beforeEach(async ({ page }) => { + await seedUsername(page, 'e2e-router'); + await seedHintSeen(page); + await stubGameApis(page, 'e2e-router'); +}); + +test('serves the canonical region URL without redirecting', async ({ page }) => { + const response = await page.goto('/game/tphcm'); + expect(response.status()).toBe(200); + expect(landedAt(page)).toBe('/game/tphcm'); + // Landed on a real round, not an error panel. + await expect(page.getByText('Ho Chi Minh', { exact: true })).toBeVisible(); +}); + +test('redirects the legacy ?region= form', async ({ page }) => { + await page.goto('/game?region=TPHCM'); + expect(landedAt(page)).toBe('/game/tphcm'); +}); + +test('redirects the legacy ?location= form', async ({ page }) => { + await page.goto('/game?location=TPHCM'); + expect(landedAt(page)).toBe('/game/tphcm'); +}); + +test('prefers region over location when a link carries both', async ({ page }) => { + // Matches how the old client resolved it: `region` first, `location` as the + // fallback. A link carrying both must not change meaning in the migration. + await page.goto('/game?region=TPHCM&location=HN'); + expect(landedAt(page)).toBe('/game/tphcm'); +}); + +test('sends a region-less /game to the country round', async ({ page }) => { + await page.goto('/game'); + expect(landedAt(page)).toBe('/game/vn'); +}); + +test('treats an empty region param as region-less', async ({ page }) => { + // firstValue() reads '' as "no value", so an empty param falls through to + // the country rather than landing on a broken `/game/`. + await page.goto('/game?region='); + expect(landedAt(page)).toBe('/game/vn'); +}); + +test('serves an uppercase URL directly, without redirecting', async ({ page }) => { + // Nothing in the app generates an uppercase URL, but a hand-edited one has + // to play rather than 404. Deliberately NOT redirected to the lowercase + // form: a redirect on a prerendered route gets cached as that route's + // response, and one stray analytics row costs less than that mechanism. + const response = await page.goto('/game/TPHCM'); + expect(response.status()).toBe(200); + expect(landedAt(page)).toBe('/game/TPHCM'); + await expect(page.getByText('Ho Chi Minh', { exact: true })).toBeVisible(); +}); + +test('lowercases an uppercase legacy region in one hop', async ({ page }) => { + // The legacy handler applies the slug convention while building the + // destination, so this lands lowercase in a single redirect. Also proves the + // redirect leaves no query behind -- a config-level rule would have carried + // ?region=HN onto the destination. + await page.goto('/game?region=HN'); + expect(landedAt(page)).toBe('/game/hn'); +}); + +test('encodes a hostile legacy region value instead of 500ing or escaping the path', async ({ page }) => { + // The legacy value goes into a Location header. Unencoded, a CRLF makes Node + // throw (500) and a '../' would be normalised by the browser onto a + // different path. Both must end at the honest 404 instead. + const crlf = await page.goto('/game?region=a%0d%0ab'); + expect(crlf.status()).toBe(404); + + const traversal = await page.goto('/game?region=..%2F..%2Fdebug'); + expect(traversal.status()).toBe(404); + expect(landedAt(page)).not.toBe('/debug'); +}); + +test('404s an unknown region', async ({ page }) => { + const response = await page.goto('/game/NOTAREGION'); + // Asserted on status, never on the not-found page's wording, which is free + // to change without breaking this. + expect(response.status()).toBe(404); +}); + +test('serves a real region with no imagery instead of 404ing it', async ({ page }) => { + // Deliberately NOT a 404. The API answers an uncovered region with a written + // message ("... has no street view coverage yet") and the page renders it; + // a 404 would replace a good answer with a worse one. The APIs are stubbed + // here, so this asserts only the routing half -- that the page exists and + // serves. The message itself is the API's contract, covered in + // tests/region-request.test.js. + const { isRegion, isPlayable } = await import('../../src/lib/regions.js'); + const code = 'TPHCM-CUCHI'; + expect(isRegion(code), `${code} should still be a region`).toBe(true); + expect(isPlayable(code), `${code} gained coverage; pick another fixture`).toBe(false); + + const { regionSlug } = await import('../../src/lib/regions.js'); + const response = await page.goto(`/game/${regionSlug(code)}`); + expect(response.status()).toBe(200); + expect(landedAt(page)).toBe(`/game/${regionSlug(code)}`); +}); diff --git a/tests/e2e/username.spec.js b/tests/e2e/username.spec.js index 5117ce0..930bd32 100644 --- a/tests/e2e/username.spec.js +++ b/tests/e2e/username.spec.js @@ -1,9 +1,14 @@ import { test, expect } from '@playwright/test'; import { stubGameApis, seedUsername, seedHintSeen, USERNAME_STORAGE_KEY } from './helpers.js'; -// The name prompt appears at the first Play click -- when the name is about -// to matter -- not on landing. No seeded name in the fresh-profile tests: -// that timing is the behavior under test. +// With no stored name the prompt opens on landing, so every way out of it +// (save, skip, dismiss) leaves a name behind before Play is ever clicked -- +// see the comment above the effect in src/app/page.js. No seeded name in the +// fresh-profile tests: that timing is the behavior under test. +// +// The Play-click interception in handlePlayClick() is the remaining fallback +// for a name that goes missing between landing and the click. It is not +// reachable by an ordinary gesture, so nothing here exercises it. test.beforeEach(async ({ page }) => { await stubGameApis(page, 'fresh-player'); @@ -11,39 +16,43 @@ test.beforeEach(async ({ page }) => { await page.goto('/'); }); -test('landing shows no prompt; the first Play click asks, saves, and starts the game', async ({ page }) => { - // The landing page introduces the game first: no ambush modal, just an - // always-visible entry point for setting a name later. - await expect(page.getByRole('dialog')).toBeHidden(); - await expect(page.getByRole('button', { name: 'Set name' })).toBeVisible(); - - await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click(); - +test('landing asks for a name, and Play then goes straight into the round', async ({ page }) => { + // The prompt comes before Play rather than interrupting it, so the name is + // already settled by the time the player commits to a region. const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ }); await expect(dialog).toBeVisible(); await dialog.getByLabel(/Username/).fill('fresh-player'); await dialog.getByRole('button', { name: 'Save name' }).click(); - // Saving resumes the intercepted navigation into the round. - await expect(page).toHaveURL(/\/game\?region=VN/); + // Saving with nothing pending just closes: there is no navigation to resume. + await expect(dialog).toBeHidden(); expect( await page.evaluate((key) => window.localStorage.getItem(key), USERNAME_STORAGE_KEY) ).toBe('fresh-player'); + + // Play is now uninterrupted. + await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click(); + await expect(page).toHaveURL(/\/game\/vn$/); }); -test('skip generates a random name and still starts the game', async ({ page }) => { - await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click(); +test('skip generates a random name and still lets the game start', async ({ page }) => { const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ }); await dialog.getByRole('button', { name: /Skip — random name/ }).click(); + await expect(dialog).toBeHidden(); - await expect(page).toHaveURL(/\/game\?region=VN/); + // Skip is not "play anonymously": it persists a generated name exactly like + // a typed one, so the round still reaches the leaderboard under something + // the player can recognise and later edit. expect( await page.evaluate((key) => window.localStorage.getItem(key), USERNAME_STORAGE_KEY) ).toMatch(/^Player-[0-9a-z]{6}$/); + + await page.getByRole('link', { name: /Play anywhere in Vietnam/ }).click(); + await expect(page).toHaveURL(/\/game\/vn$/); }); test('rejects an invalid username with a visible error', async ({ page }) => { - await page.getByRole('button', { name: 'Set name' }).click(); + // Already open on landing -- no stored name in this profile. const dialog = page.getByRole('dialog', { name: /Welcome to VNGeoGuessr/ }); await dialog.getByLabel(/Username/).fill('bad name!'); await dialog.getByRole('button', { name: 'Save name' }).click(); diff --git a/tests/regions.test.js b/tests/regions.test.js index f2d77f6..f9297f8 100644 --- a/tests/regions.test.js +++ b/tests/regions.test.js @@ -13,6 +13,8 @@ import { regionPath, allRegions, isUnresolved, + regionSlug, + regionFromSlug, } from '../src/lib/regions.js'; const LEAVES = allRegions().filter((code) => childrenOf(code).length === 0); @@ -25,6 +27,36 @@ describe('region tree shape', () => { expect(getRegion(COUNTRY_CODE).level).toBe('country'); }); + it('every code has a URL-safe lowercase slug', () => { + // The slug is the /game/[region] path segment. Anything outside this class + // would need percent-encoding, which would break both the prerendered + // params and the legacy redirect that builds these URLs by hand. + for (const code of allRegions()) { + const slug = regionSlug(code); + expect(slug, code).toMatch(/^[a-z0-9-]+$/); + expect(encodeURIComponent(slug), code).toBe(slug); + } + }); + + it('every slug resolves back to the code it came from', () => { + // regionSlug and regionFromSlug are the only two directions this app + // converts between, and three call sites depend on the round trip. + for (const code of allRegions()) { + expect(regionFromSlug(regionSlug(code)), code).toBe(code); + } + }); + + it('resolves a slug whatever its casing, and rejects a non-region', () => { + // Uppercase URLs are not generated anywhere, but a hand-edited one plays + // rather than 404ing -- there is deliberately no canonical-casing redirect. + expect(regionFromSlug('tphcm')).toBe('TPHCM'); + expect(regionFromSlug('TPHCM')).toBe('TPHCM'); + expect(regionFromSlug('TpHcM')).toBe('TPHCM'); + expect(regionFromSlug('hn-badinh')).toBe('HN-BADINH'); + expect(regionFromSlug('notaregion')).toBeNull(); + expect(regionFromSlug('')).toBeNull(); + }); + it('every parent reference resolves', () => { for (const code of allRegions()) { const { parent } = getRegion(code);