Skip to content

fix: handle collective multiple host on destinationCalendar#3

Draft
ShashankFC wants to merge 1 commit into
enhance-collective-scheduling-foundationfrom
fix/handle-collective-multiple-host-destinations
Draft

fix: handle collective multiple host on destinationCalendar#3
ShashankFC wants to merge 1 commit into
enhance-collective-scheduling-foundationfrom
fix/handle-collective-multiple-host-destinations

Conversation

@ShashankFC

Copy link
Copy Markdown
Collaborator

Test 4nn---nReplicated from ai-code-review-evaluation/cal.com-cursor#4

@GitHoobar

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd797c1ea0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +118 to +119
const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Guard against empty destination calendars in Meet fallback

When evt.destinationCalendar is null or [] (e.g., several callers now pass []), destructuring yields undefined and mainHostDestinationCalendar.integration throws, so MeetLocationType bookings without a destination calendar crash before event creation. This used to be protected by optional chaining on destinationCalendar, so it’s a regression for events that don’t specify a destination calendar. Consider using mainHostDestinationCalendar?.integration or guarding for empty arrays.

Useful? React with 👍 / 👎.

Comment on lines 254 to +256
const selectedCalendar = externalCalendarId
? externalCalendarId
: event.destinationCalendar?.externalId;
: event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid undefined calendarId on Google updateEvent fallback

If externalCalendarId is missing (which can happen for older booking references), the fallback find(... === externalCalendarId) compares against undefined, so selectedCalendar becomes undefined and calendar.events.update is called with calendarId: undefined. Google rejects that request, so updates can fail for those bookings. Previously this defaulted to the destination calendar’s externalId; you likely need a real fallback (destination calendar or primary).

Useful? React with 👍 / 👎.

Comment on lines +147 to +149
const selectedCalendar = calEventRaw.destinationCalendar?.find(
(cal) => cal.credentialId === credentialId
)?.externalId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve legacy destinationCalendar externalId selection

Calendar selection now only matches by credentialId. For legacy DestinationCalendar rows that have externalId but no credentialId, selectedCalendar becomes undefined and inserts into "primary". In the no-credentialId branch of createAllCalendarEvents, that means events are created on every credential’s primary instead of the intended externalId, producing duplicates and wrong placement. Consider falling back to the destination calendar’s externalId when no credentialId is present.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants