From 0e0a78c47e576ab6e98a36071fa5d8b44d474edc Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Thu, 22 Dec 2022 01:02:42 +0530 Subject: [PATCH] Fix/Round Robin (#6121) * Identify owner for a timeslot and check busyness against his schedule only * Fix TS errors * Fix flatmap * Fix xisting unit tests * Prevent duplicate slots by merging, userId -> userIds (multi) * Small fix to potential undefined * Moved duplicate prevention to buildSlots function * Apply date override on a per user basis * Prevent -1 being added to computedLocalAvailability * Removed console.log * Apply override properly on COLLECTIVE * Default timeZone to UTC when undefined * isSame doesn't work when the day shifts, isBetween instead * Iterate over all slot.userIds schedules to identify the first user schedule that matches * add round-robin test Co-authored-by: Alex van Andel Co-authored-by: Peer Richelsen --- .../web/components/booking/AvailableTimes.tsx | 3 +- .../test/lib/getAggregateWorkingHours.test.ts | 64 ++++++++++++- apps/web/test/lib/slots.test.ts | 5 + packages/core/getUserAvailability.ts | 12 ++- .../components/TeamAvailabilityTimes.tsx | 12 +-- .../components/v2/TeamAvailabilityTimes.tsx | 12 +-- packages/lib/availability.ts | 20 ++-- packages/lib/slots.ts | 86 +++++++++++------ packages/trpc/server/routers/viewer/slots.tsx | 92 ++++++++++++------- packages/types/schedule.d.ts | 2 + 10 files changed, 221 insertions(+), 87 deletions(-) diff --git a/apps/web/components/booking/AvailableTimes.tsx b/apps/web/components/booking/AvailableTimes.tsx index 9cf5f51d96..9de08cde1b 100644 --- a/apps/web/components/booking/AvailableTimes.tsx +++ b/apps/web/components/booking/AvailableTimes.tsx @@ -101,7 +101,8 @@ const AvailableTimes: FC = ({ } return ( -
+
+ {/* ^ data-slot-owner is helpful in debugging and used to identify the owners of the slot. Owners are the users which have the timeslot in their schedule. It doesn't consider if a user has that timeslot booked */} {/* Current there is no way to disable Next.js Links */} {seatsPerTimeSlot && slot.attendees && slot.attendees >= seatsPerTimeSlot ? (
{ ], "endTime": 180, "startTime": 0, + "userId": 2, }, Object { "days": Array [ @@ -63,4 +64,57 @@ it("Sydney and Shiraz can live in harmony 🙏", async () => { }, ] `); + + expect(getAggregateWorkingHours(HAWAII_AND_NEWYORK_TEAM, "ROUND_ROBIN")).toMatchInlineSnapshot(` + Array [ + Object { + "days": Array [ + 1, + 2, + 3, + 4, + 5, + ], + "endTime": 1260, + "startTime": 780, + "userId": 1, + }, + Object { + "days": Array [ + 3, + 4, + 5, + ], + "endTime": 360, + "startTime": 0, + "userId": 1, + }, + Object { + "days": Array [ + 6, + ], + "endTime": 180, + "startTime": 0, + "userId": 2, + }, + Object { + "days": Array [ + 2, + 3, + 4, + ], + "endTime": 1439, + "startTime": 780, + "userId": 3, + }, + Object { + "days": Array [ + 5, + ], + "endTime": 1439, + "startTime": 780, + "userId": 4, + }, + ] + `); }); diff --git a/apps/web/test/lib/slots.test.ts b/apps/web/test/lib/slots.test.ts index c299d4b422..57f775e790 100644 --- a/apps/web/test/lib/slots.test.ts +++ b/apps/web/test/lib/slots.test.ts @@ -18,6 +18,7 @@ describe("Tests the slot logic", () => { minimumBookingNotice: 0, workingHours: [ { + userId: 1, days: Array.from(Array(7).keys()), startTime: MINUTES_DAY_START, endTime: MINUTES_DAY_END, @@ -38,6 +39,7 @@ describe("Tests the slot logic", () => { minimumBookingNotice: 0, workingHours: [ { + userId: 1, days: Array.from(Array(7).keys()), startTime: MINUTES_DAY_START, endTime: MINUTES_DAY_END, @@ -56,6 +58,7 @@ describe("Tests the slot logic", () => { minimumBookingNotice: 0, workingHours: [ { + userId: 1, days: [0], startTime: 23 * 60, // 23h endTime: MINUTES_DAY_END, @@ -69,6 +72,7 @@ describe("Tests the slot logic", () => { it("can cut off dates that due to invitee timezone differences fall on the previous day", async () => { const workingHours = [ { + userId: 1, days: [0], startTime: MINUTES_DAY_START, endTime: 1 * 60, // 1h @@ -94,6 +98,7 @@ describe("Tests the slot logic", () => { minimumBookingNotice: 1500, workingHours: [ { + userId: 1, days: Array.from(Array(7).keys()), startTime: MINUTES_DAY_START, endTime: MINUTES_DAY_END, diff --git a/packages/core/getUserAvailability.ts b/packages/core/getUserAvailability.ts index 2f419b71e5..9811516c6f 100644 --- a/packages/core/getUserAvailability.ts +++ b/packages/core/getUserAvailability.ts @@ -225,13 +225,19 @@ export async function getUserAvailability( } } + const userSchedule = currentUser.schedules.filter( + (schedule) => !currentUser.defaultScheduleId || schedule.id === currentUser.defaultScheduleId + )[0]; + const schedule = !eventType?.metadata?.config?.useHostSchedulesForTeamEvent && eventType?.schedule ? { ...eventType?.schedule } : { - ...currentUser.schedules.filter( - (schedule) => !currentUser.defaultScheduleId || schedule.id === currentUser.defaultScheduleId - )[0], + ...userSchedule, + availability: userSchedule.availability.map((a) => ({ + ...a, + userId: currentUser.id, + })), }; const startGetWorkingHours = performance.now(); diff --git a/packages/features/ee/teams/components/TeamAvailabilityTimes.tsx b/packages/features/ee/teams/components/TeamAvailabilityTimes.tsx index 711d369512..37619ea374 100644 --- a/packages/features/ee/teams/components/TeamAvailabilityTimes.tsx +++ b/packages/features/ee/teams/components/TeamAvailabilityTimes.tsx @@ -31,7 +31,7 @@ export default function TeamAvailabilityTimes(props: Props) { } ); - const times = !isLoading + const slots = !isLoading ? getSlots({ frequency: props.frequency, inviteeDate: props.selectedDate, @@ -44,18 +44,18 @@ export default function TeamAvailabilityTimes(props: Props) { return (
{props.HeaderComponent} - {isLoading && times.length === 0 && } - {!isLoading && times.length === 0 && ( + {isLoading && slots.length === 0 && } + {!isLoading && slots.length === 0 && (
No Available Slots
)} - {times.map((time) => ( -
+ {slots.map((slot) => ( + ))} diff --git a/packages/features/ee/teams/components/v2/TeamAvailabilityTimes.tsx b/packages/features/ee/teams/components/v2/TeamAvailabilityTimes.tsx index 3eb6b862c7..e7e7ae3d9f 100644 --- a/packages/features/ee/teams/components/v2/TeamAvailabilityTimes.tsx +++ b/packages/features/ee/teams/components/v2/TeamAvailabilityTimes.tsx @@ -35,7 +35,7 @@ export default function TeamAvailabilityTimes(props: Props) { } ); - const times = !isLoading + const slots = !isLoading ? getSlots({ frequency: props.frequency, inviteeDate: props.selectedDate, @@ -48,8 +48,8 @@ export default function TeamAvailabilityTimes(props: Props) { return (
{props.HeaderComponent} - {isLoading && times.length === 0 && } - {!isLoading && times.length === 0 ? ( + {isLoading && slots.length === 0 && } + {!isLoading && slots.length === 0 ? (
{t("no_available_slots")}
@@ -57,12 +57,12 @@ export default function TeamAvailabilityTimes(props: Props) { <>{!isLoading &&

{t("time_available")}

} )}
- {times.map((time) => ( -
+ {slots.map((slot) => ( + ))} diff --git a/packages/lib/availability.ts b/packages/lib/availability.ts index 7beab8349a..5d3a04fa72 100644 --- a/packages/lib/availability.ts +++ b/packages/lib/availability.ts @@ -63,7 +63,7 @@ export function getWorkingHours( timeZone?: string; utcOffset?: number; }, - availability: { days: number[]; startTime: ConfigType; endTime: ConfigType }[] + availability: { userId?: number | null; days: number[]; startTime: ConfigType; endTime: ConfigType }[] ) { if (!availability.length) { return []; @@ -90,28 +90,34 @@ export function getWorkingHours( return currentWorkingHours; } if (sameDayStartTime !== sameDayEndTime) { - currentWorkingHours.push({ + const newWorkingHours: WorkingHours = { days: schedule.days, startTime: sameDayStartTime, endTime: sameDayEndTime, - }); + }; + if (schedule.userId) newWorkingHours.userId = schedule.userId; + currentWorkingHours.push(newWorkingHours); } // check for overflow to the previous day // overflowing days constraint to 0-6 day range (Sunday-Saturday) if (startTime < MINUTES_DAY_START || endTime < MINUTES_DAY_START) { - currentWorkingHours.push({ + const newWorkingHours: WorkingHours = { days: schedule.days.map((day) => (day - 1 >= 0 ? day - 1 : 6)), startTime: startTime + MINUTES_IN_DAY, endTime: Math.min(endTime + MINUTES_IN_DAY, MINUTES_DAY_END), - }); + }; + if (schedule.userId) newWorkingHours.userId = schedule.userId; + currentWorkingHours.push(newWorkingHours); } // else, check for overflow in the next day else if (startTime > MINUTES_DAY_END || endTime > MINUTES_IN_DAY) { - currentWorkingHours.push({ + const newWorkingHours: WorkingHours = { days: schedule.days.map((day) => (day + 1) % 7), startTime: Math.max(startTime - MINUTES_IN_DAY, MINUTES_DAY_START), endTime: endTime - MINUTES_IN_DAY, - }); + }; + if (schedule.userId) newWorkingHours.userId = schedule.userId; + currentWorkingHours.push(newWorkingHours); } return currentWorkingHours; diff --git a/packages/lib/slots.ts b/packages/lib/slots.ts index 66df31328e..c9328a70cf 100644 --- a/packages/lib/slots.ts +++ b/packages/lib/slots.ts @@ -11,7 +11,7 @@ export type GetSlots = { minimumBookingNotice: number; eventLength: number; }; -export type TimeFrame = { startTime: number; endTime: number }; +export type TimeFrame = { userIds?: number[]; startTime: number; endTime: number }; const minimumOfOne = (input: number) => (input < 1 ? 1 : input); @@ -66,14 +66,21 @@ function buildSlots({ const slotsTimeFrameAvailable: TimeFrame[] = []; computedLocalAvailability.forEach((item) => { - slotsTimeFrameAvailable.push(...splitAvailableTime(item.startTime, item.endTime, frequency, eventLength)); + const userSlotsTimeFrameAvailable = splitAvailableTime( + item.startTime, + item.endTime, + frequency, + eventLength + ).map((slot) => ({ ...slot, userIds: item.userIds })); + + slotsTimeFrameAvailable.push(...userSlotsTimeFrameAvailable); }); - const slots: Dayjs[] = []; - + 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"]; + const timeZone = + (startOfInviteeDay as unknown as { $x: { $timezone: string } })["$x"]["$timezone"] || "UTC"; /* * @calcom/web:dev: 2022-11-06T00:00:00-04:00 * @calcom/web:dev: 2022-11-06T01:00:00-04:00 @@ -82,21 +89,30 @@ function buildSlots({ * @calcom/web:dev: 2022-11-06T03:00:00-04:00 * ... */ - let slot = dayjs.tz( - startOfInviteeDay.add(item.startTime, "minute").format("YYYY-MM-DDTHH:mm:ss"), - timeZone - ); + const slot = { + userIds: item.userIds, + time: dayjs.tz(startOfInviteeDay.add(item.startTime, "minute").format("YYYY-MM-DDTHH:mm:ss"), timeZone), + }; // If the startOfInviteeDay has a different UTC offset than the slot, a DST change has occurred. // 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 = slot.add(startOfInviteeDay.utcOffset() - slot.utcOffset(), "minutes"); - // Validating slot its not on the past - if (!slot.isBefore(startDate)) { - slots.push(slot); + 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 slots; + return Object.values(slots); } const getSlots = ({ @@ -131,19 +147,8 @@ const getSlots = ({ // eslint-disable-next-line @typescript-eslint/no-explicit-any const timeZone: string = (inviteeDate as any)["$x"]["$timezone"]; - // an override precedes all the local working hour availability logic. - const activeOverrides = dateOverrides.filter((override) => - dayjs.utc(override.start).tz(timeZone).isSame(startOfInviteeDay, "day") - ); - if (!!activeOverrides.length) { - const computedLocalAvailability = activeOverrides.flatMap((override) => ({ - startTime: override.start.getUTCHours() * 60 + override.start.getUTCMinutes(), - endTime: override.end.getUTCHours() * 60 + override.end.getUTCMinutes(), - })); - return buildSlots({ computedLocalAvailability, startDate, startOfInviteeDay, eventLength, frequency }); - } - const workingHoursUTC = workingHours.map((schedule) => ({ + userId: schedule.userId, days: schedule.days, startTime: /* Why? */ startOfDayUTC.add(schedule.startTime, "minute"), endTime: /* Why? */ startOfDayUTC.add(schedule.endTime, "minute"), @@ -162,6 +167,7 @@ const getSlots = ({ let tempComputeTimeFrame: TimeFrame | undefined; const computeLength = localWorkingHours.length - 1; const makeTimeFrame = (item: typeof localWorkingHours[0]): TimeFrame => ({ + userIds: item.userId ? [item.userId] : [], startTime: item.startTime, endTime: item.endTime, }); @@ -183,8 +189,34 @@ const getSlots = ({ computedLocalAvailability.push(tempComputeTimeFrame); } }); + // an override precedes all the local working hour availability logic. + const activeOverrides = dateOverrides.filter((override) => { + return dayjs.utc(override.start).isBetween(startOfInviteeDay, startOfInviteeDay.endOf("day"), null, "[)"); + }); - return buildSlots({ computedLocalAvailability, startOfInviteeDay, startDate, frequency, eventLength }); + if (!!activeOverrides.length) { + const overrides = activeOverrides.flatMap((override) => ({ + userIds: override.userId ? [override.userId] : [], + startTime: override.start.getUTCHours() * 60 + override.start.getUTCMinutes(), + endTime: override.end.getUTCHours() * 60 + override.end.getUTCMinutes(), + })); + overrides.forEach((override) => { + const index = computedLocalAvailability.findIndex( + (a) => !a.userIds?.length || (override.userIds[0] && a.userIds?.includes(override.userIds[0])) + ); + if (index >= 0) { + computedLocalAvailability[index] = override; + } + }); + } + + return buildSlots({ + computedLocalAvailability, + startOfInviteeDay, + startDate, + frequency, + eventLength, + }); }; export default getSlots; diff --git a/packages/trpc/server/routers/viewer/slots.tsx b/packages/trpc/server/routers/viewer/slots.tsx index 23d553c271..1e552195e8 100644 --- a/packages/trpc/server/routers/viewer/slots.tsx +++ b/packages/trpc/server/routers/viewer/slots.tsx @@ -13,7 +13,6 @@ import getTimeSlots from "@calcom/lib/slots"; import prisma, { availabilityUserSelect } from "@calcom/prisma"; import { EventTypeMetaDataSchema } from "@calcom/prisma/zod-utils"; import { EventBusyDate } from "@calcom/types/Calendar"; -import { TimeRange } from "@calcom/types/schedule"; import { TRPCError } from "@trpc/server"; @@ -47,6 +46,7 @@ const getScheduleSchema = z export type Slot = { time: string; + userIds?: number[]; attendees?: number; bookingUid?: string; users?: string[]; @@ -255,12 +255,16 @@ export async function getSchedule(input: z.infer, ctx: workingHours, dateOverrides, busy, + userId: currentUser.id, }; }) ); // flattens availability of multiple users - const dateOverrides = userAvailability.flatMap((availability) => availability.dateOverrides); + const dateOverrides = userAvailability.flatMap((availability) => + availability.dateOverrides.map((override) => ({ userId: availability.userId, ...override })) + ); const workingHours = getAggregateWorkingHours(userAvailability, eventType.schedulingType); + const computedAvailableSlots: Record = {}; const availabilityCheckProps = { eventLength: eventType.length, @@ -281,7 +285,6 @@ export async function getSchedule(input: z.infer, ctx: let checkForAvailabilityTime = 0; let getSlotsCount = 0; let checkForAvailabilityCount = 0; - do { const startGetSlots = performance.now(); // get slots retrieves the available times for a given day @@ -297,38 +300,63 @@ export async function getSchedule(input: z.infer, ctx: const endGetSlots = performance.now(); getSlotsTime += endGetSlots - startGetSlots; getSlotsCount++; - // if ROUND_ROBIN - slots stay available on some() - if normal / COLLECTIVE - slots only stay available on every() - const filterStrategy = - !eventType.schedulingType || eventType.schedulingType === SchedulingType.COLLECTIVE - ? ("every" as const) - : ("some" as const); - const availableTimeSlots = timeSlots.filter(isTimeWithinBounds).filter((time) => - userAvailability[filterStrategy]((schedule) => { - const startCheckForAvailability = performance.now(); - const isAvailable = checkIfIsAvailable({ time, ...schedule, ...availabilityCheckProps }); - const endCheckForAvailability = performance.now(); - checkForAvailabilityCount++; - checkForAvailabilityTime += endCheckForAvailability - startCheckForAvailability; - return isAvailable; + const isCollective = !eventType.schedulingType || eventType.schedulingType === SchedulingType.COLLECTIVE; + + 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, + }); + }); + })() + ); + + computedAvailableSlots[currentCheckedTime.format("YYYY-MM-DD")] = availableTimeSlots.map( + ({ time: time, ...passThroughProps }) => ({ + ...passThroughProps, + time: time.toISOString(), + users: eventType.users.map((user) => user.username || ""), + // Conditionally add the attendees and booking id to slots object if there is already a booking during that time + ...(currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString()) && { + attendees: + currentSeats[ + currentSeats.findIndex((booking) => booking.startTime.toISOString() === time.toISOString()) + ]._count.attendees, + bookingUid: + currentSeats[ + currentSeats.findIndex((booking) => booking.startTime.toISOString() === time.toISOString()) + ].uid, + }), }) ); - - computedAvailableSlots[currentCheckedTime.format("YYYY-MM-DD")] = availableTimeSlots.map((time) => ({ - time: time.toISOString(), - users: eventType.users.map((user) => user.username || ""), - // Conditionally add the attendees and booking id to slots object if there is already a booking during that time - ...(currentSeats?.some((booking) => booking.startTime.toISOString() === time.toISOString()) && { - attendees: - currentSeats[ - currentSeats.findIndex((booking) => booking.startTime.toISOString() === time.toISOString()) - ]._count.attendees, - bookingUid: - currentSeats[ - currentSeats.findIndex((booking) => booking.startTime.toISOString() === time.toISOString()) - ].uid, - }), - })); currentCheckedTime = currentCheckedTime.add(1, "day"); } while (currentCheckedTime.isBefore(endTime)); diff --git a/packages/types/schedule.d.ts b/packages/types/schedule.d.ts index ba5e74b45e..7b36261da4 100644 --- a/packages/types/schedule.d.ts +++ b/packages/types/schedule.d.ts @@ -1,4 +1,5 @@ export type TimeRange = { + userId?: number | null; start: Date; end: Date; }; @@ -15,4 +16,5 @@ export type WorkingHours = { days: number[]; startTime: number; endTime: number; + userId?: number | null; };