-
Notifications
You must be signed in to change notification settings - Fork 0
fix: handle collective multiple host on destinationCalendar #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: enhance-collective-scheduling-foundation
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
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.
ℹ️ 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".
| // @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") { | ||
| evt["location"] = "integrations:daily"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard Meet fallback when no destination calendar
The new Meet fallback code now destructures const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [] and immediately accesses mainHostDestinationCalendar.integration. When a host has no destination calendar configured (which is common whenever Cal Video should be used as the fallback), mainHostDestinationCalendar is undefined and this line throws before the fallback runs. The previous code used optional chaining on evt.destinationCalendar?.integration and did not crash. Please null‑check the array element (e.g. mainHostDestinationCalendar?.integration) before reading integration so bookings without a destination calendar do not fail with a runtime TypeError.
Useful? React with 👍 / 👎.
| const selectedCalendar = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.externalId; | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google update/delete lose calendarId when not provided
Both updateEvent and deleteEvent now compute selectedCalendar/calendarId as externalCalendarId ? externalCalendarId : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId. When externalCalendarId is not passed (the common case when we just want to update/delete the organizer’s default calendar), the predicate compares each calendar’s externalId to undefined, so the fallback always returns undefined. The subsequent Google API calls therefore omit the required calendarId parameter and Google rejects the request, breaking reschedule/cancel flows for any booking whose reference didn’t store an explicit calendar id. The fallback should instead pick a real destination (e.g. the organizer’s calendar) rather than searching for externalCalendarId again.
Useful? React with 👍 / 👎.
| organization: { | ||
| create: { | ||
| name, | ||
| ...(!IS_TEAM_BILLING_ENABLED && { slug }), | ||
| ...(IS_TEAM_BILLING_ENABLED ? { slug } : {}), | ||
| metadata: { | ||
| ...(IS_TEAM_BILLING_ENABLED && { requestedSlug: slug }), | ||
| ...(IS_TEAM_BILLING_ENABLED ? { requestedSlug: slug } : {}), | ||
| isOrganization: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Organization slug assignment inverted
The organization creation logic now spreads ...(IS_TEAM_BILLING_ENABLED ? { slug } : {}), whereas the previous code only set slug when team billing was disabled. This reverses the intended behaviour: when team billing is disabled we now fail to assign any slug to the newly created organization, leaving it without a subdomain, and when team billing is enabled we prematurely claim the slug instead of storing it in requestedSlug for later approval. Please restore the original condition (set slug when team billing is not enabled, store requestedSlug when it is) so organizations receive the correct slug in both modes.
Useful? React with 👍 / 👎.
Test 4
Replicated from ai-code-review-evaluation/cal.com-cursor#4