From f34d6e3c17552373d6045f769c75ed93be744544 Mon Sep 17 00:00:00 2001 From: sean-brydon <55134778+sean-brydon@users.noreply.github.com> Date: Wed, 2 Nov 2022 09:40:30 +0000 Subject: [PATCH] Buffer limits fix (#5248) * Buffer limits + remove redundant tests * Fixing buffer * Compound * Afterbuffer fix for no event afterbuffer set * Bug fixes * Buffer includes eventType before --- apps/web/lib/hooks/useSlots.ts | 235 ------------------ apps/web/test/lib/slots.test.ts | 205 ++++++--------- packages/core/getBusyTimes.ts | 45 +++- packages/core/getUserAvailability.ts | 12 +- packages/trpc/server/routers/viewer/slots.tsx | 8 +- 5 files changed, 123 insertions(+), 382 deletions(-) delete mode 100644 apps/web/lib/hooks/useSlots.ts diff --git a/apps/web/lib/hooks/useSlots.ts b/apps/web/lib/hooks/useSlots.ts deleted file mode 100644 index c319a04907..0000000000 --- a/apps/web/lib/hooks/useSlots.ts +++ /dev/null @@ -1,235 +0,0 @@ -import { SchedulingType } from "@prisma/client"; -import { stringify } from "querystring"; -import { useEffect, useState } from "react"; - -import type { CurrentSeats } from "@calcom/core/getUserAvailability"; -import dayjs, { Dayjs } from "@calcom/dayjs"; - -import getSlots from "@lib/slots"; -import type { TimeRange, WorkingHours } from "@lib/types/schedule"; - -type AvailabilityUserResponse = { - busy: TimeRange[]; - timeZone: string; - workingHours: WorkingHours[]; - currentSeats?: CurrentSeats; -}; - -type Slot = { - time: Dayjs; - users?: string[]; - bookingUid?: string; - attendees?: number; -}; - -type UseSlotsProps = { - slotInterval: number | null; - eventLength: number; - eventTypeId: number; - minimumBookingNotice?: number; - date: Dayjs; - users: { username: string | null }[]; - schedulingType: SchedulingType | null; - beforeBufferTime?: number; - afterBufferTime?: number; -}; - -type getFilteredTimesProps = { - times: dayjs.Dayjs[]; - busy: TimeRange[]; - eventLength: number; - beforeBufferTime: number; - afterBufferTime: number; - currentSeats?: CurrentSeats; -}; - -export const getFilteredTimes = (props: getFilteredTimesProps) => { - const { times, busy, eventLength, beforeBufferTime, afterBufferTime, currentSeats } = props; - const finalizationTime = times[times.length - 1]?.add(eventLength, "minutes"); - // Check for conflicts - for (let i = times.length - 1; i >= 0; i -= 1) { - // const totalSlotLength = eventLength + beforeBufferTime + afterBufferTime; - // Check if the slot surpasses the user's availability end time - const slotEndTimeWithAfterBuffer = times[i].add(eventLength + afterBufferTime, "minutes"); - if (slotEndTimeWithAfterBuffer.isAfter(finalizationTime, "minute")) { - times.splice(i, 1); - } else { - const slotStartTime = times[i]; - const slotEndTime = times[i].add(eventLength, "minutes"); - const slotStartTimeWithBeforeBuffer = times[i].subtract(beforeBufferTime, "minutes"); - // If the event has seats then see if there is already a booking (want to show full bookings as well) - if (currentSeats?.some((booking) => booking.startTime === slotStartTime.toDate())) { - break; - } - busy.every((busyTime): boolean => { - const startTime = dayjs(busyTime.start); - const endTime = dayjs(busyTime.end); - // Check if start times are the same - if (slotStartTime.isBetween(startTime, endTime, null, "[)")) { - times.splice(i, 1); - } - // Check if slot end time is between start and end time - else if (slotEndTime.isBetween(startTime, endTime)) { - times.splice(i, 1); - } - // Check if startTime is between slot - else if (startTime.isBetween(slotStartTime, slotEndTime)) { - times.splice(i, 1); - } - // Check if timeslot has before buffer time space free - else if ( - slotStartTimeWithBeforeBuffer.isBetween( - startTime.subtract(beforeBufferTime, "minutes"), - endTime.add(afterBufferTime, "minutes") - ) - ) { - times.splice(i, 1); - } - // Check if timeslot has after buffer time space free - else if ( - slotEndTimeWithAfterBuffer.isBetween( - startTime.subtract(beforeBufferTime, "minutes"), - endTime.add(afterBufferTime, "minutes") - ) - ) { - times.splice(i, 1); - } else { - return true; - } - return false; - }); - } - } - return times; -}; - -export const useSlots = (props: UseSlotsProps) => { - const { - slotInterval, - eventLength, - minimumBookingNotice = 0, - beforeBufferTime = 0, - afterBufferTime = 0, - date, - users, - eventTypeId, - } = props; - const [slots, setSlots] = useState([]); - const [loading, setLoading] = useState(false); - const [error, setError] = useState(null); - - useEffect(() => { - setSlots([]); - setLoading(true); - setError(null); - - const dateFrom = date.startOf("day").format(); - const dateTo = date.endOf("day").format(); - const query = stringify({ dateFrom, dateTo, eventTypeId }); - - const handleAvailableSlots = async (res: Response) => { - const responseBody: AvailabilityUserResponse = await res.json(); - const { workingHours, busy, currentSeats } = responseBody; - const times = getSlots({ - frequency: slotInterval || eventLength, - inviteeDate: date, - workingHours, - minimumBookingNotice, - eventLength, - }); - const filterTimeProps = { - times, - busy, - eventLength, - beforeBufferTime, - afterBufferTime, - currentSeats, - }; - const filteredTimes = getFilteredTimes(filterTimeProps); - // temporary - const user = res.url.substring(res.url.lastIndexOf("/") + 1, res.url.indexOf("?")); - return filteredTimes.map((time) => ({ - time, - users: [user], - // Conditionally add the attendees and booking id to slots object if there is already a booking during that time - ...(currentSeats?.some((booking) => booking.startTime === time.toDate()) && { - attendees: - currentSeats[currentSeats.findIndex((booking) => booking.startTime === time.toDate())]._count - .attendees, - bookingUid: - currentSeats[currentSeats.findIndex((booking) => booking.startTime === time.toDate())].uid, - }), - })); - }; - - Promise.all( - users.map((user) => fetch(`/api/availability/${user.username}?${query}`).then(handleAvailableSlots)) - ) - .then((results) => { - let loadedSlots: Slot[] = results[0] || []; - if (results.length === 1) { - loadedSlots = loadedSlots?.sort((a, b) => (a.time.isAfter(b.time) ? 1 : -1)); - setSlots(loadedSlots); - setLoading(false); - return; - } - - let poolingMethod; - switch (props.schedulingType) { - // intersect by time, does not take into account eventLength (yet) - case SchedulingType.COLLECTIVE: - poolingMethod = (slots: Slot[], compareWith: Slot[]) => - slots.filter((slot) => compareWith.some((compare) => compare.time.isSame(slot.time))); - break; - case SchedulingType.ROUND_ROBIN: - // TODO: Create a Reservation (lock this slot for X minutes) - // this will make the following code redundant - poolingMethod = (slots: Slot[], compareWith: Slot[]) => { - compareWith.forEach((compare) => { - const match = slots.findIndex((slot) => slot.time.isSame(compare.time)); - if (match !== -1) { - slots[match].users?.push(compare.users![0]); - } else { - slots.push(compare); - } - }); - return slots; - }; - break; - } - - if (!poolingMethod) { - throw Error(`No poolingMethod found for schedulingType: "${props.schedulingType}""`); - } - - for (let i = 1; i < results.length; i++) { - loadedSlots = poolingMethod(loadedSlots, results[i]); - } - loadedSlots = loadedSlots.sort((a, b) => (a.time.isAfter(b.time) ? 1 : -1)); - setSlots(loadedSlots); - setLoading(false); - }) - .catch((e) => { - console.error(e); - setError(e); - }); - }, [ - afterBufferTime, - beforeBufferTime, - eventLength, - minimumBookingNotice, - slotInterval, - eventTypeId, - props.schedulingType, - users, - date, - ]); - - return { - slots, - loading, - error, - }; -}; - -export default useSlots; diff --git a/apps/web/test/lib/slots.test.ts b/apps/web/test/lib/slots.test.ts index c08324953f..c299d4b422 100644 --- a/apps/web/test/lib/slots.test.ts +++ b/apps/web/test/lib/slots.test.ts @@ -5,109 +5,14 @@ import dayjs from "@calcom/dayjs"; import getSlots from "@calcom/lib/slots"; import { MINUTES_DAY_END, MINUTES_DAY_START } from "@lib/availability"; -import { getFilteredTimes } from "@lib/hooks/useSlots"; MockDate.set("2021-06-20T11:59:59Z"); -it("can fit 24 hourly slots for an empty day", async () => { - // 24h in a day. - expect( - getSlots({ - inviteeDate: dayjs.utc().add(1, "day"), - frequency: 60, - minimumBookingNotice: 0, - workingHours: [ - { - days: Array.from(Array(7).keys()), - startTime: MINUTES_DAY_START, - endTime: MINUTES_DAY_END, - }, - ], - eventLength: 60, - }) - ).toHaveLength(24); -}); - -// TODO: This test is sound; it should pass! -it("only shows future booking slots on the same day", async () => { - // The mock date is 1s to midday, so 12 slots should be open given 0 booking notice. - expect( - getSlots({ - inviteeDate: dayjs.utc(), - frequency: 60, - minimumBookingNotice: 0, - workingHours: [ - { - days: Array.from(Array(7).keys()), - startTime: MINUTES_DAY_START, - endTime: MINUTES_DAY_END, - }, - ], - eventLength: 60, - }) - ).toHaveLength(12); -}); - -it("can cut off dates that due to invitee timezone differences fall on the next day", async () => { - expect( - getSlots({ - inviteeDate: dayjs().tz("Europe/Amsterdam").startOf("day"), // time translation +01:00 - frequency: 60, - minimumBookingNotice: 0, - workingHours: [ - { - days: [0], - startTime: 23 * 60, // 23h - endTime: MINUTES_DAY_END, - }, - ], - eventLength: 60, - }) - ).toHaveLength(0); -}); - -it("can cut off dates that due to invitee timezone differences fall on the previous day", async () => { - const workingHours = [ - { - days: [0], - startTime: MINUTES_DAY_START, - endTime: 1 * 60, // 1h - }, - ]; - expect( - getSlots({ - inviteeDate: dayjs().tz("Atlantic/Cape_Verde").startOf("day"), // time translation -01:00 - frequency: 60, - minimumBookingNotice: 0, - workingHours, - eventLength: 60, - }) - ).toHaveLength(0); -}); - -it("adds minimum booking notice correctly", async () => { - // 24h in a day. - expect( - getSlots({ - inviteeDate: dayjs.utc().add(1, "day").startOf("day"), - frequency: 60, - minimumBookingNotice: 1500, - workingHours: [ - { - days: Array.from(Array(7).keys()), - startTime: MINUTES_DAY_START, - endTime: MINUTES_DAY_END, - }, - ], - eventLength: 60, - }) - ).toHaveLength(11); -}); - -it("adds buffer time", async () => { - expect( - getFilteredTimes({ - times: getSlots({ +describe("Tests the slot logic", () => { + it("can fit 24 hourly slots for an empty day", async () => { + // 24h in a day. + expect( + getSlots({ inviteeDate: dayjs.utc().add(1, "day"), frequency: 60, minimumBookingNotice: 0, @@ -119,26 +24,17 @@ it("adds buffer time", async () => { }, ], eventLength: 60, - }), - busy: [ - { - start: dayjs.utc("2021-06-21 12:50:00", "YYYY-MM-DD HH:mm:ss").toDate(), - end: dayjs.utc("2021-06-21 13:50:00", "YYYY-MM-DD HH:mm:ss").toDate(), - }, - ], - eventLength: 60, - beforeBufferTime: 15, - afterBufferTime: 15, - }) - ).toHaveLength(20); -}); + }) + ).toHaveLength(24); + }); -it("adds buffer time with custom slot interval", async () => { - expect( - getFilteredTimes({ - times: getSlots({ - inviteeDate: dayjs.utc().add(1, "day"), - frequency: 5, + // TODO: This test is sound; it should pass! + it("only shows future booking slots on the same day", async () => { + // The mock date is 1s to midday, so 12 slots should be open given 0 booking notice. + expect( + getSlots({ + inviteeDate: dayjs.utc(), + frequency: 60, minimumBookingNotice: 0, workingHours: [ { @@ -148,16 +44,63 @@ it("adds buffer time with custom slot interval", async () => { }, ], eventLength: 60, - }), - busy: [ - { - start: dayjs.utc("2021-06-21 12:50:00", "YYYY-MM-DD HH:mm:ss").toDate(), - end: dayjs.utc("2021-06-21 13:50:00", "YYYY-MM-DD HH:mm:ss").toDate(), - }, - ], - eventLength: 60, - beforeBufferTime: 15, - afterBufferTime: 15, - }) - ).toHaveLength(239); + }) + ).toHaveLength(12); + }); + + it("can cut off dates that due to invitee timezone differences fall on the next day", async () => { + expect( + getSlots({ + inviteeDate: dayjs().tz("Europe/Amsterdam").startOf("day"), // time translation +01:00 + frequency: 60, + minimumBookingNotice: 0, + workingHours: [ + { + days: [0], + startTime: 23 * 60, // 23h + endTime: MINUTES_DAY_END, + }, + ], + eventLength: 60, + }) + ).toHaveLength(0); + }); + + it("can cut off dates that due to invitee timezone differences fall on the previous day", async () => { + const workingHours = [ + { + days: [0], + startTime: MINUTES_DAY_START, + endTime: 1 * 60, // 1h + }, + ]; + expect( + getSlots({ + inviteeDate: dayjs().tz("Atlantic/Cape_Verde").startOf("day"), // time translation -01:00 + frequency: 60, + minimumBookingNotice: 0, + workingHours, + eventLength: 60, + }) + ).toHaveLength(0); + }); + + it("adds minimum booking notice correctly", async () => { + // 24h in a day. + expect( + getSlots({ + inviteeDate: dayjs.utc().add(1, "day").startOf("day"), + frequency: 60, + minimumBookingNotice: 1500, + workingHours: [ + { + days: Array.from(Array(7).keys()), + startTime: MINUTES_DAY_START, + endTime: MINUTES_DAY_END, + }, + ], + eventLength: 60, + }) + ).toHaveLength(11); + }); }); diff --git a/packages/core/getBusyTimes.ts b/packages/core/getBusyTimes.ts index a63720bb2d..8baa2d6a64 100644 --- a/packages/core/getBusyTimes.ts +++ b/packages/core/getBusyTimes.ts @@ -1,6 +1,7 @@ import { BookingStatus, Credential, SelectedCalendar } from "@prisma/client"; import { getBusyCalendarTimes } from "@calcom/core/CalendarManager"; +import dayjs from "@calcom/dayjs"; import logger from "@calcom/lib/logger"; import { performance } from "@calcom/lib/server/perfObserver"; import prisma from "@calcom/prisma"; @@ -11,10 +12,21 @@ export async function getBusyTimes(params: { userId: number; eventTypeId?: number; startTime: string; + beforeEventBuffer?: number; + afterEventBuffer?: number; endTime: string; selectedCalendars: SelectedCalendar[]; }) { - const { credentials, userId, eventTypeId, startTime, endTime, selectedCalendars } = params; + const { + credentials, + userId, + eventTypeId, + startTime, + endTime, + selectedCalendars, + beforeEventBuffer, + afterEventBuffer, + } = params; logger.silly( `Checking Busy time from Cal Bookings in range ${startTime} to ${endTime} for input ${JSON.stringify({ userId, @@ -27,7 +39,6 @@ export async function getBusyTimes(params: { .findMany({ where: { userId, - eventTypeId, startTime: { gte: new Date(startTime) }, endTime: { lte: new Date(endTime) }, status: { @@ -39,12 +50,22 @@ export async function getBusyTimes(params: { startTime: true, endTime: true, title: true, + eventType: { + select: { + afterEventBuffer: true, + beforeEventBuffer: true, + }, + }, }, }) .then((bookings) => - bookings.map(({ startTime, endTime, title, id }) => ({ - end: endTime, - start: startTime, + bookings.map(({ startTime, endTime, title, id, eventType }) => ({ + start: dayjs(startTime) + .subtract((eventType?.beforeEventBuffer || 0) + (afterEventBuffer || 0), "minute") + .toDate(), + end: dayjs(endTime) + .add((eventType?.afterEventBuffer || 0) + (beforeEventBuffer || 0), "minute") + .toDate(), title, source: `eventType-${eventTypeId}-booking-${id}`, })) @@ -55,7 +76,19 @@ export async function getBusyTimes(params: { if (credentials?.length > 0) { const calendarBusyTimes = await getBusyCalendarTimes(credentials, startTime, endTime, selectedCalendars); - busyTimes.push(...calendarBusyTimes); /* + busyTimes.push( + ...calendarBusyTimes.map((value) => ({ + ...value, + end: dayjs(value.end) + .add(beforeEventBuffer || 0, "minute") + .toDate(), + start: dayjs(value.start) + .subtract(afterEventBuffer || 0, "minute") + .toDate(), + })) + ); + + /* // TODO: Disabled until we can filter Zoom events by date. Also this is adding too much latency. const videoBusyTimes = (await getBusyVideoTimes(credentials)).filter(notEmpty); console.log("videoBusyTimes", videoBusyTimes); diff --git a/packages/core/getUserAvailability.ts b/packages/core/getUserAvailability.ts index 406da17903..0c084f32a2 100644 --- a/packages/core/getUserAvailability.ts +++ b/packages/core/getUserAvailability.ts @@ -22,6 +22,7 @@ const availabilitySchema = z username: z.string().optional(), userId: z.number().optional(), afterEventBuffer: z.number().optional(), + beforeEventBuffer: z.number().optional(), withSource: z.boolean().optional(), }) .refine((data) => !!data.username || !!data.userId, "Either username or userId should be filled in."); @@ -92,6 +93,7 @@ export async function getUserAvailability( dateTo: string; eventTypeId?: number; afterEventBuffer?: number; + beforeEventBuffer?: number; }, initialData?: { user?: User; @@ -99,7 +101,7 @@ export async function getUserAvailability( currentSeats?: CurrentSeats; } ) { - const { username, userId, dateFrom, dateTo, eventTypeId, afterEventBuffer } = + const { username, userId, dateFrom, dateTo, eventTypeId, afterEventBuffer, beforeEventBuffer } = availabilitySchema.parse(query); if (!dateFrom.isValid() || !dateTo.isValid()) @@ -134,14 +136,14 @@ export async function getUserAvailability( eventTypeId, userId: currentUser.id, selectedCalendars, + beforeEventBuffer, + afterEventBuffer, }); const bufferedBusyTimes: EventBusyDetails[] = busyTimes.map((a) => ({ ...a, - start: dayjs(a.start).subtract(currentUser.bufferTime, "minute").toISOString(), - end: dayjs(a.end) - .add(currentUser.bufferTime + (afterEventBuffer || 0), "minute") - .toISOString(), + start: dayjs(a.start).toISOString(), + end: dayjs(a.end).toISOString(), title: a.title, source: query.withSource ? a.source : undefined, })); diff --git a/packages/trpc/server/routers/viewer/slots.tsx b/packages/trpc/server/routers/viewer/slots.tsx index adb366980b..72939da7f0 100644 --- a/packages/trpc/server/routers/viewer/slots.tsx +++ b/packages/trpc/server/routers/viewer/slots.tsx @@ -50,13 +50,11 @@ const checkIfIsAvailable = ({ time, busy, eventLength, - beforeBufferTime, currentSeats, }: { time: Dayjs; - busy: (TimeRange | { start: string; end: string } | EventBusyDate)[]; + busy: EventBusyDate[]; eventLength: number; - beforeBufferTime: number; currentSeats?: CurrentSeats; }): boolean => { if (currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString())) { @@ -67,7 +65,7 @@ const checkIfIsAvailable = ({ const slotStartTime = time.utc(); return busy.every((busyTime) => { - const startTime = dayjs.utc(busyTime.start).subtract(beforeBufferTime, "minutes").utc(); + const startTime = dayjs.utc(busyTime.start).utc(); const endTime = dayjs.utc(busyTime.end); if (endTime.isBefore(slotStartTime) || startTime.isAfter(slotEndTime)) { @@ -230,6 +228,7 @@ export async function getSchedule(input: z.infer, ctx: dateTo: endTime.format(), eventTypeId: input.eventTypeId, afterEventBuffer: eventType.afterEventBuffer, + beforeEventBuffer: eventType.beforeEventBuffer, }, { user: currentUser, eventType, currentSeats } ); @@ -246,7 +245,6 @@ export async function getSchedule(input: z.infer, ctx: const computedAvailableSlots: Record = {}; const availabilityCheckProps = { eventLength: eventType.length, - beforeBufferTime: eventType.beforeEventBuffer, currentSeats, };