fix: Do not consider salesforce ownership when making rerouting decis…

…ion (calcom#18034)

apps/web/test/lib/getSchedule.test.ts

@@ -2278,7 +2278,7 @@ describe("getSchedule", () => {
       );
     });
 
-    test("Reschedule: should reschedule with same host if rescheduleWithSameRoundRobinHost is true", async () => {
+    test("Reschedule: should show timeslots of the same host if rescheduleWithSameRoundRobinHost is true", async () => {
       vi.setSystemTime("2024-05-21T00:00:13Z");
 
       const plus1DateString = "2024-05-22";
@@ -2359,7 +2359,7 @@ describe("getSchedule", () => {
       );
     });
 
-    test("Reschedule: should reschedule as per routedTeamMemberIds(instead of same host) even if rescheduleWithSameRoundRobinHost is true but it is a rerouting scenario", async () => {
+    test("Reschedule: should show timeslots as per routedTeamMemberIds(instead of same host) even if rescheduleWithSameRoundRobinHost is true but it is a rerouting scenario", async () => {
       vi.setSystemTime("2024-05-21T00:00:13Z");
 
       const plus1DateString = "2024-05-22";

packages/features/bookings/lib/handleNewBooking.ts

@@ -46,6 +46,7 @@ import {
   scheduleTrigger,
 } from "@calcom/features/webhooks/lib/scheduleTrigger";
 import { getVideoCallUrlFromCalEvent } from "@calcom/lib/CalEventParser";
+import { isRerouting, shouldIgnoreContactOwner } from "@calcom/lib/bookings/routing/utils";
 import { getDefaultEvent, getUsernameList } from "@calcom/lib/defaultEvents";
 import { ErrorCode } from "@calcom/lib/errorCodes";
 import { getErrorFromUnknown } from "@calcom/lib/errors";
@@ -464,7 +465,18 @@ async function handler(
   });
 
   const contactOwnerFromReq = reqBody.teamMemberEmail ?? null;
-  const skipContactOwner = reqBody.skipContactOwner ?? false;
+
+  const isReroutingCase = isRerouting({
+    rescheduleUid: reqBody.rescheduleUid ?? null,
+    routedTeamMemberIds: routedTeamMemberIds ?? null,
+  });
+
+  const skipContactOwner = shouldIgnoreContactOwner({
+    skipContactOwner: reqBody.skipContactOwner ?? null,
+    rescheduleUid: reqBody.rescheduleUid ?? null,
+    routedTeamMemberIds: routedTeamMemberIds ?? null,
+  });
+
   const contactOwnerEmail = skipContactOwner ? null : contactOwnerFromReq;
 
   const allHostUsers = await loadAndValidateUsers({
@@ -620,15 +632,13 @@ async function handler(
         const originalRescheduledBookingUserId =
           originalRescheduledBooking && originalRescheduledBooking.userId;
 
-        const isRouting = !!routedTeamMemberIds;
-        const isRerouting = originalRescheduledBookingUserId && isRouting;
         const shouldUseSameRRHost =
           !!originalRescheduledBookingUserId &&
           eventType.schedulingType === SchedulingType.ROUND_ROBIN &&
           eventType.rescheduleWithSameRoundRobinHost &&
           // If it is rerouting, we should not force reschedule with same host.
           // It will be unexpected plus could cause unavailable slots as original host might not be part of routedTeamMemberIds
-          !isRerouting;
+          !isReroutingCase;
 
         const userIdsSet = new Set(users.map((user) => user.id));
 

packages/lib/bookings/routing/utils.test.ts

@@ -0,0 +1,58 @@
+import { describe, expect, it } from "vitest";
+
+import { isRerouting, shouldIgnoreContactOwner } from "./utils";
+
+describe("isRerouting", () => {
+  it("should return true when both rescheduleUid and routedTeamMemberIds are present", () => {
+    const result = isRerouting({
+      rescheduleUid: "123",
+      routedTeamMemberIds: [1, 2],
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should return false when rescheduleUid is null", () => {
+    const result = isRerouting({
+      rescheduleUid: null,
+      routedTeamMemberIds: [1, 2],
+    });
+    expect(result).toBe(false);
+  });
+
+  it("should return false when routedTeamMemberIds is null", () => {
+    const result = isRerouting({
+      rescheduleUid: "123",
+      routedTeamMemberIds: null,
+    });
+    expect(result).toBe(false);
+  });
+});
+
+describe("shouldIgnoreContactOwner", () => {
+  it("should return true when skipContactOwner is true", () => {
+    const result = shouldIgnoreContactOwner({
+      skipContactOwner: true,
+      rescheduleUid: null,
+      routedTeamMemberIds: null,
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should return true when rerouting", () => {
+    const result = shouldIgnoreContactOwner({
+      skipContactOwner: false,
+      rescheduleUid: "123",
+      routedTeamMemberIds: [1, 2],
+    });
+    expect(result).toBe(true);
+  });
+
+  it("should return false when skipContactOwner is false and not rerouting", () => {
+    const result = shouldIgnoreContactOwner({
+      skipContactOwner: false,
+      rescheduleUid: null,
+      routedTeamMemberIds: null,
+    });
+    expect(result).toBe(false);
+  });
+});

packages/lib/bookings/routing/utils.ts

@@ -0,0 +1,27 @@
+/**
+ * It returns true only if there is actually some team members to route to at the moment.
+ * But I think we should also consider the case where no routedTeamMemberIds is provided or it is null which is when we want to consider all the assigned members of the team event for rerouting.
+ * So, it could be better to just read cal.rerouting query param instead of routedTeamMemberIds.
+ */
+export function isRerouting({
+  rescheduleUid,
+  routedTeamMemberIds,
+}: {
+  rescheduleUid: string | null;
+  routedTeamMemberIds: number[] | null;
+}) {
+  return !!rescheduleUid && !!routedTeamMemberIds;
+}
+
+export function shouldIgnoreContactOwner({
+  skipContactOwner,
+  rescheduleUid,
+  routedTeamMemberIds,
+}: {
+  skipContactOwner: boolean | null;
+  rescheduleUid: string | null;
+  routedTeamMemberIds: number[] | null;
+}) {
+  // During rerouting, we don't want to consider salesforce ownership as it could potentially choose a member that isn't part of routedTeamMemberIds and thus ending up with no available timeslots.
+  return skipContactOwner || isRerouting({ rescheduleUid, routedTeamMemberIds });
+}

packages/trpc/server/routers/viewer/slots/util.ts

@@ -18,6 +18,7 @@ import {
   getRoutedHostsWithContactOwnerAndFixedHosts,
   findMatchingHostsWithEventSegment,
 } from "@calcom/lib/bookings/getRoutedUsers";
+import { shouldIgnoreContactOwner, isRerouting } from "@calcom/lib/bookings/routing/utils";
 import { RESERVED_SUBDOMAINS } from "@calcom/lib/constants";
 import { getUTCOffsetByTimezone } from "@calcom/lib/date-fns";
 import { getDefaultEvent } from "@calcom/lib/defaultEvents";
@@ -489,9 +490,15 @@ async function _getAvailableSlots({ input, ctx }: GetScheduleOptions): Promise<I
     normalizedHosts: eventHosts,
   });
   const contactOwnerEmailFromInput = input.teamMemberEmail ?? null;
-  const skipContactOwner = input.skipContactOwner;
-  const contactOwnerEmail = skipContactOwner ? null : contactOwnerEmailFromInput;
+
   const routedTeamMemberIds = input.routedTeamMemberIds ?? null;
+  const skipContactOwner = shouldIgnoreContactOwner({
+    skipContactOwner: input.skipContactOwner ?? null,
+    rescheduleUid: input.rescheduleUid ?? null,
+    routedTeamMemberIds: input.routedTeamMemberIds ?? null,
+  });
+
+  const contactOwnerEmail = skipContactOwner ? null : contactOwnerEmailFromInput;
   const routedHostsWithContactOwnerAndFixedHosts = getRoutedHostsWithContactOwnerAndFixedHosts({
     hosts: hostsAfterSegmentMatching,
     routedTeamMemberIds,
@@ -1031,15 +1038,13 @@ const calculateHostsAndAvailabilities = async ({
   bypassBusyCalendarTimes: boolean;
 }) => {
   const routedTeamMemberIds = input.routedTeamMemberIds ?? null;
-  const isRouting = !!routedTeamMemberIds;
-  const isRerouting = input.rescheduleUid && isRouting;
   if (
     input.rescheduleUid &&
     eventType.rescheduleWithSameRoundRobinHost &&
     eventType.schedulingType === SchedulingType.ROUND_ROBIN &&
     // If it is rerouting, we should not force reschedule with same host.
     // It will be unexpected plus could cause unavailable slots as original host might not be part of routedTeamMemberIds
-    !isRerouting
+    !isRerouting({ rescheduleUid: input.rescheduleUid, routedTeamMemberIds })
   ) {
     const originalRescheduledBooking = await prisma.booking.findFirst({
       where: {