fix: MSTeams not created as online meetings #21377
Conversation
|
@vijayraghav-io is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/17/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (05/17/25)1 label was added to this PR based on Keith Williams's automation. |
| @@ -6,7 +6,7 @@ import { WEBAPP_URL_FOR_OAUTH } from "@calcom/lib/constants"; | |||
| import getAppKeysFromSlug from "../../_utils/getAppKeysFromSlug"; | |||
| import { encodeOAuthState } from "../../_utils/oauth/encodeOAuthState"; | |||
|
|
|||
| export const OFFICE365_VIDEO_SCOPES = ["OnlineMeetings.ReadWrite", "offline_access"]; | |||
| export const OFFICE365_VIDEO_SCOPES = ["OnlineMeetings.ReadWrite", "offline_access", "Calendars.ReadWrite"]; | |||
There was a problem hiding this comment.
This will break all active users who have already installed MS Teams. Changing a scope directly is high-risk
Please note, this is not work around. |
There was a problem hiding this comment.
Let's approach this in a way that doesn't affect active users. There are many users who have already installed and are actively using it. It doesn’t make sense to break it for them and ask them to reinstall. If users are happy with the current setup, we shouldn’t force them to go through that process. So let's find a fix that preserves the existing experience
…rService instead of VideoApiAdapter
…o/cal.com into fix_msTeams_events
|
To update the progress:
Tested the working with an work account in MSTeams and outlook calendar. Working as expected and as shown in after fix videos. (booking, rescheduling, cancel) Will update more notes , by testing different scenarios for existing users and existing bookings. For new users - We may have to enforce that MSTeams can be installed only if outlook calendar is installed, similar to how we do for google meet. |
|
@vijayraghav-io is your pr fix this? Because the linked issue started from this and aims to fix the double email sending issue? |
@anikdhabal 
So, this PR resolves all other issues mentioned in this issue description #14765, like
Please refer this PR description for more details. |
|
@vijayraghav-io The improvement looks good and should work fine for new installations. But how is backward compatibility being handled? I’ve seen that MS Calendar and MS Teams video dependent. Please make sure it doesn’t cause any issues for active MS Teams users. It should seamlessly support full CRUD operations for existing users without any problems |
@anikdhabal Thank you! 🙏. |
@anikdhabal , verified this again. Scenario - For Users who have only MS Teams installed and no Outlook(or MS) Calendar, after this PR is released : Scenario - For Users who have both MS Teams and Outlook(or MS) Calendar installed, after this PR is released : |
WalkthroughThe changes introduce enhanced support for Microsoft Teams (MS Teams) and Office 365 Calendar integration. Key updates include improved handling of online meetings in Office365CalendarService, ensuring that online meeting details and rich HTML descriptions are preserved and correctly formatted. The EventManager now supports fallback logic and updates video call data for MS Teams meetings. New constants and dependency declarations are added for MS Teams integration. Additionally, translation and configuration files are updated to reflect these changes, and a new function for generating rich HTML event descriptions is introduced. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
packages/lib/EventManager.ts (2)
289-321: Consider refactoring duplicate fallback logic.The fallback logic for MSTeams (lines 304-321) is nearly identical to Google Meet (lines 289-303). Consider extracting this into a reusable function to reduce duplication and improve maintainability.
+private shouldFallbackToCalVideo( + locationType: string, + expectedIntegration: string, + mainHostDestinationCalendar: typeof evt.destinationCalendar[0] | undefined, + calendarCredentials: CredentialForCalendarService[] +): boolean { + if (mainHostDestinationCalendar?.integration === expectedIntegration) { + return false; + } + + const [credential] = calendarCredentials.filter( + (cred) => cred.type === expectedIntegration + ); + + if (!isDelegationCredential({ credentialId: credential?.id })) { + log.warn( + `Falling back to Cal Video integration for Regular Credential as ${expectedIntegration} is not set as destination calendar` + ); + return true; + } + + return false; +} // Then use it: -if (evt.location === MeetLocationType && mainHostDestinationCalendar?.integration !== "google_calendar") { - const [googleCalendarCredential] = this.calendarCredentials.filter( - (cred) => cred.type === "google_calendar" - ); - if (!isDelegationCredential({ credentialId: googleCalendarCredential?.id })) { - log.warn( - "Falling back to Cal Video integration for Regular Credential as Google Calendar is not set as destination calendar" - ); - evt["location"] = "integrations:daily"; - evt["conferenceCredentialId"] = undefined; - } -} +if (evt.location === MeetLocationType && + this.shouldFallbackToCalVideo(evt.location, "google_calendar", mainHostDestinationCalendar, this.calendarCredentials)) { + evt["location"] = "integrations:daily"; + evt["conferenceCredentialId"] = undefined; +} // Similar for MSTeams
354-377: LGTM! Correctly extracts Teams meeting URL from calendar event.This implementation properly handles MSTeams by extracting the meeting URL from the Office365 calendar event instead of creating a separate video meeting. The logic correctly updates both
videoCallDataandresponseslocation data.packages/app-store/office365calendar/lib/CalendarService.ts (4)
258-264: LGTM! Correctly extracts Teams meeting URL.The implementation properly handles the optional
onlineMeeting.joinUrlfrom the Microsoft Graph API response and assigns it to the event URL.
276-285: Consider caching to reduce API calls for MSTeams updates.While fetching the existing event is necessary to preserve the meeting blob, this adds an extra Graph API call for every MSTeams update operation. Consider implementing a caching mechanism or batching strategy to reduce API calls, especially for bulk operations.
427-444: Well-structured online meeting detection and content generation.The implementation correctly handles different scenarios for online meetings, preserving existing content for rescheduled meetings and generating rich HTML descriptions. The content type is appropriately set based on meeting type.
500-510: LGTM! Comprehensive online meeting configuration.The implementation correctly sets all required properties for Teams online meetings and handles backward compatibility for meetings that weren't originally online meetings.
🧹 Nitpick comments (2)
packages/lib/CalEventParser.ts (1)
317-367: Consider HTML sanitization for security.The function generates HTML content without sanitization. While the input data is typically trusted (coming from
CalendarEventproperties), consider adding HTML sanitization as a defensive measure, especially for user-provided content like descriptions and additional notes.Consider using a library like
DOMPurifyor similar for HTML sanitization, particularly for user-provided fields:import DOMPurify from 'dompurify'; // In textToHtml helper: return `<p>${DOMPurify.sanitize(line)}</p>`;packages/app-store/office365calendar/lib/CalendarService.ts (1)
436-438: Improve HTML concatenation for better formatting.The current string concatenation might result in inconsistent spacing. Consider using a more robust approach.
- content = ` - ${getRichDescriptionHTML(event)}<hr> - ${existingBody}`.trim(); + content = [getRichDescriptionHTML(event), '<hr>', existingBody] + .filter(Boolean) + .join('\n') + .trim();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/app-store/components.tsx(2 hunks)packages/app-store/locations.ts(1 hunks)packages/app-store/office365calendar/lib/CalendarService.ts(5 hunks)packages/app-store/office365video/_metadata.ts(1 hunks)packages/app-store/office365video/config.json(1 hunks)packages/lib/CalEventParser.ts(1 hunks)packages/lib/EventManager.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/locations.tspackages/app-store/office365video/_metadata.tspackages/lib/CalEventParser.tspackages/lib/EventManager.tspackages/app-store/office365calendar/lib/CalendarService.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/locations.tspackages/app-store/office365video/_metadata.tspackages/app-store/components.tsxpackages/lib/CalEventParser.tspackages/lib/EventManager.tspackages/app-store/office365calendar/lib/CalendarService.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/app-store/components.tsx
**/*Service.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/office365calendar/lib/CalendarService.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Applied to files:
packages/app-store/office365video/_metadata.tspackages/app-store/office365video/config.jsonpackages/lib/EventManager.tspackages/app-store/office365calendar/lib/CalendarService.ts
📚 Learning: cal.com integrations: `webapp_url_for_oauth` intentionally falls back to `http://localhost:3000` whe...
Learnt from: hariombalhara
PR: calcom/cal.com#22840
File: packages/app-store/webex/api/add.ts:4-4
Timestamp: 2025-08-04T13:14:39.218Z
Learning: Cal.com integrations: `WEBAPP_URL_FOR_OAUTH` intentionally falls back to `http://localhost:3000` when neither production nor development, matching the pattern used by other apps in the repo.
Applied to files:
packages/app-store/office365video/_metadata.ts
📚 Learning: applies to **/*.tsx : always use `t()` for text localization in frontend code; direct text embedding...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
Applied to files:
packages/app-store/components.tsx
📚 Learning: when making localization changes for new features, it's often safer to add new strings rather than m...
Learnt from: bandhan-majumder
PR: calcom/cal.com#22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.233Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
Applied to files:
packages/app-store/components.tsx
📚 Learning: in cal.com's getusereventgroups handler refactor (pr #22618), the membershipcount field for team eve...
Learnt from: sean-brydon
PR: calcom/cal.com#22618
File: packages/trpc/server/routers/viewer/eventTypes/utils/transformUtils.ts:113-113
Timestamp: 2025-08-05T07:42:06.316Z
Learning: In Cal.com's getUserEventGroups handler refactor (PR #22618), the membershipCount field for team event groups is intentionally set to 0 in the new createTeamEventGroup function, as confirmed by sean-brydon (PR author). This preserves the same behavior as the old implementation that was being refactored, maintaining backward compatibility. While other parts of the codebase may use actual member counts, this specific implementation maintains the previous behavior.
Applied to files:
packages/lib/EventManager.ts
📚 Learning: the outlook calendar integration in cal.com intentionally reuses subscription ids across multiple ev...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The `upsertSelectedCalendarsForEventTypeIds` method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be applied as they would prevent this intended functionality.
Applied to files:
packages/lib/EventManager.ts
📚 Learning: in cal.com's calendar integration, both google calendar and outlook calendar are designed to allow m...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
Applied to files:
packages/lib/EventManager.ts
📚 Learning: in cal.com's embed system, internal events like "__scrollbydistance" are fired by cal.com's own code...
Learnt from: hariombalhara
PR: calcom/cal.com#22547
File: packages/embeds/embed-core/src/lib/eventHandlers/scrollByDistanceEventHandler.ts:11-14
Timestamp: 2025-07-16T11:46:28.759Z
Learning: In Cal.com's embed system, internal events like "__scrollByDistance" are fired by Cal.com's own code, so runtime validation of event data structure is unnecessary since TypeScript type system guarantees type safety for internal events.
Applied to files:
packages/app-store/office365calendar/lib/CalendarService.ts
📚 Learning: applies to **/*.{ts,tsx} : flag excessive day.js use in performance-critical code; prefer native dat...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
packages/app-store/office365calendar/lib/CalendarService.ts
🧬 Code Graph Analysis (2)
packages/lib/EventManager.ts (2)
packages/app-store/locations.ts (2)
MeetLocationType(62-62)MSTeamsLocationType(64-64)packages/lib/delegationCredential/clientAndServer.ts (1)
isDelegationCredential(11-11)
packages/app-store/office365calendar/lib/CalendarService.ts (2)
packages/app-store/locations.ts (1)
MSTeamsLocationType(64-64)packages/lib/CalEventParser.ts (1)
getRichDescriptionHTML(317-367)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
packages/app-store/locations.ts (1)
64-64: LGTM! Clean addition following established patterns.The new
MSTeamsLocationTypeconstant follows the same naming convention and format as existing location type constants likeDailyLocationTypeandMeetLocationType. The value"integrations:office365_video"correctly matches the integration identifier.packages/app-store/office365video/config.json (1)
25-25: LGTM! Logical dependency declaration.Adding the
office365-calendardependency is appropriate given that MS Teams meetings now rely on the calendar service to create proper online meetings. This ensures users have the required calendar integration installed before enabling MS Teams functionality.packages/app-store/office365video/_metadata.ts (1)
24-24: LGTM! Consistent dependency declaration.The dependency declaration matches the one added to
config.json, ensuring consistency between the app's metadata and configuration. This alignment is important for proper dependency resolution during app installation.packages/app-store/components.tsx (1)
100-100: LGTM! Proper i18next interpolation configuration.Adding
interpolation: { escapeValue: false }follows i18next best practices and addresses the previous review feedback. This ensures that HTML formatting in dependency-related messages is rendered correctly without being escaped.Also applies to: 119-119
packages/lib/EventManager.ts (2)
10-10: LGTM!The import correctly adds
MSTeamsLocationTypealongside the existingMeetLocationType, following the established pattern.
47-49: LGTM!Correctly excludes MSTeams from dedicated integrations, treating it similarly to Google Meet since both are tied to their respective calendar services.
packages/app-store/office365calendar/lib/CalendarService.ts (1)
4-6: LGTM!The imports are correctly added for MSTeams location type checking and rich HTML description generation.
E2E results are ready! |
|
Thank you! 🙏 |
What does this PR do?
/claim [Bug] MS Teams + MS Calendar Bad Implementation #14765
Visual Demo (For contributors especially)
Video Demo:
Before fix (or issue):
https://www.loom.com/share/87082087c5f64e368494ec6aea834a48?sid=4c3724fa-5c6a-4a57-8b25-d32e6989404d
After fix :
https://www.loom.com/share/75f976cf3b6f4b27b5da3301089fb2fd?sid=fe926a95-9953-4797-a7f0-bc6524f35c83
Changes in /apps/msteams (MSTeams installation page with dependency added)
https://www.loom.com/share/2881ab3bf573453585a029016c82b3ba?sid=478b4496-9112-43b0-a3db-f6050a2baaf3
Image Demo :
The root cause for the issue is -
/onlineMeetingsendpoint was used to create MSTeam meetings in office365video/lib/VideoApiAdapter.ts. But as per microsoft Graph API doc -https://learn.microsoft.com/en-us/graph/api/application-post-onlinemeetings?view=graph-rest-1.0&tabs=javascript
https://learn.microsoft.com/en-us/graph/api/user-post-events?view=graph-rest-1.0&tabs=javascript#example-4-create-and-enable-an-event-as-an-online-meeting
with key properties set
allowNewTimeProposals: true,isOnlineMeeting: true,onlineMeetingProvider: 'teamsForBusiness'Also duplicate events or emails were generated because a event was created on outlook calendar using calendar service, and
/onlineMeetingsalso created an event on same calendar by office365video/lib/VideoApiAdapter.Implemented a check in this PR -> to not create meeting using office365video/lib/videoApiAdapter, but the office365calendar/lib/CalendarService with /events endpoint will be used to create onlineMeeting by passing required parameters..
Also the body content should be of type
HTMLand html content has to be input in body for /event endpoint , for the texts and content to look good along withMicrosoft Teamsdetails at the end of the body with meeting details. This resolves content not looking good.MSTeams automatically send mails to attendees with the body passed in
/eventsendpoint. Also appending its meeting details like url, passcode, id,...Added a dependency to MSTeams installation in app-store, this suggests or warns user if they are trying to install MSTeams without outlook calendar and recommends to install outlook calendar first.
Also updated the
updateEventin office365calendar/lib/CalendarService.tsupdateEventupdates or moves the meeting without loosing itsonlineMeetingfeature by getting the meeting blob first in the existing body and then patching it. Without this blob patch the updated meeting becomes non-onlineMeeting (or without msTeams feature)https://learn.microsoft.com/en-us/graph/api/event-update?view=graph-rest-1.0&tabs=javascript
Mandatory Tasks (DO NOT REMOVE)
This functionality test requires verification on external app - MSTeams and can be verified visually.
How should this be tested?
Install Microsoft outlook calendar and MS teams with a work account for a organizer in cal
The events on MS teams should be created as online meetings.
The text should be formatted correctly inside this event on MS teams.
The buttons 'Join' , label 'Microsoft Teams Meeting' for location should be visible.
It should be marked as online meeting when event is opened to edit.
Should be able to reschedule and the rescheduled event should also be 'online meeting'.
Summary by mrge
Fixed an issue where MS Teams events were not created as online meetings and prevented duplicate calendar events in Outlook when using MS Teams.