diff --git a/docs/admin/managing-locations.md b/docs/admin/managing-locations.md index a9c39e2..0f1975e 100644 --- a/docs/admin/managing-locations.md +++ b/docs/admin/managing-locations.md @@ -50,7 +50,8 @@ The `/where-to-buy` page groups venues by the **Location Name** field. The publi - If the location name contains `sxm`, `maarten`, or `martin`, the heading displays as **Sint Maarten / Saint Martin / SXM**. - If the location name contains `statia` or `eustatius`, the heading displays as **Sint Eustatius / Statia**. -- Any other value has its first letter capitalized and is used as-is. +- `Saba` — or any value whose last comma-separated part is `Saba`, such as `Windwardside, Saba` or `Fort Bay, Saba` — groups under **Saba**. The locality prefix still appears on the venue card but does not create its own group. +- Any other value becomes its own group heading, capitalized word by word. ### Recommended exact values @@ -62,7 +63,7 @@ Use simple, consistent values so grouping works predictably: Examples: -- A venue on Saba should have **Location Name** set to `Saba`. +- A venue on Saba should have **Location Name** set to `Saba`. If the locality matters to visitors, `, Saba` also works — for example `Windwardside, Saba` still groups under **Saba** while the card shows the locality. - A venue on Sint Maarten should use `SXM` or `Sint Maarten`. - A venue on Saint Martin (French side) should use `Saint Martin` or `SXM`. diff --git a/lib/venue-filters.ts b/lib/venue-filters.ts index 41c89fa..c32a1fa 100644 --- a/lib/venue-filters.ts +++ b/lib/venue-filters.ts @@ -66,8 +66,10 @@ export function venueOffersFormat( /** * Canonical island key for a `locationName`. "sxm"/maarten/martin collapse - * to "sxm", statia/eustatius to "statia"; anything else lowercases as-is. - * An empty/missing location follows the same Saba default the public page + * to "sxm", statia/eustatius to "statia", and "saba" plus Saba localities + * written ", Saba" ("Fort Bay, Saba", "Windwardside / The + * Bottom, Saba") collapse to "saba"; anything else lowercases as-is. An + * empty/missing location follows the same Saba default the public page * has always used. */ export function islandKey(locationName: string | undefined): string { @@ -82,14 +84,35 @@ export function islandKey(locationName: string | undefined): string { if (normalized.includes("statia") || normalized.includes("eustatius")) { return "statia"; } + // Saba venues may prefix the island with a locality; the island is the + // final comma-separated segment. Matching the whole segment — not a + // bare "saba" substring — keeps unrelated names from being + // misclassified. + if (normalized.split(",").pop()?.trim() === "saba") { + return "saba"; + } return normalized === "" ? "saba" : normalized; } -/** Display heading for a canonical island key (same names as before). */ +// Intentional public labels for the known canonical islands — never +// derived from casing rules. A Map (not a Record) so keys colliding with +// Object.prototype members still take the unknown-island path. +const ISLAND_DISPLAY_NAMES: ReadonlyMap = new Map([ + ["saba", "Saba"], + ["sxm", "Sint Maarten / Saint Martin / SXM"], + ["statia", "Sint Eustatius / Statia"], +]); + +/** + * Display heading for a canonical island key. Known islands return their + * canonical label; an unknown future island key title-cases each word + * rather than only the first letter of the whole string. + */ export function islandDisplayName(key: string): string { - if (key === "sxm") return "Sint Maarten / Saint Martin / SXM"; - if (key === "statia") return "Sint Eustatius / Statia"; - return key.charAt(0).toUpperCase() + key.slice(1); + return ( + ISLAND_DISPLAY_NAMES.get(key) ?? + key.replace(/\b[a-z]/g, (ch) => ch.toUpperCase()) + ); } /** Distinct canonical island keys across venues, ordered by display name. */ diff --git a/lib/where-to-buy-fixture.ts b/lib/where-to-buy-fixture.ts index 54672e9..6821a6b 100644 --- a/lib/where-to-buy-fixture.ts +++ b/lib/where-to-buy-fixture.ts @@ -3,7 +3,10 @@ import type { Beer, Venue } from "@/lib/types"; // Deterministic fixture records for the test-only /where-to-buy-fixture // route used by the Playwright filtering suite. Shapes match the real // Firestore models so VenueDirectory exercises the same code path as the -// production page. Not real business data — names/links are placeholders. +// production page. The three Saba venues deliberately use different +// locality strings (", Saba") so the suite covers the island +// grouping regression from issue #116. Not real business data — +// names/links are placeholders. export const WHERE_TO_BUY_FIXTURE_BEERS: Beer[] = [ { name: "Saba Suds Pilsner", @@ -56,7 +59,7 @@ export const WHERE_TO_BUY_FIXTURE_VENUES: Venue[] = [ name: "Fixture Tavern", slug: "fixture-tavern", type: "bar_restaurant", - locationName: "Saba", + locationName: "Fort Bay, Saba", carriesBeerSlugs: ["saba-suds-pilsner", "fort-bay-ipa"], tapBeerSlugs: ["saba-suds-pilsner"], canBeerSlugs: ["fort-bay-ipa"], @@ -71,7 +74,7 @@ export const WHERE_TO_BUY_FIXTURE_VENUES: Venue[] = [ name: "Fixture Bottle Shop", slug: "fixture-bottle-shop", type: "retail", - locationName: "Saba", + locationName: "Windwardside, Saba", carriesBeerSlugs: ["fort-bay-ipa"], tapBeerSlugs: [], canBeerSlugs: ["fort-bay-ipa"], @@ -95,7 +98,7 @@ export const WHERE_TO_BUY_FIXTURE_VENUES: Venue[] = [ name: "Fixture Quiet Cafe", slug: "fixture-quiet-cafe", type: "bar_restaurant", - locationName: "Saba", + locationName: "Windwardside / The Bottom, Saba", carriesBeerSlugs: [], tapBeerSlugs: [], canBeerSlugs: [], diff --git a/smoke-tests/where-to-buy-filters.spec.ts b/smoke-tests/where-to-buy-filters.spec.ts index be39e86..a241799 100644 --- a/smoke-tests/where-to-buy-filters.spec.ts +++ b/smoke-tests/where-to-buy-filters.spec.ts @@ -6,11 +6,14 @@ import { test, expect } from "./fixtures"; // also proves filtering works with analytics consent declined. // // Fixture data (lib/where-to-buy-fixture.ts): -// Fixture Tavern Saba tap: pilsner can: ipa -// Fixture Bottle Shop Saba can: ipa -// Fixture Harbor Bar SXM tap: pilsner + ipa -// Fixture Quiet Cafe Saba carries nothing +// Fixture Tavern Fort Bay, Saba tap: pilsner can: ipa +// Fixture Bottle Shop Windwardside, Saba can: ipa +// Fixture Harbor Bar SXM tap: pilsner + ipa +// Fixture Quiet Cafe Windwardside / The Bottom, Saba carries nothing // Flat Point Amber is a public beer carried by no venue — never an option. +// The three Saba venues deliberately use distinct ", Saba" +// locationNames so this suite covers the island-grouping regression from +// issue #116 (localities fragmenting into per-locality island groups). const ROUTE = "/where-to-buy-fixture"; @@ -49,6 +52,13 @@ test("renders filters and all venues unfiltered", async ({ page }) => { await expect(formatButton(page, "On Tap")).toBeVisible(); await expect(formatButton(page, "In Can")).toBeVisible(); await expect(islandSelect(page)).toBeVisible(); + // One option per real island — the three Saba localities collapse to a + // single "Saba" choice (#116). + await expect(islandSelect(page).locator("option")).toHaveText([ + "All islands", + "Saba", + "Sint Maarten / Saint Martin / SXM", + ]); await expect(statusText(page)).toHaveText("4 venues shown"); expect(await venueNames(page)).toEqual([ "Fixture Tavern", @@ -110,6 +120,55 @@ test("island filter matches the canonical island", async ({ page }) => { await expect(page).toHaveURL(/island=sxm/); }); +test("Saba localities render under one Saba group and one filter option", async ({ + page, +}) => { + // Issue #116 regression: ", Saba" locationNames must not + // fragment into per-locality island groups or headings like + // "Fort bay, saba". + await expect(page.locator("main h2")).toHaveText([ + "Saba", + "Sint Maarten / Saint Martin / SXM", + ]); + await expect( + page.getByRole("heading", { name: "Fort bay, saba" }) + ).toHaveCount(0); + await expect( + page.getByRole("heading", { name: "Windwardside, saba" }) + ).toHaveCount(0); + + // All three Saba venues sit inside the single Saba section, and each + // card still shows its raw locality text. + const sabaSection = page.locator("main section").filter({ + has: page.getByRole("heading", { name: "Saba", level: 2, exact: true }), + }); + await expect(sabaSection.locator("h3")).toHaveText([ + "Fixture Tavern", + "Fixture Bottle Shop", + "Fixture Quiet Cafe", + ]); + await expect( + sabaSection.getByText("Fort Bay, Saba", { exact: true }) + ).toBeVisible(); + await expect( + sabaSection.getByText("Windwardside, Saba", { exact: true }) + ).toBeVisible(); + await expect( + sabaSection.getByText("Windwardside / The Bottom, Saba", { + exact: true, + }) + ).toBeVisible(); + + // The Saba option returns every Saba venue and excludes other islands. + await islandSelect(page).selectOption("saba"); + await expect(statusText(page)).toHaveText("3 venues shown"); + expect(await venueNames(page)).toEqual([ + "Fixture Tavern", + "Fixture Bottle Shop", + "Fixture Quiet Cafe", + ]); +}); + test("impossible combination shows the empty state, clear restores all", async ({ page, }) => { diff --git a/tests/lib/venue-filters.test.ts b/tests/lib/venue-filters.test.ts index 9a1f8fa..98457b3 100644 --- a/tests/lib/venue-filters.test.ts +++ b/tests/lib/venue-filters.test.ts @@ -66,6 +66,26 @@ describe("islandKey / islandDisplayName", () => { assert.equal(islandKey("Sint Eustatius"), "statia"); }); + it("normalizes every Saba locality variant to the one saba key", () => { + // Issue #116: Fort Bay / Windwardside / The Bottom are localities on + // Saba, not separate islands. The canonical form is ", + // Saba" — the island is the final comma-separated segment. + for (const locationName of [ + "Saba", + "Fort Bay, Saba", + "Windwardside, Saba", + "The Bottom, Saba", + "Windwardside / The Bottom, Saba", + " the bottom , saba ", + ]) { + assert.equal(islandKey(locationName), "saba", locationName); + } + }); + + it("does not misclassify values that merely contain the saba substring", () => { + assert.notEqual(islandKey("Sabana Grande"), "saba"); + }); + it("defaults empty location to saba, matching the public page", () => { assert.equal(islandKey(""), "saba"); assert.equal(islandKey(undefined), "saba"); @@ -76,14 +96,33 @@ describe("islandKey / islandDisplayName", () => { assert.equal(islandKey("Bonaire"), "bonaire"); }); - it("renders the same display names the page used before", () => { + it("renders intentional display names for known islands", () => { assert.equal( islandDisplayName("sxm"), "Sint Maarten / Saint Martin / SXM" ); assert.equal(islandDisplayName("statia"), "Sint Eustatius / Statia"); assert.equal(islandDisplayName("saba"), "Saba"); + // A Saba locality can never surface as a heading again. + assert.equal(islandDisplayName(islandKey("Fort Bay, Saba")), "Saba"); + assert.equal( + islandDisplayName(islandKey("Windwardside / The Bottom, Saba")), + "Saba" + ); + }); + + it("title-cases unknown future islands word by word", () => { assert.equal(islandDisplayName("bonaire"), "Bonaire"); + assert.equal(islandDisplayName("st. barths"), "St. Barths"); + }); + + it("treats Object.prototype-named keys as unknown islands", () => { + // A plain-object label lookup would return inherited members like + // `constructor` instead of a string. + for (const key of ["constructor", "toString", "__proto__"]) { + assert.equal(typeof islandDisplayName(key), "string", key); + } + assert.equal(islandDisplayName("constructor"), "Constructor"); }); }); @@ -123,10 +162,10 @@ describe("venueCarriesBeer / venueOffersFormat", () => { }); describe("filterVenues", () => { - // Tavern: Saba, pilsner on tap + ipa in can - // Bottle Shop: Saba, ipa in can - // Harbor Bar: SXM, pilsner + ipa on tap - // Quiet Cafe: Saba, carries nothing + // Tavern: Fort Bay, Saba — pilsner on tap + ipa in can + // Bottle Shop: Windwardside, Saba — ipa in can + // Harbor Bar: SXM — pilsner + ipa on tap + // Quiet Cafe: Windwardside / The Bottom, Saba — carries nothing const venues = WHERE_TO_BUY_FIXTURE_VENUES; const names = (list: Venue[]) => list.map((v) => v.slug); @@ -192,6 +231,15 @@ describe("filterVenues", () => { ); }); + it("island saba returns every Saba venue regardless of locality", () => { + // The three fixture Saba venues use different ", Saba" + // locationNames — one canonical key must catch them all (#116). + assert.deepEqual( + names(filterVenues(venues, { ...EMPTY_VENUE_FILTERS, island: "saba" })), + ["fixture-tavern", "fixture-bottle-shop", "fixture-quiet-cafe"] + ); + }); + it("categories combine with AND", () => { assert.deepEqual( names( @@ -245,6 +293,22 @@ describe("distinctIslands / groupVenuesByIsland", () => { ["a", "b"] ); }); + + it("offers one saba option and one saba group for all Saba localities", () => { + // Issue #116 regression: distinct ", Saba" locationNames + // must not fragment into per-locality island options or headings. + const venues = WHERE_TO_BUY_FIXTURE_VENUES; + assert.deepEqual(distinctIslands(venues), ["saba", "sxm"]); + const groups = groupVenuesByIsland(venues); + assert.deepEqual( + groups.map((g) => g.key), + ["saba", "sxm"] + ); + assert.deepEqual( + groups[0].venues.map((v) => v.slug), + ["fixture-tavern", "fixture-bottle-shop", "fixture-quiet-cafe"] + ); + }); }); describe("carriedBeerOptions", () => {