Refactored buildSlots (#6389)

* Refactored buildSlots

* Added typing for computedAvailableSlots

* Adds very simple boundary logic to make sure slots start at the right times

* Logic was cutting off too early

* Woops, should be eventLength, not frequency

* Expect 12:15 slots as availability is till 12:30 for booking

* Deliberately skip test for testing purposes

* slotInterval overrides intervals, but not duration

* Fix calendar mocking

* Another 45m length

* Unskip :(

* Fix test

Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
Co-authored-by: Omar López <zomars@me.com>
pull/6412/head^2
Alex van Andel 2023-01-11 17:33:34 +00:00 committed by GitHub
parent b34a568a93
commit c8744768c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 179 additions and 209 deletions

View File

@ -89,7 +89,6 @@
"next-seo": "^4.26.0",
"next-themes": "^0.2.0",
"next-transpile-modules": "^10.0.0",
"nock": "^13.2.8",
"nodemailer": "^6.7.8",
"otplib": "^12.0.1",
"qrcode": "^1.5.1",

View File

@ -6,7 +6,6 @@ import {
SchedulingType,
} from "@prisma/client";
import { diff } from "jest-diff";
import nock from "nock";
import { v4 as uuidv4 } from "uuid";
import logger from "@calcom/lib/logger";
@ -14,7 +13,7 @@ import prisma from "@calcom/prisma";
import { BookingStatus } from "@calcom/prisma/client";
import { getSchedule, Slot } from "@calcom/trpc/server/routers/viewer/slots";
import { prismaMock } from "../../../../tests/config/singleton";
import { prismaMock, CalendarManagerMock } from "../../../../tests/config/singleton";
// TODO: Mock properly
prismaMock.eventType.findUnique.mockResolvedValue(null);
@ -228,6 +227,7 @@ describe("getSchedule", () => {
{
id: 1,
slotInterval: 45,
length: 45,
users: [
{
id: 101,
@ -249,15 +249,12 @@ describe("getSchedule", () => {
// An event with one accepted booking
createBookingScenario(scenarioData);
addBusyTimesInGoogleCalendar(
[
{
start: `${plus2DateString}T04:30:00.000Z`,
end: `${plus2DateString}T23:00:00.000Z`,
},
],
scenarioData
);
addBusyTimesInGoogleCalendar([
{
start: `${plus2DateString}T04:45:00.000Z`,
end: `${plus2DateString}T23:00:00.000Z`,
},
]);
const scheduleForDayWithAGoogleCalendarBooking = await getSchedule(
{
eventTypeId: 1,
@ -269,7 +266,7 @@ describe("getSchedule", () => {
ctx
);
// As per Google Calendar Availability, only 4PM GMT slot would be available
// As per Google Calendar Availability, only 4PM(4-4:45PM) GMT slot would be available
expect(scheduleForDayWithAGoogleCalendarBooking).toHaveTimeSlots([`04:00:00.000Z`], {
dateString: plus2DateString,
});
@ -290,6 +287,7 @@ describe("getSchedule", () => {
id: 1,
// If `slotInterval` is set, it supersedes `length`
slotInterval: 45,
length: 45,
users: [
{
id: 101,
@ -613,15 +611,12 @@ describe("getSchedule", () => {
createBookingScenario(scenarioData);
addBusyTimesInGoogleCalendar(
[
{
start: `${plus3DateString}T04:00:00.000Z`,
end: `${plus3DateString}T05:59:59.000Z`,
},
],
scenarioData
);
addBusyTimesInGoogleCalendar([
{
start: `${plus3DateString}T04:00:00.000Z`,
end: `${plus3DateString}T05:59:59.000Z`,
},
]);
const scheduleForEventOnADayWithNonCalBooking = await getSchedule(
{
@ -690,15 +685,12 @@ describe("getSchedule", () => {
createBookingScenario(scenarioData);
addBusyTimesInGoogleCalendar(
[
{
start: `${plus3DateString}T04:00:00.000Z`,
end: `${plus3DateString}T05:59:59.000Z`,
},
],
scenarioData
);
addBusyTimesInGoogleCalendar([
{
start: `${plus3DateString}T04:00:00.000Z`,
end: `${plus3DateString}T05:59:59.000Z`,
},
]);
const scheduleForEventOnADayWithCalBooking = await getSchedule(
{
@ -785,6 +777,7 @@ describe("getSchedule", () => {
{
id: 1,
slotInterval: 45,
length: 45,
users: [
{
id: 101,
@ -797,6 +790,7 @@ describe("getSchedule", () => {
{
id: 2,
slotInterval: 45,
length: 45,
users: [
{
id: 102,
@ -905,6 +899,7 @@ describe("getSchedule", () => {
{
id: 1,
slotInterval: 45,
length: 45,
users: [
{
id: 101,
@ -918,6 +913,7 @@ describe("getSchedule", () => {
{
id: 2,
slotInterval: 45,
length: 45,
users: [
{
id: 102,
@ -1082,7 +1078,6 @@ function addEventTypes(eventTypes: InputEventType[], usersStore: InputUser[]) {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
prismaMock.eventType.findUnique.mockImplementation(({ where }) => {
console.log("eventTypesWithUsers", eventTypesWithUsers);
return new Promise((resolve) => {
const eventType = eventTypesWithUsers.find((e) => e.id === where.id) as unknown as PrismaEventType & {
users: PrismaUser[];
@ -1210,7 +1205,6 @@ const getDate = (param: { dateIncrement?: number; monthIncrement?: number; yearI
const date = _date < 10 ? "0" + _date : _date;
const month = _month < 10 ? "0" + _month : _month;
console.log(`Date, month, year for ${JSON.stringify(param)}`, date, month, year);
return {
date,
month,
@ -1220,38 +1214,13 @@ const getDate = (param: { dateIncrement?: number; monthIncrement?: number; yearI
};
/**
* Remember that this fn must be called only if you expect your test to lookup for busy times in Google Calendar.
* Calling it unnecessarily will result in a test failure. This is how nock works because it would expect a call to the requests and that too only once.
* TODO: Improve this to validate the arguments passed to getBusyCalendarTimes if they are valid or not.
*/
function addBusyTimesInGoogleCalendar(
busy: {
start: string;
end: string;
}[],
data: ScenarioData
}[]
) {
if (!data.users.find((u) => u.credentials && u.selectedCalendars)) {
throw new Error(
"Google Calendar mocking requires atleast one user with both `credentials` and `selectedCalendars`"
);
}
if (!data.apps?.find((app) => app.slug === "google-calendar")) {
throw new Error('Google Calendar mocking requires an app with slug "google-calendar"');
}
logger.silly("Adding busy times in Google Calendar", busy);
nock("https://oauth2.googleapis.com").post("/token").reply(200, {
access_token: "access_token",
expiry_date: Infinity,
});
// Google Calendar with 11th July having many events
nock("https://www.googleapis.com")
.post("/calendar/v3/freeBusy")
.reply(200, {
calendars: [
{
busy,
},
],
});
CalendarManagerMock.getBusyCalendarTimes.mockResolvedValue(busy);
}

View File

@ -164,7 +164,7 @@ export async function getUserAvailability(
let startDate = dayjs(dateFrom);
const endDate = dayjs(dateTo);
while (startDate.isBefore(endDate)) {
dates.push(startDate.add(1, "day"));
dates.push(startDate);
startDate = startDate.add(1, "day");
}
@ -173,7 +173,6 @@ export async function getUserAvailability(
);
// Apply booking limit filter against our bookings
for (const [key, limit] of Object.entries(bookingLimits)) {
const limitKey = key as keyof BookingLimit;
@ -192,18 +191,16 @@ export async function getUserAvailability(
});
break;
}
// Take PER_DAY and turn it into day and PER_WEEK into week etc.
const filter = limitKey.split("_")[1].toLocaleLowerCase() as "day" | "week" | "month" | "year";
const filter = limitKey.split("_")[1].toLowerCase() as "day" | "week" | "month" | "year";
// loop through all dates and check if we have reached the limit
for (const date of dates) {
let total = 0;
const startDate = dayjs(date).startOf(filter);
const startDate = date.startOf(filter);
// this is parsed above with parseBookingLimit so we know it's safe.
const endDate = dayjs(date).endOf(filter);
const endDate = date.endOf(filter);
for (const booking of ourBookings) {
const bookingEventTypeId = booking.source?.split("-")[1];
const bookingEventTypeId = parseInt(booking.source?.split("-")[1] as string, 10);
if (
// Only check OUR booking that matches the current eventTypeId
// we don't care about another event type in this case as we dont need to know their booking limits

View File

@ -15,41 +15,6 @@ export type TimeFrame = { userIds?: number[]; startTime: number; endTime: number
const minimumOfOne = (input: number) => (input < 1 ? 1 : input);
/**
* TODO: What does this function do?
* Why is it needed?
*/
const splitAvailableTime = (
startTimeMinutes: number,
endTimeMinutes: number,
frequency: number,
eventLength: number
): TimeFrame[] => {
let initialTime = startTimeMinutes;
const finalizationTime = endTimeMinutes;
const result = [] as TimeFrame[];
// Ensure that both the frequency and event length are at least 1 minute, if they
// would be zero, we would have an infinite loop in this while!
const frequencyMinimumOne = minimumOfOne(frequency);
const eventLengthMinimumOne = minimumOfOne(eventLength);
while (initialTime < finalizationTime) {
const periodTime = initialTime + frequencyMinimumOne;
const slotEndTime = initialTime + eventLengthMinimumOne;
/*
check if the slot end time surpasses availability end time of the user
1 minute is added to round up the hour mark so that end of the slot is considered in the check instead of x9
eg: if finalization time is 11:59, slotEndTime is 12:00, we ideally want the slot to be available
*/
if (slotEndTime <= finalizationTime + 1) result.push({ startTime: initialTime, endTime: periodTime });
// Ensure that both the frequency and event length are at least 1 minute, if they
// would be zero, we would have an infinite loop in this while!
initialTime += frequencyMinimumOne;
}
return result;
};
function buildSlots({
startOfInviteeDay,
computedLocalAvailability,
@ -63,24 +28,75 @@ function buildSlots({
frequency: number;
eventLength: number;
}) {
const slotsTimeFrameAvailable: TimeFrame[] = [];
// no slots today
if (startOfInviteeDay.isBefore(startDate, "day")) {
return [];
}
// keep the old safeguards in; may be needed.
frequency = minimumOfOne(frequency);
eventLength = minimumOfOne(eventLength);
// A day starts at 00:00 unless the startDate is the same as the current day
const dayStart = startOfInviteeDay.isSame(startDate, "day")
? Math.ceil((startDate.hour() * 60 + startDate.minute()) / frequency) * frequency
: 0;
computedLocalAvailability.forEach((item) => {
const userSlotsTimeFrameAvailable = splitAvailableTime(
item.startTime,
item.endTime,
frequency,
eventLength
).map((slot) => ({ ...slot, userIds: item.userIds }));
// Record type so we can use slotStart as key
const slotsTimeFrameAvailable: Record<
string,
{
userIds: number[];
startTime: number;
endTime: number;
}
> = {};
// get boundaries sorted by start time.
const boundaries = computedLocalAvailability
.map((item) => [item.startTime < dayStart ? dayStart : item.startTime, item.endTime])
.sort((a, b) => a[0] - b[0]);
slotsTimeFrameAvailable.push(...userSlotsTimeFrameAvailable);
});
const ranges: number[][] = [];
let currentRange: number[] = [];
for (const [start, end] of boundaries) {
// bypass invalid value
if (start >= end) continue;
// fill first elem
if (!currentRange.length) {
currentRange = [start, end];
continue;
}
if (currentRange[1] < start) {
ranges.push(currentRange);
currentRange = [start, end];
} else if (currentRange[1] < end) {
currentRange[1] = end;
}
}
if (currentRange) {
ranges.push(currentRange);
}
const slots: { [x: string]: { time: Dayjs; userIds?: number[] } } = {};
slotsTimeFrameAvailable.forEach((item) => {
// XXX: Hack alert, as dayjs is supposedly not aware of timezone the current slot may have invalid UTC offset.
const timeZone =
(startOfInviteeDay as unknown as { $x: { $timezone: string } })["$x"]["$timezone"] || "UTC";
for (const [boundaryStart, boundaryEnd] of ranges) {
// loop through the day, based on frequency.
for (let slotStart = boundaryStart; slotStart < boundaryEnd; slotStart += frequency) {
computedLocalAvailability.forEach((item) => {
// TODO: This logic does not allow for past-midnight bookings.
if (slotStart < item.startTime || slotStart > item.endTime + 15 - eventLength) {
return;
}
slotsTimeFrameAvailable[slotStart.toString()] = {
userIds: (slotsTimeFrameAvailable[slotStart]?.userIds || []).concat(item.userIds || []),
startTime: slotStart,
endTime: slotStart + eventLength,
};
});
}
}
// XXX: Hack alert, as dayjs is supposedly not aware of timezone the current slot may have invalid UTC offset.
const timeZone =
(startOfInviteeDay as unknown as { $x: { $timezone: string } })["$x"]["$timezone"] || "UTC";
const slots: { time: Dayjs; userIds?: number[] }[] = [];
for (const item of Object.values(slotsTimeFrameAvailable)) {
/*
* @calcom/web:dev: 2022-11-06T00:00:00-04:00
* @calcom/web:dev: 2022-11-06T01:00:00-04:00
@ -97,22 +113,9 @@ function buildSlots({
// As the time has now fallen backwards, or forwards; this difference -
// needs to be manually added as this is not done for us. Usually 0.
slot.time = slot.time.add(startOfInviteeDay.utcOffset() - slot.time.utcOffset(), "minutes");
if (slots[slot.time.format()]) {
slots[slot.time.format()] = {
...slot,
userIds: [...(slots[slot.time.format()].userIds || []), ...(item.userIds || [])],
};
return;
}
// Validating slot its not on the past
if (slot.time.isBefore(startDate)) {
return;
}
slots[slot.time.format()] = slot;
});
return Object.values(slots);
slots.push(slot);
}
return slots;
}
const getSlots = ({

View File

@ -265,7 +265,6 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
);
const workingHours = getAggregateWorkingHours(userAvailability, eventType.schedulingType);
const computedAvailableSlots: Record<string, Slot[]> = {};
const availabilityCheckProps = {
eventLength: eventType.length,
currentSeats,
@ -280,67 +279,80 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
periodDays: eventType.periodDays,
});
let currentCheckedTime = startTime;
let getSlotsTime = 0;
const getSlotsTime = 0;
let checkForAvailabilityTime = 0;
let getSlotsCount = 0;
const getSlotsCount = 0;
let checkForAvailabilityCount = 0;
do {
const startGetSlots = performance.now();
const timeSlots: ReturnType<typeof getTimeSlots> = [];
for (
let currentCheckedTime = startTime;
currentCheckedTime.isBefore(endTime);
currentCheckedTime = currentCheckedTime.add(1, "day")
) {
// get slots retrieves the available times for a given day
const timeSlots = getTimeSlots({
inviteeDate: currentCheckedTime,
eventLength: input.duration || eventType.length,
workingHours,
dateOverrides,
minimumBookingNotice: eventType.minimumBookingNotice,
frequency: eventType.slotInterval || input.duration || eventType.length,
});
timeSlots.push(
...getTimeSlots({
inviteeDate: currentCheckedTime,
eventLength: input.duration || eventType.length,
workingHours,
dateOverrides,
minimumBookingNotice: eventType.minimumBookingNotice,
frequency: eventType.slotInterval || input.duration || eventType.length,
})
);
}
const endGetSlots = performance.now();
getSlotsTime += endGetSlots - startGetSlots;
getSlotsCount++;
const isCollective = !eventType.schedulingType || eventType.schedulingType === SchedulingType.COLLECTIVE;
const isCollective = !eventType.schedulingType || eventType.schedulingType === SchedulingType.COLLECTIVE;
let availableTimeSlots: typeof timeSlots = [];
if (isCollective) {
availableTimeSlots = timeSlots.filter((slot) =>
userAvailability.every((schedule) => {
const startCheckForAvailability = performance.now();
const isAvailable = checkIfIsAvailable({
time: slot.time,
...schedule,
...availabilityCheckProps,
});
const endCheckForAvailability = performance.now();
checkForAvailabilityCount++;
checkForAvailabilityTime += endCheckForAvailability - startCheckForAvailability;
return isAvailable;
})
);
} else {
availableTimeSlots = timeSlots
.map((slot) => {
slot.userIds = slot.userIds?.filter((slotUserId) => {
const userSchedule = userAvailability.find(({ userId }) => userId === slotUserId);
if (!userSchedule) {
throw new TRPCError({
message: "Shouldn't happen that we don't have a matching user schedule here",
code: "INTERNAL_SERVER_ERROR",
});
}
return checkIfIsAvailable({
time: slot.time,
...userSchedule,
...availabilityCheckProps,
});
});
return slot;
})
.filter((slot) => slot.userIds && slot.userIds.length > 0);
}
const availableTimeSlots = timeSlots
.filter((slot) => isTimeWithinBounds(slot.time))
.filter((slot) =>
isCollective
? // The slot should be available for every user
userAvailability.every((schedule) => {
const startCheckForAvailability = performance.now();
const isAvailable = checkIfIsAvailable({
time: slot.time,
...schedule,
...availabilityCheckProps,
});
const endCheckForAvailability = performance.now();
checkForAvailabilityCount++;
checkForAvailabilityTime += endCheckForAvailability - startCheckForAvailability;
return isAvailable;
})
: (() => {
// The slot should be available for the atleast one of the slot owners.
return slot.userIds?.some((slotUserId) => {
const userSchedule = userAvailability.find(({ userId }) => userId === slotUserId);
if (!userSchedule) {
throw new TRPCError({
message: "Shouldn't happen that we don't have a matching user schedule here",
code: "INTERNAL_SERVER_ERROR",
});
}
return checkIfIsAvailable({
time: slot.time,
...userSchedule,
...availabilityCheckProps,
});
});
})()
);
availableTimeSlots = availableTimeSlots.filter((slot) => isTimeWithinBounds(slot.time));
computedAvailableSlots[currentCheckedTime.format("YYYY-MM-DD")] = availableTimeSlots.map(
({ time: time, ...passThroughProps }) => ({
const computedAvailableSlots = availableTimeSlots.reduce(
(
r: Record<string, { time: string; users: string[]; attendees?: number; bookingUid?: string }[]>,
{ time: time, ...passThroughProps }
) => {
r[time.format("YYYY-MM-DD")] = r[time.format("YYYY-MM-DD")] || [];
r[time.format("YYYY-MM-DD")].push({
...passThroughProps,
time: time.toISOString(),
users: eventType.users.map((user) => user.username || ""),
@ -355,10 +367,11 @@ export async function getSchedule(input: z.infer<typeof getScheduleSchema>, ctx:
currentSeats.findIndex((booking) => booking.startTime.toISOString() === time.toISOString())
].uid,
}),
})
);
currentCheckedTime = currentCheckedTime.add(1, "day");
} while (currentCheckedTime.isBefore(endTime));
});
return r;
},
Object.create(null)
);
logger.debug(`getSlots took ${getSlotsTime}ms and executed ${getSlotsCount} times`);

View File

@ -1,8 +1,11 @@
import { PrismaClient } from "@prisma/client";
import { mockDeep, mockReset, DeepMockProxy } from "jest-mock-extended";
import * as CalendarManager from "@calcom/core/CalendarManager";
import prisma from "@calcom/prisma";
jest.mock("@calcom/core/CalendarManager");
jest.mock("@calcom/prisma", () => ({
__esModule: true,
default: mockDeep<PrismaClient>(),
@ -13,3 +16,4 @@ beforeEach(() => {
});
export const prismaMock = prisma as unknown as DeepMockProxy<PrismaClient>;
export const CalendarManagerMock = CalendarManager as unknown as DeepMockProxy<typeof CalendarManager>;

View File

@ -20403,16 +20403,6 @@ no-case@^3.0.4:
lower-case "^2.0.2"
tslib "^2.0.3"
nock@^13.2.8:
version "13.2.9"
resolved "https://registry.yarnpkg.com/nock/-/nock-13.2.9.tgz#4faf6c28175d36044da4cfa68e33e5a15086ad4c"
integrity sha512-1+XfJNYF1cjGB+TKMWi29eZ0b82QOvQs2YoLNzbpWGqFMtRQHTa57osqdGj4FrFPgkO4D4AZinzUJR9VvW3QUA==
dependencies:
debug "^4.1.0"
json-stringify-safe "^5.0.1"
lodash "^4.17.21"
propagate "^2.0.0"
node-abort-controller@^3.0.1:
version "3.0.1"
resolved "https://registry.yarnpkg.com/node-abort-controller/-/node-abort-controller-3.0.1.tgz#f91fa50b1dee3f909afabb7e261b1e1d6b0cb74e"
@ -22094,11 +22084,6 @@ prop-types@^15.0.0, prop-types@^15.5.8, prop-types@^15.6.0, prop-types@^15.6.2,
object-assign "^4.1.1"
react-is "^16.13.1"
propagate@^2.0.0:
version "2.0.1"
resolved "https://registry.yarnpkg.com/propagate/-/propagate-2.0.1.tgz#40cdedab18085c792334e64f0ac17256d38f9a45"
integrity sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==
property-expr@^2.0.4:
version "2.0.5"
resolved "https://registry.yarnpkg.com/property-expr/-/property-expr-2.0.5.tgz#278bdb15308ae16af3e3b9640024524f4dc02cb4"