Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web/pages/api/cron/bookingReminder.ts
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
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
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;
Comment on lines 254 to +256

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Logic bug: calendar lookup is ineffective.

When externalCalendarId is truthy, it's returned directly (correct). But when it's falsy, the .find() compares cal.externalId === externalCalendarId where externalCalendarId is falsy (e.g., null or undefined), which won't match any calendar with a valid externalId. This makes the fallback branch ineffective.

If the intent is to find a calendar by some other criteria when externalCalendarId is not provided, the logic needs to be revised. If the intent is to just use externalCalendarId when provided and otherwise leave it undefined, then the .find() clause is dead code.

🔎 Proposed fix (assuming you want undefined when not provided)
       const selectedCalendar = externalCalendarId
-        ? externalCalendarId
-        : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
+        ? externalCalendarId
+        : undefined;

Or if you need to resolve from destinationCalendar by credentialId (similar to createEvent), you'd need to pass credentialId to updateEvent as well.

📝 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.

Suggested change
const selectedCalendar = externalCalendarId
? externalCalendarId
: event.destinationCalendar?.externalId;
: event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
const selectedCalendar = externalCalendarId
? externalCalendarId
: undefined;
🤖 Prompt for AI Agents
In @packages/app-store/googlecalendar/lib/CalendarService.ts around lines 254 -
256, The current selection logic for selectedCalendar in updateEvent is wrong
because it does .find(cal => cal.externalId === externalCalendarId) when
externalCalendarId is falsy, which never matches; either simplify to just use
externalCalendarId (i.e., set selectedCalendar = externalCalendarId) if you
intend undefined when not provided, or implement the intended resolution by
credential like createEvent: change the expression to
event.destinationCalendar?.find(cal => cal.credentialId ===
credentialId)?.externalId, and update the updateEvent signature and all its
callers to accept and pass credentialId so the lookup can succeed.


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 +317

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same logic bug as in updateEvent.

The .find() clause when externalCalendarId is falsy will never match, making this dead code.

🔎 Proposed fix
       const calendarId = externalCalendarId
-        ? externalCalendarId
-        : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
+        ? externalCalendarId
+        : undefined;
📝 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.

Suggested change
const calendarId = externalCalendarId
? externalCalendarId
: event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
const calendarId = externalCalendarId
? externalCalendarId
: undefined;
🤖 Prompt for AI Agents
In @packages/app-store/googlecalendar/lib/CalendarService.ts around lines 315 -
317, The current assignment to calendarId uses
event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)
which will never match when externalCalendarId is falsy; change the fallback to
select a valid externalId from destinationCalendar instead (e.g., use
event.destinationCalendar?.[0]?.externalId or find by a sensible property like
isPrimary) so that calendarId = externalCalendarId ??
event.destinationCalendar?.[0]?.externalId; update the code referencing
calendarId to rely on this corrected fallback.


calendar.events.delete(
{
Expand Down
12 changes: 8 additions & 4 deletions packages/app-store/larkcalendar/lib/CalendarService.ts
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");
}
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
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
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
Loading