fix(web,admin,db): keep the last forecast on ended events, not a false weather error - #1435
Conversation
…ended events Every event whose day had passed showed a "Weather unavailable" chip that pointed admins to External services, although the weather provider was fine. Providers only serve today and the days ahead, so an ended day can never have a forecast: days more than one day back were short-circuited to "unavailable", and yesterday was sent to the provider, always came back empty and logged a false weather_fetch_failed. WeatherService now returns a new "past" status once the event day is over in the event's timezone, before any provider call or cache read. The admin event card renders no chip for it (neither the error chip nor the "No weather" fallback). The public ticket page is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e event Weather providers cannot look back once an event's day is over, so an ended event's card had nothing left to show. Admitto now saves the latest forecast it sees for an event's day and shows it afterwards as "Last forecast", with the credit of the provider that gave it. - db: nullable Event.weather_snapshot (JSONB) and its migration. The value carries its own calendar day and pin, so an event moved to another day or place never shows a stale forecast. - web: the events list, a single event and the check-in event list save the forecast they just computed while the day is still ahead (a newer whole-day forecast replaces the stored one, the event day itself only fills an empty snapshot because MET Norway returns just the hours that are left, unchanged values are not rewritten). Ended events read it back through the new "past" summary. A failed read or write is logged and never fails the list. The public ticket page neither reads nor writes it. - admin: the event card labels it "Last forecast" and shows it in the operator's temperature unit; without a saved forecast the chip stays hidden. Events that ended before this, or whose day nobody had a list open for while it was ahead, have no saved forecast. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
solarssk
added
type: bug
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: solarssk/admitto/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughEvent weather summaries now retain eligible forecasts in a nullable event snapshot. After an event day ends, summaries use a matching saved forecast when available, and admin event cards label it “Last forecast.” Events without a saved forecast report past status without an error. ChangesEvent weather snapshots
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Sequence Diagram(s)sequenceDiagram
participant EventListRoute
participant summarizeEventsWeather
participant Database
participant WeatherService
participant WeatherProvider
EventListRoute->>summarizeEventsWeather: event rows and weather service
summarizeEventsWeather->>Database: load weather_snapshot values
summarizeEventsWeather->>WeatherService: summarize events with saved snapshots
WeatherService->>WeatherProvider: request forecasts for eligible event days
WeatherService-->>summarizeEventsWeather: weather summaries
summarizeEventsWeather->>Database: save eligible forecast snapshots
summarizeEventsWeather-->>EventListRoute: weather summaries
Merge Risk: ⚪ Minimal · up to The saved-forecast behavior is consistently available in the intended staff event views, with public tickets intentionally unchanged. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new saved-forecast workflow can permanently replace an event-day forecast after a failed read or an overlapping request. No cross-organisation access or public write path was established, but the access boundary could not be fully verified. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 14 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
… overlapping requests Review of the snapshot change found two ways a saved forecast could be lost. - When the saved forecasts could not be read, the writer treated every event as having an empty snapshot. On the event day that let a reading, which MET Norway may only give for the hours that are left, overwrite the full-day forecast. Nothing is saved now when the read failed. - The write was a blind update by event id, but a provider call sits between reading the saved value and writing it. A request that saved in the meantime (a full-day forecast just before midnight, say) could be overwritten by an older or partial one. Each write is now conditional on the stored value still being the one that was read; a write that lost that race is dropped without an error. Also drops an unreachable "?? null" (the decision function already accepts a missing summary), uses an optional chain in snapshotForEvent, and documents the touched helpers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
1 participant
Description
Business context. The Events list showed a "Weather unavailable" chip on every event whose day had already passed, with a tooltip telling admins to check the weather provider under External services, although the provider worked fine. Weather providers only serve today and the days ahead, so an ended event can never get a forecast, and the message sent admins to fix something that was not broken. It also meant an ended event lost its weather entirely. Now the card keeps the last forecast Admitto saw for the event's day, labelled "Last forecast", and shows nothing (never an error) when no forecast was ever saved.
Technical changes.
WeatherService.summarizereturns a newpaststatus once the event day is over in the event's timezone, before any provider call or cache read. Before, a day two or more days back was short-circuited tounavailable, and yesterday reached the provider, always came back empty and logged a falseweather_fetch_failed.pastcarries the saved forecast (temperatures, code, and the credit of the provider that gave it) when one still fits the event's day and pin. Newweather-snapshot.tsholds the validation, the day and pin matching and the save rule. Newevent-weather.ts(summarizeEventsWeather) loads the saved forecasts with one query and saves the forecast just computed while the day is still ahead; a failed read or write is logged (weather_snapshot_*) and never fails the list. Each write is conditional on the stored value still being the one that was read, because a provider call sits between the read and the write: a request that saved in the meantime (a full-day forecast just before midnight, say) is never overwritten by an older or partial reading, and a write that lost that race is dropped without an error. Nothing is saved at all when the saved forecasts could not be read. It is used by the admin events list, a single event and the check-in events list. The public ticket page neither reads nor writes it, so visitors cannot cause writes.Event.weather_snapshot(JSONB) and migration20260925190000_add_event_weather_snapshot. It is additive; applied from zero on a scratch database with no schema drift (prisma migrate diff). The stored value carries its own calendar day and rounded pin, so an event moved to another day or place never shows a stale forecast.EventCardshows the saved forecast as "Last forecast" in the operator's °C or °F, credited to the provider that gave it, and no chip for an ended event without one.How to test
Ran locally on this branch, all green:
npm run build -w @admitto/web,npm run build -w @admitto/admin,npm run lint,npm run lint:baseline:check,npm run docs:checkbash scripts/check-migrations-destructive.shand its self-testnpm test -w @admitto/<name>for all 18 workspaces: 774 test files, 11248 tests, 0 failures; after the last commit web and admin were re-run in full (new:weather-snapshot,event-weather, an integration test on a real database for save, replace by a newer forecast, show-after-the-event, single event, check-in list, moved event, a write that lost a race to another request, and the raw column never reaching the browser, plus service and EventCard cases)Manual:
What stays / known limitations
Documentation impact
Checklist
@example.comaddresses)npm test; optional:npm run coverageto match CI)🤖 Generated with Claude Code