Skip to content

fix: improve cancellation email of reschedule RR booking #17768

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

kart1ka
Copy link
Contributor

@kart1ka kart1ka commented Nov 20, 2024

What does this PR do?

Screencast.from.21-11-24.12.49.55.AM.IST.webm

How should this be tested?

  1. Create a Round Robin Event and make sure that Reschedule with same Round-Robin host is toggled off in the Event Advanced Settings Tab.
  2. Book the event.
  3. Now reschedule the booking to a different date.
  4. Check the mails and notice that the cancellation mail sent to the old round robin host shows old date and it also shows the details of the member the rescheduled booking has been reassigned to.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

Copy link

vercel bot commented Nov 20, 2024

@kart1ka is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Nov 20, 2024
@graphite-app graphite-app bot requested a review from a team November 20, 2024 19:27
@github-actions github-actions bot added emails area: emails, cancellation email, reschedule email, inbox, spam folder, not getting email Medium priority Created by Linear-GitHub Sync 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Nov 20, 2024
@dosubot dosubot bot added the bookings area: bookings, availability, timezones, double booking label Nov 20, 2024
@dosubot dosubot bot added this to the v4.8 milestone Nov 20, 2024
Copy link

graphite-app bot commented Nov 20, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (11/20/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (11/20/24)

1 label was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (11/21/24)

1 label was added to this PR based on Keith Williams's automation.

@Praashh Praashh self-assigned this Nov 20, 2024
Copy link
Contributor

@Praashh Praashh left a comment

Choose a reason for hiding this comment

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

Screenshot 2024-11-21 at 11 10 16 AM

Issue is still there, it's taking reschedule date not the old one in the mail.

@kart1ka
Copy link
Contributor Author

kart1ka commented Nov 21, 2024

Hi @Praashh
I have tested it again and it is working (see below screenshots).
I can see in the screenshot you have attached that Reassigned to section is also not showing in the mail. Please make sure you are trying to reschedule Round Robin booking.

Screenshot of cancelled mail sent to the old host (after the round robin booking has been rescheduled) with old date and reassigned to displayed

image

Screenshot of scheduled mail sent to the new host (after the round robin booking has been rescheduled) with new date displayed

image

@Praashh
Copy link
Contributor

Praashh commented Nov 21, 2024

Hi @Praashh I have tested it again and it is working (see below screenshots). I can see in the screenshot you have attached that Reassigned to section is also not showing in the mail. Please make sure you are trying to reschedule Round Robin booking.

Screenshot of cancelled mail sent to the old host (after the round robin booking has been rescheduled) with old date and reassigned to displayed

image

Screenshot of scheduled mail sent to the new host (after the round robin booking has been rescheduled) with new date displayed

image

i rescheduled and cancel the meet, it was same but i'll check it again

@kart1ka
Copy link
Contributor Author

kart1ka commented Nov 21, 2024

Hi @Praashh I have tested it again and it is working (see below screenshots). I can see in the screenshot you have attached that Reassigned to section is also not showing in the mail. Please make sure you are trying to reschedule Round Robin booking.
Screenshot of cancelled mail sent to the old host (after the round robin booking has been rescheduled) with old date and reassigned to displayed
image
Screenshot of scheduled mail sent to the new host (after the round robin booking has been rescheduled) with new date displayed
image

i rescheduled and cancel the meet, it was same but i'll check it again

Here are the steps that i am using to test my solution:

  1. Create a Round Robin Event and make sure that Reschedule with same Round-Robin host is toggled off in the Event Advanced Settings Tab.
  2. Book the event.
  3. Now reschedule the booking to a different date.
  4. Check the mails and notice that the cancellation mail sent to the old round robin host shows old date and it also shows the details of the member the rescheduled booking has been reassigned to.

@kart1ka
Copy link
Contributor Author

kart1ka commented Nov 21, 2024

A quick question.
While rescheduling a booking, all form fields are disabled except for Add Guests and Reason for Reschedule (see the screenshot below). Is the Add Guests field supposed to be enabled when rescheduling a booking?
This is causing unexpected behaviour in some edge cases.

image

@Praashh
Copy link
Contributor

Praashh commented Nov 21, 2024

A quick question. While rescheduling a booking, all form fields are disabled except for Add Guests and Reason for Reschedule (see the screenshot below). Is the Add Guests field supposed to be enabled when rescheduling a booking? This is causing unexpected behaviour in some edge cases.

image

yes add guest field is supposed to be enabled while rescheduling a booking

Praashh
Praashh previously approved these changes Nov 21, 2024
Copy link
Contributor

@Praashh Praashh left a comment

Choose a reason for hiding this comment

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

LGTM!!

Copy link
Contributor

github-actions bot commented Nov 21, 2024

E2E results are ready!

@github-actions github-actions bot marked this pull request as draft November 21, 2024 21:23
@kart1ka kart1ka marked this pull request as ready for review November 22, 2024 15:02
@kart1ka
Copy link
Contributor Author

kart1ka commented Mar 4, 2025

@kart1ka can you fix the conflicts🙏

Hi @anikdhabal Conflicts resolved.

@dosubot dosubot bot removed this from the v5.1 milestone Mar 14, 2025
@github-actions github-actions bot added consumer Low priority Created by Linear-GitHub Sync labels May 4, 2025
@kart1ka
Copy link
Contributor Author

kart1ka commented May 4, 2025

Hey @anikdhabal

Just checking on the status of this PR.
I have addressed the changes requested by @hariombalhara.
Do let me know if any other changes need to be made.
Thanks.

Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

Hey @kart1ka I’ve fixed this issue in this PR: #21390. The only thing left to fix is the email title and subtitle—we’re currently sending the cancellation email to the old host. I’ve updated the issue accordingly.
Also, you can check all the behavior on the latest branch.

@kart1ka kart1ka requested a review from a team as a code owner July 8, 2025 16:41
@keithwillcode keithwillcode added the community-interns The team responsible for reviewing, testing and shipping low/medium community PRs label Jul 8, 2025
@kart1ka
Copy link
Contributor Author

kart1ka commented Jul 8, 2025

Hey @anikdhabal, I’ve updated the title and subtitle as discussed and verified that everything else is working as expected.
image

Note: While merging main into this branch, some linting fixes were automatically applied, resulting in whitespace or newline only changes in a few files. Let me know if you’d like me to revert those.

@anikdhabal
Copy link
Contributor

@kart1ka yeah let's keep only the main changes

@kart1ka
Copy link
Contributor Author

kart1ka commented Jul 9, 2025

@anikdhabal I have merged the main branch again and those unwanted changes seem to have gone. Pls review.

@kart1ka kart1ka dismissed stale reviews from Praashh and hariombalhara July 16, 2025 02:31

stale

Copy link
Contributor

coderabbitai bot commented Aug 7, 2025

Walkthrough

This PR updates round-robin reschedule notifications to explicitly handle reassignment. Email sending logic adds a reassigned context: OrganizerCancelledEmail now conditionally renders reassigned-specific subject/title/subtitle and passes a reassigned payload (including optional reason). A new sendRoundRobinReassignedEmailsAndSMS function is introduced and used for manual reassignment paths; sendRoundRobinCancelledEmailsAndSMS accepts an extended reassignedTo argument and forwards reassigned to templates. BaseScheduledEmail displays a reason when provided. Tests are updated to reflect reassignment behavior and to expect an additional email to the previous organizer when the booker reschedules.

Assessment against linked issues

Objective Addressed Explanation
Make clear in cancellation email that booking was rescheduled to a different host (CAL-4585, #17253)
Cancellation email should show old date instead of new date (CAL-4585, #17253) No changes indicate adjusting displayed date; date rendering not shown in diff.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
Add sending of EventCancelledSMS to team members in sendRoundRobinCancelledEmailsAndSMS and sendRoundRobinReassignedEmailsAndSMS (packages/emails/email-manager.ts) SMS behavior is not mentioned in the issues; unrelated to clarifying cancellation vs reassignment or date display.

Possibly related PRs

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/emails/email-manager.ts (1)

261-285: Well-implemented reassignment function with proper parameter structure.

The function correctly uses object destructuring for named parameters as requested in past reviews. The logic properly handles:

  • Host email disable checks
  • Promise aggregation for concurrent sending
  • SMS notifications for team members with phone numbers

However, there's significant code duplication between this function and sendRoundRobinCancelledEmailsAndSMS. Consider extracting the common SMS/email sending logic into a shared helper function to follow DRY principles.

+// Helper function to reduce duplication
+const sendRoundRobinNotificationEmails = async (
+  calEvent: CalendarEvent,
+  members: Person[],
+  emailFactory: (calendarEvent: CalendarEvent, teamMember: Person) => BaseEmail,
+  eventTypeMetadata?: EventTypeMetadata
+) => {
+  if (eventTypeDisableHostEmail(eventTypeMetadata)) return;
+  const calendarEvent = formatCalEvent(calEvent);
+  const emailsAndSMSToSend: Promise<unknown>[] = [];
+  const smsService = new EventCancelledSMS(calEvent);
+  
+  for (const teamMember of members) {
+    emailsAndSMSToSend.push(sendEmail(() => emailFactory(calendarEvent, teamMember)));
+    if (teamMember.phoneNumber) {
+      emailsAndSMSToSend.push(smsService.sendSMSToAttendee(teamMember));
+    }
+  }
+  
+  await Promise.all(emailsAndSMSToSend);
+};

Also, based on past review comments about SMS/email behavior alignment, please verify that both functions should use the same SMS class (EventCancelledSMS).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 895ed48 and a9f2601.

📒 Files selected for processing (11)
  • apps/web/test/utils/bookingScenario/expects.ts (2 hunks)
  • packages/emails/email-manager.ts (1 hunks)
  • packages/emails/src/templates/BaseScheduledEmail.tsx (1 hunks)
  • packages/emails/src/templates/OrganizerCancelledEmail.tsx (1 hunks)
  • packages/emails/templates/organizer-cancelled-email.ts (2 hunks)
  • packages/features/bookings/lib/handleNewBooking.ts (2 hunks)
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (2 hunks)
  • packages/features/ee/round-robin/roundRobinManualReassignment.test.ts (2 hunks)
  • packages/features/ee/round-robin/roundRobinManualReassignment.ts (3 hunks)
  • packages/features/ee/round-robin/roundRobinReassignment.ts (3 hunks)
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts (2 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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/ee/round-robin/roundRobinManualReassignment.test.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
  • packages/features/ee/round-robin/roundRobinReassignment.ts
  • packages/features/ee/round-robin/roundRobinManualReassignment.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/emails/email-manager.ts
  • packages/emails/templates/organizer-cancelled-email.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/ee/round-robin/roundRobinManualReassignment.test.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/emails/src/templates/OrganizerCancelledEmail.tsx
  • packages/emails/src/templates/BaseScheduledEmail.tsx
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
  • packages/features/ee/round-robin/roundRobinReassignment.ts
  • packages/features/ee/round-robin/roundRobinManualReassignment.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/emails/email-manager.ts
  • packages/emails/templates/organizer-cancelled-email.ts
**/*.tsx

📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

  • packages/emails/src/templates/OrganizerCancelledEmail.tsx
  • packages/emails/src/templates/BaseScheduledEmail.tsx
🧠 Learnings (7)
📓 Common learnings
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.
📚 Learning: the filterhostsbysameroundrobinhost function in packages/lib/bookings/filterhostsbysameroundrobinhos...
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/ee/round-robin/roundRobinManualReassignment.test.ts
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
  • packages/features/ee/round-robin/roundRobinReassignment.ts
  • packages/features/ee/round-robin/roundRobinManualReassignment.ts
  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/emails/email-manager.ts
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
  • packages/emails/email-manager.ts
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/lib/bookings/filterHostsBySameRoundRobinHost.ts
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.

Applied to files:

  • packages/emails/src/templates/BaseScheduledEmail.tsx
📚 Learning: in packages/app-store/office365calendar/lib/calendarservice.ts, the fetcher method in office365calen...
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

Applied to files:

  • apps/web/test/utils/bookingScenario/expects.ts
  • packages/emails/templates/organizer-cancelled-email.ts
📚 Learning: prefer composition over prop drilling in react; use react children feature instead of passing props ...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Prefer composition over prop drilling in React; use React children feature instead of passing props through many layers

Applied to files:

  • packages/emails/email-manager.ts
🧬 Code Graph Analysis (2)
packages/features/ee/round-robin/roundRobinReassignment.ts (2)
packages/emails/email-manager.ts (1)
  • sendRoundRobinReassignedEmailsAndSMS (261-285)
packages/prisma/zod-utils.ts (1)
  • EventTypeMetadata (146-146)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)
packages/emails/email-manager.ts (1)
  • sendRoundRobinReassignedEmailsAndSMS (261-285)
packages/prisma/zod-utils.ts (1)
  • EventTypeMetadata (146-146)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (19)
packages/features/ee/round-robin/roundRobinManualReassignment.test.ts (2)

261-264: LGTM: Updated to use the new reassignment email function

The change from sendRoundRobinCancelledEmailsAndSMS to sendRoundRobinReassignedEmailsAndSMS aligns perfectly with the PR objective of improving email clarity for Round Robin reassignments. This makes the test more semantically accurate by explicitly testing reassignment email functionality rather than cancellation emails.


325-325: LGTM: Consistent expectation for the new email function

The expectation correctly verifies that the new sendRoundRobinReassignedEmailsAndSMS function is called once during manual reassignment, which is the expected behavior.

packages/emails/src/templates/BaseScheduledEmail.tsx (1)

110-112: LGTM: Enhanced reassignment reason display with consistent pattern

The addition of conditional rendering for reassignment reason when byUser is falsy creates symmetry with the existing logic for when byUser is truthy. This ensures users receive reassignment context in both scenarios, improving email clarity as intended by the PR objectives.

The implementation follows established patterns:

  • Proper optional chaining and conditional rendering
  • Consistent use of the Info component
  • Appropriate localization with t("reason")
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (1)

2161-2161: LGTM! Correctly tests booker-initiated reschedule functionality.

The addition of bookerReschedule: true properly aligns with the test scenario where the booker is rescheduling the round robin booking (as indicated by rescheduledBy: booker.email on line 2103). This change ensures the test validates the new reassignment email functionality introduced in this PR.

packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)

7-7: LGTM! Import updated to reflect reassignment semantics.

The change from sendRoundRobinCancelledEmailsAndSMS to sendRoundRobinReassignedEmailsAndSMS better reflects the actual operation being performed.


352-365: LGTM! Function call updated with improved parameter structure.

The change to named parameters and the new reassignedTo parameter structure improves readability and provides clearer context about who the booking was reassigned to. This aligns well with the PR objective to improve reassignment email communication.

packages/features/bookings/lib/handleNewBooking.ts (2)

81-81: Good organizational improvement.

Grouping imports from the same module improves code readability.


1766-1780: LGTM! Reassignment detection and context passing implemented correctly.

The logic properly identifies when a reassignment has occurred by finding non-fixed users in the new booked members and passes the appropriate context to the cancellation email function. The conditional "Booker Rescheduled" reason provides valuable clarity about who initiated the change.

packages/features/ee/round-robin/roundRobinReassignment.ts (2)

7-7: LGTM! Improved function naming aligns with PR objectives.

The import change from sendRoundRobinCancelledEmailsAndSMS to sendRoundRobinReassignedEmailsAndSMS better reflects the actual operation and supports the PR's goal of clarifying that a booking was reassigned rather than cancelled.


415-428: Function call updated to use clearer named parameters.

The refactor from positional arguments to named object properties improves code readability and maintainability. The new reassignedTo parameter clearly communicates who the booking was reassigned to, supporting better email content.

packages/lib/bookings/filterHostsBySameRoundRobinHost.ts (3)

9-9: Appropriate expansion of generic constraint.

Adding the email property requirement to the generic constraint ensures the function has access to both identification methods (ID and email) needed for the enhanced filtering logic below.


37-41: Enhanced query to support email-based host identification.

The Prisma query enhancement to include attendee emails enables more sophisticated host matching, which is essential for the improved filtering logic that follows.


46-54: Improved filtering logic with email-based host identification.

The two-step filtering approach is more robust:

  1. First identifies the current RR host by matching attendee emails with non-fixed hosts
  2. Then filters hosts to include those matching either the found RR host ID or the original booking user ID

This addresses scenarios where hosts might be identified through different means while maintaining backward compatibility.

packages/emails/src/templates/OrganizerCancelledEmail.tsx (1)

3-19: Well-structured conditional email content for reassigned vs cancelled events.

The component effectively handles both scenarios:

  • Uses appropriate translation keys (event_request_reassigned vs event_request_cancelled)
  • Includes subtitle only for reassigned events to provide additional context
  • Maintains robust translation fallback from teamMember to organizer language

This directly supports the PR's objective of clarifying whether a booking was reassigned or cancelled.

packages/emails/templates/organizer-cancelled-email.ts (2)

12-12: Conditional subject line improves email clarity.

The subject now accurately reflects the actual operation - using "event_reassigned_subject" when reassigned vs "event_cancelled_subject" for true cancellations. This directly addresses the PR's objective of clarifying communication.


26-26: Enhanced template rendering with reassignment context.

The updated getHtml method signature and call properly passes the reassignment context to the email rendering logic, enabling the template to display appropriate content and messaging based on the operation type.

Also applies to: 31-37

apps/web/test/utils/bookingScenario/expects.ts (2)

714-719: Appropriate test enhancement for booker reschedule scenarios.

The optional bookerReschedule parameter allows tests to specify when reassignment emails should be expected, providing more precise test coverage for different reschedule scenarios.


744-755: Conditional reassignment email expectation aligns with new logic.

The test correctly expects reassignment emails only when:

  1. bookerReschedule is true (reschedule initiated by booker)
  2. Organizer actually changes (different from previous organizer)

This properly validates the improved email notification system that distinguishes between cancellations and reassignments.

packages/emails/email-manager.ts (1)

240-240: LGTM - Parameter type enhancement.

The addition of the optional reason field to the reassignedTo parameter correctly supports the enhanced email messaging for clearer cancellation context.

Comment on lines +247 to +259
emailsAndSMSToSend.push(
sendEmail(
() => new OrganizerCancelledEmail({ calEvent: calendarEvent, teamMember, reassigned: reassignedTo })
)
);

if (teamMember.phoneNumber) {
emailsAndSMSToSend.push(successfullyReScheduledSMS.sendSMSToAttendee(teamMember));
}
}

await Promise.all(emailsAndSMSToSend);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Approve email logic, but verify SMS class usage.

The simplified email logic correctly aligns with PR objectives - always sending cancellation emails with optional reassignment context. However, there's an inconsistency in SMS handling:

  • Variable name suggests successfullyReScheduledSMS but the class instantiated is EventCancelledSMS
  • This appears intentional based on the context, but please verify this is the correct SMS type for cancelled/reassigned bookings.

🏁 Script executed:

#!/bin/bash
# Verify SMS class usage consistency across round robin functions
rg -A 3 -B 1 "EventCancelledSMS|EventSuccessfullyReScheduledSMS" packages/emails/email-manager.ts

Length of output: 2178


Clarify SMS class and variable naming in cancellation flow

The use of EventCancelledSMS in the round-robin cancellation function is correct, but the variable name (successfullyReScheduledSMS) implies a “reschedule” SMS and will confuse future readers. Please rename the variable to match the class and its intent:

• File: packages/emails/email-manager.ts
Location: inside sendRoundRobinCancelledEmailsAndSMS (around lines 247–259)

Replace:

- const successfullyReScheduledSMS = new EventCancelledSMS(calEvent);
+ const eventCancelledSms = new EventCancelledSMS(calEvent);- emailsAndSMSToSend.push(
-   successfullyReScheduledSMS.sendSMSToAttendee(teamMember)
- );
+ emailsAndSMSToSend.push(
+   eventCancelledSms.sendSMSToAttendee(teamMember)
+ );

This ensures the variable name accurately reflects that it’s sending a cancellation SMS.

🤖 Prompt for AI Agents
In packages/emails/email-manager.ts around lines 247 to 259, the variable named
successfullyReScheduledSMS is misleading because it suggests sending a
reschedule SMS, but it actually sends a cancellation SMS using the
EventCancelledSMS class. Rename the variable successfullyReScheduledSMS to a
name that clearly indicates it is for cancellation SMS, such as
eventCancelledSMS, to accurately reflect its purpose and avoid confusion.

@volnei
Copy link
Contributor

volnei commented Aug 14, 2025

@kart1ka can you please fix the conflicts? 🙏🏼

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)

341-347: “byUser” likely misattributes the initiator of the reassignment

For the scheduled email to the new host, byUser is set to the original organizer’s name. In a manual reassignment, the actor is reassignedById (admin/host), not the original organizer. This can produce misleading messaging.

Consider using the actual initiating user’s name:

       reassigned: {
         name: newUser.name,
         email: newUser.email,
         reason: reassignReason,
-        byUser: originalOrganizer.name || undefined,
+        byUser: reassignedByName,
       },

And define reassignedByName earlier (outside this hunk), for example:

// Near user/eventType fetches
const reassignedByUser = await prisma.user.findUnique({
  where: { id: reassignedById },
  select: { name: true },
});
const reassignedByName = reassignedByUser?.name ?? undefined;

If you prefer not to add another query now, a safer interim is to omit byUser here (to avoid misattribution) until we plumb the actor identity through this flow.


372-372: Avoid unchecked cast for eventTypeMetadata; validate with Zod before passing

as EventTypeMetadata can hide shape mismatches. Since downstream logic uses this to gate host emails, prefer validating the metadata first.

For example:

import { EventTypeMetaDataSchema } from "@calcom/prisma/zod-utils";

// ...
const parsedMeta = eventType?.metadata
  ? EventTypeMetaDataSchema.safeParse(eventType.metadata)
  : { success: false };

await sendRoundRobinReassignedEmailsAndSMS({
  // ...
  eventTypeMetadata: parsedMeta.success ? parsedMeta.data : undefined,
});

This prevents accidental enable/disable of notifications due to malformed metadata.

📜 Review details

Configuration used: CodeRabbit UI
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.

📥 Commits

Reviewing files that changed from the base of the PR and between a9f2601 and 8a4a886.

📒 Files selected for processing (4)
  • packages/features/bookings/lib/handleNewBooking.ts (2 hunks)
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (2 hunks)
  • packages/features/ee/round-robin/roundRobinManualReassignment.ts (3 hunks)
  • packages/features/ee/round-robin/roundRobinReassignment.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/features/bookings/lib/handleNewBooking.ts
  • packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
  • packages/features/ee/round-robin/roundRobinReassignment.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts

📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/ee/round-robin/roundRobinManualReassignment.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/ee/round-robin/roundRobinManualReassignment.ts
🧬 Code Graph Analysis (1)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)
packages/emails/email-manager.ts (1)
  • sendRoundRobinReassignedEmailsAndSMS (261-285)
packages/prisma/zod-utils.ts (1)
  • EventTypeMetadata (146-146)
⏰ Context from checks skipped due to timeout of 90000ms. 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 (2)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)

7-7: All call sites use the new object-args API

Both roundRobinReassignment.ts (line 423) and roundRobinManualReassignment.ts (line 360) invoke sendRoundRobinReassignedEmailsAndSMS({ … }), confirming there are no remaining positional-args usages.


360-374: Ensure previous RR host’s phoneNumber is loaded for SMS

sendRoundRobinReassignedEmailsAndSMS will only send SMS when member.phoneNumber is truthy. Currently, previousRRHost is sourced from the eventType.hosts.user selection—which spreads the default userSelect from @calcom/prisma but does not explicitly include phoneNumber. Please verify and, if missing, update the selection (or surface it via getTeamMembers) so that previousRRHost.phoneNumber is populated:

• In packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts, under hosts.user.select, add:

   hosts: {
     select: {
       /* … */
       user: {
         select: {
+          phoneNumber: true,
           ...userSelect.select,
         },
       },
     },
   },

• Or amend getTeamMembers (in ee/round-robin/utils/getTeamMembers.ts) to fetch and attach phoneNumber for the previous host.

@kart1ka
Copy link
Contributor Author

kart1ka commented Aug 19, 2025

@kart1ka can you please fix the conflicts? 🙏🏼

Done. Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bookings area: bookings, availability, timezones, double booking community Created by Linear-GitHub Sync community-interns The team responsible for reviewing, testing and shipping low/medium community PRs consumer emails area: emails, cancellation email, reschedule email, inbox, spam folder, not getting email 🧹 Improvements Improvements to existing features. Mostly UX/UI Low priority Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync ready-for-e2e
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-4585] Improve cancellation email of rescheduled RR booking
7 participants