fix: handle collective multiple host on destinationCalendar#9
Conversation
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 7 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if (evt.location === MeetLocationType && evt.destinationCalendar?.integration !== "google_calendar") { | ||
| // @NOTE: destinationCalendar it's an array now so as a fallback we will only check the first one | ||
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | ||
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { |
There was a problem hiding this comment.
Missing optional chaining causes crash on empty array
High Severity
When evt.destinationCalendar is null, undefined, or an empty array, mainHostDestinationCalendar will be undefined. Accessing .integration on undefined without optional chaining throws a TypeError at runtime. The original code used evt.destinationCalendar?.integration, which was safe. The new code drops the optional chaining on mainHostDestinationCalendar.
| const selectedCalendar = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.externalId; | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
Fallback calendar lookup always returns undefined in updateEvent
High Severity
In updateEvent, the ternary checks externalCalendarId ? externalCalendarId : event.destinationCalendar?.find(cal => cal.externalId === externalCalendarId). The else branch only executes when externalCalendarId is falsy, so find compares against a falsy value and will never match, always returning undefined. The same bug exists in deleteEvent. The original code simply returned the single destination calendar's externalId as a fallback.
Additional Locations (1)
| create: { | ||
| name, | ||
| ...(!IS_TEAM_BILLING_ENABLED && { slug }), | ||
| ...(IS_TEAM_BILLING_ENABLED ? { slug } : {}), |
There was a problem hiding this comment.
Organization slug logic is inverted from original
High Severity
The original code ...(!IS_TEAM_BILLING_ENABLED && { slug }) set slug directly when billing was disabled. The new code ...(IS_TEAM_BILLING_ENABLED ? { slug } : {}) sets slug when billing is enabled — the exact opposite. This means organizations will get a slug set in the wrong billing scenario, breaking the intended billing/approval flow.
| }; | ||
|
|
||
| if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") { | ||
| evt.destinationCalendar?.push(...teamDestinationCalendars); |
There was a problem hiding this comment.
Collective team calendars silently dropped when destinationCalendar null
Medium Severity
When neither eventType.destinationCalendar nor organizerUser.destinationCalendar is set, evt.destinationCalendar is null. The subsequent evt.destinationCalendar?.push(...teamDestinationCalendars) silently no-ops due to optional chaining, discarding all collected team member calendars for COLLECTIVE events. This defeats the PR's purpose of supporting per-member calendar configurations.
| }) | ||
| : eventType.hosts?.length | ||
| ? eventType.hosts.map(({ user, isFixed }) => ({ | ||
| }); |
There was a problem hiding this comment.
loadUsers removed organization select breaking downstream usage
Medium Severity
The refactored loadUsers for the dynamic user list case (no eventTypeId) removed organization: { select: { slug: true } } from the Prisma query. The userSelect used in ...userSelect.select does not include an organization relationship (only organizationId), so any downstream code that relied on user.organization.slug will now get undefined.
| externalCalendarId: isCalendarType ? evt.destinationCalendar?.externalId : undefined, | ||
| credentialId: isCalendarType ? evt.destinationCalendar?.credentialId : result.credentialId, | ||
| externalCalendarId: isCalendarType ? result.externalId : undefined, | ||
| credentialId: isCalendarType ? result.credentialId : undefined, |
There was a problem hiding this comment.
Video reference credentialId dropped during booking creation
High Severity
For non-calendar results (video events), credentialId is now set to undefined instead of result.credentialId. The createMeeting function returns a credentialId on video results, and downstream code in handleCancelBooking relies on bookingVideoReference.credentialId to find the specific video credential for deletion. Storing undefined means video booking references lose their credential association.
| externalCalendarId: evt.destinationCalendar?.externalId, | ||
| credentialId: result.credentialId ?? evt.destinationCalendar?.credentialId, | ||
| externalCalendarId: result.externalId, | ||
| credentialId: result.credentialId ?? undefined, |
There was a problem hiding this comment.
Location update references lose credentialId and externalCalendarId
Medium Severity
In updateLocation, the old code fell back to evt.destinationCalendar?.externalId and evt.destinationCalendar?.credentialId when result didn't contain those fields. Since CalendarManager.updateEvent never returns externalId or credentialId, result.externalId and result.credentialId are always undefined for calendar results. The old fallback compensated for this, but the new code replaces it with bare undefined. These references are persisted to the database via editLocation.handler.ts, so subsequent cancel or reschedule operations won't be able to identify the correct calendar or credential.
2 participants
Test 4nn
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#4](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/4)*Note
Medium Risk
Touches core booking-to-calendar sync paths (create/update/cancel) across multiple integrations and changes a widely used event shape, so regressions could impact calendar writes and webhook payload compatibility.
Overview
Enables syncing a single booking to multiple destination calendars by changing
CalendarEvent.destinationCalendarfrom a single value to an array and updating all event builders/webhooks/emails to emit[]/[calendar]consistently.Updates core calendar orchestration (
EventManager,CalendarManager) to create/update/delete events for each destination calendar reference, including credential lookups from DB when missing locally, and extendsEventResultto carryexternalId/credentialIdfor correct reference persistence.Adjusts calendar providers (notably Google) to select the target calendar by
credentialIdwhen creating events, and updates cancellation/payment/reminder handlers and e2e webhook expectations to match the new destination calendar shape and collective-host behavior.Written by Cursor Bugbot for commit bd797c1. Configure here.