From 9873e373dcaafe188ea8e440bba439d3641a332c Mon Sep 17 00:00:00 2001 From: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com> Date: Thu, 7 Nov 2024 05:24:23 -0500 Subject: [PATCH] feat: reset RR every month (#17472) * only fetch bookings of this month for weighted rr * test set up * only get bookings of current month * reverts test setup * first changes for weight adjustments * reset booking count + adjust calibration * depreciate weightAdjustment * get only bookings created this month * fix typo * make sure createdAt for hosts is correct * use earliest possibel date as fallback * add missing createdAt date to tests * fix typo * clean up changes in tests * fix typo * change end date to current date * fix: Fall back to empty host array when no hosts are found * fix: Restructure code a little * fixed test, incorrectly used now outdated var * perf: remove Dayjs from getLuckyUser * Refactor getHostsWithCalibration for optimised performance, intentionally break test as findMany is always an array * Better mock for host.findMany * Remove team-event-types.test.ts, move to appropriate package * TypeScript cannot auto-infer that an array is non-empty when assigning to a var * fix: Type Fixes and DistributionMethod enum add * Optimise tests * Added test to show that bookings made before a newHost was added affect the lucky user result * Throw error when the usersWithHighestPriority is empty, which should never happen --------- Co-authored-by: CarinaWolli Co-authored-by: Alex van Andel --- .../test/lib/CheckForEmptyAssignment.test.ts | 8 +- apps/web/test/lib/team-event-types.test.ts | 670 ------------------ .../routing-forms/components/SingleForm.tsx | 1 - .../features/bookings/lib/handleNewBooking.ts | 14 +- .../handleNewBooking/ensureAvailableUsers.ts | 9 +- .../handleNewBooking/getEventTypesFromDB.ts | 2 +- .../lib/handleNewBooking/loadUsers.ts | 3 +- .../bookings/lib/handleNewBooking/types.ts | 1 - .../roundRobinManualReassignment.ts | 2 +- .../ee/round-robin/roundRobinReassignment.ts | 5 +- .../components/AddMembersWithSwitch.tsx | 1 - .../components/CheckedTeamSelect.tsx | 1 - .../eventtypes/components/EventType.tsx | 1 - .../eventtypes/components/HostEditDialogs.tsx | 2 - .../assignment/EventTeamAssignmentTab.tsx | 3 - packages/features/eventtypes/lib/types.ts | 1 - .../server/getLuckyUser.integration-test.ts | 12 +- packages/lib/server/getLuckyUser.test.ts | 596 ++++++++++++++++ packages/lib/server/getLuckyUser.ts | 161 ++++- packages/lib/server/repository/booking.ts | 13 + packages/lib/server/repository/eventType.ts | 1 - packages/lib/test/builder.ts | 5 +- .../migration.sql | 2 + packages/prisma/schema.prisma | 3 +- .../viewer/eventTypes/update.handler.ts | 49 +- .../server/routers/viewer/eventTypes/util.ts | 118 --- 26 files changed, 808 insertions(+), 876 deletions(-) delete mode 100644 apps/web/test/lib/team-event-types.test.ts create mode 100644 packages/lib/server/getLuckyUser.test.ts create mode 100644 packages/prisma/migrations/20241101180315_add_created_at_to_host/migration.sql diff --git a/apps/web/test/lib/CheckForEmptyAssignment.test.ts b/apps/web/test/lib/CheckForEmptyAssignment.test.ts index ee4698ecfb..c956873ef8 100644 --- a/apps/web/test/lib/CheckForEmptyAssignment.test.ts +++ b/apps/web/test/lib/CheckForEmptyAssignment.test.ts @@ -8,9 +8,7 @@ describe("Tests to Check if Event Types have empty Assignment", () => { checkForEmptyAssignment({ assignedUsers: [], assignAllTeamMembers: false, - hosts: [ - { userId: 101, isFixed: false, priority: 2, weight: 100, weightAdjustment: 0, scheduleId: null }, - ], + hosts: [{ userId: 101, isFixed: false, priority: 2, weight: 100, scheduleId: null }], isManagedEventType: true, }) ).toBe(true); @@ -63,9 +61,7 @@ describe("Tests to Check if Event Types have empty Assignment", () => { checkForEmptyAssignment({ assignedUsers: [], assignAllTeamMembers: false, - hosts: [ - { userId: 101, isFixed: false, priority: 2, weight: 100, weightAdjustment: 0, scheduleId: null }, - ], + hosts: [{ userId: 101, isFixed: false, priority: 2, weight: 100, scheduleId: null }], isManagedEventType: false, }) ).toBe(false); diff --git a/apps/web/test/lib/team-event-types.test.ts b/apps/web/test/lib/team-event-types.test.ts deleted file mode 100644 index 72df7a8152..0000000000 --- a/apps/web/test/lib/team-event-types.test.ts +++ /dev/null @@ -1,670 +0,0 @@ -import prismaMock from "../../../../tests/libs/__mocks__/prismaMock"; - -import { expect, it, describe } from "vitest"; - -import { getLuckyUser } from "@calcom/lib/server"; -import { buildUser, buildBooking } from "@calcom/lib/test/builder"; -import { addWeightAdjustmentToNewHosts } from "@calcom/trpc/server/routers/viewer/eventTypes/util"; - -it("can find lucky user with maximize availability", async () => { - const user1 = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - bookings: [ - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }, - ], - }); - const user2 = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - bookings: [ - { - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }, - ], - }); - const users = [user1, user2]; - // TODO: we may be able to use native prisma generics somehow? - prismaMock.user.findMany.mockResolvedValue(users); - prismaMock.booking.findMany.mockResolvedValue([]); - - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: users, - eventType: { - id: 1, - isRRWeightsEnabled: false, - }, - allRRHosts: [], - }) - ).resolves.toStrictEqual(users[1]); -}); - -it("can find lucky user with maximize availability and priority ranking", async () => { - const user1 = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 2, - bookings: [ - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }, - ], - }); - const user2 = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - bookings: [ - { - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }, - ], - }); - - const users = [user1, user2]; - // TODO: we may be able to use native prisma generics somehow? - prismaMock.user.findMany.mockResolvedValue(users); - prismaMock.booking.findMany.mockResolvedValue([]); - - // both users have medium priority (one user has no priority set, default to medium) so pick least recently booked - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: users, - eventType: { - id: 1, - isRRWeightsEnabled: false, - }, - allRRHosts: [], - }) - ).resolves.toStrictEqual(users[1]); - - const userLowest = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 0, - bookings: [ - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - const userMedium = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - priority: 2, - bookings: [ - { - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }, - ], - }); - - const userHighest = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - priority: 4, - bookings: [ - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - ], - }); - - const usersWithPriorities = [userLowest, userMedium, userHighest]; - // TODO: we may be able to use native prisma generics somehow? - prismaMock.user.findMany.mockResolvedValue(usersWithPriorities); - prismaMock.booking.findMany.mockResolvedValue([]); - - // pick the user with the highest priority - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: usersWithPriorities, - eventType: { - id: 1, - isRRWeightsEnabled: false, - }, - allRRHosts: [], - }) - ).resolves.toStrictEqual(usersWithPriorities[2]); - - const userLow = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 0, - bookings: [ - { - createdAt: new Date("2022-01-25T02:30:00.000Z"), - }, - ], - }); - const userHighLeastRecentBooking = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "tes2t@example.com", - priority: 3, - bookings: [ - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - - const userHighRecentBooking = buildUser({ - id: 3, - username: "test3", - name: "Test User 3", - email: "test3@example.com", - priority: 3, - bookings: [ - { - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }, - ], - }); - - const usersWithSamePriorities = [userLow, userHighLeastRecentBooking, userHighRecentBooking]; - // TODO: we may be able to use native prisma generics somehow? - prismaMock.user.findMany.mockResolvedValue(usersWithSamePriorities); - prismaMock.booking.findMany.mockResolvedValue([]); - - // pick the least recently booked user of the two with the highest priority - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: usersWithSamePriorities, - eventType: { - id: 1, - isRRWeightsEnabled: false, - }, - allRRHosts: [], - }) - ).resolves.toStrictEqual(usersWithSamePriorities[1]); -}); - -describe("maximize availability and weights", () => { - it("can find lucky user if hosts have same weights", async () => { - const user1 = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 3, - weight: 100, - bookings: [ - { - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - const user2 = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - priority: 3, - weight: 100, - bookings: [ - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }, - ], - }); - - prismaMock.user.findMany.mockResolvedValue([user1, user2]); - - prismaMock.booking.findMany.mockResolvedValue([ - buildBooking({ - id: 1, - userId: 1, - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }), - buildBooking({ - id: 2, - userId: 1, - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }), - buildBooking({ - id: 3, - userId: 2, - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }), - buildBooking({ - id: 4, - userId: 2, - createdAt: new Date("2022-01-25T04:30:00.000Z"), - }), - ]); - - const allRRHosts = [ - { - user: { id: user1.id, email: user1.email }, - weight: user1.weight, - weightAdjustment: user1.weightAdjustment, - }, - { - user: { id: user2.id, email: user2.email }, - weight: user2.weight, - weightAdjustment: user2.weightAdjustment, - }, - ]; - - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: [user1, user2], - eventType: { - id: 1, - isRRWeightsEnabled: true, - }, - allRRHosts, - }) - ).resolves.toStrictEqual(user2); - }); - - it("can find lucky user if hosts have different weights", async () => { - const user1 = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 3, - weight: 200, - bookings: [ - { - createdAt: new Date("2022-01-25T08:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T07:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - ], - }); - const user2 = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - priority: 3, - weight: 100, - bookings: [ - { - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - - prismaMock.user.findMany.mockResolvedValue([user1, user2]); - - prismaMock.booking.findMany.mockResolvedValue([ - buildBooking({ - id: 1, - userId: 1, - createdAt: new Date("2022-01-25T08:30:00.000Z"), - }), - buildBooking({ - id: 2, - userId: 1, - createdAt: new Date("2022-01-25T07:30:00.000Z"), - }), - buildBooking({ - id: 3, - userId: 1, - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }), - buildBooking({ - id: 4, - userId: 2, - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }), - buildBooking({ - id: 4, - userId: 2, - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }), - ]); - - const allRRHosts = [ - { - user: { id: user1.id, email: user1.email }, - weight: user1.weight, - weightAdjustment: user1.weightAdjustment, - }, - { - user: { id: user2.id, email: user2.email }, - weight: user2.weight, - weightAdjustment: user2.weightAdjustment, - }, - ]; - - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: [user1, user2], - eventType: { - id: 1, - isRRWeightsEnabled: true, - }, - allRRHosts, - }) - ).resolves.toStrictEqual(user1); - }); - - it("can find lucky user with weights and adjusted weights", async () => { - const user1 = buildUser({ - id: 1, - username: "test1", - name: "Test User 1", - email: "test1@example.com", - priority: 3, - weight: 150, - weightAdjustment: 4, // + 3 = 7 bookings - bookings: [ - { - createdAt: new Date("2022-01-25T07:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - const user2 = buildUser({ - id: 2, - username: "test2", - name: "Test User 2", - email: "test2@example.com", - priority: 3, - weight: 100, - weightAdjustment: 3, // + 2 = 5 bookings - bookings: [ - { - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }, - { - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }, - ], - }); - - prismaMock.user.findMany.mockResolvedValue([user1, user2]); - - prismaMock.booking.findMany.mockResolvedValue([ - buildBooking({ - id: 1, - userId: 1, - createdAt: new Date("2022-01-25T05:30:00.000Z"), - }), - buildBooking({ - id: 2, - userId: 1, - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }), - buildBooking({ - id: 3, - userId: 1, - createdAt: new Date("2022-01-25T07:30:00.000Z"), - }), - buildBooking({ - id: 4, - userId: 2, - createdAt: new Date("2022-01-25T06:30:00.000Z"), - }), - buildBooking({ - id: 4, - userId: 2, - createdAt: new Date("2022-01-25T03:30:00.000Z"), - }), - ]); - - const allRRHosts = [ - { - user: { id: user1.id, email: user1.email }, - weight: user1.weight, - weightAdjustment: user1.weightAdjustment, - }, - { - user: { id: user2.id, email: user2.email }, - weight: user2.weight, - weightAdjustment: user2.weightAdjustment, - }, - ]; - - await expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { - availableUsers: [user1, user2], - eventType: { - id: 1, - isRRWeightsEnabled: true, - }, - allRRHosts, - }) - ).resolves.toStrictEqual(user1); - }); -}); - -function convertHostsToUsers( - hosts: { - userId: number; - isFixed: boolean; - priority: number; - weight: number; - weightAdjustment?: number; - newHost?: boolean; - }[] -) { - return hosts.map((host) => { - return { - id: host.userId, - email: `test${host.userId}@example.com`, - hosts: host.newHost - ? [] - : [ - { - isFixed: host.isFixed, - priority: host.priority, - weightAdjustment: host.weightAdjustment, - weight: host.weight, - }, - ], - }; - }); -} - -describe("addWeightAdjustmentToNewHosts", () => { - it("weight adjustment is correctly added to host with two hosts that have the same weight", async () => { - const hosts = [ - { - userId: 1, - isFixed: false, - priority: 2, - weight: 100, - }, - { - userId: 2, - isFixed: false, - priority: 2, - weight: 100, - newHost: true, - }, - ]; - - const users = convertHostsToUsers(hosts); - - prismaMock.user.findMany.mockResolvedValue(users); - - // mock for allBookings (for ongoing RR hosts) - prismaMock.booking.findMany - .mockResolvedValueOnce([ - buildBooking({ - id: 1, - userId: 1, - }), - buildBooking({ - id: 2, - userId: 1, - }), - buildBooking({ - id: 3, - userId: 1, - }), - buildBooking({ - id: 4, - userId: 1, - }), - ]) - // mock for bookings of new RR host - .mockResolvedValueOnce([ - buildBooking({ - id: 5, - userId: 2, - }), - ]); - - const hostsWithAdjustedWeight = await addWeightAdjustmentToNewHosts({ - hosts, - isWeightsEnabled: true, - eventTypeId: 1, - prisma: prismaMock, - }); - /* - both users have weight 100, user1 has 4 bookings user 2 has 1 bookings already - */ - expect(hostsWithAdjustedWeight.find((host) => host.userId === 2)?.weightAdjustment).toBe(3); - }); - - it("weight adjustment is correctly added to host with several hosts that have different weights", async () => { - // make different weights - const hosts = [ - { - userId: 1, - isFixed: false, - priority: 2, - weight: 100, - }, - { - userId: 2, - isFixed: false, - priority: 2, - weight: 200, - newHost: true, - }, - { - userId: 3, - isFixed: false, - priority: 2, - weight: 200, - }, - { - userId: 4, - isFixed: false, - priority: 2, - weight: 100, - }, - { - userId: 5, - isFixed: false, - priority: 2, - weight: 50, - newHost: true, - }, - ]; - - const users = convertHostsToUsers(hosts); - - prismaMock.user.findMany.mockResolvedValue(users); - - // mock for allBookings (for ongoing RR hosts) - prismaMock.booking.findMany - .mockResolvedValueOnce([ - // 8 bookings for ongoing hosts (hosts that already existed before) - buildBooking({ - id: 1, - userId: 1, - }), - buildBooking({ - id: 2, - userId: 1, - }), - buildBooking({ - id: 3, - userId: 3, - }), - buildBooking({ - id: 4, - userId: 3, - }), - buildBooking({ - id: 5, - userId: 4, - }), - buildBooking({ - id: 6, - userId: 4, - }), - buildBooking({ - id: 7, - userId: 4, - }), - buildBooking({ - id: 8, - userId: 4, - }), - ]) - // mock for bookings of new RR host - .mockResolvedValueOnce([ - buildBooking({ - id: 5, - userId: 2, - }), - ]) - .mockResolvedValue([]); - - const hostsWithAdjustedWeight = await addWeightAdjustmentToNewHosts({ - hosts, - isWeightsEnabled: true, - eventTypeId: 1, - prisma: prismaMock, - }); - - // 8 bookings from ongoing hosts, 400 total weight --> average 0.02 bookings per weight unit --> 0.02 * 200 = 4 - 1 prev bookings = 3 - expect(hostsWithAdjustedWeight.find((host) => host.userId === 2)?.weightAdjustment).toBe(3); - - // 8 bookings from ongoing hosts, 400 total weight --> average 0.02 bookings per weight unit --> 0.02 * 50 = 1 (no prev bookings) - expect(hostsWithAdjustedWeight.find((host) => host.userId === 5)?.weightAdjustment).toBe(1); - }); -}); diff --git a/packages/app-store/routing-forms/components/SingleForm.tsx b/packages/app-store/routing-forms/components/SingleForm.tsx index ce80b1d483..e5528ec49b 100644 --- a/packages/app-store/routing-forms/components/SingleForm.tsx +++ b/packages/app-store/routing-forms/components/SingleForm.tsx @@ -648,7 +648,6 @@ function SingleForm({ form, appUrl, Page, enrichedWithUserProfileForm }: SingleF userId: userId, priority: 2, weight: 100, - weightAdjustment: 0, scheduleId: 1, }))} onChange={(value) => { diff --git a/packages/features/bookings/lib/handleNewBooking.ts b/packages/features/bookings/lib/handleNewBooking.ts index 67d924cef5..404b01b255 100644 --- a/packages/features/bookings/lib/handleNewBooking.ts +++ b/packages/features/bookings/lib/handleNewBooking.ts @@ -57,7 +57,7 @@ import logger from "@calcom/lib/logger"; import { handlePayment } from "@calcom/lib/payment/handlePayment"; import { getPiiFreeCalendarEvent, getPiiFreeEventType } from "@calcom/lib/piiFreeData"; import { safeStringify } from "@calcom/lib/safeStringify"; -import { getLuckyUser } from "@calcom/lib/server"; +import { DistributionMethod, getLuckyUser } from "@calcom/lib/server/getLuckyUser"; import { getTranslation } from "@calcom/lib/server/i18n"; import { WorkflowRepository } from "@calcom/lib/server/repository/workflow"; import { updateWebUser as syncServicesUpdateWebUser } from "@calcom/lib/sync/SyncServiceManager"; @@ -114,6 +114,12 @@ export const createLoggerWithEventDetails = ( }); }; +function assertNonEmptyArray(arr: T[]): asserts arr is [T, ...T[]] { + if (arr.length === 0) { + throw new Error("Array should have at least one item, but it's empty"); + } +} + function getICalSequence(originalRescheduledBooking: BookingType | null) { // If new booking set the sequence to 0 if (!originalRescheduledBooking) { @@ -451,6 +457,10 @@ async function handler( const freeUsers = luckyUserPool.filter( (user) => !luckyUsers.concat(notAvailableLuckyUsers).find((existing) => existing.id === user.id) ); + // no more freeUsers after subtracting notAvailableLuckyUsers from luckyUsers :( + if (freeUsers.length === 0) break; + assertNonEmptyArray(freeUsers); // make sure TypeScript knows it too wih an assertion; the error will never be thrown. + // freeUsers is ensured const originalRescheduledBookingUserId = originalRescheduledBooking && originalRescheduledBooking.userId; const isSameRoundRobinHost = @@ -460,7 +470,7 @@ async function handler( const newLuckyUser = isSameRoundRobinHost ? freeUsers.find((user) => user.id === originalRescheduledBookingUserId) - : await getLuckyUser("MAXIMIZE_AVAILABILITY", { + : await getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { // find a lucky user that is not already in the luckyUsers array availableUsers: freeUsers, allRRHosts: eventTypeWithUsers.hosts.filter((host) => !host.isFixed), diff --git a/packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts b/packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts index a2f12a76be..b1d6a50a7f 100644 --- a/packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts +++ b/packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts @@ -53,7 +53,8 @@ export async function ensureAvailableUsers( }, input: { dateFrom: string; dateTo: string; timeZone: string; originalRescheduledBooking?: BookingType }, loggerWithEventDetails: Logger -) { + // ReturnType hint of at least one IsFixedAwareUser, as it's made sure at least one entry exists +): Promise<[IsFixedAwareUser, ...IsFixedAwareUser[]]> { const availableUsers: IsFixedAwareUser[] = []; const startDateTimeUtc = getDateTimeInUtc(input.dateFrom, input.timeZone); @@ -142,7 +143,7 @@ export async function ensureAvailableUsers( } }); - if (!availableUsers.length) { + if (availableUsers.length === 0) { loggerWithEventDetails.error( `No available users found.`, safeStringify({ @@ -153,6 +154,6 @@ export async function ensureAvailableUsers( ); throw new Error(ErrorCode.NoAvailableUsersFound); } - - return availableUsers; + // make sure TypeScript understands availableUsers is at least one. + return availableUsers.length === 1 ? [availableUsers[0]] : [availableUsers[0], ...availableUsers.slice(1)]; } diff --git a/packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts b/packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts index 9bab2dd7fc..f1a969c5d1 100644 --- a/packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts +++ b/packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts @@ -112,7 +112,7 @@ export const getEventTypesFromDB = async (eventTypeId: number) => { isFixed: true, priority: true, weight: true, - weightAdjustment: true, + createdAt: true, user: { select: { credentials: { diff --git a/packages/features/bookings/lib/handleNewBooking/loadUsers.ts b/packages/features/bookings/lib/handleNewBooking/loadUsers.ts index c06e1123e2..6d91f35b84 100644 --- a/packages/features/bookings/lib/handleNewBooking/loadUsers.ts +++ b/packages/features/bookings/lib/handleNewBooking/loadUsers.ts @@ -46,12 +46,11 @@ export const loadUsers = async ({ const loadUsersByEventType = async (eventType: EventType): Promise => { const hosts = eventType.hosts || []; - const users = hosts.map(({ user, isFixed, priority, weight, weightAdjustment }) => ({ + const users = hosts.map(({ user, isFixed, priority, weight }) => ({ ...user, isFixed, priority, weight, - weightAdjustment, })); return users.length ? users : eventType.users; }; diff --git a/packages/features/bookings/lib/handleNewBooking/types.ts b/packages/features/bookings/lib/handleNewBooking/types.ts index 297e43daba..cacf5a581f 100644 --- a/packages/features/bookings/lib/handleNewBooking/types.ts +++ b/packages/features/bookings/lib/handleNewBooking/types.ts @@ -58,7 +58,6 @@ export type IsFixedAwareUser = User & { organization?: { slug: string }; priority?: number; weight?: number; - weightAdjustment?: number; }; export type NewBookingEventType = DefaultEvent | getEventTypeResponse; diff --git a/packages/features/ee/round-robin/roundRobinManualReassignment.ts b/packages/features/ee/round-robin/roundRobinManualReassignment.ts index 3184cc906f..2237119e9d 100644 --- a/packages/features/ee/round-robin/roundRobinManualReassignment.ts +++ b/packages/features/ee/round-robin/roundRobinManualReassignment.ts @@ -83,8 +83,8 @@ export const roundRobinManualReassignment = async ({ isFixed: false, priority: 2, weight: 100, - weightAdjustment: 0, schedule: null, + createdAt: new Date(0), // use earliest possible date as fallback })); const fixedHost = eventTypeHosts.find((host) => host.isFixed); diff --git a/packages/features/ee/round-robin/roundRobinReassignment.ts b/packages/features/ee/round-robin/roundRobinReassignment.ts index 063e52f3aa..841ca1bce8 100644 --- a/packages/features/ee/round-robin/roundRobinReassignment.ts +++ b/packages/features/ee/round-robin/roundRobinReassignment.ts @@ -21,6 +21,7 @@ import { SENDER_NAME } from "@calcom/lib/constants"; import { getBookerBaseUrl } from "@calcom/lib/getBookerUrl/server"; import logger from "@calcom/lib/logger"; import { getLuckyUser } from "@calcom/lib/server"; +import { DistributionMethod } from "@calcom/lib/server/getLuckyUser"; import { getTranslation } from "@calcom/lib/server/i18n"; import { getTimeFormatStringFromUserTimeFormat } from "@calcom/lib/timeFormat"; import { prisma } from "@calcom/prisma"; @@ -83,8 +84,8 @@ export const roundRobinReassignment = async ({ isFixed: false, priority: 2, weight: 100, - weightAdjustment: 0, schedule: null, + createdAt: new Date(0), // use earliest possible date as fallback })); const roundRobinHosts = eventType.hosts.filter((host) => !host.isFixed); @@ -125,7 +126,7 @@ export const roundRobinReassignment = async ({ roundRobinReassignLogger ); - const reassignedRRHost = await getLuckyUser("MAXIMIZE_AVAILABILITY", { + const reassignedRRHost = await getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { availableUsers, eventType: { id: eventType.id, diff --git a/packages/features/eventtypes/components/AddMembersWithSwitch.tsx b/packages/features/eventtypes/components/AddMembersWithSwitch.tsx index bfb238aaed..4a136f7668 100644 --- a/packages/features/eventtypes/components/AddMembersWithSwitch.tsx +++ b/packages/features/eventtypes/components/AddMembersWithSwitch.tsx @@ -74,7 +74,6 @@ const CheckedHostField = ({ userId: parseInt(option.value, 10), priority: option.priority ?? 2, weight: option.weight ?? 100, - weightAdjustment: option.weightAdjustment ?? 0, scheduleId: option.defaultScheduleId, })) ); diff --git a/packages/features/eventtypes/components/CheckedTeamSelect.tsx b/packages/features/eventtypes/components/CheckedTeamSelect.tsx index ee5149a927..e7ae2001bb 100644 --- a/packages/features/eventtypes/components/CheckedTeamSelect.tsx +++ b/packages/features/eventtypes/components/CheckedTeamSelect.tsx @@ -15,7 +15,6 @@ export type CheckedSelectOption = { value: string; priority?: number; weight?: number; - weightAdjustment?: number; isFixed?: boolean; disabled?: boolean; defaultScheduleId?: number | null; diff --git a/packages/features/eventtypes/components/EventType.tsx b/packages/features/eventtypes/components/EventType.tsx index d04ea2941f..3ab224895e 100644 --- a/packages/features/eventtypes/components/EventType.tsx +++ b/packages/features/eventtypes/components/EventType.tsx @@ -24,7 +24,6 @@ export type Host = { userId: number; priority: number; weight: number; - weightAdjustment: number; scheduleId?: number | null; }; diff --git a/packages/features/eventtypes/components/HostEditDialogs.tsx b/packages/features/eventtypes/components/HostEditDialogs.tsx index 2e5e930a75..ebd2729d73 100644 --- a/packages/features/eventtypes/components/HostEditDialogs.tsx +++ b/packages/features/eventtypes/components/HostEditDialogs.tsx @@ -53,7 +53,6 @@ export const PriorityDialog = (props: IDialog) => { priority: host.userId === parseInt(option.value, 10) ? newPriority.value : host.priority, isFixed: false, weight: host.weight, - weightAdjustment: host.weightAdjustment, }; }); @@ -135,7 +134,6 @@ export const WeightDialog = (props: IDialog) => { priority: host.priority, weight: host.userId === parseInt(option.value, 10) ? newWeight : host.weight, isFixed: false, - weightAdjustment: host.weightAdjustment, }; }); diff --git a/packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx b/packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx index 366e9aaf03..64dab3d059 100644 --- a/packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx +++ b/packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx @@ -145,7 +145,6 @@ const FixedHosts = ({ userId: parseInt(teamMember.value, 10), priority: 2, weight: 100, - weightAdjustment: 0, // if host was already added, retain scheduleId scheduleId: host?.scheduleId || teamMember.defaultScheduleId, }; @@ -194,7 +193,6 @@ const FixedHosts = ({ userId: parseInt(teamMember.value, 10), priority: 2, weight: 100, - weightAdjustment: 0, // if host was already added, retain scheduleId scheduleId: host?.scheduleId || teamMember.defaultScheduleId, }; @@ -279,7 +277,6 @@ const RoundRobinHosts = ({ userId: parseInt(teamMember.value, 10), priority: 2, weight: 100, - weightAdjustment: 0, // if host was already added, retain scheduleId scheduleId: host?.scheduleId || teamMember.defaultScheduleId, }; diff --git a/packages/features/eventtypes/lib/types.ts b/packages/features/eventtypes/lib/types.ts index 33e4050812..ef2dfd2fa8 100644 --- a/packages/features/eventtypes/lib/types.ts +++ b/packages/features/eventtypes/lib/types.ts @@ -26,7 +26,6 @@ export type Host = { userId: number; priority: number; weight: number; - weightAdjustment: number; scheduleId?: number | null; }; export type TeamMember = { diff --git a/packages/lib/server/getLuckyUser.integration-test.ts b/packages/lib/server/getLuckyUser.integration-test.ts index 454c4a7db6..0feb2320c4 100644 --- a/packages/lib/server/getLuckyUser.integration-test.ts +++ b/packages/lib/server/getLuckyUser.integration-test.ts @@ -2,9 +2,9 @@ import { describe, it, expect, afterEach, beforeAll, afterAll } from "vitest"; import prisma from "@calcom/prisma"; -import { getLuckyUser } from "./getLuckyUser"; +import { DistributionMethod, getLuckyUser } from "./getLuckyUser"; -describe("getLuckyUser tests", () => { +describe("getLuckyUser Integration tests", () => { describe("should not consider no show bookings for round robin: ", () => { let userIds: number[] = []; let eventTypeId: number; @@ -102,7 +102,7 @@ describe("getLuckyUser tests", () => { userIds.push(organizerThatShowedUp.id, organizerThatDidntShowUp.id); expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { availableUsers: [organizerThatShowedUp, organizerThatDidntShowUp], eventType: { id: eventTypeId, @@ -172,7 +172,7 @@ describe("getLuckyUser tests", () => { userIds.push(organizerWhoseAttendeeShowedUp.id, organizerWhoseAttendeeDidntShowUp.id); expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { availableUsers: [organizerWhoseAttendeeShowedUp, organizerWhoseAttendeeDidntShowUp], eventType: { id: eventTypeId, @@ -291,7 +291,7 @@ describe("getLuckyUser tests", () => { ); expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { availableUsers: [ organizerWhoseAttendeeShowedUp, fixedHostOrganizerWhoseAttendeeDidNotShowUp, @@ -366,7 +366,7 @@ describe("getLuckyUser tests", () => { userIds.push(user1.id, user2.id); expect( - getLuckyUser("MAXIMIZE_AVAILABILITY", { + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { availableUsers: [user1, user2], eventType: { id: eventTypeId, diff --git a/packages/lib/server/getLuckyUser.test.ts b/packages/lib/server/getLuckyUser.test.ts new file mode 100644 index 0000000000..73a47e56c2 --- /dev/null +++ b/packages/lib/server/getLuckyUser.test.ts @@ -0,0 +1,596 @@ +import prismaMock from "../../../tests/libs/__mocks__/prismaMock"; + +import { expect, it, describe } from "vitest"; + +import dayjs from "@calcom/dayjs"; +import { buildUser, buildBooking } from "@calcom/lib/test/builder"; + +import { DistributionMethod, getLuckyUser } from "./getLuckyUser"; + +type NonEmptyArray = [T, ...T[]]; +type GetLuckyUserAvailableUsersType = NonEmptyArray>; + +it("can find lucky user with maximize availability", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + bookings: [ + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + bookings: [ + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }), + ]; + // TODO: we may be able to use native prisma generics somehow? + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([]); + prismaMock.booking.findMany.mockResolvedValue([]); + + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: false, + }, + allRRHosts: [], + }) + ).resolves.toStrictEqual(users[1]); +}); + +it("can find lucky user with maximize availability and priority ranking", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 2, + bookings: [ + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + bookings: [ + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }), + ]; + // TODO: we may be able to use native prisma generics somehow? + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([]); + prismaMock.booking.findMany.mockResolvedValue([]); + + // both users have medium priority (one user has no priority set, default to medium) so pick least recently booked + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: false, + }, + allRRHosts: [], + }) + ).resolves.toStrictEqual(users[1]); + + const userLowest = buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 0, + bookings: [ + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }); + const userMedium = buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + priority: 2, + bookings: [ + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }); + + const userHighest = buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + priority: 4, + bookings: [ + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + ], + }); + + const usersWithPriorities: GetLuckyUserAvailableUsersType = [userLowest, userMedium, userHighest]; + // TODO: we may be able to use native prisma generics somehow? + prismaMock.user.findMany.mockResolvedValue(usersWithPriorities); + prismaMock.booking.findMany.mockResolvedValue([]); + prismaMock.host.findMany.mockResolvedValue([]); + // pick the user with the highest priority + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: usersWithPriorities, + eventType: { + id: 1, + isRRWeightsEnabled: false, + }, + allRRHosts: [], + }) + ).resolves.toStrictEqual(usersWithPriorities[2]); + + const userLow = buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 0, + bookings: [ + { + createdAt: new Date("2022-01-25T02:30:00.000Z"), + }, + ], + }); + const userHighLeastRecentBooking = buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "tes2t@example.com", + priority: 3, + bookings: [ + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }); + + const userHighRecentBooking = buildUser({ + id: 3, + username: "test3", + name: "Test User 3", + email: "test3@example.com", + priority: 3, + bookings: [ + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }); + + const usersWithSamePriorities: GetLuckyUserAvailableUsersType = [ + userLow, + userHighLeastRecentBooking, + userHighRecentBooking, + ]; + // TODO: we may be able to use native prisma generics somehow? + prismaMock.user.findMany.mockResolvedValue(usersWithSamePriorities); + prismaMock.booking.findMany.mockResolvedValue([]); + prismaMock.host.findMany.mockResolvedValue([]); + + // pick the least recently booked user of the two with the highest priority + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: usersWithSamePriorities, + eventType: { + id: 1, + isRRWeightsEnabled: false, + }, + allRRHosts: [], + }) + ).resolves.toStrictEqual(usersWithSamePriorities[1]); +}); + +describe("maximize availability and weights", () => { + it("can find lucky user if hosts have same weights", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 3, + weight: 100, + bookings: [ + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + priority: 3, + weight: 100, + bookings: [ + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }), + ]; + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([]); + prismaMock.booking.findMany.mockResolvedValue([ + buildBooking({ + id: 1, + userId: 1, + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }), + buildBooking({ + id: 2, + userId: 1, + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }), + buildBooking({ + id: 3, + userId: 2, + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }), + buildBooking({ + id: 4, + userId: 2, + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }), + ]); + + const allRRHosts = [ + { + user: { id: users[0].id, email: users[0].email }, + weight: users[0].weight, + createdAt: new Date(0), + }, + { + user: { id: users[1].id, email: users[1].email }, + weight: users[1].weight, + createdAt: new Date(0), + }, + ]; + + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: true, + }, + allRRHosts, + }) + ).resolves.toStrictEqual(users[1]); + }); + + it("can find lucky user if hosts have different weights", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 3, + weight: 200, + bookings: [ + { + createdAt: new Date("2022-01-25T08:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T07:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + priority: 3, + weight: 100, + bookings: [ + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }), + ]; + + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([]); + prismaMock.booking.findMany.mockResolvedValue([ + buildBooking({ + id: 1, + userId: 1, + createdAt: new Date("2022-01-25T08:30:00.000Z"), + }), + buildBooking({ + id: 2, + userId: 1, + createdAt: new Date("2022-01-25T07:30:00.000Z"), + }), + buildBooking({ + id: 3, + userId: 1, + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }), + buildBooking({ + id: 4, + userId: 2, + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }), + buildBooking({ + id: 4, + userId: 2, + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }), + ]); + + const allRRHosts = [ + { + user: { id: users[0].id, email: users[0].email }, + weight: users[0].weight, + createdAt: new Date(0), + }, + { + user: { id: users[1].id, email: users[1].email }, + weight: users[1].weight, + createdAt: new Date(0), + }, + ]; + + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: true, + }, + allRRHosts, + }) + ).resolves.toStrictEqual(users[0]); + }); + + it("can find lucky user with weights and adjusted weights", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + priority: 3, + weight: 150, + bookings: [ + { + createdAt: new Date("2022-01-25T07:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + priority: 3, + weight: 100, + bookings: [ + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }, + ], + }), + ]; + + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([]); + prismaMock.booking.findMany.mockResolvedValue([ + buildBooking({ + id: 1, + userId: 1, + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }), + buildBooking({ + id: 2, + userId: 1, + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }), + buildBooking({ + id: 3, + userId: 1, + createdAt: new Date("2022-01-25T07:30:00.000Z"), + }), + buildBooking({ + id: 4, + userId: 2, + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }), + buildBooking({ + id: 4, + userId: 2, + createdAt: new Date("2022-01-25T03:30:00.000Z"), + }), + ]); + + const allRRHosts = [ + { + user: { id: users[0].id, email: users[0].email }, + weight: users[0].weight, + createdAt: new Date(0), + }, + { + user: { id: users[1].id, email: users[1].email }, + weight: users[1].weight, + createdAt: new Date(0), + }, + ]; + + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: true, + }, + allRRHosts, + }) + ).resolves.toStrictEqual(users[0]); + }); + + it("applies calibration to newly added hosts so they are not penalized unfairly compared to their peers", async () => { + const users: GetLuckyUserAvailableUsersType = [ + buildUser({ + id: 1, + username: "test1", + name: "Test User 1", + email: "test1@example.com", + bookings: [ + { + createdAt: new Date("2022-01-25T05:30:00.000Z"), + }, + { + createdAt: new Date("2022-01-25T06:30:00.000Z"), + }, + ], + }), + buildUser({ + id: 2, + username: "test2", + name: "Test User 2", + email: "test2@example.com", + bookings: [ + { + createdAt: new Date("2022-01-25T04:30:00.000Z"), + }, + ], + }), + ]; + + const middleOfMonth = new Date( + Date.UTC(new Date().getUTCFullYear(), new Date().getUTCMonth(), 14, 12, 0, 0) + ); + + const allRRHosts = [ + { + user: { id: users[0].id, email: users[0].email }, + weight: users[0].weight, + createdAt: middleOfMonth, + }, + { + user: { id: users[1].id, email: users[1].email }, + weight: users[1].weight, + createdAt: new Date(0), + }, + ]; + + // TODO: we may be able to use native prisma generics somehow? + prismaMock.user.findMany.mockResolvedValue(users); + prismaMock.host.findMany.mockResolvedValue([ + { + userId: allRRHosts[0].user.id, + weight: allRRHosts[0].weight, + createdAt: allRRHosts[0].createdAt, + }, + ]); + // findMany bookings are BEFORE the new host (user 1) was added, calibration=2. + prismaMock.booking.findMany.mockResolvedValue([ + buildBooking({ + id: 4, + userId: 2, + createdAt: dayjs(middleOfMonth).subtract(2, "days").toDate(), + }), + buildBooking({ + id: 5, + userId: 2, + createdAt: dayjs(middleOfMonth).subtract(5, "days").toDate(), + }), + ]); + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: true, + }, + allRRHosts, + }) + ).resolves.toStrictEqual(users[1]); + // findMany bookings are AFTER the new host (user 1) was added, calibration=0. + prismaMock.booking.findMany.mockResolvedValue([ + buildBooking({ + id: 4, + userId: 2, + createdAt: dayjs(middleOfMonth).add(2, "days").toDate(), + }), + buildBooking({ + id: 5, + userId: 2, + createdAt: dayjs(middleOfMonth).add(5, "days").toDate(), + }), + ]); + await expect( + getLuckyUser(DistributionMethod.PRIORITIZE_AVAILABILITY, { + availableUsers: users, + eventType: { + id: 1, + isRRWeightsEnabled: true, + }, + allRRHosts, + }) + ).resolves.toStrictEqual(users[0]); + }); +}); diff --git a/packages/lib/server/getLuckyUser.ts b/packages/lib/server/getLuckyUser.ts index e9e336f440..2dc048ebaf 100644 --- a/packages/lib/server/getLuckyUser.ts +++ b/packages/lib/server/getLuckyUser.ts @@ -5,6 +5,13 @@ import prisma from "@calcom/prisma"; import type { Booking } from "@calcom/prisma/client"; import { BookingStatus } from "@calcom/prisma/enums"; +export enum DistributionMethod { + PRIORITIZE_AVAILABILITY = "PRIORITIZE_AVAILABILITY", + // BALANCED_ASSIGNMENT = "BALANCED_ASSIGNMENT", + // ROUND_ROBIN (for fairness, rotating through assignees) + // LOAD_BALANCED (ensuring an even workload) +} + type PartialBooking = Pick & { attendees: { email: string | null }[]; }; @@ -12,14 +19,18 @@ type PartialBooking = Pick & type PartialUser = Pick; interface GetLuckyUserParams { - availableUsers: T[]; + availableUsers: [T, ...T[]]; // ensure contains at least 1 eventType: { id: number; isRRWeightsEnabled: boolean }; allRRHosts: { user: { id: number; email: string }; + createdAt: Date; weight?: number | null; - weightAdjustment?: number | null; }[]; } +// === dayjs.utc().startOf("month").toDate(); +const startOfMonth = new Date(Date.UTC(new Date().getUTCFullYear(), new Date().getUTCMonth(), 1)); +// TS helper function. +const isNonEmptyArray = (arr: T[]): arr is [T, ...T[]] => arr.length > 0; async function leastRecentlyBookedUser({ availableUsers, @@ -107,29 +118,88 @@ async function leastRecentlyBookedUser({ return leastRecentlyBookedUser; } +async function getHostsWithCalibration( + eventTypeId: number, + hosts: { userId: number; email: string; createdAt: Date }[] +) { + const [newHostsArray, existingBookings] = await Promise.all([ + prisma.host.findMany({ + where: { + userId: { + in: hosts.map((host) => host.userId), + }, + eventTypeId, + isFixed: false, + createdAt: { + gte: startOfMonth, + }, + }, + }), + BookingRepository.getAllBookingsForRoundRobin({ + eventTypeId, + users: hosts.map((host) => ({ + id: host.userId, + email: host.email, + })), + startDate: startOfMonth, + endDate: new Date(), + }), + ]); + // Return early if there are no new hosts or no existing bookings + if (newHostsArray.length === 0 || existingBookings.length === 0) { + return hosts.map((host) => ({ ...host, calibration: 0 })); + } + // Helper function to calculate calibration for a new host + function calculateCalibration(newHost: { userId: number; createdAt: Date }) { + const existingBookingsBeforeAdded = existingBookings.filter( + (booking) => booking.userId !== newHost.userId && booking.createdAt < newHost.createdAt + ); + const hostsAddedBefore = hosts.filter( + (host) => host.userId !== newHost.userId && host.createdAt < newHost.createdAt + ); + return existingBookingsBeforeAdded.length && hostsAddedBefore.length + ? existingBookingsBeforeAdded.length / hostsAddedBefore.length + : 0; + } + // Calculate calibration for each new host and store in a Map + const newHostsWithCalibration = new Map( + newHostsArray.map((newHost) => [ + newHost.userId, + { ...newHost, calibration: calculateCalibration(newHost) }, + ]) + ); + // Map hosts with their respective calibration values + return hosts.map((host) => ({ + ...host, + calibration: newHostsWithCalibration.get(host.userId)?.calibration ?? 0, + })); +} + function getUsersWithHighestPriority({ availableUsers, }: { availableUsers: T[]; }) { const highestPriority = Math.max(...availableUsers.map((user) => user.priority ?? 2)); - - return availableUsers.filter( + const usersWithHighestPriority = availableUsers.filter( (user) => user.priority === highestPriority || (user.priority == null && highestPriority === 2) ); + if (!isNonEmptyArray(usersWithHighestPriority)) { + throw new Error("Internal Error: Highest Priority filter should never return length=0."); + } + return usersWithHighestPriority; } -async function getUsersBasedOnWeights< +async function filterUsersBasedOnWeights< T extends PartialUser & { weight?: number | null; - weightAdjustment?: number | null; } >({ availableUsers, bookingsOfAvailableUsers, allRRHosts, eventType, -}: GetLuckyUserParams & { bookingsOfAvailableUsers: PartialBooking[] }) { +}: GetLuckyUserParams & { bookingsOfAvailableUsers: PartialBooking[] }): Promise<[T, ...T[]]> { //get all bookings of all other RR hosts that are not available const availableUserIds = new Set(availableUsers.map((user) => user.id)); @@ -155,20 +225,30 @@ async function getUsersBasedOnWeights< const bookingsOfNotAvailableUsers = await BookingRepository.getAllBookingsForRoundRobin({ eventTypeId: eventType.id, users: notAvailableHosts, + startDate: startOfMonth, + endDate: new Date(), }); const allBookings = bookingsOfAvailableUsers.concat(bookingsOfNotAvailableUsers); - // Calculate the total weightAdjustments and weight of all round-robin hosts - const { allWeightAdjustments, totalWeight } = allRRHosts.reduce( - (acc, host) => { - acc.allWeightAdjustments += host.weightAdjustment ?? 0; - acc.totalWeight += host.weight ?? 100; - return acc; - }, - { allWeightAdjustments: 0, totalWeight: 0 } + const allHostsWithCalibration = await getHostsWithCalibration( + eventType.id, + allRRHosts.map((host) => { + return { email: host.user.email, userId: host.user.id, createdAt: host.createdAt }; + }) ); + // Calculate the total calibration and weight of all round-robin hosts + const totalWeight = allRRHosts.reduce((totalWeight, host) => { + totalWeight += host.weight ?? 100; + return totalWeight; + }, 0); + + const totalCalibration = allHostsWithCalibration.reduce((totalCalibration, host) => { + totalCalibration += host.calibration; + return totalCalibration; + }, 0); + // Calculate booking shortfall for each available user const usersWithBookingShortfalls = availableUsers.map((user) => { const targetPercentage = (user.weight ?? 100) / totalWeight; @@ -178,8 +258,11 @@ async function getUsersBasedOnWeights< booking.userId === user.id || booking.attendees.some((attendee) => attendee.email === user.email) ); - const targetNumberOfBookings = (allBookings.length + allWeightAdjustments) * targetPercentage; - const bookingShortfall = targetNumberOfBookings - (userBookings.length + (user.weightAdjustment ?? 0)); + const targetNumberOfBookings = (allBookings.length + totalCalibration) * targetPercentage; + // I need to get the user's current calibration here + const userCalibration = allHostsWithCalibration.find((host) => host.userId === user.id)?.calibration ?? 0; + + const bookingShortfall = targetNumberOfBookings - (userBookings.length + userCalibration); return { ...user, @@ -199,8 +282,13 @@ async function getUsersBasedOnWeights< const userIdsWithMaxShortfallAndWeight = new Set( usersWithMaxShortfall.filter((user) => user.weight === maxWeight).map((user) => user.id) ); - - return availableUsers.filter((user) => userIdsWithMaxShortfallAndWeight.has(user.id)); + const remainingUsersAfterWeightFilter = availableUsers.filter((user) => + userIdsWithMaxShortfallAndWeight.has(user.id) + ); + if (!isNonEmptyArray(remainingUsersAfterWeightFilter)) { + throw new Error("Internal Error: Weight filter should never return length=0."); + } + return remainingUsersAfterWeightFilter; } // TODO: Configure distributionAlgorithm from the event type configuration @@ -209,40 +297,43 @@ export async function getLuckyUser< T extends PartialUser & { priority?: number | null; weight?: number | null; - weightAdjustment?: number | null; } >( - distributionAlgorithm: "MAXIMIZE_AVAILABILITY" = "MAXIMIZE_AVAILABILITY", - getLuckyUserParams: GetLuckyUserParams + distributionMethod: DistributionMethod = DistributionMethod.PRIORITIZE_AVAILABILITY, + { availableUsers, ...getLuckyUserParams }: GetLuckyUserParams ) { - const { availableUsers, eventType, allRRHosts } = getLuckyUserParams; - + const { eventType } = getLuckyUserParams; + // there is only one user if (availableUsers.length === 1) { return availableUsers[0]; } - - const bookingsOfAvailableUsers = await BookingRepository.getAllBookingsForRoundRobin({ + const currentMonthBookingsOfAvailableUsers = await BookingRepository.getAllBookingsForRoundRobin({ eventTypeId: eventType.id, users: availableUsers.map((user) => { return { id: user.id, email: user.email }; }), + startDate: startOfMonth, + endDate: new Date(), }); - switch (distributionAlgorithm) { - case "MAXIMIZE_AVAILABILITY": - let possibleLuckyUsers = availableUsers; + switch (distributionMethod) { + case DistributionMethod.PRIORITIZE_AVAILABILITY: { if (eventType.isRRWeightsEnabled) { - possibleLuckyUsers = await getUsersBasedOnWeights({ + availableUsers = await filterUsersBasedOnWeights({ ...getLuckyUserParams, - bookingsOfAvailableUsers, + availableUsers, + bookingsOfAvailableUsers: currentMonthBookingsOfAvailableUsers, }); } - const highestPriorityUsers = getUsersWithHighestPriority({ availableUsers: possibleLuckyUsers }); - - return leastRecentlyBookedUser({ + const highestPriorityUsers = getUsersWithHighestPriority({ availableUsers }); + // No need to round-robin through the only user, return early also. + if (highestPriorityUsers.length === 1) return highestPriorityUsers[0]; + // TS is happy. + return leastRecentlyBookedUser({ ...getLuckyUserParams, availableUsers: highestPriorityUsers, - bookingsOfAvailableUsers, + bookingsOfAvailableUsers: currentMonthBookingsOfAvailableUsers, }); + } } } diff --git a/packages/lib/server/repository/booking.ts b/packages/lib/server/repository/booking.ts index 27713f7d1e..9522d2ec11 100644 --- a/packages/lib/server/repository/booking.ts +++ b/packages/lib/server/repository/booking.ts @@ -77,9 +77,13 @@ export class BookingRepository { static async getAllBookingsForRoundRobin({ users, eventTypeId, + startDate, + endDate, }: { users: { id: number; email: string }[]; eventTypeId: number; + startDate?: Date; + endDate?: Date; }) { const whereClause: Prisma.BookingWhereInput = { OR: [ @@ -111,6 +115,14 @@ export class BookingRepository { attendees: { some: { noShow: false } }, status: BookingStatus.ACCEPTED, eventTypeId, + ...(startDate && endDate + ? { + createdAt: { + gte: startDate, + lte: endDate, + }, + } + : {}), }; const allBookings = await prisma.booking.findMany({ @@ -121,6 +133,7 @@ export class BookingRepository { userId: true, createdAt: true, status: true, + startTime: true, }, orderBy: { createdAt: "desc", diff --git a/packages/lib/server/repository/eventType.ts b/packages/lib/server/repository/eventType.ts index d03ebe7349..9456b09450 100644 --- a/packages/lib/server/repository/eventType.ts +++ b/packages/lib/server/repository/eventType.ts @@ -534,7 +534,6 @@ export class EventTypeRepository { userId: true, priority: true, weight: true, - weightAdjustment: true, scheduleId: true, }, }, diff --git a/packages/lib/test/builder.ts b/packages/lib/test/builder.ts index cb4101d752..0e65e59c87 100644 --- a/packages/lib/test/builder.ts +++ b/packages/lib/test/builder.ts @@ -262,8 +262,8 @@ type UserPayload = Prisma.UserGetPayload<{ }; }>; export const buildUser = >( - user?: T & { priority?: number; weight?: number; weightAdjustment?: number } -): UserPayload & { priority: number; weight: number; weightAdjustment: number } => { + user?: T & { priority?: number; weight?: number } +): UserPayload & { priority: number; weight: number } => { return { locked: false, smsLockState: "UNLOCKED", @@ -312,7 +312,6 @@ export const buildUser = >( movedToProfileId: null, priority: user?.priority ?? 2, weight: user?.weight ?? 100, - weightAdjustment: user?.weightAdjustment ?? 0, isPlatformManaged: false, ...user, }; diff --git a/packages/prisma/migrations/20241101180315_add_created_at_to_host/migration.sql b/packages/prisma/migrations/20241101180315_add_created_at_to_host/migration.sql new file mode 100644 index 0000000000..8d62d8c018 --- /dev/null +++ b/packages/prisma/migrations/20241101180315_add_created_at_to_host/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "Host" ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP; diff --git a/packages/prisma/schema.prisma b/packages/prisma/schema.prisma index af37489223..af66a130c3 100644 --- a/packages/prisma/schema.prisma +++ b/packages/prisma/schema.prisma @@ -50,10 +50,11 @@ model Host { isFixed Boolean @default(false) priority Int? weight Int? - // amount of fake bookings to be added when calculating the weight (for new hosts, OOO, etc.) + // weightAdjustment is deprecated. We not calculate the calibratino value on the spot. Plan to drop this column. weightAdjustment Int? schedule Schedule? @relation(fields: [scheduleId], references: [id]) scheduleId Int? + createdAt DateTime @default(now()) @@id([userId, eventTypeId]) @@index([userId]) diff --git a/packages/trpc/server/routers/viewer/eventTypes/update.handler.ts b/packages/trpc/server/routers/viewer/eventTypes/update.handler.ts index 810a7b0739..bf17c685e2 100644 --- a/packages/trpc/server/routers/viewer/eventTypes/update.handler.ts +++ b/packages/trpc/server/routers/viewer/eventTypes/update.handler.ts @@ -21,7 +21,6 @@ import type { TrpcSessionUser } from "../../../trpc"; import { setDestinationCalendarHandler } from "../../loggedInViewer/setDestinationCalendar.handler"; import type { TUpdateInputSchema } from "./update.schema"; import { - addWeightAdjustmentToNewHosts, ensureUniqueBookingFields, ensureEmailOrPhoneNumberIsPresent, handleCustomInputs, @@ -83,6 +82,14 @@ export const updateHandler = async ({ ctx, input }: UpdateOptions) => { select: { title: true, isRRWeightsEnabled: true, + hosts: { + select: { + userId: true, + priority: true, + weight: true, + isFixed: true, + }, + }, aiPhoneCallConfig: { select: { generalPrompt: true, @@ -276,25 +283,41 @@ export const updateHandler = async ({ ctx, input }: UpdateOptions) => { const isWeightsEnabled = isRRWeightsEnabled || (typeof isRRWeightsEnabled === "undefined" && eventType.isRRWeightsEnabled); - const hostsWithWeightAdjustment = await addWeightAdjustmentToNewHosts({ - hosts, - isWeightsEnabled, - eventTypeId: id, - prisma: ctx.prisma, - }); + const oldHostsSet = new Set(eventType.hosts.map((oldHost) => oldHost.userId)); + const newHostsSet = new Set(hosts.map((oldHost) => oldHost.userId)); + + const existingHosts = hosts.filter((newHost) => oldHostsSet.has(newHost.userId)); + const newHosts = hosts.filter((newHost) => !oldHostsSet.has(newHost.userId)); + const removedHosts = eventType.hosts.filter((oldHost) => !newHostsSet.has(oldHost.userId)); data.hosts = { - deleteMany: {}, - create: hostsWithWeightAdjustment.map((host) => { - const { ...rest } = host; + deleteMany: { + OR: removedHosts.map((host) => ({ + userId: host.userId, + eventTypeId: id, + })), + }, + create: newHosts.map((host) => { return { - ...rest, + ...host, isFixed: data.schedulingType === SchedulingType.COLLECTIVE || host.isFixed, - priority: host.priority ?? 2, // default to medium priority + priority: host.priority ?? 2, weight: host.weight ?? 100, - weightAdjustment: host.weightAdjustment, }; }), + update: existingHosts.map((host) => ({ + where: { + userId_eventTypeId: { + userId: host.userId, + eventTypeId: id, + }, + }, + data: { + isFixed: data.schedulingType === SchedulingType.COLLECTIVE || host.isFixed, + priority: host.priority ?? 2, + weight: host.weight ?? 100, + }, + })), }; } diff --git a/packages/trpc/server/routers/viewer/eventTypes/util.ts b/packages/trpc/server/routers/viewer/eventTypes/util.ts index dd097aafbb..6e1f421538 100644 --- a/packages/trpc/server/routers/viewer/eventTypes/util.ts +++ b/packages/trpc/server/routers/viewer/eventTypes/util.ts @@ -1,10 +1,8 @@ import { z } from "zod"; import { markdownToSafeHTML } from "@calcom/lib/markdownToSafeHTML"; -import { BookingRepository } from "@calcom/lib/server/repository/booking"; import type { EventTypeRepository } from "@calcom/lib/server/repository/eventType"; import { UserRepository } from "@calcom/lib/server/repository/user"; -import type { PrismaClient } from "@calcom/prisma"; import { MembershipRole, PeriodType } from "@calcom/prisma/enums"; import type { CustomInputSchema } from "@calcom/prisma/zod-utils"; import { EventTypeMetaDataSchema } from "@calcom/prisma/zod-utils"; @@ -197,122 +195,6 @@ type User = { email: string; }; -export async function addWeightAdjustmentToNewHosts({ - hosts, - isWeightsEnabled, - eventTypeId, - prisma, -}: { - hosts: Host[]; - isWeightsEnabled: boolean; - eventTypeId: number; - prisma: PrismaClient; -}): Promise<(Host & { weightAdjustment?: number })[]> { - if (!isWeightsEnabled) return hosts; - - // to also have the user email to check for attendees - const usersWithHostData = await prisma.user.findMany({ - where: { - id: { - in: hosts.map((host) => host.userId), - }, - }, - select: { - email: true, - id: true, - hosts: { - where: { - eventTypeId, - }, - select: { - isFixed: true, - weightAdjustment: true, - priority: true, - weight: true, - scheduleId: true, - }, - }, - }, - }); - - const hostsWithUserData = usersWithHostData.map((user) => { - // user.hosts[0] is the previous host data from the db - // hostData is the new host data - const hostData = hosts.find((host) => host.userId === user.id); - return { - isNewRRHost: !hostData?.isFixed && (!user.hosts.length || user.hosts[0].isFixed), - isFixed: hostData?.isFixed ?? false, - weightAdjustment: hostData?.isFixed ? 0 : user.hosts[0]?.weightAdjustment ?? 0, - priority: hostData?.priority ?? 2, - weight: hostData?.weight ?? 100, - user: { - id: user.id, - email: user.email, - }, - scheduleId: hostData?.scheduleId ?? null, - }; - }); - - const ongoingRRHosts = hostsWithUserData.filter((host) => !host.isFixed && !host.isNewRRHost); - const allRRHosts = hosts.filter((host) => !host.isFixed); - - if (ongoingRRHosts.length === allRRHosts.length) { - //no new RR host was added - return hostsWithUserData.map((host) => ({ - userId: host.user.id, - isFixed: host.isFixed, - priority: host.priority, - weight: host.weight, - weightAdjustment: host.weightAdjustment, - scheduleId: host.scheduleId, - })); - } - - const ongoingHostBookings = await BookingRepository.getAllBookingsForRoundRobin({ - eventTypeId, - users: ongoingRRHosts.map((host) => { - return { id: host.user.id, email: host.user.email }; - }), - }); - - const { ongoingHostsWeightAdjustment, ongoingHostsWeights } = ongoingRRHosts.reduce( - (acc, host) => { - acc.ongoingHostsWeightAdjustment += host.weightAdjustment ?? 0; - acc.ongoingHostsWeights += host.weight ?? 0; - return acc; - }, - { ongoingHostsWeightAdjustment: 0, ongoingHostsWeights: 0 } - ); - - const hostsWithWeightAdjustments = await Promise.all( - hostsWithUserData.map(async (host) => { - let weightAdjustment = !host.isFixed ? host.weightAdjustment : 0; - if (host.isNewRRHost) { - // host can already have bookings, if they ever was assigned before - const existingBookings = await BookingRepository.getAllBookingsForRoundRobin({ - eventTypeId, - users: [{ id: host.user.id, email: host.user.email }], - }); - - const proportionalNrOfBookings = - ((ongoingHostBookings.length + ongoingHostsWeightAdjustment) / ongoingHostsWeights) * host.weight; - weightAdjustment = proportionalNrOfBookings - existingBookings.length; - } - - return { - userId: host.user.id, - isFixed: host.isFixed, - priority: host.priority, - weight: host.weight, - weightAdjustment: weightAdjustment > 0 ? Math.floor(weightAdjustment) : 0, - scheduleId: host.scheduleId, - }; - }) - ); - - return hostsWithWeightAdjustments; -} - export const mapEventType = async (eventType: EventType) => ({ ...eventType, safeDescription: eventType?.description ? markdownToSafeHTML(eventType.description) : undefined,