Add guest management functionality to existing bookings#8
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>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request introduces a feature to add multiple guests to existing bookings. It includes a new UI dialog for email input, a backend TRPC handler for guest management with access control, email templates for notification (organizers and attendees), multi-email form components, and supporting translations. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as AddGuestsDialog
participant TRPC as TRPC Handler
participant DB as Prisma/Database
participant Calendar as EventManager
participant Email as Email Manager
User->>UI: Opens dialog, enters guest emails
User->>UI: Clicks "Add"
UI->>UI: Validates emails (unique, valid format)
UI->>TRPC: Calls viewer.bookings.addGuests(bookingId, emails)
TRPC->>DB: Fetch booking + attendees
TRPC->>TRPC: Verify user access (organizer/admin/attendee)
TRPC->>TRPC: Filter guests (deduplicate, remove blacklist)
TRPC->>DB: Create attendee records
TRPC->>Calendar: Update calendar event attendees
TRPC->>Email: Trigger sendAddGuestsEmails()
Email->>Email: Send OrganizerAddGuestsEmail
Email->>Email: Send AttendeeAddGuestsEmail to new guests
Email->>Email: Send AttendeeScheduledEmail to existing attendees
Email-->>UI: Success
UI->>UI: Show success toast, close dialog
UI->>UI: Invalidate bookings cache
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @apps/web/components/booking/BookingListItem.tsx:
- Around line 193-200: The addGuests flow needs server-side and client-side
booking status/time validation: in the backend addGuests handler (the
function/file named addGuests handler) fetch booking.status and booking.startAt
(or equivalent start time) before adding guests and reject with a 4xx if the
booking is in the past or status is not an active/confirmed state (mirror the
logic used in requestReschedule.handler.ts — check for cancelled/rejected and
startAt < now). On the frontend BookingListItem.tsx hide/disable the
"add_members" action (the menu item with id "add_members" / icon "user-plus" and
the setIsOpenAddGuestsDialog(true) call) when booking.status is cancelled or
rejected or when booking.startAt is before now so the UI matches the backend
guard. Ensure error messages returned by the handler are specific so the UI can
surface them.
In @apps/web/components/dialog/AddGuestsDialog.tsx:
- Around line 48-58: The current handleAdd and ZAddGuestsInputSchema validation
miss edge cases: update handleAdd to first filter multiEmailValue for non-empty,
non-whitespace strings (e.g., const nonEmptyEmails = multiEmailValue.filter(e =>
e.trim() !== "") ) and use nonEmptyEmails for the empty-check and for the
validation call instead of raw multiEmailValue; when validation fails call
setIsInvalidEmail(true) as before and pass nonEmptyEmails to
addGuestsMutation.mutate({ bookingId, guests: nonEmptyEmails }). Also update
ZAddGuestsInputSchema (the schema used in the safeParse) to include a custom
message on the refine that enforces uniqueness (e.g., add the message: "Emails
must be unique") so validation errors are descriptive.
- Around line 60-106: Reset the dialog-local state when the dialog closes by
clearing multiEmailValue and isInvalidEmail whenever isOpenDialog becomes false;
specifically, add a useEffect that watches isOpenDialog and when it transitions
to false calls setMultiEmailValue([""]) and setIsInvalidEmail(false). This
ensures the same cleanup currently done in the Cancel button handler (which
calls setMultiEmailValue and setIsInvalidEmail) also runs for other close
actions triggered via the Dialog's onOpenChange (isOpenDialog/setIsOpenDialog).
In @packages/emails/email-manager.ts:
- Around line 525-550: In sendAddGuestsEmails, the comparison
newGuests.includes(attendee.email) is case-sensitive; normalize both sides by
lowercasing emails before comparing — update sendAddGuestsEmails to use
newGuestsLower = newGuests.map(e => e.toLowerCase()) (or equivalent) and check
newGuestsLower.includes(attendee.email.toLowerCase()) when deciding which email
class to send; also ensure the addGuests handler logic that lowercases
blacklisted emails and guest emails (referenced in addGuests.handler.ts)
consistently normalizes incoming guest addresses to lowercase before
deduplication/blacklist checks so comparisons are case-insensitive across both
places.
In @packages/emails/templates/organizer-add-guests-email.ts:
- Line 25: The replyTo currently expands to [this.calEvent.organizer.email,
...this.calEvent.attendees.map(({ email }) => email)] which can create oversized
headers for large attendee lists; update the logic in the code that builds
replyTo (where replyTo is set) to limit entries—e.g., always include
this.calEvent.organizer.email and then append only the first N attendee emails
(pick a constant like 10) or omit attendees entirely when
this.calEvent.attendees.length exceeds a threshold; ensure you dedupe and filter
falsy emails when slicing and keep the same replyTo shape (array) so callers
remain unchanged.
In @packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts:
- Around line 46-56: The access check currently sets isTeamAdminOrOwner by
ANDing isTeamAdmin and isTeamOwner which wrongly requires both roles; change the
logic so isTeamAdminOrOwner is true if either isTeamAdmin(user.id,
booking.eventType?.teamId ?? 0) OR isTeamOwner(user.id,
booking.eventType?.teamId ?? 0) and update the conditional that throws the
TRPCError accordingly; while doing this, consolidate the two role checks into a
single optimized call (or helper) that returns whether the user has admin or
owner privileges for the team to avoid two separate DB queries.
- Around line 74-78: uniqueGuests filtering is doing case-sensitive comparisons
causing misses; normalize emails to lowercase when comparing by changing the
predicate in guests.filter so guest is lowercased for both checks: compare
guest.toLowerCase() against blacklistedGuestEmails entries (already lowercased)
and against booking.attendees by lowercasing attendee.email (e.g., ensure the
booking.attendees.some(...) uses attendee.email.toLowerCase() for a
case-insensitive match); update the guests.filter predicate and any related
variable names if needed to reflect the normalized comparison.
- Around line 58-68: The code uses booking.userId || 0 when calling
prisma.user.findFirstOrThrow which can mask a null userId and cause a misleading
lookup for id=0; replace this by explicitly checking booking.userId for
null/undefined and either throw a clear error (e.g., "booking.userId is
missing") or handle the case before calling prisma, then call
prisma.user.findUniqueOrThrow (or findFirstOrThrow) with the real booking.userId
to fetch the organizer and assign to the organizer variable.
- Around line 167-171: Replace the console.log in the sendAddGuestsEmails catch
block with the project's logger: import logger from @calcom/lib/logger (same
pattern used in requestReschedule.handler.ts/editLocation.handler.ts) and call
logger.error including a descriptive message and the caught error (e.g.,
logger.error("Error sending AddGuestsEmails", { evt, guests, err })). Keep the
current non-fatal behavior if intended, but add a short TODO/comment near the
catch noting that email failures are intentionally non-blocking (or change to
rethrow if you want the whole operation to fail) so team intent is documented.
In @packages/ui/form/MultiEmail.tsx:
- Around line 27-53: Remove the redundant empty label prop from the EmailField
usage (label={<></>}) since no name prop is passed and the label is optional,
and make the required prop conditional so newly added empty emails do not
trigger validation; update the EmailField props in MultiEmail.tsx to set
required only when the field has a value and the component is not readOnly
(i.e., use the current field value and readOnly flag to compute required).
🧹 Nitpick comments (6)
packages/ui/form/MultiEmailLazy.tsx (1)
4-4: Simplify the type assertion for better type safety.The current double type assertion (
as unknown as typeof) bypasses TypeScript's type checking and could break if the module structure changes. Next.js dynamic imports support a cleaner pattern.♻️ Proposed refactor for cleaner typing
-const MultiEmail = dynamic(() => import("./MultiEmail")) as unknown as typeof import("./MultiEmail").default; +const MultiEmail = dynamic(() => import("./MultiEmail").then((mod) => mod.default));Alternatively, if you need to maintain the exact type:
+import type MultiEmailType from "./MultiEmail"; -const MultiEmail = dynamic(() => import("./MultiEmail")) as unknown as typeof import("./MultiEmail").default; +const MultiEmail = dynamic(() => import("./MultiEmail")) as unknown as typeof MultiEmailType;packages/emails/templates/organizer-add-guests-email.ts (1)
9-9: Simplify toAddresses since it's always a single recipient.The array wrapping appears unnecessary since there's always exactly one recipient (either the team member or organizer email). The
.join(",")on Line 24 works but suggests potential confusion.♻️ Proposed simplification
- const toAddresses = [this.teamMember?.email || this.calEvent.organizer.email]; + const toAddress = this.teamMember?.email || this.calEvent.organizer.email; return { icalEvent: { filename: "event.ics", content: generateIcsString({ event: this.calEvent, title: this.t("new_guests_added"), subtitle: this.t("emailed_you_and_any_other_attendees"), role: "organizer", status: "CONFIRMED", }), method: "REQUEST", }, from: `${APP_NAME} <${this.getMailerOptions().from}>`, - to: toAddresses.join(","), + to: toAddress, replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)],apps/web/components/dialog/AddGuestsDialog.tsx (1)
18-22: Consider modern TypeScript naming convention.The interface is named
IAddGuestsDialogusing the Hungarian notation "I" prefix, which is not recommended in modern TypeScript style guides. Consider renaming toAddGuestsDialogPropsfor better consistency with React conventions.♻️ Suggested refactor
-interface IAddGuestsDialog { +interface AddGuestsDialogProps { isOpenDialog: boolean; setIsOpenDialog: Dispatch<SetStateAction<boolean>>; bookingId: number; } -export const AddGuestsDialog = (props: IAddGuestsDialog) => { +export const AddGuestsDialog = (props: AddGuestsDialogProps) => {packages/ui/form/MultiEmail.tsx (2)
15-16: Consider extracting class string to a constant.The
inputClassNameis a long string that could be extracted to improve readability. This is a minor style improvement.♻️ Optional refactor
+const EMAIL_INPUT_CLASSNAME = + "dark:placeholder:text-muted focus:border-emphasis border-subtle block w-full rounded-md border-default text-sm focus:ring-black disabled:bg-emphasis disabled:hover:cursor-not-allowed dark:selection:bg-green-500 disabled:dark:text-subtle bg-default"; + function MultiEmail({ value, readOnly, label, setValue, placeholder }: MultiEmailProps) { const { t } = useLocale(); value = value || []; - const inputClassName = - "dark:placeholder:text-muted focus:border-emphasis border-subtle block w-full rounded-md border-default text-sm focus:ring-black disabled:bg-emphasis disabled:hover:cursor-not-allowed dark:selection:bg-green-500 disabled:dark:text-subtle bg-default";
57-91: LGTM with minor cleanup suggestion.The add functionality is well-implemented with proper state management. The component correctly handles both the initial "add" state and subsequent "add another" actions.
Minor cleanup: Lines 73-75 render an empty fragment that serves no purpose and can be removed:
</div> - ) : ( - <></> - )} + )}packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1)
83-90: Document guest attendee behavior.Line 85 sets the guest name to an empty string. While this may be intentional (guests added via email may not have provided names), it's worth documenting this behavior or considering whether the system should:
- Extract a name from the email address as a default
- Mark these attendees as "Guest" instead of empty
- Keep the current behavior but add a code comment explaining why
Consider adding a comment:
const guestsFullDetails = uniqueGuests.map((guest) => { return { name: "", // Guests added by email don't have names initially; can be updated later email: guest, timeZone: organizer.timeZone, locale: organizer.locale, }; });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/web/components/booking/BookingListItem.tsxapps/web/components/dialog/AddGuestsDialog.tsxapps/web/public/static/locales/en/common.jsonpackages/emails/email-manager.tspackages/emails/src/templates/AttendeeAddGuestsEmail.tsxpackages/emails/src/templates/OrganizerAddGuestsEmail.tsxpackages/emails/src/templates/index.tspackages/emails/templates/attendee-add-guests-email.tspackages/emails/templates/organizer-add-guests-email.tspackages/trpc/server/routers/viewer/bookings/_router.tsxpackages/trpc/server/routers/viewer/bookings/addGuests.handler.tspackages/trpc/server/routers/viewer/bookings/addGuests.schema.tspackages/ui/form/MultiEmail.tsxpackages/ui/form/MultiEmailLazy.tsxpackages/ui/index.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
packages/emails/email-manager.ts (4)
packages/types/Calendar.d.ts (1)
CalendarEvent(157-211)packages/lib/formatCalendarEvent.ts (1)
formatCalEvent(19-27)packages/emails/src/templates/index.ts (3)
OrganizerAddGuestsEmail(38-38)AttendeeScheduledEmail(9-9)AttendeeAddGuestsEmail(39-39)packages/emails/src/templates/AttendeeAddGuestsEmail.tsx (1)
AttendeeAddGuestsEmail(3-10)
packages/emails/src/templates/AttendeeAddGuestsEmail.tsx (2)
packages/emails/src/templates/index.ts (2)
AttendeeAddGuestsEmail(39-39)AttendeeScheduledEmail(9-9)packages/emails/templates/attendee-add-guests-email.ts (1)
AttendeeAddGuestsEmail(5-34)
packages/trpc/server/routers/viewer/bookings/_router.tsx (1)
packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1)
ZAddGuestsInputSchema(3-6)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (5)
packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1)
TAddGuestsInputSchema(8-8)packages/lib/server/queries/teams/index.ts (2)
isTeamAdmin(264-284)isTeamOwner(286-295)packages/lib/server/getUsersCredentials.ts (1)
getUsersCredentials(8-19)packages/core/EventManager.ts (1)
EventManager(99-1044)packages/emails/email-manager.ts (1)
sendAddGuestsEmails(525-550)
packages/ui/form/MultiEmail.tsx (3)
apps/web/next.config.js (1)
value(95-95)packages/lib/hooks/useLocale.ts (1)
useLocale(5-22)packages/ui/index.tsx (3)
EmailField(21-21)Tooltip(130-130)Button(14-14)
apps/web/components/booking/BookingListItem.tsx (1)
apps/web/components/dialog/AddGuestsDialog.tsx (1)
AddGuestsDialog(24-107)
🔇 Additional comments (15)
packages/ui/index.tsx (1)
153-153: LGTM!The export follows the established pattern for lazy-loaded form components (AddressInput, PhoneInput) and is correctly placed in the appropriate section.
packages/emails/src/templates/index.ts (1)
38-39: LGTM!The new email template exports follow the established pattern and are correctly positioned in the export list.
apps/web/components/booking/BookingListItem.tsx (1)
357-361: LGTM!The dialog integration follows the established pattern used for other booking dialogs (RescheduleDialog, ReassignDialog, EditLocationDialog). State management and props are correctly implemented.
packages/emails/src/templates/AttendeeAddGuestsEmail.tsx (1)
1-10: LGTM! Clean email template wrapper.The component follows the established pattern used by similar email templates, correctly wrapping
AttendeeScheduledEmailwith preconfigured props for the "add guests" use case.packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts (1)
1-8: LGTM! Well-defined input schema.The Zod schema provides appropriate validation with email format checking for the guests array. The type export follows TypeScript best practices.
packages/emails/src/templates/OrganizerAddGuestsEmail.tsx (1)
1-11: LGTM! Consistent organizer email template.The component correctly wraps
OrganizerScheduledEmailwith appropriate preset props. The explicitcallToAction={null}is a sensible choice for the organizer's add-guests notification.packages/emails/email-manager.ts (1)
21-21: LGTM! New email template imports.The imports are correctly added for the new guest management email templates.
Also applies to: 52-52
packages/trpc/server/routers/viewer/bookings/_router.tsx (3)
4-4: LGTM! Schema import added.The import is correctly placed with other schema imports.
18-18: LGTM! Type definition extended.The handler cache type is properly extended to include the new
addGuestshandler.
79-95: LGTM! Route implementation follows established pattern.The
addGuestsmutation correctly implements the lazy-loading pattern with proper input validation, authentication viaauthedProcedure, and type-safe runtime checks. The implementation is consistent with other routes in this router.apps/web/components/dialog/AddGuestsDialog.tsx (1)
35-46: LGTM!The mutation handlers are well-structured:
- Success handler properly shows toast, closes dialog, resets form, and invalidates the cache
- Error handler gracefully handles optional error codes and provides a fallback message
apps/web/public/static/locales/en/common.json (1)
1122-1122: LGTM!The new translation keys are well-structured and follow existing conventions:
- Clear, descriptive messages for user-facing text
- Proper variable interpolation syntax in email subject
- Appropriately placed near related translations
- Grammatically correct and consistent with the application's tone
Also applies to: 1124-1124, 1136-1137, 1139-1139, 2434-2434
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (2)
1-21: LGTM!Imports are well-organized and the type definition is clean and appropriate.
108-156: LGTM!The calendar event construction is comprehensive and handles all necessary fields properly:
- Attendee translations are loaded correctly
- Proper fallback logic for organizer email and destination calendar
- Video call data is conditionally included
- All required CalendarEvent fields are populated
packages/emails/templates/attendee-add-guests-email.ts (1)
6-33: The iCalendar combination used is correct. According to RFC 5546 (iTIP), when an organizer adds guests to a confirmed event,method: "REQUEST"is the appropriate iTIP message type to send to new attendees. Thestatus: "CONFIRMED"property separately indicates the event's confirmation state and does not conflict with the REQUEST method—these address different concerns per the specification. The codebase uses this combination consistently across all event scheduling templates and complies with RFC 5546 requirements.Likely an incorrect or invalid review comment.
| { | ||
| id: "add_members", | ||
| label: t("additional_guests"), | ||
| onClick: () => { | ||
| setIsOpenAddGuestsDialog(true); | ||
| }, | ||
| icon: "user-plus" as const, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the addGuests handler implementation
rg -n "addGuests" apps/ packages/ --type ts --type tsx | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 114
🏁 Script executed:
# Search for booking status validations in the server/routers
rg -n -B5 -A10 "addGuests" packages/trpc/server/routers/ --type tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 8147
🏁 Script executed:
# Look for BookingStatus validation patterns
rg -n "BookingStatus\.(CANCELLED|REJECTED|COMPLETED)" packages/trpc/server/routers/viewer/bookings/ --type ts | head -30Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 817
🏁 Script executed:
# Find the addGuests procedure definition more specifically
fd -e ts -e tsx packages/trpc/server/routers/ | xargs rg -l "addGuests" | head -5Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 695
🏁 Script executed:
cat -n packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 6526
🏁 Script executed:
# Check what other handlers do for booking status validation
rg -n -A5 "BookingStatus" packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 625
🏁 Script executed:
# Check if booking status is validated anywhere in the action flow
rg -n "booking.status" packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 69
🏁 Script executed:
# Check the BookingListItem logic for when editBookingActions are shown
cat -n apps/web/components/booking/BookingListItem.tsx | sed -n '540,570p'Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1799
🏁 Script executed:
# Check the isBookingInPast logic
rg -n "isBookingInPast" apps/web/components/booking/BookingListItem.tsx -B2 -A2Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 3054
🏁 Script executed:
# Check where editBookingActions is defined and filtered
rg -n "editBookingActions" apps/web/components/booking/BookingListItem.tsx | head -15Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 208
🏁 Script executed:
# Check if booking.status is included in the Prisma query for addGuests
cat -n packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts | sed -n '26,42p'Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 547
🏁 Script executed:
# Check the schema to understand what fields are included
cat -n packages/trpc/server/routers/viewer/bookings/addGuests.schema.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 360
Add booking status validation to prevent adding guests to past, cancelled, or rejected bookings.
The backend addGuests handler lacks status validation. It allows guests to be added to any confirmed booking without checking if it's in the past or if the status is cancelled/rejected. The handler doesn't even fetch the booking.status field. This is inconsistent with other handlers like requestReschedule which explicitly validate booking status.
Additionally, the UI in BookingListItem.tsx (line 554) displays the "add guests" action for past confirmed bookings without backend guard rails to prevent this operation.
Add a booking status check in the handler to reject past bookings and invalid statuses (similar to requestReschedule.handler.ts).
🤖 Prompt for AI Agents
In @apps/web/components/booking/BookingListItem.tsx around lines 193 - 200, The
addGuests flow needs server-side and client-side booking status/time validation:
in the backend addGuests handler (the function/file named addGuests handler)
fetch booking.status and booking.startAt (or equivalent start time) before
adding guests and reject with a 4xx if the booking is in the past or status is
not an active/confirmed state (mirror the logic used in
requestReschedule.handler.ts — check for cancelled/rejected and startAt < now).
On the frontend BookingListItem.tsx hide/disable the "add_members" action (the
menu item with id "add_members" / icon "user-plus" and the
setIsOpenAddGuestsDialog(true) call) when booking.status is cancelled or
rejected or when booking.startAt is before now so the UI matches the backend
guard. Ensure error messages returned by the handler are specific so the UI can
surface them.
| const handleAdd = () => { | ||
| if (multiEmailValue.length === 0) { | ||
| return; | ||
| } | ||
| const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue); | ||
| if (validationResult.success) { | ||
| addGuestsMutation.mutate({ bookingId, guests: multiEmailValue }); | ||
| } else { | ||
| setIsInvalidEmail(true); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve email validation logic.
The validation has two issues:
-
Line 49: The check
multiEmailValue.length === 0won't catch the case where the user hasn't entered any emails but still has the initial empty string[""]. Consider checking for empty or whitespace-only emails:const nonEmptyEmails = multiEmailValue.filter(email => email.trim() !== ""); if (nonEmptyEmails.length === 0) { return; }
-
Lines 26-29: The Zod schema's
refinefunction lacks a custom error message, making it harder to debug validation failures. Add a message parameter:const ZAddGuestsInputSchema = z.array(z.string().email()).refine((emails) => { const uniqueEmails = new Set(emails); return uniqueEmails.size === emails.length; }, { message: "Emails must be unique" });
♻️ Proposed fix
const handleAdd = () => {
- if (multiEmailValue.length === 0) {
+ const nonEmptyEmails = multiEmailValue.filter(email => email.trim() !== "");
+ if (nonEmptyEmails.length === 0) {
return;
}
- const validationResult = ZAddGuestsInputSchema.safeParse(multiEmailValue);
+ const validationResult = ZAddGuestsInputSchema.safeParse(nonEmptyEmails);
if (validationResult.success) {
- addGuestsMutation.mutate({ bookingId, guests: multiEmailValue });
+ addGuestsMutation.mutate({ bookingId, guests: nonEmptyEmails });
} else {
setIsInvalidEmail(true);
}
};🤖 Prompt for AI Agents
In @apps/web/components/dialog/AddGuestsDialog.tsx around lines 48 - 58, The
current handleAdd and ZAddGuestsInputSchema validation miss edge cases: update
handleAdd to first filter multiEmailValue for non-empty, non-whitespace strings
(e.g., const nonEmptyEmails = multiEmailValue.filter(e => e.trim() !== "") ) and
use nonEmptyEmails for the empty-check and for the validation call instead of
raw multiEmailValue; when validation fails call setIsInvalidEmail(true) as
before and pass nonEmptyEmails to addGuestsMutation.mutate({ bookingId, guests:
nonEmptyEmails }). Also update ZAddGuestsInputSchema (the schema used in the
safeParse) to include a custom message on the refine that enforces uniqueness
(e.g., add the message: "Emails must be unique") so validation errors are
descriptive.
| return ( | ||
| <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}> | ||
| <DialogContent enableOverflow> | ||
| <div className="flex flex-row space-x-3"> | ||
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | ||
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | ||
| </div> | ||
| <div className="w-full pt-1"> | ||
| <DialogHeader title={t("additional_guests")} /> | ||
| <MultiEmail | ||
| label={t("add_emails")} | ||
| value={multiEmailValue} | ||
| readOnly={false} | ||
| setValue={setMultiEmailValue} | ||
| /> | ||
|
|
||
| {isInvalidEmail && ( | ||
| <div className="my-4 flex text-sm text-red-700"> | ||
| <div className="flex-shrink-0"> | ||
| <Icon name="triangle-alert" className="h-5 w-5" /> | ||
| </div> | ||
| <div className="ml-3"> | ||
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <DialogFooter> | ||
| <Button | ||
| onClick={() => { | ||
| setMultiEmailValue([""]); | ||
| setIsInvalidEmail(false); | ||
| setIsOpenDialog(false); | ||
| }} | ||
| type="button" | ||
| color="secondary"> | ||
| {t("cancel")} | ||
| </Button> | ||
| <Button data-testid="add_members" loading={addGuestsMutation.isPending} onClick={handleAdd}> | ||
| {t("add")} | ||
| </Button> | ||
| </DialogFooter> | ||
| </div> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reset error state when dialog closes.
The isInvalidEmail state is reset when the Cancel button is clicked (lines 90-92), but not when the dialog is closed via other means (e.g., clicking outside or pressing ESC). This could leave the error message visible when the dialog is reopened.
♻️ Proposed fix
- <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}>
+ <Dialog open={isOpenDialog} onOpenChange={(open) => {
+ if (!open) {
+ setMultiEmailValue([""]);
+ setIsInvalidEmail(false);
+ }
+ setIsOpenDialog(open);
+ }}>Alternatively, you could use a useEffect to reset state when the dialog closes:
useEffect(() => {
if (!isOpenDialog) {
setMultiEmailValue([""]);
setIsInvalidEmail(false);
}
}, [isOpenDialog]);📝 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.
| return ( | |
| <Dialog open={isOpenDialog} onOpenChange={setIsOpenDialog}> | |
| <DialogContent enableOverflow> | |
| <div className="flex flex-row space-x-3"> | |
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | |
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | |
| </div> | |
| <div className="w-full pt-1"> | |
| <DialogHeader title={t("additional_guests")} /> | |
| <MultiEmail | |
| label={t("add_emails")} | |
| value={multiEmailValue} | |
| readOnly={false} | |
| setValue={setMultiEmailValue} | |
| /> | |
| {isInvalidEmail && ( | |
| <div className="my-4 flex text-sm text-red-700"> | |
| <div className="flex-shrink-0"> | |
| <Icon name="triangle-alert" className="h-5 w-5" /> | |
| </div> | |
| <div className="ml-3"> | |
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | |
| </div> | |
| </div> | |
| )} | |
| <DialogFooter> | |
| <Button | |
| onClick={() => { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| setIsOpenDialog(false); | |
| }} | |
| type="button" | |
| color="secondary"> | |
| {t("cancel")} | |
| </Button> | |
| <Button data-testid="add_members" loading={addGuestsMutation.isPending} onClick={handleAdd}> | |
| {t("add")} | |
| </Button> | |
| </DialogFooter> | |
| </div> | |
| </div> | |
| </DialogContent> | |
| </Dialog> | |
| ); | |
| return ( | |
| <Dialog open={isOpenDialog} onOpenChange={(open) => { | |
| if (!open) { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| } | |
| setIsOpenDialog(open); | |
| }}> | |
| <DialogContent enableOverflow> | |
| <div className="flex flex-row space-x-3"> | |
| <div className="bg-subtle flex h-10 w-10 flex-shrink-0 justify-center rounded-full "> | |
| <Icon name="user-plus" className="m-auto h-6 w-6" /> | |
| </div> | |
| <div className="w-full pt-1"> | |
| <DialogHeader title={t("additional_guests")} /> | |
| <MultiEmail | |
| label={t("add_emails")} | |
| value={multiEmailValue} | |
| readOnly={false} | |
| setValue={setMultiEmailValue} | |
| /> | |
| {isInvalidEmail && ( | |
| <div className="my-4 flex text-sm text-red-700"> | |
| <div className="flex-shrink-0"> | |
| <Icon name="triangle-alert" className="h-5 w-5" /> | |
| </div> | |
| <div className="ml-3"> | |
| <p className="font-medium">{t("emails_must_be_unique_valid")}</p> | |
| </div> | |
| </div> | |
| )} | |
| <DialogFooter> | |
| <Button | |
| onClick={() => { | |
| setMultiEmailValue([""]); | |
| setIsInvalidEmail(false); | |
| setIsOpenDialog(false); | |
| }} | |
| type="button" | |
| color="secondary"> | |
| {t("cancel")} | |
| </Button> | |
| <Button data-testid="add_members" loading={addGuestsMutation.isPending} onClick={handleAdd}> | |
| {t("add")} | |
| </Button> | |
| </DialogFooter> | |
| </div> | |
| </div> | |
| </DialogContent> | |
| </Dialog> | |
| ); |
🤖 Prompt for AI Agents
In @apps/web/components/dialog/AddGuestsDialog.tsx around lines 60 - 106, Reset
the dialog-local state when the dialog closes by clearing multiEmailValue and
isInvalidEmail whenever isOpenDialog becomes false; specifically, add a
useEffect that watches isOpenDialog and when it transitions to false calls
setMultiEmailValue([""]) and setIsInvalidEmail(false). This ensures the same
cleanup currently done in the Cancel button handler (which calls
setMultiEmailValue and setIsInvalidEmail) also runs for other close actions
triggered via the Dialog's onOpenChange (isOpenDialog/setIsOpenDialog).
| 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)); | ||
| } | ||
| }) | ||
| ); | ||
|
|
||
| await Promise.all(emailsToSend); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for email normalization patterns in the codebase
rg -n '(toLowerCase|toLocaleLowerCase|normalizeEmail)' packages/emails/ --type=ts -C2
# Check for email includes comparisons
rg -n '\.includes\([^)]*email[^)]*\)' packages/emails/email-manager.ts -C2
# Look for email comparison patterns in the entire packages directory
rg -n 'email.*includes|includes.*email' packages/ --type=ts | head -20
# Check how newGuests is used and where it comes from
rg -n 'newGuests' packages/emails/email-manager.ts -B5 -A5Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 10772
🏁 Script executed:
# Find where sendAddGuestsEmails is called
rg -n 'sendAddGuestsEmails' --type=ts -B3 -A3
# Check how CalendarEvent and attendees are created/normalized
rg -n 'formatCalEvent|CalendarEvent' packages/emails/email-manager.ts -B2 -A2
# Look for email normalization functions in the codebase
fd -t f -e ts -e tsx | xargs rg -l 'normalizeEmail|toLowerCase.*email|email.*toLowerCase' | head -10
# Check how attendee.email is set in the system
rg -n 'attendee\.email.*=' packages/ --type=ts | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 12300
🏁 Script executed:
# Read the addGuests.handler.ts file to understand how guests are constructed
cat -n packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts | head -100
# Look for where guests are normalized or processed
rg -n 'guests' packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts -B2 -A2 | head -50Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 4478
🏁 Script executed:
# Check the schema for how attendee emails are stored
fd -t f -name "*.prisma" | xargs cat | grep -A10 -B10 "attendee" | head -50
# Look for any email normalization in the booking creation
rg -n 'toLowerCase.*email|normalizeEmail' packages/ --type=ts | head -20
# Check the input schema for addGuests
cat -n packages/trpc/server/routers/viewer/bookings/addGuests.schema.ts
# Check if attendee.email is normalized when stored
rg -n 'attendee.*email.*=' packages/features/bookings/lib/ --type=ts -A2 | head -30Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 3372
🏁 Script executed:
# Find Prisma schema file
find . -name "schema.prisma" -type f
# Check for email normalization at database creation
rg -n 'email.*lowercase|email.*normalize' packages/prisma/ --type=ts
# Look at how emails are stored when creating attendees
rg -n 'attendees.*create|createMany' packages/features/bookings/lib/ --type=ts -B3 -A5 | head -40Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 3026
🏁 Script executed:
# Extract Attendee model from Prisma schema
rg -A20 "^model Attendee" packages/prisma/schema.prisma
# Check if there's any email field configuration
rg -B5 -A15 "model Attendee" packages/prisma/schema.prismaRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1180
Normalize email addresses before comparison to prevent case-sensitivity issues.
The comparison at line 541 (newGuests.includes(attendee.email)) is case-sensitive, which will cause new guests to receive incorrect email templates if there's a case mismatch. Additionally, the handler file has an inconsistency where blacklisted emails are normalized to lowercase (line 71 of addGuests.handler.ts) but guest emails are compared without normalization (line 76-77), potentially allowing duplicate guests to be added if they differ only in case.
Normalize email addresses with .toLowerCase() before comparison in both the handler and email-manager functions to ensure consistent behavior.
🤖 Prompt for AI Agents
In @packages/emails/email-manager.ts around lines 525 - 550, In
sendAddGuestsEmails, the comparison newGuests.includes(attendee.email) is
case-sensitive; normalize both sides by lowercasing emails before comparing —
update sendAddGuestsEmails to use newGuestsLower = newGuests.map(e =>
e.toLowerCase()) (or equivalent) and check
newGuestsLower.includes(attendee.email.toLowerCase()) when deciding which email
class to send; also ensure the addGuests handler logic that lowercases
blacklisted emails and guest emails (referenced in addGuests.handler.ts)
consistently normalizes incoming guest addresses to lowercase before
deduplication/blacklist checks so comparisons are case-insensitive across both
places.
| }, | ||
| from: `${APP_NAME} <${this.getMailerOptions().from}>`, | ||
| to: toAddresses.join(","), | ||
| replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)], |
There was a problem hiding this comment.
Consider limiting the replyTo list for events with many guests.
Including all attendees in the replyTo field could cause issues:
- Email servers and clients may have limits on header sizes (typically 998 characters per line per RFC 5322)
- Events with many guests (e.g., 50+) could exceed these limits
- This may cause email delivery failures or truncation
💡 Potential solutions
Option 1: Limit to organizer only
- replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)],
+ replyTo: [this.calEvent.organizer.email],Option 2: Limit to first N attendees
- replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)],
+ replyTo: [
+ this.calEvent.organizer.email,
+ ...this.calEvent.attendees.slice(0, 5).map(({ email }) => email)
+ ],Option 3: Add conditional logic based on attendee count
+ replyTo: this.calEvent.attendees.length > 10
+ ? [this.calEvent.organizer.email]
+ : [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)],📝 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.
| replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)], | |
| replyTo: [this.calEvent.organizer.email], |
| replyTo: [this.calEvent.organizer.email, ...this.calEvent.attendees.map(({ email }) => email)], | |
| replyTo: [ | |
| this.calEvent.organizer.email, | |
| ...this.calEvent.attendees.slice(0, 5).map(({ email }) => email) | |
| ], |
🤖 Prompt for AI Agents
In @packages/emails/templates/organizer-add-guests-email.ts at line 25, The
replyTo currently expands to [this.calEvent.organizer.email,
...this.calEvent.attendees.map(({ email }) => email)] which can create oversized
headers for large attendee lists; update the logic in the code that builds
replyTo (where replyTo is set) to limit entries—e.g., always include
this.calEvent.organizer.email and then append only the first N attendee emails
(pick a constant like 10) or omit attendees entirely when
this.calEvent.attendees.length exceeds a threshold; ensure you dedupe and filter
falsy emails when slicing and keep the same replyTo shape (array) so callers
remain unchanged.
| const isTeamAdminOrOwner = | ||
| (await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) && | ||
| (await isTeamOwner(user.id, booking.eventType?.teamId ?? 0)); | ||
|
|
||
| const isOrganizer = booking.userId === user.id; | ||
|
|
||
| const isAttendee = !!booking.attendees.find((attendee) => attendee.email === user.email); | ||
|
|
||
| if (!isTeamAdminOrOwner && !isOrganizer && !isAttendee) { | ||
| throw new TRPCError({ code: "FORBIDDEN", message: "you_do_not_have_permission" }); | ||
| } |
There was a problem hiding this comment.
Fix access control logic.
Line 46-48 uses && (AND) between isTeamAdmin and isTeamOwner, requiring a user to be both an admin AND an owner. This is overly restrictive and likely incorrect. Typically, either role should be sufficient to add guests.
Change to:
const isTeamAdminOrOwner =
(await isTeamAdmin(user.id, booking.eventType?.teamId ?? 0)) ||
(await isTeamOwner(user.id, booking.eventType?.teamId ?? 0));Additionally, the current logic performs two separate database queries that could be optimized into a single query.
🐛 Proposed fix
- 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 - 56, The access check currently sets isTeamAdminOrOwner by ANDing
isTeamAdmin and isTeamOwner which wrongly requires both roles; change the logic
so isTeamAdminOrOwner is true if either isTeamAdmin(user.id,
booking.eventType?.teamId ?? 0) OR isTeamOwner(user.id,
booking.eventType?.teamId ?? 0) and update the conditional that throws the
TRPCError accordingly; while doing this, consolidate the two role checks into a
single optimized call (or helper) that returns whether the user has admin or
owner privileges for the team to avoid two separate DB queries.
| const organizer = await prisma.user.findFirstOrThrow({ | ||
| where: { | ||
| id: booking.userId || 0, | ||
| }, | ||
| select: { | ||
| name: true, | ||
| email: true, | ||
| timeZone: true, | ||
| locale: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add null check for booking.userId.
Line 60 uses booking.userId || 0 as a fallback, but a userId of 0 will cause findFirstOrThrow to fail. Since the booking has already been validated to exist, the userId should not be null, but it's safer to add an explicit check.
🛡️ Defensive check
+ if (!booking.userId) {
+ throw new TRPCError({ code: "NOT_FOUND", message: "booking_organizer_not_found" });
+ }
+
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.
| 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: "NOT_FOUND", message: "booking_organizer_not_found" }); | |
| } | |
| 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 - 68, The code uses booking.userId || 0 when calling
prisma.user.findFirstOrThrow which can mask a null userId and cause a misleading
lookup for id=0; replace this by explicitly checking booking.userId for
null/undefined and either throw a clear error (e.g., "booking.userId is
missing") or handle the case before calling prisma, then call
prisma.user.findUniqueOrThrow (or findFirstOrThrow) with the real booking.userId
to fetch the organizer and assign to the organizer variable.
| const uniqueGuests = guests.filter( | ||
| (guest) => | ||
| !booking.attendees.some((attendee) => guest === attendee.email) && | ||
| !blacklistedGuestEmails.includes(guest) | ||
| ); |
There was a problem hiding this comment.
Fix case-insensitive email comparison.
Line 77 compares guest (not lowercased) against blacklistedGuestEmails (which are lowercased on line 71). This creates a case-sensitivity bug where User@Example.com wouldn't match the blacklisted user@example.com.
🐛 Proposed fix
const uniqueGuests = guests.filter(
(guest) =>
!booking.attendees.some((attendee) => guest === attendee.email) &&
- !blacklistedGuestEmails.includes(guest)
+ !blacklistedGuestEmails.includes(guest.toLowerCase())
);Also consider normalizing the attendee email comparison for consistency:
!booking.attendees.some((attendee) => guest.toLowerCase() === attendee.email.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.
| const uniqueGuests = guests.filter( | |
| (guest) => | |
| !booking.attendees.some((attendee) => guest === attendee.email) && | |
| !blacklistedGuestEmails.includes(guest) | |
| ); | |
| const uniqueGuests = guests.filter( | |
| (guest) => | |
| !booking.attendees.some((attendee) => guest === attendee.email) && | |
| !blacklistedGuestEmails.includes(guest.toLowerCase()) | |
| ); |
🤖 Prompt for AI Agents
In @packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 74 - 78, uniqueGuests filtering is doing case-sensitive comparisons
causing misses; normalize emails to lowercase when comparing by changing the
predicate in guests.filter so guest is lowercased for both checks: compare
guest.toLowerCase() against blacklistedGuestEmails entries (already lowercased)
and against booking.attendees by lowercasing attendee.email (e.g., ensure the
booking.attendees.some(...) uses attendee.email.toLowerCase() for a
case-insensitive match); update the guests.filter predicate and any related
variable names if needed to reflect the normalized comparison.
| try { | ||
| await sendAddGuestsEmails(evt, guests); | ||
| } catch (err) { | ||
| console.log("Error sending AddGuestsEmails"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "addGuests.handler.ts" 2>/dev/null | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 154
🏁 Script executed:
wc -l ./packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 158
🏁 Script executed:
sed -n '160,174p' ./packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 410
🏁 Script executed:
head -50 ./packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1659
🏁 Script executed:
grep -n "logger\|console\." ./packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 140
🏁 Script executed:
find ./packages/trpc/server/routers/viewer/bookings -type f -name "*.ts" | head -5 | xargs grep -l "logger\|getLogger" 2>/dev/nullRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 71
🏁 Script executed:
rg "logger\.|console\.(log|error|warn)" packages/trpc/server/routers/viewer/bookings/ --type ts -c -lRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 367
🏁 Script executed:
rg "console\.(log|error|warn)|logger" packages/trpc/server/routers/viewer/bookings/confirm.handler.ts --type ts -B2 -A2Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 295
🏁 Script executed:
rg "sendAddGuestsEmails|sendEmails" packages/trpc/server/routers/viewer/bookings/ --type ts -B3 -A3Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1457
🏁 Script executed:
sed -n '20,80p' ./packages/trpc/server/routers/viewer/bookings/addGuests.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1754
🏁 Script executed:
rg "import.*logger|from.*logger" packages/trpc/server/routers/ --type ts | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1746
🏁 Script executed:
rg "logger\.(error|warn|info)" packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts --type ts -B1 -A1Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 69
🏁 Script executed:
grep -n "logger" packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 208
🏁 Script executed:
sed -n '40,120p' packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts | grep -A5 "catch\|log\."Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 459
🏁 Script executed:
rg "catch|throw|error" packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts --type ts -B2 -A2Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1542
🏁 Script executed:
rg "sendAddGuestsEmails\|sendEmails" packages/ --type ts -B5 -A5 | head -50Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 69
Replace console.log with proper logging using the logger from @calcom/lib/logger.
Line 170 uses console.log for error logging. The codebase has an established logger pattern (seen in requestReschedule.handler.ts and editLocation.handler.ts) that should be used here for consistency and better observability.
+import logger from "@calcom/lib/logger";
+
export const addGuestsHandler = async ({ ctx, input }: AddGuestsOptions) => {
const { user } = ctx;
const { bookingId, guests } = input;
+
+ const log = logger.getSubLogger({ prefix: ["addGuestsHandler"] });Then update the error handling:
try {
await sendAddGuestsEmails(evt, guests);
} catch (err) {
- console.log("Error sending AddGuestsEmails");
+ log.error("Error sending AddGuestsEmails", { bookingId, error: err });
}Additionally, consider whether email failures should fail the entire operation. The current implementation allows guests to be added even if email notifications fail. This may be the desired behavior, but it should be documented or discussed with the team.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts around
lines 167 - 171, Replace the console.log in the sendAddGuestsEmails catch block
with the project's logger: import logger from @calcom/lib/logger (same pattern
used in requestReschedule.handler.ts/editLocation.handler.ts) and call
logger.error including a descriptive message and the caught error (e.g.,
logger.error("Error sending AddGuestsEmails", { evt, guests, err })). Keep the
current non-fatal behavior if intended, but add a short TODO/comment near the
catch noting that email failures are intentionally non-blocking (or change to
rethrow if you want the whole operation to fail) so team intent is documented.
| <EmailField | ||
| disabled={readOnly} | ||
| value={field} | ||
| className={inputClassName} | ||
| onChange={(e) => { | ||
| const updatedValue = [...value]; | ||
| updatedValue[index] = e.target.value; | ||
| setValue(updatedValue); | ||
| }} | ||
| placeholder={placeholder} | ||
| label={<></>} | ||
| required | ||
| onClickAddon={() => { | ||
| const updatedValue = [...value]; | ||
| updatedValue.splice(index, 1); | ||
| setValue(updatedValue); | ||
| }} | ||
| addOnSuffix={ | ||
| !readOnly ? ( | ||
| <Tooltip content="Remove email"> | ||
| <button className="m-1" type="button"> | ||
| <Icon name="x" width={12} className="text-default" /> | ||
| </button> | ||
| </Tooltip> | ||
| ) : null | ||
| } | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find EmailField component definition
echo "=== Searching for EmailField component ==="
fd -t f -e tsx -e ts | xargs grep -l "export.*EmailField" | head -5
# Search for EmailField interface or type definition
echo "=== Searching for EmailField type definition ==="
rg "interface EmailField|type EmailField" -A 15 --type ts --type tsx
# Also check the specific file to understand imports
echo "=== Checking MultiEmail.tsx imports ==="
head -30 packages/ui/form/MultiEmail.tsxRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 1539
🏁 Script executed:
#!/bin/bash
# Read the fields.tsx file where EmailField is defined
echo "=== EmailField component definition ==="
cat packages/ui/form/fields.tsx | head -200
# Also search for EmailField within the file with context
echo -e "\n=== Searching for EmailField export ==="
rg "export.*EmailField" -A 30 packages/ui/form/fields.tsxRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 6605
🏁 Script executed:
#!/bin/bash
# Read the full MultiEmail.tsx file
echo "=== Full MultiEmail.tsx content ==="
cat packages/ui/form/MultiEmail.tsx
# Also check if EmailField is extended/wrapped in the UI index
echo -e "\n=== Check UI index exports ==="
rg "EmailField" packages/ui/index.tsx -B 2 -A 2Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 3283
🏁 Script executed:
#!/bin/bash
# Search for onClickAddon and addOnSuffix definitions
echo "=== Searching for onClickAddon definition ==="
rg "onClickAddon|addOnSuffix" --type ts --type tsx -B 3 -A 3
# Check if there's a custom wrapper for EmailField
echo -e "\n=== Searching for EmailField in ui index ==="
cat packages/ui/index.tsx | grep -A 20 "export.*EmailField\|from.*fields"Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 223
🏁 Script executed:
#!/bin/bash
# Search without type flags
echo "=== Searching for onClickAddon ==="
rg "onClickAddon" -B 3 -A 3
# Check packages/ui/index.tsx content
echo -e "\n=== UI index exports ==="
cat packages/ui/index.tsx | head -100
# Find what's exported from form/fields
echo -e "\n=== What is exported from form/fields ==="
rg "export" packages/ui/form/fields.tsx | head -20Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 7901
🏁 Script executed:
#!/bin/bash
# Find EmailField in components/form
echo "=== Finding EmailField in components/form ==="
find packages/ui/components/form -name "*.tsx" -o -name "*.ts" | xargs grep -l "EmailField" | head -5
# Check TextField.tsx which has the addon functionality
echo -e "\n=== Reading TextField.tsx (first 150 lines) ==="
head -150 packages/ui/components/form/inputs/TextField.tsx
# Check if EmailField is defined there
echo -e "\n=== Searching for EmailField export in components/form ==="
rg "export.*EmailField" packages/ui/components/form -B 5 -A 10Repository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 6440
🏁 Script executed:
#!/bin/bash
# Read the types.d.ts to see InputFieldProps definition
echo "=== InputFieldProps type definition ==="
cat packages/ui/components/form/inputs/types.d.tsRepository: AI-Code-Review-Evals/coderabbit-cal_dot_com
Length of output: 885
Simplify empty label and reconsider required attribute.
Two observations about the EmailField usage:
-
Line 37:
label={<></>}is redundant since nonameprop is passed to EmailField, which means the label won't render regardless. Since thelabelprop is optional, simply remove this line or omit the label prop entirely. -
Line 38: The
requiredattribute is always true, even for newly added empty fields. Consider making it conditional to avoid premature validation errors:required={!!field && !readOnly}
🤖 Prompt for AI Agents
In @packages/ui/form/MultiEmail.tsx around lines 27 - 53, Remove the redundant
empty label prop from the EmailField usage (label={<></>}) since no name prop is
passed and the label is optional, and make the required prop conditional so
newly added empty emails do not trigger validation; update the EmailField props
in MultiEmail.tsx to set required only when the field has a value and the
component is not readOnly (i.e., use the current field value and readOnly flag
to compute required).
2 participants
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)*