Add guest management functionality to existing bookings#3
Open
ShashankFC wants to merge 1 commit into
Open
Conversation
* feat: ability to add guests via app.cal.com/bookings * fix: some update * fix: minor issue * fix: final update * update * update * add requested changes * fix type error * small update * final update * fix type error * fix location * update calender event --------- Co-authored-by: Somay Chauhan <somaychauhan98@gmail.com>
Author
|
@CodeAnt-AI: review |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
the
size:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| }); | ||
|
|
||
| const handleAdd = () => { | ||
| if (multiEmailValue.length === 0) { |
There was a problem hiding this comment.
Suggestion: The invalid-email error flag is only ever set to true and never cleared on subsequent submissions (except when pressing "cancel"), so once a user triggers a validation error the red error message will continue to show even after they enter valid emails and successfully add guests or reopen the dialog; resetting the flag at the start of each submission prevents this stale error state. [logic error]
Severity Level: Major ⚠️
- ⚠️ Add-guests dialog shows stale error after fixing emails.
- ⚠️ Users see error banner on reopen with valid state.
- ⚠️ Confusing UX when adding guests to existing bookings.
Suggested change
| if (multiEmailValue.length === 0) { | |
| setIsInvalidEmail(false); |
Steps of Reproduction ✅
1. Open the booking list UI rendered by `apps/web/components/booking/BookingListItem.tsx`
(AddGuestsDialog imported at line 46 and used around lines 357-359).
2. Click the "Additional guests" action in `BookingListItem` to open `<AddGuestsDialog>`
(component defined in `apps/web/components/dialog/AddGuestsDialog.tsx:24`).
3. In the dialog, enter an invalid or duplicate email in the `MultiEmail` field and click
"Add"; `handleAdd` at `AddGuestsDialog.tsx:48-58` runs, `ZAddGuestsInputSchema.safeParse`
fails, and `setIsInvalidEmail(true)` executes, showing the red validation message at lines
76-83.
4. Correct the emails so they are all valid and unique and click "Add" again; `handleAdd`
does not clear `isInvalidEmail` before re-validating, so the error flag remains true and
the error UI at lines 76-83 stays visible until the dialog is closed, and on later
re-openings of the same dialog instance the error message appears immediately despite no
current validation error.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** apps/web/components/dialog/AddGuestsDialog.tsx
**Line:** 49:49
**Comment:**
*Logic Error: The invalid-email error flag is only ever set to true and never cleared on subsequent submissions (except when pressing "cancel"), so once a user triggers a validation error the red error message will continue to show even after they enter valid emails and successfully add guests or reopen the dialog; resetting the flag at the start of each submission prevents this stale error state.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Comment on lines
+525
to
+547
| export const sendAddGuestsEmails = async (calEvent: CalendarEvent, newGuests: string[]) => { | ||
| const calendarEvent = formatCalEvent(calEvent); | ||
|
|
||
| const emailsToSend: Promise<unknown>[] = []; | ||
| emailsToSend.push(sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent }))); | ||
|
|
||
| if (calendarEvent.team?.members) { | ||
| for (const teamMember of calendarEvent.team.members) { | ||
| emailsToSend.push( | ||
| sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent, teamMember })) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| emailsToSend.push( | ||
| ...calendarEvent.attendees.map((attendee) => { | ||
| if (newGuests.includes(attendee.email)) { | ||
| return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee)); | ||
| } else { | ||
| return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee)); | ||
| } | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Suggestion: Guest-added email sending ignores the existing event-type settings for disabling standard host/attendee emails, so organizers and attendees will still receive these notifications even when standard emails are configured as disabled; the function should accept optional event metadata and apply the same eventTypeDisableHostEmail/eventTypeDisableAttendeeEmail guards used by other booking-notification senders. [logic error]
Severity Level: Major ⚠️
- ❌ Hosts emailed about guests despite 'disable host emails' setting.
- ❌ Attendees emailed despite 'disable attendee emails' configured.
- ⚠️ Breaks admin expectation of global email suppression.
- ⚠️ Inconsistent behavior versus booking confirmation email settings.
Suggested change
| export const sendAddGuestsEmails = async (calEvent: CalendarEvent, newGuests: string[]) => { | |
| const calendarEvent = formatCalEvent(calEvent); | |
| const emailsToSend: Promise<unknown>[] = []; | |
| emailsToSend.push(sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent }))); | |
| if (calendarEvent.team?.members) { | |
| for (const teamMember of calendarEvent.team.members) { | |
| emailsToSend.push( | |
| sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent, teamMember })) | |
| ); | |
| } | |
| } | |
| emailsToSend.push( | |
| ...calendarEvent.attendees.map((attendee) => { | |
| if (newGuests.includes(attendee.email)) { | |
| return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee)); | |
| } else { | |
| return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee)); | |
| } | |
| }) | |
| ); | |
| export const sendAddGuestsEmails = async ( | |
| calEvent: CalendarEvent, | |
| newGuests: string[], | |
| eventTypeMetadata?: EventTypeMetadata | |
| ) => { | |
| const calendarEvent = formatCalEvent(calEvent); | |
| const emailsToSend: Promise<unknown>[] = []; | |
| if (!eventTypeDisableHostEmail(eventTypeMetadata)) { | |
| emailsToSend.push(sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent }))); | |
| if (calendarEvent.team?.members) { | |
| for (const teamMember of calendarEvent.team.members) { | |
| emailsToSend.push( | |
| sendEmail(() => new OrganizerAddGuestsEmail({ calEvent: calendarEvent, teamMember })) | |
| ); | |
| } | |
| } | |
| } | |
| if (!eventTypeDisableAttendeeEmail(eventTypeMetadata)) { | |
| emailsToSend.push( | |
| ...calendarEvent.attendees.map((attendee) => { | |
| if (newGuests.includes(attendee.email)) { | |
| return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee)); | |
| } else { | |
| return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee)); | |
| } | |
| }) | |
| ); | |
| } |
Steps of Reproduction ✅
1. In the web UI, configure an event type to disable all standard emails for hosts and
attendees via the toggles bound to `metadata.disableStandardEmails.all.attendee` and
`metadata.disableStandardEmails.all.host` (see
`apps/web/components/eventtype/EventAdvancedTab.tsx:687-710`, where these fields are
wired).
2. Create and confirm a booking for this event type. During booking creation
(`packages/features/bookings/lib/handleNewBooking.ts:1471-1506`), the code reads
`eventType.metadata.disableStandardEmails` and passes `eventType.metadata` plus derived
`isHostConfirmationEmailsDisabled`/`isAttendeeConfirmationEmailDisabled` into
`sendScheduledEmails`, which in turn checks
`eventTypeDisableHostEmail`/`eventTypeDisableAttendeeEmail`
(`packages/emails/email-manager.ts:81-87, 89-132`). As a result, the standard scheduled
emails for this event are correctly suppressed in line with the settings.
3. After the booking exists, use the "add guests" functionality on that booking, which
invokes the TRPC handler `addGuestsHandler` at
`packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:16-22`. This handler
loads the booking including its `eventType` and metadata (`addGuests.handler.ts:26-41`),
builds a `CalendarEvent` object `evt` (`addGuests.handler.ts:124-147`), and updates the
calendar attendees.
4. Still inside `addGuestsHandler`, observe that it calls `await sendAddGuestsEmails(evt,
guests);` without passing any event-type metadata (`addGuests.handler.ts:167-168`), even
though the booking's `eventType.metadata.disableStandardEmails.all.*` flags are set.
5. In `packages/emails/email-manager.ts:525-550`, the implementation of
`sendAddGuestsEmails` formats the calendar event and unconditionally pushes organizer and
team-member emails using `OrganizerAddGuestsEmail`, and attendee emails using
`AttendeeScheduledEmail` or `AttendeeAddGuestsEmail`, with no checks against
`eventTypeDisableHostEmail` or `eventTypeDisableAttendeeEmail` and no metadata parameter
at all.
6. Because of step 5, when step 3's add-guests action runs on an event type with
`metadata.disableStandardEmails.all.host`/`attendee` set, organizers, team members, and
attendees still receive guest-added emails, contradicting the global "disable standard
emails" configuration that other email flows (e.g., scheduled, rescheduled, cancelled,
location change) respect.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/emails/email-manager.ts
**Line:** 525:547
**Comment:**
*Logic Error: Guest-added email sending ignores the existing event-type settings for disabling standard host/attendee emails, so organizers and attendees will still receive these notifications even when standard emails are configured as disabled; the function should accept optional event metadata and apply the same `eventTypeDisableHostEmail`/`eventTypeDisableAttendeeEmail` guards used by other booking-notification senders.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Comment on lines
+5
to
+9
| title="new_guests_added" | ||
| headerType="calendarCircle" | ||
| subject="guests_added_event_type_subject" | ||
| callToAction={null} | ||
| {...props} |
There was a problem hiding this comment.
Suggestion: In the organizer guest-added email wrapper, callToAction is set to null but then immediately overridden by the spread props, so any provided callToAction will still appear in the email; spreading props first and then setting callToAction={null} (and the fixed title/header/subject) ensures these values cannot be accidentally overridden. [logic error]
Severity Level: Major ⚠️
- ⚠️ Organizer "guests added" emails still show manage-link CTA.
- ⚠️ Template cannot reliably disable call-to-action region.
Suggested change
| title="new_guests_added" | |
| headerType="calendarCircle" | |
| subject="guests_added_event_type_subject" | |
| callToAction={null} | |
| {...props} | |
| {...props} | |
| title="new_guests_added" | |
| headerType="calendarCircle" | |
| subject="guests_added_event_type_subject" | |
| callToAction={null} |
Steps of Reproduction ✅
1. In the web app, use the "add guests" flow for an existing booking, which hits the TRPC
handler at `packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:168` where
`await sendAddGuestsEmails(evt, guests);` is called.
2. Inside `sendAddGuestsEmails` in `packages/emails/email-manager.ts:525-549`, the code
formats the event and enqueues organizer emails via `new OrganizerAddGuestsEmail({
calEvent: calendarEvent })` and `new OrganizerAddGuestsEmail({ calEvent: calendarEvent,
teamMember })`.
3. The class `OrganizerAddGuestsEmail` in
`packages/emails/templates/organizer-add-guests-email.ts:7-36` builds the nodemailer
payload and renders HTML with `html: await renderEmail("OrganizerAddGuestsEmail", {
attendee: this.calEvent.organizer, calEvent: this.calEvent })`, invoking the React
template `packages/emails/src/templates/OrganizerAddGuestsEmail.tsx:3-11`.
4. In that React template, `<OrganizerScheduledEmail title="new_guests_added"
headerType="calendarCircle" subject="guests_added_event_type_subject" callToAction={null}
{...props} />` passes `callToAction={null}` and then immediately spreads `props`, where
`props.callToAction` is `undefined` from the `renderEmail` call; this override makes the
final `callToAction` prop `undefined`, so `BaseScheduledEmail` at
`packages/emails/src/templates/BaseScheduledEmail.tsx:63-67` treats it as not-null and
falls back to the default `<ManageLink ...>` call-to-action, causing a manage-link CTA to
appear even though this template explicitly attempts to disable it.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/emails/src/templates/OrganizerAddGuestsEmail.tsx
**Line:** 5:9
**Comment:**
*Logic Error: In the organizer guest-added email wrapper, `callToAction` is set to `null` but then immediately overridden by the spread `props`, so any provided `callToAction` will still appear in the email; spreading `props` first and then setting `callToAction={null}` (and the fixed title/header/subject) ensures these values cannot be accidentally overridden.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| if (!booking) throw new TRPCError({ code: "NOT_FOUND", message: "booking_not_found" }); | ||
|
|
||
| const isTeamAdminOrOwner = | ||
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && |
There was a problem hiding this comment.
Suggestion: The permission check currently requires the user to be both a team admin and a team owner, which will block team admins who are not owners from adding guests, contrary to the intended "admin or owner" access. [logic error]
Severity Level: Critical 🚨
- ❌ Team admins blocked from adding guests to team bookings.
- ⚠️ Support burden from admins denied expected guest-management permissions.
Suggested change
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && | |
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) || |
Steps of Reproduction ✅
1. Call the TRPC mutation `viewer.bookings.addGuests` wired in
`packages/trpc/server/routers/viewer/bookings/_router.tsx:79-95`, which forwards to
`addGuestsHandler` in `addGuests.handler.ts:22`.
2. Ensure the booking being modified belongs to a team (has `booking.eventType?.teamId`
set) and that the authenticated user is a team admin but not a team owner for that team
(checked via `isTeamAdmin` and `isTeamOwner` from
`packages/lib/server/queries/teams/index.ts:264,286`).
3. Inside `addGuestsHandler` (`addGuests.handler.ts:46-52`), `isTeamAdminOrOwner` is
computed using a logical AND: `(await isTeamAdmin(...)) && (await isTeamOwner(...))`, so a
user who is only admin or only owner yields `false`.
4. If the user is neither the organizer (`isOrganizer` at `addGuests.handler.ts:50`) nor
an attendee (`isAttendee` at `addGuests.handler.ts:52`), the permission guard at
`addGuests.handler.ts:54-55` evaluates `!isTeamAdminOrOwner && !isOrganizer &&
!isAttendee` to `true` and throws `TRPCError({ code: "FORBIDDEN" })`, blocking a
legitimate team admin from adding guests.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**Line:** 47:47
**Comment:**
*Logic Error: The permission check currently requires the user to be both a team admin and a team owner, which will block team admins who are not owners from adding guests, contrary to the intended "admin or owner" access.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| const uniqueGuests = guests.filter( | ||
| (guest) => | ||
| !booking.attendees.some((attendee) => guest === attendee.email) && | ||
| !blacklistedGuestEmails.includes(guest) |
There was a problem hiding this comment.
Suggestion: Blacklisted guest emails are stored lowercased but compared against the raw input string, so a guest can bypass the blacklist by changing the email's casing. [logic error]
Severity Level: Major ⚠️
- ❌ Blacklisted email addresses can be added as guests.
- ⚠️ Violates organizer expectations about blocked domains or addresses.
Suggested change
| !blacklistedGuestEmails.includes(guest) | |
| !blacklistedGuestEmails.includes(guest.toLowerCase()) |
Steps of Reproduction ✅
1. Configure a blacklist entry using `BLACKLISTED_GUEST_EMAILS` (documented in
`.env.example:371` and referenced in multiple booking and verification handlers via Grep,
e.g. `packages/features/bookings/lib/handleNewBooking/checkIfBookerEmailIsBlocked.ts:13`).
2. Trigger the TRPC mutation `viewer.bookings.addGuests` via the router in
`packages/trpc/server/routers/viewer/bookings/_router.tsx:79-95`, providing `guests` that
include an email whose lowercase form matches a blacklisted entry, but with different
casing (e.g. env has `blocked@example.com`, input uses `Blocked@example.com`).
3. In `addGuests.handler.ts:70-72`, `blacklistedGuestEmails` is built by lowercasing the
env entries, producing `["blocked@example.com"]` in this example.
4. In the `uniqueGuests` filter at `addGuests.handler.ts:74-78`, the condition
`!blacklistedGuestEmails.includes(guest)` compares the raw `guest` string
(`"Blocked@example.com"`) against the lowercased list, so `includes` returns `false` and
the guest passes the blacklist check.
5. The handler continues, creating attendee records and updating the calendar for this
guest, despite their email effectively matching a configured blacklist entry, showing that
differing casing bypasses the blacklist.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**Line:** 77:77
**Comment:**
*Logic Error: Blacklisted guest emails are stored lowercased but compared against the raw input string, so a guest can bypass the blacklist by changing the email's casing.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| await eventManager.updateCalendarAttendees(evt, booking); | ||
|
|
||
| try { | ||
| await sendAddGuestsEmails(evt, guests); |
There was a problem hiding this comment.
Suggestion: The email notification function is called with the original guest list instead of the filtered unique/allowed guests, causing emails to be sent for guests that were not actually added (e.g., duplicates or blacklisted emails). [logic error]
Severity Level: Major ⚠️
- ❌ Blocked or duplicate guests receive incorrect invitation emails.
- ⚠️ Mismatch between calendar attendees and notification recipients.
Suggested change
| await sendAddGuestsEmails(evt, guests); | |
| await sendAddGuestsEmails(evt, uniqueGuests); |
Steps of Reproduction ✅
1. Note that `sendAddGuestsEmails` is defined as `sendAddGuestsEmails(calEvent:
CalendarEvent, newGuests: string[])` in `packages/emails/email-manager.ts:525`, indicating
the second parameter is meant to represent the newly added guests.
2. Call the TRPC mutation `viewer.bookings.addGuests` via `bookingsRouter` in
`packages/trpc/server/routers/viewer/bookings/_router.tsx:79-95`, passing a `guests` array
that includes at least one email that will be filtered out: for example, a duplicate of an
existing attendee (`booking.attendees`) or an address present in
`BLACKLISTED_GUEST_EMAILS` so it is dropped in `uniqueGuests` at
`addGuests.handler.ts:74-78`.
3. The handler creates new attendee records only for `uniqueGuests`
(`addGuests.handler.ts:83-106`) and updates the calendar attendees via
`eventManager.updateCalendarAttendees(evt, booking)` at `addGuests.handler.ts:165`, so
only the filtered guests are reflected in the calendar.
4. At `addGuests.handler.ts:167-170`, `sendAddGuestsEmails` is called with the original
`guests` array rather than `uniqueGuests`, so emails are sent to all requested guests,
including those that were filtered out and not actually added as attendees.
5. As a result, filtered or blocked guests can receive "added to booking" emails while not
appearing as attendees in the calendar event, creating a concrete inconsistency between
the system state and the notifications.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**Line:** 168:168
**Comment:**
*Logic Error: The email notification function is called with the original guest list instead of the filtered unique/allowed guests, causing emails to be sent for guests that were not actually added (e.g., duplicates or blacklisted emails).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Nitpicks 🔍
|
|
CodeAnt AI finished running the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Test 10nn
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#10](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/10)*CodeAnt-AI Description
Add ability to add guests to existing bookings and notify attendees
What Changed
Impact
✅ Easier to add guests to bookings✅ Updated calendar invites for newly added guests✅ Clearer guest-added notification emails💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.