maronato/cal
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: Requires Confirmation when organizer reschedules (#11848)

This commit is contained in:
Hariom Balhara 2023-10-13 22:52:57 +05:30 committed by GitHub
parent b076a7dabc
commit 59faffe0d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 419 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/core/EventManager.ts
View File

@ -454,7 +454,8 @@ export default class EventManager {
// Because we are just cleaning up the events and meetings, we don't want to throw an error if one of them fails.
(await Promise.allSettled(allPromises)).some((result) => {
if (result.status === "rejected") {
log.error(
// Make it a soft error because in case a PENDING booking is rescheduled there would be no calendar events or video meetings.
log.warn(
"Error deleting calendar event or video meeting for booking",
safeStringify({ error: result.reason })
);

3
packages/features/bookings/lib/handleBookingRequested.ts
View File

@ -6,7 +6,7 @@ import logger from "@calcom/lib/logger";
import { WebhookTriggerEvents } from "@calcom/prisma/enums";
import type { CalendarEvent } from "@calcom/types/Calendar";
const log = logger.getChildLogger({ prefix: ["[handleConfirmation] book:user"] });
const log = logger.getChildLogger({ prefix: ["[handleBookingRequested] book:user"] });
/**
* Supposed to do whatever is needed when a booking is requested.
@ -31,6 +31,7 @@ export async function handleBookingRequested(args: {
}) {
const { evt, booking } = args;
log.debug("Emails: Sending booking requested emails");
await sendOrganizerRequestEmail({ ...evt });
await sendAttendeeRequestEmail({ ...evt }, evt.attendees[0]);

102
packages/features/bookings/lib/handleNewBooking.ts
View File

@ -979,10 +979,17 @@ async function handler(
const allCredentials = await getAllCredentials(organizerUser, eventType);
const isOrganizerRescheduling = organizerUser.id === userId;
const { userReschedulingIsOwner, isConfirmedByDefault } = getRequiresConfirmationFlags({
eventType,
bookingStartTime: reqBody.start,
userId,
originalRescheduledBookingOrganizerId: originalRescheduledBooking?.user?.id,
paymentAppData,
});
// If the Organizer himself is rescheduling, the booker should be sent the communication in his timezone and locale.
const attendeeInfoOnReschedule =
isOrganizerRescheduling && originalRescheduledBooking
userReschedulingIsOwner && originalRescheduledBooking
? originalRescheduledBooking.attendees.find((attendee) => attendee.email === bookerEmail)
: null;
@ -1088,14 +1095,6 @@ async function handler(
t: tOrganizer,
};
let requiresConfirmation = eventType?.requiresConfirmation;
const rcThreshold = eventType?.metadata?.requiresConfirmationThreshold;
if (rcThreshold) {
if (dayjs(dayjs(reqBody.start).utc().format()).diff(dayjs(), rcThreshold.unit) > rcThreshold.time) {
requiresConfirmation = false;
}
}
const calEventUserFieldsResponses =
"calEventUserFieldsResponses" in reqBody ? reqBody.calEventUserFieldsResponses : null;
@ -1128,7 +1127,7 @@ async function handler(
? [organizerUser.destinationCalendar]
: null,
hideCalendarNotes: eventType.hideCalendarNotes,
requiresConfirmation: requiresConfirmation ?? false,
requiresConfirmation: !isConfirmedByDefault,
eventTypeId: eventType.id,
// if seats are not enabled we should default true
seatsShowAttendees: eventType.seatsPerTimeSlot ? eventType.seatsShowAttendees : true,
@ -1210,7 +1209,6 @@ async function handler(
const eventTypeInfo: EventTypeInfo = {
eventTitle: eventType.title,
eventDescription: eventType.description,
requiresConfirmation: requiresConfirmation || null,
price: paymentAppData.price,
currency: eventType.currency,
length: eventType.length,
@ -1438,8 +1436,9 @@ async function handler(
}
}
if (noEmail !== true && (!requiresConfirmation || isOrganizerRescheduling)) {
if (noEmail !== true && isConfirmedByDefault) {
const copyEvent = cloneDeep(evt);
loggerWithEventDetails.debug("Emails: Sending reschedule emails - handleSeats");
await sendRescheduledEmails({
...copyEvent,
additionalNotes, // Resets back to the additionalNote input and not the override value
@ -1558,8 +1557,9 @@ async function handler(
? calendarResult?.updatedEvent[0]?.iCalUID
: calendarResult?.updatedEvent?.iCalUID || undefined;
if (!requiresConfirmation || isOrganizerRescheduling) {
if (noEmail !== true && isConfirmedByDefault) {
// TODO send reschedule emails to attendees of the old booking
loggerWithEventDetails.debug("Emails: Sending reschedule emails - handleSeats");
await sendRescheduledEmails({
...copyEvent,
additionalNotes, // Resets back to the additionalNote input and not the override value
@ -1890,12 +1890,6 @@ async function handler(
evt.recurringEvent = eventType.recurringEvent;
}
// If the user is not the owner of the event, new booking should be always pending.
// Otherwise, an owner rescheduling should be always accepted.
// Before comparing make sure that userId is set, otherwise undefined === undefined
const userReschedulingIsOwner = userId && originalRescheduledBooking?.user?.id === userId;
const isConfirmedByDefault = (!requiresConfirmation && !paymentAppData.price) || userReschedulingIsOwner;
async function createBooking() {
if (originalRescheduledBooking) {
evt.title = originalRescheduledBooking?.title || evt.title;
@ -1914,12 +1908,6 @@ async function handler(
const dynamicEventSlugRef = !eventTypeId ? eventTypeSlug : null;
const dynamicGroupSlugRef = !eventTypeId ? (reqBody.user as string).toLowerCase() : null;
// If the user is not the owner of the event, new booking should be always pending.
// Otherwise, an owner rescheduling should be always accepted.
// Before comparing make sure that userId is set, otherwise undefined === undefined
const userReschedulingIsOwner = userId && originalRescheduledBooking?.user?.id === userId;
const isConfirmedByDefault = (!requiresConfirmation && !paymentAppData.price) || userReschedulingIsOwner;
const attendeesData = evt.attendees.map((attendee) => {
//if attendee is team member, it should fetch their locale not booker's locale
//perhaps make email fetch request to see if his locale is stored, else
@ -2049,7 +2037,7 @@ async function handler(
safeStringify({
organizerUser: organizerUser.id,
attendeesList: attendeesList.map((guest) => ({ timeZone: guest.timeZone })),
requiresConfirmation,
requiresConfirmation: evt.requiresConfirmation,
isConfirmedByDefault,
userReschedulingIsOwner,
})
@ -2214,8 +2202,9 @@ async function handler(
videoCallUrl = metadata.hangoutLink || videoCallUrl || updatedEvent?.url;
}
}
if (noEmail !== true && (!requiresConfirmation || isOrganizerRescheduling)) {
if (noEmail !== true && isConfirmedByDefault) {
const copyEvent = cloneDeep(evt);
loggerWithEventDetails.debug("Emails: Sending rescheduled emails for booking confirmation");
await sendRescheduledEmails({
...copyEvent,
additionalInformation: metadata,
@ -2226,7 +2215,7 @@ async function handler(
}
// If it's not a reschedule, doesn't require confirmation and there's no price,
// Create a booking
} else if (!requiresConfirmation && !paymentAppData.price) {
} else if (isConfirmedByDefault) {
// Use EventManager to conditionally use all needed integrations.
const createManager = await eventManager.create(evt);
@ -2333,7 +2322,7 @@ async function handler(
}
loggerWithEventDetails.debug(
"Sending scheduled emails for booking confirmation",
"Emails: Sending scheduled emails for booking confirmation",
safeStringify({
calEvent: getPiiFreeCalendarEvent(evt),
})
@ -2351,6 +2340,16 @@ async function handler(
);
}
}
} else {
// If isConfirmedByDefault is false, then booking can't be considered ACCEPTED and thus EventManager has no role to play. Booking is created as PENDING
loggerWithEventDetails.debug(
`EventManager doesn't need to create or reschedule event for booking ${organizerUser.username}`,
safeStringify({
calEvent: getPiiFreeCalendarEvent(evt),
isConfirmedByDefault,
paymentValue: paymentAppData.price,
})
);
}
const bookingRequiresPayment =
@ -2361,7 +2360,7 @@ async function handler(
if (!isConfirmedByDefault && noEmail !== true && !bookingRequiresPayment) {
loggerWithEventDetails.debug(
`Booking ${organizerUser.username} requires confirmation, sending request emails`,
`Emails: Booking ${organizerUser.username} requires confirmation, sending request emails`,
safeStringify({
calEvent: getPiiFreeCalendarEvent(evt),
})
@ -2458,6 +2457,7 @@ async function handler(
videoCallUrl = booking.location;
}
// We are here so, booking doesn't require payment and booking is also created in DB already, through createBooking call
if (isConfirmedByDefault) {
try {
const subscribersMeetingEnded = await getWebhooks(subscriberOptionsMeetingEnded);
@ -2479,7 +2479,7 @@ async function handler(
// Send Webhook call if hooked to BOOKING_CREATED & BOOKING_RESCHEDULED
await handleWebhookTrigger({ subscriberOptions, eventTrigger, webhookData });
} else if (eventType.requiresConfirmation) {
} else {
// if eventType requires confirmation we will trigger the BOOKING REQUESTED Webhook
const eventTrigger: WebhookTriggerEvents = WebhookTriggerEvents.BOOKING_REQUESTED;
subscriberOptions.triggerEvent = eventTrigger;
@ -2558,6 +2558,44 @@ async function handler(
export default handler;
function getRequiresConfirmationFlags({
eventType,
bookingStartTime,
userId,
paymentAppData,
originalRescheduledBookingOrganizerId,
}: {
eventType: Pick<Awaited<ReturnType<typeof getEventTypesFromDB>>, "metadata" | "requiresConfirmation">;
bookingStartTime: string;
userId: number | undefined;
paymentAppData: { price: number };
originalRescheduledBookingOrganizerId: number | undefined;
}) {
let requiresConfirmation = eventType?.requiresConfirmation;
const rcThreshold = eventType?.metadata?.requiresConfirmationThreshold;
if (rcThreshold) {
if (dayjs(dayjs(bookingStartTime).utc().format()).diff(dayjs(), rcThreshold.unit) > rcThreshold.time) {
requiresConfirmation = false;
}
}
// If the user is not the owner of the event, new booking should be always pending.
// Otherwise, an owner rescheduling should be always accepted.
// Before comparing make sure that userId is set, otherwise undefined === undefined
const userReschedulingIsOwner = !!(userId && originalRescheduledBookingOrganizerId === userId);
const isConfirmedByDefault = (!requiresConfirmation && !paymentAppData.price) || userReschedulingIsOwner;
return {
/**
* Organizer of the booking is rescheduling
*/
userReschedulingIsOwner,
/**
* Booking won't need confirmation to be ACCEPTED
*/
isConfirmedByDefault,
};
}
function handleCustomInputs(
eventTypeCustomInputs: EventTypeCustomInput[],
reqCustomInputs: {

137
packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
View File

@ -13,6 +13,7 @@ import { describe, expect } from "vitest";
import { appStoreMetadata } from "@calcom/app-store/appStoreMetaData";
import { WEBAPP_URL } from "@calcom/lib/constants";
import { resetTestEmails } from "@calcom/lib/testEmails";
import { BookingStatus } from "@calcom/prisma/enums";
import { test } from "@calcom/web/test/fixtures/fixtures";
import {
@ -1213,6 +1214,122 @@ describe("handleNewBooking", () => {
timeout
);
/**
* NOTE: We might want to think about making the bookings get ACCEPTED automatically if the booker is the organizer of the event-type. This is a design decision it seems for now.
*/
test(
`should make a fresh booking in PENDING state even when the booker is the organizer of the event-type
1. Should create a booking in the database with status PENDING
2. Should send emails to the booker as well as organizer for booking request and awaiting approval
3. Should trigger BOOKING_REQUESTED webhook
`,
async ({ emails }) => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const subscriberUrl = "http://my-webhook.example.com";
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});
const organizer = getOrganizer({
name: "Organizer",
email: "organizer@example.com",
id: 101,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
});
const scenarioData = getScenarioData({
webhooks: [
{
userId: organizer.id,
eventTriggers: ["BOOKING_CREATED"],
subscriberUrl,
active: true,
eventTypeId: 1,
appId: null,
},
],
eventTypes: [
{
id: 1,
slotInterval: 45,
requiresConfirmation: true,
length: 45,
users: [
{
id: 101,
},
],
},
],
organizer,
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
});
await createBookingScenario(scenarioData);
mockSuccessfulVideoMeetingCreation({
metadataLookupKey: "dailyvideo",
});
mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});
const mockBookingData = getMockRequestDataForBooking({
data: {
eventTypeId: 1,
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: BookingLocations.CalVideo },
},
},
});
const { req } = createMockNextJsRequest({
method: "POST",
body: mockBookingData,
});
req.userId = organizer.id;
const createdBooking = await handleNewBooking(req);
await expectBookingToBeInDatabase({
description: "",
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: createdBooking.uid!,
eventTypeId: mockBookingData.eventTypeId,
status: BookingStatus.PENDING,
location: BookingLocations.CalVideo,
responses: expect.objectContaining({
email: booker.email,
name: booker.name,
}),
});
expectWorkflowToBeTriggered();
expectBookingRequestedEmails({
booker,
organizer,
emails,
});
expectBookingRequestedWebhookToHaveBeenFired({
booker,
organizer,
location: BookingLocations.CalVideo,
subscriberUrl,
eventType: scenarioData.eventTypes[0],
});
},
timeout
);
test(
`should create a booking for event that requires confirmation based on a booking notice duration threshold, if threshold is not met
1. Should create a booking in the database with status ACCEPTED
@ -1720,21 +1837,24 @@ describe("handleNewBooking", () => {
});
const createdBooking = await handleNewBooking(req);
expect(createdBooking.responses).toContain({
email: booker.email,
name: booker.name,
});
expect(createdBooking).toContain({
location: BookingLocations.CalVideo,
paymentUid: paymentUid,
});
await expectBookingToBeInDatabase({
description: "",
location: BookingLocations.CalVideo,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: createdBooking.uid!,
eventTypeId: mockBookingData.eventTypeId,
status: BookingStatus.PENDING,
responses: expect.objectContaining({
email: booker.email,
name: booker.name,
}),
});
expectWorkflowToBeTriggered();
expectAwaitingPaymentEmails({ organizer, booker, emails });
@ -1887,6 +2007,9 @@ describe("handleNewBooking", () => {
paymentId: createdBooking.paymentId!,
});
// FIXME: Right now we need to reset the test Emails because email expects only tests first email content for an email address
// Reset Test Emails to test for more Emails
resetTestEmails();
const { webhookResponse } = await mockPaymentSuccessWebhookFromStripe({ externalId });
expect(webhookResponse?.statusCode).toBe(200);
@ -1897,6 +2020,12 @@ describe("handleNewBooking", () => {
eventTypeId: mockBookingData.eventTypeId,
status: BookingStatus.PENDING,
});
expectBookingRequestedEmails({
booker,
organizer,
emails,
});
expectBookingRequestedWebhookToHaveBeenFired({
booker,
organizer,

216
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
View File

@ -616,7 +616,7 @@ describe("handleNewBooking", () => {
describe("Event Type that requires confirmation", () => {
test(
`should reschedule a booking that requires confirmation in PENDING state - When a booker(who is not the organizer himself) is doing the schedule
`should reschedule a booking that requires confirmation in PENDING state - When a booker(who is not the organizer himself) is doing the reschedule
1. Should cancel the existing booking
2. Should delete existing calendar invite and Video meeting
2. Should create a new booking in the database in PENDING state
@ -813,9 +813,8 @@ describe("handleNewBooking", () => {
timeout
);
// eslint-disable-next-line playwright/no-skipped-test
test.skip(
`should rechedule a booking, that requires confirmation, without confirmation - When Organizer is doing the reschedule
test(
`should rechedule a booking, that requires confirmation, without confirmation - When booker is the organizer of the existing booking as well as the event-type
1. Should cancel the existing booking
2. Should delete existing calendar invite and Video meeting
2. Should create a new booking in the database in ACCEPTED state
@ -873,6 +872,7 @@ describe("handleNewBooking", () => {
{
uid: uidOfBookingToBeRescheduled,
eventTypeId: 1,
userId: organizer.id,
status: BookingStatus.ACCEPTED,
startTime: `${plus1DateString}T05:00:00.000Z`,
endTime: `${plus1DateString}T05:15:00.000Z`,
@ -1054,9 +1054,209 @@ describe("handleNewBooking", () => {
timeout
);
// eslint-disable-next-line playwright/no-skipped-test
test.skip(
`should rechedule a booking, that requires confirmation, without confirmation - When the owner of the previous booking is doing the reschedule
test(
`should rechedule a booking, that requires confirmation, in PENDING state - Even when the rescheduler is the organizer of the event-type but not the organizer of the existing booking
1. Should cancel the existing booking
2. Should delete existing calendar invite and Video meeting
2. Should create a new booking in the database in PENDING state
3. Should send booking requested emails to the booker as well as organizer
4. Should trigger BOOKING_REQUESTED webhook
`,
async ({ emails }) => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const subscriberUrl = "http://my-webhook.example.com";
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});
const organizer = getOrganizer({
name: "Organizer",
email: "organizer@example.com",
id: 101,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
});
const { dateString: plus1DateString } = getDate({ dateIncrement: 1 });
const uidOfBookingToBeRescheduled = "n5Wv3eHgconAED2j4gcVhP";
const scenarioData = getScenarioData({
webhooks: [
{
userId: organizer.id,
eventTriggers: ["BOOKING_CREATED"],
subscriberUrl,
active: true,
eventTypeId: 1,
appId: null,
},
],
eventTypes: [
{
id: 1,
slotInterval: 45,
requiresConfirmation: true,
length: 45,
users: [
{
id: 101,
},
],
},
],
bookings: [
{
uid: uidOfBookingToBeRescheduled,
eventTypeId: 1,
status: BookingStatus.ACCEPTED,
startTime: `${plus1DateString}T05:00:00.000Z`,
endTime: `${plus1DateString}T05:15:00.000Z`,
references: [
getMockBookingReference({
type: appStoreMetadata.dailyvideo.type,
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASS",
meetingUrl: "http://mock-dailyvideo.example.com",
credentialId: 0,
}),
getMockBookingReference({
type: appStoreMetadata.googlecalendar.type,
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASSWORD",
meetingUrl: "https://UNUSED_URL",
externalCalendarId: "MOCK_EXTERNAL_CALENDAR_ID",
credentialId: 1,
}),
],
},
],
organizer,
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
});
await createBookingScenario(scenarioData);
const videoMock = mockSuccessfulVideoMeetingCreation({
metadataLookupKey: "dailyvideo",
});
const calendarMock = mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
uid: "MOCK_ID",
},
update: {
uid: "UPDATED_MOCK_ID",
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});
const mockBookingData = getMockRequestDataForBooking({
data: {
eventTypeId: 1,
rescheduleUid: uidOfBookingToBeRescheduled,
start: `${plus1DateString}T04:00:00.000Z`,
end: `${plus1DateString}T04:15:00.000Z`,
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: BookingLocations.CalVideo },
},
},
});
const { req } = createMockNextJsRequest({
method: "POST",
body: mockBookingData,
});
// Fake the request to be from organizer
req.userId = organizer.id;
const createdBooking = await handleNewBooking(req);
expect(createdBooking.responses).toContain({
email: booker.email,
name: booker.name,
});
await expectBookingInDBToBeRescheduledFromTo({
from: {
uid: uidOfBookingToBeRescheduled,
},
to: {
description: "",
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: createdBooking.uid!,
eventTypeId: mockBookingData.eventTypeId,
// Rescheduled booking sill stays in pending state
status: BookingStatus.PENDING,
location: BookingLocations.CalVideo,
responses: expect.objectContaining({
email: booker.email,
name: booker.name,
}),
references: [
{
type: appStoreMetadata.dailyvideo.type,
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASS",
meetingUrl: "http://mock-dailyvideo.example.com",
},
{
type: appStoreMetadata.googlecalendar.type,
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASSWORD",
meetingUrl: "https://UNUSED_URL",
externalCalendarId: "MOCK_EXTERNAL_CALENDAR_ID",
},
],
},
});
expectWorkflowToBeTriggered();
expectBookingRequestedEmails({
booker,
organizer,
emails,
});
expectBookingRequestedWebhookToHaveBeenFired({
booker,
organizer,
location: BookingLocations.CalVideo,
subscriberUrl,
eventType: scenarioData.eventTypes[0],
});
expectSuccessfulVideoMeetingDeletionInCalendar(videoMock, {
bookingRef: {
type: appStoreMetadata.dailyvideo.type,
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASS",
meetingUrl: "http://mock-dailyvideo.example.com",
},
});
expectSuccessfulCalendarEventDeletionInCalendar(calendarMock, {
externalCalendarId: "MOCK_EXTERNAL_CALENDAR_ID",
calEvent: {
videoCallData: expect.objectContaining({
url: "http://mock-dailyvideo.example.com",
}),
},
uid: "MOCK_ID",
});
},
timeout
);
test(
`should rechedule a booking, that requires confirmation, without confirmation - When the owner of the previous booking is doing the reschedule(but he isn't the organizer of the event-type now)
1. Should cancel the existing booking
2. Should delete existing calendar invite and Video meeting
2. Should create a new booking in the database in ACCEPTED state
@ -1163,9 +1363,9 @@ describe("handleNewBooking", () => {
{
id: previousOrganizerIdForTheBooking,
name: "Previous Organizer",
username: "prev-organizer",
email: "",
schedules: [TestData.schedules.IstWorkHours],
username: "prev-organizer",
timeZone: "Europe/London",
},
],

4
packages/lib/testEmails.ts
View File

@ -20,3 +20,7 @@ export const setTestEmail = (email: (typeof globalThis.testEmails)[number]) => {
export const getTestEmails = () => {
return globalThis.testEmails;
};
export const resetTestEmails = () => {
globalThis.testEmails = [];
};