-
Notifications
You must be signed in to change notification settings - Fork 0
Normalize legacy weather cache labels #348
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| /** | ||
| * @jest-environment jsdom | ||
| */ | ||
|
|
||
| import { render, screen } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
|
|
||
| import WeatherSummary from '@/app/components/Weather/WeatherSummary'; | ||
| import type { WeatherSummary as WeatherSummaryType } from '@/app/types/weather'; | ||
|
|
||
| const buildLegacySummary = (dates: string[]): WeatherSummaryType => ({ | ||
| locationId: 'location-1', | ||
| startDate: dates[0], | ||
| endDate: dates[dates.length - 1], | ||
| dailyWeather: dates.map(date => ({ | ||
| id: `weather-${date}`, | ||
| date, | ||
| coordinates: [47.5, 19] as [number, number], | ||
| temperature: { min: 10, max: 20, average: 15 }, | ||
| precipitation: { total: 0, probability: null }, | ||
| wind: { speed: null, direction: null }, | ||
| conditions: { description: 'Clear', icon: 'sun', code: 0, cloudCover: null, humidity: null }, | ||
| fetchedAt: '2026-06-01T00:00:00.000Z', | ||
| })) as WeatherSummaryType['dailyWeather'], | ||
| summary: { | ||
| averageTemp: 15, | ||
| totalPrecipitation: 0, | ||
| predominantCondition: 'Clear', | ||
| }, | ||
| }); | ||
|
|
||
| describe('WeatherSummary', () => { | ||
| afterEach(() => { | ||
| jest.useRealTimers(); | ||
| }); | ||
|
|
||
| it('labels all-future legacy cached weather as historical averages', () => { | ||
| jest.useFakeTimers(); | ||
| jest.setSystemTime(new Date('2026-06-01T12:00:00.000Z')); | ||
|
|
||
| render(<WeatherSummary summary={buildLegacySummary(['2027-06-01', '2027-06-02'])} />); | ||
|
|
||
| expect(screen.getByText('Weather · Hist. avg.')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('labels past legacy cached weather as recorded instead of rendering a blank source label', () => { | ||
| jest.useFakeTimers(); | ||
| jest.setSystemTime(new Date('2026-06-01T12:00:00.000Z')); | ||
|
|
||
| render(<WeatherSummary summary={buildLegacySummary(['2026-05-31', '2026-06-01'])} />); | ||
|
|
||
| expect(screen.getByText('Weather · Recorded')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
|
Comment on lines
+1
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -282,7 +282,7 @@ async function readCache(key: string): Promise<CacheEntry | null> { | |||||
| const exp = parseISO(d.expiresAt); | ||||||
| return isValid(exp) && isAfter(exp, today); | ||||||
| }); | ||||||
| const summary: WeatherSummary = { ...parsed.summary, dailyWeather: filtered }; | ||||||
| const summary = normalizeCachedWeatherSummary({ ...parsed.summary, dailyWeather: filtered }); | ||||||
| const fetchedAt = typeof parsed.fetchedAt === 'string' ? parsed.fetchedAt : LEGACY_FETCHED_AT; | ||||||
| const entry: CacheEntry = { | ||||||
| key: parsed.key || key, | ||||||
|
|
@@ -346,6 +346,48 @@ function computeSources(daily: WeatherData[]): CacheSources { | |||||
| return result; | ||||||
| } | ||||||
|
|
||||||
| function isKnownWeatherDataSource(value: unknown): value is WeatherData['dataSource'] { | ||||||
| return value === 'open-meteo' || value === 'cache' || value === 'historical-average'; | ||||||
| } | ||||||
|
|
||||||
| function normalizeCachedWeatherSummary(summary: WeatherSummary, today: Date = new Date()): WeatherSummary { | ||||||
| const todayISO = toISODate(today); | ||||||
| const hasExplicitSourceMetadata = summary.dailyWeather.some(day => { | ||||||
| const rawDay = day as Partial<WeatherData>; | ||||||
| return ( | ||||||
| isKnownWeatherDataSource(rawDay.dataSource) || | ||||||
| typeof rawDay.isForecast === 'boolean' || | ||||||
| typeof rawDay.isHistorical === 'boolean' | ||||||
| ); | ||||||
| }); | ||||||
| const legacyAllFuture = !hasExplicitSourceMetadata && summary.dailyWeather.every(day => day.date > todayISO); | ||||||
|
|
||||||
| return { | ||||||
| ...summary, | ||||||
| dailyWeather: summary.dailyWeather.map(day => { | ||||||
| const rawDay = day as Partial<WeatherData>; | ||||||
| const dataSource = isKnownWeatherDataSource(rawDay.dataSource) | ||||||
| ? rawDay.dataSource | ||||||
| : legacyAllFuture | ||||||
| ? 'historical-average' | ||||||
| : 'open-meteo'; | ||||||
| const isHistorical = typeof rawDay.isHistorical === 'boolean' | ||||||
| ? rawDay.isHistorical | ||||||
| : dataSource === 'historical-average' || day.date <= todayISO; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comparison
Suggested change
Comment on lines
+363
to
+376
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| const isForecast = typeof rawDay.isForecast === 'boolean' | ||||||
| ? rawDay.isForecast | ||||||
| : dataSource === 'open-meteo' && !isHistorical; | ||||||
|
|
||||||
| return { | ||||||
| ...day, | ||||||
| dataSource, | ||||||
| isHistorical, | ||||||
| isForecast, | ||||||
| }; | ||||||
| }), | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| function enumerateDateStrings(startISO: string, endISO: string): string[] { | ||||||
| const start = parseISO(startISO); | ||||||
| const end = parseISO(endISO); | ||||||
|
|
@@ -844,6 +886,7 @@ export const weatherServiceTestUtils = { | |||||
| getRateLimitHeaderDelayMs, | ||||||
| needsForecastRefresh, | ||||||
| isFreshEmptyCacheEntry, | ||||||
| normalizeCachedWeatherSummary, | ||||||
| resetRateLimitStateForTests | ||||||
| }; | ||||||
|
|
||||||
|
|
||||||
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.
The use of
todayISO(defined as UTC at line 14) for comparison withd.date(which follows local-date semantics from the service) can lead to off-by-one errors in labeling depending on the user's timezone and time of day. Additionally, this fallback logic labels today as "Recorded" (sincetoday > todayISOis false), which is inconsistent with the "Forecast" label usually applied to the current day. Consider using a consistent date representation and matching the service's heuristic for today's status.References