diff --git a/backend/app/api/weather.py b/backend/app/api/weather.py index 665c68ae..66e09253 100644 --- a/backend/app/api/weather.py +++ b/backend/app/api/weather.py @@ -3,15 +3,18 @@ from fastapi import APIRouter, Depends, HTTPException, Query, status from pydantic import BaseModel, Field - from app.models.user import User -from app.services.weather_service import WeatherService, WeatherServiceError +from app.services.weather_service import GeocodingServiceError, WeatherService, WeatherServiceError from app.utils.auth import get_current_user logger = logging.getLogger(__name__) router = APIRouter(prefix="/weather", tags=["Weather"]) +GEOCODING_FAILURE_DETAIL = ( + "Unable to geocode saved location name. Please try again later or update your location in settings." +) + class WeatherResponse(BaseModel): temperature: float = Field(description="Temperature in Celsius") @@ -54,18 +57,28 @@ async def get_current_weather( latitude: float | None = Query(None, ge=-90, le=90), longitude: float | None = Query(None, ge=-180, le=180), ) -> WeatherResponse: - # Use provided coordinates or fall back to user's location + 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 + if lat is None or lon is None: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Location not set. Please provide coordinates or set your location in settings.", ) - weather_service = WeatherService() - try: weather = await weather_service.get_current_weather(float(lat), float(lon)) except WeatherServiceError as e: @@ -97,18 +110,28 @@ async def get_weather_forecast( longitude: float | None = Query(None, ge=-180, le=180), days: int = Query(7, ge=1, le=16), ) -> ForecastResponse: - # Use provided coordinates or fall back to user's location + 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 + if lat is None or lon is None: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Location not set. Please provide coordinates or set your location in settings.", ) - weather_service = WeatherService() - try: forecast = await weather_service.get_daily_forecast(float(lat), float(lon), days) except WeatherServiceError as e: diff --git a/backend/app/config.py b/backend/app/config.py index e8846819..03fbaf8d 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -52,6 +52,7 @@ class Settings(BaseSettings): # Weather openmeteo_url: str = Field(default="https://api.open-meteo.com/v1") + geocoding_user_agent: str | None = Field(default=None) # Notifications - default ntfy channel (used when user has none configured) ntfy_server: str | None = None @@ -110,6 +111,9 @@ def get_auth_mode(self) -> str: return "oidc" return "unknown" + def get_geocoding_user_agent(self) -> str: + return self.geocoding_user_agent or "Wardrowbe/1.0" + @lru_cache def get_settings() -> Settings: diff --git a/backend/app/services/weather_service.py b/backend/app/services/weather_service.py index 9ba227f9..a8e50fe9 100644 --- a/backend/app/services/weather_service.py +++ b/backend/app/services/weather_service.py @@ -98,6 +98,49 @@ class WeatherService: def __init__(self): self.base_url = settings.openmeteo_url + async def geocode_location_name(self, location_name: str) -> tuple[float, float, str | None] | None: + """Resolve a free-form location name to coordinates using Nominatim.""" + query = location_name.strip() + if not query: + return None + + params = { + "q": query, + "format": "jsonv2", + "limit": 1, + } + + headers = { + "User-Agent": settings.get_geocoding_user_agent(), + } + + async with httpx.AsyncClient(timeout=10.0, follow_redirects=True, headers=headers) as client: + try: + response = await client.get("https://nominatim.openstreetmap.org/search", params=params) + response.raise_for_status() + data = response.json() + except httpx.HTTPError as e: + logger.error(f"Geocoding error for {query!r}: {e}") + raise GeocodingServiceError(f"Failed to geocode location {query!r}: {e}") from None + except ValueError as e: + logger.error(f"Geocoding returned invalid JSON for {query!r}: {e}") + raise GeocodingServiceError( + f"Failed to decode geocoding response for location {query!r}: {e}" + ) from None + + if not data: + return None + + first = data[0] + try: + latitude = float(first["lat"]) + longitude = float(first["lon"]) + except (KeyError, TypeError, ValueError): + return None + + display_name = first.get("display_name") + return latitude, longitude, display_name + @staticmethod def _cache_key(lat: float, lon: float) -> str: return f"{CACHE_PREFIX}{round(lat, 2)},{round(lon, 2)}" @@ -349,3 +392,7 @@ async def check_health(self) -> dict: class WeatherServiceError(Exception): pass + + +class GeocodingServiceError(WeatherServiceError): + pass diff --git a/backend/tests/test_weather_api.py b/backend/tests/test_weather_api.py new file mode 100644 index 00000000..3d6f4778 --- /dev/null +++ b/backend/tests/test_weather_api.py @@ -0,0 +1,106 @@ +from datetime import datetime +from unittest.mock import AsyncMock, patch + +import pytest +from httpx import AsyncClient + +from app.api.weather import GEOCODING_FAILURE_DETAIL +from app.services.weather_service import GeocodingServiceError + + +class TestWeatherApi: + @pytest.mark.asyncio + async def test_current_weather_uses_location_name_without_persisting_coordinates( + self, client: AsyncClient, test_user, auth_headers, db_session + ): + test_user.location_name = "New York City" + test_user.location_lat = None + test_user.location_lon = None + await db_session.commit() + + geocode_mock = AsyncMock(return_value=(40.7128, -74.0060, "New York City")) + weather_mock = AsyncMock( + return_value=type( + "Weather", + (), + { + "temperature": 12.5, + "feels_like": 7.3, + "humidity": 50, + "precipitation_chance": 10, + "precipitation_mm": 0.0, + "wind_speed": 23.4, + "condition": "partly cloudy", + "condition_code": 2, + "is_day": True, + "uv_index": 1.8, + "timestamp": datetime(2026, 5, 12, 15, 21, 35), + }, + )() + ) + + with ( + patch("app.api.weather.WeatherService.geocode_location_name", geocode_mock), + patch("app.api.weather.WeatherService.get_current_weather", weather_mock), + ): + response = await client.get("/api/v1/weather/current", headers=auth_headers) + + assert response.status_code == 200 + geocode_mock.assert_awaited_once_with("New York City") + weather_mock.assert_awaited_once_with(40.7128, -74.0060) + + await db_session.refresh(test_user) + assert test_user.location_lat is None + assert test_user.location_lon is None + + @pytest.mark.asyncio + async def test_forecast_returns_400_when_location_missing( + self, client: AsyncClient, test_user, auth_headers, db_session + ): + test_user.location_name = None + test_user.location_lat = None + test_user.location_lon = None + await db_session.commit() + + response = await client.get("/api/v1/weather/forecast", headers=auth_headers) + + assert response.status_code == 400 + assert response.json()["detail"] == ( + "Location not set. Please provide coordinates or set your location in settings." + ) + + @pytest.mark.asyncio + async def test_current_weather_returns_503_when_saved_location_geocoding_fails( + self, client: AsyncClient, test_user, auth_headers, db_session + ): + test_user.location_name = "New York City" + test_user.location_lat = None + test_user.location_lon = None + await db_session.commit() + + geocode_mock = AsyncMock(side_effect=GeocodingServiceError("geocoder unavailable")) + + with patch("app.api.weather.WeatherService.geocode_location_name", geocode_mock): + response = await client.get("/api/v1/weather/current", headers=auth_headers) + + assert response.status_code == 503 + assert response.json()["detail"] == GEOCODING_FAILURE_DETAIL + geocode_mock.assert_awaited_once_with("New York City") + + @pytest.mark.asyncio + async def test_forecast_returns_503_when_saved_location_geocoding_fails( + self, client: AsyncClient, test_user, auth_headers, db_session + ): + test_user.location_name = "New York City" + test_user.location_lat = None + test_user.location_lon = None + await db_session.commit() + + geocode_mock = AsyncMock(side_effect=GeocodingServiceError("geocoder unavailable")) + + with patch("app.api.weather.WeatherService.geocode_location_name", geocode_mock): + response = await client.get("/api/v1/weather/forecast", headers=auth_headers) + + assert response.status_code == 503 + assert response.json()["detail"] == GEOCODING_FAILURE_DETAIL + geocode_mock.assert_awaited_once_with("New York City") diff --git a/backend/tests/test_weather_service.py b/backend/tests/test_weather_service.py index fc1804ed..40dd7fea 100644 --- a/backend/tests/test_weather_service.py +++ b/backend/tests/test_weather_service.py @@ -8,6 +8,7 @@ from app.services.weather_service import ( CACHE_PREFIX, + GeocodingServiceError, WMO_CODES, WeatherData, WeatherService, @@ -237,6 +238,19 @@ async def test_handles_missing_hourly_precipitation(self, weather_service, mock_ assert result.precipitation_chance == 0 +class TestGeocodeLocationName: + @pytest.mark.asyncio + async def test_raises_on_invalid_json_response(self, weather_service): + request = httpx.Request("GET", "https://nominatim.openstreetmap.org/search") + mock_response = httpx.Response(200, text="rate limited", request=request) + + with patch("httpx.AsyncClient.get", return_value=mock_response): + with pytest.raises( + GeocodingServiceError, match="Failed to decode geocoding response" + ): + await weather_service.geocode_location_name("New York City") + + class TestGetDailyForecast: @pytest.mark.asyncio async def test_returns_forecast_days(self, weather_service, mock_redis): diff --git a/frontend/app/dashboard/settings/page.tsx b/frontend/app/dashboard/settings/page.tsx index b5797346..305c2d0c 100644 --- a/frontend/app/dashboard/settings/page.tsx +++ b/frontend/app/dashboard/settings/page.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useState, useEffect } from 'react'; +import { useState, useEffect, useRef } from 'react'; import { useSession } from 'next-auth/react'; import { Loader2, Save, RotateCcw, Check, Plus, Trash2, ChevronUp, ChevronDown, Server, MapPin, Navigation, Ruler } from 'lucide-react'; import { Button } from '@/components/ui/button'; @@ -19,6 +19,12 @@ import { import { Badge } from '@/components/ui/badge'; import { usePreferences, useUpdatePreferences, useResetPreferences, useTestAIEndpoint } from '@/lib/hooks/use-preferences'; import { useUserProfile, useUpdateUserProfile } from '@/lib/hooks/use-user'; +import { + getNetworkLocationUrl, + formatReverseGeocodedLocation, + getGeolocationFailureMessage, + resolveNetworkLocation, +} from '@/lib/location'; import { CLOTHING_COLORS, OCCASIONS, Preferences, StyleProfile, AIEndpoint } from '@/lib/types'; import { toF, toCelsius } from '@/lib/temperature'; import { toast } from 'sonner'; @@ -191,7 +197,11 @@ export default function SettingsPage() { } return 'metric'; }); + const unitSystemRef = useRef(unitSystem); + useEffect(() => { + unitSystemRef.current = unitSystem; + }, [unitSystem]); useEffect(() => { if (userProfile) { @@ -203,9 +213,10 @@ export default function SettingsPage() { if (userProfile.body_measurements) { const initial: Record = {}; const numericKeys = ['chest', 'waist', 'hips', 'inseam', 'height', 'weight']; + const displayUnitSystem = unitSystemRef.current; for (const [key, value] of Object.entries(userProfile.body_measurements)) { if (numericKeys.includes(key) && typeof value === 'number') { - const converted = convertMeasurement(value, key, 'metric', unitSystem); + const converted = convertMeasurement(value, key, 'metric', displayUnitSystem); initial[key] = String(converted); } else { initial[key] = String(value); @@ -216,49 +227,90 @@ export default function SettingsPage() { } }, [userProfile]); + const detectLocationFromNetwork = async () => { + const response = await fetch(getNetworkLocationUrl(), { + headers: { Accept: 'application/json' }, + }); + + if (!response.ok) { + throw new Error( + `Network-based location lookup failed (${response.status}${response.statusText ? ` ${response.statusText}` : ''})` + ); + } + + const data = await response.json(); + const resolved = resolveNetworkLocation(data, timezone); + setLocationLat(resolved.lat); + setLocationLon(resolved.lon); + if (resolved.locationName) { + setLocationName(resolved.locationName); + } + if (resolved.timezone) { + setTimezone(resolved.timezone); + } + return resolved; + }; + const handleGetCurrentLocation = () => { + setIsGettingLocation(true); + const finalizeFromCoordinates = async (lat: string, lon: string) => { + // Reverse geocode to get city name + try { + const response = await fetch( + `https://nominatim.openstreetmap.org/reverse?lat=${lat}&lon=${lon}&format=json` + ); + if (response.ok) { + const data = await response.json(); + const nextLocationName = formatReverseGeocodedLocation(data); + if (nextLocationName) { + setLocationName(nextLocationName); + return nextLocationName; + } + } + } catch { + // Ignore geocoding errors, we still have coordinates + } + + return undefined; + }; + + const fallbackToNetworkLocation = async (reason?: string) => { + try { + await detectLocationFromNetwork(); + toast.success( + reason + ? `${reason} Approximate location filled in. Review it, then save.` + : 'Approximate location filled in. Review it, then save.' + ); + } catch (fallbackError) { + const fallbackMessage = fallbackError instanceof Error + ? fallbackError.message + : 'Unable to detect your location'; + toast.error(fallbackMessage); + } finally { + setIsGettingLocation(false); + } + }; + if (!navigator.geolocation) { - toast.error('Geolocation is not supported by your browser'); + void fallbackToNetworkLocation('Geolocation is not supported by your browser.'); return; } - setIsGettingLocation(true); navigator.geolocation.getCurrentPosition( async (position) => { const lat = position.coords.latitude.toFixed(6); const lon = position.coords.longitude.toFixed(6); setLocationLat(lat); setLocationLon(lon); - - // Reverse geocode to get city name - try { - const response = await fetch( - `https://nominatim.openstreetmap.org/reverse?lat=${lat}&lon=${lon}&format=json`, - { headers: { 'User-Agent': 'WardrobeAI/1.0' } } - ); - if (response.ok) { - const data = await response.json(); - const city = data.address?.city || data.address?.town || data.address?.village || data.address?.municipality; - const country = data.address?.country; - if (city && country) { - setLocationName(`${city}, ${country}`); - } else if (city) { - setLocationName(city); - } else if (data.display_name) { - // Fallback to first part of display name - setLocationName(data.display_name.split(',').slice(0, 2).join(',').trim()); - } - } - } catch { - // Ignore geocoding errors, we still have coordinates - } - + await finalizeFromCoordinates(lat, lon); setIsGettingLocation(false); - toast.success('Location detected'); + toast.success('Location detected. Review it, then save.'); }, (error) => { - setIsGettingLocation(false); - toast.error(`Failed to get location: ${error.message}`); + void fallbackToNetworkLocation( + getGeolocationFailureMessage(error) + ); }, { enableHighAccuracy: true, timeout: 10000 } ); diff --git a/frontend/lib/location.ts b/frontend/lib/location.ts new file mode 100644 index 00000000..a3e283bb --- /dev/null +++ b/frontend/lib/location.ts @@ -0,0 +1,114 @@ +export interface NetworkLocationApiResponse { + success?: boolean; + latitude?: number; + longitude?: number; + city?: string; + region?: string; + country?: string; + country_name?: string; + timezone?: { + id?: string; + } | string; + error?: boolean; + reason?: string; + message?: string; +} + +export interface ResolvedLocation { + lat: string; + lon: string; + locationName?: string; + timezone?: string; +} + +export interface ReverseGeocodeResponse { + address?: { + city?: string; + town?: string; + village?: string; + municipality?: string; + country?: string; + }; + display_name?: string; +} + +export const DEFAULT_NETWORK_LOCATION_URL = 'https://ipapi.co/json/'; + +export function getNetworkLocationUrl(): string { + return process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL || DEFAULT_NETWORK_LOCATION_URL; +} + +export function resolveNetworkLocation( + data: NetworkLocationApiResponse, + fallbackTimezone?: string +): ResolvedLocation { + if ( + data.success === false || + data.error === true || + typeof data.latitude !== 'number' || + typeof data.longitude !== 'number' + ) { + throw new Error(data.reason || data.message || 'Unable to determine location from network'); + } + + const city = typeof data.city === 'string' ? data.city : ''; + const region = typeof data.region === 'string' ? data.region : ''; + const country = + typeof data.country === 'string' + ? data.country + : typeof data.country_name === 'string' + ? data.country_name + : ''; + const labelParts = [city, region, country].filter(Boolean); + const timezoneId = + typeof data.timezone === 'string' + ? data.timezone + : data.timezone?.id; + + return { + lat: data.latitude.toFixed(6), + lon: data.longitude.toFixed(6), + locationName: labelParts.length > 0 ? labelParts.slice(0, 2).join(', ') : undefined, + timezone: + typeof timezoneId === 'string' && timezoneId + ? timezoneId + : fallbackTimezone, + }; +} + +export function formatReverseGeocodedLocation( + data: ReverseGeocodeResponse +): string | undefined { + const city = + data.address?.city || + data.address?.town || + data.address?.village || + data.address?.municipality; + const country = data.address?.country; + + if (city && country) return `${city}, ${country}`; + if (city) return city; + if (data.display_name) { + return data.display_name.split(',').slice(0, 2).join(',').trim(); + } + return undefined; +} + +export function getGeolocationFailureMessage(error: { + code?: number; + message?: string; +}): string { + const reasons: Record = { + 1: 'Location access was denied.', + 2: 'Location is currently unavailable.', + 3: 'Location request timed out.', + }; + + if (typeof error.code === 'number' && reasons[error.code]) { + return reasons[error.code]; + } + if (error.message) { + return `Failed to get exact location: ${error.message}`; + } + return 'Failed to get exact location.'; +} diff --git a/frontend/tests/location.test.ts b/frontend/tests/location.test.ts new file mode 100644 index 00000000..865b77fc --- /dev/null +++ b/frontend/tests/location.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from 'vitest' + +import { + DEFAULT_NETWORK_LOCATION_URL, + formatReverseGeocodedLocation, + getNetworkLocationUrl, + getGeolocationFailureMessage, + resolveNetworkLocation, +} from '@/lib/location' + +describe('location helpers', () => { + it('uses the default network provider URL when no env override is set', () => { + const original = process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL + delete process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL + + expect(getNetworkLocationUrl()).toBe(DEFAULT_NETWORK_LOCATION_URL) + + if (original === undefined) { + delete process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL + } else { + process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL = original + } + }) + + it('uses the configured network provider URL when provided', () => { + const original = process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL + process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL = 'https://geo.example.com/json' + + expect(getNetworkLocationUrl()).toBe('https://geo.example.com/json') + + if (original === undefined) { + delete process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL + } else { + process.env.NEXT_PUBLIC_NETWORK_LOCATION_URL = original + } + }) + + it('resolves network location with formatted city label and timezone', () => { + const result = resolveNetworkLocation({ + success: true, + latitude: 40.7128, + longitude: -74.006, + city: 'New York', + region: 'New York', + country: 'United States', + timezone: { id: 'America/New_York' }, + }, 'UTC') + + expect(result).toEqual({ + lat: '40.712800', + lon: '-74.006000', + locationName: 'New York, New York', + timezone: 'America/New_York', + }) + }) + + it('falls back to provided timezone when network response omits one', () => { + const result = resolveNetworkLocation({ + success: true, + latitude: 40.7128, + longitude: -74.006, + }, 'America/New_York') + + expect(result.timezone).toBe('America/New_York') + }) + + it('supports alternate provider response shapes', () => { + const result = resolveNetworkLocation({ + latitude: 37.7749, + longitude: -122.4194, + city: 'San Francisco', + region: 'California', + country_name: 'United States', + timezone: 'America/Los_Angeles', + }, 'UTC') + + expect(result).toEqual({ + lat: '37.774900', + lon: '-122.419400', + locationName: 'San Francisco, California', + timezone: 'America/Los_Angeles', + }) + }) + + it('throws when network location is incomplete', () => { + expect(() => resolveNetworkLocation({ success: false }, 'UTC')).toThrow( + 'Unable to determine location from network' + ) + }) + + it('formats reverse geocoding responses consistently', () => { + expect(formatReverseGeocodedLocation({ + address: { city: 'London', country: 'United Kingdom' }, + })).toBe('London, United Kingdom') + + expect(formatReverseGeocodedLocation({ + display_name: 'Paris, Ile-de-France, France', + })).toBe('Paris, Ile-de-France') + }) + + it('maps geolocation failure reasons to user-facing messages', () => { + expect(getGeolocationFailureMessage({ code: 1 })).toBe('Location access was denied.') + expect(getGeolocationFailureMessage({ code: 2 })).toBe('Location is currently unavailable.') + expect(getGeolocationFailureMessage({ code: 3 })).toBe('Location request timed out.') + expect(getGeolocationFailureMessage({ message: 'Permission prompt dismissed' })).toBe( + 'Failed to get exact location: Permission prompt dismissed' + ) + }) +})