fix: guests are not adding when reschedule booking#23620
fix: guests are not adding when reschedule booking#23620anikdhabal merged 6 commits intocalcom:mainfrom
Conversation
WalkthroughThe handler at packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts now persists added guests into booking.responses by merging existing booking.responses with a new guests array that concatenates booking.responses?.guests and the computed uniqueGuests. It casts booking.responses to the new BookingResponses type. The attendees createMany logic and function signatures remain unchanged; no exported/public API signatures were modified. Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
@anikdhabal is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
bot
added
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (5)
175-176: Send emails only to actually-added guestsUse
uniqueGuests(post-filtered) to avoid emailing blacklisted/duplicate or already-present attendees.- await sendAddGuestsEmails(evt, guests); + await sendAddGuestsEmails(evt, uniqueGuests);
70-81: Normalize and de-duplicate input guest emails; honor blacklist case-insensitivelyPrevents duplicate attendee rows and inconsistent casing.
- const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS - ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((email) => email.toLowerCase()) - : []; - - const uniqueGuests = guests.filter( - (guest) => - !booking.attendees.some((attendee) => guest === attendee.email) && - !blacklistedGuestEmails.includes(guest) - ); + const blacklistedGuestEmails = process.env.BLACKLISTED_GUEST_EMAILS + ? process.env.BLACKLISTED_GUEST_EMAILS.split(",").map((e) => e.trim().toLowerCase()) + : []; + const attendeeEmails = new Set(booking.attendees.map((a) => a.email.toLowerCase())); + const normalizedGuests = guests.map((g) => g.trim().toLowerCase()); + const uniqueGuests = Array.from( + new Set(normalizedGuests.filter((g) => !attendeeEmails.has(g) && !blacklistedGuestEmails.includes(g))) + );
101-104: Harden createMany against races/duplicatesSkip duplicates at DB level (assuming unique index on [bookingId,email]); reduces failure surface if the same email slips through.
attendees: { createMany: { - data: guestsFullDetails, + data: guestsFullDetails, + skipDuplicates: true, }, },
26-42: Replace broad Prisma include with minimal select (performance, security guideline)Avoid fetching unnecessary relations (especially
user.credentials). Select only what’s needed.- const booking = await prisma.booking.findUnique({ + const booking = await prisma.booking.findUnique({ where: { id: bookingId, }, - include: { - attendees: true, - eventType: true, - destinationCalendar: true, - references: true, - user: { - include: { - destinationCalendar: true, - credentials: true, - }, - }, - }, + select: { + id: true, + userId: true, + userPrimaryEmail: true, + title: true, + description: true, + startTime: true, + endTime: true, + uid: true, + iCalUID: true, + location: true, + responses: true, + attendees: { select: { email: true } }, + eventType: { + select: { + title: true, + teamId: true, + recurringEvent: true, + hideOrganizerEmail: true, + seatsPerTimeSlot: true, + seatsShowAttendees: true, + customReplyToEmail: true, + }, + }, + destinationCalendar: true, + references: { select: { type: true, meetingId: true, meetingPassword: true, meetingUrl: true } }, + user: { select: { destinationCalendar: true } }, + }, });
92-110: Wrap the read and update ofbooking.responsesin a single transaction (or use atomic JSON merging) to avoid lost concurrent writes.
TheresponsesJSON column is also updated in editLocation.handler.ts and confirm.handler.ts, so spreading a stale snapshot (...booking.responses) risks overwriting concurrent edits (e.g., location updates or other response fields). Use prisma.$transaction or a DB‐side JSON patch (e.g. jsonb_set) to atomically appendguestswithout dropping otherresponsesdata.
🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (3)
96-99: Use select instead of include on update resultReturn only what you need from
bookingAttendees.- include: { - attendees: true, - }, + select: { + attendees: { select: { name: true, email: true, timeZone: true, locale: true } }, + },
176-178: Log the error object for diagnosisKeep message, add the error for observability.
- } catch (err) { - console.log("Error sending AddGuestsEmails"); - } + } catch (err) { + console.error("Error sending AddGuestsEmails", err); + }
52-56: Minor: attendee check should be case-insensitiveIf emails differ only by case, the current check can misclassify role.
- const isAttendee = !!booking.attendees.find((attendee) => attendee.email === user.email); + const isAttendee = !!booking.attendees.find((attendee) => attendee.email.toLowerCase() === user.email.toLowerCase());
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
Pull request was converted to draft
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (2)
75-79: Normalize and de-duplicate input emails case-insensitively before filteringPrevents duplicate attendee rows and aligns comparisons regardless of casing.
Apply:
- const uniqueGuests = guests.filter( - (guest) => - !booking.attendees.some((attendee) => guest === attendee.email) && - !blacklistedGuestEmails.includes(guest) - ); + // Preserve first-seen casing while comparing case-insensitively + const normalizedMap = new Map<string, string>(); + for (const g of guests) { + const k = g.toLowerCase().trim(); + if (k) normalizedMap.set(k, normalizedMap.get(k) ?? g); + } + const dedupedInputGuests = Array.from(normalizedMap.values()); + const uniqueGuests = dedupedInputGuests.filter( + (guest) => + !booking.attendees.some( + (attendee) => (attendee.email ?? "").toLowerCase() === guest.toLowerCase() + ) && !blacklistedGuestEmails.includes(guest.toLowerCase()) + );
178-181: Email only the filtered, deduplicated guestsAvoid emailing blacklisted/duplicate addresses by using the finalized set.
Apply:
- await sendAddGuestsEmails(evt, guests); + await sendAddGuestsEmails(evt, Array.from(new Set(uniqueGuests)));
♻️ Duplicate comments (2)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (2)
93-93: Don’t cast nullable responses; build a safe payload and de-duplicate guests
booking.responsesisnullableper schema. Spreading a nullable value later causesTypeError: Cannot convert undefined or null to object. Also ensure we de-duplicate new guests before persisting.Apply:
- const bookingResponses = booking.responses as BookingResponses; + // Build safe responses payload and dedupe guests + const prevResponses = (booking.responses ?? {}) as Record<string, unknown>; + const prevGuests = Array.isArray((prevResponses as any).guests) + ? ((prevResponses as any).guests as string[]) + : []; + // uniqueGuests is computed below (see separate comment to normalize + dedupe inputs) + const dedupedUniqueGuests = Array.from(new Set(uniqueGuests)); + const responsesPayload = { + ...prevResponses, + guests: Array.from(new Set([...prevGuests, ...dedupedUniqueGuests])), + };
108-111: Use the precomputed saferesponsesPayloadinstead of spreading a nullable objectPrevents runtime crash and ensures guest list uniqueness.
Apply:
- responses: { - ...bookingResponses, - guests: [...(bookingResponses?.guests || []), ...uniqueGuests], - }, + responses: responsesPayload,
🧹 Nitpick comments (4)
packages/prisma/zod-utils.ts (1)
180-181: Export a non-nullable alias to reduce downstream null checks and unsafe spreadsThe current alias infers
BookingResponsesasobject | null. Many call-sites (like the handler in this PR) tend to spread this value. Exporting a non-nullable alias helps avoid accidental...nullerrors.Apply:
export type BookingResponses = z.infer<typeof bookingResponses>; +export type NonNullableBookingResponses = NonNullable<z.infer<typeof bookingResponses>>;packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (3)
103-107: AddskipDuplicatessafety tocreateManyAs a guardrail against race conditions or edge-case dupes.
Apply:
attendees: { createMany: { data: guestsFullDetails, + skipDuplicates: true, }, },
27-43: Prisma: useselect(noinclude) and drop unusedcredentialsper guidelinesOnly fetch fields used later and avoid fetching sensitive data unnecessarily.
Apply:
- const booking = await prisma.booking.findUnique({ + const booking = await prisma.booking.findUnique({ where: { id: bookingId, }, - include: { - attendees: true, - eventType: true, - destinationCalendar: true, - references: true, - user: { - include: { - destinationCalendar: true, - credentials: true, - }, - }, - }, + select: { + id: true, + title: true, + description: true, + startTime: true, + endTime: true, + uid: true, + iCalUID: true, + location: true, + userPrimaryEmail: true, + userId: true, + attendees: { select: { name: true, email: true, timeZone: true, locale: true } }, + eventType: { + select: { + title: true, + teamId: true, + hideOrganizerEmail: true, + seatsPerTimeSlot: true, + seatsShowAttendees: true, + customReplyToEmail: true, + recurringEvent: true, + }, + }, + destinationCalendar: true, + references: true, + user: { + select: { + name: true, + email: true, + timeZone: true, + locale: true, + destinationCalendar: true, + }, + }, + responses: true, + }, });
95-102: Prisma: preferselectoverincludefor attendees on updateFetch only what you use to build
attendeesList.Apply:
- include: { - attendees: true, - }, + select: { + attendees: { select: { name: true, email: true, timeZone: true, locale: true } }, + },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/prisma/zod-utils.ts(1 hunks)packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/prisma/zod-utils.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/prisma/zod-utils.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/prisma/zod-utils.tspackages/trpc/server/routers/viewer/bookings/addGuests.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1)
packages/prisma/zod-utils.ts (2)
bookingResponses(155-178)BookingResponses(180-180)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (1)
packages/trpc/server/routers/viewer/bookings/addGuests.handler.ts (1)
9-9: LGTM: type-only importNo bundle impact; consistent with usage.
E2E results are ready! |
4 participants
What does this PR do?
After a successful booking when any guest added from the add guests option, those newly added guests are not included during reschedule