Skip to content
Jump to bottom

Automated Test: fix/handle-collective-multiple-host-destinations #367

Closed
wants to merge 1 commit into from
+368 −216
Closed

Automated Test: fix/handle-collective-multiple-host-destinations #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web/pages/api/cron/bookingReminder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
});

const attendeesList = await Promise.all(attendeesListPromises);

const selectedDestinationCalendar = booking.destinationCalendar || user.destinationCalendar;
const evt: CalendarEvent = {
type: booking.title,
title: booking.title,
Expand All @@ -127,7 +127,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
attendees: attendeesList,
uid: booking.uid,
recurringEvent: parseRecurringEvent(booking.eventType?.recurringEvent),
destinationCalendar: booking.destinationCalendar || user.destinationCalendar,
destinationCalendar: selectedDestinationCalendar ? [selectedDestinationCalendar] : [],
};

await sendOrganizerRequestReminderEmail(evt);
Expand Down
2 changes: 1 addition & 1 deletion apps/web/playwright/webhook.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ test.describe("BOOKING_REJECTED", async () => {
},
],
location: "[redacted/dynamic]",
destinationCalendar: null,
destinationCalendar: [],
// hideCalendarNotes: false,
requiresConfirmation: "[redacted/dynamic]",
eventTypeId: "[redacted/dynamic]",
Expand Down
33 changes: 22 additions & 11 deletions packages/app-store/googlecalendar/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default class GoogleCalendarService implements Calendar {
};
};

async createEvent(calEventRaw: CalendarEvent): Promise<NewCalendarEventType> {
async createEvent(calEventRaw: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> {
const eventAttendees = calEventRaw.attendees.map(({ id: _id, ...rest }) => ({
...rest,
responseStatus: "accepted",
Expand All @@ -97,6 +97,10 @@ export default class GoogleCalendarService implements Calendar {
responseStatus: "accepted",
})) || [];
return new Promise(async (resolve, reject) => {
const [mainHostDestinationCalendar] =
calEventRaw?.destinationCalendar && calEventRaw?.destinationCalendar.length > 0
? calEventRaw.destinationCalendar
: [];
const myGoogleAuth = await this.auth.getToken();
const payload: calendar_v3.Schema$Event = {
summary: calEventRaw.title,
Expand All @@ -115,8 +119,8 @@ export default class GoogleCalendarService implements Calendar {
id: String(calEventRaw.organizer.id),
responseStatus: "accepted",
organizer: true,
email: calEventRaw.destinationCalendar?.externalId
? calEventRaw.destinationCalendar.externalId
email: mainHostDestinationCalendar?.externalId
? mainHostDestinationCalendar.externalId
: calEventRaw.organizer.email,
},
...eventAttendees,
Expand All @@ -138,13 +142,16 @@ export default class GoogleCalendarService implements Calendar {
const calendar = google.calendar({
version: "v3",
});
const selectedCalendar = calEventRaw.destinationCalendar?.externalId
? calEventRaw.destinationCalendar.externalId
: "primary";
// Find in calEventRaw.destinationCalendar the one with the same credentialId

const selectedCalendar = calEventRaw.destinationCalendar?.find(
(cal) => cal.credentialId === credentialId
)?.externalId;

calendar.events.insert(
{
auth: myGoogleAuth,
calendarId: selectedCalendar,
calendarId: selectedCalendar || "primary",
requestBody: payload,
conferenceDataVersion: 1,
sendUpdates: "none",
Expand Down Expand Up @@ -188,6 +195,8 @@ export default class GoogleCalendarService implements Calendar {

async updateEvent(uid: string, event: CalendarEvent, externalCalendarId: string): Promise<any> {
return new Promise(async (resolve, reject) => {
const [mainHostDestinationCalendar] =
event?.destinationCalendar && event?.destinationCalendar.length > 0 ? event.destinationCalendar : [];
const myGoogleAuth = await this.auth.getToken();
const eventAttendees = event.attendees.map(({ ...rest }) => ({
...rest,
Expand Down Expand Up @@ -216,8 +225,8 @@ export default class GoogleCalendarService implements Calendar {
id: String(event.organizer.id),
organizer: true,
responseStatus: "accepted",
email: event.destinationCalendar?.externalId
? event.destinationCalendar.externalId
email: mainHostDestinationCalendar?.externalId
? mainHostDestinationCalendar.externalId
: event.organizer.email,
},
...(eventAttendees as any),
Expand All @@ -244,7 +253,7 @@ export default class GoogleCalendarService implements Calendar {

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

calendar.events.update(
{
Expand Down Expand Up @@ -303,7 +312,9 @@ export default class GoogleCalendarService implements Calendar {
});

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

Comment on lines +315 to 318
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && wc -l CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 112

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '300,330p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1119

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && grep -n "deleteEvent" CalendarService.ts | head -20

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 201

🏁 Script executed:

cd packages/app-store/googlecalendar && find . -name "*.ts" -type f | xargs grep -l "interface CalendarEvent\|type CalendarEvent" | head -5

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 72

🏁 Script executed:

cd packages && rg "interface CalendarEvent|type CalendarEvent" -A 10

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1896

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '306,340p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1428

🏁 Script executed:

cd packages && rg "interface CalendarEvent" -A 50 types/Calendar.d.ts | head -80

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1752

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && grep -n "destinationCalendar" CalendarService.ts | head -20

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 719

🏁 Script executed:

cd packages && rg "destinationCalendar" -B 2 -A 2 types/Calendar.d.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 293

🏁 Script executed:

cd packages && rg "interface DestinationCalendar|type DestinationCalendar" -A 10 types/Calendar.d.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 72

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '250,270p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 755

🏁 Script executed:

cd packages && rg "DestinationCalendar" types/Calendar.d.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 241

🏁 Script executed:

cd packages && rg "export.*DestinationCalendar" -A 10

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 2443

🏁 Script executed:

cd packages && find . -name "*.d.ts" -type f | xargs grep -l "DestinationCalendar" | head -5

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 111

🏁 Script executed:

cd packages && rg "model DestinationCalendar" -A 15

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1030

🏁 Script executed:

cd packages && rg "SelectedCalendar" types/Calendar.d.ts -B 2 -A 2

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 409

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '240,280p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1431

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '230,260p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 971

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && grep -B 5 "async updateEvent" CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 220

🏁 Script executed:

cd packages && rg "deleteEvent\|updateEvent" --type ts -A 3 packages/app-store/googlecalendar/lib/CalendarService.ts | head -60

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 186

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && rg "async.*Event" CalendarService.ts | head -20

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 399

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '140,180p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1519

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && grep -n "async createEvent\|async updateEvent\|async deleteEvent" CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 410

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '87,160p' CalendarService.ts | head -60

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 2271

🏁 Script executed:

cd packages/app-store/googlecalendar/lib && sed -n '306,340p' CalendarService.ts

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1428

Fix deleteEvent fallback logic when externalCalendarId is missing

The fallback logic on line 315-318 is broken. When externalCalendarId is undefined/null, the .find() searches for cal.externalId === undefined, which never matches any calendar. This causes calendarId to become undefined, forcing deletion to always target the "primary" calendar regardless of where the event was created. Since 404 errors are silently swallowed (line 339), deletion silently fails when the event is on another calendar.

Use the first destination calendar or primary directly:

🔧 Suggested fix 
-      const calendarId = externalCalendarId
-        ? externalCalendarId
-        : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
+      const calendarId =
+        externalCalendarId || event.destinationCalendar?.[0]?.externalId || defaultCalendarId;

       calendar.events.delete(
         {
           auth: myGoogleAuth,
-          calendarId: calendarId ? calendarId : defaultCalendarId,
+          calendarId,
🤖 Prompt for AI Agents 
In `@packages/app-store/googlecalendar/lib/CalendarService.ts` around lines 315 -
318, The fallback for computing calendarId is incorrect: change the logic in the
delete path (where calendarId is computed using externalCalendarId and
event.destinationCalendar) so that when externalCalendarId is null/undefined you
use the first destination calendar's externalId if present, otherwise fall back
to "primary"; update the code that computes calendarId (reference
externalCalendarId, calendarId, and event.destinationCalendar in the deleteEvent
flow) to implement this new fallback order.

calendar.events.delete(
{
Expand Down
12 changes: 8 additions & 4 deletions packages/app-store/larkcalendar/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ export default class LarkCalendarService implements Calendar {
async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> {
let eventId = "";
let eventRespData;
const calendarId = event.destinationCalendar?.externalId;
const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];
const calendarId = mainHostDestinationCalendar?.externalId;
if (!calendarId) {
throw new Error("no calendar id");
Comment on lines 125 to 131
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find the Calendar interface definition
fd "\.ts$" packages/app-store | xargs rg "interface Calendar" -A 20

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1562

🏁 Script executed:

# Check the actual LarkCalendarService createEvent signature
cat packages/app-store/larkcalendar/lib/CalendarService.ts | head -150

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 5129

🏁 Script executed:

# Find all calendar services
fd -type d "packages/app-store/*/lib" | grep -E "calendar|outlook|google" | head -10

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 260

🏁 Script executed:

# Find the Calendar interface definition in types
fd -type f "Calendar.ts" packages/

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 260

🏁 Script executed:

# Search for interface Calendar in the codebase
rg "interface Calendar" --type ts -A 15

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 3906

🏁 Script executed:

# Find all calendar services and their createEvent signatures
find packages/app-store -name "CalendarService.ts" -o -name "calendar.ts" | head -20

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 919

🏁 Script executed:

# Check other calendar services
ls -la packages/app-store | grep -E "google|outlook|caldav|ical"

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 284

🏁 Script executed:

# Check Google Calendar Service createEvent signature
head -200 packages/app-store/googlecalendar/lib/CalendarService.ts | grep -A 5 "createEvent"

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 396

🏁 Script executed:

# Check Outlook Calendar Service createEvent signature
head -200 packages/app-store/office365calendar/lib/CalendarService.ts | grep -A 5 "createEvent"

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 422

🏁 Script executed:

# Check CalDAV Calendar Service createEvent signature
head -200 packages/app-store/caldavcalendar/lib/CalendarService.ts | grep -A 5 "createEvent"

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 72

🏁 Script executed:

# Count how many calendar services have the credentialId parameter
rg "async createEvent\(event: CalendarEvent, credentialId: number\)" packages/app-store

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 72

🏁 Script executed:

# Count how many calendar services still have the old signature
rg "async createEvent\(event: CalendarEvent\):" packages/app-store

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 1649

🏁 Script executed:

# Check if tsconfig enforces strict type checking
cat tsconfig.json | head -50

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 135

🏁 Script executed:

# Check if there's a build or type-check script
cat package.json | grep -A 5 '"scripts"'

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 353

🏁 Script executed:

# Look for any type errors or TODO comments about the credentialId migration
rg "credentialId" packages/app-store --type ts -B 2 -A 2 | head -100

Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror

Length of output: 7274

Interface compliance: createEvent signature mismatch with Calendar interface.

The Calendar interface (packages/types/Calendar.d.ts) requires createEvent(event: CalendarEvent, credentialId: number), but this implementation uses createEvent(event: CalendarEvent) without the credentialId parameter. This violates the interface contract and will cause a TypeScript type error.

Note: This issue affects multiple calendar services (Office 365, Exchange, Hubspot, Salesforce, and others) — only the Google Calendar service has been updated so far. Consider whether this is part of a broader migration effort.

🤖 Prompt for AI Agents 
In `@packages/app-store/larkcalendar/lib/CalendarService.ts` around lines 125 -
131, The createEvent implementation in CalendarService.ts must match the
Calendar interface: change the method signature of createEvent to include the
second parameter credentialId: number (i.e., createEvent(event: CalendarEvent,
credentialId: number)), update any internal references to accept and use
credentialId as needed, and propagate this signature change to all callers of
CalendarService.createEvent (and other calendar service implementations like
Office365/Exchange/Hubspot/Salesforce) so the type contract in
packages/types/Calendar.d.ts is satisfied.

}
Expand Down Expand Up @@ -160,7 +161,8 @@ export default class LarkCalendarService implements Calendar {
}

private createAttendees = async (event: CalendarEvent, eventId: string) => {
const calendarId = event.destinationCalendar?.externalId;
const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];
const calendarId = mainHostDestinationCalendar?.externalId;
if (!calendarId) {
this.log.error("no calendar id provided in createAttendees");
throw new Error("no calendar id provided in createAttendees");
Expand All @@ -187,7 +189,8 @@ export default class LarkCalendarService implements Calendar {
async updateEvent(uid: string, event: CalendarEvent, externalCalendarId?: string) {
const eventId = uid;
let eventRespData;
const calendarId = externalCalendarId || event.destinationCalendar?.externalId;
const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];
const calendarId = externalCalendarId || mainHostDestinationCalendar?.externalId;
if (!calendarId) {
this.log.error("no calendar id provided in updateEvent");
throw new Error("no calendar id provided in updateEvent");
Expand Down Expand Up @@ -231,7 +234,8 @@ export default class LarkCalendarService implements Calendar {
* @returns
*/
async deleteEvent(uid: string, event: CalendarEvent, externalCalendarId?: string) {
const calendarId = externalCalendarId || event.destinationCalendar?.externalId;
const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];
const calendarId = externalCalendarId || mainHostDestinationCalendar?.externalId;
if (!calendarId) {
this.log.error("no calendar id provided in deleteEvent");
throw new Error("no calendar id provided in deleteEvent");
Expand Down
5 changes: 3 additions & 2 deletions packages/app-store/office365calendar/lib/CalendarService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ export default class Office365CalendarService implements Calendar {
}

async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> {
const [mainHostDestinationCalendar] = event.destinationCalendar ?? [];
try {
const eventsUrl = event.destinationCalendar?.externalId
? `/me/calendars/${event.destinationCalendar?.externalId}/events`
const eventsUrl = mainHostDestinationCalendar?.externalId
? `/me/calendars/${mainHostDestinationCalendar?.externalId}/events`
: "/me/calendar/events";

const response = await this.fetcher(eventsUrl, {
Expand Down
43 changes: 24 additions & 19 deletions packages/core/CalendarManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ export const getBusyCalendarTimes = async (

export const createEvent = async (
credential: CredentialWithAppName,
calEvent: CalendarEvent
calEvent: CalendarEvent,
externalId?: string
): Promise<EventResult<NewCalendarEventType>> => {
const uid: string = getUid(calEvent);
const calendar = await getCalendar(credential);
Expand All @@ -226,29 +227,31 @@ export const createEvent = async (

// Check if the disabledNotes flag is set to true
if (calEvent.hideCalendarNotes) {
calEvent.additionalNotes = "Notes have been hidden by the organiser"; // TODO: i18n this string?
calEvent.additionalNotes = "Notes have been hidden by the organizer"; // TODO: i18n this string?
}

// TODO: Surface success/error messages coming from apps to improve end user visibility
const creationResult = calendar
? await calendar.createEvent(calEvent).catch(async (error: { code: number; calError: string }) => {
success = false;
/**
* There is a time when selectedCalendar externalId doesn't match witch certain credential
* so google returns 404.
* */
if (error?.code === 404) {
? await calendar
.createEvent(calEvent, credential.id)
.catch(async (error: { code: number; calError: string }) => {
success = false;
/**
* There is a time when selectedCalendar externalId doesn't match witch certain credential
* so google returns 404.
* */
if (error?.code === 404) {
return undefined;
}
if (error?.calError) {
calError = error.calError;
}
log.error("createEvent failed", JSON.stringify(error), calEvent);
// @TODO: This code will be off till we can investigate an error with it
//https://github.com/calcom/cal.com/issues/3949
// await sendBrokenIntegrationEmail(calEvent, "calendar");
return undefined;
}
if (error?.calError) {
calError = error.calError;
}
log.error("createEvent failed", JSON.stringify(error), calEvent);
// @TODO: This code will be off till we can investigate an error with it
//https://github.com/calcom/cal.com/issues/3949
// await sendBrokenIntegrationEmail(calEvent, "calendar");
return undefined;
})
})
: undefined;

return {
Expand All @@ -261,6 +264,8 @@ export const createEvent = async (
originalEvent: calEvent,
calError,
calWarnings: creationResult?.additionalInfo?.calWarnings || [],
externalId,
credentialId: credential.id,
};
};

Expand Down