Repository navigation
Date override fixes #8330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Date override fixes #8330
Changes from all commits
754e63e
ae94d69
de351dd
dbf4bfa
79dc235
40ae4e5
3140203
d5d798b
b634898
017ed53
ee38fd2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -784,6 +784,24 @@ describe("getSchedule", () => { | |
| dateString: plus2DateString, | ||
| } | ||
| ); | ||
|
|
||
| const scheduleForEventOnADayWithDateOverrideDifferentTimezone = await getSchedule( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 This new assertion only covers the timezone-adjustment fix (#8329): it re-queries the existing single-user personal event (no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 The added test is a single-user, non-team event ( Suggest adding a team scenario: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 This test is a single-user personal event ( |
||
| { | ||
| eventTypeId: 1, | ||
| eventTypeSlug: "", | ||
| startTime: `${plus1DateString}T18:30:00.000Z`, | ||
| endTime: `${plus2DateString}T18:29:59.999Z`, | ||
| timeZone: Timezones["+6:00"], | ||
| }, | ||
| ctx | ||
| ); | ||
| // it should return the same as this is the utc time | ||
| expect(scheduleForEventOnADayWithDateOverrideDifferentTimezone).toHaveTimeSlots( | ||
| ["08:30:00.000Z", "09:30:00.000Z", "10:30:00.000Z", "11:30:00.000Z"], | ||
| { | ||
| dateString: plus2DateString, | ||
| } | ||
| ); | ||
| }); | ||
|
|
||
| test("that a user is considered busy when there's a booking they host", async () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,11 +208,22 @@ const getSlots = ({ | |
| }); | ||
|
|
||
| if (!!activeOverrides.length) { | ||
| const overrides = activeOverrides.flatMap((override) => ({ | ||
| userIds: override.userId ? [override.userId] : [], | ||
| startTime: override.start.getUTCHours() * 60 + override.start.getUTCMinutes(), | ||
| endTime: override.end.getUTCHours() * 60 + override.end.getUTCMinutes(), | ||
| })); | ||
| const overrides = activeOverrides.flatMap((override) => { | ||
| const organizerUtcOffset = dayjs(override.start.toString()).tz(override.timeZone).utcOffset(); | ||
|
emrysal marked this conversation as resolved.
|
||
| const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset(); | ||
| const offset = inviteeUtcOffset - organizerUtcOffset; | ||
|
|
||
| return { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the times of |
||
| userIds: override.userId ? [override.userId] : [], | ||
| startTime: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The offset conversion can wrap past midnight for large timezone gaps. |
||
| dayjs(override.start).utc().add(offset, "minute").hour() * 60 + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The offset conversion can wrap past midnight for large timezone gaps. |
||
| dayjs(override.start).utc().add(offset, "minute").minute(), | ||
| endTime: | ||
| dayjs(override.end).utc().add(offset, "minute").hour() * 60 + | ||
| dayjs(override.end).utc().add(offset, "minute").minute(), | ||
| }; | ||
| }); | ||
|
|
||
| // unset all working hours that relate to this user availability override | ||
| overrides.forEach((override) => { | ||
| let i = -1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import type prisma from "@calcom/prisma"; | |
| import { availabilityUserSelect } from "@calcom/prisma"; | ||
| import { EventTypeMetaDataSchema } from "@calcom/prisma/zod-utils"; | ||
| import type { EventBusyDate } from "@calcom/types/Calendar"; | ||
| import type { WorkingHours } from "@calcom/types/schedule"; | ||
|
|
||
| import { TRPCError } from "@trpc/server"; | ||
|
|
||
|
|
@@ -75,12 +76,21 @@ const checkIfIsAvailable = ({ | |
| time, | ||
| busy, | ||
| eventLength, | ||
| dateOverrides = [], | ||
| workingHours = [], | ||
| currentSeats, | ||
| organizerTimeZone, | ||
| }: { | ||
| time: Dayjs; | ||
| busy: EventBusyDate[]; | ||
| eventLength: number; | ||
| dateOverrides?: { | ||
| start: Date; | ||
| end: Date; | ||
| }[]; | ||
| workingHours?: WorkingHours[]; | ||
| currentSeats?: CurrentSeats; | ||
| organizerTimeZone?: string; | ||
| }): boolean => { | ||
| if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) { | ||
| return true; | ||
|
|
@@ -89,6 +99,57 @@ const checkIfIsAvailable = ({ | |
| const slotEndTime = time.add(eventLength, "minutes").utc(); | ||
| const slotStartTime = time.utc(); | ||
|
|
||
| //check if date override for slot exists | ||
| let dateOverrideExist = false; | ||
|
|
||
| if ( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a date override exists on the day of the slot and the slot starts before that or ends after that date override it will return false (not available) |
||
| dateOverrides.find((date) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Multiple date overrides on the same day for one host are mishandled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking — multiple same-day overrides reject all slots, and partial overlaps are accepted.
Separately, the boundary checks only reject slots entirely-before or entirely-after an override, so a slot that partially overlaps (e.g. 09:30–10:30 vs override 10:00–11:00, generated from the aggregate window of another host) makes the callback return Fix: collect same-day overrides, then require containment in at least one: const slotLocalDate = organizerTimeZone ? time.tz(organizerTimeZone).format("YYYY MM DD") : slotStartTime.format("YYYY MM DD");
const sameDay = dateOverrides.filter((d) =>
(organizerTimeZone ? dayjs.tz(d.start, organizerTimeZone).format("YYYY MM DD") : dayjs(d.start).format("YYYY MM DD")) === slotLocalDate
);
if (sameDay.length) {
const contained = sameDay.some((d) => {
const oStart = organizerTimeZone ? dayjs.tz(d.start, organizerTimeZone) : dayjs(d.start);
const oEnd = organizerTimeZone ? dayjs.tz(d.end, organizerTimeZone) : dayjs(d.end);
return !slotStartTime.isBefore(oStart) && !slotEndTime.isAfter(oEnd);
});
if (!contained) return false;
return busy.every(/* ...existing busy check... */);
} |
||
| const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The override day-match compares a shifted
const overrideLocalDate = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).format("YYYY MM DD") : dayjs(date.start).format("YYYY MM DD");
const slotLocalDate = organizerTimeZone ? time.tz(organizerTimeZone).format("YYYY MM DD") : slotStartTime.format("YYYY MM DD");
if (overrideLocalDate === slotLocalDate) { ... } |
||
|
|
||
| if ( | ||
| dayjs(date.start).add(utcOffset, "minutes").format("YYYY MM DD") === | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 |
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) {
return true;
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 |
||
| 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"))) { | ||
| return true; | ||
| } | ||
| } | ||
| }) | ||
| ) { | ||
| // slot is not within the date override | ||
| return false; | ||
| } | ||
|
emrysal marked this conversation as resolved.
|
||
|
|
||
| if (dateOverrideExist) { | ||
| return true; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 When a fixed host has a date override on a day, slots within the override window set if (dateOverrideExist) {
// Override replaces working hours, but still check for conflicting bookings
return busy.every((busyTime) => { /* ...existing busy check... */ });
} |
||
| } | ||
|
|
||
| //if no date override for slot exists check if it is within normal work hours | ||
| if ( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking — The callback returns Invert to "available if any block contains the slot": const inSomeBlock = workingHours.some((wh) => {
if (!wh.days.includes(slotStartTime.day())) return false;
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();
return start >= wh.startTime && end <= wh.endTime;
});
if (!inSomeBlock) {
return false;
} |
||
| workingHours.find((workingHour) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 const inSomeBlock = workingHours.some((wh) => {
if (!wh.days.includes(slotStartTime.day())) return false;
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();
return start >= wh.startTime && end <= wh.endTime;
});
if (!inSomeBlock) return false; |
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Blocking — Line 141 sets const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 const end = slotEndTime.hour() * 60 + slotEndTime.minute(); |
||
| if (start < workingHour.startTime || end > workingHour.endTime) { | ||
| return true; | ||
| } | ||
| } | ||
| }) | ||
| ) { | ||
| // slot is outside of working hours | ||
| return false; | ||
| } | ||
|
|
||
| return busy.every((busyTime) => { | ||
| const startTime = dayjs.utc(busyTime.start).utc(); | ||
| const endTime = dayjs.utc(busyTime.end); | ||
|
|
@@ -115,7 +176,6 @@ const checkIfIsAvailable = ({ | |
| else if (startTime.isBetween(time, slotEndTime)) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| }); | ||
| }; | ||
|
|
@@ -348,7 +408,11 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
| ); | ||
| // flattens availability of multiple users | ||
| const dateOverrides = userAvailability.flatMap((availability) => | ||
| availability.dateOverrides.map((override) => ({ userId: availability.user.id, ...override })) | ||
| availability.dateOverrides.map((override) => ({ | ||
| userId: availability.user.id, | ||
| timeZone: availability.timeZone, | ||
| ...override, | ||
| })) | ||
| ); | ||
| const workingHours = getAggregateWorkingHours(userAvailability, eventType.schedulingType); | ||
| const availabilityCheckProps = { | ||
|
|
@@ -372,6 +436,9 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
|
|
||
| const timeSlots: ReturnType<typeof getTimeSlots> = []; | ||
|
|
||
| const organizerTimeZone = | ||
| eventType.timeZone || eventType?.schedule?.timeZone || userAvailability?.[0]?.timeZone; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure userAvailability?.[0] works; I think you need to involve the defaultSchedule here |
||
|
|
||
| for ( | ||
| let currentCheckedTime = startTime; | ||
| currentCheckedTime.isBefore(endTime); | ||
|
|
@@ -386,8 +453,7 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
| dateOverrides, | ||
| minimumBookingNotice: eventType.minimumBookingNotice, | ||
| frequency: eventType.slotInterval || input.duration || eventType.length, | ||
| organizerTimeZone: | ||
| eventType.timeZone || eventType?.schedule?.timeZone || userAvailability?.[0]?.timeZone, | ||
| organizerTimeZone, | ||
| }) | ||
| ); | ||
| } | ||
|
|
@@ -423,13 +489,15 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
| time: slot.time, | ||
| ...schedule, | ||
| ...availabilityCheckProps, | ||
| organizerTimeZone: schedule.timeZone, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alex we want to have the schedule's timezone here |
||
| }); | ||
| const endCheckForAvailability = performance.now(); | ||
| checkForAvailabilityCount++; | ||
| checkForAvailabilityTime += endCheckForAvailability - startCheckForAvailability; | ||
| return isAvailable; | ||
| }); | ||
| }); | ||
|
|
||
| // what else are you going to call it? | ||
| const looseHostAvailability = userAvailability.filter(({ user: { isFixed } }) => !isFixed); | ||
| if (looseHostAvailability.length > 0) { | ||
|
|
@@ -446,6 +514,7 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
| time: slot.time, | ||
| ...userSchedule, | ||
| ...availabilityCheckProps, | ||
| organizerTimeZone: userSchedule.timeZone, | ||
| }); | ||
| }); | ||
| return slot; | ||
|
|
@@ -507,17 +576,19 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx: | |
| return false; | ||
| } | ||
|
|
||
| const userSchedule = userAvailability.find(({ user: { id: userId } }) => userId === slotUserId); | ||
|
|
||
| return checkIfIsAvailable({ | ||
| time: slot.time, | ||
| busy, | ||
| ...availabilityCheckProps, | ||
| organizerTimeZone: userSchedule?.timeZone, | ||
| }); | ||
| }); | ||
| return slot; | ||
| }) | ||
| .filter((slot) => !!slot.userIds?.length); | ||
| } | ||
|
|
||
| availableTimeSlots = availableTimeSlots.filter((slot) => isTimeWithinBounds(slot.time)); | ||
|
|
||
| const computedAvailableSlots = availableTimeSlots.reduce( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here attendee has a different timezone than the organizer. This test would have failed before and fixes #8329