fix: handle collective multiple host on destinationCalendar

[ShashankFC]

Test 4nn---nReplicated from ai-code-review-evaluation/cal.com-greptile#4

[ShashankFC]

@claude

[claude]

Claude finished @ShashankFC's task in 2m 58s —— View job

---

Code Review Complete

I've reviewed the PR changes that refactor destinationCalendar from a single object to an array to support collective events with multiple hosts. Here's my detailed feedback:

---

Critical Issues

1. Potential Null Pointer Exception in EventManager.ts:119

Location: packages/core/EventManager.ts:119

const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") {

Issue: When destinationCalendar is an empty array or undefined, mainHostDestinationCalendar will be undefined, causing a runtime error when accessing .integration.

Fix: Add a null check:

if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") {

Fix this →

2. Incorrect Logic in Google Calendar Service deleteEvent

Location: packages/app-store/googlecalendar/lib/CalendarService.ts:315-316

const calendarId = externalCalendarId
  ? externalCalendarId
  : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;

Issue: This logic is circular - if externalCalendarId is falsy, it tries to find a calendar whose externalId matches externalCalendarId (which is falsy). This will never work.

Fix: Should likely be:

const calendarId = externalCalendarId
  ? externalCalendarId
  : event.destinationCalendar?.[0]?.externalId;

Fix this →

3. Similar Issue in updateEvent

Location: packages/app-store/googlecalendar/lib/CalendarService.ts:255-256

Same circular logic problem exists in updateEvent method.

---

High Priority Issues

4. Type Safety Issue with destinationCalendar

Location: packages/types/Calendar.d.ts:171

destinationCalendar?: DestinationCalendar[] | null;

Issue: Using both | null and optional ? is redundant and can lead to inconsistent handling. The codebase mixes checks for null, undefined, and empty arrays.

Recommendation: Standardize to either DestinationCalendar[] | null (non-optional) or DestinationCalendar[]? (optional, no null).

5. Missing Null Safety in handleNewBooking.ts

Location: packages/features/bookings/lib/handleNewBooking.ts:1076-1079

if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") {
  evt.destinationCalendar?.push(...teamDestinationCalendars);
}

Issue: If evt.destinationCalendar is null (as per the type definition), this will throw a runtime error. The optional chaining ?. prevents the error but silently fails to add calendars.

Fix: Ensure destinationCalendar is initialized:

if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") {
  if (!evt.destinationCalendar) evt.destinationCalendar = [];
  evt.destinationCalendar.push(...teamDestinationCalendars);
}

Fix this →

---

Medium Priority Issues

6. Inconsistent credentialId Parameter

Location: packages/app-store/googlecalendar/lib/CalendarService.ts:87

async createEvent(calEventRaw: CalendarEvent, credentialId: number): Promise<NewCalendarEventType>

Issue: The credentialId parameter is added but the base Calendar interface in Calendar.d.ts:221 doesn't require it:

createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType>;

Concern: This signature change affects all calendar integrations. Verify that:

• All implementations have been updated (Office365, Lark, etc.)
• The interface is correctly updated to match
• Existing calls pass the correct credentialId

7. Database Query in Loop

Location: packages/core/EventManager.ts:343-366

for (const destination of event.destinationCalendar) {
  if (destination.credentialId) {
    let credential = this.calendarCredentials.find((c) => c.id === destination.credentialId);
    if (!credential) {
      const credentialFromDB = await prisma.credential.findUnique({ ... });
      // ...
    }
  }
}

Issue: This queries the database inside a loop, which is inefficient for collective events with many hosts.

Recommendation: Batch fetch all missing credentials:

const missingCredentialIds = event.destinationCalendar
  .map(d => d.credentialId)
  .filter(id => id && !this.calendarCredentials.find(c => c.id === id));

const credentialsFromDB = await prisma.credential.findMany({
  where: { id: { in: missingCredentialIds } },
  include: { app: { select: { slug: true } } }
});

---

Code Quality Issues

8. Typo Fix Already Included

Location: packages/features/bookings/lib/handleNewBooking.ts:370

Good catch fixing orginalBookingDuration → originalBookingDuration

9. Empty firstName/lastName Fields

Location: packages/features/bookings/lib/handleNewBooking.ts:997-998

firstName: "",
lastName: "",

Issue: These fields are added but always set to empty strings. Either populate them properly or remove them.

10. Duplicate Check Removed

Location: packages/core/EventManager.ts:573-575

The duplicate if (!calendarReference) check was fixed. Good cleanup!

---

Testing Recommendations

This is a significant refactoring that touches critical booking flows. Ensure testing covers:

1. Collective events with multiple hosts, each with different calendar integrations

2. Round-robin events (should not be affected)

3. Single-user events (backward compatibility)

4. Edge cases:

   • Users without destination calendars
   • Mixed credential scenarios (some users with Google, some with Office365)
   • Events where calendar creation fails for some but not all hosts
   • Cancellation and rescheduling of collective events

5. Database migration: Ensure existing bookings with single destinationCalendar are handled correctly

---

Summary

Positives:

• Good approach to solving the multi-host calendar problem
• Proper credential fetching fallback when not in memory
• Consistent pattern across create/update/delete operations

Must Fix Before Merge:

1. Null pointer exception in EventManager.ts:119
2. Circular logic in Google Calendar deleteEvent and updateEvent
3. Null safety in handleNewBooking.ts:1076-1079

Recommend Addressing:
4. Type safety standardization for destinationCalendar
5. Database query optimization (N+1 problem)
6. Interface consistency for credentialId parameter

---

| Branch

[github-actions]

This PR is being marked as stale due to inactivity.