Advanced date override handling and timezone compatibility improvements#6
Open
ShashankFC wants to merge 1 commit into
Open
Advanced date override handling and timezone compatibility improvements#6ShashankFC wants to merge 1 commit into
ShashankFC wants to merge 1 commit into
Conversation
* fix date override for fixed round robin + time zone in date override * check if slot is within working hours of fixed hosts * add test for date override in different time zone * fix date overrides for not fixed hosts (round robin) * code clean up * fix added test * use the correct timezone of user for date overrides --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances date override handling and timezone compatibility in the scheduling system by ensuring proper timezone conversions between organizers and invitees, and adding working hours validation to availability checks.
Changes:
- Added timezone-aware date override processing that accounts for UTC offset differences between organizer and invitee timezones
- Implemented working hours validation in the availability checking logic
- Added test coverage for cross-timezone date override scenarios
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/trpc/server/routers/viewer/slots.ts | Added date override and working hours validation logic to checkIfIsAvailable, and ensured organizer timezone is passed through availability checks |
| packages/lib/slots.ts | Enhanced date override processing to calculate and apply timezone offset differences between organizer and invitee |
| apps/web/test/lib/getSchedule.test.ts | Added test case verifying timezone consistency for date overrides across different timezones |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| workingHours.find((workingHour) => { | ||
| if (workingHour.days.includes(slotStartTime.day())) { | ||
| const start = slotStartTime.hour() * 60 + slotStartTime.minute(); | ||
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); |
There was a problem hiding this comment.
The 'end' variable calculation duplicates the 'start' calculation. It should use 'slotEndTime' instead of 'slotStartTime' to correctly represent the end of the time slot.
Suggested change
| const end = slotStartTime.hour() * 60 + slotStartTime.minute(); | |
| const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; | ||
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { |
There was a problem hiding this comment.
This comparison uses strict equality (===) on dayjs objects, which compares object references rather than values. Use .isSame() method for proper date comparison.
Suggested change
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | |
| if ( | |
| dayjs(date.start) | |
| .add(utcOffset, "minutes") | |
| .isSame(dayjs(date.end).add(utcOffset, "minutes")) | |
| ) { |
| const slotEndTime = time.add(eventLength, "minutes").utc(); | ||
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists |
There was a problem hiding this comment.
Comment should start with a space after '//' for consistency with code style conventions.
Suggested change
| //check if date override for slot exists | |
| // check if date override for slot exists |
| return true; | ||
| } | ||
|
|
||
| //if no date override for slot exists check if it is within normal work hours |
There was a problem hiding this comment.
Comment should start with a space after '//' for consistency with code style conventions.
Comment on lines
+107
to
+123
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | ||
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
| slotStartTime.format("YYYY MM DD") | ||
| ) { | ||
| dateOverrideExist = true; | ||
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | ||
| return true; | ||
| } | ||
| if ( | ||
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | ||
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | ||
| ) { | ||
| return true; | ||
| } | ||
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { |
There was a problem hiding this comment.
The UTC offset calculation appears inconsistent with the approach in packages/lib/slots.ts (lines 212-214), which computes the difference between invitee and organizer offsets. Consider using a consistent approach across both files for maintainability.
Suggested change
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | |
| if ( | |
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | |
| slotStartTime.format("YYYY MM DD") | |
| ) { | |
| dateOverrideExist = true; | |
| if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) { | |
| return true; | |
| } | |
| if ( | |
| slotEndTime.isBefore(dayjs(date.start).add(utcOffset, "minutes")) || | |
| slotEndTime.isSame(dayjs(date.start).add(utcOffset, "minutes")) | |
| ) { | |
| return true; | |
| } | |
| if (slotStartTime.isAfter(dayjs(date.end).add(utcOffset, "minutes"))) { | |
| const slotStartInOrganizerTz = organizerTimeZone ? slotStartTime.tz(organizerTimeZone) : slotStartTime; | |
| const slotEndInOrganizerTz = organizerTimeZone ? slotEndTime.tz(organizerTimeZone) : slotEndTime; | |
| const dateStartInOrganizerTz = organizerTimeZone | |
| ? dayjs(date.start).tz(organizerTimeZone) | |
| : dayjs(date.start).utc(); | |
| const dateEndInOrganizerTz = organizerTimeZone | |
| ? dayjs(date.end).tz(organizerTimeZone) | |
| : dayjs(date.end).utc(); | |
| if ( | |
| dateStartInOrganizerTz.format("YYYY MM DD") === | |
| slotStartInOrganizerTz.format("YYYY MM DD") | |
| ) { | |
| dateOverrideExist = true; | |
| if (dateStartInOrganizerTz.isSame(dateEndInOrganizerTz)) { | |
| return true; | |
| } | |
| if (slotEndInOrganizerTz.isBefore(dateStartInOrganizerTz) || slotEndInOrganizerTz.isSame(dateStartInOrganizerTz)) { | |
| return true; | |
| } | |
| if (slotStartInOrganizerTz.isAfter(dateEndInOrganizerTz)) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test 7nn
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#7](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/7)*