Skip to content

Add guest management functionality to existing bookings#2

Open
akshayutture-augment wants to merge 1 commit into
guest-management-basefrom
guest-management-enhanced
Open

Add guest management functionality to existing bookings#2
akshayutture-augment wants to merge 1 commit into
guest-management-basefrom
guest-management-enhanced

Conversation

@akshayutture-augment

@akshayutture-augment akshayutture-augment commented Nov 14, 2025

Copy link
Copy Markdown

Test 10

Summary by CodeRabbit

Release Notes

  • New Features
    • Users can now add additional guests to existing bookings through a new dialog interface
    • Guest email validation ensures unique and valid addresses with clear error messaging
    • Automated email notifications sent to organizers and attendees when guests are added to an event

✏️ Tip: You can customize this high-level summary in your review settings.


Replicated from ai-code-review-evaluation/cal.com-coderabbit#10

* 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>
@akshayutture-augment

Copy link
Copy Markdown
Author

@coderabbitai review

@akshayutture-augment

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 18, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Nov 18, 2025

Copy link
Copy Markdown

Walkthrough

This change introduces a new feature to add guest emails to existing bookings. It includes a UI dialog component with multi-email input validation, a TRPC backend handler with permission checks and database operations, email notification templates for organizers and attendees, calendar updates via EventManager, and supporting localization strings.

Changes

Cohort / File(s) Summary
UI - Booking Dialog
apps/web/components/booking/BookingListItem.tsx, apps/web/components/dialog/AddGuestsDialog.tsx
Added "add_members" action to BookingListItem that triggers AddGuestsDialog; new AddGuestsDialog component manages email input validation, TRPC mutation call with success/error handlers, and dialog state.
UI - Form Components
packages/ui/form/MultiEmail.tsx, packages/ui/form/MultiEmailLazy.tsx, packages/ui/index.tsx
New MultiEmail component renders dynamic list of email inputs with add/remove functionality; lazy-loaded wrapper provided; exported from UI package index.
Email System
packages/emails/email-manager.ts, packages/emails/src/templates/AttendeeAddGuestsEmail.tsx, packages/emails/src/templates/OrganizerAddGuestsEmail.tsx, packages/emails/src/templates/index.ts, packages/emails/templates/attendee-add-guests-email.ts, packages/emails/templates/organizer-add-guests-email.ts
New sendAddGuestsEmails function queues emails for organizer and attendees; new React wrapper templates (AttendeeAddGuestsEmail, OrganizerAddGuestsEmail) extend existing scheduled email components; new Node.js email classes generate NodeMailer payloads with ICS attachments.
TRPC API
packages/trpc/server/routers/viewer/bookings/_router.tsx, packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts, packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts
New addGuests mutation route with lazy-loaded handler; handler validates permissions (admin/organizer/attendee), filters guests by blacklist and uniqueness, updates booking with new attendees, refreshes calendar via EventManager, triggers email notifications; schema validates bookingId and email array.
Localization
apps/web/public/static/locales/en/common.json
Added six new translation keys: new_guests_added, guests_added_event_type_subject, guests_added, unable_to_add_guests, emails_must_be_unique_valid, add_emails.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant UI as AddGuestsDialog
    participant Schema as Validation
    participant TRPC as addGuests Mutation
    participant Handler as addGuestsHandler
    participant Calendar as EventManager
    participant Email as sendAddGuestsEmails

    User->>UI: Enter emails & click Add
    UI->>Schema: Validate emails (unique & valid)
    alt Validation Failed
        Schema-->>UI: Error: Invalid emails
        UI-->>User: Show error toast
    else Validation Success
        Schema-->>UI: Valid ✓
        UI->>TRPC: Call mutation(bookingId, guests)
        TRPC->>Handler: Process with ctx & input
        Handler->>Handler: Check permissions<br/>(admin/organizer/attendee)
        alt Permission Denied
            Handler-->>TRPC: FORBIDDEN error
            TRPC-->>UI: Error
            UI-->>User: Show error toast
        else Permission Granted
            Handler->>Handler: Filter guests<br/>(unique, not blacklisted)
            Handler->>Handler: Create attendee records
            Handler->>Handler: Update booking in DB
            Handler->>Calendar: Update calendar<br/>with new attendees
            Handler->>Email: Send notifications
            Email-->>Handler: Emails queued
            Handler-->>TRPC: Success message
            TRPC-->>UI: Success
            UI->>UI: Close dialog<br/>Reset state<br/>Invalidate cache
            UI-->>User: Show success toast
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • addGuests.handler.ts: Requires careful review of permission logic (admin/organizer/attendee checks), guest filtering logic (blacklist and uniqueness), and database transaction integrity when creating multiple attendees.
  • email-manager.ts / email template classes: Verify correct email routing (organizer vs. new attendees) and ICS attachment generation.
  • AddGuestsDialog.tsx: Validate TRPC integration, error handling, and state management for dialog lifecycle and cache invalidation.

Poem

New whiskers twitch—guests gather near! 🐰
Emails hop and calendars align with cheer,
Dialogs dance, permissions held dear,
Our feature bounds forth without fear! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add guest management functionality to existing bookings' directly and specifically summarizes the main purpose of the changeset—implementing a feature to add guests to existing bookings.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch guest-management-enhanced

Comment @coderabbitai help to get the list of available commands and usage tips.

@akshayutture-augment

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 21, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (5)
packages/ui/form/MultiEmail.tsx (2)

24-56: Consider using a stable identifier instead of array index as React key.

Line 26 uses index as the key for list items. When emails are added or removed, this can cause React to incorrectly reuse components, leading to bugs with component state or focus management.

Consider generating a unique ID for each email entry:

-{value.map((field, index) => (
-  <li key={index}>
+{value.map((field, index) => (
+  <li key={`email-${index}-${field}`}>

Or better yet, maintain an array of objects with stable IDs in the parent component.


73-91: Simplify initialization logic.

Lines 73-91 render a button when there are no values, which is very similar to the "Add another email" button logic above. Consider extracting to reduce duplication.

{!readOnly && value.length === 0 && (
  <Button
    data-testid="add-emails"
    color="minimal"
    variant="button"
    StartIcon="user-plus"
    onClick={() => setValue([""])}
    className="mr-auto">
    {label}
  </Button>
)}
apps/web/components/booking/BookingListItem.tsx (1)

193-201: Consider adding conditions for when "Add Guests" action should be available.

The "add_members" action is currently added to all bookings unconditionally. You may want to consider:

  • Disabling for cancelled or past bookings
  • Checking if the event type allows additional guests (e.g., not disabled via disableGuests)
  • Checking user permissions

Consider adding guards:

+   ...(!booking.eventType.disableGuests ? [{
      id: "add_members",
      label: t("additional_guests"),
      onClick: () => {
        setIsOpenAddGuestsDialog(true);
      },
      icon: "user-plus" as const,
-   },
+   }] : []),
packages/emails/email-manager.ts (1)

525-550: Consider adding email preference checks for consistency.

The sendAddGuestsEmails function doesn't include an eventTypeMetadata parameter or check eventTypeDisableHostEmail/eventTypeDisableAttendeeEmail like other email-sending functions in this file (e.g., sendScheduledEmails, sendRescheduledEmails). This means organizers and attendees will receive add-guest emails even if they've disabled standard emails.

Consider adding the eventTypeMetadata parameter and conditionally sending emails:

-export const sendAddGuestsEmails = async (calEvent: CalendarEvent, newGuests: string[]) => {
+export const sendAddGuestsEmails = async (
+  calEvent: CalendarEvent,
+  newGuests: string[],
+  eventTypeMetadata?: EventTypeMetadata
+) => {
   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 }))
-      );
+
+  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 }))
+        );
+      }
     }
   }

-  emailsToSend.push(
-    ...calendarEvent.attendees.map((attendee) => {
-      if (newGuests.includes(attendee.email)) {
-        return sendEmail(() => new AttendeeScheduledEmail(calendarEvent, attendee));
-      } else {
-        return sendEmail(() => new AttendeeAddGuestsEmail(calendarEvent, attendee));
-      }
-    })
-  );
+  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));
+        }
+      })
+    );
+  }

   await Promise.all(emailsToSend);
 };
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1)

92-106: Consider using a database transaction for atomicity.

The handler performs multiple database operations (reading booking, updating with new attendees), but calendar and email operations can fail after the database is modified. If calendar/email operations fail, attendees remain in the database even though notifications weren't sent. Consider wrapping the critical operations in a transaction or implementing compensating actions.

This is a broader architectural consideration. For now, the error handling improvements suggested above will help, but for production robustness, consider:

  • Using a database transaction to roll back attendee additions if subsequent operations fail
  • Or implementing an event-driven approach where email/calendar updates are retried asynchronously
  • Or at minimum, adding compensating rollback logic if email delivery fails critically
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b004587 and baa9045.

📒 Files selected for processing (15)
  • apps/web/components/booking/BookingListItem.tsx (4 hunks)
  • apps/web/components/dialog/AddGuestsDialog.tsx (1 hunks)
  • apps/web/public/static/locales/en/common.json (3 hunks)
  • packages/emails/email-manager.ts (3 hunks)
  • packages/emails/src/templates/AttendeeAddGuestsEmail.tsx (1 hunks)
  • packages/emails/src/templates/OrganizerAddGuestsEmail.tsx (1 hunks)
  • packages/emails/src/templates/index.ts (1 hunks)
  • packages/emails/templates/attendee-add-guests-email.ts (1 hunks)
  • packages/emails/templates/organizer-add-guests-email.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/_router.tsx (3 hunks)
  • packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1 hunks)
  • packages/ui/form/MultiEmail.tsx (1 hunks)
  • packages/ui/form/MultiEmailLazy.tsx (1 hunks)
  • packages/ui/index.tsx (1 hunks)
🔇 Additional comments (8)
apps/web/public/static/locales/en/common.json (1)

1122-1140: LGTM! Translation keys are well-structured.

The new translation keys for guest management follow the existing naming conventions and include appropriate template variables. The keys cover all necessary UI and email scenarios for the add guests feature.

packages/ui/index.tsx (1)

150-154: LGTM! Export follows established lazy-loading pattern.

The MultiEmail export is consistent with other lazy-loaded form components in this file (AddressInput, PhoneInput), maintaining a uniform approach to code-splitting.

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

1-10: LGTM! Clean wrapper component implementation.

This component follows the established pattern for email template wrappers, correctly forwarding props while setting specific defaults for the add guests scenario.

packages/emails/src/templates/index.ts (1)

38-39: LGTM! New email template exports correctly added.

The new email template exports are properly positioned and make the AttendeeAddGuestsEmail and OrganizerAddGuestsEmail templates available for use by the email manager.

packages/ui/form/MultiEmailLazy.tsx (1)

1-6: LGTM!

The lazy loading wrapper is properly implemented with correct type preservation. The 40kb optimization justification is clear and appropriate for components that aren't universally needed.

packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1)

3-6: Schema validation is correct.

The Zod schema properly validates the bookingId and guest email array. Email uniqueness is validated separately in the UI (AddGuestsDialog.tsx) and backend handler (addGuests.handler.ts), which is an acceptable layered validation approach.

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

1-11: LGTM!

The wrapper component correctly reuses OrganizerScheduledEmail with appropriate preset values for the add-guests notification scenario. Setting callToAction={null} is sensible for a notification-only email.

packages/trpc/server/routers/viewer/bookings/_router.tsx (1)

4-4: LGTM!

The addGuests route integration follows the established pattern in this router, with proper lazy loading, type safety checks, and input validation. The handler cache pattern is consistent with other routes.

Also applies to: 18-18, 79-95

});
const { isOpenDialog, setIsOpenDialog, bookingId } = props;
const utils = trpc.useUtils();
const [multiEmailValue, setMultiEmailValue] = useState<string[]>([""]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Initial state contains empty string, creating potential validation issues.

The multiEmailValue state is initialized to [""] (array with one empty string), but the validation logic at line 49 checks if (multiEmailValue.length === 0). This condition will never be true with the current initialization, potentially allowing submission of an array containing only empty strings.

Consider initializing to an empty array instead:

-  const [multiEmailValue, setMultiEmailValue] = useState<string[]>([""]);
+  const [multiEmailValue, setMultiEmailValue] = useState<string[]>([]);

And update the reset logic at line 39 and lines 90:

-      setMultiEmailValue([""]);
+      setMultiEmailValue([]);

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/web/components/dialog/AddGuestsDialog.tsx around line 32 (and also
update related reset logic at lines 39 and 90), the multiEmailValue state is
initialized to [""] which prevents the validation check if
(multiEmailValue.length === 0) from ever being true; change the initial state to
an empty array [] and update any reset calls (at lines 39 and 90) to set
multiEmailValue back to [] instead of [""] so the empty-state validation behaves
correctly and submissions with only empty strings are prevented.

Comment on lines +48 to +58
const handleAdd = () => {
if (multiEmailValue.length === 0) {
return;
}
const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
if (validationResult.success) {
addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
} else {
setIsInvalidEmail(true);
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Filter out empty strings before submitting to backend.

The handleAdd function doesn't filter out empty strings from multiEmailValue before passing it to the mutation. If users leave empty email fields, these will be sent to the backend, potentially causing validation errors or unexpected behavior.

Apply this diff to filter empty strings:

   const handleAdd = () => {
-    if (multiEmailValue.length === 0) {
-      return;
-    }
-    const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+    const filteredEmails = multiEmailValue.filter((email) => email.trim() !== "");
+    if (filteredEmails.length === 0) {
+      return;
+    }
+    const validationResult = ZAddGuestsInputSchema.safeParse(filteredEmails);
     if (validationResult.success) {
-      addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
+      addGuestsMutation.mutate({ bookingId, guests: filteredEmails });
     } else {
       setIsInvalidEmail(true);
     }
   };
🤖 Prompt for AI Agents
In apps/web/components/dialog/AddGuestsDialog.tsx around lines 48 to 58, the
handleAdd function currently sends multiEmailValue directly which can include
empty strings; filter out empty or whitespace-only strings first (e.g., const
filtered = multiEmailValue.filter(s => s.trim() !== '')), check filtered.length
=== 0 and return early if empty, run ZAddGuestsInputSchema.safeParse(filtered)
and on success call addGuestsMutation.mutate({ bookingId, guests: filtered }) so
only non-empty emails are validated and submitted.

Comment on lines +22 to +30
subject: `${this.t("guests_added_event_type_subject", {
eventType: this.calEvent.type,
name: this.calEvent.team?.name || this.calEvent.organizer.name,
date: this.getFormattedDate(),
})}`,
html: await renderEmail("AttendeeAddGuestsEmail", {
calEvent: this.calEvent,
attendee: this.attendee,
}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Verify attendees array is non-empty before accessing first element.

Line 28 accesses this.calEvent.attendees[0].name without checking if the attendees array has any elements. If the array is empty, this will cause a runtime error.

Consider adding a guard or using optional chaining:

      subject: `${this.t("guests_added_event_type_subject", {
        eventType: this.calEvent.type,
-       name: this.calEvent.attendees[0].name,
+       name: this.calEvent.attendees[0]?.name || this.calEvent.organizer.name,
        date: this.getFormattedDate(),
      })}`,

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/emails/templates/attendee-add-guests-email.ts around lines 22 to 30,
the subject build reads properties from this.calEvent.attendees[0].name without
ensuring attendees exists or has elements; add a guard so you only access
attendees[0].name when Array.isArray(this.calEvent.attendees) &&
this.calEvent.attendees.length > 0, otherwise fall back to
this.calEvent.team?.name or this.calEvent.organizer.name (or a default string).
Use optional chaining and a concise conditional expression when constructing the
subject so no runtime error occurs if attendees is empty.

Comment on lines +26 to +30
subject: `${this.t("guests_added_event_type_subject", {
eventType: this.calEvent.type,
name: this.calEvent.attendees[0].name,
date: this.getFormattedDate(),
})}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Verify attendees array is non-empty before accessing first element.

Line 28 accesses this.calEvent.attendees[0].name without verifying the attendees array has any elements. This could cause a runtime error if the array is empty.

Apply this defensive fix:

      subject: `${this.t("guests_added_event_type_subject", {
        eventType: this.calEvent.type,
-       name: this.calEvent.attendees[0].name,
+       name: this.calEvent.attendees[0]?.name || "Guest",
        date: this.getFormattedDate(),
      })}`,
📝 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.

Suggested change
subject: `${this.t("guests_added_event_type_subject", {
eventType: this.calEvent.type,
name: this.calEvent.attendees[0].name,
date: this.getFormattedDate(),
})}`,
subject: `${this.t("guests_added_event_type_subject", {
eventType: this.calEvent.type,
name: this.calEvent.attendees[0]?.name || "Guest",
date: this.getFormattedDate(),
})}`,
🤖 Prompt for AI Agents
In packages/emails/templates/organizer-add-guests-email.ts around lines 26 to
30, the subject template directly accesses this.calEvent.attendees[0].name which
will throw if attendees is missing or empty; update the code to defensively
check that Array.isArray(this.calEvent.attendees) &&
this.calEvent.attendees.length > 0 before using [0].name and provide a safe
fallback (e.g. empty string or a localized fallback key) when no attendee
exists; ensure you pass that safe value into this.t(...) so the subject
construction never reads properties from an undefined element.

Comment on lines +46 to +48
const isTeamAdminOrOwner =
(await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) &&
(await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect permission logic using AND instead of OR.

The permission check on lines 46-48 uses && (AND) between isTeamAdmin and isTeamOwner, which requires a user to be BOTH a team admin AND a team owner. This is overly restrictive and likely incorrect—a user should have permission if they are an admin OR an owner.

Apply this fix:

   const isTeamAdminOrOwner =
-    (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) &&
-    (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
+    (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) ||
+    (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
📝 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.

Suggested change
const isTeamAdminOrOwner =
(await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) &&
(await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
const isTeamAdminOrOwner =
(await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) ||
(await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 46 to 48, the permission check incorrectly requires the user to be both
team admin AND team owner; change the logic to allow either role by replacing
the logical AND with OR so the expression returns true if the user is an admin
or an owner (e.g., evaluate isTeamAdmin(...) || isTeamOwner(...), ensuring
short-circuiting behavior by awaiting each call as shown).

Comment on lines +58 to +68
const organizer = await prisma.user.findFirstOrThrow({
where: {
id: booking.userId || 0,
},
select: {
name: true,
email: true,
timeZone: true,
locale: true,
},
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential null reference error.

Line 60 uses booking.userId || 0 as a fallback, but if booking.userId is actually null, this will query for a user with id: 0, which likely doesn't exist. The findFirstOrThrow will then throw an unexpected error instead of a meaningful one.

Consider adding an explicit null check:

+  if (!booking.userId) {
+    throw new TRPCError({ code: "BAD_REQUEST", message: "booking_has_no_organizer" });
+  }
+
   const organizer = await prisma.user.findFirstOrThrow({
     where: {
-      id: booking.userId || 0,
+      id: booking.userId,
     },
     select: {
       name: true,
       email: true,
       timeZone: true,
       locale: true,
     },
   });
📝 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.

Suggested change
const organizer = await prisma.user.findFirstOrThrow({
where: {
id: booking.userId || 0,
},
select: {
name: true,
email: true,
timeZone: true,
locale: true,
},
});
if (!booking.userId) {
throw new TRPCError({ code: "BAD_REQUEST", message: "booking_has_no_organizer" });
}
const organizer = await prisma.user.findFirstOrThrow({
where: {
id: booking.userId,
},
select: {
name: true,
email: true,
timeZone: true,
locale: true,
},
});
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 58 to 68, the code uses booking.userId || 0 which masks a null userId and
can cause a findFirstOrThrow for id 0; add an explicit null/undefined check for
booking.userId before calling prisma, and if missing throw a clear, contextual
error (or handle according to business logic) so you never query for id 0 — then
pass the validated booking.userId to prisma.user.findFirstOrThrow with the same
select fields.

Comment on lines +70 to +72
const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase())
: [];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Trim whitespace when parsing blacklisted emails.

The blacklisted email parsing doesn't trim whitespace, which could cause legitimate emails to not match if the environment variable contains extra spaces.

   const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
-    ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase())
+    ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.trim().toLowerCase())
     : [];
📝 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.

Suggested change
const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase())
: [];
const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS
? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.trim().toLowerCase())
: [];
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 70 to 72, the BLACKLISTED_GUEST_EMAILS parsing doesn't trim whitespace so
entries like " a@b.com" won't match; update the parsing to split on commas, map
each entry through trim() then toLowerCase(), and filter out any empty strings
(e.g., after trimming) so the resulting array contains normalized, non-empty
lowercase emails.

Comment on lines +165 to +171
await eventManager.updateCalendarAttendees(evt, booking);

try {
await sendAddGuestsEmails(evt, guests);
} catch (err) {
console.log("Error sending AddGuestsEmails");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing error handling for calendar update and insufficient email error logging.

Two issues:

  1. eventManager.updateCalendarAttendees(evt, booking) at line 165 is not wrapped in error handling. If the calendar update fails, the entire operation will throw, even though attendees have already been added to the database.
  2. The email sending error at line 170 only logs to console instead of using the proper logger imported at the top of the file.

Apply this fix:

   const eventManager = new EventManager({
     ...user,
     credentials: [...credentials],
   });

-  await eventManager.updateCalendarAttendees(evt, booking);
+  try {
+    await eventManager.updateCalendarAttendees(evt, booking);
+  } catch (err) {
+    logger.error("Error updating calendar attendees", err);
+    // Continue - don't fail the operation if calendar update fails
+  }

   try {
     await sendAddGuestsEmails(evt, guests);
   } catch (err) {
-    console.log("Error sending AddGuestsEmails");
+    logger.error("Error sending AddGuestsEmails", err);
   }

   return { message: "Guests added" };
📝 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.

Suggested change
await eventManager.updateCalendarAttendees(evt, booking);
try {
await sendAddGuestsEmails(evt, guests);
} catch (err) {
console.log("Error sending AddGuestsEmails");
}
try {
await eventManager.updateCalendarAttendees(evt, booking);
} catch (err) {
logger.error("Error updating calendar attendees", err);
// Continue - don't fail the operation if calendar update fails
}
try {
await sendAddGuestsEmails(evt, guests);
} catch (err) {
logger.error("Error sending AddGuestsEmails", err);
}
return { message: "Guests added" };
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 165 to 171, wrap the call to eventManager.updateCalendarAttendees(evt,
booking) in a try/catch so a calendar update failure does not abort the whole
handler after DB changes; on catch, log the error with the module logger
(include evt.id/booking id for context) and continue. Also replace the
console.log in the sendAddGuestsEmails catch with the module logger
(logger.error) and include the caught error object and contextual details so the
failure is recorded with stack/message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants