fix(caldav): use toJSDate instead of toString to prevent timezone double-conversion

[pmartin-dev]

What does this PR do?

The getEvents() method in CalendarService.ts double-converts timezone offsets
when parsing CalDAV events that include a VTIMEZONE component.
event.startDate.toString() returns a UTC time without the Z suffix, then
dayjs.tz(string, timezone) re-interprets it as local time — applying the offset
twice. This causes events synced from Zimbra (and potentially other CalDAV servers)
to display end times before start times (negative duration).

The fix aligns getEvents() with the existing correct pattern in
getAvailability() (same file):

• Register the VTIMEZONE with ical.js via convertToZone()

• Use toJSDate() instead of toString() + dayjs.tz() to avoid
  double-conversion

• Fixes Events synced via CalDAV from Zimbra have end time before start time #27877

• Fixes CAL-7198

Visual Demo (For contributors especially)

N/A — this is a backend data parsing fix with no UI changes. The effect is that
CalDAV events from Zimbra now display correct start/end times instead of negative
durations.

Mandatory Tasks (DO NOT REMOVE)

• I have self-reviewed the code (A decent size PR without self-review might be rejected).
• I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
• I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

1. Run the unit tests: TZ=UTC yarn vitest run packages/lib/CalendarService.test.ts
2. All 29 tests should pass, including 4 new tests in the "CalDAV event time
   parsing" describe block:
   • "buggy code: UTC times with VTIMEZONE produce wrong timestamps" — proves
     the old code produces incorrect absolute timestamps
   • "fixed code: UTC times with VTIMEZONE produce correct 15min duration" —
     proves the fix resolves the Zimbra UTC case
   • "fixed code: TZID-referenced times produce correct 15min duration" —
     proves non-UTC timezone references still work
   • "fixed code: events without VTIMEZONE parse correctly" — proves no
     regression on the simple case
3. To reproduce the original bug manually: create a CalDAV event on a Zimbra server
   with UTC times (DTSTART/DTEND with Z suffix) where the calendar also includes a
   VTIMEZONE component, then sync via CalDAV and check that end time is after start
   time.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have checked that my changes generate no new warnings

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes update CalDAV event timestamp parsing in the calendar service to improve timezone handling accuracy. The production code in BaseCalendarService.getEvents() replaces the previous dayjs.tz() conversion approach with ICAL.Timezone registration and convertToZone() method calls, ensuring proper UTC conversion when a VTIMEZONE component exists. The test file introduces comprehensive test coverage for multiple timezone scenarios: UTC events with VTIMEZONE, TZID-referenced local times, and events without timezone information, with assertions validating absolute timestamps and event duration.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main code change: switching from toString() to toJSDate() to prevent double timezone conversion in CalDAV parsing.
Description check	✅ Passed	The description clearly explains the timezone double-conversion bug in CalDAV parsing and how the fix aligns with the correct pattern in getAvailability().
Linked Issues check	✅ Passed	The changes address issue #27877 by fixing CalDAV event parsing to prevent timezone double-conversion, ensuring CalDAV-synced events from Zimbra maintain correct start/end times and positive durations.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the timezone double-conversion bug in CalDAV parsing: modifications to CalendarService.ts production code and comprehensive test coverage in CalendarService.test.ts.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/lib/CalendarService.test.ts (1)

768-780: Minor: Clarify the comment refers to the old buggy code.

The comment on line 769 says "Current code in getEvents()" but after this PR merges, this is no longer current—it's the previous/buggy implementation. Consider updating for clarity:

    if (strategy === "buggy") {
-      // Current code in getEvents() — double conversion
+      // OLD code in getEvents() — double conversion bug

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/lib/CalendarService.test.ts` around lines 768 - 780, The inline
comment "Current code in getEvents()" is misleading because this block under
strategy === "buggy" is the old/previous buggy implementation; update the
comment to clearly indicate it is the previous/buggy implementation (e.g.,
"Previous/buggy implementation of getEvents — double conversion") so reviewers
understand this branch is intentionally kept for comparison; ensure the comment
references getEvents and the strategy === "buggy" branch to make the intent
explicit.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/lib/CalendarService.test.ts`:
- Around line 768-780: The inline comment "Current code in getEvents()" is
misleading because this block under strategy === "buggy" is the old/previous
buggy implementation; update the comment to clearly indicate it is the
previous/buggy implementation (e.g., "Previous/buggy implementation of getEvents
— double conversion") so reviewers understand this branch is intentionally kept
for comparison; ensure the comment references getEvents and the strategy ===
"buggy" branch to make the intent explicit.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f33b1025-7bf8-472c-87ee-47575e8b2f38

📥 Commits

Reviewing files that changed from the base of the PR and between f3e07c5 and 1270128.

📒 Files selected for processing (2)

• packages/lib/CalendarService.test.ts
• packages/lib/CalendarService.ts