Group Saba venue localities under one island heading (#116) - #120
Merged
Merged
Conversation
islandKey() canonicalized SXM and Statia spelling variants but returned any other locationName lowercased as-is, so Saba localities like "Fort Bay, Saba" or "Windwardside / The Bottom, Saba" each produced a separate island group — and islandDisplayName() then rendered those keys as headings like "Fort bay, saba". The Island filter exposed several options that all meant Saba. The island is now the final comma-separated segment of locationName: "<locality>, Saba" resolves to the canonical "saba" key without relying on a bare substring match, so values that merely contain "saba" are not misclassified. Known keys get intentional display labels via ISLAND_DISPLAY_NAMES; unknown future keys title-case word by word instead of only capitalizing the first letter. The raw locationName still renders on each venue card — grouping changed, data did not. The /where-to-buy-fixture records now use distinct Saba localities so the Playwright suite covers this regression (uniform "Saba" values could never have exposed it), and new unit/smoke tests pin the one-option, one-heading, all-venues-under-Saba contract. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reviewer's GuideThe PR fixes fragmented Saba venue groups by deriving the island from the final comma-separated location segment, adds explicit canonical labels and safe fallback formatting, updates fixtures to exercise real locality variants, and adds unit, smoke-test, and documentation coverage without changing venue data schemas or persistence. Flow diagram for canonical Saba venue groupingflowchart LR
A["Raw locationName"] --> B["Normalize location"]
B --> C{"Final comma-separated segment is saba?"}
C -->|Yes| D["Canonical key: saba"]
C -->|No| E["Existing SXM/Statia rules or fallback key"]
D --> F["Display heading: Saba"]
D --> G["Venue card keeps raw locality"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="lib/venue-filters.ts" line_range="99-103" />
<code_context>
- if (key === "statia") return "Sint Eustatius / Statia";
- return key.charAt(0).toUpperCase() + key.slice(1);
+ return (
+ ISLAND_DISPLAY_NAMES[key] ??
+ key.replace(/\b[a-z]/g, (ch) => ch.toUpperCase())
+ );
}
</code_context>
<issue_to_address>
**issue (bug_risk):** `islandDisplayName()` returns inherited Object prototype properties instead of a string for unknown keys such as `constructor`, `toString`, or `__proto__`. A venue with one of those location names then causes `distinctIslands()` or `groupVenuesByIsland()` to call `.localeCompare()` on a non-string and crash.
**Triggers:** When a venue has a location name that lowercases to `constructor`, `toString`, or `__proto__`.
**Suggested fix:** Use an object with a null prototype or guard the lookup with `Object.hasOwn(ISLAND_DISPLAY_NAMES, key)` before returning the mapped label.
```suggestion
const ISLAND_DISPLAY_NAMES: Record<string, string> = Object.assign(
Object.create(null),
{
saba: "Saba",
sxm: "Sint Maarten / Saint Martin / SXM",
statia: "Sint Eustatius / Statia",
}
);
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: lib/venue-filters.ts:103
A plain-object lookup returns inherited Object.prototype members for keys like "constructor" or "__proto__", which would surface a non-string from islandDisplayName() and crash the localeCompare ordering. Addresses a Sourcery review finding on the PR. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This was referenced Sep 24, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/where-to-buyfragmented Saba venues into multiple island groups.islandKey()canonicalized SXM/Statia spelling variants but lowercased every otherlocationNameas-is, so production values likeFort Bay, Saba,Windwardside, Saba, andWindwardside / The Bottom, Sabaeach became a distinct "island" — rendered byislandDisplayName()as headings likeFort bay, saba— and the Island filter offered several choices that all meant Saba.Fix per Issue #116 Option 1 (canonical island-level grouping): the island is the final comma-separated segment of
locationName. Any<locality>, Sabavalue resolves to the canonical keysaba; matching the whole segment (not a baresabasubstring) avoids misclassifying unrelated future values likeSabana. Known keys now have intentional labels viaISLAND_DISPLAY_NAMES(saba→Saba, SXM/Statia labels unchanged); the fallback for unknown islands title-cases each word rather than only the first letter.Grouping and filtering changed — venue data did not: each card still renders the raw
locationName(e.g.Windwardside / The Bottom, Saba). No Firestore writes, no data migration.Closes #116
Changes
lib/venue-filters.ts:islandKey()maps the<locality>, Sabasuffix to"saba"(SXm/Statia paths and the empty→sabadefault unchanged);islandDisplayName()uses an explicit known-island label map plus a word-wise title-case fallback.lib/where-to-buy-fixture.ts: the three Saba fixture venues now use distinct real-world localities (Fort Bay, Saba,Windwardside, Saba,Windwardside / The Bottom, Saba) — the previous uniformSabavalues could never expose this bug.tests/lib/venue-filters.test.ts: +5 tests — every Saba variant resolves to"saba", substring-safety (Sabana Grande), canonical display label viaislandDisplayName(islandKey(...)), onesabaoption/group for mixed localities, andisland: "saba"filtering returning all Saba venues.smoke-tests/where-to-buy-filters.spec.ts: island select now asserted to contain exactlyAll islands / Saba / Sint Maarten / Saint Martin / SXM; new regression test proves oneSabah2, noFort bay, saba/Windwardside, sabaheadings, all 3 Saba cards under the Saba section, raw locality text still on cards, and thesabafilter returning all three.docs/admin/managing-locations.md: grouping rules now document the<locality>, Sabaconvention.Verification
npm cinpm run check:react-versionsnpx tsc --noEmitnpm run lintnpm test— 334 tests / 101 suites passnpm run test:rules— 29 tests / 7 suites passnpm run buildnpm run test:smoke— 117 Playwright tests pass, including the new Where to Buy fragments Saba venues into multiple sections with lowercase headings #116 regression testnpm run check:md-linksnpm audit --omit=dev— 0 vulnerabilitiesislandKey/islandDisplayName(verified by running the suite against themainversion oflib/venue-filters.ts)Risk / deployment notes
islands.length > 1rule.Generated with Devin
Summary by Sourcery
Consolidate locality-qualified Saba venues into one canonical island group while preserving their displayed location details.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: