refactor: handle emails and sms side effects#23578
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughReplaces in-file email/SMS helper calls in packages/features/bookings/lib/handleNewBooking.ts with a side-effect orchestration using a new BookingEmailSmsHandler. Adds packages/features/bookings/lib/BookingEmailSmsHandler.ts which defines BookingActionMap (confirmed/rescheduled/requested), typed side-effect payloads (including iCalUID, originalRescheduledBooking, workflows, customInputs where applicable), an IBookingEmailSmsHandler interface, and a BookingEmailSmsHandler class with send(payload). handleNewBooking now instantiates emailsAndSmsHandler and emits BOOKING_CONFIRMED / BOOKING_RESCHEDULED / BOOKING_REQUESTED payloads instead of calling per-file email helpers; round‑robin reschedule, confirmation, and request flows were moved into the new handler. Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
keithwillcode
added
core
dosubot
bot
added
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/features/bookings/lib/handleBookingSideEffects.ts (1)
55-58: Typo: rename isReschduledByBooker → isRescheduledByBooker (and update usage).Prevents confusion and future searchability issues. Update both type and references here and in the caller.
- isReschduledByBooker: boolean; + isRescheduledByBooker: boolean;- isReschduledByBooker, + isRescheduledByBooker,- ...(isReschduledByBooker && { reason: "Booker Rescheduled" }), + ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }),Also applies to: 113-114, 221-222
packages/features/bookings/lib/handleNewBooking.ts (3)
1957-1960: Trim payload: send only required eventType fields.Reduces payload surface and tightens typing against the side-effects module.
- emailsAndSmsSideEffectsPayload = { - action: BookingSideEffectAction.BOOKING_CONFIRMED, - data: { eventType, eventNameObject, workflows, evt }, - }; + emailsAndSmsSideEffectsPayload = { + action: BookingSideEffectAction.BOOKING_CONFIRMED, + data: { + eventType: { metadata: eventType.metadata, schedulingType: eventType.schedulingType }, + eventNameObject, + workflows, + evt, + }, + };
1991-1993: Same here: constrain eventType in REQUESTED payload.Avoids over-serializing the entire event type object.
- emailsAndSmsSideEffectsPayload = { - action: BookingSideEffectAction.BOOKING_REQUESTED, - data: { evt, attendees: attendeesList, eventType, additionalNotes }, - }; + emailsAndSmsSideEffectsPayload = { + action: BookingSideEffectAction.BOOKING_REQUESTED, + data: { + evt, + attendees: attendeesList, + eventType: { metadata: eventType.metadata, schedulingType: eventType.schedulingType }, + additionalNotes, + }, + };
2326-2329: Don’t let side-effect failures fail booking; catch and log.Email/SMS are ancillary; isolate their failures from primary booking success path.
- if (emailsAndSmsSideEffectsPayload) { - await handleSendingEmailsAndSms(emailsAndSmsSideEffectsPayload); - } + if (emailsAndSmsSideEffectsPayload) { + try { + await handleSendingEmailsAndSms(emailsAndSmsSideEffectsPayload); + } catch (e) { + loggerWithEventDetails.error( + "Side effects failed", + safeStringify({ error: getErrorFromUnknown(e) }) + ); + } + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/bookings/lib/handleBookingSideEffects.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleBookingSideEffects.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleBookingSideEffects.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/handleBookingSideEffects.tspackages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-28T10:48:03.862Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-28T10:48:03.862Z
Learning: All host arrays emitted by findQualifiedHostsWithDelegationCredentials.ts must include groupId: string | null on each item (qualifiedRRHosts, fixedHosts, allFallbackRRHosts, and intermediate “AfterXMatching” arrays). Missing groupId triggers TS errors in BaseHost expectations.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (2)
packages/features/bookings/lib/handleBookingSideEffects.ts (7)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(146-146)packages/features/bookings/lib/handleNewBooking/originalRescheduledBookingUtils.ts (1)
BookingType(24-24)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/emails/email-manager.ts (7)
sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendRescheduledEmailsAndSMS(318-321)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/handleBookingSideEffects.ts (2)
EmailsAndSmsSideEffectsPayload(85-88)handleSendingEmailsAndSms(90-307)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
982-983: LGTM: cleaner tuple pattern avoids unused var lint warning.The tuple destructure
([_groupId, hosts])is clear and avoids the lint issue.
| const newBookingMemberEmails: Person[] = | ||
| copyEvent.team?.members | ||
| .map((member) => member) | ||
| .concat(copyEvent.organizer) | ||
| .concat(copyEvent.attendees) || []; | ||
|
|
There was a problem hiding this comment.
Fix undefined access and separate host-vs-attendee recipient sets.
copyEvent.team?.members.map(...) can throw when team is undefined. Also, using attendees to compute host scheduled/cancelled recipients can incorrectly send host templates to attendees. Build team members separately and use them for scheduled/cancelled; keep attendees included only for rescheduled.
- const newBookingMemberEmails: Person[] =
- copyEvent.team?.members
- .map((member) => member)
- .concat(copyEvent.organizer)
- .concat(copyEvent.attendees) || [];
+ const newTeamMembers: Person[] = [
+ ...((copyEvent.team?.members as Person[]) ?? []),
+ copyEvent.organizer,
+ ].filter(Boolean);
+ const newBookingMemberEmails: Person[] = [...newTeamMembers, ...copyEvent.attendees];
@@
- // scheduled Emails
- const newBookedMembers = newBookingMemberEmails.filter(
+ // scheduled Emails (hosts only)
+ const newBookedMembers = newTeamMembers.filter(
(member) =>
!originalBookingMemberEmails.find((originalMember) =>
matchOriginalMemberWithNewMember(originalMember, member)
)
);
- // cancelled Emails
- const cancelledMembers = originalBookingMemberEmails.filter(
- (member) =>
- !newBookingMemberEmails.find((newMember) => matchOriginalMemberWithNewMember(member, newMember))
- );
+ // cancelled Emails (previous host only)
+ const originalAttendeeEmails = new Set(originalRescheduledBooking.attendees.map((a) => a.email));
+ const originalTeamMembers = originalBookingMemberEmails.filter(
+ (m) => !originalAttendeeEmails.has(m.email)
+ );
+ const cancelledMembers = originalTeamMembers.filter(
+ (member) => !newTeamMembers.find((newMember) => matchOriginalMemberWithNewMember(member, newMember))
+ );Also applies to: 181-199
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/features/bookings/lib/handleBookingSideEffects.ts (2)
170-175: Fix undefined access and separate host vs attendee recipient sets (RR).
copyEvent.team?.members.map(...)will throw whenteamis undefined. Also, mixing attendees into host scheduled/cancelled computations can send the wrong templates. Build host lists separately and use them for scheduled/cancelled; keep attendees only in the rescheduled set.Apply this diff:
- const newBookingMemberEmails: Person[] = - copyEvent.team?.members - .map((member) => member) - .concat(copyEvent.organizer) - .concat(copyEvent.attendees) || []; + const newTeamMembers: Person[] = [ + ...((copyEvent.team?.members as Person[]) ?? []), + copyEvent.organizer, + ].filter(Boolean) as Person[]; + const newBookingMemberEmails: Person[] = [...newTeamMembers, ...copyEvent.attendees]; @@ - // scheduled Emails - const newBookedMembers = newBookingMemberEmails.filter( + // scheduled Emails (hosts only) + const newBookedMembers = newTeamMembers.filter( (member) => !originalBookingMemberEmails.find((originalMember) => matchOriginalMemberWithNewMember(originalMember, member) ) ); - // cancelled Emails - const cancelledMembers = originalBookingMemberEmails.filter( - (member) => - !newBookingMemberEmails.find((newMember) => matchOriginalMemberWithNewMember(member, newMember)) - ); + // cancelled Emails (previous hosts only) + const originalAttendeeEmails = new Set( + originalRescheduledBooking.attendees.map((a) => a.email) + ); + const originalTeamMembers = originalBookingMemberEmails.filter( + (m) => !originalAttendeeEmails.has(m.email) + ); + const cancelledMembers = originalTeamMembers.filter( + (member) => + !newTeamMembers.find((newMember) => + matchOriginalMemberWithNewMember(member, newMember) + ) + );Also applies to: 180-191
199-223: Await async mail/SMS senders to prevent premature return and unhandled rejections.These functions return promises; not awaiting risks lost errors and non-deterministic behavior.
Apply this diff:
- sendRoundRobinRescheduledEmailsAndSMS( - { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, - rescheduledMembers, - metadata - ); - sendRoundRobinScheduledEmailsAndSMS({ - calEvent: copyEventAdditionalInfo, - members: newBookedMembers, - eventTypeMetadata: metadata, - }); + await Promise.all([ + sendRoundRobinRescheduledEmailsAndSMS( + { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, + rescheduledMembers, + metadata + ), + sendRoundRobinScheduledEmailsAndSMS({ + calEvent: copyEventAdditionalInfo, + members: newBookedMembers, + eventTypeMetadata: metadata, + }), + ]); @@ - sendRoundRobinCancelledEmailsAndSMS( + await sendRoundRobinCancelledEmailsAndSMS( cancelledRRHostEvt, cancelledMembers, metadata, !!reassignedTo ? { name: reassignedTo.name, email: reassignedTo.email, ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), } : undefined );
🧹 Nitpick comments (3)
packages/features/bookings/lib/handleBookingSideEffects.ts (3)
132-141: Avoid sequential awaits; parallelize translations for attendees.Fetching translations in a loop serializes network/IO. Use
Promise.allto build the list.Apply this diff:
- for (const user of originalRescheduledBooking.attendees) { - const translate = await getTranslation(user.locale ?? "en", "common"); - originalBookingMemberEmails.push({ - name: user.name, - email: user.email, - timeZone: user.timeZone, - phoneNumber: user.phoneNumber, - language: { translate, locale: user.locale ?? "en" }, - }); - } + const attendeePersons: Person[] = await Promise.all( + originalRescheduledBooking.attendees.map(async (user) => { + const translate = await getTranslation(user.locale ?? "en", "common"); + return { + name: user.name, + email: user.email, + timeZone: user.timeZone, + phoneNumber: user.phoneNumber, + language: { translate, locale: user.locale ?? "en" }, + } as Person; + }) + ); + originalBookingMemberEmails.push(...attendeePersons);
122-122: Use the same sub-logger instance consistently.Minor consistency nit: prefer
log.debughere.Apply this diff:
- logger.debug("Emails: Sending rescheduled emails for booking confirmation"); + log.debug("Emails: Sending rescheduled emails for booking confirmation");
149-149: Guard optional time format conversion (minor).Avoid calling the formatter with undefined; harmless now but safer.
Apply this diff:
- timeFormat: getTimeFormatStringFromUserTimeFormat(originalRescheduledBooking.user.timeFormat), + timeFormat: originalRescheduledBooking.user.timeFormat + ? getTimeFormatStringFromUserTimeFormat(originalRescheduledBooking.user.timeFormat) + : undefined,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/handleBookingSideEffects.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleBookingSideEffects.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleBookingSideEffects.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/handleBookingSideEffects.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleBookingSideEffects.ts
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/handleBookingSideEffects.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
| cancelledRRHostEvt.startTime = | ||
| dayjs(originalRescheduledBooking?.startTime).utc().format() || | ||
| copyEventAdditionalInfo.startTime; | ||
| cancelledRRHostEvt.endTime = | ||
| dayjs(originalRescheduledBooking?.endTime).utc().format() || copyEventAdditionalInfo.endTime; | ||
| cancelledRRHostEvt.organizer = { | ||
| email: originalOrganizer.email, | ||
| name: originalOrganizer.name || "", | ||
| timeZone: originalOrganizer.timeZone, | ||
| language: { translate, locale: originalOrganizer.locale || "en" }, | ||
| }; |
There was a problem hiding this comment.
Guard against “Invalid Date” propagation when building cancelled event times.
dayjs(x).utc().format() returns the string "Invalid Date" for bad/empty inputs, which is truthy and will override the fallback.
Apply this diff:
- cancelledRRHostEvt.startTime =
- dayjs(originalRescheduledBooking?.startTime).utc().format() ||
- copyEventAdditionalInfo.startTime;
- cancelledRRHostEvt.endTime =
- dayjs(originalRescheduledBooking?.endTime).utc().format() || copyEventAdditionalInfo.endTime;
+ {
+ const dStart = dayjs(originalRescheduledBooking?.startTime);
+ cancelledRRHostEvt.startTime = dStart.isValid()
+ ? dStart.utc().format()
+ : copyEventAdditionalInfo.startTime;
+ const dEnd = dayjs(originalRescheduledBooking?.endTime);
+ cancelledRRHostEvt.endTime = dEnd.isValid()
+ ? dEnd.utc().format()
+ : copyEventAdditionalInfo.endTime;
+ }🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleBookingSideEffects.ts around lines
156-166, the current code uses dayjs(...).utc().format() which returns the
string "Invalid Date" (truthy) for bad or empty inputs and therefore overrides
the intended fallback; fix this by validating the parsed date before formatting:
parse originalRescheduledBooking?.startTime and .endTime into dayjs objects,
call .isValid() on each, and only assign utc().format() when valid, otherwise
fall back to copyEventAdditionalInfo.startTime / .endTime; ensure null/undefined
guards and preserve existing organizer assignment logic.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts (2)
9-16: Add observability to round-robin email paths to match withReporting coverageThe non-RR senders are wrapped with
withReporting, the RR ones are not. Consider wrapping RR senders similarly (or add structured logs here) for parity in metrics and error tracking.
277-299: Attendee[0] assumptionOnly the first attendee receives the request email/SMS. If multi-attendee requests exist, confirm this is intentional; otherwise iterate all attendees or the designated “booker”.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/bookings/lib/handleNewBooking.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts (6)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/features/bookings/lib/handleNewBooking/originalRescheduledBookingUtils.ts (1)
BookingType(24-24)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/emails/email-manager.ts (7)
sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendRescheduledEmailsAndSMS(318-321)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (3)
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts (3)
252-262: Confirm policy semantics for allowDisabling helpers*The assignment replaces the event-type disable flag with the workflow “allow” decision. Verify with tests that:
- eventType disables, workflows disallow → emails are sent
- eventType disables, workflows allow → emails suppressed
I can add a small unit spec exercising both branches if helpful.
114-122: structuredClone availabilityEnsure runtime Node/browser targets support
structuredClone. If not guaranteed, add a small fallback (or centralize in a utility) to avoid runtime errors.
290-294: Good: logs are PII-safeUsing
getPiiFreeCalendarEventwithsafeStringifykeeps logs compliant.
packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts
Outdated
Show resolved
Hide resolved
| const newBookingMemberEmails: Person[] = | ||
| copyEvent.team?.members | ||
| .map((member) => member) | ||
| .concat(copyEvent.organizer) | ||
| .concat(copyEvent.attendees) || []; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix null-safe composition of newBookingMemberEmails (can throw when team is undefined) and normalize to Person
When team is missing, copyEvent.team?.members.map(...) still calls .map on undefined. Also ensure team members conform to Person.
Apply:
- const newBookingMemberEmails: Person[] =
- copyEvent.team?.members
- .map((member) => member)
- .concat(copyEvent.organizer)
- .concat(copyEvent.attendees) || [];
+ const newBookingMemberEmails: Person[] = [
+ ...((copyEvent.team?.members?.map((m) => ({
+ name: m.name ?? "",
+ email: m.email,
+ timeZone: m.timeZone ?? copyEvent.organizer.timeZone,
+ phoneNumber: m.phoneNumber,
+ // Use organizer language as a sane default for hosts
+ language: copyEvent.organizer.language,
+ })) ?? []) as Person[]),
+ copyEvent.organizer,
+ ...copyEvent.attendees,
+ ];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const newBookingMemberEmails: Person[] = | |
| copyEvent.team?.members | |
| .map((member) => member) | |
| .concat(copyEvent.organizer) | |
| .concat(copyEvent.attendees) || []; | |
| const newBookingMemberEmails: Person[] = [ | |
| ...((copyEvent.team?.members?.map((m) => ({ | |
| name: m.name ?? "", | |
| email: m.email, | |
| timeZone: m.timeZone ?? copyEvent.organizer.timeZone, | |
| phoneNumber: m.phoneNumber, | |
| // Use organizer language as a sane default for hosts | |
| language: copyEvent.organizer.language, | |
| })) ?? []) as Person[]), | |
| copyEvent.organizer, | |
| ...copyEvent.attendees, | |
| ]; |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts around
lines 170-175, the current expression can call .map on undefined when team is
missing and does not ensure items are Person; replace with a null-safe
composition that treats team members as an empty array when absent (e.g. use
copyEvent.team?.members ?? []), map/normalize each entry to a Person shape (or
cast/transform as needed), concat organizer and attendees after normalizing
them, and finally filter out any null/undefined entries so
newBookingMemberEmails is always an array of Person.
| sendRoundRobinRescheduledEmailsAndSMS( | ||
| { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, | ||
| rescheduledMembers, | ||
| metadata | ||
| ); | ||
| sendRoundRobinScheduledEmailsAndSMS({ | ||
| calEvent: copyEventAdditionalInfo, | ||
| members: newBookedMembers, | ||
| eventTypeMetadata: metadata, | ||
| }); | ||
| const reassignedTo = users.find( | ||
| (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email) | ||
| ); | ||
| sendRoundRobinCancelledEmailsAndSMS( | ||
| cancelledRRHostEvt, | ||
| cancelledMembers, | ||
| metadata, | ||
| !!reassignedTo | ||
| ? { | ||
| name: reassignedTo.name, | ||
| email: reassignedTo.email, | ||
| ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), | ||
| } | ||
| : undefined | ||
| ); |
There was a problem hiding this comment.
Await round-robin email/SMS sends; current fire-and-forget can drop errors and return early
These three async calls aren’t awaited, unlike the non-RR paths. This can mask failures and finish the handler before emails/SMS are sent. Await them concurrently.
Apply:
- sendRoundRobinRescheduledEmailsAndSMS(
- { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID },
- rescheduledMembers,
- metadata
- );
- sendRoundRobinScheduledEmailsAndSMS({
- calEvent: copyEventAdditionalInfo,
- members: newBookedMembers,
- eventTypeMetadata: metadata,
- });
- const reassignedTo = users.find(
- (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email)
- );
- sendRoundRobinCancelledEmailsAndSMS(
- cancelledRRHostEvt,
- cancelledMembers,
- metadata,
- !!reassignedTo
- ? {
- name: reassignedTo.name,
- email: reassignedTo.email,
- ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }),
- }
- : undefined
- );
+ const reassignedTo = users.find(
+ (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email)
+ );
+ const rrTasks: Promise<unknown>[] = [
+ sendRoundRobinRescheduledEmailsAndSMS(
+ { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID },
+ rescheduledMembers,
+ metadata
+ ),
+ sendRoundRobinScheduledEmailsAndSMS({
+ calEvent: copyEventAdditionalInfo,
+ members: newBookedMembers,
+ eventTypeMetadata: metadata,
+ }),
+ sendRoundRobinCancelledEmailsAndSMS(
+ cancelledRRHostEvt,
+ cancelledMembers,
+ metadata,
+ reassignedTo
+ ? {
+ name: reassignedTo.name,
+ email: reassignedTo.email,
+ ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }),
+ }
+ : undefined
+ ),
+ ];
+ await Promise.all(rrTasks);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sendRoundRobinRescheduledEmailsAndSMS( | |
| { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, | |
| rescheduledMembers, | |
| metadata | |
| ); | |
| sendRoundRobinScheduledEmailsAndSMS({ | |
| calEvent: copyEventAdditionalInfo, | |
| members: newBookedMembers, | |
| eventTypeMetadata: metadata, | |
| }); | |
| const reassignedTo = users.find( | |
| (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email) | |
| ); | |
| sendRoundRobinCancelledEmailsAndSMS( | |
| cancelledRRHostEvt, | |
| cancelledMembers, | |
| metadata, | |
| !!reassignedTo | |
| ? { | |
| name: reassignedTo.name, | |
| email: reassignedTo.email, | |
| ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), | |
| } | |
| : undefined | |
| ); | |
| // Determine if we’ve reassigned a non-fixed user before sending any RR emails/SMS | |
| const reassignedTo = users.find( | |
| (user) => | |
| !user.isFixed && | |
| newBookedMembers.some((member) => member.email === user.email) | |
| ); | |
| // Fire off all round-robin side-effects concurrently and await them | |
| const rrTasks: Promise<unknown>[] = [ | |
| sendRoundRobinRescheduledEmailsAndSMS( | |
| { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, | |
| rescheduledMembers, | |
| metadata | |
| ), | |
| sendRoundRobinScheduledEmailsAndSMS({ | |
| calEvent: copyEventAdditionalInfo, | |
| members: newBookedMembers, | |
| eventTypeMetadata: metadata, | |
| }), | |
| sendRoundRobinCancelledEmailsAndSMS( | |
| cancelledRRHostEvt, | |
| cancelledMembers, | |
| metadata, | |
| reassignedTo | |
| ? { | |
| name: reassignedTo.name, | |
| email: reassignedTo.email, | |
| ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), | |
| } | |
| : undefined | |
| ), | |
| ]; | |
| await Promise.all(rrTasks); |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleBookingEmailsAndSmsSideEffects.ts around
lines 199 to 223 the three round-robin send functions are fire-and-forget which
can drop errors and allow the handler to finish before sends complete; replace
the separate calls with an awaited concurrent call (e.g., await
Promise.all([...])) that includes sendRoundRobinRescheduledEmailsAndSMS(...),
sendRoundRobinScheduledEmailsAndSMS(...), and
sendRoundRobinCancelledEmailsAndSMS(...), passing the exact same arguments
currently used, so all sends run in parallel but errors are propagated and the
handler waits for completion.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
2031-2049: Do not select credential.key; avoid pulling secrets unnecessarily.You don’t use
keyand loading it increases risk. Select only what you need.Apply this diff:
- const credentialPaymentAppCategories = await prisma.credential.findMany({ + const credentialPaymentAppCategories = await prisma.credential.findMany({ where: { ...(paymentAppData.credentialId ? { id: paymentAppData.credentialId } : { userId: organizerUser.id }), app: { categories: { hasSome: ["payment"], }, }, }, select: { - key: true, appId: true, app: { select: { categories: true, dirName: true, }, }, }, });
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
1832-1850: Don’t overwrite evt.iCalUID and pass only minimal eventType in side-effect payload.You’re reintroducing the clobber by setting
iCalUIDin the payload, potentially desyncing ICS threading after providers return a new UID. Also pass only{ metadata, schedulingType }as per the side-effect contract.Apply this diff:
- emailsAndSmsSideEffectsPayload = { + emailsAndSmsSideEffectsPayload = { action: BookingState.BOOKING_RESCHEDULED, data: { - evt: { - ...evt, - additionalInformation: metadata, - additionalNotes, - iCalUID, - }, - eventType, + evt: { + ...evt, + additionalInformation: metadata, + additionalNotes, + }, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, videoMetadata, originalRescheduledBooking, rescheduleReason, isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, users, changedOrganizer, }, };
🧹 Nitpick comments (3)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
129-138: Avoid serial awaits for translations; parallelize attendee translations.Fetching translations in a loop forces sequential I/O. Build
Person[]viaPromise.allfor better latency.Apply this diff:
- for (const user of originalRescheduledBooking.attendees) { - const translate = await getTranslation(user.locale ?? "en", "common"); - originalBookingMemberEmails.push({ - name: user.name, - email: user.email, - timeZone: user.timeZone, - phoneNumber: user.phoneNumber, - language: { translate, locale: user.locale ?? "en" }, - }); - } + { + const attendeesAsPeople = await Promise.all( + originalRescheduledBooking.attendees.map(async (user) => { + const translate = await getTranslation(user.locale ?? "en", "common"); + return { + name: user.name, + email: user.email, + timeZone: user.timeZone, + phoneNumber: user.phoneNumber, + language: { translate, locale: user.locale ?? "en" }, + } as Person; + }) + ); + originalBookingMemberEmails.push(...attendeesAsPeople); + }packages/features/bookings/lib/handleNewBooking.ts (2)
2326-2330: Consider isolating side-effect failures from the booking response.If email/SMS sending fails, the whole request fails despite the booking being created. Wrap in try/catch and log/report, unless product requirements demand hard-fail.
Apply this diff:
- if (emailsAndSmsSideEffectsPayload) { - const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); - await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); - } + if (emailsAndSmsSideEffectsPayload) { + try { + const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); + await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); + } catch (err) { + logger.error("Emails/SMS side-effects failed", { err }); + } + }
2343-2343: Prefer named exports over default exports.For libs, avoid default exports to improve tree-shaking and refactors.
Would you like a follow-up PR to switch
export default handler;to a named export and update imports?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/bookings/lib/BookingEmailSmsHandler.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-28T10:48:03.862Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-28T10:48:03.862Z
Learning: All host arrays emitted by findQualifiedHostsWithDelegationCredentials.ts must include groupId: string | null on each item (qualifiedRRHosts, fixedHosts, allFallbackRRHosts, and intermediate “AfterXMatching” arrays). Missing groupId triggers TS errors in BaseHost expectations.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (2)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (7)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(146-146)packages/features/bookings/lib/handleNewBooking/originalRescheduledBookingUtils.ts (1)
BookingType(24-24)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/emails/email-manager.ts (7)
sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendRescheduledEmailsAndSMS(318-321)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (2)
EmailsAndSmsSideEffectsPayload(85-88)BookingEmailSmsHandler(94-300)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
275-277: Confirm attendee selection logic.Only
attendees[0]is notified. Confirm this is always the primary booker and multi-attendee bookings don’t require notifying all invitees.
| emailsAndSmsSideEffectsPayload = { | ||
| action: BookingState.BOOKING_REQUESTED, | ||
| data: { evt, attendees: attendeesList, eventType, additionalNotes }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Trim eventType to the side-effect shape for REQUESTED flow.
Same as above; pass only the required fields.
Apply this diff:
- emailsAndSmsSideEffectsPayload = {
- action: BookingState.BOOKING_REQUESTED,
- data: { evt, attendees: attendeesList, eventType, additionalNotes },
- };
+ emailsAndSmsSideEffectsPayload = {
+ action: BookingState.BOOKING_REQUESTED,
+ data: {
+ evt,
+ attendees: attendeesList,
+ eventType: {
+ metadata: eventType.metadata,
+ schedulingType: eventType.schedulingType,
+ },
+ additionalNotes,
+ },
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| emailsAndSmsSideEffectsPayload = { | |
| action: BookingState.BOOKING_REQUESTED, | |
| data: { evt, attendees: attendeesList, eventType, additionalNotes }, | |
| }; | |
| } | |
| emailsAndSmsSideEffectsPayload = { | |
| action: BookingState.BOOKING_REQUESTED, | |
| data: { | |
| evt, | |
| attendees: attendeesList, | |
| eventType: { | |
| metadata: eventType.metadata, | |
| schedulingType: eventType.schedulingType, | |
| }, | |
| additionalNotes, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleNewBooking.ts around lines 1990-1994,
the emailsAndSmsSideEffectsPayload currently passes the full eventType; trim
eventType to the side-effect shape by replacing it with an object containing
only the fields the side-effects consume—e.g., pick id, title, slug, and
duration—so emailsAndSmsSideEffectsPayload.data.eventType only includes those
required properties.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
166-210: Fix RR recipient classification; only hosts should get Scheduled/Cancelled, and run sends concurrently.Current diff mixes attendees with hosts for “new/cancelled/rescheduled” sets, causing attendee emails to be treated as OrganizerScheduled/Cancelled. Also, the three sends run sequentially. Compute host sets from selected users and original organizer, send Scheduled/Cancelled to hosts only, and Rescheduled to attendees + unchanged hosts. Run all three with Promise.all.
Apply this diff:
- const newBookingMemberEmails: Person[] = [ - ...(copyEvent.team?.members || []), - copyEvent.organizer, - ...copyEvent.attendees, - ]; - - const matchOriginalMemberWithNewMember = (originalMember: Person, newMember: Person) => - originalMember.email === newMember.email; - - const newBookedMembers = newBookingMemberEmails.filter( - (member) => !originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); - const cancelledMembers = originalBookingMemberEmails.filter( - (member) => !newBookingMemberEmails.some((nm) => matchOriginalMemberWithNewMember(member, nm)) - ); - const rescheduledMembers = newBookingMemberEmails.filter((member) => - originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); - - try { - await sendRoundRobinRescheduledEmailsAndSMS( - { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, - rescheduledMembers, - metadata - ); - await sendRoundRobinScheduledEmailsAndSMS({ - calEvent: copyEventAdditionalInfo, - members: newBookedMembers, - eventTypeMetadata: metadata, - }); - const reassignedTo = users.find( - (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email) - ); - await sendRoundRobinCancelledEmailsAndSMS( - cancelledRRHostEvt, - cancelledMembers, - metadata, - reassignedTo - ? { - name: reassignedTo.name, - email: reassignedTo.email, - ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), - } - : undefined - ); - } catch (err) { + // Build a lookup of team members (hosts) by email + const teamMembersByEmail = new Map<string, Person>((copyEvent.team?.members || []).map((m) => [m.email, m])); + + // New hosts selected for this booking (fixed + lucky) + const newHosts: Person[] = users + .map((u) => teamMembersByEmail.get(u.email)) + .filter((p): p is Person => Boolean(p)); + + // Original host for the previous booking (if any) + const originalHosts: Person[] = originalRescheduledBooking.user + ? [teamMembersByEmail.get(originalRescheduledBooking.user.email) ?? copyEvent.organizer] + : []; + + const newlyAssignedHosts = newHosts.filter((nh) => !originalHosts.some((oh) => oh.email === nh.email)); + const removedHosts = originalHosts.filter((oh) => !newHosts.some((nh) => nh.email === oh.email)); + const commonHosts = newHosts.filter((nh) => originalHosts.some((oh) => oh.email === nh.email)); + + const reassignedTo = users.find( + (u) => !u.isFixed && newlyAssignedHosts.some((m) => m.email === u.email) + ); + + try { + await Promise.all([ + // Rescheduled: attendees + any host that stayed the same + sendRoundRobinRescheduledEmailsAndSMS( + { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, + [...copyEvent.attendees, ...commonHosts], + metadata + ), + // Scheduled: only newly assigned hosts + sendRoundRobinScheduledEmailsAndSMS({ + calEvent: copyEventAdditionalInfo, + members: newlyAssignedHosts, + eventTypeMetadata: metadata, + }), + // Cancelled: only previously assigned hosts that were removed + sendRoundRobinCancelledEmailsAndSMS( + cancelledRRHostEvt, + removedHosts, + metadata, + reassignedTo + ? { + name: reassignedTo.name, + email: reassignedTo.email, + ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), + } + : undefined + ), + ]); + } catch (err) { this.log.error("Failed to send rescheduled round robin event related emails", err); }
🧹 Nitpick comments (3)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (3)
277-281: Send attendee request emails/SMS to all attendees, not just the first.Currently only attendees[0] receives the request. Broadcast to each attendee.
- await sendOrganizerRequestEmail(eventWithNotes, metadata); - await sendAttendeeRequestEmailAndSMS(eventWithNotes, attendees[0], metadata); + await Promise.all([ + sendOrganizerRequestEmail(eventWithNotes, metadata), + ...attendees.map((a) => sendAttendeeRequestEmailAndSMS(eventWithNotes, a, metadata)), + ]);
116-124: Avoid deep cloning hot objects unless necessary.cloneDeep on CalendarEvent is expensive. A shallow copy suffices since you overwrite specific props into new objects.
-import { default as cloneDeep } from "lodash/cloneDeep"; +// cloneDeep avoided; shallow copies below are sufficient- const copyEvent = cloneDeep(evt); + const copyEvent = { ...evt }; @@ - const cancelledRRHostEvt = cloneDeep(copyEventAdditionalInfo); + const cancelledRRHostEvt = { ...copyEventAdditionalInfo };Also applies to: 1-1
129-138: Cache translations per locale to cut N× awaits.getTranslation is awaited per user; reuse per-locale results.
Example (sketch):
- const tByLocale = new Map<string, TFunction>();
- const getT = async (loc: string) => tByLocale.get(loc) ?? (tByLocale.set(loc, await getTranslation(loc, "common")), tByLocale.get(loc)!);
- Replace awaits with: const translate = await getT(user.locale ?? "en");
Also applies to: 140-149
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (6)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/prisma/zod-utils.ts (1)
EventTypeMetadata(146-146)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/emails/email-manager.ts (7)
sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendRescheduledEmailsAndSMS(318-321)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (3)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (3)
90-103: Good abstraction: centralizes email/SMS side effects behind a typed handler.Enum + discriminated union payload + per-action private methods keep concerns isolated and composable. Constructor sub-logger is a nice touch.
269-276: Nice: PII-safe debug logging.Using getPiiFreeCalendarEvent + safeStringify avoids accidental leakage.
238-247: No changes needed. TheallowDisabling…helpers return “allowed-to-disable,” and the code correctly computes
is…Disabled = metadataFlag && allowDisabling…, so the flags passed tosendScheduledEmailsAndSMSmatch the intended behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
1840-1859: Don’t override evt.iCalUID; trim eventType to the side-effect shape.Overwriting evt.iCalUID risks breaking ICS threading; the handler only needs eventType.metadata and eventType.schedulingType, so avoid passing the full object. This aligns with the handler’s destructuring and avoids oversharing.
Apply this diff:
- if (noEmail !== true && isConfirmedByDefault && !isDryRun) { - emailsAndSmsSideEffectsPayload = { - action: BookingState.BOOKING_RESCHEDULED, - data: { - evt: { - ...evt, - additionalInformation: metadata, - additionalNotes, - customInputs, - iCalUID, - }, - eventType, - videoMetadata, - originalRescheduledBooking, - rescheduleReason, - isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, - users, - changedOrganizer, - }, - }; - } + if (noEmail !== true && isConfirmedByDefault && !isDryRun) { + emailsAndSmsSideEffectsPayload = { + action: BookingState.BOOKING_RESCHEDULED, + data: { + evt: { + ...evt, + additionalInformation: metadata, + additionalNotes, + customInputs, + // keep evt.iCalUID as set by EventManager; do not override here + }, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + videoMetadata, + originalRescheduledBooking, + rescheduleReason, + isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, + users, + changedOrganizer, + }, + }; + }
🧹 Nitpick comments (3)
packages/features/bookings/lib/handleNewBooking.ts (3)
1655-1655: Helpful ACTION marker.Keeps the high-level flow easy to scan.
2012-2024: Trim eventType for REQUESTED flow (and avoid redundant notes in evt).Pass only { metadata, schedulingType }. optional: avoid duplicating additionalNotes inside evt since it’s provided top-level for the handler.
Apply this diff:
- emailsAndSmsSideEffectsPayload = { - action: BookingState.BOOKING_REQUESTED, - data: { - evt: { - ...evt, - additionalNotes, - customInputs, - }, - attendees: attendeesList, - eventType, - additionalNotes, - }, - }; + emailsAndSmsSideEffectsPayload = { + action: BookingState.BOOKING_REQUESTED, + data: { + evt: { + ...evt, + // additionalNotes is supplied top-level; no need to duplicate + customInputs, + }, + attendees: attendeesList, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + additionalNotes, + }, + };
2403-2403: Prefer named export over default export.Improves tree-shaking and refactors. Consider in a follow-up to avoid broad import changes.
Apply this diff (follow-up PR recommended):
-export default handler; +export { handler };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/handleNewBooking.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-28T10:48:03.862Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-28T10:48:03.862Z
Learning: All host arrays emitted by findQualifiedHostsWithDelegationCredentials.ts must include groupId: string | null on each item (qualifiedRRHosts, fixedHosts, allFallbackRRHosts, and intermediate “AfterXMatching” arrays). Missing groupId triggers TS errors in BaseHost expectations.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (2)
EmailsAndSmsSideEffectsPayload(85-88)BookingEmailSmsHandler(94-304)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (4)
packages/features/bookings/lib/handleNewBooking.ts (4)
95-99: Good modularization: centralized email/SMS side-effects import.The move to a dedicated handler and typed payloads improves separation of concerns.
435-435: Side-effects accumulator is clear and scoped.Init with a precise union keeps control flow simple at the call site.
990-991: Non-empty host-groups filter reads cleanly.Destructuring with an unused
_groupIdis fine and explicit.
1966-1982: CONFIRMED payload shape looks correct.eventType is trimmed; evt carries only what’s needed. Good.
| // Side Effects | ||
| if (emailsAndSmsSideEffectsPayload) { | ||
| const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); | ||
| await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Shield booking response from email/SMS failures.
Wrap side-effect dispatch in try/catch so transient email/SMS errors don’t fail an otherwise successful booking.
Apply this diff:
// Side Effects
if (emailsAndSmsSideEffectsPayload) {
- const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger });
- await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload);
+ try {
+ const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger });
+ await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload);
+ } catch (err) {
+ logger.error("Emails/SMS side-effects failed", err);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Side Effects | |
| if (emailsAndSmsSideEffectsPayload) { | |
| const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); | |
| await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); | |
| } | |
| // Side Effects | |
| if (emailsAndSmsSideEffectsPayload) { | |
| try { | |
| const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); | |
| await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); | |
| } catch (err) { | |
| logger.error("Emails/SMS side-effects failed", err); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/features/bookings/lib/handleNewBooking.ts around lines 2386 to 2390,
the dispatch of email/SMS side-effects is currently unprotected and can cause
the whole booking to fail if external delivery errors occur; wrap the call to
emailsAndSmsHandler.send(...) in a try/catch, await it inside the try, and on
error log the failure with logger.error including context (e.g., bookingId and
the error) but do not rethrow so the booking flow continues; keep original
behavior when there is no payload.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
170-188: Fix RR recipient classification and await all sends concurrently.Current logic mixes attendees with hosts and sends organizer-specific emails to attendees; also sends are awaited serially. Derive host sets from
usersand the original organizer, andPromise.allthe three sends.Apply this diff:
- const newBookingMemberEmails: Person[] = [ - ...(copyEvent.team?.members || []), - copyEvent.organizer, - ...copyEvent.attendees, - ]; - - const matchOriginalMemberWithNewMember = (originalMember: Person, newMember: Person) => - originalMember.email === newMember.email; - - const newBookedMembers = newBookingMemberEmails.filter( - (member) => !originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); - const cancelledMembers = originalBookingMemberEmails.filter( - (member) => !newBookingMemberEmails.some((nm) => matchOriginalMemberWithNewMember(member, nm)) - ); - const rescheduledMembers = newBookingMemberEmails.filter((member) => - originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); - - try { - await sendRoundRobinRescheduledEmailsAndSMS( - { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, - rescheduledMembers, - metadata - ); - await sendRoundRobinScheduledEmailsAndSMS({ - calEvent: copyEventAdditionalInfo, - members: newBookedMembers, - eventTypeMetadata: metadata, - }); - const reassignedTo = users.find( - (user) => !user.isFixed && newBookedMembers.some((member) => member.email === user.email) - ); - await sendRoundRobinCancelledEmailsAndSMS( - cancelledRRHostEvt, - cancelledMembers, - metadata, - reassignedTo - ? { - name: reassignedTo.name, - email: reassignedTo.email, - ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), - } - : undefined - ); - } catch (err) { + // Build host sets using selected users and the original organizer + const teamMembersByEmail = new Map<string, Person>( + (copyEvent.team?.members || []).map((m) => [m.email, m]) + ); + const newHosts: Person[] = users + .map((u) => teamMembersByEmail.get(u.email)) + .filter((p): p is Person => Boolean(p)); + + const originalHosts: Person[] = originalRescheduledBooking.user + ? [teamMembersByEmail.get(originalRescheduledBooking.user.email) || copyEvent.organizer].filter( + (p): p is Person => Boolean(p) + ) + : []; + + const newlyAssignedHosts = newHosts.filter((nh) => !originalHosts.some((oh) => oh.email === nh.email)); + const removedHosts = originalHosts.filter((oh) => !newHosts.some((nh) => nh.email === oh.email)); + const commonHosts = newHosts.filter((nh) => originalHosts.some((oh) => oh.email === nh.email)); + + const reassignedTo = users.find( + (u) => !u.isFixed && newlyAssignedHosts.some((m) => m.email === u.email) + ); + + try { + await Promise.all([ + // Rescheduled: attendees + unchanged hosts + sendRoundRobinRescheduledEmailsAndSMS( + { ...copyEventAdditionalInfo, iCalUID: evt.iCalUID }, + [...copyEvent.attendees, ...commonHosts], + metadata + ), + // Scheduled: only newly assigned hosts + sendRoundRobinScheduledEmailsAndSMS({ + calEvent: copyEventAdditionalInfo, + members: newlyAssignedHosts, + eventTypeMetadata: metadata, + }), + // Cancelled: only previously assigned hosts that were removed + sendRoundRobinCancelledEmailsAndSMS( + cancelledRRHostEvt, + removedHosts, + metadata, + reassignedTo + ? { + name: reassignedTo.name, + email: reassignedTo.email, + ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), + } + : undefined + ), + ]); + } catch (err) { this.log.error("Failed to send rescheduled round robin event related emails", err); }Also applies to: 189-214
packages/features/bookings/lib/handleNewBooking.ts (3)
1841-1859: Don’t clobber evt.iCalUID and trim eventType to the side-effect shape.Overwriting
evt.iCalUIDwith a stale value can break ICS threading; alsodata.eventTypemust only contain{ metadata, schedulingType }per the declared payload type.Apply this diff:
- emailsAndSmsSideEffectsPayload = { - action: BookingState.BOOKING_RESCHEDULED, - data: { - evt: { - ...evt, - additionalInformation: metadata, - additionalNotes, - customInputs, - iCalUID, - }, - eventType, - videoMetadata, - originalRescheduledBooking, - rescheduleReason, - isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, - users, - changedOrganizer, - }, - }; + emailsAndSmsSideEffectsPayload = { + action: BookingState.BOOKING_RESCHEDULED, + data: { + evt: { + ...evt, + additionalInformation: metadata, + additionalNotes, + customInputs, + }, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + videoMetadata, + originalRescheduledBooking, + rescheduleReason, + isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, + users, + changedOrganizer, + }, + };Run to confirm only the minimal eventType shape is used in all side-effect payloads:
#!/bin/bash rg -n -C2 'action:\s*BookingState\.' --type ts | sed -n '1,2400p'
2384-2388: Shield booking response from email/SMS failures.External send errors should not fail an otherwise successful booking.
- // Side Effects - if (emailsAndSmsSideEffectsPayload) { - const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); - await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); - } + // Side Effects + if (emailsAndSmsSideEffectsPayload) { + try { + const emailsAndSmsHandler = new BookingEmailSmsHandler({ logger }); + await emailsAndSmsHandler.send(emailsAndSmsSideEffectsPayload); + } catch (err) { + logger.error("Emails/SMS side-effects failed", err); + } + }
2011-2022: Trim eventType for REQUESTED flow to match payload contract.Pass only
{ metadata, schedulingType }; current code leaks the full eventType and likely breaks TS types.- emailsAndSmsSideEffectsPayload = { - action: BookingState.BOOKING_REQUESTED, - data: { - evt: { - ...evt, - additionalNotes, - customInputs, - }, - attendees: attendeesList, - eventType, - additionalNotes, - }, - }; + emailsAndSmsSideEffectsPayload = { + action: BookingState.BOOKING_REQUESTED, + data: { + evt: { + ...evt, + additionalNotes, + customInputs, + }, + attendees: attendeesList, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + additionalNotes, + }, + };
🧹 Nitpick comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
2401-2401: Prefer named exports over default exports.Improves tree-shaking and refactor safety.
-export default handler; +export { handler };Note: apply repo-wide import updates in a follow-up PR to keep this change scoped.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/bookings/lib/BookingEmailSmsHandler.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/BookingEmailSmsHandler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/BookingEmailSmsHandler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/BookingEmailSmsHandler.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-28T10:48:03.862Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-28T10:48:03.862Z
Learning: All host arrays emitted by findQualifiedHostsWithDelegationCredentials.ts must include groupId: string | null on each item (qualifiedRRHosts, fixedHosts, allFallbackRRHosts, and intermediate “AfterXMatching” arrays). Missing groupId triggers TS errors in BaseHost expectations.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (6)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/features/bookings/lib/handleNewBooking/originalRescheduledBookingUtils.ts (1)
BookingType(24-24)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/emails/email-manager.ts (7)
sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendRescheduledEmailsAndSMS(318-321)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
🔇 Additional comments (2)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
31-38: Nice public API and payload modeling.Clear enum, minimal eventType surface, and well-typed side-effect payloads make this easy to reason about.
Also applies to: 40-93
packages/features/bookings/lib/handleNewBooking.ts (1)
990-991: Good readability nit: tuple destructuring in filter.Cleaner than indexing into the [key, value] pair.
Pull request was converted to draft
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (3)
227-251: Consider Promise.allSettled for partial-delivery tolerance.If one delivery fails, others still go out; log individual failures. Up to product requirements.
44-56: Remove iCalUID from payload to avoid ICS threading regressions.Don’t carry iCalUID in the side-effect payload; rely on evt.iCalUID set by EventManager. Also remove its usages downstream.
type RescheduleEmailAndSmsPayload = EmailAndSmsPayload & { rescheduleReason?: string; additionalInformation: AdditionalInformation; additionalNotes: string | null | undefined; - iCalUID: string; users: (Pick<User, "id" | "name" | "timeZone" | "locale" | "email"> & { destinationCalendar: DestinationCalendar | null; isFixed?: boolean; })[]; changedOrganizer?: boolean; isRescheduledByBooker: boolean; originalRescheduledBooking: NonNullable<BookingType>; };
142-155: Fix RR recipient classification; don’t email attendees as hosts; drop iCalUID here.Current diffing mixes attendees, organizer, and team members, so host-only emails go to attendees. Build host sets from selected users and original organizer; send host emails to hosts only; rescheduled emails to attendees + unchanged hosts. Also, don’t pass iCalUID here.
private async _handleRoundRobinRescheduled(data: RescheduleEmailAndSmsPayload) { const { evt, eventType: { metadata }, originalRescheduledBooking, rescheduleReason, additionalNotes, changedOrganizer, additionalInformation, users, isRescheduledByBooker, - iCalUID, } = data; @@ - const newBookingMemberEmails: Person[] = [ - ...(copyEvent.team?.members || []), - copyEvent.organizer, - ...copyEvent.attendees, - ]; - - const matchOriginalMemberWithNewMember = (originalMember: Person, newMember: Person) => - originalMember.email === newMember.email; - - const newBookedMembers = newBookingMemberEmails.filter( - (member) => !originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); - const cancelledMembers = originalBookingMemberEmails.filter( - (member) => !newBookingMemberEmails.some((nm) => matchOriginalMemberWithNewMember(member, nm)) - ); - const rescheduledMembers = newBookingMemberEmails.filter((member) => - originalBookingMemberEmails.some((om) => matchOriginalMemberWithNewMember(om, member)) - ); + // Build accurate host sets + const teamMembersByEmail = new Map<string, Person>((copyEvent.team?.members || []).map((m) => [m.email, m])); + const newHosts: Person[] = users + .map((u) => teamMembersByEmail.get(u.email)) + .filter((p): p is Person => Boolean(p)); + const originalHosts: Person[] = originalRescheduledBooking.user + ? [teamMembersByEmail.get(originalRescheduledBooking.user.email) || copyEvent.organizer].filter( + (p): p is Person => Boolean(p) + ) + : []; + const newlyAssignedHosts = newHosts.filter((nh) => !originalHosts.some((oh) => oh.email === nh.email)); + const removedHosts = originalHosts.filter((oh) => !newHosts.some((nh) => nh.email === oh.email)); + const unchangedHosts = newHosts.filter((nh) => originalHosts.some((oh) => oh.email === nh.email)); @@ - await Promise.all([ - sendRoundRobinRescheduledEmailsAndSMS( - { ...copyEventAdditionalInfo, iCalUID }, - rescheduledMembers, - metadata - ), - sendRoundRobinScheduledEmailsAndSMS({ - calEvent: copyEventAdditionalInfo, - members: newBookedMembers, - eventTypeMetadata: metadata, - }), - sendRoundRobinCancelledEmailsAndSMS( - cancelledRRHostEvt, - cancelledMembers, - metadata, - reassignedTo - ? { - name: reassignedTo.name, - email: reassignedTo.email, - ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), - } - : undefined - ), - ]); + await Promise.all([ + // Rescheduled: attendees + unchanged hosts + sendRoundRobinRescheduledEmailsAndSMS( + { ...copyEventAdditionalInfo }, + [...copyEvent.attendees, ...unchangedHosts], + metadata + ), + // Scheduled: newly assigned hosts + sendRoundRobinScheduledEmailsAndSMS({ + calEvent: copyEventAdditionalInfo, + members: newlyAssignedHosts, + eventTypeMetadata: metadata, + }), + // Cancelled: removed hosts + sendRoundRobinCancelledEmailsAndSMS( + cancelledRRHostEvt, + removedHosts, + metadata, + reassignedTo + ? { + name: reassignedTo.name, + email: reassignedTo.email, + ...(isRescheduledByBooker && { reason: "Booker Rescheduled" }), + } + : undefined + ), + ]);Also applies to: 204-221, 227-251
packages/features/bookings/lib/handleNewBooking.ts (3)
1835-1851: Trim eventType, drop iCalUID, and don’t fail booking on email/SMS errors.Send only { metadata, schedulingType }; do not pass iCalUID; wrap send in try/catch.
- if (!noEmail && isConfirmedByDefault && !isDryRun) { - await emailsAndSmsHandler.send({ - action: BookingActionMap.rescheduled, - data: { - evt, - eventType, - additionalInformation: metadata, - additionalNotes, - iCalUID, - originalRescheduledBooking, - rescheduleReason, - isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, - users, - changedOrganizer, - }, - }); - } + if (!noEmail && isConfirmedByDefault && !isDryRun) { + try { + await emailsAndSmsHandler.send({ + action: BookingActionMap.rescheduled, + data: { + evt, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + additionalInformation: metadata, + additionalNotes, + originalRescheduledBooking, + rescheduleReason, + isRescheduledByBooker: reqBody.rescheduledBy === bookerEmail, + users, + changedOrganizer, + }, + }); + } catch (err) { + loggerWithEventDetails.error("Emails/SMS side-effects failed (rescheduled)", err); + } + }
1955-1972: Shield booking from notify failures (confirmed flow).Wrap
.send()in try/catch to avoid aborting successful bookings due to delivery transient errors.- if (!noEmail) { - if (!isDryRun && !(eventType.seatsPerTimeSlot && rescheduleUid)) { - await emailsAndSmsHandler.send({ + if (!noEmail) { + if (!isDryRun && !(eventType.seatsPerTimeSlot && rescheduleUid)) { + try { + await emailsAndSmsHandler.send({ action: BookingActionMap.confirmed, data: { eventType: { metadata: eventType.metadata, schedulingType: eventType.schedulingType, }, eventNameObject, workflows, evt, additionalInformation, additionalNotes, customInputs, }, - }); + }); + } catch (err) { + loggerWithEventDetails.error("Emails/SMS side-effects failed (confirmed)", err); + } } }
2001-2004: Same here: trim eventType and catch errors (requested flow).Match handler’s type and keep booking resilient.
- if (!isDryRun) { - await emailsAndSmsHandler.send({ - action: BookingActionMap.requested, - data: { evt, attendees: attendeesList, eventType, additionalNotes }, - }); - } + if (!isDryRun) { + try { + await emailsAndSmsHandler.send({ + action: BookingActionMap.requested, + data: { + evt, + attendees: attendeesList, + eventType: { + metadata: eventType.metadata, + schedulingType: eventType.schedulingType, + }, + additionalNotes, + }, + }); + } catch (err) { + loggerWithEventDetails.error("Emails/SMS side-effects failed (requested)", err); + } + }
🧹 Nitpick comments (5)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (3)
102-114: Use enum constant for scheduling type comparison.Avoid string literal typos; prefer SchedulingType.ROUND_ROBIN.
- if (action === BookingActionMap.rescheduled) { - if (data.eventType.schedulingType === "ROUND_ROBIN") return this._handleRoundRobinRescheduled(data); + if (action === BookingActionMap.rescheduled) { + if (data.eventType.schedulingType === SchedulingType.ROUND_ROBIN) return this._handleRoundRobinRescheduled(data); return this._handleRescheduled(data); }
163-164: Clarify log message.Message says “for booking confirmation” inside the reschedule handler; adjust to “round-robin reschedule”.
- this.log.debug("Emails: Sending rescheduled emails for booking confirmation"); + this.log.debug("Emails: Sending round-robin rescheduled emails");
167-176: Parallelize translations to avoid N+1 awaits.Collect translations via Promise.all and then map, instead of awaiting in a loop. Minor but cleaner.
Also applies to: 177-188
packages/features/bookings/lib/handleNewBooking.ts (2)
492-493: Inject BookingEmailSmsHandler via deps for testability.Pass the handler through handler/RegularBookingService dependencies instead of instantiating inline.
1650-1650: Remove stale comment.Redundant with surrounding code; drop “this is the actual rescheduling logic”.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
apps/api/v2/package.json(1 hunks)packages/features/bookings/lib/BookingEmailSmsHandler.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/v2/package.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
📚 Learning: 2025-08-27T16:39:38.192Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/bookings/lib/BookingEmailSmsHandler.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-08T10:26:13.362Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts:212-220
Timestamp: 2025-08-08T10:26:13.362Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/providers/retellAI/services/PhoneNumberService.ts should include the phone number in client-facing HttpError messages (e.g., in updatePhoneNumber/getPhoneNumber catch blocks). Do not suggest redacting the phone number from these errors unless requirements change (per maintainer: Udit-takkar).
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-28T10:48:03.862Z
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-28T10:48:03.862Z
Learning: All host arrays emitted by findQualifiedHostsWithDelegationCredentials.ts must include groupId: string | null on each item (qualifiedRRHosts, fixedHosts, allFallbackRRHosts, and intermediate “AfterXMatching” arrays). Missing groupId triggers TS errors in BaseHost expectations.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (2)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (6)
packages/types/Calendar.d.ts (3)
CalendarEvent(163-226)AdditionalInformation(239-243)Person(33-45)packages/features/bookings/lib/handleNewBooking/originalRescheduledBookingUtils.ts (1)
BookingType(24-24)packages/lib/event.ts (1)
EventNameObjectType(19-29)packages/emails/email-manager.ts (7)
sendRescheduledEmailsAndSMS(318-321)sendRoundRobinRescheduledEmailsAndSMS(187-218)sendRoundRobinScheduledEmailsAndSMS(159-185)sendRoundRobinCancelledEmailsAndSMS(236-259)sendScheduledEmailsAndSMS(153-156)sendOrganizerRequestEmail(433-436)sendAttendeeRequestEmailAndSMS(451-454)packages/lib/timeFormat.ts (1)
getTimeFormatStringFromUserTimeFormat(27-29)packages/lib/piiFreeData.ts (1)
getPiiFreeCalendarEvent(14-36)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/bookings/lib/BookingEmailSmsHandler.ts (2)
BookingEmailSmsHandler(95-325)BookingActionMap(30-34)
4 participants
What does this PR do?
What does this PR do?
This pull request refactors the monolithic handleNewBooking function by extracting all email and SMS sending logic into a new, dedicated BookingEmailSmsHandler class.
This new class centralizes the responsibility for sending notifications for different booking states:
The main handleNewBooking function is updated to instantiate this handler and delegate all email/SMS side effects to it, making the primary booking flow cleaner and more focused.
Why is this change needed?
The handleNewBooking function had grown significantly in complexity, mixing core booking creation logic with numerous side effects, particularly notifications. This made the function difficult to read, maintain, and test.
This refactor achieves several key goals:
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
already covered by unit and e2e tests