fix(calendar): use local day boundaries for agenda date filters#369
fix(calendar): use local day boundaries for agenda date filters#369sonwr wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
|
googleworkspace-bot
added
the
area: core
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 date filtering, where previous implementations led to incorrect results in non-UTC timezones. By refactoring the date range computation to use local day boundaries and the Highlights
Changelog
Activity
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly refactors the agenda date filtering logic to respect local timezones, fixing a bug where events could be shown on the wrong day. The new implementation is moved into separate, testable functions, and new unit tests have been added to verify the behavior with different timezone offsets. The changes are a significant improvement.
My review includes one suggestion to handle an edge case where a user might provide --days 0, which would currently lead to an API error. Adding a validation check for this would improve the user experience.
| .get_one::<String>("days") | ||
| .and_then(|s| s.parse::<u64>().ok()) | ||
| { | ||
| return local_date_range(&now.timezone(), local_date, days); |
There was a problem hiding this comment.
The code doesn't handle the case where a user provides --days 0. This will result in time_min and time_max being identical, which is invalid for the Google Calendar API (it requires timeMin to be smaller than timeMax). This will lead to an API error. It would be better to validate this input and return a user-friendly error.
A check should be added to ensure the number of days is positive.
if days == 0 {
return Err(GwsError::Validation("Number of days for agenda must be a positive integer.".to_string()));
}
return local_date_range(&now.timezone(), local_date, days);2 participants
This fixes
calendar +agendadate filters so they respect local day boundaries instead of using UTC/current-time windows.Fixes #259
What changed:
--todaynow queries from local midnight to the next local midnight--tomorrownow queries the next local calendar day--weeknow starts at local midnight today--days Nnow starts at local midnight today and spansNlocal calendar days+agendabehavior without date flags remains unchanged (nowtonow + 24h)Why:
The previous implementation used:
now..now+86400for--today/--days--tomorrowThat produced incorrect results in non-UTC timezones, especially later in the day, where
--todaycould include tomorrow’s events.Tests added:
test_compute_agenda_range_today_uses_local_midnight_boundariestest_compute_agenda_range_tomorrow_uses_next_local_daytest_compute_agenda_range_days_starts_at_local_midnight