fix(calendar): pass timeZone to events.list to fix all-day event date shift#661
fix(calendar): pass timeZone to events.list to fix all-day event date shift#661jorj-pineda wants to merge 4 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 an issue where all-day calendar events in the Highlights
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. Footnotes
|
🦋 Changeset detectedLatest commit: 6091bd2 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 |
![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 addresses an issue where all-day calendar events displayed incorrect dates in the agenda by passing the user's timezone to the Google Calendar API and introducing a helper function to extract event times. Review feedback highlights that the logic for detecting all-day events is inconsistent with the goal of prioritizing the 'date' field when both 'date' and 'dateTime' are present. Furthermore, a regression test was found to be inadequate as it does not include both fields to properly verify the intended prioritization logic.
| fn extract_event_times(event: &Value) -> (String, String, bool) { | ||
| let start_obj = event.get("start"); | ||
| let end_obj = event.get("end"); | ||
|
|
||
| // All-day events carry a `date` field with no `dateTime`. | ||
| let all_day = start_obj | ||
| .map(|s| s.get("date").is_some() && s.get("dateTime").is_none()) | ||
| .unwrap_or(false); | ||
|
|
||
| let start = start_obj | ||
| .and_then(|s| { | ||
| if all_day { | ||
| s.get("date") | ||
| } else { | ||
| s.get("dateTime").or_else(|| s.get("date")) | ||
| } | ||
| }) | ||
| .and_then(|v| v.as_str()) | ||
| .unwrap_or("") | ||
| .to_string(); | ||
|
|
||
| let end = end_obj | ||
| .and_then(|s| { | ||
| if all_day { | ||
| s.get("date") | ||
| } else { | ||
| s.get("dateTime").or_else(|| s.get("date")) | ||
| } | ||
| }) | ||
| .and_then(|v| v.as_str()) | ||
| .unwrap_or("") | ||
| .to_string(); | ||
|
|
||
| (start, end, all_day) | ||
| } |
There was a problem hiding this comment.
The logic in extract_event_times for detecting all-day events and extracting their values is redundant and inconsistent with the stated goal of preferring the date field when both date and dateTime are present.
Currently, all_day is only true if dateTime is absent. If both are present, all_day becomes false, and the extraction logic then prefers dateTime. This contradicts the comment on line 212 and the intent of the regression test on line 845. Additionally, the extraction of the end time is fragile because it relies on the all_day flag derived solely from the start object.
A simpler and more robust approach is to prefer the date field if it exists, which correctly identifies all-day events and avoids timezone shifts as intended.
fn extract_event_times(event: &Value) -> (String, String, bool) {
let start_obj = event.get("start");
let end_obj = event.get("end");
// Prefer 'date' (all-day) over 'dateTime' (timed) to avoid timezone shifts.
let start_date = start_obj.and_then(|s| s.get("date")).and_then(|v| v.as_str());
let start_time = start_obj.and_then(|s| s.get("dateTime")).and_then(|v| v.as_str());
let all_day = start_date.is_some();
let start = start_date.or(start_time).unwrap_or("").to_string();
let end = end_obj
.and_then(|s| s.get("date").or_else(|| s.get("dateTime")))
.and_then(|v| v.as_str())
.unwrap_or("")
.to_string();
(start, end, all_day)
}| fn extract_event_times_all_day_prefers_date_over_datetime() { | ||
| // Regression: if both `date` and `dateTime` exist on an all-day | ||
| // event, the bare date must win so no timezone shift occurs. | ||
| let event = json!({ | ||
| "start": { "date": "2026-03-23" }, | ||
| "end": { "date": "2026-03-24" }, | ||
| "summary": "Mixed" | ||
| }); | ||
| let (start, _end, all_day) = extract_event_times(&event); | ||
| assert!(all_day); | ||
| assert_eq!(start, "2026-03-23", "bare date must not be shifted"); | ||
| } |
There was a problem hiding this comment.
This regression test does not actually test the scenario where both date and dateTime are present, as the mock JSON only contains the date field. To properly verify that the bare date is preferred over dateTime, the test case should include both fields in the input.
#[test]
fn extract_event_times_all_day_prefers_date_over_datetime() {
// Regression: if both 'date' and 'dateTime' exist on an all-day
// event, the bare date must win so no timezone shift occurs.
let event = json!({
"start": { "date": "2026-03-23", "dateTime": "2026-03-23T10:00:00Z" },
"end": { "date": "2026-03-24", "dateTime": "2026-03-24T10:00:00Z" },
"summary": "Mixed"
});
let (start, end, all_day) = extract_event_times(&event);
assert!(all_day);
assert_eq!(start, "2026-03-23", "bare date must not be shifted");
assert_eq!(end, "2026-03-24");
}
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where all-day calendar events displayed incorrect dates in the +agenda command. It introduces a helper function extract_event_times that correctly prioritizes the date field for all-day events to prevent timezone offsets. Additionally, the user's timezone is now passed as a query parameter to the Calendar API. Unit tests were added to verify the new logic. I have no feedback to provide.
2 participants
Description
Fixes #579
All-day events in
gws calendar +agendawere displaying incorrect dates,shifted backwards by up to a day for users in non-UTC timezones (e.g.
Asia/Seoul, UTC+9).
Root cause: The
events.listAPI call was not passing atimeZoneparameter, so the Google Calendar API interpreted all-day event boundaries
using the calendar's default timezone rather than the user's resolved
account timezone.
Changes:
events.listAPI call viathe
timeZonequery parameterextract_event_times()helper that properly handlesdate(all-day) vs
dateTime(timed) fields — all-day events use the baredate string to avoid any timezone shifting
allDayfield to each event in the outputregression case
Checklist
AGENTS.mdguidelines (no generatedgoogle-*crates)cargo fmt --allto format the codecargo clippy -- -D warningsand resolved all warnings