fix: handle collective multiple host on destinationCalendar #10967

alannnc · 2023-08-25T21:08:08Z

What does this PR do?

Converts destinationCalendar on CalendarEvent to be an array.
Patches and fixes so every sentence of code that was using evet.destinationCalendar can support the new type of array.
The logic here is to fallback to normal behaviour has having one main destinationCalendar but also supports multiple so host can get invited correctly.

Loom

Creating event and rescheduling:
https://www.loom.com/share/3cb3af513d274ed2bdfed44e48edddf6?sid=d81ce3de-2893-4a30-b085-ff67c31b7dfb

Canceling event:
https://www.loom.com/share/7ef505d4665e4d4982d2bb2d7b118085?sid=cd43c97c-aa86-4667-979f-f6492adf3365

Requirement/Documentation

Type of change

Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Are there environment variables that should be set? No
What are the minimal test data to have? Setup a collective event using teampro and teamfree users. Add any app calendar to those accounts and add them as host for collective event.
What is expected (happy path) to have (input and output)? Collective event with 2 host and every user with any calendar app installed should receive proper calendar events.
Any other important info that could help to test that PR

Mandatory Tasks

Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

I haven't added tests that prove my fix is effective or that my feature works

vercel · 2023-08-25T21:08:15Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
api	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 31, 2023 2:48am
cal-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 31, 2023 2:48am
dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 31, 2023 2:48am
ui	❌ Failed (Inspect)			Aug 31, 2023 2:48am

2 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		Aug 31, 2023 2:48am
qa	⬜️ Ignored (Inspect)	Visit Preview		Aug 31, 2023 2:48am

github-actions · 2023-08-25T21:08:23Z

Thank you for following the naming conventions! 🙏

linear · 2023-08-25T21:12:06Z

CAL-1267 Collective Events have only one team attendee on the google event

Looks like when booking a collective event, in the google calendar event it only displays the current user.

alannnc · 2023-08-25T21:13:34Z

packages/core/CalendarManager.ts

@@ -217,7 +217,8 @@ export const getBusyCalendarTimes = async (

 export const createEvent = async (
  credential: CredentialWithAppName,
-  calEvent: CalendarEvent
+  calEvent: CalendarEvent,
+  externalId?: string


I had a hard time figuring out how to pass externalId here, so I opted for this approach as it worn't interfere with createEvent calls that don't send this new parameter.

Basically here we have 3 types of data eventCalendar, resultEvent and credential and I find that if I added the anchor value on resultEvent it wont have that many side effects in the entire process so it can be passed properly without trouble.

alannnc · 2023-08-25T21:14:09Z

packages/core/CalendarManager.ts

@@ -226,7 +227,7 @@ 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?


alannnc · 2023-08-25T21:14:17Z

packages/core/CalendarManager.ts

@@ -261,6 +262,7 @@ export const createEvent = async (
    originalEvent: calEvent,
    calError,
    calWarnings: creationResult?.additionalInfo?.calWarnings || [],
+    externalId,


sending it back

alannnc · 2023-08-25T21:16:13Z

packages/core/EventManager.ts

-    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") {


supporting only 1 item inside event.destinationCalendar.

alannnc · 2023-08-25T21:16:55Z

packages/core/EventManager.ts

-        externalCalendarId: isCalendarType ? evt.destinationCalendar?.externalId : undefined,
-        credentialId: isCalendarType ? evt.destinationCalendar?.credentialId : result.credentialId,
+        externalCalendarId: isCalendarType ? result.externalId : undefined,
+        credentialId: isCalendarType ? result.credentialId : undefined,


With the fix on the first file we should have an explicit externalId and credentialId and with this we can stop the guessing game and just grab it from result.externalId.

Causes regression re video refs that need correct credentialId. Think you inadverntently undid your own fix #9281

alannnc · 2023-08-25T21:17:19Z

packages/core/EventManager.ts

-        externalCalendarId: evt.destinationCalendar?.externalId,
-        credentialId: result.credentialId ?? evt.destinationCalendar?.credentialId,
+        externalCalendarId: result.externalId,
+        credentialId: result.credentialId ?? undefined,


Same as above.

alannnc · 2023-08-25T21:18:34Z

packages/core/EventManager.ts

-          const createdEvent = await createEvent(credential, event);
-          if (createdEvent) {
-            createdEvents.push(createdEvent);
+    if (event.destinationCalendar && event.destinationCalendar.length > 0) {


Supporting array for event.destinationCalendar. Right now when fetching appCredentials we only use organizer one, now we can fetch any required calendar for every host.

alannnc · 2023-08-25T21:18:47Z