fix(calendar): respect configured timezone - #2427
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Visual recap — skippedThe visual recap job did not run for this pull request. This is informational only and does not block the PR. Recap skipped for |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🔴
Review Details
Code Review Summary
This incremental review covers the timezone propagation added across the calendar actions, server range resolution, calendar views, event cards, and drag interactions. The overall direction is sound: saved IANA timezone validation is centralized, date-only server ranges account for DST, and event instants are projected into the configured timezone for rendering. The focused tests reported by the agents pass, but the new coverage is primarily action-level and does not exercise browser-local timezone differences or DST interaction behavior.
Risk assessment: Standard (business/UI date-range and interaction logic).
Key findings
- 🔴 HIGH — Calendar boundary calculations still operate on
selectedDatein the browser timezone while event instants are projected into the configured timezone, so non-browser timezone users can get wrong ranges and positions. - 🔴 HIGH — Dragging/resizing through a spring-forward gap can independently convert a nonexistent local end time and persist a zero or incorrect duration.
- 🟡 MEDIUM — Adjacent-range prefetching still uses unzoned ranges and therefore does not warm the same query keys as the primary range.
The centralized settings helper and server-side validation/defaulting are good improvements, and the range tests cover a date-only request plus a DST boundary. The prefetch issue is included in the summary because its unchanged lines are not commentable in this diff.
🧪 Browser testing: Will run after this review (PR touches UI code)
| return { | ||
| from: startOfWeek(ms).toISOString(), | ||
| to: endOfWeek(me).toISOString(), | ||
| from: fromZonedTime(startOfWeek(ms), calendarTimezone).toISOString(), |
There was a problem hiding this comment.
🔴 Compute calendar boundaries in the configured timezone
startOfMonth/endOfMonth are applied directly to selectedDate, which is initialized and navigated as a browser-local Date, before the result is passed to fromZonedTime. When the browser and configured calendar zones differ, the selected calendar day/month boundary can be wrong. Convert the selected instant with toZonedTime(selectedDate, calendarTimezone) before deriving month/week/day boundaries, and keep the same representation for the child views.
| date: Date, | ||
| timezone: string, | ||
| ) { | ||
| const start = toZonedTime(event.start, timezone); |
There was a problem hiding this comment.
🔴 Use one timezone context for day event positioning
The event start/end are projected with toZonedTime, but dayStart/dayEnd are derived directly from date. With a browser timezone different from the configured timezone, differenceInMinutes compares incompatible wall-clock representations and places events at the wrong vertical time. Project date into timezone before calculating the day boundaries.
| onEventTimeChange(state.eventId, newStart, newEnd); | ||
| onEventTimeChange( | ||
| state.eventId, | ||
| fromZonedTime(newStart, timezone), |
There was a problem hiding this comment.
🔴 Preserve elapsed duration across DST gaps
The drag path performs wall-clock addMinutes arithmetic on a zoned pseudo-Date, then converts the new start and end independently. If the end lands in a spring-forward gap, fromZonedTime can collapse the nonexistent local time onto the transition and persist a zero or incorrect duration. Calculate the new instants from the original elapsed duration (or explicitly resolve DST gaps) and add a spring-forward drag test.
|
thanks @mpiniarski! some CI errors and some PR feedback here worth taking a look at |
There was a problem hiding this comment.
Builder reviewed your changes and found 5 potential issues 🔴
Review Details
Code Review Summary
This incremental review evaluated the latest PR head while excluding the three unresolved findings from the prior review. The timezone propagation remains directionally sound, and the centralized IANA validation/defaulting plus DST-aware server range tests are useful improvements. However, the latest implementation still has several newly identified edge cases in inventory pagination, month drag/drop, legacy settings compatibility, and timezone-projected rendering.
Risk assessment: Standard (business logic and UI date/time behavior).
Key findings
- 🔴 HIGH — Command palette can call
toZonedTimewith its optional timezone prop unset. - 🟡 MEDIUM — Inventory cursor preflight reconstructs a different range than the first page when saved and request timezones differ.
- 🟡 MEDIUM — Month drag/drop does not preserve displayed wall-clock time across a DST transition.
- 🟡 MEDIUM — Existing invalid saved timezone values now make event listing fail without a recovery path.
- 🟡 MEDIUM — Week view compares a timezone-shifted display proxy with the real current instant for past-event styling.
The previous comments remain open and were intentionally not reposted or resolved because their underlying code is still present.
🧪 Browser testing: Will run after this review (PR touches UI code)
| ): Promise<CalendarEventsResult> { | ||
| const email = getRequestUserEmail(); | ||
| if (!email) throw new Error("no authenticated user"); | ||
| const timezone = await getCalendarTimezone(email); |
There was a problem hiding this comment.
🟡 Use the saved timezone when validating inventory cursors
The first inventory page resolves its range with the saved calendar timezone, but the cursor preflight later calls resolveCalendarEventRange without timezone. For date-only or omitted bounds, page two can compute a different query key using the request timezone and reject the cursor as query-bound. Resolve and pass the same saved timezone in the cursor path.
| const evStart = e.allDay | ||
| ? parseISO(e.start) | ||
| : toZonedTime(e.start, timezone); |
There was a problem hiding this comment.
🟡 Preserve local time when moving events across DST
Timed events are now grouped by their configured-calendar wall clock, but CalendarView.handleEventDrop still changes the dates of the original UTC instants with browser-local setFullYear. Moving an event across a DST boundary therefore changes its displayed local time (for example 10:00 becomes 11:00 after the offset changes). Convert the event times to the calendar timezone before replacing the date and convert them back with fromZonedTime.
| timezone?: unknown; | ||
| } | null; | ||
| if (settings?.timezone === undefined) return getDefaultSettings().timezone; | ||
| if (!isCalendarTimezone(settings.timezone)) { |
There was a problem hiding this comment.
🟡 Provide recovery for legacy invalid timezone settings
Reads now strictly validate the saved timezone, but older settings could contain arbitrary strings because the previous update path accepted them. getCalendarTimezone throws for those accounts, so list-events fails before any events can load, while get-settings returns the invalid record and offers no repair path. Fall back to a valid timezone or expose a recoverable settings update path instead of making the calendar unusable.
| : null; | ||
| const start = parseISO(event.start); | ||
| const end = parseISO(event.end); | ||
| const start = toZonedTime(event.start, timezone); |
There was a problem hiding this comment.
🟡 Compare past status using the real event instant
end is now a toZonedTime wall-clock proxy used for grid geometry, but it is also compared directly with now at the past-status check. Those values are not on the same instant timeline, so an event can be marked past while it is still active (or vice versa) when the configured zone differs from the browser zone. Keep a separately parsed real end instant for this comparison.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Code Review Summary
This incremental review evaluated the latest PR head and skipped all seven unresolved findings from the prior review because they remain present and unchanged; none were resolved. The latest changes continue to propagate the configured timezone through calendar rendering and settings while adding focused action-level coverage. The dev server is healthy, but browser execution remains unavailable in this environment.
Risk assessment: Standard (calendar business logic and UI interaction behavior).
New findings
- 🔴 HIGH —
view-screenparses a bareYYYY-MM-DDnavigation date as a server-local date before applying the configured timezone, so agent screen context can report the wrong week when the server timezone differs. - 🔴 HIGH — The drag callbacks use
timezonebut omit it from their dependency arrays, leaving stale closures after a live timezone setting change.
The existing settings validation/defaulting and date-range tests remain useful, but browser-level verification is needed for timezone changes and drag interactions.
🧪 Browser testing: Attempted after this review; all 17 planned flows were blocked because Chrome/browser automation tools were unavailable in the executor environment.
| if (!email) throw new Error("no authenticated user"); | ||
| const timezone = await getCalendarTimezone(email); | ||
| const viewDate = nav?.date | ||
| ? parseISO(nav.date) |
There was a problem hiding this comment.
🔴 Parse navigation dates in the configured timezone
navigate documents date as a bare YYYY-MM-DD, but parseISO(nav.date) creates a server-local date before startOfWeek and fromZonedTime are applied. When the server timezone differs from the saved calendar timezone, view-screen can compute a shifted week and return context for the wrong dates; parse the date-only value as a wall-clock date in timezone instead.
| onEventTimeChange(state.eventId, newStart, newEnd); | ||
| onEventTimeChange( | ||
| state.eventId, | ||
| fromZonedTime(newStart, timezone), |
There was a problem hiding this comment.
🔴 Refresh drag callbacks when the timezone changes
The newly timezone-aware startDrag and onPointerUp callbacks use timezone, but their dependency arrays omit it. After a live settings change, a drag can retain the previous timezone closure and save the event using the old zone; include timezone in both callback dependencies.
2 participants
Summary
Gmail does use configured timezone instead of browser timezone as well.
Bug
When the configured timezone differed from the browser timezone, the week grid used browser-local wall-clock values while event creation used the configured timezone. For example, clicking 7:00 AM in a New York calendar from Central Europe created an event displayed at 1:00 PM.
Testing
pnpm --dir templates/calendar test -- actions/list-events.spec.ts actions/update-settings.spec.tspnpm --dir templates/calendar typecheck