From 829325c9c7e79fe5c59c0f475b99d4fc615807d2 Mon Sep 17 00:00:00 2001 From: Mischa Date: Fri, 24 Jul 2026 21:34:17 +0200 Subject: [PATCH] fix(review): make the mismatch-blocks-submit test able to fail; carve out the map doctrine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The U5 guard asserted only `.notices` toHaveCount(0) and toHaveURL(/\/post/). Both pass instantly, and both also hold for a *successful* submit — the form posts to /post and only renders its notice after the round trip, and the click is dispatched via evaluate(el => el.click()), which skips Playwright's navigation-aware waiting. So the one test standing between a bad coordinate and the server could not fail. It now proves the negative on disk via findEntry() after a settling interval, registers the tag for cleanup before the click, and asserts the flag and value survived. CLAUDE.md's "one map path" section stated flatly that a single map code path exists, which location-map.js now contradicts. Recorded it as the one sanctioned exception (an editor, not a display map; lazy-imported; shares only MAP_STYLE) rather than leaving the doctrine wrong. The `user` gitlink is deliberately NOT bumped here: its pin already points at an unpushed submodule commit, which must be pushed and re-pointed before this branch merges. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 4 +++- tests/ui/post/location-override.spec.js | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ac2389b..76f9e70 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,9 @@ The site is structured around Trip entities. Key facts: ### One map path: `MapUtils.initEntryMap` + the `entry-map` partial -There is a **single** map code path on the site. The engine is `MapUtils.initEntryMap(opts)` in `js/src/maplibre-utils.js` (bundled into `js/map.js` via `make build-assets` — never hand-edit `js/map.js`). It builds the MapLibre map, places markers/popups, fits bounds, draws the GPX journey, and wires the fullscreen toggle. +There is a **single** map code path for every **display** map on the site. The engine is `MapUtils.initEntryMap(opts)` in `js/src/maplibre-utils.js` (bundled into `js/map.js` via `make build-assets` — never hand-edit `js/map.js`). It builds the MapLibre map, places markers/popups, fits bounds, draws the GPX journey, and wires the fullscreen toggle. + +**One sanctioned exception — the post form's pin editor.** `js/src/location-map.js` (`getOrCreateLocationMap()`) is a deliberately separate, minimal engine for the `/post` form's "More location details" panel: one *draggable* marker, no popups, no GPX, no bounds-fitting, and `maplibre-gl` **lazy-imported** so a GPS-only submit never fetches it. It is an editor, not a display map, so it shares none of `initEntryMap`'s concerns. The two share exactly one thing — the style URL, extracted into `js/src/map-style.js` (`MAP_STYLE`) and imported by both, so the basemap can never drift between them. Do not fold it into `initEntryMap`, and do not add a *third* path: a new display map uses `initEntryMap` + the `entry-map` partial. The map **markup + invocation** is shared via one partial: diff --git a/tests/ui/post/location-override.spec.js b/tests/ui/post/location-override.spec.js index cd49055..e37ca4e 100644 --- a/tests/ui/post/location-override.spec.js +++ b/tests/ui/post/location-override.spec.js @@ -312,9 +312,21 @@ test('submitting with an unresolved lat/lng mismatch is blocked', async ({ page await lngEl.blur(); await expect(latEl).toHaveClass(/location-field--mismatch/); + // Register for cleanup BEFORE the click: if the gate ever regresses, the + // entry lands on disk and the afterAll hook must still see the tag. + created.push(tag); await page.locator('.btn-post').evaluate((el) => el.click()); - await expect(page.locator('.notices')).toHaveCount(0); - await expect(page).toHaveURL(/\/post/); + + // `.notices` toHaveCount(0) and toHaveURL(/\/post/) both pass instantly and + // both also hold for a SUCCESSFUL submit (the form posts to /post and only + // renders its notice after the round trip), so neither can distinguish a + // working gate from a regressed one. Prove the negative on disk instead, + // after giving a regressed submit time to actually write. + await page.waitForTimeout(2000); + expect(findEntry(tag), 'a flagged coordinate must never reach the server').toBeFalsy(); + // And prove the block was the gate's doing: still flagged, value untouched. + await expect(latEl).toHaveClass(/location-field--mismatch/); + await expect(latEl).toHaveValue('999'); }); // ── U4: lazy-load boundary — an ordinary GPS-only submit never fetches maplibre-gl ──