Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Create BookingCreatedService CAL-[5026] #18516

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 29 additions & 210 deletions packages/features/bookings/lib/handleNewBooking.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note some post booking functions we need to keep in handleNewBooking as they're being used for rescheduled bookings, payments, and setting up bookings that require confirmation.

Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,18 @@ import {
sendRoundRobinCancelledEmailsAndSMS,
sendRoundRobinRescheduledEmailsAndSMS,
sendRoundRobinScheduledEmailsAndSMS,
sendScheduledEmailsAndSMS,
} from "@calcom/emails";
import getICalUID from "@calcom/emails/lib/getICalUID";
import { getBookingFieldsWithSystemFields } from "@calcom/features/bookings/lib/getBookingFields";
import { handleWebhookTrigger } from "@calcom/features/bookings/lib/handleWebhookTrigger";
import { isEventTypeLoggingEnabled } from "@calcom/features/bookings/lib/isEventTypeLoggingEnabled";
import BookingListener from "@calcom/features/bookings/listener/bookingListener";
import { getShouldServeCache } from "@calcom/features/calendar-cache/lib/getShouldServeCache";
import AssignmentReasonRecorder from "@calcom/features/ee/round-robin/assignmentReason/AssignmentReasonRecorder";
import {
allowDisablingAttendeeConfirmationEmails,
allowDisablingHostConfirmationEmails,
} from "@calcom/features/ee/workflows/lib/allowDisablingStandardEmails";
import { scheduleWorkflowReminders } from "@calcom/features/ee/workflows/lib/reminders/reminderScheduler";
import { getFullName } from "@calcom/features/form-builder/utils";
import { UsersRepository } from "@calcom/features/users/users.repository";
import type { GetSubscriberOptions } from "@calcom/features/webhooks/lib/getWebhooks";
import getWebhooks from "@calcom/features/webhooks/lib/getWebhooks";
import {
deleteWebhookScheduledTriggers,
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";
Expand Down Expand Up @@ -1107,7 +1098,6 @@ async function handler(
subscriberOptions,
eventTrigger,
responses,
workflows,
rescheduledBy: reqBody.rescheduledBy,
isDryRun,
});
Expand Down Expand Up @@ -1175,7 +1165,6 @@ async function handler(
reroutingFormResponses: reroutingFormResponses ?? null,
reqBody: {
user: reqBody.user,
metadata: reqBody.metadata,
recurringEventId: reqBody.recurringEventId,
},
eventType: {
Expand All @@ -1195,6 +1184,7 @@ async function handler(
},
evt,
originalRescheduledBooking,
metadata: { ...reqBody.metadata, noEmail, reqAppsStatus },
});

if (booking?.userId) {
Expand Down Expand Up @@ -1540,146 +1530,34 @@ async function handler(
// If it's not a reschedule, doesn't require confirmation and there's no price,
// Create a booking
} else if (isConfirmedByDefault) {
// Use EventManager to conditionally use all needed integrations.
const createManager = await eventManager.create(evt);
if (evt.location) {
booking.location = evt.location;
}
// This gets overridden when creating the event - to check if notes have been hidden or not. We just reset this back
// to the default description when we are sending the emails.
evt.description = eventType.description;

results = createManager.results;
referencesToCreate = createManager.referencesToCreate;
videoCallUrl = evt.videoCallData && evt.videoCallData.url ? evt.videoCallData.url : null;
if (!isDryRun) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A benefit is we don't have to wrap (!isDryRun) around multiple functions.

await BookingListener.create({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BookingListener.create() reads weird. If it’s a listener, why is it creating anything?

Seems like we need to switch to the pub/sub naming convention.

BookingPublisher/BookingSubscriber

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or BookingService

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the confusion. .create() refers to when a booking is created. I'm hoping to add BookingListener.reschedule(), BookingListener.cancel().

I was debating on BookingListener.bookingCreated(), would that be clearer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It's the fact that it's called BookingListener. Typically classes with name Listener don't have functions like .create() or .reschedule(). They are typically just .listen()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you plan to add those functions, BookingService operating as an application service makes a lot more sense.

Copy link
Contributor

@keithwillcode keithwillcode Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, in many big systems like ours with complex code, a service per operation is sometimes preferred. e.g. BookingCreationService, BookingRescheduleService, etc.

evt,
allCredentials,
organizerUser,
eventType,
tOrganizer,
booking,
eventNameObject,
bookerUrl,
});

if (results.length > 0 && results.every((res) => !res.success)) {
const error = {
errorCode: "BookingCreatingMeetingFailed",
message: "Booking failed",
req.statusCode = 201;
const bookingResponse = {
...booking,
user: {
...booking.user,
email: null,
},
paymentRequired: false,
};

loggerWithEventDetails.error(
`EventManager.create failure in some of the integrations ${organizerUser.username}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the following removed code can be moved to the BookingListener

safeStringify({ error, results })
);
} else {
const additionalInformation: AdditionalInformation = {};

if (results.length) {
// Handle Google Meet results
// We use the original booking location since the evt location changes to daily
if (bookingLocation === MeetLocationType) {
const googleMeetResult = {
appName: GoogleMeetMetadata.name,
type: "conferencing",
uid: results[0].uid,
originalEvent: results[0].originalEvent,
};

// Find index of google_calendar inside createManager.referencesToCreate
const googleCalIndex = createManager.referencesToCreate.findIndex(
(ref) => ref.type === "google_calendar"
);
const googleCalResult = results[googleCalIndex];

if (!googleCalResult) {
loggerWithEventDetails.warn("Google Calendar not installed but using Google Meet as location");
results.push({
...googleMeetResult,
success: false,
calWarnings: [tOrganizer("google_meet_warning")],
});
}

if (googleCalResult?.createdEvent?.hangoutLink) {
results.push({
...googleMeetResult,
success: true,
});

// Add google_meet to referencesToCreate in the same index as google_calendar
createManager.referencesToCreate[googleCalIndex] = {
...createManager.referencesToCreate[googleCalIndex],
meetingUrl: googleCalResult.createdEvent.hangoutLink,
};

// Also create a new referenceToCreate with type video for google_meet
createManager.referencesToCreate.push({
type: "google_meet_video",
meetingUrl: googleCalResult.createdEvent.hangoutLink,
uid: googleCalResult.uid,
credentialId: createManager.referencesToCreate[googleCalIndex].credentialId,
});
} else if (googleCalResult && !googleCalResult.createdEvent?.hangoutLink) {
results.push({
...googleMeetResult,
success: false,
});
}
}
// TODO: Handle created event metadata more elegantly
additionalInformation.hangoutLink = results[0].createdEvent?.hangoutLink;
additionalInformation.conferenceData = results[0].createdEvent?.conferenceData;
additionalInformation.entryPoints = results[0].createdEvent?.entryPoints;
evt.appsStatus = handleAppsStatus(results, booking, reqAppsStatus);
videoCallUrl =
additionalInformation.hangoutLink ||
organizerOrFirstDynamicGroupMemberDefaultLocationUrl ||
videoCallUrl;

if (!isDryRun && evt.iCalUID !== booking.iCalUID) {
// The eventManager could change the iCalUID. At this point we can update the DB record
await prisma.booking.update({
where: {
id: booking.id,
},
data: {
iCalUID: evt.iCalUID || booking.iCalUID,
},
});
}
}
if (noEmail !== true) {
let isHostConfirmationEmailsDisabled = false;
let isAttendeeConfirmationEmailDisabled = false;

isHostConfirmationEmailsDisabled =
eventType.metadata?.disableStandardEmails?.confirmation?.host || false;
isAttendeeConfirmationEmailDisabled =
eventType.metadata?.disableStandardEmails?.confirmation?.attendee || false;

if (isHostConfirmationEmailsDisabled) {
isHostConfirmationEmailsDisabled = allowDisablingHostConfirmationEmails(workflows);
}

if (isAttendeeConfirmationEmailDisabled) {
isAttendeeConfirmationEmailDisabled = allowDisablingAttendeeConfirmationEmails(workflows);
}

loggerWithEventDetails.debug(
"Emails: Sending scheduled emails for booking confirmation",
safeStringify({
calEvent: getPiiFreeCalendarEvent(evt),
})
);

if (!isDryRun) {
await monitorCallbackAsync(
sendScheduledEmailsAndSMS,
{
...evt,
additionalInformation,
additionalNotes,
customInputs,
},
eventNameObject,
isHostConfirmationEmailsDisabled,
isAttendeeConfirmationEmailDisabled,
eventType.metadata
);
}
}
return {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the post booking actions are taken we can return

...bookingResponse,
...luckyUserResponse,
isDryRun,
...(isDryRun ? { troubleshooterData } : {}),
};
}
} 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
Expand Down Expand Up @@ -1832,67 +1710,8 @@ async function handler(

loggerWithEventDetails.debug(`Booking ${organizerUser.username} completed`);

// We are here so, booking doesn't require payment and booking is also created in DB already, through createBooking call
if (isConfirmedByDefault) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calls are moved to the booking listener

const subscribersMeetingEnded = await monitorCallbackAsync(getWebhooks, subscriberOptionsMeetingEnded);
const subscribersMeetingStarted = await monitorCallbackAsync(
getWebhooks,
subscriberOptionsMeetingStarted
);

let deleteWebhookScheduledTriggerPromise: Promise<unknown> = Promise.resolve();
const scheduleTriggerPromises = [];

if (rescheduleUid && originalRescheduledBooking) {
//delete all scheduled triggers for meeting ended and meeting started of booking
deleteWebhookScheduledTriggerPromise = deleteWebhookScheduledTriggers({
booking: originalRescheduledBooking,
isDryRun,
});
}

if (booking && booking.status === BookingStatus.ACCEPTED) {
for (const subscriber of subscribersMeetingEnded) {
scheduleTriggerPromises.push(
scheduleTrigger({
booking,
subscriberUrl: subscriber.subscriberUrl,
subscriber,
triggerEvent: WebhookTriggerEvents.MEETING_ENDED,
isDryRun,
})
);
}

for (const subscriber of subscribersMeetingStarted) {
scheduleTriggerPromises.push(
scheduleTrigger({
booking,
subscriberUrl: subscriber.subscriberUrl,
subscriber,
triggerEvent: WebhookTriggerEvents.MEETING_STARTED,
isDryRun,
})
);
}
}

await Promise.all([deleteWebhookScheduledTriggerPromise, ...scheduleTriggerPromises]).catch((error) => {
loggerWithEventDetails.error(
"Error while scheduling or canceling webhook triggers",
JSON.stringify({ error })
);
});

// Send Webhook call if hooked to BOOKING_CREATED & BOOKING_RESCHEDULED
await monitorCallbackAsync(handleWebhookTrigger, {
subscriberOptions,
eventTrigger,
webhookData,
isDryRun,
});
} else {
// if eventType requires confirmation we will trigger the BOOKING REQUESTED Webhook
// if eventType requires confirmation we will trigger the BOOKING REQUESTED Webhook
if (!isConfirmedByDefault) {
const eventTrigger: WebhookTriggerEvents = WebhookTriggerEvents.BOOKING_REQUESTED;
subscriberOptions.triggerEvent = eventTrigger;
webhookData.status = "PENDING";
Expand Down
15 changes: 12 additions & 3 deletions packages/features/bookings/lib/handleNewBooking/createBooking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { BookingStatus } from "@calcom/prisma/enums";
import type { CalendarEvent } from "@calcom/types/Calendar";

import type { TgetBookingDataSchema } from "../getBookingDataSchema";
import type { NoEmail } from "./getBookingData";
import type { ReqAppsStatus } from "./types";
import type {
EventTypeId,
AwaitedBookingData,
Expand All @@ -28,7 +30,6 @@ type CreateBookingParams = {
rescheduledBy: string | undefined;
reqBody: {
user: ReqBodyWithEnd["user"];
metadata: ReqBodyWithEnd["metadata"];
recurringEventId: ReqBodyWithEnd["recurringEventId"];
};
eventType: {
Expand All @@ -51,6 +52,11 @@ type CreateBookingParams = {
};
evt: CalendarEvent;
originalRescheduledBooking: OriginalRescheduledBooking;
metadata: {
[key: string]: string | NoEmail | ReqAppsStatus;
noEmail?: NoEmail;
reqAppsStatus?: ReqAppsStatus;
Comment on lines +57 to +58
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These properties only exist in handleNewBooking although we'll need to access these in the tasker.

};
};

function updateEventDetails(
Expand Down Expand Up @@ -85,6 +91,7 @@ export async function createBooking({
routingFormResponseId,
reroutingFormResponses,
rescheduledBy,
metadata,
}: CreateBookingParams & { rescheduledBy: string | undefined }) {
updateEventDetails(evt, originalRescheduledBooking, input.changedOrganizer);
const associatedBookingForFormResponse = routingFormResponseId
Expand All @@ -101,6 +108,7 @@ export async function createBooking({
input,
evt,
originalRescheduledBooking,
metadata,
});

return await saveBooking(
Expand Down Expand Up @@ -211,6 +219,7 @@ function buildNewBookingData(params: CreateBookingParams) {
routingFormResponseId,
reroutingFormResponses,
rescheduledBy,
metadata,
} = params;

const attendeesData = getAttendeesData(evt);
Expand All @@ -234,7 +243,7 @@ function buildNewBookingData(params: CreateBookingParams) {
location: evt.location,
eventType: eventTypeRel,
smsReminderNumber: input.smsReminderNumber,
metadata: reqBody.metadata,
metadata,
attendees: {
createMany: {
data: attendeesData,
Expand Down Expand Up @@ -269,7 +278,7 @@ function buildNewBookingData(params: CreateBookingParams) {
if (originalRescheduledBooking) {
newBookingData.metadata = {
...(typeof originalRescheduledBooking.metadata === "object" && originalRescheduledBooking.metadata),
...reqBody.metadata,
...metadata,
};
newBookingData.paid = originalRescheduledBooking.paid;
newBookingData.fromReschedule = originalRescheduledBooking.uid;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export const getEventTypesFromDB = async (eventTypeId: number) => {
parentId: true,
parent: {
select: {
id: true,
teamId: true,
team: {
select: {
Expand Down
Loading
Loading