fix: Booking Error - cannot be booked at this time (#15288)

Co-authored-by: Joe Au-Yeung <65426560+joeauyeung@users.noreply.github.com>
This commit is contained in:
Hariom Balhara
2024-06-04 15:09:57 +00:00
committed by GitHub
co-authored by Joe Au-Yeung
parent 3a22fdb428
commit 765d601ff9
5 changed files with 1345 additions and 89 deletions
+27 -6
View File
@@ -37,7 +37,10 @@ declare global {
namespace jest {
interface Matchers<R> {
toHaveTimeSlots(expectedSlots: string[], date: { dateString: string; doExactMatch?: boolean }): R;
toHaveNoTimeSlots(date: { dateString: string }): R;
/**
* Explicitly checks if the date is disabled and fails if date is marked as OOO
*/
toHaveDateDisabled(date: { dateString: string }): R;
}
}
}
@@ -55,18 +58,28 @@ expect.extend({
};
}
const expectedSlotHasFullTimestamp = expectedSlots[0].split("-").length === 3;
if (
!schedule.slots[`${dateString}`]
.map((slot) => slot.time)
.every((actualSlotTime, index) => {
return `${dateString}T${expectedSlots[index]}` === actualSlotTime;
const expectedSlotTime = expectedSlotHasFullTimestamp
? expectedSlots[index]
: `${dateString}T${expectedSlots[index]}`;
return expectedSlotTime === actualSlotTime;
})
) {
return {
pass: false,
message: () =>
`has incorrect timeslots for ${dateString}.\n\r ${diff(
expectedSlots.map((expectedSlot) => `${dateString}T${expectedSlot}`),
expectedSlots.map((expectedSlot) => {
if (expectedSlotHasFullTimestamp) {
return expectedSlot;
}
return `${dateString}T${expectedSlot}`;
}),
schedule.slots[`${dateString}`].map((slot) => slot.time)
)}`,
};
@@ -88,11 +101,19 @@ expect.extend({
};
},
toHaveNoTimeSlots(schedule: { slots: Record<string, Slot[]> }, { dateString }: { dateString: string }) {
if (!schedule.slots[`${dateString}`] || schedule.slots[`${dateString}`].length === 0) {
toHaveDateDisabled(schedule: { slots: Record<string, Slot[]> }, { dateString }: { dateString: string }) {
// Frontend requires that the date must not be set for that date to be shown as disabled.Because weirdly, if an empty array is provided the date itself isn't shown which we don't want
if (!schedule.slots[`${dateString}`]) {
return {
pass: true,
message: () => `has no timeslots for ${dateString}`,
message: () => `is not disabled for ${dateString}`,
};
}
if (schedule.slots[`${dateString}`].length === 0) {
return {
pass: false,
message: () => `is all day OOO for ${dateString}.`,
};
}
return {
File diff suppressed because it is too large Load Diff
@@ -166,6 +166,7 @@ type InputBooking = Partial<Omit<Booking, keyof WhiteListedBookingProps>> & Whit
export const Timezones = {
"+5:30": "Asia/Kolkata",
"+6:00": "Asia/Dhaka",
"-11:00": "Pacific/Pago_Pago",
};
async function addHostsToDb(eventTypes: InputEventType[]) {
@@ -928,6 +929,21 @@ export const TestData = {
],
timeZone: Timezones["+5:30"],
}),
IstWorkHoursNoWeekends: {
id: 1,
name: "9:30AM to 6PM in India - 4:00AM to 12:30PM in GMT",
availability: [
{
// userId: null,
// eventTypeId: null,
days: [/*0*/ 1, 2, 3, 4, 5 /*6*/],
startTime: new Date("1970-01-01T09:30:00.000Z"),
endTime: new Date("1970-01-01T18:00:00.000Z"),
date: null,
},
],
timeZone: Timezones["+5:30"],
},
},
users: {
example: {
@@ -1669,3 +1685,9 @@ export const getMockFailingAppStatus = ({ slug }: { slug: string }) => {
export const getMockPassingAppStatus = ({ slug, overrideName }: { slug: string; overrideName?: string }) => {
return getMockAppStatus({ slug, overrideName, failures: 0, success: 1 });
};
export const replaceDates = (dates: string[], replacement: Record<string, string>) => {
return dates.map((date) => {
return date.replace(/(.*)T/, (_, group1) => `${replacement[group1]}T`);
});
};
+25 -27
View File
@@ -51,13 +51,25 @@ export function calculatePeriodLimits({
}): PeriodLimits {
const currentTime = dayjs().utcOffset(utcOffset);
periodDays = periodDays || 0;
const log = logger.getSubLogger({ prefix: ["calculatePeriodLimits"] });
log.debug(
safeStringify({
periodType,
periodDays,
periodCountCalendarDays,
periodStartDate: periodStartDate,
periodEndDate: periodEndDate,
currentTime: currentTime.format(),
})
);
switch (periodType) {
case PeriodType.ROLLING: {
const rollingEndDay = periodCountCalendarDays
? currentTime.add(periodDays, "days").endOf("day")
: currentTime.businessDaysAdd(periodDays).endOf("day");
return { rollingEndDay, rangeStartDay: null, rangeEndDay: null };
? currentTime.add(periodDays, "days")
: currentTime.businessDaysAdd(periodDays);
// The future limit talks in terms of days so we take the end of the day here to consider the entire day
return { rollingEndDay: rollingEndDay.endOf("day"), rangeStartDay: null, rangeEndDay: null };
}
case PeriodType.ROLLING_WINDOW: {
@@ -80,6 +92,7 @@ export function calculatePeriodLimits({
}
case PeriodType.RANGE: {
// The future limit talks in terms of days so we take the start of the day for starting range and endOf the day for ending range
const rangeStartDay = dayjs(periodStartDate).utcOffset(utcOffset).startOf("day");
const rangeEndDay = dayjs(periodEndDate).utcOffset(utcOffset).endOf("day");
@@ -159,7 +172,7 @@ export function getRollingWindowEndDate({
const rollingEndDayOrLastPossibleDayAsPerLimit = rollingEndDay ?? currentDate;
log.debug("Returning rollingEndDay", rollingEndDayOrLastPossibleDayAsPerLimit.format());
// Return endOfDay so that any timeslot in the last day is considered within bounds
// The future limit talks in terms of days so we take the end of the day here to consider the entire day
return rollingEndDayOrLastPossibleDayAsPerLimit.endOf("day");
}
@@ -198,34 +211,19 @@ type PeriodLimits = {
* To be used when we work on just Dates(and not specific timeslots) to check boundaries
* e.g. It checks for Future Limits which operate on dates and not times.
*/
export function isDateOutOfBounds({
dateString,
export function isTimeViolatingFutureLimit({
time,
periodLimits,
}: {
dateString: dayjs.ConfigType;
time: dayjs.ConfigType;
periodLimits: PeriodLimits;
_skipRollingWindowCheck?: boolean;
}) {
const log = logger.getSubLogger({ prefix: ["isDateOutOfBounds"] });
const date = dayjs(dateString);
log.debug(
safeStringify({
dateString,
endOfDay: date.format(),
periodLimits: {
rollingEndDay: periodLimits.rollingEndDay?.format(),
rangeStartDay: periodLimits.rangeStartDay?.format(),
rangeEndDay: periodLimits.rangeEndDay?.format(),
},
})
);
const log = logger.getSubLogger({ prefix: ["isTimeViolatingFutureLimit"] });
const date = dayjs(time);
if (periodLimits.rollingEndDay) {
const isAfterRollingEndDay = date.isAfter(periodLimits.rollingEndDay);
log.debug({
dateString,
endOfDay: date.format(),
formattedDate: date.format(),
isAfterRollingEndDay,
rollingEndDay: periodLimits.rollingEndDay.format(),
});
@@ -257,8 +255,8 @@ export default function isOutOfBounds(
) {
return (
isTimeOutOfBounds({ time, minimumBookingNotice }) ||
isDateOutOfBounds({
dateString: time,
isTimeViolatingFutureLimit({
time,
periodLimits: calculatePeriodLimits({
periodType,
periodDays,
@@ -16,7 +16,11 @@ import { parseBookingLimit, parseDurationLimit } from "@calcom/lib";
import { RESERVED_SUBDOMAINS } from "@calcom/lib/constants";
import { getUTCOffsetByTimezone } from "@calcom/lib/date-fns";
import { getDefaultEvent } from "@calcom/lib/defaultEvents";
import { isDateOutOfBounds, isTimeOutOfBounds, calculatePeriodLimits } from "@calcom/lib/isOutOfBounds";
import {
isTimeOutOfBounds,
calculatePeriodLimits,
isTimeViolatingFutureLimit,
} from "@calcom/lib/isOutOfBounds";
import logger from "@calcom/lib/logger";
import { safeStringify } from "@calcom/lib/safeStringify";
import { performance } from "@calcom/lib/server/perfObserver";
@@ -761,6 +765,7 @@ export async function getAvailableSlots({ input, ctx }: GetScheduleOptions): Pro
const allDatesWithBookabilityStatus = getAllDatesWithBookabilityStatus(availableDates);
loggerWithEventDetails.debug(safeStringify({ availableDates }));
const utcOffset = input.timeZone ? getUTCOffsetByTimezone(input.timeZone) ?? 0 : 0;
const periodLimits = calculatePeriodLimits({
periodType: eventType.periodType,
periodDays: eventType.periodDays,
@@ -768,27 +773,37 @@ export async function getAvailableSlots({ input, ctx }: GetScheduleOptions): Pro
periodStartDate: eventType.periodStartDate,
periodEndDate: eventType.periodEndDate,
allDatesWithBookabilityStatus,
utcOffset: input.timeZone ? getUTCOffsetByTimezone(input.timeZone) ?? 0 : 0,
utcOffset,
});
let foundALimitViolation = false;
let foundAFutureLimitViolation = false;
const withinBoundsSlotsMappedToDate = Object.entries(slotsMappedToDate).reduce(
(withinBoundsSlotsMappedToDate, [date, slots]) => {
// Computation Optimization: If a limit violation has been found, we just consider all slots to be out of bounds beyond that date.
// Computation Optimization: If a future limit violation has been found, we just consider all slots to be out of bounds beyond that slot.
// We can't do the same for periodType=RANGE because it can start from a day other than today and today will hit the violation then.
if (foundALimitViolation && doesRangeStartFromToday(eventType.periodType)) {
if (foundAFutureLimitViolation && doesRangeStartFromToday(eventType.periodType)) {
return withinBoundsSlotsMappedToDate;
}
const isDateWithinBound = !isDateOutOfBounds({ dateString: date, periodLimits });
if (isDateWithinBound) {
// TODO: Slots calculation logic already seems to consider the minimum booking notice and past booking time and thus there shouldn't be need to filter out slots here.
withinBoundsSlotsMappedToDate[date] = slots.filter(
(slot) =>
!isTimeOutOfBounds({ time: slot.time, minimumBookingNotice: eventType.minimumBookingNotice })
const filteredSlots = slots.filter((slot) => {
const isFutureLimitViolationForTheSlot = isTimeViolatingFutureLimit({
time: slot.time,
periodLimits,
});
if (isFutureLimitViolationForTheSlot) {
foundAFutureLimitViolation = true;
}
return (
!isFutureLimitViolationForTheSlot &&
// TODO: Perf Optmization: Slots calculation logic already seems to consider the minimum booking notice and past booking time and thus there shouldn't be need to filter out slots here.
!isTimeOutOfBounds({ time: slot.time, minimumBookingNotice: eventType.minimumBookingNotice })
);
} else {
foundALimitViolation = true;
});
if (!filteredSlots.length) {
// If there are no slots available, we don't set that date, otherwise having an empty slots array makes frontend consider it as an all day OOO case
return withinBoundsSlotsMappedToDate;
}
withinBoundsSlotsMappedToDate[date] = filteredSlots;
return withinBoundsSlotsMappedToDate;
},
{} as typeof slotsMappedToDate