-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add venue import from Google Maps #420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughUpdates 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
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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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. Comment |
There was a problem hiding this 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
googleMapsApiKeyout ofpublicinto the top-levelruntimeConfig(e.g.runtimeConfig.googleMapsApiKey = process.env.GOOGLE_MAPS_API_KEY) and setruntimeConfig.public.googleMapsApiKey = process.env.NUXT_PUBLIC_GOOGLE_MAPS_API_KEYinstead.- In server code (server/utils/google_maps.ts:32, server/utils/city.ts:66) and lib/googleMapsApi.ts:10, use
config.googleMapsApiKeyrather thanconfig.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
@uniqueonCity.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_townfor 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 = stringand 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
Clientper 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_townis 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) readsuseRuntimeConfig().public.googleMapsApiKeyand interpolatesplaceIdunencoded. Switch to private key andencodeURIComponent(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 ofnode-fetchto 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.locationexposeslat()/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
websiteandphonefields 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
📒 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
getSlugutility for consistent slug formatting.
365-365: Null cityId is safe to allow: filtering, includes, and schema definitions all handlecityId: nullwithout errors—no further changes needed.
There was a problem hiding this 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 toprofile.placeId
TheplaceIdfield inmodel Profileis missing an@uniqueannotation. Add@uniquetoplaceIdin bothprisma/schema.prismaandcli/prisma/schema.prisma, then runprisma migrate dev.
profile.usernameandcity.slugalready have unique constraints.server/utils/city.ts (1)
61-76: Add null-check foraddressinaddCitybefore destructuring
Inserver/utils/city.tsat the start ofaddCity(around line 79),getAddressFromPlaceIdcan returnnull; you must guard against this before doingconst { locality, region, country } = address // … address.lat, address.lngto 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.slugis 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”).
getCityIdFromGooglePlaceuses a text search by name. Prefer reverse geocoding of the venue’s lat/lng withresult_type=localityto get an exact cityplace_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
📒 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.
There was a problem hiding this 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 handlenullnow;addCitywill throw on destructuring.
getAddressFromPlaceIdreturnsnullon failures, butaddCitydestructuresaddresswithout 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
googleMapsServerApiKeyand 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 existingplaceIdmatches.
🧹 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 mapsClientAlso 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 avoidanyleaks.
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 aboveAlso applies to: 47-47
127-159: Slug collisions still possible under concurrency; add retry on unique error.Rely on DB unique constraint for
slugand retry with suffix onP2002.- 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 successfulAdditionally, consider a DB unique index on
city.slugif not present.server/trpc/routers/profiles.ts (3)
316-326: Wrap Place Details fetch to return controlled errors.If Google fails (network/quota),
getPlaceDetailsmay 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.
|| nullcoerces 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.placeIdandprofile.usernamein Prisma schema; this complements the retry logic and prevents duplicates under races.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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)
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: Returnnullon 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.
There was a problem hiding this 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” fallbackUsing 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 observabilityAdd structured logs for non-OK and timeouts to aid debugging and alerting.
Also applies to: 72-74
21-59: Type the helpersDefine 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
📒 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)
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 goodReturning null on empty results makes downstream checks simpler. LGTM.
server/utils/city.ts
Outdated
| 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 | ||
| } |
There was a problem hiding this comment.
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' -nLength 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' || trueLength of output: 899
Enforce unique slug on City and update call sites
- Add
@uniqueto theslugfield in bothprisma/schema.prismaandcli/prisma/schema.prismaunder themodel Cityto guarantee slug uniqueness. - Replace all remaining
addCity(…)invocations—inserver/utils/city.ts,server/trpc/routers/cities.ts,cli/importer/profile.ts, andcli/importer/index.ts—withfindOrCreateCity(…)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.
There was a problem hiding this 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 checksNice use of AbortController, result validation, and finally to clear the timer.
89-91: Guarding locality and country is correctPrevents 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/**' || trueIf 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 getAddressSome 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 checkMinor consistency/readability nit.
- data.status != 'OK' || + data.status !== 'OK' ||
142-144: Make the error actionable and include placeIdHelps 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
📒 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)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 handlingImport is appropriate for differentiating unique-constraint errors in upsert.
There was a problem hiding this 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
📒 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.
There was a problem hiding this 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
📒 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.
| 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) |
There was a problem hiding this comment.
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/**' || trueLength 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*\()' || trueLength 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 withfindOrCreateCity(...)(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
findOrCreateCityand 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.
There was a problem hiding this 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 themThe 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 UXfindOrCreateCity 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
📒 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 goodAligns router with the new collision-safe slug/upsert flow in server/utils/city.ts.
There was a problem hiding this 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>andCityAddress|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.slughas 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
📒 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
|| nullfor 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
| where: { placeId }, | ||
| }) | ||
| if (existingVenue) { | ||
| return existingVenue | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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" || trueLength 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.
There was a problem hiding this 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 @uniqueplus@@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)}, keeppairKeyand drop the composite unique; otherwise droppairKeyand 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/bandMessage.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: SetNullto 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/Messagein 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
📒 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
| 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 |
There was a problem hiding this comment.
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_KEYlooks accidental; use a clear placeholder. - Potential duplication: confirm whether
GOOGLE_MAPS_API_KEY_BACKENDis still referenced now thatNUXT_GOOGLE_MAPS_SERVER_API_KEYexists; 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_KEYVerify 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' -C2Length 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_KEYCommittable 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.
4062cf2 to
ff95814
Compare
There was a problem hiding this 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_BACKENDserver/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
📒 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.
This PR adds venue imports directly from Google Maps.
Closes #417
Summary by CodeRabbit
New Features
Changes
Breaking Changes
Documentation