commonscomputer/cal.diy
4
0 Fork 0
Code Issues Pull Requests Actions 1.9k Packages Projects Releases Wiki Activity

fix: Don't force reschedule with same RR host if reschedule is actually rerouting (#17511)

Browse Source 
* Dont force reschedule with same RR host if routed there

* Add unit tests
This commit is contained in:
Hariom Balhara authored and GitHub committed 2024-12-05 09:30:22 -05:00
1 parent 119edb5389
commit 2408337c78
7 files changed
+499 -19
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

apps/web/test/lib/getSchedule.test.ts
+280 -3
View File
@@ -19,7 +19,7 @@ import dayjs from "@calcom/dayjs";
import { SchedulingType, type BookingStatus } from "@calcom/prisma/enums";
import { getAvailableSlots as getSchedule } from "@calcom/trpc/server/routers/viewer/slots/util";
import { expect } from "./getSchedule/expects";
import { expect, expectedSlotsForSchedule } from "./getSchedule/expects";
import { setupAndTeardown } from "./getSchedule/setupAndTeardown";
import { timeTravelToTheBeginningOfToday } from "./getSchedule/utils";
@@ -89,7 +89,8 @@ describe("getSchedule", () => {
});
});
describe("Round robin lead skip - CRM", async () => {
// TODO: Move these inside describe('Team Event')
describe("Round robin lead skip(i.e. use contact owner specified by teamMemberEmail) - CRM", async () => {
test("correctly get slots for event with only round robin hosts", async () => {
vi.setSystemTime("2024-05-21T00:00:13Z");
@@ -367,7 +368,7 @@ describe("getSchedule", () => {
}
);
});
test("correctly get slots for event with only round robin hosts", async () => {
test("correctly get slots for event with only round robin hosts - When no availability is found", async () => {
vi.setSystemTime("2024-05-21T00:00:13Z");
const plus1DateString = "2024-05-22";
@@ -496,6 +497,119 @@ describe("getSchedule", () => {
}
);
});
test("Negative case(i.e. skipContactOwner is true) - when teamMemberEmail is provided but not used", async () => {
vi.setSystemTime("2024-05-21T00:00:13Z");
const plus1DateString = "2024-05-22";
const plus2DateString = "2024-05-23";
const crmCredential = {
id: 1,
type: "salesforce_crm",
key: {
clientId: "test-client-id",
},
userId: 1,
teamId: null,
appId: "salesforce",
invalid: false,
user: { email: "test@test.com" },
};
await createCredentials([crmCredential]);
mockCrmApp("salesforce", {
getContacts: [
{
id: "contact-id",
email: "test@test.com",
ownerEmail: "example@example.com",
},
],
createContacts: [{ id: "contact-id", email: "test@test.com" }],
});
await createBookingScenario({
eventTypes: [
{
id: 1,
slotInterval: 60,
length: 60,
hosts: [
{
userId: 101,
isFixed: false,
},
{
userId: 102,
isFixed: false,
},
],
schedulingType: "ROUND_ROBIN",
metadata: {
apps: {
salesforce: {
enabled: true,
appCategories: ["crm"],
roundRobinLeadSkip: true,
},
},
},
},
],
users: [
{
...TestData.users.example,
email: "example@example.com",
id: 101,
schedules: [TestData.schedules.IstEveningShift],
},
{
...TestData.users.example,
email: "example1@example.com",
id: 102,
schedules: [TestData.schedules.IstMorningShift],
defaultScheduleId: 2,
},
],
bookings: [],
});
const scheduleWhenContactOwnerIsSkipped = await getSchedule({
input: {
eventTypeId: 1,
eventTypeSlug: "",
startTime: `${plus1DateString}T18:30:00.000Z`,
endTime: `${plus2DateString}T18:29:59.999Z`,
timeZone: Timezones["+5:30"],
isTeamEvent: true,
teamMemberEmail: "example@example.com",
skipContactOwner: true,
orgSlug: null,
},
});
// Both users slot would be available as contact owner(example@example.com) is skipped and we fallback to all users of RR
expect(scheduleWhenContactOwnerIsSkipped).toHaveTimeSlots(
[
`04:30:00.000Z`,
`05:30:00.000Z`,
`06:30:00.000Z`,
`07:30:00.000Z`,
`08:30:00.000Z`,
`09:30:00.000Z`,
`10:30:00.000Z`,
`11:30:00.000Z`,
`12:30:00.000Z`,
`13:30:00.000Z`,
`14:30:00.000Z`,
`15:30:00.000Z`,
],
{
dateString: plus2DateString,
}
);
});
});
describe("User Event", () => {
@@ -2163,5 +2277,168 @@ describe("getSchedule", () => {
}
);
});
test("Reschedule: should reschedule with same host if rescheduleWithSameRoundRobinHost is true", async () => {
vi.setSystemTime("2024-05-21T00:00:13Z");
const plus1DateString = "2024-05-22";
const plus2DateString = "2024-05-23";
// An event with common schedule
await createBookingScenario({
eventTypes: [
{
id: 1,
slotInterval: 60,
length: 60,
rescheduleWithSameRoundRobinHost: true,
hosts: [
{
userId: 101,
isFixed: false,
},
{
userId: 102,
isFixed: false,
},
],
schedulingType: "ROUND_ROBIN",
},
],
users: [
{
...TestData.users.example,
email: "example@example.com",
id: 101,
schedules: [TestData.schedules.IstEveningShift],
defaultScheduleId: 1,
},
{
...TestData.users.example,
email: "example1@example.com",
id: 102,
schedules: [TestData.schedules.IstMorningShift],
defaultScheduleId: 2,
},
],
bookings: [
{
uid: "BOOKING_TO_RESCHEDULE_UID",
userId: 101,
attendees: [
{
email: "IntegrationTestUser102@example.com",
},
],
eventTypeId: 1,
status: "ACCEPTED",
startTime: `${plus2DateString}T04:00:00.000Z`,
endTime: `${plus2DateString}T04:15:00.000Z`,
},
],
});
const schedule = await getSchedule({
input: {
eventTypeId: 1,
eventTypeSlug: "",
startTime: `${plus1DateString}T18:30:00.000Z`,
endTime: `${plus2DateString}T18:29:59.999Z`,
timeZone: Timezones["+5:30"],
isTeamEvent: true,
rescheduleUid: "BOOKING_TO_RESCHEDULE_UID",
},
});
// expect only slots of IstEveningShift as this is the slots for the original host of the booking
expect(schedule).toHaveTimeSlots(
[`11:30:00.000Z`, `12:30:00.000Z`, `13:30:00.000Z`, `14:30:00.000Z`, `15:30:00.000Z`],
{
dateString: plus2DateString,
}
);
});
test("Reschedule: should reschedule 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";
const plus2DateString = "2024-05-23";
// An event with common schedule
await createBookingScenario({
eventTypes: [
{
id: 1,
slotInterval: 60,
length: 60,
rescheduleWithSameRoundRobinHost: true,
hosts: [
{
userId: 101,
isFixed: false,
},
{
userId: 102,
isFixed: false,
},
],
schedulingType: "ROUND_ROBIN",
},
],
users: [
{
...TestData.users.example,
email: "example@example.com",
id: 101,
schedules: [TestData.schedules.IstEveningShift],
defaultScheduleId: 1,
},
{
...TestData.users.example,
email: "example1@example.com",
id: 102,
schedules: [TestData.schedules.IstMorningShift],
defaultScheduleId: 2,
},
],
bookings: [
{
uid: "BOOKING_TO_RESCHEDULE_UID",
userId: 101,
attendees: [
{
email: "IntegrationTestUser102@example.com",
},
],
eventTypeId: 1,
status: "ACCEPTED",
startTime: `${plus2DateString}T04:00:00.000Z`,
endTime: `${plus2DateString}T04:15:00.000Z`,
},
],
});
const schedule = await getSchedule({
input: {
eventTypeId: 1,
eventTypeSlug: "",
startTime: `${plus1DateString}T18:30:00.000Z`,
endTime: `${plus2DateString}T18:29:59.999Z`,
timeZone: Timezones["+5:30"],
isTeamEvent: true,
rescheduleUid: "BOOKING_TO_RESCHEDULE_UID",
routedTeamMemberIds: [102],
},
});
// expect only slots of IstEveningShift as this is the slots for the original host of the booking
expect(schedule).toHaveTimeSlots(
expectedSlotsForSchedule.IstMorningShift.interval["1hr"].allPossibleSlotsStartingAt430,
{
dateString: plus2DateString,
}
);
});
});
});
apps/web/test/lib/getSchedule/expects.ts
+26
View File
@@ -30,6 +30,32 @@ export const expectedSlotsForSchedule = {
},
},
},
IstMorningShift: {
interval: {
"1hr": {
allPossibleSlotsStartingAt430: [
"04:30:00.000Z",
"05:30:00.000Z",
"06:30:00.000Z",
"07:30:00.000Z",
"08:30:00.000Z",
"09:30:00.000Z",
"10:30:00.000Z",
"11:30:00.000Z",
],
allPossibleSlotsStartingAt4: [
"04:00:00.000Z",
"05:00:00.000Z",
"06:00:00.000Z",
"07:00:00.000Z",
"08:00:00.000Z",
"09:00:00.000Z",
"10:00:00.000Z",
"11:00:00.000Z",
],
},
},
},
};
declare global {
apps/web/test/utils/bookingScenario/bookingScenario.ts
+3
View File
@@ -1283,6 +1283,9 @@ export function getOrganizer({
export function getScenarioData(
{
/**
* organizer has no special meaning. It is a regular user. It is supposed to be deprecated along with `usersApartFromOrganizer` and we should introduce a new `users` field instead
*/
organizer,
eventTypes,
usersApartFromOrganizer = [],
packages/features/bookings/lib/handleNewBooking.ts
+9 -3
View File
@@ -619,10 +619,16 @@ async function handler(
// freeUsers is ensured
const originalRescheduledBookingUserId =
originalRescheduledBooking && originalRescheduledBooking.userId;
const isSameRoundRobinHost =
const isRouting = !!routedTeamMemberIds;
const isRerouting = originalRescheduledBookingUserId && isRouting;
const shouldUseSameRRHost =
!!originalRescheduledBookingUserId &&
eventType.schedulingType === SchedulingType.ROUND_ROBIN &&
eventType.rescheduleWithSameRoundRobinHost;
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;
const userIdsSet = new Set(users.map((user) => user.id));
@@ -646,7 +652,7 @@ async function handler(
});
}
const newLuckyUser = isSameRoundRobinHost
const newLuckyUser = shouldUseSameRRHost
? freeUsers.find((user) => user.id === originalRescheduledBookingUserId)
: await getLuckyUser({
// find a lucky user that is not already in the luckyUsers array
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
+159
View File
@@ -2546,6 +2546,165 @@ describe("handleNewBooking", () => {
},
timeout
);
test(
"should reschedule as per routedTeamMemberIds(instead of same host) even if rescheduleWithSameRoundRobinHost is true but it is a rerouting scenario",
async ({ emails }) => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});
const otherHost = getOrganizer({
name: "RR Host 1",
email: "rrhost1@example.com",
id: 101,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
});
const hostOfOriginalBooking = getOrganizer({
name: "RR Host 2",
email: "rrhost2@example.com",
id: 102,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
});
const { dateString: plus1DateString } = getDate({ dateIncrement: 1 });
const uidOfBookingToBeRescheduled = "n5Wv3eHgconAED2j4gcVhP";
await createBookingScenario(
getScenarioData({
eventTypes: [
{
id: 1,
slotInterval: 15,
length: 15,
hosts: [
{
userId: 101,
isFixed: false,
},
{
userId: 102,
isFixed: false,
},
],
schedulingType: SchedulingType.ROUND_ROBIN,
rescheduleWithSameRoundRobinHost: true,
},
],
bookings: [
{
uid: uidOfBookingToBeRescheduled,
eventTypeId: 1,
userId: 102,
status: BookingStatus.ACCEPTED,
startTime: `${plus1DateString}T05:00:00.000Z`,
endTime: `${plus1DateString}T05:15:00.000Z`,
metadata: {
videoCallUrl: "https://existing-daily-video-call-url.example.com",
},
},
],
organizer: otherHost,
usersApartFromOrganizer: [hostOfOriginalBooking],
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
})
);
mockSuccessfulVideoMeetingCreation({
metadataLookupKey: "dailyvideo",
});
mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
uid: "MOCK_ID",
},
update: {
uid: "UPDATED_MOCK_ID",
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});
const mockBookingData = getMockRequestDataForBooking({
data: {
eventTypeId: 1,
user: otherHost.name,
rescheduleUid: uidOfBookingToBeRescheduled,
start: `${plus1DateString}T04:00:00.000Z`,
end: `${plus1DateString}T04:15:00.000Z`,
routedTeamMemberIds: [101],
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: BookingLocations.CalVideo },
},
},
});
const { req } = createMockNextJsRequest({
method: "POST",
body: mockBookingData,
});
const createdBooking = await handleNewBooking(req);
const previousBooking = await prismaMock.booking.findUnique({
where: {
uid: uidOfBookingToBeRescheduled,
},
});
logger.silly({
previousBooking,
allBookings: await prismaMock.booking.findMany(),
});
// Expect previous booking to be cancelled
await expectBookingToBeInDatabase({
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: uidOfBookingToBeRescheduled,
status: BookingStatus.CANCELLED,
});
expect(previousBooking?.status).toBe(BookingStatus.CANCELLED);
/**
* Booking Time should be new time
*/
expect(createdBooking.startTime?.toISOString()).toBe(`${plus1DateString}T04:00:00.000Z`);
expect(createdBooking.endTime?.toISOString()).toBe(`${plus1DateString}T04:15:00.000Z`);
expect(createdBooking.userId).toBe(otherHost.id);
await expectBookingInDBToBeRescheduledFromTo({
from: {
uid: uidOfBookingToBeRescheduled,
},
to: {
description: "",
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: createdBooking.uid!,
eventTypeId: mockBookingData.eventTypeId,
status: BookingStatus.ACCEPTED,
location: BookingLocations.CalVideo,
responses: expect.objectContaining({
email: booker.email,
name: booker.name,
}),
},
});
expectSuccessfulRoundRobinReschedulingEmails({
prevOrganizer: hostOfOriginalBooking,
newOrganizer: otherHost,
emails,
});
},
timeout
);
});
});
});
packages/lib/isOutOfBounds.tsx
+2 -2
View File
@@ -245,7 +245,7 @@ export function isTimeViolatingFutureLimit({
const dateInSystemTz = dayjs(time);
if (periodLimits.endOfRollingPeriodEndDayInBookerTz) {
const isAfterRollingEndDay = dateInSystemTz.isAfter(periodLimits.endOfRollingPeriodEndDayInBookerTz);
log.debug("rollingEndDayCheck", {
log.silly("rollingEndDayCheck", {
formattedDate: dateInSystemTz.format(),
isAfterRollingEndDay,
endOfRollingPeriodEndDayInBookerTz: periodLimits.endOfRollingPeriodEndDayInBookerTz.format(),
@@ -256,7 +256,7 @@ export function isTimeViolatingFutureLimit({
if (periodLimits.startOfRangeStartDayInEventTz && periodLimits.endOfRangeEndDayInEventTz) {
const isBeforeRangeStart = dateInSystemTz.isBefore(periodLimits.startOfRangeStartDayInEventTz);
const isAfterRangeEnd = dateInSystemTz.isAfter(periodLimits.endOfRangeEndDayInEventTz);
log.debug("rangeCheck", {
log.silly("rangeCheck", {
formattedDate: dateInSystemTz.format(),
isAfterRangeEnd,
isBeforeRangeStart,
packages/trpc/server/routers/viewer/slots/util.ts
+20 -11
View File
@@ -502,7 +502,7 @@ async function _getAvailableSlots({ input, ctx }: GetScheduleOptions): Promise<I
await calculateHostsAndAvailabilities({
input,
eventType,
routedHostsWithContactOwnerAndFixedHosts,
hosts: routedHostsWithContactOwnerAndFixedHosts,
contactOwnerEmail,
loggerWithEventDetails,
startTime,
@@ -526,7 +526,7 @@ async function _getAvailableSlots({ input, ctx }: GetScheduleOptions): Promise<I
await calculateHostsAndAvailabilities({
input,
eventType,
routedHostsWithContactOwnerAndFixedHosts: routedHostsAndFixedHosts,
hosts: routedHostsAndFixedHosts,
contactOwnerEmail,
loggerWithEventDetails,
startTime,
@@ -1011,7 +1011,7 @@ export function getAllDatesWithBookabilityStatus(availableDates: string[]) {
const calculateHostsAndAvailabilities = async ({
input,
eventType,
routedHostsWithContactOwnerAndFixedHosts,
hosts,
contactOwnerEmail,
loggerWithEventDetails,
startTime,
@@ -1020,20 +1020,26 @@ const calculateHostsAndAvailabilities = async ({
}: {
input: TGetScheduleInputSchema;
eventType: Exclude<Awaited<ReturnType<typeof getRegularOrDynamicEventType>>, null>;
routedHostsWithContactOwnerAndFixedHosts: {
hosts: {
isFixed?: boolean;
user: GetAvailabilityUser;
}[];
contactOwnerEmail?: string | null;
contactOwnerEmail: string | null;
loggerWithEventDetails: Logger<unknown>;
startTime: ReturnType<typeof getStartTime>;
endTime: Dayjs;
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
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
) {
const originalRescheduledBooking = await prisma.booking.findFirst({
where: {
@@ -1046,14 +1052,12 @@ const calculateHostsAndAvailabilities = async ({
userId: true,
},
});
routedHostsWithContactOwnerAndFixedHosts = routedHostsWithContactOwnerAndFixedHosts.filter(
(host) => host.user.id === originalRescheduledBooking?.userId || 0
);
hosts = hosts.filter((host) => host.user.id === originalRescheduledBooking?.userId || 0);
}
const usersWithCredentials = monitorCallbackSync(getUsersWithCredentialsConsideringContactOwner, {
contactOwnerEmail,
hosts: routedHostsWithContactOwnerAndFixedHosts,
hosts,
});
loggerWithEventDetails.debug("Using users", {
@@ -1174,5 +1178,10 @@ const calculateHostsAndAvailabilities = async ({
eventType.schedulingType
);
return { aggregatedAvailability, allUsersAvailability, usersWithCredentials, currentSeats };
return {
aggregatedAvailability,
allUsersAvailability,
usersWithCredentials,
currentSeats,
};
};