diff --git a/README.md b/README.md index 3a08d7b..7e101a0 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,6 @@ Never commit real values to source control. `.env.local` is already ignored by G | `npm run seed:venues` | Seed sample venue data (developer script) | | `npm run bootstrap-superadmin` | Grant the initial superadmin role to `SUPER_ADMIN_EMAIL` (see docs first) | | `npm run prune:trade-leads` | Prune trade inquiries past the 24-month retention window (dry-run; `-- --delete` executes) | -| `npm run migrate:venue-islands` | Backfill the canonical venue `island` field from legacy `locationName` values (dry-run; `-- --write` executes) | ## Deployment overview diff --git a/docs/TECHNICAL.md b/docs/TECHNICAL.md index 133bafa..b3ceedf 100644 --- a/docs/TECHNICAL.md +++ b/docs/TECHNICAL.md @@ -126,7 +126,7 @@ client SDK writes (content management) or through Admin-SDK-backed API routes | `lib/` | Shared logic. Client-safe: `firebase.ts`, `beers.ts`, `venues.ts`, `analytics.ts`, `types.ts`, `utils.ts`, `email.ts`, `trade-leads-common.ts`, admin `*-common`/`admin-format.ts` helpers. Server-only (`import "server-only"`): `firebase-admin.ts`, `admin-auth.ts`, `admin-users.ts`, `admin-invitations.ts`, `admin-invitation-email.ts`, `admin-invitation-resend-core.ts`, `admin-audit.ts`, `trade-leads.ts`. Policy/serialization helpers shared by both: `admin-policy.ts`, `admin-serializers.ts`, `admin-invitation-policy.ts`, `admin-invitation-resend-policy.ts`, `admin-types.ts`. | | `tests/` | Node `node:test` unit tests (`tsx` loader) for admin/auth/invitation/audit helpers and for the *contents* of `firestore.rules` and `storage.rules`. | | `rules-tests/` | Emulator-backed security-rules tests (`@firebase/rules-unit-testing` against the Firestore/Storage emulators). Run via `npm run test:rules`, which wraps `firebase emulators:exec`; each file uses its own `demo-*` project so parallel `node:test` files stay isolated. | -| `scripts/` | Local/manual tooling: Playwright diagnostics (`*-check.mjs`, `hero-video-network.mjs`), `check-md-links.mjs`, `check-react-versions.mjs`, `optimize-assets.mjs`, `bootstrap-superadmin.ts`, `prune-trade-leads.ts`, `migrate-venue-islands.ts`, `seed-beers.ts`, `seed-venues.ts`. `check-md-links.mjs` and `check-react-versions.mjs` run in CI; the Playwright diagnostics and data scripts do not (CI browser coverage lives in `smoke-tests/`). | +| `scripts/` | Local/manual tooling: Playwright diagnostics (`*-check.mjs`, `hero-video-network.mjs`), `check-md-links.mjs`, `check-react-versions.mjs`, `optimize-assets.mjs`, `bootstrap-superadmin.ts`, `prune-trade-leads.ts`, `seed-beers.ts`, `seed-venues.ts`. `check-md-links.mjs` and `check-react-versions.mjs` run in CI; the Playwright diagnostics and data scripts do not (CI browser coverage lives in `smoke-tests/`). | | `docs/` | Admin handbook (`docs/admin/`), operations guides (`docs/operations/`: deployment, troubleshooting, post-deploy checklist), and this file. | | `content/` | Legacy placeholder (`.gitkeep` only). MDX content is co-located under `app/(pages)/`; do not add files here expecting them to render. | | `firestore.rules`, `storage.rules` | Firebase security rules — see §6/§15. | @@ -209,8 +209,7 @@ in code are listed. `tapBeerSlugs[]`/`canBeerSlugs[]`, `isPublic`, `sortOrder`, `links` (`website`/`maps`/`instagram`/`facebook`/`untappd`), `notesPublic`. Issue #134 split island identity out of `locationName`; reads fall back - to parsing `locationName` for legacy records until the - `migrate:venue-islands` backfill completes. + to parsing `locationName` for legacy documents predating the field. - **Reads:** `/where-to-buy` via `getVenues()` (`isPublic` + `sortOrder`); rules allow public reads only of `isPublic` docs; admin dashboard reads all. - **Writes:** admin dashboard `setDoc` merge keyed by `slug` (client SDK). @@ -747,7 +746,7 @@ placeholder values exist anywhere in CI. - **Local-only scripts (`scripts/`):** Playwright-based manual diagnostics (`screenshot-check`, `overflow-check`, `hero-video-*`), `optimize-assets.mjs`, and Admin-SDK utilities (`bootstrap-superadmin.ts`, - `prune-trade-leads.ts`, `migrate-venue-islands.ts`, `seed-beers.ts`, + `prune-trade-leads.ts`, `seed-beers.ts`, `seed-venues.ts`). These remain manual/local; the CI smoke suite lives in `smoke-tests/`. - **Verification parity:** local pre-PR checks are the same commands CI diff --git a/docs/admin/managing-locations.md b/docs/admin/managing-locations.md index d9b2a25..ccadba1 100644 --- a/docs/admin/managing-locations.md +++ b/docs/admin/managing-locations.md @@ -57,7 +57,7 @@ Island is required; a venue cannot be saved without one, and free-text island na **Location / locality** is a separate, optional free-text field for a more specific place — `Windwardside`, `The Bottom`, `Fort Bay`, `Philipsburg`. It is shown on the venue card together with the island (for example `Windwardside, Saba` or `Philipsburg, Sint Maarten`) but never affects grouping or the Island filter. Leave it blank when the island alone is enough — the card then shows just the island label. -> **Why two fields?** Locality text used to double as the island, which let values like `Philipsburg` or `Windwardside` accidentally become their own public island group. Older records were migrated so `locationName` holds only the locality; reads still understand the old combined format until every record is migrated, so nothing breaks in between. +> **Why two fields?** Locality text used to double as the island, which let values like `Philipsburg` or `Windwardside` accidentally become their own public island group. Existing records now carry a separate canonical island and `locationName` holds only the locality; reads still understand the old combined format, so a stale record would not break the site. ## How locations appear on `/where-to-buy` diff --git a/lib/types.ts b/lib/types.ts index 1ab3218..9e9359e 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -27,7 +27,7 @@ export interface Venue { locationName: string; // Canonical island key ("saba" | "sxm" | "statia"). Optional only because // legacy documents predate the field; reads fall back to parsing - // `locationName` until migration completes (see lib/venue-filters.ts). + // `locationName` for those records (see lib/venue-filters.ts). island?: VenueIsland; carriesBeerSlugs: string[]; tapBeerSlugs?: string[]; diff --git a/lib/venue-filters.ts b/lib/venue-filters.ts index c0b6164..d143241 100644 --- a/lib/venue-filters.ts +++ b/lib/venue-filters.ts @@ -112,8 +112,7 @@ export function islandKey(locationName: string | undefined): string { * Canonical island key for a venue: the stored `island` field when present, * falling back to legacy `locationName` inference for records written before * Issue #134. New writes always set `island`, so grouping no longer depends - * on parsing locality text — the fallback exists only until migration - * completes. + * on parsing locality text — the fallback remains for legacy records. */ export function venueIslandKey(venue: Venue): string { return isVenueIsland(venue.island) @@ -134,57 +133,6 @@ export function resolveVenueIsland(venue: Venue): VenueIsland | undefined { return isVenueIsland(legacy) ? legacy : undefined; } -/** - * Known-locality → island map for the venue migration (Issue #134). Only - * entries that are unambiguous within the brewery's operating region belong - * here — a bare locality string can never reach this map through inference. - * "Oranjestad" is deliberately absent: it is also the capital of Aruba, so a - * bare "Oranjestad" record stays unresolved and is reported for owner review - * rather than guessed. - */ -const KNOWN_LOCALITY_ISLANDS: ReadonlyMap = new Map([ - ["windwardside", "saba"], - ["the bottom", "saba"], - ["fort bay", "saba"], - ["philipsburg", "sxm"], -]); - -export interface VenueGeography { - island: VenueIsland; - locality: string; -} - -/** - * Split a legacy `locationName` into canonical `{ island, locality }` for - * the Issue #134 backfill. ", " forms keep their locality - * prefix; a bare island name ("Saba", "SXM", "Sint Maarten") yields an empty - * locality; a bare locality is mapped only via KNOWN_LOCALITY_ISLANDS. - * Returns null when the value cannot be classified confidently — callers - * must report it for owner review instead of writing a guess. - */ -export function resolveVenueGeography( - locationName: string | undefined -): VenueGeography | null { - const parts = (locationName ?? "") - .split(",") - .map((part) => part.trim()) - .filter((part) => part !== ""); - - // The legacy Saba default for an empty location, preserved verbatim. - if (parts.length === 0) return { island: "saba", locality: "" }; - - const lastKey = islandKey(parts[parts.length - 1]); - if (isVenueIsland(lastKey)) { - return { island: lastKey, locality: parts.slice(0, -1).join(", ") }; - } - - const whole = parts.join(", "); - const known = KNOWN_LOCALITY_ISLANDS.get(whole.toLowerCase()); - if (known) return { island: known, locality: whole }; - - return null; -} - /** * Display heading for a canonical island key. Known islands return their * canonical label; an unknown future island key title-cases each word @@ -240,7 +188,7 @@ export function distinctIslands(venues: Venue[]): string[] { * the canonical `island` field means a locality like "Philipsburg" or * "Windwardside" can never become its own section (Issue #134); the legacy * `locationName` fallback inside `venueIslandKey` still merges spelling - * variants like "SXM" and "Sint Maarten" for unmigrated records. + * variants like "SXM" and "Sint Maarten" for records predating the field. */ export function groupVenuesByIsland( venues: Venue[] diff --git a/package.json b/package.json index 8918014..e9fb421 100644 --- a/package.json +++ b/package.json @@ -19,8 +19,7 @@ "seed:venues": "tsx scripts/seed-venues.ts", "optimize-assets": "node scripts/optimize-assets.mjs", "bootstrap-superadmin": "node --env-file=.env.local --import tsx scripts/bootstrap-superadmin.ts", - "prune:trade-leads": "node --env-file=.env.local --import tsx scripts/prune-trade-leads.ts", - "migrate:venue-islands": "node --env-file=.env.local --import tsx scripts/migrate-venue-islands.ts" + "prune:trade-leads": "node --env-file=.env.local --import tsx scripts/prune-trade-leads.ts" }, "dependencies": { "@mdx-js/loader": "^3.1.1", diff --git a/scripts/migrate-venue-islands.ts b/scripts/migrate-venue-islands.ts deleted file mode 100644 index 4bef1c2..0000000 --- a/scripts/migrate-venue-islands.ts +++ /dev/null @@ -1,190 +0,0 @@ -/** - * Backfill the canonical `island` field on `venues` documents (Issue #134). - * - * Each legacy record encodes the island inside the free-text `locationName` - * (e.g. "Windwardside, Saba", "Sint Maarten"). The migration splits that into - * `{ island, locationName: locality }` via `resolveVenueGeography` in - * lib/venue-filters.ts. Records whose location can't be classified - * confidently — and records whose stored `island` disagrees with the parsed - * location — are reported for owner review and never overwritten. - * - * Usage: - * npm run migrate:venue-islands # dry run — prints the plan - * npm run migrate:venue-islands -- --write # apply the planned updates - * - * Prerequisites: - * - FIREBASE_ADMIN_* credentials in .env.local (loaded via --env-file) - * - Operator-level Firebase access - * - * Safety: dry-run is the default; writes require the explicit --write flag. - * The script is idempotent — already-migrated records are reported as - * unchanged and skipped. Output is limited to venue slugs/names/locations, - * which are public business data. - */ - -import { cert, getApps, initializeApp, type App } from "firebase-admin/app"; -import { getFirestore } from "firebase-admin/firestore"; -import { resolveVenueGeography } from "@/lib/venue-filters"; -import { isVenueIsland } from "@/lib/venue-islands"; -import type { Venue } from "@/lib/types"; - -const WRITE = process.argv.includes("--write"); - -function getPrivateKey() { - const key = process.env.FIREBASE_ADMIN_PRIVATE_KEY; - if (!key) return ""; - return key.replace(/\\n/g, "\n"); -} - -function getFirebaseAdminApp(): App { - const existing = getApps()[0]; - if (existing) return existing; - - const projectId = - process.env.FIREBASE_ADMIN_PROJECT_ID ?? - process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID; - const clientEmail = process.env.FIREBASE_ADMIN_CLIENT_EMAIL; - const privateKey = getPrivateKey(); - - if (!projectId || !clientEmail || !privateKey) { - throw new Error( - "Missing Firebase Admin credentials. Set FIREBASE_ADMIN_PROJECT_ID, FIREBASE_ADMIN_CLIENT_EMAIL, and FIREBASE_ADMIN_PRIVATE_KEY." - ); - } - - return initializeApp({ - credential: cert({ projectId, clientEmail, privateKey }), - }); -} - -interface PlannedUpdate { - slug: string; - island: string; - locality: string; -} - -async function main() { - const db = getFirestore(getFirebaseAdminApp()); - const snapshot = await db.collection("venues").orderBy("sortOrder", "asc").get(); - - console.log( - `${WRITE ? "WRITE" : "DRY-RUN"} — venue island migration (Issue #134)` - ); - console.log(`Scanned ${snapshot.size} venue(s).`); - - const updates: PlannedUpdate[] = []; - const conflicts: string[] = []; - const ambiguous: string[] = []; - let unchanged = 0; - - for (const docSnap of snapshot.docs) { - const venue = docSnap.data() as Venue; - const resolved = resolveVenueGeography(venue.locationName); - const label = `${venue.name ?? docSnap.id} (${docSnap.id})`; - - if (isVenueIsland(venue.island)) { - if (resolved && resolved.island !== venue.island) { - conflicts.push( - `${label}: stored island "${venue.island}" disagrees with locationName "${venue.locationName}" (resolves to "${resolved.island}")` - ); - continue; - } - const locality = resolved ? resolved.locality : venue.locationName; - if (!resolved) { - // Island is already canonical; the location text is a pure locality - // the map doesn't know — safe to keep verbatim. - console.log(` = ${label}: island "${venue.island}", locality "${venue.locationName}" (kept as-is)`); - unchanged++; - continue; - } - if (venue.locationName === locality) { - console.log(` = ${label}: already migrated (island "${venue.island}", locality "${locality}")`); - unchanged++; - continue; - } - updates.push({ slug: docSnap.id, island: venue.island, locality }); - console.log( - ` ~ ${label}: island "${venue.island}" kept; locationName "${venue.locationName}" → locality "${locality}"` - ); - continue; - } - - if (!resolved) { - ambiguous.push(`${label}: locationName "${venue.locationName}"`); - continue; - } - - updates.push({ slug: docSnap.id, island: resolved.island, locality: resolved.locality }); - console.log( - ` + ${label}: locationName "${venue.locationName}" → island "${resolved.island}", locality "${resolved.locality}"` - ); - } - - console.log( - `\nPlan: ${updates.length} to update, ${unchanged} unchanged, ${conflicts.length} conflict(s), ${ambiguous.length} ambiguous.` - ); - - for (const line of conflicts) { - console.log(` CONFLICT ${line}`); - } - for (const line of ambiguous) { - console.log(` AMBIGUOUS ${line}`); - } - - if (conflicts.length > 0 || ambiguous.length > 0) { - console.log( - "\nConflicting/ambiguous records were NOT changed — review them with the owner and re-run." - ); - } - - if (!WRITE) { - if (updates.length > 0) { - console.log("\nDry run only — re-run with `--write` to apply these updates."); - } - return; - } - - if (updates.length === 0) { - console.log("Nothing to write."); - return; - } - - const collection = db.collection("venues"); - let written = 0; - for (const update of updates) { - // Re-verify inside a transaction so a venue edited between scan and - // write keeps its newer island value rather than being overwritten. - const ref = collection.doc(update.slug); - const applied = await db.runTransaction(async (tx) => { - const fresh = await tx.get(ref); - const data = (fresh.data() ?? {}) as Venue; - const reResolved = resolveVenueGeography(data.locationName); - if ( - !reResolved || - reResolved.island !== update.island || - reResolved.locality !== update.locality || - (isVenueIsland(data.island) && data.island !== update.island) - ) { - return false; - } - tx.set( - ref, - { island: update.island, locationName: update.locality }, - { merge: true } - ); - return true; - }); - if (applied) { - written++; - console.log(` wrote ${update.slug}`); - } else { - console.log(` skipped ${update.slug} (island changed since scan)`); - } - } - console.log(`Updated ${written} venue(s).`); -} - -main().catch((err) => { - console.error("Migration failed:", err); - process.exit(1); -}); diff --git a/tests/lib/venue-filters.test.ts b/tests/lib/venue-filters.test.ts index 5b0f959..4ea4dfe 100644 --- a/tests/lib/venue-filters.test.ts +++ b/tests/lib/venue-filters.test.ts @@ -10,7 +10,6 @@ import { islandDisplayName, islandKey, parseVenueFilters, - resolveVenueGeography, resolveVenueIsland, sanitizeVenueFilters, venueCardLocation, @@ -201,78 +200,6 @@ describe("resolveVenueIsland (admin editor preselection)", () => { }); }); -describe("resolveVenueGeography (migration mapping)", () => { - it("splits ', Saba' forms into island + locality", () => { - assert.deepEqual(resolveVenueGeography("Saba"), { - island: "saba", - locality: "", - }); - assert.deepEqual(resolveVenueGeography("Windwardside, Saba"), { - island: "saba", - locality: "Windwardside", - }); - assert.deepEqual(resolveVenueGeography("Fort Bay, Saba"), { - island: "saba", - locality: "Fort Bay", - }); - assert.deepEqual(resolveVenueGeography("Windwardside / The Bottom, Saba"), { - island: "saba", - locality: "Windwardside / The Bottom", - }); - }); - - it("maps SXM legacy spellings to sxm with an empty locality", () => { - for (const value of ["SXM", "Sint Maarten", "Saint Martin"]) { - assert.deepEqual(resolveVenueGeography(value), { - island: "sxm", - locality: "", - }); - } - }); - - it("maps a bare Philipsburg locality to sxm, preserving the locality", () => { - assert.deepEqual(resolveVenueGeography("Philipsburg"), { - island: "sxm", - locality: "Philipsburg", - }); - }); - - it("maps known bare Saba localities to saba", () => { - for (const locality of ["Windwardside", "The Bottom", "Fort Bay"]) { - assert.deepEqual(resolveVenueGeography(locality), { - island: "saba", - locality, - }); - } - }); - - it("splits ', Statia' forms", () => { - assert.deepEqual(resolveVenueGeography("Oranjestad, Statia"), { - island: "statia", - locality: "Oranjestad", - }); - }); - - it("returns null for values that cannot be classified confidently", () => { - // Bare "Oranjestad" is genuinely ambiguous (also Aruba's capital); - // unknown islands stay unresolved — never guessed. - assert.equal(resolveVenueGeography("Oranjestad"), null); - assert.equal(resolveVenueGeography("Sabana Grande"), null); - assert.equal(resolveVenueGeography("Bonaire"), null); - }); - - it("keeps the legacy Saba default for an empty location", () => { - assert.deepEqual(resolveVenueGeography(""), { - island: "saba", - locality: "", - }); - assert.deepEqual(resolveVenueGeography(undefined), { - island: "saba", - locality: "", - }); - }); -}); - describe("venueCardLocation", () => { it("composes locality + short island label for migrated venues", () => { assert.equal(