fix(weather): day-local named anchor, cache ttl and past-days forecast window (#2167)

2026-10-20 05:56:53 +00:00 · 2026-09-01 15:48:37 +02:00
parent c21e86e3bd
commit 03bd5c77af
41 changed files with 568 additions and 105 deletions
@@ -56,9 +56,11 @@ interface DayDetailPanelProps {
  mobile?: boolean
  /** Rename the day from here — the sidebar pencil moved to the transit search (#1065). */
  onUpdateDayTitle?: (dayId: number, title: string) => void
+  /** Name of the place the day's weather is anchored to — captioned above the forecast (#2167). */
+  weatherPlaceName?: string | null
 }

-export default function DayDetailPanel({ day, days, places, categories = [], tripId, assignments, reservations = [], lat, lng, onClose, onAccommodationChange, leftWidth = 0, rightWidth = 0, collapsed: collapsedProp = false, onToggleCollapse, mobile = false, onUpdateDayTitle }: DayDetailPanelProps) {
+export default function DayDetailPanel({ day, days, places, categories = [], tripId, assignments, reservations = [], lat, lng, onClose, onAccommodationChange, leftWidth = 0, rightWidth = 0, collapsed: collapsedProp = false, onToggleCollapse, mobile = false, onUpdateDayTitle, weatherPlaceName = null }: DayDetailPanelProps) {
  const { t, language, locale } = useTranslation()
  const can = useCanDo()
  const tripObj = useTripStore((s) => s.trip)
@@ -211,6 +213,13 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri
              </div>
            ) : weather ? (
              <div>
+                {/* Which place the forecast is for — on a roadtrip "the day's weather"
+                    is ambiguous without it (#2167). */}
+                {weatherPlaceName && (
+                  <div className="text-content-faint" style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', fontWeight: 600, textTransform: 'uppercase', letterSpacing: '0.05em', marginBottom: 6 }}>
+                    {t('day.weatherFor', { name: weatherPlaceName })}
+                  </div>
+                )}
                {/* Summary row */}
                <div style={{ display: 'flex', alignItems: 'center', gap: 12, marginBottom: 10 }}>
                  <div style={{ width: 40, height: 40, borderRadius: 12, background: 'var(--bg-secondary)', display: 'flex', alignItems: 'center', justifyContent: 'center', flexShrink: 0 }}>
@@ -87,7 +87,9 @@ vi.mock('../../hooks/useDayNotes', () => ({
 }))

 vi.mock('../Weather/WeatherWidget', () => ({
-  default: () => <span data-testid="weather-widget" />,
+  default: (props: { locationName?: string | null }) => (
+    <span data-testid="weather-widget" data-location={props.locationName ?? ''} />
+  ),
 }))

 // A stable toast object so tests can assert on the messages the sidebar raises.
@@ -2896,11 +2898,34 @@ describe('DayPlanSidebar', () => {
    expect(onReorder).not.toHaveBeenCalled()
  })

-  it('FE-PLANNER-DAYPLAN-130: the weather badge falls back to any located trip place', () => {
+  // #2167 — the badge used to borrow ANY located trip place, so a roadtrip day
+  // without stops silently showed another city's weather. Day-local only now.
+  it('FE-PLANNER-DAYPLAN-130: no weather badge for a day without located stops or a bookend hotel (#2167)', () => {
    const located = buildPlace({ id: 5, name: 'Somewhere', lat: 48.85, lng: 2.35 })
    const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' })
    render(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [located] })} />)
-    expect(screen.getByTestId('weather-widget')).toBeInTheDocument()
+    expect(screen.queryByTestId('weather-widget')).not.toBeInTheDocument()
+  })
+
+  it('FE-PLANNER-DAYPLAN-130b: the weather badge anchors to the day bookend hotel, independent of the optimize setting (#2167)', () => {
+    // The hotel fallback is unconditional (matching the mobile timeline) — turn the
+    // route-optimization setting OFF to prove the weather anchor ignores it.
+    seedStore(useSettingsStore, { settings: { time_format: '24h', temperature_unit: 'celsius', optimize_from_accommodation: false } } as any)
+    const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' })
+    const day2 = buildDay({ id: 11, date: '2025-06-02', title: 'Day 2' })
+    const accommodations = [{ id: 1, start_day_id: 10, end_day_id: 11, place_name: 'Hotel Roma', place_lat: 41.9, place_lng: 12.5 }]
+    render(<DayPlanSidebar {...makeDefaultProps({ days: [day, day2], accommodations: accommodations as any })} />)
+    const widgets = screen.getAllByTestId('weather-widget')
+    expect(widgets.length).toBeGreaterThan(0)
+    expect(widgets[0]).toHaveAttribute('data-location', 'Hotel Roma')
+  })
+
+  it('FE-PLANNER-DAYPLAN-130c: the weather badge names the day-local stop it is anchored to (#2167)', () => {
+    const place = buildPlace({ id: 5, name: 'Louvre', lat: 48.86, lng: 2.34 })
+    const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' })
+    const a = buildAssignment({ id: 99, day_id: 10, order_index: 0, place })
+    render(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [place], assignments: { '10': [a] } })} />)
+    expect(screen.getByTestId('weather-widget')).toHaveAttribute('data-location', 'Louvre')
  })

  it('FE-PLANNER-DAYPLAN-131: a read-only trip drops the edit affordances from day and note rows', () => {
@@ -1043,10 +1043,6 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
    return formatMoneySum(entries, costBase, locale, fxRates)
  }, [days, assignments, currency, costBase, locale, fxRates])

-  // Bester verfügbarer Standort für Wetter: zugewiesene Orte zuerst, dann beliebiger Reiseort
-  const anyGeoAssignment = Object.values(assignments).flatMap(da => da).find(a => a.place?.lat && a.place?.lng)
-  const anyGeoPlace = anyGeoAssignment || (places || []).find(p => p.lat && p.lng)
-
  return {
    tripId,
    trip,
@@ -1182,8 +1178,6 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
    handleOptimize,
    handleDropOnDay,
    totalCostLabel,
-    anyGeoAssignment,
-    anyGeoPlace,
    expandedRouteDayIds,
    setExpandedRouteDayIds,
  }
@@ -1355,8 +1349,6 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
    handleOptimize,
    handleDropOnDay,
    totalCostLabel,
-    anyGeoAssignment,
-    anyGeoPlace,
    expandedRouteDayIds,
    setExpandedRouteDayIds,
  } = S
@@ -1554,7 +1546,12 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
          // routable when accommodation optimization can bookend it with a hotel
          // (hotel → place → hotel, the same line the map draws) — otherwise the tools
          // vanish on such a day (#1330). Purely additive to the 2+ case.
-          const routeBookends = optimizeFromAccommodation !== false ? getDayBookendHotels(day, days, accommodations) : null
+          // The weather anchor below must not depend on the optimize-from-accommodation
+          // setting — where you wake up is a fact about the day, not a routing choice —
+          // so the bookend lookup runs unconditionally (mirrors useMPlanTimeline) while
+          // the route tools keep honoring the setting via routeBookends.
+          const dayBookends = getDayBookendHotels(day, days, accommodations)
+          const routeBookends = optimizeFromAccommodation !== false ? dayBookends : null
          const hasRouteBookend = !!(
            (routeBookends?.morning?.place_lat != null && routeBookends?.morning?.place_lng != null) ||
            (routeBookends?.evening?.place_lat != null && routeBookends?.evening?.place_lng != null)
@@ -1668,12 +1665,14 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
              >
                {/* Tages-Badge: Nummer oben, darunter (falls vorhanden) das Wetter des Tages */}
                {(() => {
-                  // anyGeoPlace is an assignment (has .place) or a bare place — read coords from either.
-                  const geoLat = anyGeoPlace ? ('place' in anyGeoPlace ? anyGeoPlace.place?.lat : anyGeoPlace.lat) : undefined
-                  const geoLng = anyGeoPlace ? ('place' in anyGeoPlace ? anyGeoPlace.place?.lng : anyGeoPlace.lng) : undefined
-                  const wLat = loc?.place?.lat ?? geoLat
-                  const wLng = loc?.place?.lng ?? geoLng
-                  const hasWeather = !!(day.date && anyGeoPlace && wLat != null && wLng != null)
+                  // Day-local anchor only (#2167): the day's first located stop, else the
+                  // hotel you wake up in. No trip-wide fallback — on a roadtrip that
+                  // silently showed another city's weather with nothing naming the place.
+                  const weatherHotel = loc == null ? dayBookends.morning : undefined
+                  const wLat = loc?.place?.lat ?? weatherHotel?.place_lat
+                  const wLng = loc?.place?.lng ?? weatherHotel?.place_lng
+                  const weatherName = loc?.place?.name ?? weatherHotel?.place_name ?? null
+                  const hasWeather = !!(day.date && wLat != null && wLng != null)
                  return (
                    <div style={{
                      // With weather the badge is a tall stack and has to start at
@@ -1700,7 +1699,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
                        <>
                          <div style={{ width: '64%', height: 1, background: 'currentColor', opacity: 0.25 }} />
                          <div style={{ padding: '3px 0 4px' }}>
-                            <WeatherWidget lat={wLat} lng={wLng} date={day.date} stacked />
+                            <WeatherWidget lat={wLat ?? null} lng={wLng ?? null} date={day.date} stacked locationName={weatherName} />
                          </div>
                        </>
                      )}
@@ -113,9 +113,9 @@ describe('WeatherWidget', () => {
    })
  })

-  it('FE-COMP-WEATHERWIDGET-010: uses cached data from sessionStorage', async () => {
+  it('FE-COMP-WEATHERWIDGET-010: uses fresh cached data from sessionStorage without refetching', async () => {
    const cached = buildWeather({ temp: 20 })
-    sessionStorage.setItem('weather_48.86_2.35_2025-06-01', JSON.stringify(cached))
+    sessionStorage.setItem('weather_48.86_2.35_2025-06-01_en', JSON.stringify({ data: cached, fetchedAt: Date.now() }))
    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius' } })
    render(<WeatherWidget lat={48.86} lng={2.35} date="2025-06-01" />)
    await waitFor(() => {
@@ -127,7 +127,7 @@ describe('WeatherWidget', () => {
  it('FE-COMP-WEATHERWIDGET-011: re-fetches in background for cached climate data', async () => {
    const climateData = buildWeather({ temp: 15, main: 'Clouds', type: 'climate', description: 'cloudy' })
    const forecastData = buildWeather({ temp: 22, main: 'Clear', type: 'forecast', description: 'clear sky' })
-    sessionStorage.setItem('weather_48.86_2.35_2025-06-01', JSON.stringify(climateData))
+    sessionStorage.setItem('weather_48.86_2.35_2025-06-01_en', JSON.stringify({ data: climateData, fetchedAt: Date.now() }))
    vi.mocked(weatherApi.get).mockResolvedValue(forecastData)
    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius' } })

@@ -144,4 +144,68 @@ describe('WeatherWidget', () => {
    })
    expect(screen.queryByText(/Ø/)).not.toBeInTheDocument()
  })
+
+  // #2167 — a standalone PWA keeps sessionStorage alive across days, so a
+  // forecast fetched before the trip was still shown unchanged on the day itself.
+  it('FE-COMP-WEATHERWIDGET-012: revalidates a forecast cache entry older than the TTL', async () => {
+    const staleData = buildWeather({ temp: 20 })
+    const freshData = buildWeather({ temp: 22 })
+    sessionStorage.setItem(
+      'weather_48.86_2.35_2025-06-01_en',
+      JSON.stringify({ data: staleData, fetchedAt: Date.now() - 2 * 60 * 60 * 1000 }),
+    )
+    vi.mocked(weatherApi.get).mockResolvedValue(freshData)
+    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius' } })
+
+    render(<WeatherWidget lat={48.86} lng={2.35} date="2025-06-01" />)
+
+    // Serve-then-revalidate: the stale value renders immediately …
+    await waitFor(() => {
+      expect(screen.getByText(/20°C|22°C/)).toBeInTheDocument()
+    })
+    // … and the background refresh replaces it and the cache entry.
+    await waitFor(() => {
+      expect(screen.getByText('22°C')).toBeInTheDocument()
+    })
+    expect(weatherApi.get).toHaveBeenCalledTimes(1)
+    const stored = JSON.parse(sessionStorage.getItem('weather_48.86_2.35_2025-06-01_en')!)
+    expect(stored.data.temp).toBe(22)
+    expect(typeof stored.fetchedAt).toBe('number')
+  })
+
+  it('FE-COMP-WEATHERWIDGET-013: treats a pre-TTL raw cache entry as stale and refreshes it', async () => {
+    // Live PWA sessions still hold entries in the old bare-result format.
+    sessionStorage.setItem('weather_48.86_2.35_2025-06-01_en', JSON.stringify(buildWeather({ temp: 18 })))
+    vi.mocked(weatherApi.get).mockResolvedValue(buildWeather({ temp: 25 }))
+    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius' } })
+
+    render(<WeatherWidget lat={48.86} lng={2.35} date="2025-06-01" />)
+
+    await waitFor(() => {
+      expect(screen.getByText('25°C')).toBeInTheDocument()
+    })
+    expect(weatherApi.get).toHaveBeenCalledTimes(1)
+  })
+
+  // #2167 (mrwulf) — the widget must be able to say WHERE the forecast is for.
+  it('FE-COMP-WEATHERWIDGET-014: renders locationName as a tooltip title', async () => {
+    vi.mocked(weatherApi.get).mockResolvedValue(buildWeather({ temp: 20 }))
+    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius' } })
+    render(<WeatherWidget lat={48.86} lng={2.35} date="2025-06-01" locationName="Rome" />)
+    await waitFor(() => {
+      expect(screen.getByText('20°C')).toBeInTheDocument()
+    })
+    expect(screen.getByTitle('Rome')).toBeInTheDocument()
+  })
+
+  it('FE-COMP-WEATHERWIDGET-015: sends the user language and keys the cache by it', async () => {
+    vi.mocked(weatherApi.get).mockResolvedValue(buildWeather({ temp: 20 }))
+    useSettingsStore.setState({ settings: { ...useSettingsStore.getState().settings, temperature_unit: 'celsius', language: 'de' } })
+    render(<WeatherWidget lat={48.86} lng={2.35} date="2025-06-01" />)
+    await waitFor(() => {
+      expect(screen.getByText('20°C')).toBeInTheDocument()
+    })
+    expect(weatherApi.get).toHaveBeenCalledWith(48.86, 2.35, '2025-06-01', 'de')
+    expect(sessionStorage.getItem('weather_48.86_2.35_2025-06-01_de')).not.toBeNull()
+  })
 })
@@ -37,6 +37,11 @@ function setWeatherCache(key, value) {
  try { sessionStorage.setItem(key, JSON.stringify(value)) } catch {}
 }

+// Matches the server's own forecast TTL. In a standalone PWA sessionStorage lives
+// as long as the app process, so without a TTL a forecast fetched days before the
+// trip day was still shown unchanged on the day itself (#2167).
+const WEATHER_TTL_MS = 60 * 60 * 1000
+
 interface WeatherWidgetProps {
  lat: number | null
  lng: number | null
@@ -44,53 +49,70 @@ interface WeatherWidgetProps {
  compact?: boolean
  /** Vertical icon-over-temp layout that inherits its color (for the day badge). */
  stacked?: boolean
+  /** Name of the place the forecast is anchored to — surfaces as a tooltip (#2167). */
+  locationName?: string | null
 }

-export default function WeatherWidget({ lat, lng, date, compact = false, stacked = false }: WeatherWidgetProps) {
+export default function WeatherWidget({ lat, lng, date, compact = false, stacked = false, locationName = null }: WeatherWidgetProps) {
  const [weather, setWeather] = useState(null)
  const [loading, setLoading] = useState(false)
  const [failed, setFailed] = useState(false)
  const isFahrenheit = useSettingsStore(s => s.settings.temperature_unit) === 'fahrenheit'
+  const language = useSettingsStore(s => s.settings.language) || 'en'

  useEffect(() => {
    if (!lat || !lng || !date) return
+    let cancelled = false
    const rLat = Math.round(lat * 100) / 100
    const rLng = Math.round(lng * 100) / 100
-    const cacheKey = `weather_${rLat}_${rLng}_${date}`
-    const cached = getWeatherCache(cacheKey)
-    if (cached !== undefined) {
-      if (cached === null) setFailed(true)
-      // Climate data: use from cache but re-fetch in background to upgrade to forecast
-      else if (cached.type === 'climate') {
-        setWeather(cached)
-        fetchWeather(lat, lng, date)
-          .then(data => {
-            if (!data.error && data.temp !== undefined && data.type === 'forecast') {
-              setWeatherCache(cacheKey, data)
-              setWeather(data)
-            }
-          })
-          .catch(() => {})
-        return
-      } else {
-        setWeather(cached)
-        return
-      }
-      return
-    }
-    setLoading(true)
-    fetchWeather(lat, lng, date)
-      .then(data => {
-        if (data.error || data.temp === undefined) {
-          setFailed(true)
-        } else {
-          setWeatherCache(cacheKey, data)
+    // The language is part of the key: descriptions come back localized, and the
+    // server caches per language too — a lang-less key would re-mix them here.
+    const cacheKey = `weather_${rLat}_${rLng}_${date}_${language}`
+
+    const revalidate = (acceptOnlyForecast) => {
+      fetchWeather(lat, lng, date, language)
+        .then(data => {
+          if (cancelled || data.error || data.temp === undefined) return
+          if (acceptOnlyForecast && data.type !== 'forecast') return
+          setWeatherCache(cacheKey, { data, fetchedAt: Date.now() })
          setWeather(data)
-        }
-      })
-      .catch(() => { setFailed(true) })
-      .finally(() => setLoading(false))
-  }, [lat, lng, date])
+        })
+        .catch(() => {})
+    }
+
+    const cached = getWeatherCache(cacheKey)
+    if (cached !== undefined && cached !== null) {
+      // Entries written before the TTL landed are the bare result — treat as stale.
+      const entry = cached.data !== undefined && typeof cached.fetchedAt === 'number'
+        ? cached
+        : { data: cached, fetchedAt: 0 }
+      setWeather(entry.data)
+      if (entry.data?.type === 'climate') {
+        // Climate data: shown from cache, but re-fetched to upgrade to a forecast.
+        revalidate(true)
+      } else if (Date.now() - entry.fetchedAt > WEATHER_TTL_MS) {
+        // Stale forecast: serve it, then refresh in the background.
+        revalidate(false)
+      }
+    } else if (cached === null) {
+      setFailed(true)
+    } else {
+      setLoading(true)
+      fetchWeather(lat, lng, date, language)
+        .then(data => {
+          if (cancelled) return
+          if (data.error || data.temp === undefined) {
+            setFailed(true)
+          } else {
+            setWeatherCache(cacheKey, { data, fetchedAt: Date.now() })
+            setWeather(data)
+          }
+        })
+        .catch(() => { if (!cancelled) setFailed(true) })
+        .finally(() => { if (!cancelled) setLoading(false) })
+    }
+    return () => { cancelled = true }
+  }, [lat, lng, date, language])

  if (!lat || !lng) return null

@@ -113,9 +135,13 @@ export default function WeatherWidget({ lat, lng, date, compact = false, stacked
  const unit = isFahrenheit ? '°F' : '°C'
  const isClimate = weather.type === 'climate'

+  // The forecast's anchor place, as a tooltip on every variant — no layout change,
+  // but the widget finally says WHERE the weather is for (#2167).
+  const title = locationName || undefined
+
  if (stacked) {
    return (
-      <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 1, fontSize: 'calc(9.5px * var(--fs-scale-caption, 1))', fontWeight: 600, lineHeight: 1, color: 'inherit', ...fontStyle }}>
+      <div title={title} style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 1, fontSize: 'calc(9.5px * var(--fs-scale-caption, 1))', fontWeight: 600, lineHeight: 1, color: 'inherit', ...fontStyle }}>
        <WeatherIcon main={weather.main} size={13} />
        {temp !== null && <span>{isClimate ? 'Ø' : ''}{temp}°</span>}
      </div>
@@ -124,7 +150,7 @@ export default function WeatherWidget({ lat, lng, date, compact = false, stacked

  if (compact) {
    return (
-      <span style={{ display: 'inline-flex', alignItems: 'center', gap: 3, fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: isClimate ? '#a1a1aa' : '#6b7280', ...fontStyle }}>
+      <span title={title} style={{ display: 'inline-flex', alignItems: 'center', gap: 3, fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: isClimate ? '#a1a1aa' : '#6b7280', ...fontStyle }}>
        <WeatherIcon main={weather.main} size={12} />
        {temp !== null && <span>{isClimate ? 'Ø ' : ''}{temp}{unit}</span>}
      </span>
@@ -132,7 +158,7 @@ export default function WeatherWidget({ lat, lng, date, compact = false, stacked
  }

  return (
-    <div style={{ display: 'flex', alignItems: 'center', gap: 6, fontSize: 'calc(13px * var(--fs-scale-body, 1))', color: isClimate ? '#71717a' : '#374151', background: 'rgba(0,0,0,0.04)', borderRadius: 8, padding: '5px 10px', ...fontStyle }}>
+    <div title={title} style={{ display: 'flex', alignItems: 'center', gap: 6, fontSize: 'calc(13px * var(--fs-scale-body, 1))', color: isClimate ? '#71717a' : '#374151', background: 'rgba(0,0,0,0.04)', borderRadius: 8, padding: '5px 10px', ...fontStyle }}>
      <WeatherIcon main={weather.main} size={15} />
      {temp !== null && <span style={{ fontWeight: 500 }}>{isClimate ? 'Ø ' : ''}{temp}{unit}</span>}
      {weather.description && <span style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: '#9ca3af', textTransform: 'capitalize' }}>{weather.description}</span>}