feat: add geocoding and location provider utilities#77
Closed
gzhang33 wants to merge 5 commits into
Closed
Conversation
Add geocoding service to resolve location names to coordinates using Nominatim API. Add frontend location utilities for network-based location detection and reverse geocoding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Move geocoding user-agent configuration to Settings class. Add environment variable overrides for geocoding and network location provider. Update frontend location utilities with configurable URL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Simplify exception handling in geocoding service. Fix unit system reference in settings page body measurements initialization. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced May 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds geocoding support (Nominatim) and network-based location utilities so the app can resolve a saved location_name into coordinates when coordinates aren’t available, and improves the frontend UX around location detection.
Changes:
- Backend: add
WeatherService.geocode_location_name()and geocoding-related config (GEOCODING_USER_AGENT,GEOCODING_CONTACT,APP_VERSION), plus API fallback to geocodecurrent_user.location_name. - Frontend: add location helper utilities + tests, and add a network-based location fallback flow in Settings.
- Config/docs: surface new env vars in
.env.exampleand wire FastAPI version tosettings.app_version.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/tests/location.test.ts |
Adds Vitest coverage for new location helper utilities. |
frontend/lib/location.ts |
Introduces network location parsing, reverse geocode label formatting, and geolocation error messaging helpers. |
frontend/app/dashboard/settings/page.tsx |
Adds network-based location detection fallback and refactors reverse-geocoding formatting/error messaging. |
backend/tests/test_weather_service.py |
Adds a test for geocoding invalid JSON error handling. |
backend/tests/test_weather_api.py |
Adds tests for weather endpoints’ geocoding fallback behavior and failure mapping. |
backend/tests/test_config.py |
Adds tests for geocoding User-Agent composition logic. |
backend/app/services/weather_service.py |
Implements Nominatim geocoding and introduces GeocodingServiceError. |
backend/app/main.py |
Uses settings.app_version for FastAPI app version metadata. |
backend/app/config.py |
Centralizes geocoding configuration and builds a Nominatim-compliant User-Agent string. |
backend/app/api/weather.py |
Adds “location_name → geocode → coords” fallback and returns 503 on geocoding service failures. |
.env.example |
Documents new geocoding/network location-related environment variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+245
to
+247
| if (resolved.locationName) { | ||
| setLocationName(resolved.locationName); | ||
| } |
Comment on lines
61
to
+75
| lat = latitude if latitude is not None else current_user.location_lat | ||
| lon = longitude if longitude is not None else current_user.location_lon | ||
|
|
||
| if (lat is None or lon is None) and current_user.location_name: | ||
| try: | ||
| geocoded = await weather_service.geocode_location_name(current_user.location_name) | ||
| except GeocodingServiceError as e: | ||
| logger.error(f"Geocoding service error: {e}") | ||
| raise HTTPException( | ||
| status_code=status.HTTP_503_SERVICE_UNAVAILABLE, | ||
| detail=GEOCODING_FAILURE_DETAIL, | ||
| ) from None | ||
| if geocoded: | ||
| lat, lon, _ = geocoded | ||
|
|
Comment on lines
+113
to
+128
| weather_service = WeatherService() | ||
| lat = latitude if latitude is not None else current_user.location_lat | ||
| lon = longitude if longitude is not None else current_user.location_lon | ||
|
|
||
| if (lat is None or lon is None) and current_user.location_name: | ||
| try: | ||
| geocoded = await weather_service.geocode_location_name(current_user.location_name) | ||
| except GeocodingServiceError as e: | ||
| logger.error(f"Geocoding service error: {e}") | ||
| raise HTTPException( | ||
| status_code=status.HTTP_503_SERVICE_UNAVAILABLE, | ||
| detail=GEOCODING_FAILURE_DETAIL, | ||
| ) from None | ||
| if geocoded: | ||
| lat, lon, _ = geocoded | ||
|
|
Author
|
Closing in favor of existing PR #75 which covers the same geocoding and location fallback functionality. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
pytest— all backend tests pass including new geocoding testsnpx vitest run— all frontend location tests pass🤖 Generated with Claude Code