Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/admin/managing-locations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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, `<locality>, 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`.

Expand Down
35 changes: 29 additions & 6 deletions lib/venue-filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<locality>, 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 {
Expand All @@ -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<string, string> = 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. */
Expand Down
11 changes: 7 additions & 4 deletions lib/where-to-buy-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ("<locality>, 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",
Expand Down Expand Up @@ -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"],
Expand All @@ -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"],
Expand All @@ -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: [],
Expand Down
67 changes: 63 additions & 4 deletions smoke-tests/where-to-buy-filters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<locality>, 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";

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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: "<locality>, 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,
}) => {
Expand Down
74 changes: 69 additions & 5 deletions tests/lib/venue-filters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<locality>,
// 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");
Expand All @@ -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");
});
});

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -192,6 +231,15 @@ describe("filterVenues", () => {
);
});

it("island saba returns every Saba venue regardless of locality", () => {
// The three fixture Saba venues use different "<locality>, 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(
Expand Down Expand Up @@ -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 "<locality>, 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", () => {
Expand Down
Loading