fix: handle collective multiple host on destinationCalendar #4
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements support for collective scheduling with multiple hosts by converting the destinationCalendar field from a single object to an array throughout the codebase. The core architectural change transforms DestinationCalendar to DestinationCalendar[] in the CalendarEvent interface, enabling events to be created across multiple calendars when multiple hosts are involved.
The changes span the entire calendar management system:
- Type System Updates: The
CalendarEventinterface now expectsdestinationCalendaras an array, and theCalendarinterface'screateEventmethod accepts acredentialIdparameter to identify which specific calendar to use - Calendar Services: Google Calendar, Office365, and Lark Calendar services were updated to extract the main host's calendar using array destructuring (
[mainHostDestinationCalendar] = event.destinationCalendar ?? []) - EventManager: Modified to iterate through multiple destination calendars during event creation/updates, with database credential fallback logic for accessing team member credentials
- Booking Handlers: All booking-related operations (create, cancel, confirm, reschedule) now wrap single destination calendars in arrays to maintain compatibility
- Email Templates: Updated to use the first destination calendar for display purposes in broken integration emails
The implementation follows a consistent pattern: existing single-calendar logic is preserved by using the first element of the array as the "main host" calendar, while the array structure enables future expansion to handle multiple calendars simultaneously. This maintains backward compatibility while enabling collective scheduling scenarios where each team member can have events created in their preferred calendar.
Confidence score: 2/5
- This PR has significant architectural changes that could cause runtime errors if the array structure isn't handled properly
- Multiple files contain logic that assumes non-empty arrays without proper validation, particularly in GoogleCalendarService's update/delete methods
- The EventManager contains duplicated database query logic and potential performance issues with queries in loops
- Several calendar services have incomplete error handling for empty destination calendar arrays
22 files reviewed, 6 comments
| let calendar = mainHostDestinationCalendar | ||
| ? mainHostDestinationCalendar?.integration.split("_") | ||
| : "calendar"; |
There was a problem hiding this comment.
style: The optional chaining on mainHostDestinationCalendar?.integration is redundant since you already check mainHostDestinationCalendar in the ternary condition.
| let calendar = mainHostDestinationCalendar | |
| ? mainHostDestinationCalendar?.integration.split("_") | |
| : "calendar"; | |
| let calendar = mainHostDestinationCalendar | |
| ? mainHostDestinationCalendar.integration.split("_") | |
| : "calendar"; |
| const selectedCalendar = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.externalId; | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
logic: Logic error: when externalCalendarId is provided, you're searching for a calendar where externalId === externalCalendarId, but this will always fail since you're looking for a calendar that matches itself. Should likely find by credentialId or use different logic.
| const calendarId = externalCalendarId ? externalCalendarId : event.destinationCalendar?.externalId; | ||
| const calendarId = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; |
There was a problem hiding this comment.
logic: Same logic error as in updateEvent: searching for calendar with externalId === externalCalendarId will not work as intended
| return calendars.reduce<IntegrationCalendar[]>((newCalendars, calendar) => { | ||
| if (!calendar.components?.includes("VEVENT")) return newCalendars; | ||
|
|
||
| const [mainHostDestinationCalendar] = event?.destinationCalendar ?? []; |
There was a problem hiding this comment.
style: Consider extracting this pattern into a helper function since it's repeated twice in the file
| 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") { |
There was a problem hiding this comment.
logic: Potential null reference if mainHostDestinationCalendar is undefined when array is empty
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") { |
| } | ||
| } | ||
| if (credential) { | ||
| const createdEvent = await createEvent(credential, event, destination.externalId); |
There was a problem hiding this comment.
logic: Third parameter destination.externalId is passed but the fallback calls without this parameter - inconsistent usage
|
Thank you for following the naming conventions! 🙏 |
|
This PR is being marked as stale due to inactivity. |
2 participants
Test 4