From 37ce8860b5ea0f9a229e551adf0b219b0718dafe Mon Sep 17 00:00:00 2001 From: nicktrn <55853254+nicktrn@users.noreply.github.com> Date: Wed, 9 Aug 2023 23:51:38 +0100 Subject: [PATCH] fix: all booking and duration limits (#10480) Co-authored-by: rkreddy99 Co-authored-by: alannnc --- .../components/eventtype/EventLimitsTab.tsx | 11 +- apps/web/test/lib/checkBookingLimits.test.ts | 15 - apps/web/test/lib/checkDurationLimits.test.ts | 15 - packages/core/getBusyTimes.ts | 59 +++ packages/core/getUserAvailability.ts | 377 ++++++++++++------ packages/lib/intervalLimit.ts | 12 + packages/lib/server/checkBookingLimits.ts | 95 ++--- packages/lib/server/checkDurationLimits.ts | 57 ++- packages/lib/server/queries/booking/index.ts | 9 +- packages/lib/validateIntervalLimitOrder.ts | 5 +- packages/types/Calendar.d.ts | 9 +- 11 files changed, 398 insertions(+), 266 deletions(-) create mode 100644 packages/lib/intervalLimit.ts diff --git a/apps/web/components/eventtype/EventLimitsTab.tsx b/apps/web/components/eventtype/EventLimitsTab.tsx index 595bbba614..44ba1743f3 100644 --- a/apps/web/components/eventtype/EventLimitsTab.tsx +++ b/apps/web/components/eventtype/EventLimitsTab.tsx @@ -13,6 +13,7 @@ import type { DurationType } from "@calcom/lib/convertToNewDurationType"; import convertToNewDurationType from "@calcom/lib/convertToNewDurationType"; import findDurationType from "@calcom/lib/findDurationType"; import { useLocale } from "@calcom/lib/hooks/useLocale"; +import { ascendingLimitKeys, intervalLimitKeyToUnit } from "@calcom/lib/intervalLimit"; import type { PeriodType } from "@calcom/prisma/enums"; import type { IntervalLimit } from "@calcom/types/Calendar"; import { Button, DateRangePicker, InputField, Label, Select, SettingsToggle, TextField } from "@calcom/ui"; @@ -476,11 +477,9 @@ export const EventLimitsTab = ({ eventType }: Pick ({ +const INTERVAL_LIMIT_OPTIONS = ascendingLimitKeys.map((key) => ({ value: key as keyof IntervalLimit, - label: `Per ${key.split("_")[1].toLocaleLowerCase()}`, + label: `Per ${intervalLimitKeyToUnit(key)}`, })); type IntervalLimitItemProps = { @@ -589,8 +588,8 @@ const IntervalLimitsManager = ({ Object.entries(currentIntervalLimits) .sort(([limitKeyA], [limitKeyB]) => { return ( - intervalOrderKeys.indexOf(limitKeyA as IntervalLimitsKey) - - intervalOrderKeys.indexOf(limitKeyB as IntervalLimitsKey) + ascendingLimitKeys.indexOf(limitKeyA as IntervalLimitsKey) - + ascendingLimitKeys.indexOf(limitKeyB as IntervalLimitsKey) ); }) .map(([key, value]) => { diff --git a/apps/web/test/lib/checkBookingLimits.test.ts b/apps/web/test/lib/checkBookingLimits.test.ts index 281380e2df..e834c10482 100644 --- a/apps/web/test/lib/checkBookingLimits.test.ts +++ b/apps/web/test/lib/checkBookingLimits.test.ts @@ -81,21 +81,6 @@ describe("Check Booking Limits Tests", () => { }) ).rejects.toThrowError(); }); - it("Should return busyTimes when set", async () => { - prismaMock.booking.count.mockResolvedValue(2); - expect( - checkBookingLimit({ - key: "PER_DAY", - limitingNumber: 2, - eventStartDate: MOCK_DATA.startDate, - eventId: MOCK_DATA.id, - returnBusyTimes: true, - }) - ).resolves.toEqual({ - start: dayjs(MOCK_DATA.startDate).startOf("day").toDate(), - end: dayjs(MOCK_DATA.startDate).endOf("day").toDate(), - }); - }); }); describe("Booking limit validation", () => { diff --git a/apps/web/test/lib/checkDurationLimits.test.ts b/apps/web/test/lib/checkDurationLimits.test.ts index 6186563b76..2fd6d39c27 100644 --- a/apps/web/test/lib/checkDurationLimits.test.ts +++ b/apps/web/test/lib/checkDurationLimits.test.ts @@ -97,21 +97,6 @@ describe("Check Duration Limit Tests", () => { }) ).resolves.toBeUndefined(); }); - it("Should return busyTimes when set and limit is reached", async () => { - prismaMock.$queryRaw.mockResolvedValue([{ totalMinutes: 60 }]); - await expect( - checkDurationLimit({ - key: "PER_DAY", - limitingNumber: 60, - eventStartDate: MOCK_DATA.startDate, - eventId: MOCK_DATA.id, - returnBusyTimes: true, - }) - ).resolves.toEqual({ - start: dayjs(MOCK_DATA.startDate).startOf("day").toDate(), - end: dayjs(MOCK_DATA.startDate).endOf("day").toDate(), - }); - }); }); describe("Duration limit validation", () => { diff --git a/packages/core/getBusyTimes.ts b/packages/core/getBusyTimes.ts index 1bd141d7b7..c132d08c51 100644 --- a/packages/core/getBusyTimes.ts +++ b/packages/core/getBusyTimes.ts @@ -207,4 +207,63 @@ export async function getBusyTimes(params: { return busyTimes; } +export async function getBusyTimesForLimitChecks(params: { + userId: number; + eventTypeId: number; + startDate: Date; + endDate: Date; +}) { + const { userId, eventTypeId, startDate, endDate } = params; + logger.silly( + `Fetch limit checks bookings in range ${startDate} to ${endDate} for input ${JSON.stringify({ + userId, + eventTypeId, + status: BookingStatus.ACCEPTED, + })}` + ); + performance.mark("getBusyTimesForLimitChecksStart"); + + const bookings = await prisma.booking.findMany({ + where: { + userId, + eventTypeId, + status: BookingStatus.ACCEPTED, + // FIXME: bookings that overlap on one side will never be counted + startTime: { + gte: startDate, + }, + endTime: { + lte: endDate, + }, + }, + select: { + id: true, + startTime: true, + endTime: true, + eventType: { + select: { + id: true, + }, + }, + title: true, + }, + }); + + const busyTimes = bookings.map(({ id, startTime, endTime, eventType, title }) => ({ + start: dayjs(startTime).toDate(), + end: dayjs(endTime).toDate(), + title, + source: `eventType-${eventType?.id}-booking-${id}`, + })); + + logger.silly(`Fetch limit checks bookings for eventId: ${eventTypeId} ${JSON.stringify(busyTimes)}`); + performance.mark("getBusyTimesForLimitChecksEnd"); + performance.measure( + `prisma booking get for limits took $1'`, + "getBusyTimesForLimitChecksStart", + "getBusyTimesForLimitChecksEnd" + ); + return busyTimes; +} + export default getBusyTimes; diff --git a/packages/core/getUserAvailability.ts b/packages/core/getUserAvailability.ts index bb8c22f708..9d70fcc93b 100644 --- a/packages/core/getUserAvailability.ts +++ b/packages/core/getUserAvailability.ts @@ -7,6 +7,7 @@ import { parseBookingLimit, parseDurationLimit } from "@calcom/lib"; import { getWorkingHours } from "@calcom/lib/availability"; import { buildDateRanges, subtract } from "@calcom/lib/date-ranges"; import { HttpError } from "@calcom/lib/http-error"; +import { descendingLimitKeys, intervalLimitKeyToUnit } from "@calcom/lib/intervalLimit"; import logger from "@calcom/lib/logger"; import { checkBookingLimit } from "@calcom/lib/server"; import { performance } from "@calcom/lib/server/perfObserver"; @@ -14,9 +15,14 @@ import { getTotalBookingDuration } from "@calcom/lib/server/queries"; import prisma, { availabilityUserSelect } from "@calcom/prisma"; import { BookingStatus } from "@calcom/prisma/enums"; import { EventTypeMetaDataSchema, stringToDayjs } from "@calcom/prisma/zod-utils"; -import type { EventBusyDetails, IntervalLimit } from "@calcom/types/Calendar"; +import type { + EventBusyDate, + EventBusyDetails, + IntervalLimit, + IntervalLimitUnit, +} from "@calcom/types/Calendar"; -import { getBusyTimes } from "./getBusyTimes"; +import { getBusyTimes, getBusyTimesForLimitChecks } from "./getBusyTimes"; const availabilitySchema = z .object({ @@ -105,7 +111,7 @@ export const getCurrentSeats = (eventTypeId: number, dateFrom: Dayjs, dateTo: Da export type CurrentSeats = Awaited>; /** This should be called getUsersWorkingHoursAndBusySlots (...and remaining seats, and final timezone) */ -export async function getUserAvailability( +export const getUserAvailability = async function getUsersWorkingHoursLifeTheUniverseAndEverythingElse( query: { withSource?: boolean; username?: string; @@ -141,17 +147,36 @@ export async function getUserAvailability( if (!eventType && eventTypeId) eventType = await getEventType(eventTypeId); /* Current logic is if a booking is in a time slot mark it as busy, but seats can have more than one attendee so grab - current bookings with a seats event type and display them on the calendar, even if they are full */ + current bookings with a seats event type and display them on the calendar, even if they are full */ let currentSeats: CurrentSeats | null = initialData?.currentSeats || null; if (!currentSeats && eventType?.seatsPerTimeSlot) { currentSeats = await getCurrentSeats(eventType.id, dateFrom, dateTo); } + const bookingLimits = parseBookingLimit(eventType?.bookingLimits); + const durationLimits = parseDurationLimit(eventType?.durationLimits); + + const busyTimesFromLimits = + eventType && (bookingLimits || durationLimits) + ? await getBusyTimesFromLimits( + bookingLimits, + durationLimits, + dateFrom, + dateTo, + duration, + eventType, + user.id + ) + : []; + + // TODO: only query what we need after applying limits (shrink date range) + const getBusyTimesStart = dateFrom.toISOString(); + const getBusyTimesEnd = dateTo.toISOString(); + const busyTimes = await getBusyTimes({ credentials: user.credentials, - // needed to correctly apply limits (weeks can be part of two months) - startTime: dateFrom.startOf("week").toISOString(), - endTime: dateTo.endOf("week").toISOString(), + startTime: getBusyTimesStart, + endTime: getBusyTimesEnd, eventTypeId, userId: user.id, username: `${user.username}`, @@ -161,40 +186,16 @@ export async function getUserAvailability( seatedEvent: !!eventType?.seatsPerTimeSlot, }); - let bufferedBusyTimes: EventBusyDetails[] = busyTimes.map((a) => ({ - ...a, - start: dayjs(a.start).toISOString(), - end: dayjs(a.end).toISOString(), - title: a.title, - source: query.withSource ? a.source : undefined, - })); - - const bookings = busyTimes.filter((busyTime) => busyTime.source?.startsWith(`eventType-${eventType?.id}`)); - - const bookingLimits = parseBookingLimit(eventType?.bookingLimits); - if (bookingLimits) { - const bookingBusyTimes = await getBusyTimesFromBookingLimits( - bookings, - bookingLimits, - dateFrom, - dateTo, - eventType - ); - bufferedBusyTimes = bufferedBusyTimes.concat(bookingBusyTimes); - } - - const durationLimits = parseDurationLimit(eventType?.durationLimits); - if (durationLimits) { - const durationBusyTimes = await getBusyTimesFromDurationLimits( - bookings, - durationLimits, - dateFrom, - dateTo, - duration, - eventType - ); - bufferedBusyTimes = bufferedBusyTimes.concat(durationBusyTimes); - } + const detailedBusyTimes: EventBusyDetails[] = [ + ...busyTimes.map((a) => ({ + ...a, + start: dayjs(a.start).toISOString(), + end: dayjs(a.end).toISOString(), + title: a.title, + source: query.withSource ? a.source : undefined, + })), + ...busyTimesFromLimits, + ]; const userSchedule = user.schedules.filter( (schedule) => !user?.defaultScheduleId || schedule.id === user?.defaultScheduleId @@ -239,22 +240,22 @@ export async function getUserAvailability( timeZone, }); - const formattedBusyTimes = bufferedBusyTimes.map((busy) => ({ + const formattedBusyTimes = detailedBusyTimes.map((busy) => ({ start: dayjs(busy.start), end: dayjs(busy.end), })); return { - busy: bufferedBusyTimes, + busy: detailedBusyTimes, timeZone, dateRanges: subtract(dateRanges, formattedBusyTimes), workingHours, dateOverrides, currentSeats, }; -} +}; -const getDatesBetween = (dateFrom: Dayjs, dateTo: Dayjs, period: "day" | "week" | "month" | "year") => { +const getPeriodStartDatesBetween = (dateFrom: Dayjs, dateTo: Dayjs, period: IntervalLimitUnit) => { const dates = []; let startDate = dayjs(dateFrom).startOf(period); const endDate = dayjs(dateTo).endOf(period); @@ -265,65 +266,191 @@ const getDatesBetween = (dateFrom: Dayjs, dateTo: Dayjs, period: "day" | "week" return dates; }; +type BusyMapKey = `${IntervalLimitUnit}-${ReturnType}`; + +/** + * Helps create, check, and return busy times from limits (with parallel support) + */ +class LimitManager { + private busyMap: Map = new Map(); + + /** + * Creates a busy map key + */ + private static createKey(start: Dayjs, unit: IntervalLimitUnit): BusyMapKey { + return `${unit}-${start.startOf(unit).toISOString()}`; + } + + /** + * Checks if already marked busy by ancestors or siblings + */ + isAlreadyBusy(start: Dayjs, unit: IntervalLimitUnit) { + if (this.busyMap.has(LimitManager.createKey(start, "year"))) return true; + + if (unit === "month" && this.busyMap.has(LimitManager.createKey(start, "month"))) { + return true; + } else if ( + unit === "week" && + // weeks can be part of two months + ((this.busyMap.has(LimitManager.createKey(start, "month")) && + this.busyMap.has(LimitManager.createKey(start.endOf("week"), "month"))) || + this.busyMap.has(LimitManager.createKey(start, "week"))) + ) { + return true; + } else if ( + unit === "day" && + (this.busyMap.has(LimitManager.createKey(start, "month")) || + this.busyMap.has(LimitManager.createKey(start, "week")) || + this.busyMap.has(LimitManager.createKey(start, "day"))) + ) { + return true; + } else { + return false; + } + } + + /** + * Adds a new busy time + */ + addBusyTime(start: Dayjs, unit: IntervalLimitUnit) { + this.busyMap.set(`${unit}-${start.toISOString()}`, { + start: start.toISOString(), + end: start.endOf(unit).toISOString(), + }); + } + + /** + * Returns all busy times + */ + getBusyTimes() { + return Array.from(this.busyMap.values()); + } +} + +const getBusyTimesFromLimits = async ( + bookingLimits: IntervalLimit | null, + durationLimits: IntervalLimit | null, + dateFrom: Dayjs, + dateTo: Dayjs, + duration: number | undefined, + eventType: NonNullable, + userId: number +) => { + performance.mark("limitsStart"); + + // shared amongst limiters to prevent processing known busy periods + const limitManager = new LimitManager(); + + let limitDateFrom = dayjs(dateFrom); + let limitDateTo = dayjs(dateTo); + + // expand date ranges by absolute minimum required to apply limits + // (yearly limits are handled separately for performance) + for (const key of ["PER_MONTH", "PER_WEEK", "PER_DAY"] as Exclude[]) { + if (bookingLimits?.[key] || durationLimits?.[key]) { + const unit = intervalLimitKeyToUnit(key); + limitDateFrom = dayjs.min(limitDateFrom, dateFrom.startOf(unit)); + limitDateTo = dayjs.max(limitDateTo, dateTo.endOf(unit)); + } + } + + // fetch only the data we need to check limits + const bookings = await getBusyTimesForLimitChecks({ + userId, + eventTypeId: eventType.id, + startDate: limitDateFrom.toDate(), + endDate: limitDateTo.toDate(), + }); + + // run this first, as counting bookings should always run faster.. + if (bookingLimits) { + performance.mark("bookingLimitsStart"); + await getBusyTimesFromBookingLimits( + bookings, + bookingLimits, + dateFrom, + dateTo, + eventType.id, + limitManager + ); + performance.mark("bookingLimitsEnd"); + performance.measure(`checking booking limits took $1'`, "bookingLimitsStart", "bookingLimitsEnd"); + } + + // ..than adding up durations (especially for the whole year) + if (durationLimits) { + performance.mark("durationLimitsStart"); + await getBusyTimesFromDurationLimits( + bookings, + durationLimits, + dateFrom, + dateTo, + duration, + eventType, + limitManager + ); + performance.mark("durationLimitsEnd"); + performance.measure(`checking duration limits took $1'`, "durationLimitsStart", "durationLimitsEnd"); + } + + performance.mark("limitsEnd"); + performance.measure(`checking all limits took $1'`, "limitsStart", "limitsEnd"); + + return limitManager.getBusyTimes(); +}; + const getBusyTimesFromBookingLimits = async ( bookings: EventBusyDetails[], bookingLimits: IntervalLimit, dateFrom: Dayjs, dateTo: Dayjs, - eventType: EventType | undefined + eventTypeId: number, + limitManager: LimitManager ) => { - const busyTimes: EventBusyDetails[] = []; + for (const key of descendingLimitKeys) { + const limit = bookingLimits?.[key]; + if (!limit) continue; - // Apply booking limit filter against our bookings - for (const [key, limit] of Object.entries(bookingLimits)) { - const limitKey = key as keyof IntervalLimit; + const unit = intervalLimitKeyToUnit(key); + const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit); - if (limitKey === "PER_YEAR") { - const yearlyBusyTime = await checkBookingLimit({ - eventStartDate: dateFrom.toDate(), - limitingNumber: limit, - eventId: eventType?.id as number, - key: "PER_YEAR", - returnBusyTimes: true, - }); - if (!yearlyBusyTime) break; - busyTimes.push({ - start: yearlyBusyTime.start.toISOString(), - end: yearlyBusyTime.end.toISOString(), - }); - break; - } + for (const periodStart of periodStartDates) { + if (limitManager.isAlreadyBusy(periodStart, unit)) continue; - // Take PER_DAY and turn it into day and PER_WEEK into week etc. - const filter = key.split("_")[1].toLowerCase() as "day" | "week" | "month" | "year"; - const dates = getDatesBetween(dateFrom, dateTo, filter); + // special handling of yearly limits to improve performance + if (unit === "year") { + try { + await checkBookingLimit({ + eventStartDate: periodStart.toDate(), + limitingNumber: limit, + eventId: eventTypeId, + key, + }); + } catch (_) { + limitManager.addBusyTime(periodStart, unit); + if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit))) { + return; + } + } + continue; + } + + const periodEnd = periodStart.endOf(unit); + let totalBookings = 0; - // loop through all dates and check if we have reached the limit - for (const date of dates) { - let total = 0; - const startDate = date.startOf(filter); - // this is parsed above with parseBookingLimit so we know it's safe. - const endDate = date.endOf(filter); for (const booking of bookings) { - 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 - !(bookingEventTypeId == eventType?.id && dayjs(booking.start).isBetween(startDate, endDate)) - ) { + // consider booking part of period independent of end date + if (!dayjs(booking.start).isBetween(periodStart, periodEnd)) { continue; } - // increment total and check against the limit, adding a busy time if condition is met. - total++; - if (total >= limit) { - busyTimes.push({ start: startDate.toISOString(), end: endDate.toISOString() }); + totalBookings++; + if (totalBookings >= limit) { + limitManager.addBusyTime(periodStart, unit); break; } } } } - - return busyTimes; }; const getBusyTimesFromDurationLimits = async ( @@ -332,60 +459,56 @@ const getBusyTimesFromDurationLimits = async ( dateFrom: Dayjs, dateTo: Dayjs, duration: number | undefined, - eventType: EventType | undefined + eventType: NonNullable, + limitManager: LimitManager ) => { - const busyTimes: EventBusyDetails[] = []; - // Start check from larger time periods to smaller time periods, to skip unnecessary checks - for (const [key, limit] of Object.entries(durationLimits).reverse()) { - // Use aggregate sql query if we are checking PER_YEAR - if (key === "PER_YEAR") { - const totalBookingDuration = await getTotalBookingDuration({ - eventId: eventType?.id as number, - startDate: dateFrom.startOf("year").toDate(), - endDate: dateFrom.endOf("year").toDate(), - }); - if (totalBookingDuration + (duration ?? 0) > limit) { - busyTimes.push({ - start: dateFrom.startOf("year").toISOString(), - end: dateFrom.endOf("year").toISOString(), - }); - } - continue; - } + for (const key of descendingLimitKeys) { + const limit = durationLimits?.[key]; + if (!limit) continue; - const filter = key.split("_")[1].toLowerCase() as "day" | "week" | "month" | "year"; - const dates = getDatesBetween(dateFrom, dateTo, filter); + const unit = intervalLimitKeyToUnit(key); + const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit); - // loop through all dates and check if we have reached the limit - for (const date of dates) { - let total = (duration || eventType?.length) ?? 0; - const startDate = date.startOf(filter); - const endDate = date.endOf(filter); + for (const periodStart of periodStartDates) { + if (limitManager.isAlreadyBusy(periodStart, unit)) continue; - // add busy time if we have already reached the limit with just the selected duration - if (total > limit) { - busyTimes.push({ start: startDate.toISOString(), end: endDate.toISOString() }); + const selectedDuration = (duration || eventType.length) ?? 0; + + if (selectedDuration > limit) { + limitManager.addBusyTime(periodStart, unit); continue; } + // special handling of yearly limits to improve performance + if (unit === "year") { + const totalYearlyDuration = await getTotalBookingDuration({ + eventId: eventType.id, + startDate: periodStart.toDate(), + endDate: periodStart.endOf(unit).toDate(), + }); + if (totalYearlyDuration + selectedDuration > limit) { + limitManager.addBusyTime(periodStart, unit); + if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit))) { + return; + } + } + continue; + } + + const periodEnd = periodStart.endOf(unit); + let totalDuration = selectedDuration; + for (const booking of bookings) { - 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 - !(bookingEventTypeId == eventType?.id && dayjs(booking.start).isBetween(startDate, endDate)) - ) { + // consider booking part of period independent of end date + if (!dayjs(booking.start).isBetween(periodStart, periodEnd)) { continue; } - // Add current booking duration to total and check against the limit, adding a busy time if condition is met. - total += dayjs(booking.end).diff(dayjs(booking.start), "minute"); - if (total > limit) { - busyTimes.push({ start: startDate.toISOString(), end: endDate.toISOString() }); + totalDuration += dayjs(booking.end).diff(dayjs(booking.start), "minute"); + if (totalDuration > limit) { + limitManager.addBusyTime(periodStart, unit); break; } } } } - - return busyTimes; }; diff --git a/packages/lib/intervalLimit.ts b/packages/lib/intervalLimit.ts new file mode 100644 index 0000000000..132293e105 --- /dev/null +++ b/packages/lib/intervalLimit.ts @@ -0,0 +1,12 @@ +import type { IntervalLimit, IntervalLimitUnit } from "@calcom/types/Calendar"; + +export const ascendingLimitKeys: (keyof IntervalLimit)[] = ["PER_DAY", "PER_WEEK", "PER_MONTH", "PER_YEAR"]; + +export const descendingLimitKeys = [...ascendingLimitKeys].reverse(); + +/** + * Turns `PER_DAY` into `day`, `PER_WEEK` into `week` etc. + */ +export function intervalLimitKeyToUnit(key: keyof IntervalLimit) { + return key.split("_")[1].toLowerCase() as IntervalLimitUnit; +} diff --git a/packages/lib/server/checkBookingLimits.ts b/packages/lib/server/checkBookingLimits.ts index b3352c330d..0778ad92b2 100644 --- a/packages/lib/server/checkBookingLimits.ts +++ b/packages/lib/server/checkBookingLimits.ts @@ -1,34 +1,31 @@ import dayjs from "@calcom/dayjs"; import prisma from "@calcom/prisma"; +import { BookingStatus } from "@calcom/prisma/enums"; import type { IntervalLimit } from "@calcom/types/Calendar"; +import { getErrorFromUnknown } from "../errors"; import { HttpError } from "../http-error"; +import { ascendingLimitKeys, intervalLimitKeyToUnit } from "../intervalLimit"; import { parseBookingLimit } from "../isBookingLimits"; export async function checkBookingLimits( bookingLimits: IntervalLimit, eventStartDate: Date, - eventId: number, - returnBusyTimes?: boolean + eventId: number ) { const parsedBookingLimits = parseBookingLimit(bookingLimits); - if (parsedBookingLimits) { - const limitCalculations = Object.entries(parsedBookingLimits).map( - async ([key, limitingNumber]) => - await checkBookingLimit({ key, limitingNumber, eventStartDate, eventId, returnBusyTimes }) - ); - await Promise.all(limitCalculations) - .then((res) => { - if (returnBusyTimes) { - return res; - } - }) - .catch((error) => { - throw new HttpError({ message: error.message, statusCode: 401 }); - }); - return true; + if (!parsedBookingLimits) return false; + + // not iterating entries to preserve types + const limitCalculations = ascendingLimitKeys.map((key) => + checkBookingLimit({ key, limitingNumber: parsedBookingLimits[key], eventStartDate, eventId }) + ); + + try { + return !!(await Promise.all(limitCalculations)); + } catch (error) { + throw new HttpError({ message: getErrorFromUnknown(error).message, statusCode: 401 }); } - return false; } export async function checkBookingLimit({ @@ -36,59 +33,39 @@ export async function checkBookingLimit({ eventId, key, limitingNumber, - returnBusyTimes = false, }: { eventStartDate: Date; eventId: number; - key: string; - limitingNumber: number; - returnBusyTimes?: boolean; + key: keyof IntervalLimit; + limitingNumber: number | undefined; }) { { - const limitKey = key as keyof IntervalLimit; - // 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"; // Have to cast here - const startDate = dayjs(eventStartDate).startOf(filter).toDate(); - // this is parsed above with parseBookingLimit so we know it's safe. + if (!limitingNumber) return; - const endDate = dayjs(startDate).endOf(filter).toDate(); + const unit = intervalLimitKeyToUnit(key); + + const startDate = dayjs(eventStartDate).startOf(unit).toDate(); + const endDate = dayjs(eventStartDate).endOf(unit).toDate(); - // This allows us to easily add it within dayjs const bookingsInPeriod = await prisma.booking.count({ where: { - status: "ACCEPTED", - eventType: { - AND: [ - { - id: eventId, - bookings: { - every: { - startTime: { - gte: startDate, - }, - endTime: { - lte: endDate, - }, - }, - }, - }, - ], + status: BookingStatus.ACCEPTED, + eventTypeId: eventId, + // FIXME: bookings that overlap on one side will never be counted + startTime: { + gte: startDate, + }, + endTime: { + lte: endDate, }, }, }); - if (bookingsInPeriod >= limitingNumber) { - // This is used when getting availability - if (returnBusyTimes) { - return { - start: startDate, - end: endDate, - }; - } - throw new HttpError({ - message: `booking_limit_reached`, - statusCode: 403, - }); - } + if (bookingsInPeriod < limitingNumber) return; + + throw new HttpError({ + message: `booking_limit_reached`, + statusCode: 403, + }); } } diff --git a/packages/lib/server/checkDurationLimits.ts b/packages/lib/server/checkDurationLimits.ts index 98a2849321..643a06e355 100644 --- a/packages/lib/server/checkDurationLimits.ts +++ b/packages/lib/server/checkDurationLimits.ts @@ -1,7 +1,9 @@ import dayjs from "@calcom/dayjs"; import type { IntervalLimit } from "@calcom/types/Calendar"; +import { getErrorFromUnknown } from "../errors"; import { HttpError } from "../http-error"; +import { ascendingLimitKeys, intervalLimitKeyToUnit } from "../intervalLimit"; import { parseDurationLimit } from "../isDurationLimits"; import { getTotalBookingDuration } from "./queries"; @@ -11,20 +13,18 @@ export async function checkDurationLimits( eventId: number ) { const parsedDurationLimits = parseDurationLimit(durationLimits); - if (!parsedDurationLimits) { - return false; - } + if (!parsedDurationLimits) return false; - const limitCalculations = Object.entries(parsedDurationLimits).map( - async ([key, limitingNumber]) => - await checkDurationLimit({ key, limitingNumber, eventStartDate, eventId }) + // not iterating entries to preserve types + const limitCalculations = ascendingLimitKeys.map((key) => + checkDurationLimit({ key, limitingNumber: parsedDurationLimits[key], eventStartDate, eventId }) ); - await Promise.all(limitCalculations).catch((error) => { - throw new HttpError({ message: error.message, statusCode: 401 }); - }); - - return true; + try { + return !!(await Promise.all(limitCalculations)); + } catch (error) { + throw new HttpError({ message: getErrorFromUnknown(error).message, statusCode: 401 }); + } } export async function checkDurationLimit({ @@ -32,34 +32,27 @@ export async function checkDurationLimit({ eventId, key, limitingNumber, - returnBusyTimes = false, }: { eventStartDate: Date; eventId: number; - key: string; - limitingNumber: number; - returnBusyTimes?: boolean; + key: keyof IntervalLimit; + limitingNumber: number | undefined; }) { { - // Take PER_DAY and turn it into day and PER_WEEK into week etc. - const filter = key.split("_")[1].toLocaleLowerCase() as "day" | "week" | "month" | "year"; - const startDate = dayjs(eventStartDate).startOf(filter).toDate(); - const endDate = dayjs(startDate).endOf(filter).toDate(); + if (!limitingNumber) return; + + const unit = intervalLimitKeyToUnit(key); + + const startDate = dayjs(eventStartDate).startOf(unit).toDate(); + const endDate = dayjs(eventStartDate).endOf(unit).toDate(); const totalBookingDuration = await getTotalBookingDuration({ eventId, startDate, endDate }); - if (totalBookingDuration >= limitingNumber) { - // This is used when getting availability - if (returnBusyTimes) { - return { - start: startDate, - end: endDate, - }; - } - throw new HttpError({ - message: `duration_limit_reached`, - statusCode: 403, - }); - } + if (totalBookingDuration < limitingNumber) return; + + throw new HttpError({ + message: `duration_limit_reached`, + statusCode: 403, + }); } } diff --git a/packages/lib/server/queries/booking/index.ts b/packages/lib/server/queries/booking/index.ts index 11ce110fcb..990ca9d7d5 100644 --- a/packages/lib/server/queries/booking/index.ts +++ b/packages/lib/server/queries/booking/index.ts @@ -10,14 +10,15 @@ export const getTotalBookingDuration = async ({ endDate: Date; }) => { // Aggregates the total booking time for a given event in a given time period - const [totalBookingTime] = (await prisma.$queryRaw` + // FIXME: bookings that overlap on one side will never be counted + const [totalBookingTime] = await prisma.$queryRaw<[{ totalMinutes: number | null }]>` SELECT SUM(EXTRACT(EPOCH FROM ("endTime" - "startTime")) / 60) as "totalMinutes" FROM "Booking" WHERE "status" = 'accepted' - AND "id" = ${eventId} + AND "eventTypeId" = ${eventId} AND "startTime" >= ${startDate} AND "endTime" <= ${endDate}; - `) as { totalMinutes: number }[]; + `; - return totalBookingTime.totalMinutes; + return totalBookingTime.totalMinutes ?? 0; }; diff --git a/packages/lib/validateIntervalLimitOrder.ts b/packages/lib/validateIntervalLimitOrder.ts index 73a066e6c1..ee92f3b054 100644 --- a/packages/lib/validateIntervalLimitOrder.ts +++ b/packages/lib/validateIntervalLimitOrder.ts @@ -1,6 +1,7 @@ import type { IntervalLimit } from "@calcom/types/Calendar"; -const validationOrderKeys = ["PER_DAY", "PER_WEEK", "PER_MONTH", "PER_YEAR"]; +import { ascendingLimitKeys } from "./intervalLimit"; + export const validateIntervalLimitOrder = (input: IntervalLimit) => { // Sort limits by validationOrder const sorted = Object.entries(input) @@ -9,7 +10,7 @@ export const validateIntervalLimitOrder = (input: IntervalLimit) => { }) .map(([key]) => key); - const validationOrderWithoutMissing = validationOrderKeys.filter((key) => sorted.includes(key)); + const validationOrderWithoutMissing = ascendingLimitKeys.filter((key) => sorted.includes(key)); return sorted.every((key, index) => validationOrderWithoutMissing[index] === key); }; diff --git a/packages/types/Calendar.d.ts b/packages/types/Calendar.d.ts index b92196cacb..a7e3ba70a9 100644 --- a/packages/types/Calendar.d.ts +++ b/packages/types/Calendar.d.ts @@ -120,12 +120,9 @@ export interface RecurringEvent { tzid?: string | undefined; } -export interface IntervalLimit { - PER_DAY?: number | undefined; - PER_WEEK?: number | undefined; - PER_MONTH?: number | undefined; - PER_YEAR?: number | undefined; -} +export type IntervalLimitUnit = "day" | "week" | "month" | "year"; + +export type IntervalLimit = Partial}`, number | undefined>>; export type AppsStatus = { appName: string;