diff --git a/apps/web/package.json b/apps/web/package.json index b621013f70..c5cb21de7c 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -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", diff --git a/apps/web/test/lib/getSchedule.test.ts b/apps/web/test/lib/getSchedule.test.ts index 5e4dbf3c68..24b17b9360 100644 --- a/apps/web/test/lib/getSchedule.test.ts +++ b/apps/web/test/lib/getSchedule.test.ts @@ -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); } diff --git a/packages/core/getUserAvailability.ts b/packages/core/getUserAvailability.ts index 9811516c6f..02de43a249 100644 --- a/packages/core/getUserAvailability.ts +++ b/packages/core/getUserAvailability.ts @@ -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 diff --git a/packages/lib/slots.ts b/packages/lib/slots.ts index c9328a70cf..c2fa08c2ab 100644 --- a/packages/lib/slots.ts +++ b/packages/lib/slots.ts @@ -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 = ({ diff --git a/packages/trpc/server/routers/viewer/slots.tsx b/packages/trpc/server/routers/viewer/slots.tsx index 1e552195e8..0e0e761f8b 100644 --- a/packages/trpc/server/routers/viewer/slots.tsx +++ b/packages/trpc/server/routers/viewer/slots.tsx @@ -265,7 +265,6 @@ export async function getSchedule(input: z.infer, ctx: ); const workingHours = getAggregateWorkingHours(userAvailability, eventType.schedulingType); - const computedAvailableSlots: Record = {}; const availabilityCheckProps = { eventLength: eventType.length, currentSeats, @@ -280,67 +279,80 @@ export async function getSchedule(input: z.infer, 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 = []; + + 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, + { 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, 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`); diff --git a/tests/config/singleton.ts b/tests/config/singleton.ts index e6e38deaf1..3d09ed5db5 100644 --- a/tests/config/singleton.ts +++ b/tests/config/singleton.ts @@ -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(), @@ -13,3 +16,4 @@ beforeEach(() => { }); export const prismaMock = prisma as unknown as DeepMockProxy; +export const CalendarManagerMock = CalendarManager as unknown as DeepMockProxy; diff --git a/yarn.lock b/yarn.lock index ab68ff09e6..535f4b6c9b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"