Skip to content

Conversation

@egemengunel
Copy link
Contributor

@egemengunel egemengunel commented Sep 10, 2025

This PR adds venue imports directly from Google Maps.
Closes #417

Summary by CodeRabbit

  • New Features

    • Adding a venue now auto-detects/creates its city from Google Maps and ensures a unique venue handle.
    • More robust Google Maps place and city lookups for smoother venue onboarding.
  • Changes

    • Simpler flow when adding a venue (only the Place ID is required).
  • Breaking Changes

    • Legacy direct messaging has been removed; existing conversations are no longer available.
    • Deleting a profile now also removes its calendars.
  • Documentation

    • Renamed environment variable for Maps: use NUXT_GOOGLE_MAPS_API_KEY.

@vercel
Copy link

vercel bot commented Sep 10, 2025

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

Project Deployment Preview Comments Updated (UTC)
v4 Ready Ready Preview Comment Sep 18, 2025 0:57am

@coderabbitai
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Updates venue creation to rely on Google Place details and city resolution. Introduces city find-or-create logic and Google Maps utilities. Adjusts TRPC routers to new APIs and simplifies client payload. Renames env key for frontend. Removes Conversation/Message models from Prisma and adds cascade delete to Calendar.profile.

Changes

Cohort / File(s) Summary
TRPC: Profiles & Cities
server/trpc/routers/profiles.ts, server/trpc/routers/cities.ts
Profiles: mutation now accepts only placeId, fetches place details, derives city via Google, resolves/creates city, generates unique slug, and creates/returns venue; uses TRPCError. Cities: switches from addCity(city) to findOrCreateCity(city.id).
Google Maps Utilities
server/utils/google_maps.ts
Adds getCityIdFromGooglePlace(place) and getPlaceDetails(placeId) using runtime config API key and Google Maps client; returns IDs/details with error handling.
City Utilities
server/utils/city.ts
Replaces addCity with findOrCreateCity(placeId); early return if exists, validates address, computes slug with conflict handling, includes countryCode, and upserts by placeId.
Client Input
components/inputs/VenueInput.vue
Mutation call updated to pass only { placeId }; removes googleMapsPlace from payload.
Environment
.env.example
Renames frontend key to NUXT_GOOGLE_MAPS_API_KEY; keeps backend key unchanged.
Prisma Schema
prisma/schema.prisma
Removes Conversation and Message models and related Profile fields; adds onDelete: Cascade to Calendar.profile relation; minor formatting tweaks.

Sequence Diagram(s)

sequenceDiagram
  participant U as UI (VenueInput)
  participant T as TRPC profiles.findVenueOrCreate
  participant G as Google Maps Utils
  participant C as City Utils
  participant DB as Prisma

  U->>T: mutate({ placeId })
  T->>G: getPlaceDetails(placeId)
  alt invalid/missing place
    T-->>U: TRPCError(BAD_REQUEST)
  else valid place
    T->>DB: $transaction(...)
    activate DB
    DB->>DB: find venue by placeId
    alt venue exists
      DB-->>T: existing venue
    else create new venue
      T->>G: getCityIdFromGooglePlace(place)
      alt no cityId
        DB-->>T: rollback
        T-->>U: TRPCError(BAD_REQUEST)
      else cityId found
        T->>C: findOrCreateCity(cityId)
        C->>DB: find city by id
        alt city exists
          DB-->>C: city
        else
          C->>G: getAddressFromPlaceId(cityId)
          C->>DB: upsert City (slug handling)
          DB-->>C: city
        end
        C-->>T: city
        T->>DB: create Venue (unique slug attempts)
        DB-->>T: venue
      end
    end
    deactivate DB
    T-->>U: venue
  end
Loading
sequenceDiagram
  participant A as TRPC cities.create
  participant C as City Utils
  participant DB as Prisma
  A->>C: findOrCreateCity(cityPlaceId)
  C->>DB: find by id / upsert with address + slug
  DB-->>A: city
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Chat feature #414 — Conflicts with this PR at Prisma schema level: it adds Conversation/Message while this PR removes them.
  • feat: Calendar Sync #413 — Related Prisma changes around Calendar relations; this PR adds onDelete: Cascade and may intersect with that schema work.

Suggested reviewers

  • razbakov

Poem

I hop through maps with whiskers bright,
Plucking placeIds in moonlit night.
A city found, a venue spun,
Slugs lined up—now choose the one!
Chats hop away, calendars stay—
Carrots for code, we ship today. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes unrelated schema changes in prisma/schema.prisma: removal of Conversation and Message models and their Profile relation fields, plus a change to Calendar.profile onDelete behavior; these database-model removals and cascade semantics are not required by issue [#417] (venue import) and represent out-of-scope, high-impact changes. Remove the unrelated Prisma schema changes from this PR and open a separate, focused PR for chat/schema migration (or provide a clear justification, backwards-compatibility plan, and migration steps/tests in this PR), and ensure DB migration and downstream code are handled before merging.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: add venue import from Google Maps" is concise, focused, and accurately summarizes the primary change in the changeset (adding Google Maps–based venue import behavior across client, server, and utilities), so it clearly communicates the main intent to reviewers.
Linked Issues Check ✅ Passed The PR implements the linked issue [#417]: the client VenueInput now sends a place_id, the server fetches place details (getPlaceDetails), checks for an existing venue by placeId and returns it if present, and otherwise creates a new Venue record with city resolution via getCityIdFromGooglePlace and findOrCreateCity, satisfying the requirement to add selected Google Maps places to the database when absent.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch event-editor-venue

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff95814 and a1ec31d.

📒 Files selected for processing (2)
  • .env.example (1 hunks)
  • server/utils/google_maps.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .env.example
  • server/utils/google_maps.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci
  • GitHub Check: test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
nuxt.config.ts (1)

132-155: Update runtimeConfig and key usage for Maps API

  • In nuxt.config.ts (lines 132–155), move googleMapsApiKey out of public into the top-level runtimeConfig (e.g. runtimeConfig.googleMapsApiKey = process.env.GOOGLE_MAPS_API_KEY) and set runtimeConfig.public.googleMapsApiKey = process.env.NUXT_PUBLIC_GOOGLE_MAPS_API_KEY instead.
  • In server code (server/utils/google_maps.ts:32, server/utils/city.ts:66) and lib/googleMapsApi.ts:10, use config.googleMapsApiKey rather than config.public.googleMapsApiKey.
  • Ensure CI/CD exports both NUXT_PUBLIC_GOOGLE_MAPS_API_KEY (HTTP-restricted) and GOOGLE_MAPS_API_KEY (IP/service-restricted).
  • Verify your Prisma schema file defines @unique on City.slug.
🧹 Nitpick comments (9)
server/utils/google_maps.ts (3)

16-23: Tighten city component detection (include postal_town; de-prioritize admin levels).

Administrative levels can resolve to states/regions (e.g., “California”). Prefer locality-like components first, then neighborhood, then admins as last resort. Add postal_town for UK.

-  const cityComponent = place.address_components.find(
-    (c) =>
-      c.types.includes(AddressType.locality) ||
-      c.types.includes(AddressType.administrative_area_level_1) ||
-      c.types.includes(AddressType.administrative_area_level_2) ||
-      c.types.includes(AddressType.sublocality) ||
-      c.types.includes(AddressType.neighborhood)
-  )
+  const cityComponent =
+    place.address_components.find((c) => c.types.includes(AddressType.locality)) ||
+    place.address_components.find((c) => c.types.includes('postal_town' as any)) ||
+    place.address_components.find((c) => c.types.includes(AddressType.sublocality)) ||
+    place.address_components.find((c) => c.types.includes(AddressType.neighborhood)) ||
+    place.address_components.find((c) => c.types.includes(AddressType.administrative_area_level_2)) ||
+    place.address_components.find((c) => c.types.includes(AddressType.administrative_area_level_1))

7-12: Add explicit return type alias and JSDoc for clarity.

Small DX improvement: define a type PlaceId = string and document that empty string means “not found”.

-export async function getCityIdFromGooglePlace(
-  place: Partial<PlaceData>
-): Promise<string> {
+/**
+ * Returns Place ID for a city inferred from a place; '' when not found.
+ */
+export async function getCityIdFromGooglePlace(
+  place: Partial<PlaceData>
+): Promise<string> {

14-15: Set a single reusable client instance.

Creating a new Client per call is unnecessary. Hoist a shared instance to reduce overhead.

-  const client = new Client({})
+  // module-level singleton:
+  const client = new Client({})

Or move const client = new Client({}) to module scope.

server/utils/city.ts (3)

22-47: Locality extraction: include postal_town; admin-level fallbacks last.

For UK/IE, postal_town is often the only city-like component. Also, picking admin levels can yield regions instead of cities.

-  // Find the most specific name for the locality by checking multiple types.
+  // Find the most specific locality-like name; prefer actual localities.
   const nameComponent =
-    place.address_components.find((c: any) => c.types.includes('locality')) ||
+    place.address_components.find((c: any) => c.types.includes('locality')) ||
+    place.address_components.find((c: any) => c.types.includes('postal_town')) ||
     place.address_components.find((c: any) =>
       c.types.includes('sublocality_level_1')
     ) ||
     place.address_components.find((c: any) =>
       c.types.includes('neighborhood')
     ) ||
     place.address_components.find((c: any) =>
       c.types.includes('administrative_area_level_2')
     ) ||
     place.address_components.find((c: any) =>
       c.types.includes('administrative_area_level_1')
     )

127-161: Use private server key inside getAddressFromPlaceId and encode parameters.

getAddressFromPlaceId (defined earlier in this file) reads useRuntimeConfig().public.googleMapsApiKey and interpolates placeId unencoded. Switch to private key and encodeURIComponent(placeId).

Outside this hunk, update:

const apiKey = useRuntimeConfig().googleMapsServerApiKey
const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${apiKey}`

Also consider $fetch (ofetch) instead of node-fetch to align with Nitro and reduce deps.


22-47: Latitude/longitude accessors may differ across APIs; ensure numbers.

Geocoding JSON returns numbers, but if this ever gets wired to Places JS objects, geometry.location exposes lat()/lng() functions. Normalize to numbers defensively.

-  if (place.geometry?.location) {
-    result.lat = place.geometry.location.lat
-    result.lng = place.geometry.location.lng
-  }
+  if (place.geometry?.location) {
+    const loc = place.geometry.location
+    // support both plain numbers and LatLng-like objects
+    result.lat = typeof loc.lat === 'function' ? loc.lat() : loc.lat
+    result.lng = typeof loc.lng === 'function' ? loc.lng() : loc.lng
+  }
server/trpc/routers/profiles.ts (3)

367-368: Consider data sanitization for external inputs.

The website and phone fields are directly assigned from Google Maps data. Consider validating or sanitizing these values to ensure they meet your application's requirements.

-            website: googleMapsPlace.website,
-            phone: googleMapsPlace.international_phone_number,
+            website: googleMapsPlace.website?.startsWith('http') ? googleMapsPlace.website : null,
+            phone: googleMapsPlace.international_phone_number?.replace(/[^\d+\-\s()]/g, '') || null,

375-378: Consider providing more specific error messages.

The generic error re-throw might not provide enough context to users. Consider wrapping the error with a more specific TRPC error.

       } catch (error) {
         console.error('Error creating venue:', error)
-        throw error
+        throw new TRPCError({
+          code: 'INTERNAL_SERVER_ERROR',
+          message: 'Failed to create venue. Please try again.',
+          cause: error,
+        })
       }

317-317: Remove or conditionally include console.log statements.

Debug logging should be removed or wrapped in a development environment check before merging to production.

-      console.log('Place types:', placeTypes)
+      if (process.env.NODE_ENV === 'development') {
+        console.log('Place types:', placeTypes)
+      }

Similarly for line 346:

-          console.log('Could not determine city.')
+          if (process.env.NODE_ENV === 'development') {
+            console.log('Could not determine city.')
+          }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a971c6 and 7d1e46c.

📒 Files selected for processing (4)
  • nuxt.config.ts (1 hunks)
  • server/trpc/routers/profiles.ts (3 hunks)
  • server/utils/city.ts (2 hunks)
  • server/utils/google_maps.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
server/utils/google_maps.ts (2)
cli/utils/google_maps.ts (1)
  • getCityId (56-88)
lib/googleMapsApi.ts (1)
  • loadGoogleMapsApi (9-31)
nuxt.config.ts (1)
lib/googleMapsApi.ts (1)
  • loadGoogleMapsApi (9-31)
server/utils/city.ts (4)
server/prisma.ts (1)
  • prisma (5-8)
utils/slug.ts (1)
  • getSlug (11-88)
utils/country.ts (1)
  • getCountryCode (22-49)
cli/importer/profile.ts (1)
  • addCity (200-295)
server/trpc/routers/profiles.ts (2)
server/utils/google_maps.ts (1)
  • getCityIdFromGooglePlace (7-48)
server/utils/city.ts (1)
  • findOrCreateCity (127-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (4)
server/trpc/routers/profiles.ts (4)

6-9: LGTM! Imports are appropriate for the new functionality.

The new imports are well-organized and necessary for the Google Maps venue creation feature.


316-327: Good validation logic for place types.

The check prevents users from selecting broad geographical areas instead of specific venues, which aligns with the feature requirements. The error message is clear and user-friendly.


349-356: Username generation logic is robust.

The implementation correctly generates unique usernames with incremental suffixes to avoid conflicts. Good use of the getSlug utility for consistent slug formatting.


365-365: Null cityId is safe to allow: filtering, includes, and schema definitions all handle cityId: null without errors—no further changes needed.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
server/trpc/routers/profiles.ts (2)

312-313: Validate input schema; avoid z.any().

If you keep client payloads, validate the exact fields you use (name, types, formatted_address, website, international_phone_number, url, geometry.location.{lat,lng}, address_components). Better yet, drop it per main comment.


311-383: Add unique constraint to profile.placeId
The placeId field in model Profile is missing an @unique annotation. Add @unique to placeId in both prisma/schema.prisma and cli/prisma/schema.prisma, then run prisma migrate dev.
profile.username and city.slug already have unique constraints.

server/utils/city.ts (1)

61-76: Add null-check for address in addCity before destructuring
In server/utils/city.ts at the start of addCity (around line 79), getAddressFromPlaceId can return null; you must guard against this before doing

const { locality, region, country } = address
// … address.lat, address.lng

to avoid runtime errors.

♻️ Duplicate comments (3)
nuxt.config.ts (1)

133-156: Good split: private server key vs public browser key.

Server key moved to private runtimeConfig and browser key under public. This addresses earlier feedback to separate keys.

server/utils/city.ts (2)

22-24: Return null instead of {} when no places for consistent nullability.

Align with getAddressFromPlaceId’s null contract to avoid truthy {} leaking to callers.

-  if (!places || !places.length) {
-    return {}
-  }
+  if (!places || !places.length) {
+    return null
+  }

135-155: Slug collision handling can still race; add retry on unique conflict.

If city.slug is unique, upsert may fail on create due to an existing slug for a different city. Catch P2002 and append a numeric suffix.

-  const newCity = await prisma.city.upsert({
-    where: { id: placeId },
-    create: cityData,
-    update: cityData,
-  })
+  let attempt = 0
+  while (attempt < 5) {
+    try {
+      const newCity = await prisma.city.upsert({
+        where: { id: placeId },
+        create: cityData,
+        update: cityData,
+      })
+      return newCity
+    } catch (e: any) {
+      if (e?.code === 'P2002' && String(e.meta?.target || '').includes('slug')) {
+        attempt++
+        cityData.slug = `${slug}-${attempt}`
+        continue
+      }
+      throw e
+    }
+  }
+  throw new Error('Failed to allocate unique city slug')
-
-  return newCity
🧹 Nitpick comments (7)
nuxt.config.ts (1)

153-154: Standardize env var name and add safe default.

Prefer Nuxt’s public prefix and avoid undefined leaking into script URLs.

-      googleMapsApiKey: process.env.GOOGLE_MAPS_API_KEY_PUBLIC,
+      googleMapsApiKey: process.env.NUXT_PUBLIC_GOOGLE_MAPS_API_KEY ?? '',

Optionally assert presence at startup and fail fast if missing.

server/trpc/routers/profiles.ts (4)

316-327: Guard is helpful but brittle; validate against server-fetched Place Details.

Types sent from the client are untrusted and incomplete. Fetch Place Details server-side and validate there (see main comment below), then use those types for this check.


329-337: Re-check existence inside the transaction to avoid TOCTOU races.

Between the pre-check and creation another request can create the same venue. Move the existence check into the transaction before attempting create.

-      const existingVenue = await prisma.profile.findFirst({ where: { placeId } })
-      if (existingVenue) { return existingVenue }
-      return await prisma.$transaction(async (tx) => {
+      return await prisma.$transaction(async (tx) => {
+        const existingVenue = await tx.profile.findFirst({ where: { placeId } })
+        if (existingVenue) { return existingVenue }
         try {
           // ...

Also applies to: 338-342


317-318: Remove stray console logs; prefer a structured logger.

Drop noisy logs or guard behind a debug flag.

-      console.log('Place types:', placeTypes)
...
-          console.error('Error creating venue:', error)

Also applies to: 379-381


341-347: City resolution may misidentify cities (e.g., “San Jose”).

getCityIdFromGooglePlace uses a text search by name. Prefer reverse geocoding of the venue’s lat/lng with result_type=locality to get an exact city place_id, reducing ambiguity. Consider updating that utility accordingly.

server/utils/city.ts (2)

66-68: Encode placeId and assert server key.

Avoid malformed URLs and fail fast on missing key.

-  const apiKey = useRuntimeConfig().googleMapsServerApiKey
-  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${placeId}&key=${apiKey}`
+  const apiKey = useRuntimeConfig().googleMapsServerApiKey
+  if (!apiKey) throw new Error('Google Maps server API key is missing')
+  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${apiKey}`

127-133: Fix typo and clarify error.

Use a clear, user-safe error string.

-  if (!address || !address.locality) {
-    throw new Error('Couldnt find city')
-  }
+  if (!address || !address.locality) {
+    throw new Error("Couldn't determine city from Google data")
+  }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d1e46c and 8093009.

📒 Files selected for processing (4)
  • nuxt.config.ts (2 hunks)
  • server/trpc/routers/profiles.ts (3 hunks)
  • server/utils/city.ts (3 hunks)
  • server/utils/google_maps.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/utils/google_maps.ts
🧰 Additional context used
🧬 Code graph analysis (3)
server/utils/city.ts (3)
utils/slug.ts (1)
  • getSlug (11-88)
server/prisma.ts (1)
  • prisma (5-8)
utils/country.ts (1)
  • getCountryCode (22-49)
server/trpc/routers/profiles.ts (3)
server/prisma.ts (1)
  • prisma (5-8)
server/utils/google_maps.ts (1)
  • getCityIdFromGooglePlace (8-54)
server/utils/city.ts (1)
  • findOrCreateCity (127-158)
nuxt.config.ts (1)
lib/googleMapsApi.ts (2)
  • loadGoogleMapsApi (9-31)
  • Window (4-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (2)
server/trpc/routers/profiles.ts (1)

6-9: Imports look correct and scoped.

New utilities are imported where used. No issues.

server/utils/city.ts (1)

71-76: OK to return null on non-OK responses.

This hardens callers against bad responses.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/utils/city.ts (1)

61-77: Callers must handle null now; addCity will throw on destructuring.

getAddressFromPlaceId returns null on failures, but addCity destructures address without a guard. Add a null-check.

Example fix (outside this hunk, in addCity):

export async function addCity(city: any) {
  const address = await getAddressFromPlaceId(city.id)
  if (!address || !address.locality) {
    throw new Error("Couldn't find city")
  }
  const { locality, region, country } = address
  ...
}
♻️ Duplicate comments (3)
server/utils/google_maps.ts (1)

31-36: Good fix: use private server key and fail fast.

Switched to googleMapsServerApiKey and throw on missing key. Matches earlier feedback.

server/utils/city.ts (1)

66-69: Good: server key usage with clear error.
Matches prior guidance.

server/trpc/routers/profiles.ts (1)

338-345: Good transactional flow and idempotency checks.
Creation is wrapped in a Prisma transaction and returns existing placeId matches.

🧹 Nitpick comments (7)
server/utils/google_maps.ts (2)

14-15: Hoist Google Maps client to a module singleton.

Avoid re-instantiating the client per call.

 import {
   AddressType,
   Client,
   type PlaceData,
 } from '@googlemaps/google-maps-services-js'
 
+// Reuse a single client instance
+const mapsClient = new Client({})
 
 export async function getCityIdFromGooglePlace(
   place: Partial<PlaceData>
 ): Promise<string> {
-  const client = new Client({})
+  // use mapsClient
 
   ...
 
 export async function getPlaceDetails(placeId: string) {
-  const client = new Client({})
+  // use mapsClient

Also applies to: 59-60, 1-5


37-52: Prefer reverse geocode when lat/lng is known; keep Text Search as fallback.

This reduces ambiguity for common names like “Springfield”.

   try {
-    const response = await client.textSearch({
+    // If venue has coordinates, prefer reverse geocoding to a locality/admin area
+    if (place.geometry?.location) {
+      const rev = await mapsClient.reverseGeocode({
+        params: {
+          latlng: {
+            lat: place.geometry.location.lat,
+            lng: place.geometry.location.lng,
+          },
+          result_type: ['locality', 'administrative_area_level_2', 'administrative_area_level_1'],
+          key: apiKey,
+        },
+        timeout: 5000,
+      })
+      const rc = rev.data.results?.[0]
+      const pid = rc?.place_id
+      if (pid) return pid
+    }
+    const response = await mapsClient.textSearch({
       params: {
         query: cityName,
         key: apiKey,
       },
       timeout: 5000,
     })
server/utils/city.ts (2)

31-47: Type the parsed address to avoid any leaks.
Define a return type and use it in callers.

-const getAddress = (places: any) => {
+type ParsedAddress = {
+  locality: string
+  region: string
+  country: string
+  lat?: number
+  lng?: number
+  place_id: string
+}
+
+const getAddress = (places: any): ParsedAddress | null => {
   ...
-  const result: any = {}
+  const result: ParsedAddress = {
+    locality: '',
+    region: '',
+    country: '',
+    place_id: place.place_id,
+  }
   ...
-  result.place_id = place.place_id
+  // already set above

Also applies to: 47-47


127-159: Slug collisions still possible under concurrency; add retry on unique error.

Rely on DB unique constraint for slug and retry with suffix on P2002.

-  const newCity = await prisma.city.upsert({
-    where: { id: placeId },
-    create: cityData,
-    update: cityData,
-  })
+  // Attempt create/update; if slug is unique-constrained and collides, retry with suffix.
+  for (let i = 0; i < 5; i++) {
+    try {
+      const payload = { ...cityData, slug }
+      const newCity = await prisma.city.upsert({
+        where: { id: placeId },
+        create: payload,
+        update: payload,
+      })
+      return newCity
+    } catch (e: any) {
+      if (e?.code === 'P2002' && String(e.meta?.target || '').includes('slug')) {
+        slug = getSlug(`${slug}-${i + 1}`)
+        continue
+      }
+      throw e
+    }
+  }
+  throw new Error('Could not allocate a unique city slug')
 
-  return newCity
+  // unreachable when successful

Additionally, consider a DB unique index on city.slug if not present.

server/trpc/routers/profiles.ts (3)

316-326: Wrap Place Details fetch to return controlled errors.

If Google fails (network/quota), getPlaceDetails may throw; map to TRPC error.

-      const { placeId } = input
-      const place = await getPlaceDetails(placeId)
+      const { placeId } = input
+      const place = await getPlaceDetails(placeId).catch((e) => {
+        console.error('getPlaceDetails error:', e)
+        throw new TRPCError({
+          code: 'SERVICE_UNAVAILABLE',
+          message: 'Google Maps is temporarily unavailable.',
+        })
+      })
       if (!place || !place.name) {
         throw new TRPCError({
           code: 'BAD_REQUEST',
           message: 'Invalid Google Place ID or place has no name.',
         })
       }

362-381: Preserve zero coordinates; use nullish coalescing.

|| null coerces 0 to null. Use ?? null.

-                lat: place.geometry?.location?.lat || null,
-                lng: place.geometry?.location?.lng || null,
+                lat: place.geometry?.location?.lat ?? null,
+                lng: place.geometry?.location?.lng ?? null,

338-402: Enforce DB uniqueness for true idempotency.

Ensure unique indexes on profile.placeId and profile.username in Prisma schema; this complements the retry logic and prevents duplicates under races.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8093009 and e1bd0a9.

📒 Files selected for processing (4)
  • components/inputs/VenueInput.vue (0 hunks)
  • server/trpc/routers/profiles.ts (2 hunks)
  • server/utils/city.ts (3 hunks)
  • server/utils/google_maps.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • components/inputs/VenueInput.vue
🧰 Additional context used
🧬 Code graph analysis (3)
server/utils/google_maps.ts (2)
cli/utils/google_maps.ts (2)
  • getPlace (7-54)
  • getCityId (56-88)
composables/useCities.ts (5)
  • google (8-23)
  • query (3-38)
  • prediction (16-20)
  • loadGoogleMapsApi (7-24)
  • predictions (15-21)
server/trpc/routers/profiles.ts (2)
server/utils/google_maps.ts (2)
  • getPlaceDetails (55-79)
  • getCityIdFromGooglePlace (7-53)
server/utils/city.ts (1)
  • findOrCreateCity (128-159)
server/utils/city.ts (3)
utils/slug.ts (1)
  • getSlug (11-88)
server/prisma.ts (1)
  • prisma (5-8)
utils/country.ts (1)
  • getCountryCode (22-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci
  • GitHub Check: test
🔇 Additional comments (2)
server/utils/city.ts (1)

22-29: Return null on empty results — good hardening.
This makes callers’ intent clearer compared to {}/[].

server/trpc/routers/profiles.ts (1)

327-337: Reject non-specific types — good validation.
Prevents cities/regions from being added as venues.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (5)
server/utils/city.ts (5)

31-45: Avoid using neighborhood/sublocality as “city” fallback

Using neighborhood/sublocality may create too-granular “cities”. Prefer locality → admin_area_level_2 → admin_area_level_1, and drop neighborhood/sublocality from the chain. Also trim the final value.

Apply:

-  const nameComponent =
-    place.address_components.find((c: any) => c.types.includes('locality')) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('sublocality_level_1')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('neighborhood')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_2')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_1')
-    )
+  const nameComponent =
+    place.address_components.find((c: any) => c.types.includes('locality')) ||
+    place.address_components.find((c: any) =>
+      c.types.includes('administrative_area_level_2')
+    ) ||
+    place.address_components.find((c: any) =>
+      c.types.includes('administrative_area_level_1')
+    )
 
-  result.locality = nameComponent?.long_name || ''
+  result.locality = (nameComponent?.long_name || '').trim()

Also applies to: 47-47


131-162: Consider consolidating addCity → findOrCreateCity and add a small cache

  • Keep one path to avoid divergence (addCity’s logic differs).
  • Add a short-lived cache (in-memory/LRU or Nitro storage) for placeId→city to reduce API calls.

66-69: Minor: log non-OK statuses for observability

Add structured logs for non-OK and timeouts to aid debugging and alerting.

Also applies to: 72-74


21-59: Type the helpers

Define a narrow return type for getAddress/getAddressFromPlaceId and avoid any.

Example (outside hunks):

type CityAddress = {
  locality: string
  region: string
  country: string
  lat?: number
  lng?: number
  place_id: string
}

const getAddress = (places: any[]): CityAddress | null => { /* ... */ }
const getAddressFromPlaceId = async (placeId: string): Promise<CityAddress | null> => { /* ... */ }

Also applies to: 61-78, 131-162


79-129: Optional: replace addCity body with findOrCreateCity(city.id)

Keeps behavior consistent and idempotent.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1bd0a9 and 7ac4474.

📒 Files selected for processing (2)
  • server/utils/city.ts (3 hunks)
  • server/utils/google_maps.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/utils/google_maps.ts
🧰 Additional context used
🧬 Code graph analysis (1)
server/utils/city.ts (4)
utils/slug.ts (1)
  • getSlug (11-88)
server/prisma.ts (1)
  • prisma (5-8)
utils/country.ts (1)
  • getCountryCode (22-49)
composables/useCities.ts (3)
  • google (8-23)
  • prediction (16-20)
  • predictions (15-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci
  • GitHub Check: test
🔇 Additional comments (1)
server/utils/city.ts (1)

22-28: Return-null standardization looks good

Returning null on empty results makes downstream checks simpler. LGTM.

Comment on lines 131 to 121
export async function findOrCreateCity(placeId: string) {
const address = await getAddressFromPlaceId(placeId)

if (!address || !address.locality) {
throw new Error("Couldn't determine city from Google data")
}
const { locality, region, country, lat, lng } = address

let slug = getSlug(locality)
const cityWithSameSlug = await prisma.city.findFirst({ where: { slug } })
if (cityWithSameSlug) {
slug = getSlug([region, locality].join('-'))
}
const countryCode = await getCountryCode(country)
const cityData = {
id: placeId,
name: locality,
region: region || '',
slug,
countryCode,
lat: lat || 0,
lng: lng || 0,
}

const newCity = await prisma.city.upsert({
where: { id: placeId },
create: cityData,
update: cityData,
})

return newCity
}
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verification: check unique index and call sites

Run to confirm DB uniqueness and usages.


🏁 Script executed:

#!/bin/bash
# 1) Verify unique index on City.slug
rg -nP -C2 'model\s+City\b[\s\S]*?slug\b.*@unique' prisma/schema.prisma

# 2) Find all call sites to migrate from addCity to findOrCreateCity
rg -nP '\baddCity\s*\(' -g '!**/dist/**' -g '!**/.nuxt/**'

# 3) Confirm runtime key presence
rg -n 'googleMapsServerApiKey' -n

Length of output: 546


🏁 Script executed:

#!/bin/bash
# Locate all Prisma schema files
echo "Prisma schema files:"
find . -maxdepth 3 -type f -name '*.prisma'

# Verify unique index on City.slug in each schema file
echo -e "\nSearching for City model and slug @unique:"
rg -nP -C2 'model\s+City\b[\s\S]*?slug\b.*@unique' -g '*.prisma' || true

# Inspect slug attribute for context
echo -e "\nInspecting slug attribute in City model:"
rg -nP -C2 'model\s+City\b' -g '*.prisma' | sed -n '1,20p'

# Search for any addCity call sites
echo -e "\nSearching for addCity call sites in TypeScript files:"
rg -n '\baddCity\s*\(' -g '*.ts' || true

Length of output: 899


Enforce unique slug on City and update call sites

  • Add @unique to the slug field in both prisma/schema.prisma and cli/prisma/schema.prisma under the model City to guarantee slug uniqueness.
  • Replace all remaining addCity(…) invocations—in server/utils/city.ts, server/trpc/routers/cities.ts, cli/importer/profile.ts, and cli/importer/index.ts—with findOrCreateCity(…) to consolidate city creation logic.
🤖 Prompt for AI Agents
In server/utils/city.ts around lines 131 to 162: the PR requires enforcing
unique slugs and consolidating city creation; add `@unique` to the `slug` field
in both prisma/schema.prisma and cli/prisma/schema.prisma under model City, run
prisma migrate/format to apply the schema change, then replace every remaining
call to addCity(...) in server/utils/city.ts, server/trpc/routers/cities.ts,
cli/importer/profile.ts, and cli/importer/index.ts with findOrCreateCity(...) so
all city creation uses the upsert/find logic in this file and avoids
duplicate-slug race conditions.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
server/utils/city.ts (3)

60-84: Good: bounded external call with timeout + defensive checks

Nice use of AbortController, result validation, and finally to clear the timer.


89-91: Guarding locality and country is correct

Prevents corrupt Country rows and bad slugs. Good.


139-204: Ensure slug uniqueness at the DB layer; consider deprecating addCity in favor of findOrCreateCity

  • Enforce a unique index on City.slug to backstop the retry-on-conflict logic.
  • Prefer a single code path: make addCity delegate to findOrCreateCity to avoid divergent slug/creation rules.

Run:

#!/bin/bash
set -euo pipefail

echo "Checking unique index on City.slug across Prisma schemas..."
fd -t f -a '*.prisma' | while read -r f; do
  echo "File: $f"
  if rg -nP -C2 'model\s+City\b[\s\S]*?\bslug\b[^\n]*@unique' "$f" > /dev/null; then
    rg -nP -C2 'model\s+City\b[\s\S]*?\bslug\b[^\n]*@unique' "$f"
  else
    echo "  -> slug not marked @unique in $f"
  fi
done

echo
echo "Call sites still using addCity(...):"
rg -nP '\baddCity\s*\(' -g '!**/node_modules/**' -g '!**/dist/**' || true

If addCity must remain, consider:

export async function addCity(city: { id: string }) {
  return findOrCreateCity(city.id)
}
🧹 Nitpick comments (3)
server/utils/city.ts (3)

31-39: Add postal_town fallback and type the return for getAddress

Some countries (e.g., UK) use postal_town in place of locality. Also, explicitly type the return to avoid leaking any.

Apply:

-  const nameComponent =
-    place.address_components.find((c: any) => c.types.includes('locality')) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_2')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_1')
-    )
+  const nameComponent =
+    place.address_components.find((c: any) => c.types.includes('locality')) ||
+    place.address_components.find((c: any) => c.types.includes('postal_town')) ||
+    place.address_components.find((c: any) =>
+      c.types.includes('administrative_area_level_2')
+    ) ||
+    place.address_components.find((c: any) =>
+      c.types.includes('administrative_area_level_1')
+    )

Optional typing (outside this hunk):

type Address = {
  locality: string
  country: string
  region?: string
  lat?: number
  lng?: number
  place_id: string
}

73-75: Use strict equality for status check

Minor consistency/readability nit.

-      data.status != 'OK' ||
+      data.status !== 'OK' ||

142-144: Make the error actionable and include placeId

Helps debugging while keeping the guard explicit.

-    throw new Error("Couldn't determine city from Google data")
+    throw new Error(`Couldn't determine city (need locality and country) for placeId=${placeId}`)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ac4474 and 89cead3.

📒 Files selected for processing (1)
  • server/utils/city.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/utils/city.ts (4)
utils/slug.ts (1)
  • getSlug (11-88)
server/prisma.ts (1)
  • prisma (5-8)
utils/country.ts (1)
  • getCountryCode (22-49)
composables/useCities.ts (3)
  • prediction (16-20)
  • google (8-23)
  • predictions (15-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (1)
server/utils/city.ts (1)

4-4: LGTM: Prisma import for P2002 handling

Import is appropriate for differentiating unique-constraint errors in upsert.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (2)
server/utils/city.ts (2)

170-211: Ensure unique index on City.slug; otherwise P2002 retry never triggers.

Verify schema has slug @unique and that lat/lng are nullable to match the change above.

#!/bin/bash
# 1) Find Prisma schemas
fd -t f -e prisma

# 2) Check City.slug unique and lat/lng nullability in all schemas
rg -nP -C3 'model\s+City\b[\s\S]*?slug\s*:\s*\w+.*@unique' -g '*.prisma' || true
rg -nP -C2 'model\s+City\b[\s\S]*?\blat\s*:\s*Float\??' -g '*.prisma' || true
rg -nP -C2 'model\s+City\b[\s\S]*?\blng\s*:\s*Float\??' -g '*.prisma' || true

172-189: Avoid Null Island: don’t default lat/lng to 0; only persist when present.

Persisting 0,0 corrupts geospatial views and analytics.

   while (true) {
-    const createData = {
+    const createData: any = {
       id: placeId,
       name: locality,
       region: region || '',
       slug: slugToTry,
       countryCode,
-      lat: lat ?? 0,
-      lng: lng ?? 0,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
     }
-    const updateData = {
+    const updateData: any = {
       name: locality,
       region: region || '',
       countryCode,
-      lat: lat ?? 0,
-      lng: lng ?? 0,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
     }
🧹 Nitpick comments (5)
server/utils/city.ts (5)

63-87: Good hardening on the Google call (timeout + null semantics).

AbortController timeout, OK checks, and null returns look solid. Consider making the 5s timeout configurable via runtimeConfig to aid ops without redeploys.

-  const controller = new AbortController()
-  const timer = setTimeout(() => controller.abort(), 5000)
+  const controller = new AbortController()
+  const { googleMapsTimeoutMs = 5000 } = useRuntimeConfig()
+  const timer = setTimeout(() => controller.abort(), googleMapsTimeoutMs)

21-56: Standardize the address shape with an explicit return type.

Explicit typing here prevents accidental shape drift into downstream slug/country code logic.

-const getAddress = (places: any) => {
+type ResolvedAddress = {
+  locality: string
+  region: string
+  country: string
+  lat?: number
+  lng?: number
+  place_id: string
+} | null
+
+const getAddress = (places: any): ResolvedAddress => {

90-140: Deprecate addCity; delegate to findOrCreateCity to avoid divergence and race windows.

Current flow has a TOCTOU window between uniqueness checks and create. Keep one code path.

 export async function addCity(city: any) {
-  const address = await getAddressFromPlaceId(city.id)
-  if (!address || !address.locality || !address.country) {
-    throw new Error(`Could not find city details for id: ${city.id}`)
-  }
-  const existingCity = await prisma.city.findUnique({
-    where: { id: city.id },
-  })
-  if (!!existingCity) {
-    return existingCity
-  }
-  const { locality, region, country } = address
-  let slug = getSlug(locality)
-  let existingLocality = await prisma.city.findFirst({ where: { slug } })
-  if (!!existingLocality) {
-    slug = getSlug([region, locality].join('-'))
-  }
-  let existingRegion = await prisma.city.findFirst({ where: { slug } })
-  if (!!existingRegion) {
-    throw new Error(`city: region-locality slug already exists: ${slug}`)
-  }
-  const result = {
-    id: city.id,
-    name: locality,
-    region,
-    slug,
-    countryCode: await getCountryCode(country),
-    description: '',
-    lat: address.lat,
-    lng: address.lng,
-  }
-  const newCity = await prisma.city.create({ data: result })
-  return newCity
+  return findOrCreateCity(city.id)
 }

142-149: Short‑circuit if city already exists; avoid unnecessary Google calls.

Check DB by id before hitting the network.

 export async function findOrCreateCity(placeId: string) {
-  const address = await getAddressFromPlaceId(placeId)
+  const existing = await prisma.city.findUnique({ where: { id: placeId } })
+  if (existing) return existing
+
+  const address = await getAddressFromPlaceId(placeId)

199-209: Slug retry strategy could still collide; consider deterministic numeric suffixing.

Optional: probe existing slugs with -2, -3… to avoid time-based collisions.

-        attempt++
-        slugToTry = getSlug(
-          `${baseSlug}-${Date.now().toString().slice(-4)}-${attempt}`
-        )
+        attempt++
+        slugToTry = getSlug(`${baseSlug}-${attempt + 1}`)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89cead3 and 50b1837.

📒 Files selected for processing (1)
  • server/utils/city.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/utils/city.ts (4)
utils/slug.ts (1)
  • getSlug (11-88)
server/prisma.ts (1)
  • prisma (5-8)
utils/country.ts (1)
  • getCountryCode (22-49)
composables/useCities.ts (1)
  • prediction (16-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (1)
server/utils/city.ts (1)

183-189: Nice: slug is not updated on upsert update path.

Prevents URL churn; good alignment with stable slugs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
server/utils/city.ts (2)

152-161: Verify unique index on City.slug; consider random suffix on repeated P2002.

P2002 assumes slug has a unique DB constraint. Also, after 2–3 collisions, add a short random or time-based suffix to reduce contention.

Run to confirm schema and see where to adjust:

#!/bin/bash
# 1) Ensure City.slug has a unique index in all Prisma schemas
rg -nP -C2 'model\s+City\b[\s\S]*?\bslug\b.*@unique' -g '**/*.prisma' || true

# 2) Grep migrations for a unique index on City.slug
rg -nP -C2 'CREATE\s+UNIQUE\s+INDEX.*City.*slug' -g 'prisma/migrations/**/migration.sql' || true

127-142: Do not default lat/lng to 0 (“Null Island”); only persist when provided.

Persisting 0,0 corrupts geodata and complicates downstream maps logic. Include coordinates conditionally.

-    const createData = {
+    const createData = {
       id: placeId,
       name: locality,
       region: region || '',
       slug: slugToTry,
       countryCode,
-      lat: lat ?? 0,
-      lng: lng ?? 0,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
     }
-    const updateData = {
+    const updateData = {
       name: locality,
       region: region || '',
       countryCode,
-      lat: lat ?? 0,
-      lng: lng ?? 0,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
     }
🧹 Nitpick comments (5)
server/utils/city.ts (5)

31-45: Locality extraction isn’t “longest match” as the summary claims; consider picking the most specific name.

Currently this picks the first available type; it does not select the longest/most specific candidate. Optional tweak below keeps behavior but prefers the longest among allowed types.

-  // Find the most specific name for the locality by checking multiple types.
-  const nameComponent =
-    place.address_components.find((c: any) => c.types.includes('locality')) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('postal_town')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_2')
-    ) ||
-    place.address_components.find((c: any) =>
-      c.types.includes('administrative_area_level_1')
-    )
+  // Prefer the longest long_name among these types
+  const allowed = new Set([
+    'locality',
+    'postal_town',
+    'administrative_area_level_2',
+    'administrative_area_level_1',
+  ])
+  const candidates = (place.address_components || []).filter(
+    (c: any) => Array.isArray(c.types) && c.types.some((t: string) => allowed.has(t)),
+  )
+  const nameComponent = candidates.sort(
+    (a: any, b: any) => (b.long_name?.length || 0) - (a.long_name?.length || 0),
+  )[0]

48-51: Guard lat/lng shape.

Geocode JSON uses numbers, but a defensive typeof check prevents accidental non-number assignment.

-  if (place.geometry?.location) {
-    result.lat = place.geometry.location.lat
-    result.lng = place.geometry.location.lng
-  }
+  if (place.geometry?.location) {
+    const { lat, lng } = place.geometry.location
+    if (typeof lat === 'number') result.lat = lat
+    if (typeof lng === 'number') result.lng = lng
+  }

63-87: Minor hardening: strict equality + configurable timeout.

Use strict compare for status and make timeout configurable to tune in ops.

-  const controller = new AbortController()
-  const timer = setTimeout(() => controller.abort(), 5000)
+  const controller = new AbortController()
+  const timeoutMs = Number(useRuntimeConfig().googleMapsTimeoutMs ?? 5000)
+  const timer = setTimeout(() => controller.abort(), timeoutMs)
@@
-    if (
-      data.status != 'OK' ||
+    if (
+      data.status !== 'OK' ||
       !Array.isArray(data.results) ||
       !data.results.length
     ) {
       return null
     }

98-100: Make error actionable (include placeId and validate trimmed).

Improves debuggability for edge cases where Google returns incomplete components.

-  if (!address || !address.locality || !address.country) {
-    throw new Error("Couldn't determine city from Google data")
-  }
+  if (
+    !address ||
+    !address.locality?.trim() ||
+    !address.country?.trim()
+  ) {
+    throw new Error(`Couldn't determine city (need locality+country) for placeId=${placeId}`)
+  }

103-109: Slug candidates: consider adding a country-coded option before numeric suffixes.

Optional: after resolving countryCode, push getSlug(${locality} ${countryCode}) to candidates to prefer human-meaningful slugs over “-2/-3”. This may reduce retries and improve UX for duplicate city names across countries.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50b1837 and e4351f3.

📒 Files selected for processing (1)
  • server/utils/city.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/utils/city.ts (4)
server/prisma.ts (1)
  • prisma (5-8)
utils/slug.ts (1)
  • getSlug (11-88)
utils/country.ts (1)
  • getCountryCode (22-49)
server/trpc/routers/cities.ts (3)
  • city (53-95)
  • input (106-110)
  • city (25-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (4)
server/utils/city.ts (4)

22-23: Consistent null sentinel — good.

Returning null for empty results avoids leaking {}/[] downstream.


59-61: Early null on falsy placeId — good.


90-96: Idempotent fast-path — good.

Short-circuit on existing id avoids external calls.


110-121: Preflight + retry covers TOCTOU — good.

P2002 retry logic complements the pre-check.

Comment on lines +90 to +68
export async function findOrCreateCity(placeId: string) {
const existing = await prisma.city.findUnique({
where: { id: placeId },
})
if (existing) return existing

if (!!existingCity) {
return existingCity
}
const address = await getAddressFromPlaceId(placeId)
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Update call sites to new API (findOrCreateCity).

Ensure no stale addCity(...) usages remain (e.g., server/trpc/routers/cities.ts snippet).


🏁 Script executed:

#!/bin/bash
# Find legacy addCity call sites and confirm new usage
rg -n '\baddCity\s*\(' -g '!**/dist/**' -g '!**/.nuxt/**' || true
rg -n '\bfindOrCreateCity\s*\(' -g '!**/dist/**' -g '!**/.nuxt/**' || true

Length of output: 297


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show ripgrep debug to understand why files were skipped previously
rg --debug 'addCity\s*\(' || true
rg --debug 'findOrCreateCity\s*\(' || true

# Search repository for legacy and new API call sites, don't respect .gitignore to avoid skipping files,
# but explicitly exclude common generated directories to reduce noise.
rg -n -C2 --hidden --no-ignore -S --follow -g '!node_modules/**' -g '!**/dist/**' -g '!**/.nuxt/**' '(\baddCity\s*\(|\bfindOrCreateCity\s*\()' || true

Length of output: 13374


Update call sites to the new findOrCreateCity API — remove legacy addCity usages

  • server/trpc/routers/cities.ts — contains return await addCity(city); replace with findOrCreateCity(...) (or migrate/wrap accordingly).
  • cli/importer/profile.ts & cli/importer/index.ts — export/call a local addCity(...); either migrate these to the new API or rename to avoid confusion.
  • server/utils/city.ts already provides findOrCreateCity and server/trpc/routers/profiles.ts already uses it.
🤖 Prompt for AI Agents
In server/utils/city.ts around lines 90 to 96, the project now exposes
findOrCreateCity and legacy addCity usages must be replaced: update
server/trpc/routers/cities.ts to remove the call to addCity(city) and instead
call await findOrCreateCity(cityPlaceId) (or pass the proper placeId/value that
addCity formerly accepted), refactor cli/importer/profile.ts and
cli/importer/index.ts to either call the new findOrCreateCity API or rename
their local addCity to avoid confusion (export the new function name and update
all imports/usages), and ensure any type/signature differences are handled (map
arguments/returned value to the new API). Make these changes consistently across
the listed files and run tests to validate no remaining references to addCity
remain.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/trpc/routers/cities.ts (1)

96-111: Require auth and rate-limiting for cities.create (public endpoint)

  • server/trpc/routers/cities.ts:96-111 defines create as a publicProcedure and calls findOrCreateCity (external API + writes); components/CommunitiesSelect.vue:75 calls it from the client.
  • Action: require authentication (e.g. verify ctx.session?.user?.id or use getServerSession) and add rate-limiting middleware (global in server/trpc/init.ts or per-route) to prevent quota abuse.
🧹 Nitpick comments (2)
server/trpc/routers/cities.ts (2)

96-105: Input schema still requires unused fields; relax or deprecate them

The mutation only uses city.id now. Keep backward compatibility but stop requiring name and googleMapsPlace; also validate id is non-empty.

Apply:

-    .input(
-      z.object({
-        city: z.object({
-          id: z.string(),
-          name: z.string(),
-          googleMapsPlace: z.any(),
-        }),
-      })
-    )
+    .input(
+      z.object({
+        city: z.object({
+          id: z.string().min(1),
+          name: z.string().optional(), // deprecated
+          googleMapsPlace: z.unknown().optional(), // deprecated
+        }),
+      })
+    )

Alternative (breaking, cleaner): accept { placeId: string } and update the mutation accordingly.


109-110: Map city util errors to TRPCError for predictable client UX

findOrCreateCity throws plain errors; surface BAD_REQUEST for invalid place IDs and avoid leaking internal messages.

+import { TRPCError } from '@trpc/server'
 ...
     .mutation(async ({ ctx, input }) => {
       const { city } = input
-
-      return await findOrCreateCity(city.id)
+      try {
+        return await findOrCreateCity(city.id)
+      } catch (err) {
+        const message = err instanceof Error ? err.message : 'Unknown error'
+        if (message.includes("Couldn't determine city")) {
+          throw new TRPCError({ code: 'BAD_REQUEST', message })
+        }
+        throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', message: 'Failed to create city' })
+      }
     }),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4351f3 and ab767f3.

📒 Files selected for processing (1)
  • server/trpc/routers/cities.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/trpc/routers/cities.ts (1)
server/utils/city.ts (2)
  • findOrCreateCity (90-164)
  • addCity (63-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (1)
server/trpc/routers/cities.ts (1)

4-4: Switch to findOrCreateCity looks good

Aligns router with the new collision-safe slug/upsert flow in server/utils/city.ts.

@github-project-automation github-project-automation bot moved this from Triage to Todo in v4 Launch Sep 16, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/utils/city.ts (1)

45-60: Harden Google API call (server key, timeout, encode, consistent nulls).

Use the server-only key, add an AbortController timeout, encode placeId, check res.ok, and return null on any failure. Also make getAddress return null (not {}) when no places for a consistent type.

-const getAddress = (places: any) => {
+const getAddress = (places: any) => {
   if (!places) {
-    return {}
+    return null
   }
@@
-const getAddressFromPlaceId = async (placeId: string) => {
+const getAddressFromPlaceId = async (placeId: string) => {
   if (!placeId) {
     return null
   }
 
-  const apiKey = useRuntimeConfig().public.googleMapsApiKey
-  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${placeId}&key=${apiKey}`
-  const res = await fetch(url)
-  const data = await res.json()
-
-  if (data.status !== 'OK') {
-    return []
-  }
-
-  return getAddress(data.results)
+  const apiKey = useRuntimeConfig().googleMapsServerApiKey
+  if (!apiKey) throw new Error('Google Maps server API key is missing')
+  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${apiKey}`
+  const controller = new AbortController()
+  const timer = setTimeout(() => controller.abort(), 5000)
+  try {
+    const res = await fetch(url, { signal: controller.signal })
+    if (!res.ok) return null
+    const data = await res.json()
+    if (data.status !== 'OK' || !Array.isArray(data.results) || !data.results.length) {
+      return null
+    }
+    const addr = getAddress(data.results)
+    return addr ?? null
+  } catch {
+    return null
+  } finally {
+    clearTimeout(timer)
+  }
 }

Optional follow‑up: type these helpers to return Promise<CityAddress|null> and CityAddress|null. I can provide the types if you want.

Also applies to: 20-23, 50-52, 55-57

♻️ Duplicate comments (1)
server/utils/city.ts (1)

75-90: Make slug creation race‑safe; don’t write Null Island (lat/lng=0); keep slug immutable on updates.

Replace pre‑checks + throw with upsert‑with‑retry on P2002 and include lat/lng only when provided.

-  let slug = getSlug(locality)
-  const existingLocality = await prisma.city.findFirst({
-    where: { slug },
-  })
-
-  if (existingLocality) {
-    slug = getSlug(`${region} ${locality}`)
-  }
-  const existingRegion = await prisma.city.findFirst({
-    where: { slug },
-  })
-  if (existingRegion) {
-    throw new Error(
-      `City slug conflict: A city with slug "${slug}" already exists.`
-    )
-  }
-
-  const countryCode = await getCountryCode(country)
-
-  const createData = {
-    id: placeId,
-    name: locality,
-    region: region || '',
-    slug: slug,
-    countryCode,
-    lat: address.lat ?? 0,
-    lng: address.lng ?? 0,
-  }
-  const updateData = {
-    name: locality,
-    region: region || '',
-    countryCode,
-    lat: address.lat ?? 0,
-    lng: address.lng ?? 0,
-  }
-
-  try {
-    const city = await prisma.city.upsert({
-      where: { id: placeId },
-      create: createData,
-      update: updateData,
-    })
-    return city
-  } catch (error) {
-    throw error
-  }
+  const baseSlug = getSlug(locality)
+  const regionCandidate = region ? getSlug(`${region} ${locality}`) : null
+  const candidates = [baseSlug, ...(regionCandidate ? [regionCandidate] : [])]
+  let slug = candidates[0]
+  for (const c of candidates) {
+    const exists = await prisma.city.findFirst({ where: { slug: c }, select: { id: true } })
+    if (!exists) { slug = c; break }
+  }
+
+  const countryCode = await getCountryCode(country)
+
+  let attempt = 0
+  // Retry a few times if slug unique constraint races
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    const createData: any = {
+      id: placeId,
+      name: locality,
+      region: region || '',
+      slug,
+      countryCode,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
+    }
+    const updateData: any = {
+      // keep slug immutable on update
+      name: locality,
+      region: region || '',
+      countryCode,
+      ...(typeof lat === 'number' ? { lat } : {}),
+      ...(typeof lng === 'number' ? { lng } : {}),
+    }
+    try {
+      const city = await prisma.city.upsert({
+        where: { id: placeId },
+        create: createData,
+        update: updateData,
+      })
+      return city
+    } catch (e: any) {
+      if (e?.code === 'P2002' && String(e?.meta?.target || '').includes('slug') && attempt < 3) {
+        attempt++
+        slug = getSlug(`${baseSlug}-${attempt}`)
+        continue
+      }
+      throw e
+    }
+  }

Note: ensure city.slug has a unique index in Prisma schema to make this effective. See next comment for a quick verification script.

#!/bin/bash
# Verify unique indexes exist
fd -e prisma | xargs rg -nP -C2 'model\s+City\b[\s\S]*?\bslug\b[^\n]*@unique'
fd -e prisma | xargs rg -nP -C2 'model\s+Profile\b[\s\S]*?\b(username|placeId)\b[^\n]*@unique'

Also applies to: 92-118

🧹 Nitpick comments (3)
server/utils/city.ts (1)

45-60: Optional: reuse existing maps client util to avoid duplicate HTTP logic.

You already have server/utils/google_maps.ts with a configured client and timeouts; consider delegating to it here for consistency and to centralize retries/logging.

Would you like me to consolidate these into a single helper and update call sites?

server/trpc/routers/profiles.ts (2)

357-365: Run city creation inside the same transaction.

findOrCreateCity currently uses the global prisma, so it runs outside tx. Accept a TransactionClient in findOrCreateCity and pass tx here to keep the whole flow atomic.

Example (outside this file):

// server/utils/city.ts
import type { Prisma, PrismaClient } from '@prisma/client'
export async function findOrCreateCity(placeId: string, db: Prisma.TransactionClient | PrismaClient = prisma) {
  // use db.city everywhere instead of prisma.city
}

And here:

-          city = await findOrCreateCity(cityPlaceId)
+          city = await findOrCreateCity(cityPlaceId, tx)

321-338: Cache Google Place lookups to cut latency and quota use.

Memoize getPlaceDetails(placeId) and cityPlaceId derivation (e.g., in KV/LRU with short TTL). This endpoint can be chatty under typeahead imports.

If helpful, I can add a simple in‑memory LRU wrapper and metrics hooks.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88facee and b9b65c4.

📒 Files selected for processing (2)
  • server/trpc/routers/profiles.ts (2 hunks)
  • server/utils/city.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
server/trpc/routers/profiles.ts (2)
server/utils/google_maps.ts (2)
  • getPlaceDetails (55-84)
  • getCityIdFromGooglePlace (9-53)
server/utils/city.ts (1)
  • findOrCreateCity (62-121)
server/utils/city.ts (3)
server/prisma.ts (1)
  • prisma (5-8)
utils/slug.ts (1)
  • getSlug (11-88)
utils/country.ts (1)
  • getCountryCode (22-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ci
  • GitHub Check: test
🔇 Additional comments (1)
server/trpc/routers/profiles.ts (1)

368-384: Make venue creation idempotent/race‑safe; retry on P2002; use ?? for coords.

Replace pre‑checks with create‑with‑retry handling unique conflicts on username/placeId; avoid || null for numbers.
[ suggest_recommended_refactor ]
[ duplicate_comment ]

-        // Generate a unique username for the venue to avoid conflicts.
-        const baseSlug = getSlug(place.name!)
-        let username = baseSlug
-        const existingProfile = await tx.profile.findFirst({
-          where: { username },
-        })
-        if (existingProfile) {
-          username = getSlug(`${place.name!}-${city.name}`)
-          const anotherExisting = await tx.profile.findFirst({
-            where: { username },
-          })
-          if (anotherExisting) {
-            throw new Error(
-              `Could not generate a unique username for this venue.`
-            )
-          }
-        }
-        try {
-          const newVenue = await tx.profile.create({
-            data: {
-              name: place.name!,
-              username,
-              type: 'Venue',
-              placeId,
-              cityId: city.id,
-              formattedAddress: place.formatted_address,
-              website: place.website || null,
-              phone: place.international_phone_number || null,
-              lat: place.geometry?.location?.lat || null,
-              lng: place.geometry?.location?.lng || null,
-              mapUrl: place.url || null,
-            },
-          })
-          return newVenue
-        } catch (error) {
-          throw error
-        }
+        const baseSlug = getSlug(place.name!)
+        let username = baseSlug
+        for (let attempt = 0; attempt < 5; attempt++) {
+          try {
+            const newVenue = await tx.profile.create({
+              data: {
+                name: place.name!,
+                username,
+                type: 'Venue',
+                placeId,
+                cityId: city.id,
+                formattedAddress: place.formatted_address,
+                website: place.website || null,
+                phone: place.international_phone_number || null,
+                lat: place.geometry?.location?.lat ?? null,
+                lng: place.geometry?.location?.lng ?? null,
+                mapUrl: place.url || null,
+              },
+            })
+            return newVenue
+          } catch (e: any) {
+            if (e?.code === 'P2002') {
+              const target = String(e?.meta?.target || '')
+              if (target.includes('username')) {
+                username = `${baseSlug}-${attempt + 1}`
+                continue
+              }
+              if (target.includes('placeId')) {
+                const concurrent = await tx.profile.findFirst({ where: { placeId } })
+                if (concurrent) return concurrent
+              }
+            }
+            throw e
+          }
+        }
+        throw new TRPCError({ code: 'CONFLICT', message: 'Could not allocate a unique username.' })

Also applies to: 385-401

Comment on lines +341 to +345
where: { placeId },
})
if (existingVenue) {
return existingVenue
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Schema guardrails: ensure uniqueness.

Double‑check unique indexes for Profile.username and Profile.placeId exist; otherwise the retry logic can’t rely on P2002.


🏁 Script executed:

#!/bin/bash
fd -e prisma | xargs rg -nP -C2 'model\s+Profile\b[\s\S]*?\busername\b[^\n]*@unique'
fd -e prisma | xargs rg -nP -C2 'model\s+Profile\b[\s\S]*?\bplaceId\b[^\n]*@unique'

Length of output: 0


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo "Top-level files:"
ls -la | sed -n '1,200p'

echo
echo "Looking for Prisma schema files (schema.prisma and any *.prisma) and model definitions..."
rg -nP --hidden --no-ignore-vcs -S --glob '!**/node_modules/**' "datasource\s+db" -g '*.prisma' || true
rg --files -g '*.prisma' || true

echo
echo "Search for 'model Profile' (case-sensitive and insensitive):"
rg -nP -S "model\s+Profile\b" || true
rg -nP -S "(?i)model\s+profile\b" || true

echo
echo "Search inside .prisma files for field-level uniques and @@unique involving username/placeId:"
rg -nP -S "username\b.*@unique" -g '*.prisma' || true
rg -nP -S "placeId\b.*@unique" -g '*.prisma' || true
rg -nP -S "@@unique\([^\)]*(username|placeId)" -g '*.prisma' || true

echo
echo "Fallback: search the entire repo for 'username' and 'placeId' occurrences (to locate alternate schemas or naming):"
rg -nP -S "username\b" || true
rg -nP -S "placeId\b" || true

Length of output: 26869


Schema guardrails: add a unique constraint for Profile.placeId or stop relying on P2002

prisma/schema.prisma — Profile.username is declared as String @unique (line ~127); Profile.placeId is String? with no @unique (line ~139). If your retry logic depends on Prisma P2002, add a unique constraint on placeId (field @unique or @@unique([placeId])) and run a migration, or change the create/retry flow to handle concurrent duplicates.

🤖 Prompt for AI Agents
In server/trpc/routers/profiles.ts around lines 341 to 345, the code queries by
placeId then returns existingVenue but the review warns that you rely on Prisma
P2002 for duplicate protection while Profile.placeId is not unique in the
schema; either add a unique constraint to Profile.placeId in
prisma/schema.prisma (add `@unique` on the field or an `@@unique([placeId])`)
and run a migration, or modify this create/retry flow to handle concurrent
creates safely (e.g., perform an atomic upsert, wrap creation in a transaction
with a select-for-update pattern, or catch duplicate create errors without
assuming P2002); choose one approach and implement it consistently across the
codebase.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
prisma/schema.prisma (3)

657-678: Redundant uniqueness: prefer pairKey or (aId,bId), not both.

pairKey @unique plus @@unique([aId, bId]) duplicates invariants. Keep one to avoid extra write overhead and ambiguous constraints. If you rely on normalized ${min(aId,bId)}-${max(aId,bId)}, keep pairKey and drop the composite unique; otherwise drop pairKey and keep the composite unique.

Suggested (keep pairKey):

   @@unique([aId, bId])

Also ensure a guard prevents aId == bId (self‑conversation). If Prisma constraints aren’t feasible, enforce in service layer.


657-678: Specify delete behaviors on chat relations to avoid stuck deletes.

  • Deleting a Profile currently RESTRICTs due to missing actions on Conversation.a/b and Message.sender. Confirm desired behavior. Typical choices:

    • Prevent user deletion if chat exists (Restrict), or
    • Soft‑delete Profiles and keep history, or
    • Cascade to Conversations/Messages (usually not desired).
  • If you keep lastMessage (not recommended), it must be onDelete: SetNull to avoid broken FKs when a Message is deleted.

If you insist on lastMessage:

-  lastMessage   Message?  @relation("LastMessage", fields: [lastMessageId], references: [id])
+  lastMessage   Message?  @relation("LastMessage", fields: [lastMessageId], references: [id], onDelete: SetNull)

Consider documenting Profile deletion policy and aligning all chat FKs accordingly.

Also applies to: 680-694


657-694: Out-of-scope schema in a Maps/venue PR — split migrations.

Introducing Conversation/Message in a PR about Google Maps venue import increases risk and complicates rollout. Recommend extracting chat schema into its own PR with a dedicated migration, seed/backfill plan, and clear deletion policy.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9b65c4 and 886149c.

📒 Files selected for processing (2)
  • .env.example (1 hunks)
  • prisma/schema.prisma (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T06:33:48.474Z
Learnt from: egemengunel
PR: we-dance/v4#414
File: server/trpc/routers/chat.ts:24-35
Timestamp: 2025-09-09T06:33:48.474Z
Learning: In the WeDance chat system, lastMessageId and lastMessage fields were removed from the Conversation schema and should not be reintroduced. The team prefers to avoid schema changes for chat ordering improvements.

Applied to files:

  • prisma/schema.prisma
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 22-22: [UnorderedKey] The NUXT_GOOGLE_MAPS_SERVER_API_KEY key should go before the NUXT_PUBLIC_GOOGLE_MAPS_API_KEY key

(UnorderedKey)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (1)
prisma/schema.prisma (1)

298-299: No functional change here.

Timestamp fields look standard. LGTM.

.env.example Outdated
Comment on lines 21 to 23
NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY
NUXT_GOOGLE_MAPS_SERVER_API_KEY=AGOOGLE_MAPS_API_KEY
GOOGLE_MAPS_API_KEY_BACKEND=GOOGLE_MAPS_API_KEY_BACKEND
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Fix placeholder, de-dup keys, and satisfy dotenv-linter ordering.

  • Typo: AGOOGLE_MAPS_API_KEY looks accidental; use a clear placeholder.
  • Potential duplication: confirm whether GOOGLE_MAPS_API_KEY_BACKEND is still referenced now that NUXT_GOOGLE_MAPS_SERVER_API_KEY exists; if not, deprecate/remove to avoid confusion.
  • Ordering: move the server key before the public key to appease dotenv-linter.

Apply:

-NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY
-NUXT_GOOGLE_MAPS_SERVER_API_KEY=AGOOGLE_MAPS_API_KEY
+NUXT_GOOGLE_MAPS_SERVER_API_KEY=GOOGLE_MAPS_SERVER_API_KEY
+NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY

Verify usage and exposure:


🏁 Script executed:

#!/bin/bash
# 1) Is the legacy backend key still used?
rg -n --hidden -S '\bGOOGLE_MAPS_API_KEY_BACKEND\b' -C2

# 2) Ensure the server key is not referenced in client-only code
rg -n --hidden -S '\bNUXT_GOOGLE_MAPS_SERVER_API_KEY\b|\bgoogleMapsServerApiKey\b' -g '!**/server/**' -C2

# 3) Public key only in client contexts
rg -n --hidden -S '\bNUXT_PUBLIC_GOOGLE_MAPS_API_KEY\b' -C2

Length of output: 788


Fix typo, remove unused legacy key, and reorder env vars in .env.example (lines 21–23).

  • Typo: NUXT_GOOGLE_MAPS_SERVER_API_KEY is set to "AGOOGLE_MAPS_API_KEY" — change to GOOGLE_MAPS_SERVER_API_KEY.
  • Remove GOOGLE_MAPS_API_KEY_BACKEND from .env.example (no other references found).
  • Move NUXT_GOOGLE_MAPS_SERVER_API_KEY above NUXT_PUBLIC_GOOGLE_MAPS_API_KEY to satisfy dotenv-linter.

Apply:

-NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY
-NUXT_GOOGLE_MAPS_SERVER_API_KEY=AGOOGLE_MAPS_API_KEY
+NUXT_GOOGLE_MAPS_SERVER_API_KEY=GOOGLE_MAPS_SERVER_API_KEY
+NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 dotenv-linter (3.3.0)

[warning] 22-22: [UnorderedKey] The NUXT_GOOGLE_MAPS_SERVER_API_KEY key should go before the NUXT_PUBLIC_GOOGLE_MAPS_API_KEY key

(UnorderedKey)


[warning] 23-23: [UnorderedKey] The GOOGLE_MAPS_API_KEY_BACKEND key should go before the NUXT_GOOGLE_MAPS_SERVER_API_KEY key

(UnorderedKey)

🤖 Prompt for AI Agents
In .env.example around lines 21 to 23: fix the typo and reorder/remove keys —
change the value for NUXT_GOOGLE_MAPS_SERVER_API_KEY from "AGOOGLE_MAPS_API_KEY"
to "GOOGLE_MAPS_SERVER_API_KEY", remove the unused GOOGLE_MAPS_API_KEY_BACKEND
entry entirely, and place the NUXT_GOOGLE_MAPS_SERVER_API_KEY line above
NUXT_PUBLIC_GOOGLE_MAPS_API_KEY to satisfy dotenv-linter ordering.

-add transaction support for atomicity.
-Separate client and server Google Maps API keys
-Make getAddressFromPlaceId return a consistent shape; harden findOrCreateCity against empty locality; upsert for idempotency.
-Constrain search to locality and add timeout/error handling; prefer locality result.
-fetch Place Details server-side.
-Encode placeId and assert server key.
-Re-check existence inside the transaction to avoid TOCTOU races.
-Return null instead of {} when no places for consistent nullability.
-Hoist Google Maps client to a module singleton.
-Callers handle null; addCity won't throw on destructuring.
-Timeout + error handling for Google API call;
-Make slug generation race-safe
- Remove using neighborhood as city fallback
Do not mutate slug on updates
Feat:Add postal_town fallback and type the return for getAddress
-deprecate addCity
-Short‑circuit if city already exists; avoid unnecessary Google calls.
-Adjust slug retry strategy
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (3)
.env.example (1)

21-23: Fix key ordering/placeholder; drop legacy key if unused.

  • Move NUXT_GOOGLE_MAPS_SERVER_API_KEY before the public key to satisfy dotenv-linter.
  • Replace placeholder "AGOOGLE_MAPS_API_KEY" with "GOOGLE_MAPS_SERVER_API_KEY".
  • Remove GOOGLE_MAPS_API_KEY_BACKEND if no longer referenced.

Apply:

-NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY
-NUXT_GOOGLE_MAPS_SERVER_API_KEY=AGOOGLE_MAPS_API_KEY
+NUXT_GOOGLE_MAPS_SERVER_API_KEY=GOOGLE_MAPS_SERVER_API_KEY
+NUXT_PUBLIC_GOOGLE_MAPS_API_KEY=GOOGLE_MAPS_API_KEY
-GOOGLE_MAPS_API_KEY_BACKEND=GOOGLE_MAPS_API_KEY_BACKEND
server/utils/city.ts (2)

45-57: Use server API key, encode placeId, add timeout + consistent null failures.

  • Don’t use public key on server; read googleMapsServerApiKey.
  • Encode placeId; add AbortController timeout and ok/status checks.
  • Return null (not []) on failure for a consistent type.
 const getAddressFromPlaceId = async (placeId: string) => {
   if (!placeId) {
-    return null
+    return null
   }
 
-  const apiKey = useRuntimeConfig().public.googleMapsApiKey
-  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${placeId}&key=${apiKey}`
-  const res = await fetch(url)
-  const data = await res.json()
-
-  if (data.status !== 'OK') {
-    return []
-  }
-
-  return getAddress(data.results)
+  const { googleMapsServerApiKey } = useRuntimeConfig()
+  if (!googleMapsServerApiKey) throw new Error('Google Maps server API key is missing')
+  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${googleMapsServerApiKey}`
+
+  const controller = new AbortController()
+  const timer = setTimeout(() => controller.abort(), 5000)
+  try {
+    const res = await fetch(url, { signal: controller.signal })
+    if (!res.ok) return null
+    const data = await res.json()
+    if (data.status !== 'OK' || !Array.isArray(data.results) || data.results.length === 0) {
+      return null
+    }
+    return getAddress(data.results)
+  } catch {
+    return null
+  } finally {
+    clearTimeout(timer)
+  }
 }

70-74: Fail fast when coordinates missing; avoid Null Island (0,0).

Geocoding should always yield geometry; defaulting to 0 corrupts data. Validate lat/lng before upsert.

-  const { locality, region, country, lat, lng } = address
+  const { locality, region, country, lat, lng } = address
+  if (typeof lat !== 'number' || typeof lng !== 'number') {
+    throw new Error("Couldn't determine coordinates for city")
+  }
🧹 Nitpick comments (4)
server/utils/city.ts (4)

94-109: Don’t write lat/lng = 0; persist validated numbers.

Use the validated lat/lng; remove “?? 0” defaults. Keep slug immutable on update (already good).

   const createData = {
     id: placeId,
     name: locality,
     region: region || '',
     slug: slug,
     countryCode,
-    lat: address.lat ?? 0,
-    lng: address.lng ?? 0,
+    lat,
+    lng,
   }
   const updateData = {
     name: locality,
     region: region || '',
     countryCode,
-    lat: address.lat ?? 0,
-    lng: address.lng ?? 0,
+    lat,
+    lng,
   }

75-91: Slug conflict: avoid throwing; retry with suffixed slug or handle P2002.

Throwing on an existing region+locality will block valid cities and is race-prone. Prefer a suffix/backoff or handle P2002 on upsert.

-  if (existingRegion) {
-    throw new Error(
-      `City slug conflict: A city with slug "${slug}" already exists.`
-    )
-  }
+  if (existingRegion) {
+    slug = getSlug(`${slug}-${Date.now().toString().slice(-4)}`)
+  }

Optionally, catch Prisma P2002 in the upsert and retry with a numeric suffix up to N attempts.


50-53: Minor: URL safety.

Also encode the API key to be safe, even if it’s typically URL-safe.

-  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${googleMapsServerApiKey}`
+  const url = `https://maps.googleapis.com/maps/api/geocode/json?place_id=${encodeURIComponent(placeId)}&key=${encodeURIComponent(googleMapsServerApiKey)}`

20-43: Type consistency for helpers.

getAddress/addressPart currently return loosely-typed objects/strings. Consider adding a typed return to prevent silent empty-string propagation into slug generation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 886149c and ff95814.

📒 Files selected for processing (8)
  • .env.example (1 hunks)
  • components/inputs/VenueInput.vue (0 hunks)
  • nuxt.config.ts (1 hunks)
  • prisma/schema.prisma (2 hunks)
  • server/trpc/routers/cities.ts (2 hunks)
  • server/trpc/routers/profiles.ts (2 hunks)
  • server/utils/city.ts (2 hunks)
  • server/utils/google_maps.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • components/inputs/VenueInput.vue
🚧 Files skipped from review as they are similar to previous changes (4)
  • nuxt.config.ts
  • server/utils/google_maps.ts
  • server/trpc/routers/profiles.ts
  • server/trpc/routers/cities.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T06:33:48.474Z
Learnt from: egemengunel
PR: we-dance/v4#414
File: server/trpc/routers/chat.ts:24-35
Timestamp: 2025-09-09T06:33:48.474Z
Learning: In the WeDance chat system, lastMessageId and lastMessage fields were removed from the Conversation schema and should not be reintroduced. The team prefers to avoid schema changes for chat ordering improvements.

Applied to files:

  • prisma/schema.prisma
🧬 Code graph analysis (1)
server/utils/city.ts (3)
server/prisma.ts (1)
  • prisma (5-8)
utils/slug.ts (1)
  • getSlug (11-88)
utils/country.ts (1)
  • getCountryCode (22-49)
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 22-22: [UnorderedKey] The NUXT_GOOGLE_MAPS_SERVER_API_KEY key should go before the NUXT_PUBLIC_GOOGLE_MAPS_API_KEY key

(UnorderedKey)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: ci
🔇 Additional comments (4)
prisma/schema.prisma (2)

603-603: Cascade on Calendar.profile — confirm intended deletion semantics.

Setting onDelete: Cascade will delete a user’s calendars (and thus CalendarEvents via their Cascade) when a Profile is deleted. If that’s intended, LGTM; otherwise switch to Restrict.


294-296: Non-functional formatting change — OK.

Inline createdAt/updatedAt reads fine; no behavioral impact.

server/utils/city.ts (2)

62-67: Nice: idempotent find-or-create via upsert.

Early return on existing id reduces external API calls.


45-60: Ensure server key isn't exposed; remove unused legacy env vars.

  • server/utils/city.ts and lib/googleMapsApi.ts both use useRuntimeConfig().public.googleMapsApiKey — server geocoding is using the public (client-exposed) key.
  • .env.example contains NUXT_GOOGLE_MAPS_SERVER_API_KEY and GOOGLE_MAPS_API_KEY_BACKEND but no code references were found; delete these env entries and any deployment references if they are unused.
  • If geocoding must remain server-only, move the key to a private runtime config (e.g., useRuntimeConfig().googleMapsServerApiKey) and update server code to use that instead of the public key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Event Editor: Venue (import from Google Maps)

3 participants