Skip to content

Separate venue island from locality in admin and public grouping (#134) - #137

Merged
spizeck merged 2 commits into
mainfrom
fix/issue-134-venue-island-locality
Sep 24, 2026
Merged

spizeck merged 2 commits into
mainfrom
fix/issue-134-venue-island-locality

Conversation

@spizeck

@spizeck spizeck commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

The venue model overloaded free-text locationName for two concepts: the island identity used for /where-to-buy grouping/filtering, and the locality (Windwardside, Fort Bay, Philipsburg). Valid locality text could accidentally become a public island group. This PR separates them: a required canonical island field (saba | sxm | statia) drives all grouping, while locationName becomes a pure free-text locality.

Closes #134

Changes

  • lib/venue-islands.ts (new): single canonical vocabulary — VENUE_ISLAND_KEYS, public labels (saba → Saba, sxm → Sint Maarten / Saint Martin, statia → Sint Eustatius / Statia), short card labels (Philipsburg, Sint Maarten, never …, Sint Maarten / Saint Martin), admin select options, isVenueIsland guard.
  • lib/types.ts: Venue.island?: VenueIsland (optional because legacy docs predate it).
  • lib/venue-filters.ts: venueIslandKey() prefers venue.island, falls back to legacy locationName inference (transitional — retained until every record is migrated); distinctIslands/groupVenuesByIsland/filterVenues all use it. resolveVenueIsland() preselects the island in the admin editor for legacy records; resolveVenueGeography() is the migration mapping; venueCardLocation() composes card text (Windwardside, Saba / Philipsburg, Sint Maarten, never Saba, Saba).
  • components/admin-workspace.tsx: Island is a required <select> driven by VENUE_ISLAND_OPTIONS; Location / locality stays free text.
  • components/admin-dashboard.tsx / admin-fixture.tsx: legacy records load with the inferred island preselected; saveVenue rejects non-canonical island values before writing.
  • components/venue-card.tsx: card shows the composed locality+island; the island analytics param now emits the canonical key instead of raw free text (matching the documented "stable identifiers" contract in docs/operations/analytics.md).
  • firestore.rules: venue create/update requires island in ['saba','sxm','statia'] — server-side rejection of arbitrary islands, not just the <select>. Delete is unaffected (request.resource is null there).
  • scripts/migrate-venue-islands.ts + npm run migrate:venue-islands: dry-run by default, --write applies; idempotent (skips already-migrated records), transactionally re-verifies each doc before writing, and reports conflicts/ambiguous records instead of guessing.
  • Fixtures/tests: fixture venues carry island + locality-only locationName (Harbor Bar = sxm + Philipsburg; Quiet Cafe deliberately left legacy to cover the fallback); new unit tests for the vocabulary, canonical-first grouping, card composition, admin preselection, and migration mappings; rules tests cover allow/deny; smoke spec gains a "locality never becomes an island option" regression test.
  • Docs: managing-locations.md rewritten for Island/locality; TECHNICAL.md venue model, admin-flow, and scripts tables updated; README script table updated.
  • Seed data (scripts/venue-seed-data.json) migrated to island: "saba" + blank locality.

Verification

  • npm ci
  • npm run check:react-versions
  • npx tsc --noEmit
  • npm run lint
  • npm test — 379 pass (incl. new island vocabulary, canonical grouping, card display, migration mapping tests)
  • npm run test:rules — 30 pass (incl. new island allowlist allow/deny)
  • npm run build
  • npx playwright test — 124 pass (incl. new locality-never-an-island-option test; Align venue card actions and hide venues with no current beer inventory #130 unstocked-venue exclusion intact)
  • npm run check:md-links
  • npm audit --omit=dev — 0 vulnerabilities
  • Vercel preview reviewed (for UI changes)

Risk / deployment notes

  • Production data audit (read-only): all 13 venues classify cleanly — 12 × Saba localities (Saba, Windwardside, Saba, Fort Bay, Saba, Windwardside / The Bottom, Saba), 1 × Sint Maarten. No ambiguous or conflicting records. Dry-run output: 13 to update, 0 unchanged, 0 conflicts, 0 ambiguous.
  • No production data was modified. Migration is applied manually post-merge via npm run migrate:venue-islands -- --write (dry-run default). Reads are already compatible either way.
  • Firestore rules deploy ordering: rules deploy via firebase deploy, separately from the site. Deploy the site first — once this ships, every admin venue save writes island, so rules can tighten without breaking saves. (Rules-first would briefly reject saves from a stale admin bundle.)
  • Backward compatibility: unmigrated records keep working publicly via the locationName inference fallback, and the admin editor preselects the inferred island so saving a legacy record writes the canonical field. Do not remove the fallback until the backfill is confirmed complete.
  • No secrets, credentials, or private data were committed.

Generated with Devin

Summary by Sourcery

Separate canonical venue island identity from free-text locality data throughout administration and public venue listings.

New Features:

  • Introduce canonical venue island identifiers and labels for grouping, filtering, administration, cards, and analytics.
  • Add a safe migration command to backfill island and locality data from legacy venue records.

Bug Fixes:

  • Prevent free-text localities from appearing as public island groups or filter options.
  • Preserve compatibility with unmigrated venue records through legacy inference and admin preselection.

Enhancements:

  • Separate the required island field from the free-text locality field across venue management and public display.
  • Enforce canonical island values in Firestore rules and validate venue saves in the admin interfaces.

Deployment:

  • Document the post-merge venue data migration and Firestore rules deployment considerations.

Documentation:

  • Update venue management, technical, and README documentation for the island/locality model and migration script.

Tests:

  • Add coverage for canonical island vocabulary, grouping and card composition, migration mappings, admin behavior, Firestore rules, and locality regression scenarios.

Chores:

  • Update fixtures and seed data to use canonical island fields with locality-only location names.

The free-text locationName field doubled as island identity and locality,
which let localities like "Philipsburg" or "Windwardside" surface as public
island groups. Venues now carry a canonical `island` field
(saba | sxm | statia) that drives /where-to-buy grouping, the island filter,
and the card's composed "locality, island" line; `locationName` is now a
pure free-text locality. Reads keep the legacy locationName inference as a
transitional fallback until the migrate:venue-islands backfill runs.

- lib/venue-islands.ts: canonical key/label/options vocabulary shared by
  the admin select, rules allowlist, and public display
- Admin venue form: required Island select + separate locality input;
  legacy records preselect the inferred island; save rejects non-canonical
  values
- firestore.rules: venue create/update requires a canonical island
- scripts/migrate-venue-islands.ts: dry-run/--write backfill; ambiguous or
  conflicting records are reported, never guessed
- Venue card: "Philipsburg, Sint Maarten"-style jurisdiction-aware display;
  analytics island param is now the stable canonical key

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @spizeck, this account has used its review budget of 1,500,000 diff characters for the last 7 days.

You can request another review in 4 hours and 55 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@vercel

vercel Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
deepdivebrewing-web Ready Ready Preview Sep 24, 2026 2:45pm UTC

Request Review

@sourcery-ai

sourcery-ai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR replaces free-text venue location grouping with a canonical, validated island field while retaining a legacy-read fallback, updates admin/public presentation and analytics accordingly, enforces the allowlist in Firestore rules, and supplies a dry-run-safe transactional migration plus comprehensive regression coverage.

Sequence diagram for legacy venue migration and admin save

sequenceDiagram
  actor Admin
  participant Dashboard
  participant Filters
  participant Firestore
  participant Rules

  Dashboard->>Firestore: load venue
  Dashboard->>Filters: resolveVenueIsland(venue)
  Filters-->>Dashboard: canonical island or undefined
  Admin->>Dashboard: select island and edit locality
  Dashboard->>Dashboard: isVenueIsland(venueForm.island)
  Dashboard->>Firestore: setDoc venue with island and locationName
  Firestore->>Rules: validate island
  Rules-->>Firestore: allow canonical key or reject write
Loading

Flow diagram for safe venue island migration

flowchart TD
  Start[Scan venues] --> Resolve["resolveVenueGeography(locationName)"]
  Resolve --> Classified{Classified without conflict?}
  Classified -->|No| Report[Report ambiguous or conflicting record]
  Classified -->|Yes| Plan[Add island and locality to update plan]
  Plan --> WriteFlag{--write supplied?}
  WriteFlag -->|No| DryRun[Print dry-run plan]
  WriteFlag -->|Yes| Verify[Re-read venue in transaction]
  Verify --> Changed{Still matches plan?}
  Changed -->|No| Skip[Skip changed record]
  Changed -->|Yes| Apply[Write island and locality]
  Report --> Done[Finish without guessing]
  DryRun --> Done
  Skip --> Done
  Apply --> Done
Loading

File-Level Changes

Change Details Files
Introduces a single canonical island vocabulary and separates island identity from free-text locality.
  • Adds canonical keys, public labels, card labels, admin options, and runtime validation.
  • Adds optional Venue.island for legacy compatibility while documenting the new data model.
lib/venue-islands.ts
lib/types.ts
docs/admin/managing-locations.md
docs/TECHNICAL.md
Updates venue grouping, filtering, display, analytics, and admin editing to use canonical island values.
  • Prefers venue.island for grouping and filtering, with legacy locationName inference as a transitional fallback.
  • Composes locality and jurisdiction-aware island labels for cards without duplicate or dual-jurisdiction text.
  • Requires and validates island selection in both production and fixture admin flows; emits canonical analytics identifiers.
lib/venue-filters.ts
components/admin-workspace.tsx
components/admin-dashboard.tsx
components/admin-fixture.tsx
components/venue-card.tsx
lib/where-to-buy-fixture.ts
Adds server-side enforcement preventing invalid island values from being written.
  • Allows venue creates and updates only when the merged document contains saba, sxm, or statia.
  • Adds emulator and source-level rules coverage for valid, missing, arbitrary, and preserved island values.
firestore.rules
rules-tests/firestore.rules.test.ts
tests/security-rules.test.ts
Provides a safe, explicitly invoked migration for legacy venue records.
  • Defaults to dry-run and maps combined or known locality values into canonical island plus locality fields.
  • Skips migrated records, reports ambiguous/conflicting records, and rechecks documents transactionally before writes.
scripts/migrate-venue-islands.ts
package.json
README.md
scripts/venue-seed-data.json
Expands regression coverage for canonical grouping and locality presentation.
  • Adds unit tests for vocabulary, fallback resolution, migration mappings, card composition, and canonical-first filtering/grouping.
  • Adds browser coverage ensuring localities do not become island options and cards use the expected Sint Maarten label.
  • Updates fixtures to cover migrated records alongside a legacy record.
tests/lib/venue-islands.test.ts
tests/lib/venue-filters.test.ts
smoke-tests/where-to-buy-filters.spec.ts
lib/where-to-buy-fixture.ts

Assessment against linked issues

Issue Objective Addressed Explanation
#134 Introduce a separate canonical island field with controlled, customer-facing labels, while retaining locality as an independently editable free-text field in the venue admin editor. ✅
#134 Migrate existing venue records safely by mapping legacy location values to canonical islands, preserving locality data, reporting ambiguous or conflicting records, and retaining compatibility for unmigrated records. ✅
#134 Use the canonical island field for public venue grouping and filtering, ensure localities such as Philipsburg and Windwardside cannot become island options or headings, preserve useful composed card display and canonical labels, and keep beer/format filtering and counts working. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@spizeck
spizeck merged commit ad3492f into main Sep 24, 2026
4 checks passed
@spizeck
spizeck deleted the fix/issue-134-venue-island-locality branch September 24, 2026 14:48

This branch was successfully deployed

1 active deployment
Preview — 7763eef7 Deployed Sep 24, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate venue island from locality in admin and public grouping

1 participant