fix(calendar): use local timezone for agenda day boundaries#443
fix(calendar): use local timezone for agenda day boundaries#443anshul-garg27 wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
Previously, --today and --tomorrow computed day boundaries using UTC epoch arithmetic, so after local midnight \!= UTC midnight the wrong day's events were returned. Now uses chrono::Local to derive midnight in the user's timezone. Also fixes --today which had no explicit branch and fell through to the generic "N days from now" path.
🦋 Changeset detectedLatest commit: 2ab00d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the calendar agenda feature where day boundaries were incorrectly calculated using UTC, leading to users seeing events for the wrong day in non-UTC timezones. The changes refactor the time range determination logic to leverage the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly changes the calendar agenda logic to use the local timezone instead of UTC, fixing a bug for users in non-UTC timezones. The implementation is mostly correct, but I've found a critical issue related to Daylight Saving Time transitions that could lead to incorrect agenda boundaries. The new test also contains duplicated logic with the same issue. My review includes suggestions to make the timezone handling more robust.
Replace .single().unwrap_or(local_now) with .earliest().unwrap_or(local_now) to correctly handle DST transitions where midnight may be ambiguous or non-existent. Applied in both production code and test.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue with calendar agenda boundaries by switching from UTC-based epoch arithmetic to timezone-aware calculations using chrono::Local. This ensures that commands like --today and --tomorrow respect the user's local timezone. The changes are logical, and the addition of a new test case to verify local timezone offsets is a good improvement. I have one high-severity comment regarding input validation for the number of days, which could lead to incorrect behavior if a negative value is provided.
Summary
gws calendar +agenda --todayand--tomorrownow use the local timezone for day boundaries instead of UTCRoot cause
Day boundaries were computed by dividing Unix epoch seconds by 86400, which yields UTC midnight. The RFC3339 timestamps were also formatted in UTC. Users in non-UTC timezones got the wrong day's events.
Before (PST, 8pm local = UTC midnight):
--todayreturns tomorrow's events--tomorrowreturns the day after tomorrow's eventsAfter:
--todayreturns today's events using local midnight boundaries--tomorrowreturns tomorrow's events using local midnight boundariesChanges
chrono::Localday boundary calculation--todaybranch (previously fell through to generic "N days from now")epoch_to_rfc3339helper (now usingchrono::DateTime::to_rfc3339()directly)-07:00)Test plan
Closes #259