Compare commits

...

3 Commits

Author SHA1 Message Date
Hariom Balhara 78289bb6ca Add test 2023-10-05 11:00:13 +05:30
Hariom Balhara e07b681a04 Fix booking failure when we dont find credential for the destination calendar 2023-10-05 09:51:15 +05:30
Hariom Balhara 2e6afb3f1f Fix booking failure when we dont find credential for the destination calendar 2023-10-05 09:31:13 +05:30
3 changed files with 184 additions and 22 deletions

View File

@ -25,7 +25,7 @@ declare global {
noIcs?: true;
ics?: {
filename: string;
iCalUID: string;
iCalUID?: string;
};
},
to: string
@ -41,9 +41,9 @@ expect.extend({
//TODO: Support email HTML parsing to target specific elements
htmlToContain?: string;
to: string;
ics: {
ics?: {
filename: string;
iCalUID: string;
iCalUID?: string;
};
noIcs: true;
},
@ -66,9 +66,10 @@ expect.extend({
let isHtmlContained = true;
let isToAddressExpected = true;
const isIcsFilenameExpected = expectedEmail.ics ? ics?.filename === expectedEmail.ics.filename : true;
const isIcsUIDExpected = expectedEmail.ics
? !!(icsObject ? icsObject[expectedEmail.ics.iCalUID] : null)
: true;
const isIcsUIDExpected =
expectedEmail.ics?.iCalUID !== undefined
? !!(icsObject ? icsObject[expectedEmail.ics.iCalUID] : null)
: true;
if (expectedEmail.htmlToContain) {
isHtmlContained = testEmail.html.includes(expectedEmail.htmlToContain);
@ -95,12 +96,12 @@ expect.extend({
}
if (!isIcsFilenameExpected) {
return `ICS Filename is not as expected. Expected:${expectedEmail.ics.filename} isn't equal to ${ics?.filename}`;
return `ICS Filename is not as expected. Expected:${expectedEmail.ics?.filename} isn't equal to ${ics?.filename}`;
}
if (!isIcsUIDExpected) {
return `ICS UID is not as expected. Expected:${
expectedEmail.ics.iCalUID
expectedEmail.ics?.iCalUID
} isn't present in ${JSON.stringify(icsObject)}`;
}
throw new Error("Unknown error");
@ -187,7 +188,7 @@ export function expectSuccessfulBookingCreationEmails({
emails: Fixtures["emails"];
organizer: { email: string; name: string };
booker: { email: string; name: string };
iCalUID: string;
iCalUID?: string;
}) {
expect(emails).toHaveEmail(
{

View File

@ -344,6 +344,9 @@ export default class EventManager {
* @private
*/
private async createAllCalendarEvents(event: CalendarEvent) {
const loggerWithEventDetails = log.getChildLogger({
prefix: [`eventTypeId:${event.eventTypeId ?? "unknown"}`],
});
let createdEvents: EventResult<NewCalendarEventType>[] = [];
if (event.destinationCalendar && event.destinationCalendar.length > 0) {
// Since GCal pushes events to multiple calendars we only want to create one event per booking
@ -365,8 +368,9 @@ export default class EventManager {
[] as DestinationCalendar[]
);
for (const destination of destinationCalendars) {
log.silly("Creating Calendar event", JSON.stringify({ destination }));
loggerWithEventDetails.debug("Creating Calendar event", safeStringify({ destination }));
if (destination.credentialId) {
loggerWithEventDetails.silly("destination has credentialId");
let credential = this.calendarCredentials.find((c) => c.id === destination.credentialId);
if (!credential) {
// Fetch credential from DB
@ -400,20 +404,32 @@ export default class EventManager {
(c) => c.type === destination.integration
);
// It might not be the first connected calendar as it seems that the order is not guaranteed to be ascending of credentialId.
const firstCalendarCredential = destinationCalendarCredentials[0];
log.warn(
"No credentialId found for destination calendar, falling back to first found calendar",
safeStringify({
destination: getPiiFreeDestinationCalendar(destination),
firstConnectedCalendar: getPiiFreeCredential(firstCalendarCredential),
})
);
const firstCalendarCredential: (typeof destinationCalendarCredentials)[number] | undefined =
destinationCalendarCredentials[0];
// It is seen in the production that there are `destinationCalendars` with no calendar credential at all.
if (firstCalendarCredential) {
loggerWithEventDetails.warn(
`No credentialId found for destination calendar, falling back to first found credential matching destination.integration=${destination.integration}`,
safeStringify({
destination: getPiiFreeDestinationCalendar(destination),
firstConnectedCalendar: getPiiFreeCredential(firstCalendarCredential),
})
);
createdEvents.push(await createEvent(firstCalendarCredential, event));
createdEvents.push(await createEvent(firstCalendarCredential, event));
} else {
loggerWithEventDetails.error(
`No credential found matching destination.integration=${destination.integration} - So, we won't be able to create the event in the destination calendar`,
safeStringify({
destination: getPiiFreeDestinationCalendar(destination),
allCalendarCredentials: this.calendarCredentials.map(getPiiFreeCredential),
})
);
}
}
}
} else {
log.warn(
loggerWithEventDetails.warn(
"No destination Calendar found, falling back to first connected calendar",
safeStringify({
calendarCredentials: this.calendarCredentials,
@ -427,7 +443,7 @@ export default class EventManager {
const [credential] = this.calendarCredentials.filter((cred) => !cred.type.endsWith("other_calendar"));
if (credential) {
const createdEvent = await createEvent(credential, event);
log.silly("Created Calendar event", safeStringify({ createdEvent }));
loggerWithEventDetails.silly("Created Calendar event", safeStringify({ createdEvent }));
if (createdEvent) {
createdEvents.push(createdEvent);
}

View File

@ -520,7 +520,7 @@ describe("handleNewBooking", () => {
);
test(
`an error in creating a calendar event should not stop the booking creation - Current behaviour is wrong as the booking is created but no-one is notified of it`,
`an error in createMeeting should not stop the booking creation - Current behaviour is wrong as the booking is created but no-one is notified of it`,
async ({}) => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const booker = getBooker({
@ -629,6 +629,151 @@ describe("handleNewBooking", () => {
},
timeout
);
test("In the unexpected scenario where there is a destinationCalendar but no credentials, booking should be created but calendar will be skipped", async ({
emails,
}) => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});
const organizer = getOrganizer({
name: "Organizer",
email: "organizer@example.com",
id: 101,
schedules: [TestData.schedules.IstWorkHours],
// No credentials machig google_calendar
credentials: [],
selectedCalendars: [TestData.selectedCalendars.google],
destinationCalendar: {
integration: "google_calendar",
externalId: "organizer@google-calendar.com",
},
});
await createBookingScenario(
getScenarioData({
webhooks: [
{
userId: organizer.id,
eventTriggers: ["BOOKING_CREATED"],
subscriberUrl: "http://my-webhook.example.com",
active: true,
eventTypeId: 1,
appId: null,
},
],
eventTypes: [
{
id: 1,
slotInterval: 45,
length: 45,
users: [
{
id: 101,
},
],
},
],
organizer,
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
})
);
mockSuccessfulVideoMeetingCreation({
metadataLookupKey: "dailyvideo",
videoMeetingData: {
id: "MOCK_ID",
password: "MOCK_PASS",
url: `http://mock-dailyvideo.example.com/meeting-1`,
},
});
const _calendarMock = mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
uid: "MOCK_ID",
id: "GOOGLE_CALENDAR_EVENT_ID",
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});
const mockBookingData = getMockRequestDataForBooking({
data: {
eventTypeId: 1,
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: "integrations:daily" },
},
},
});
const { req } = createMockNextJsRequest({
method: "POST",
body: mockBookingData,
});
const createdBooking = await handleNewBooking(req);
expect(createdBooking.responses).toContain({
email: booker.email,
name: booker.name,
});
expect(createdBooking).toContain({
location: "integrations:daily",
});
await expectBookingToBeInDatabase({
description: "",
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
uid: createdBooking.uid!,
eventTypeId: mockBookingData.eventTypeId,
status: BookingStatus.ACCEPTED,
references: [
{
type: "daily_video",
uid: "MOCK_ID",
meetingId: "MOCK_ID",
meetingPassword: "MOCK_PASS",
meetingUrl: "http://mock-dailyvideo.example.com/meeting-1",
},
// No reference would be created for Google Calendar
// {
// type: "google_calendar",
// uid: "GOOGLE_CALENDAR_EVENT_ID",
// meetingId: "GOOGLE_CALENDAR_EVENT_ID",
// meetingPassword: "MOCK_PASSWORD",
// meetingUrl: "https://UNUSED_URL",
// },
],
});
expectWorkflowToBeTriggered();
// Calendar event won't be created - Maybe convert the following to a negative assertion
// expectSuccessfulCalendarEventCreationInCalendar(calendarMock, {
// calendarId: "organizer@google-calendar.com",
// videoCallUrl: "http://mock-dailyvideo.example.com/meeting-1",
// });
expectSuccessfulBookingCreationEmails({
booker,
organizer,
emails,
// Because calendar can't be contacted in this scenario, we don't get UID from Google Calendar and some random UID is generated by Cal.com
// So, we aren't verifying that for now.
// iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
});
expectBookingCreatedWebhookToHaveBeenFired({
booker,
organizer,
location: "integrations:daily",
subscriberUrl: "http://my-webhook.example.com",
videoCallUrl: `${WEBAPP_URL}/video/DYNAMIC_UID`,
});
});
});
describe("Video Meeting Creation", () => {