Skip to content

Automated Test: date-algorithm-enhanced #366

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions apps/web/test/lib/getSchedule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,24 @@ describe("getSchedule", () => {
dateString: plus2DateString,
}
);

const scheduleForEventOnADayWithDateOverrideDifferentTimezone = await getSchedule(
{
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 () => {
Expand Down
21 changes: 16 additions & 5 deletions packages/lib/slots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
const inviteeUtcOffset = dayjs(override.start.toString()).tz(timeZone).utcOffset();
const offset = inviteeUtcOffset - organizerUtcOffset;

return {
userIds: override.userId ? [override.userId] : [],
startTime:
dayjs(override.start).utc().add(offset, "minute").hour() * 60 +
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;
Expand Down
81 changes: 76 additions & 5 deletions packages/trpc/server/routers/viewer/slots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand All @@ -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 (
dateOverrides.find((date) => {
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;
}
Comment on lines +114 to +116

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Bug: Dayjs object comparison with === always returns false.

Comparing two Dayjs objects with === checks reference equality, not value equality. This condition will never be true, even when the start and end times are identical.

🐛 Proposed fix using `.isSame()`
-        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"))) {
           return true;
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (dayjs(date.start).add(utcOffset, "minutes") === dayjs(date.end).add(utcOffset, "minutes")) {
return true;
}
if (dayjs(date.start).add(utcOffset, "minutes").isSame(dayjs(date.end).add(utcOffset, "minutes"))) {
return true;
}
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 114 - 116, The
comparison using === between two Dayjs objects (dayjs(date.start).add(utcOffset,
"minutes") === dayjs(date.end).add(utcOffset, "minutes")) is wrong; replace it
with Dayjs value equality using .isSame(), e.g. call .isSame(...) on one of the
adjusted Dayjs instances (optionally with a granularity like "minute") so the
condition correctly detects identical moments; update the expression where this
check occurs to use .isSame instead of ===.

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;
}
Comment on lines +106 to +131

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Refactor: find() callback should always return a boolean value.

Static analysis correctly flags that this callback doesn't return a value on all paths. When the date matches but the slot is within the override bounds, undefined is implicitly returned. While this works due to JavaScript's truthy/falsy handling, it's error-prone and hard to reason about.

Consider refactoring to use some() with explicit returns, or restructure the logic:

♻️ Suggested refactor for clarity
-  if (
-    dateOverrides.find((date) => {
-      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").isSame(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"))) {
-          return true;
-        }
-      }
-    })
-  ) {
+  const slotOutsideOverride = dateOverrides.some((date) => {
+    const utcOffset = organizerTimeZone ? dayjs.tz(date.start, organizerTimeZone).utcOffset() * -1 : 0;
+    const overrideStart = dayjs(date.start).add(utcOffset, "minutes");
+    const overrideEnd = dayjs(date.end).add(utcOffset, "minutes");
+
+    if (overrideStart.format("YYYY MM DD") !== slotStartTime.format("YYYY MM DD")) {
+      return false; // Not the same day
+    }
+
+    dateOverrideExist = true;
+
+    // Check if slot is outside the override window
+    if (overrideStart.isSame(overrideEnd)) {
+      return true; // Zero-length override blocks the day
+    }
+    if (slotEndTime.isBefore(overrideStart) || slotEndTime.isSame(overrideStart)) {
+      return true;
+    }
+    if (slotStartTime.isAfter(overrideEnd)) {
+      return true;
+    }
+    return false; // Slot is within override
+  });
+
+  if (slotOutsideOverride) {
     // slot is not within the date override
     return false;
   }
🧰 Tools
🪛 Biome (2.3.13)

[error] 106-106: This callback passed to find() iterable method should always return a value.

Add missing return statements so that this callback returns a value on all execution paths.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 106 - 131, The
find() callback for dateOverrides may return undefined on some paths; change the
logic so the predicate always returns a boolean and use a clearer API (e.g.,
replace dateOverrides.find(...) with dateOverrides.some(...)) and return
explicit true/false from the callback. Locate the block referencing
dateOverrides.find, organizerTimeZone, slotStartTime, slotEndTime and
dateOverrideExist; set dateOverrideExist when a matching date is found and have
the callback return true for both "slot outside override" and "slot
equals/overlaps" cases and false otherwise (or use some() to directly assign
dateOverrideExist = dateOverrides.some(...)). Ensure no path leaves the callback
without an explicit boolean return.


if (dateOverrideExist) {
return true;
}

//if no date override for slot exists check if it is within normal work hours
if (
workingHours.find((workingHour) => {
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotStartTime.hour() * 60 + slotStartTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
Comment on lines +141 to +145

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Both start and end are computed from slotStartTime, causing incorrect working hours validation.

Line 142 computes end identically to start, using slotStartTime for both. This means the working hours boundary check on line 143 is comparing start < workingHour.startTime || start > workingHour.endTime which doesn't account for the slot's duration.

🐛 Proposed fix
       if (workingHour.days.includes(slotStartTime.day())) {
         const start = slotStartTime.hour() * 60 + slotStartTime.minute();
-        const end = slotStartTime.hour() * 60 + slotStartTime.minute();
+        const end = slotEndTime.hour() * 60 + slotEndTime.minute();
         if (start < workingHour.startTime || end > workingHour.endTime) {
           return true;
         }
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotStartTime.hour() * 60 + slotStartTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 141 - 145, The
working-hours check incorrectly computes both start and end from slotStartTime;
update the end calculation to use the slot's end time (e.g., slotEndTime.hour()
* 60 + slotEndTime.minute() or slotStartTime.plus(duration) if slotEndTime
variable is named differently) so that the start/end values accurately reflect
the slot span, then keep the existing boundary check against
workingHour.startTime and workingHour.endTime using the corrected end variable;
adjust any variable names in the block containing start, end, slotStartTime and
workingHour accordingly.

}
})
) {
// slot is outside of working hours
return false;
}
Comment on lines +138 to +151

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Refactor: Working hours find() callback should explicitly return a boolean.

Same issue as above - this callback only returns true when the slot is outside working hours, but implicitly returns undefined otherwise.

♻️ Suggested refactor
-  if (
-    workingHours.find((workingHour) => {
-      if (workingHour.days.includes(slotStartTime.day())) {
-        const start = slotStartTime.hour() * 60 + slotStartTime.minute();
-        const end = slotEndTime.hour() * 60 + slotEndTime.minute();
-        if (start < workingHour.startTime || end > workingHour.endTime) {
-          return true;
-        }
-      }
-    })
-  ) {
+  const slotOutsideWorkingHours = workingHours.some((workingHour) => {
+    if (!workingHour.days.includes(slotStartTime.day())) {
+      return false;
+    }
+    const start = slotStartTime.hour() * 60 + slotStartTime.minute();
+    const end = slotEndTime.hour() * 60 + slotEndTime.minute();
+    return start < workingHour.startTime || end > workingHour.endTime;
+  });
+
+  if (slotOutsideWorkingHours) {
     // slot is outside of working hours
     return false;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (
workingHours.find((workingHour) => {
if (workingHour.days.includes(slotStartTime.day())) {
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotStartTime.hour() * 60 + slotStartTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {
// slot is outside of working hours
return false;
}
const slotOutsideWorkingHours = workingHours.some((workingHour) => {
if (!workingHour.days.includes(slotStartTime.day())) {
return false;
}
const start = slotStartTime.hour() * 60 + slotStartTime.minute();
const end = slotEndTime.hour() * 60 + slotEndTime.minute();
return start < workingHour.startTime || end > workingHour.endTime;
});
if (slotOutsideWorkingHours) {
// slot is outside of working hours
return false;
}
🧰 Tools
🪛 Biome (2.3.13)

[error] 139-139: This callback passed to find() iterable method should always return a value.

Add missing return statements so that this callback returns a value on all execution paths.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
In `@packages/trpc/server/routers/viewer/slots.ts` around lines 138 - 151, The
callback passed to workingHours.find currently only returns true when a
violation is detected and otherwise returns undefined; update the callback for
workingHours.find (the anonymous arrow function referencing workingHour and
slotStartTime) to always return a boolean — return true when the slot is outside
the workingHour bounds and explicitly return false otherwise (and also ensure
you compute start/end from the correct slot time variables if needed) so find()
behaves predictably.


return busy.every((busyTime) => {
const startTime = dayjs.utc(busyTime.start).utc();
const endTime = dayjs.utc(busyTime.end);
Expand All @@ -115,7 +176,6 @@ const checkIfIsAvailable = ({
else if (startTime.isBetween(time, slotEndTime)) {
return false;
}

return true;
});
};
Expand Down Expand Up @@ -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 = {
Expand All @@ -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;

for (
let currentCheckedTime = startTime;
currentCheckedTime.isBefore(endTime);
Expand All @@ -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,
})
);
}
Expand Down Expand Up @@ -423,13 +489,15 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
time: slot.time,
...schedule,
...availabilityCheckProps,
organizerTimeZone: schedule.timeZone,
});
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) {
Expand All @@ -446,6 +514,7 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
time: slot.time,
...userSchedule,
...availabilityCheckProps,
organizerTimeZone: userSchedule.timeZone,
});
});
return slot;
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions packages/types/schedule.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export type TimeRange = {
userId?: number | null;
start: Date;
end: Date;
timeZone?: string;
};

export type Schedule = TimeRange[][];
Expand Down