Skip to content

Automated Test: date-algorithm-enhanced #358

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;
}
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;
}

if (dateOverrideExist) {
return true;
}
Comment on lines +102 to +135

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Reference-equality comparison of dayjs objects on line 114 is always false.

dayjs(...) === dayjs(...) compares two distinct object references — this will never be true. Use .isSame() to compare dayjs values:

-        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"))) {

Additionally, the broader find()-based logic here is incorrect when multiple date overrides exist for the same day (e.g., two users with different override windows, or a user with morning + afternoon override blocks). The find() will match any override that the slot falls outside of, even if the slot is valid inside a different override. This causes valid slots to be rejected.

For example, with overrides A (10am–12pm) and B (2pm–4pm), a slot at 11am would be incorrectly rejected because it falls outside override B, even though it's inside override A.

Consider restructuring: if any override matches this day, the slot should be accepted if it falls within at least one of the matching overrides (i.e., use some() to check containment, not find() to check exclusion).

📝 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
//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;
}
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;
}
if (dateOverrideExist) {
return true;
}
//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").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;
}
}
})
) {
// slot is not within the date override
return false;
}
if (dateOverrideExist) {
return true;
}
🧰 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 102 - 135, The
current logic using dateOverrides.find(...) incorrectly compares dayjs objects
with === and rejects slots if they fall outside any single override; change it
to iterate with dateOverrides.some(...) to accept the slot if it is contained by
at least one override for that day. Inside the predicate use
dayjs(...).isSame(...) instead of === to test same-day equality (or compare
formatted strings), compute utcOffset once, normalize start/end with
dayjs(date.start).add(utcOffset, "minutes") and dayjs(date.end).add(utcOffset,
"minutes"), then check containment with .isSame(), .isBefore(), and .isAfter()
against slotStartTime and slotEndTime (or their normalized equivalents); set and
use a boolean like dateOverrideExist only to indicate at least one matching-day
override was found, and return false only if no matching override contains the
slot (i.e., !someContains).


//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;
}
}
})
) {
// slot is outside of working hours
return false;
}
Comment on lines +137 to +151

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Bug: end is computed identically to start — slot end time is never validated against working hours.

Lines 141-142 both use slotStartTime, making start === end always true. The end variable should use slotEndTime:

       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) {

This means a slot that starts within working hours but extends past workingHour.endTime would currently not be filtered out.

📝 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 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;
}
}
})
) {
// slot is outside of working hours
return false;
}
//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 = slotEndTime.hour() * 60 + slotEndTime.minute();
if (start < workingHour.startTime || end > workingHour.endTime) {
return true;
}
}
})
) {
// 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 137 - 151, The
check for slot containment in workingHours incorrectly computes end using
slotStartTime, so start and end are identical; update the predicate in the
workingHours.find callback to compute end from slotEndTime (use
slotEndTime.hour()/minute()) instead of slotStartTime so the comparison against
workingHour.endTime correctly detects slots that extend past working hours;
ensure you only return true when either start < workingHour.startTime or end >
workingHour.endTime.


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