Skip to content

fix: preserve booker timezone selection#23806

Closed
Aamod007 wants to merge 4 commits intocalcom:mainfrom
Aamod007:tembo/fix-github-issue-16017
Closed

fix: preserve booker timezone selection#23806
Aamod007 wants to merge 4 commits intocalcom:mainfrom
Aamod007:tembo/fix-github-issue-16017

Conversation

@Aamod007
Copy link

Description

Fix timezone handling for bookings when lockTimeZone is not enabled

Changes

  • Added logic to preserve auto-detected timezone
  • Created getInitialTimezone() function to handle timezone selection
  • Ensures booker's timezone is not overwritten by default

@Aamod007 Aamod007 requested a review from a team as a code owner September 12, 2025 17:40
@vercel
Copy link

vercel bot commented Sep 12, 2025

@tembo-io[bot] is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 12, 2025
@graphite-app graphite-app bot requested a review from a team September 12, 2025 17:40
@CLAassistant
Copy link

CLAassistant commented Sep 12, 2025

CLA assistant check
All committers have signed the CLA.

@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 12, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

  • EventMeta.tsx: Added an internal comment in useEffect clarifying that when lockTimeZone is false, the booker’s auto-detected timezone is preserved rather than defaulting to the event owner’s timezone. No logic changes.
  • timePreferences.ts: Introduced internal helper getInitialTimezone() to read a saved timezone from localStorage and fall back to CURRENT_TIMEZONE. Updated timePreferencesStore initialization to use this helper. No changes to exported/public APIs; behavior remains the same with persistence only on setTimezone.

Possibly related PRs

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix: preserve booker timezone selection" succinctly and accurately captures the primary change—preserving the booker's auto-detected timezone when lockTimeZone is disabled—which matches the modifications described in EventMeta.tsx and timePreferences.ts. It is a single clear sentence and uses a conventional commit prefix that aids history scanning. Therefore it satisfies the criteria for a concise, relevant PR title.
Description Check ✅ Passed The PR description clearly states the intent ("Fix timezone handling...") and enumerates the key changes (preserve auto-detected timezone, create getInitialTimezone, ensure booker's timezone not overwritten), which aligns with the provided file summaries. It is directly related to the changeset and not off-topic, meeting the lenient criteria for this check. Thus the description passes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/features/bookings/Booker/components/EventMeta.tsx (1)

245-248: Fix mismatch: fallback should include schedule.timeZone when lockedTimeZone is absent

When the event is locked but lockedTimeZone is null/empty while schedule.timeZone exists, the effect sets the store to schedule.timeZone, yet the select renders CURRENT_TIMEZONE, causing a visible discrepancy (and it’s disabled, so the user can’t correct it).

Apply:

-                    value={
-                      event.lockTimeZoneToggleOnBookingPage
-                        ? event.lockedTimeZone || CURRENT_TIMEZONE
-                        : timezone
-                    }
+                    value={
+                      event.lockTimeZoneToggleOnBookingPage
+                        ? event.lockedTimeZone || event.schedule?.timeZone || CURRENT_TIMEZONE
+                        : timezone
+                    }
🧹 Nitpick comments (3)
packages/features/bookings/Booker/components/EventMeta.tsx (2)

118-125: Keep BookerStore timezone in sync when enforcing a locked TZ

You set only timePreferences here. If other parts rely on the Booker store’s timezone, they may drift.

Proposed tweak:

   if (event?.lockTimeZoneToggleOnBookingPage) {
     const timezone = event.lockedTimeZone || event.schedule?.timeZone;
     if (timezone) {
       setTimezone(timezone);
+      setBookerStoreTimezone(timezone);
     }
   }

148-153: Guard navigator for SSR safety

Accessing navigator.language can throw during SSR if locale is null.

Apply:

-  const userLocale = locale ?? navigator.language;
+  const userLocale =
+    locale ?? (typeof navigator !== "undefined" ? navigator.language : i18n.language);
packages/features/bookings/lib/timePreferences.ts (1)

16-31: Validate stored timezone; fall back when invalid

If localStorage contains an invalid/old IANA TZ, downstream formatting can break. Validate before using; otherwise use CURRENT_TIMEZONE. Also good to keep SSR-safety via the webstorage wrapper (please confirm).

Apply:

 /**
  * Get the initial timezone for the booker, ensuring it's auto-detected from the browser
  * if not previously set by the user.
  */
-const getInitialTimezone = () => {
-  const savedTimezone = localStorage.getItem(timezoneLocalStorageKey);
-
-  // If user has previously set a timezone preference, use that
-  if (savedTimezone) {
-    return savedTimezone;
-  }
-
-  // Auto-detect timezone from browser - don't save to localStorage yet
-  // Only save when user explicitly changes timezone
-  return CURRENT_TIMEZONE;
-};
+const isValidTimezone = (tz: string) => {
+  try {
+    // Throws if tz is not a valid IANA zone
+    new Intl.DateTimeFormat(undefined, { timeZone: tz });
+    return true;
+  } catch {
+    return false;
+  }
+};
+
+const getInitialTimezone = () => {
+  const savedTimezone = localStorage.getItem(timezoneLocalStorageKey) ?? undefined;
+  if (savedTimezone && isValidTimezone(savedTimezone)) return savedTimezone;
+  // Auto-detect from browser; don't persist until user changes explicitly
+  return CURRENT_TIMEZONE;
+};

Verification ask: confirm @calcom/lib/webstorage is SSR-safe (no direct window.localStorage access on server).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a62c74f and 3223625.

📒 Files selected for processing (2)
  • packages/features/bookings/Booker/components/EventMeta.tsx (1 hunks)
  • packages/features/bookings/lib/timePreferences.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/features/bookings/Booker/components/EventMeta.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/bookings/Booker/components/EventMeta.tsx
  • packages/features/bookings/lib/timePreferences.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/bookings/Booker/components/EventMeta.tsx
  • packages/features/bookings/lib/timePreferences.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/bookings/lib/timePreferences.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Tests / Unit
  • GitHub Check: Type check / check-types
  • GitHub Check: Linters / lint
🔇 Additional comments (2)
packages/features/bookings/Booker/components/EventMeta.tsx (1)

126-128: Comment clarifies intent—good

Clear note prevents future regressions when lock is off.

packages/features/bookings/lib/timePreferences.ts (1)

44-45: Helper extraction LGTM

Switching init to getInitialTimezone() improves readability and centralizes behavior.

Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please mention the exact issue this PR is resolving? We are also going to need a loom video that shows this fixes the bug.

@kart1ka kart1ka marked this pull request as draft September 15, 2025 09:13
@github-actions
Copy link
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Sep 30, 2025
@keithwillcode
Copy link
Contributor

Appreciate the contribution @Aamod007 but this PR's code results in no change. Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working community Created by Linear-GitHub Sync size/S Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants