feat: outlook cache and change notifications webhook#21071
feat: outlook cache and change notifications webhook#21071ouwargui wants to merge 24 commits intocalcom:mainfrom
Conversation
|
@ouwargui is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
$500
calendar-apps
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/02/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (05/02/25)1 label was added to this PR based on Keith Williams's automation. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
mrge found 4 issues across 10 files. View them in mrge.io
| GOOGLE_WEBHOOK_URL= | ||
|
|
||
| # Token to verify incoming webhooks from Microsoft Outlook | ||
| OUTLOOK_WEBHOOK_TOKEN= |
There was a problem hiding this comment.
Missing guidance on token generation requirements
| const webhookBodyParseRes = changeNotificationWebhookPayloadSchema.safeParse(req.body); | ||
|
|
||
| if (!webhookBodyParseRes.success) { | ||
| log.error("postHandler", safeStringify(webhookBodyParseRes.error)); |
There was a problem hiding this comment.
Rule violated: Avoid Logging Sensitive Information
Logging validation errors may expose sensitive information
There was a problem hiding this comment.
I can remove this or change to something else if wanted
| const validationToken = getValidationToken(req); | ||
| if (validationToken) { | ||
| res.setHeader("Content-Type", "text/plain"); | ||
| return res.status(200).send(validationToken); | ||
| } |
There was a problem hiding this comment.
When you create a subscription, microsoft send a validation request to the webhook url with a validationToken on the search params, we have to return it in plain text.
More info here: https://learn.microsoft.com/en-us/graph/change-notifications-delivery-webhooks?tabs=http#notificationurl-validation
| const webhookBodyParseRes = changeNotificationWebhookPayloadSchema.safeParse(req.body); | ||
|
|
||
| if (!webhookBodyParseRes.success) { | ||
| log.error("postHandler", safeStringify(webhookBodyParseRes.error)); |
There was a problem hiding this comment.
I can remove this or change to something else if wanted
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | ||
| if (req.method === "POST") return postHandler(req, res); | ||
|
|
||
| res.setHeader("Allow", "POST"); | ||
| return res.status(405).json({}); | ||
| } |
There was a problem hiding this comment.
I couldn't use the defaultHandler and defaultResponder here because I need to be able to return text/plain when Microsoft sends a validation challenge
| const selectedCalendarIds = args.items.map((item) => item.id); | ||
|
|
||
| const ids = await (selectedCalendarIds.length === 0 | ||
| ? this.listCalendars().then((cals) => cals.map((e_2) => e_2.externalId).filter(Boolean) || []) | ||
| : Promise.resolve(selectedCalendarIds)); | ||
| const requestsPromises = ids.map(async (calendarId, id) => ({ | ||
| id, | ||
| method: "GET", | ||
| url: `${await this.getUserEndpoint()}/calendars/${calendarId}/calendarView${filter}&${calendarSelectParams}`, | ||
| })); | ||
| const requests = await Promise.all(requestsPromises); | ||
| const response = await this.apiGraphBatchCall(requests); | ||
| const responseBody = await this.handleErrorJsonOffice365Calendar(response); | ||
| let responseBatchApi: IBatchResponse = { responses: [] }; | ||
| if (typeof responseBody === "string") { | ||
| responseBatchApi = this.handleTextJsonResponseWithHtmlInBody(responseBody); | ||
| } | ||
| let alreadySuccessResponse = [] as ISettledResponse[]; | ||
|
|
||
| // Validate if any 429 status Retry-After is present | ||
| const retryAfter = | ||
| !!responseBatchApi?.responses && this.findRetryAfterResponse(responseBatchApi.responses); | ||
|
|
||
| if (retryAfter && responseBatchApi.responses) { | ||
| responseBatchApi = await this.fetchRequestWithRetryAfter(requests, responseBatchApi.responses, 2); | ||
| } | ||
|
|
||
| // Recursively fetch nextLink responses | ||
| alreadySuccessResponse = await this.fetchResponsesWithNextLink(responseBatchApi.responses); | ||
| return alreadySuccessResponse ? alreadySuccessResponse : []; |
There was a problem hiding this comment.
This is exactly the same code as before, just moved to a different function
| ); | ||
| } | ||
|
|
||
| async watchCalendar({ |
There was a problem hiding this comment.
This method is very similar to google calendar's as well, we should extract it
| ); | ||
| } | ||
|
|
||
| async unwatchCalendar({ |
There was a problem hiding this comment.
This method is very similar to google calendar's, we should extract it
| /** | ||
| * https://learn.microsoft.com/en-us/graph/api/subscription-post-subscriptions?view=graph-rest-1.0&tabs=http#response-1 | ||
| */ | ||
| export const startWatchingCalendarResponseSchema = z.object({ | ||
| id: z.string(), | ||
| expirationDateTime: z.string(), | ||
| }); |
There was a problem hiding this comment.
There's more fields, but I think they're not relevant for now
| */ | ||
| -- AlterTable | ||
| ALTER TABLE "SelectedCalendar" ADD COLUMN "outlookSubscriptionExpiration" TEXT, | ||
| ADD COLUMN "outlookSubscriptionId" TEXT; | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "SelectedCalendar_outlookSubscriptionId_eventTypeId_key" ON "SelectedCalendar"("outlookSubscriptionId", "eventTypeId"); |
There was a problem hiding this comment.
Added outlookSubscriptionExpiration and outlookSubscriptionId fields to the SelectedCalendar table. Also added outlookSubscriptionId as unique.
ouwargui
changed the title
ouwargui
force-pushed
the
feat/outlook-cache
branch
from
671597a to
6620546
Compare
| const events = webhookBodyParseRes.data.value; | ||
| const body: WebhookResponse = {}; | ||
|
|
||
| const uniqueEvents = uniqueBy(events, ["subscriptionId"]); | ||
|
|
||
| const promises = uniqueEvents.map(async (event) => { |
There was a problem hiding this comment.
Microsoft send a value array on the payload which may contain one or more events for different subscriptionId.
When many changes occur, Microsoft Graph may send multiple notifications that correspond to different subscriptions in the same POST request.
I'm removing duplicate subscriptionIds to avoid fetching availability multiple times for the same calendar.
I'm not sure if the way I'm handling each event and adding the result to a body object is the best, but let me know if you have a different idea.
ouwargui
force-pushed
the
feat/outlook-cache
branch
2 times, most recently
from
02e9837 to
3ac5fef
Compare
|
@hbjORbj hey, could you review my pr as well? i'm almost finished adding unit tests and i'd appreciate if i could get a review before also, i tried to record a video but my mac air couldn't handle the server running + recording - i'll see if i can do it later on my windows pc |
zomars
left a comment
zomars
left a comment
There was a problem hiding this comment.
Due to the complexity of these changes we require a recorded demo and proper integration tests in order for us to being able to approve and merge
ouwargui
force-pushed
the
feat/outlook-cache
branch
from
6eaddc9 to
4b60453
Compare
|
@zomars added some tests and a video, let me know if you need anything else - thanks :) |
ouwargui
force-pushed
the
feat/outlook-cache
branch
2 times, most recently
from
3e161ea to
391873a
Compare
… on getAvailability
ouwargui
force-pushed
the
feat/outlook-cache
branch
from
fec4642 to
27876cb
Compare
|
This PR is being marked as stale due to inactivity. |
|
This PR is being marked as stale due to inactivity. |
WalkthroughThis change set introduces comprehensive support for Outlook calendar webhook subscriptions and caching, similar to existing Google calendar functionality. Database schema and migration scripts add Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Complexity rationale: Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/api/v1/test/lib/selected-calendars/_post.test.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/office365calendar/api/webhook.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/office365calendar/api/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/app-store/office365calendar/api/webhook.ts (1)
44-44: Consider removing or sanitizing error loggingLogging validation errors may expose sensitive webhook payload information.
packages/app-store/office365calendar/lib/CalendarService.ts (1)
758-777: Consider making subscription expiration configurableThe hardcoded 10,000 minutes (~7 days) could be made configurable for flexibility.
🧹 Nitpick comments (6)
.env.example (1)
152-156: Good to see Outlook variables added
Including placeholders forOUTLOOK_WEBHOOK_TOKENandOUTLOOK_WEBHOOK_URLkeeps the sample env complete and avoids onboarding friction.It could be helpful to add an inline comment (like the Google section) mentioning that the token must match the value configured in Azure App Registration ↦ Subscriptions panel.
packages/features/calendar-cache/CalendarCache.md (2)
6-12: Fix list-item indentation & keep Markdown lint-cleanMarkdown-lint (
MD007) flags the sub-lists here because they are indented by 4 spaces instead of the expected 2/4/6 nesting pattern. Re-indent to avoid noisy failures in CI or local pre-commit hooks.- - Identifies selectedCalendars … + - Identifies selectedCalendars …
1-1: Doc still warns only about Google expiration being a stringLine 1 mentions the
googleChannelExpirationstring pitfall but the newly introducedoutlookSubscriptionExpirationwill suffer from the same issue. Either extend the warning or, even better, consider switching both fields toDateTimein a follow-up migration to enable proper numeric comparisons in Prisma.packages/app-store/office365calendar/lib/__mocks__/office365apis.ts (1)
9-38: Guard againstundefinedcalendarIds & tighten return type
config.calendarIds?.mapreturnsundefinedwhencalendarIdsis absent, violating the declaredISettledResponse[]return type and possibly crashing consumers that spread or iterate over the result.-function getEventsBatchMockResponse(config: IGetEventsBatchMockResponse): ISettledResponse[] { - return config.calendarIds?.map((calId, index) => ({ +function getEventsBatchMockResponse( + config: IGetEventsBatchMockResponse +): ISettledResponse[] { + if (!config.calendarIds?.length) return []; + return config.calendarIds.map((calId, index) => ({packages/app-store/office365calendar/lib/CalendarService.test.ts (1)
890-891: Complete the delegation credential error handling testsThese tests are marked as skipped but would be valuable for ensuring proper error handling in delegation scenarios.
Would you like me to help implement these delegation credential error handling tests?
packages/app-store/office365calendar/lib/CalendarService.ts (1)
472-476: Consider using native Date or Day.js .utc() for better performanceThe code uses Day.js with timezone operations which can be slow in performance-critical code. Consider using
.utc()or native Date methods.start: { - dateTime: dayjs(event.startTime).tz(event.organizer.timeZone).format("YYYY-MM-DDTHH:mm:ss"), + dateTime: dayjs.utc(event.startTime).tz(event.organizer.timeZone).format("YYYY-MM-DDTHH:mm:ss"), timeZone: event.organizer.timeZone, }, end: { - dateTime: dayjs(event.endTime).tz(event.organizer.timeZone).format("YYYY-MM-DDTHH:mm:ss"), + dateTime: dayjs.utc(event.endTime).tz(event.organizer.timeZone).format("YYYY-MM-DDTHH:mm:ss"), timeZone: event.organizer.timeZone, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.env.example(1 hunks)apps/api/v1/test/lib/selected-calendars/_post.test.ts(2 hunks)packages/app-store/office365calendar/api/index.ts(1 hunks)packages/app-store/office365calendar/api/webhook.ts(1 hunks)packages/app-store/office365calendar/lib/CalendarService.test.ts(1 hunks)packages/app-store/office365calendar/lib/CalendarService.ts(7 hunks)packages/app-store/office365calendar/lib/__mocks__/office365apis.ts(1 hunks)packages/app-store/office365calendar/zod.ts(1 hunks)packages/features/calendar-cache/CalendarCache.md(2 hunks)packages/lib/server/repository/selectedCalendar.ts(5 hunks)packages/prisma/migrations/20250502011606_add_outlook_subscription_id_field/migration.sql(1 hunks)packages/prisma/schema.prisma(2 hunks)turbo.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/app-store/office365calendar/api/index.tsapps/api/v1/test/lib/selected-calendars/_post.test.tspackages/app-store/office365calendar/lib/__mocks__/office365apis.tspackages/app-store/office365calendar/api/webhook.tspackages/app-store/office365calendar/lib/CalendarService.test.tspackages/app-store/office365calendar/zod.tspackages/app-store/office365calendar/lib/CalendarService.tspackages/lib/server/repository/selectedCalendar.ts
🧠 Learnings (13)
📓 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: 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.
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.
packages/app-store/office365calendar/api/index.ts (1)
Learnt from: vijayraghav-io
PR: #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.
apps/api/v1/test/lib/selected-calendars/_post.test.ts (3)
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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: #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.
.env.example (2)
Learnt from: vijayraghav-io
PR: #21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
Learnt from: vijayraghav-io
PR: #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.
packages/features/calendar-cache/CalendarCache.md (3)
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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: #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.
packages/prisma/migrations/20250502011606_add_outlook_subscription_id_field/migration.sql (2)
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
packages/app-store/office365calendar/lib/__mocks__/office365apis.ts (1)
Learnt from: vijayraghav-io
PR: #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.
packages/prisma/schema.prisma (2)
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
packages/app-store/office365calendar/api/webhook.ts (4)
Learnt from: vijayraghav-io
PR: #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: #21072
File: packages/app-store/office365calendar/api/webhook.ts:50-60
Timestamp: 2025-07-18T17:54:04.613Z
Learning: For Microsoft Graph webhook handlers, when dealing with internal configuration errors (like missing MICROSOFT_WEBHOOK_TOKEN), it's better to return 200 OK with errors tracked in the response body rather than throwing 5xx errors. This prevents retry storms from Microsoft Graph and maintains webhook subscription health, while still providing visibility through error logs and structured error responses.
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
packages/app-store/office365calendar/lib/CalendarService.test.ts (4)
Learnt from: vijayraghav-io
PR: #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: #21072
File: .env.example:354-357
Timestamp: 2025-07-18T08:49:18.779Z
Learning: For E2E integration tests that require real third-party service credentials (like Outlook calendar), it's acceptable to temporarily include actual test account credentials in .env.example during feature development and validation, provided there's a clear plan to replace them with placeholder values before final release. Test credentials should be for dedicated test tenants/accounts, not production systems.
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
packages/app-store/office365calendar/zod.ts (2)
Learnt from: vijayraghav-io
PR: #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: #21072
File: packages/app-store/office365calendar/api/webhook.ts:50-60
Timestamp: 2025-07-18T17:54:04.613Z
Learning: For Microsoft Graph webhook handlers, when dealing with internal configuration errors (like missing MICROSOFT_WEBHOOK_TOKEN), it's better to return 200 OK with errors tracked in the response body rather than throwing 5xx errors. This prevents retry storms from Microsoft Graph and maintains webhook subscription health, while still providing visibility through error logs and structured error responses.
packages/app-store/office365calendar/lib/CalendarService.ts (4)
Learnt from: vijayraghav-io
PR: #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: #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.
Learnt from: vijayraghav-io
PR: #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.
Learnt from: hariombalhara
PR: #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.
packages/lib/server/repository/selectedCalendar.ts (3)
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
Learnt from: vijayraghav-io
PR: #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.
🧬 Code Graph Analysis (2)
packages/app-store/office365calendar/lib/__mocks__/office365apis.ts (1)
packages/app-store/office365calendar/lib/CalendarService.ts (1)
ISettledResponse(43-51)
packages/lib/server/repository/selectedCalendar.ts (3)
packages/prisma/selects/credential.ts (1)
credentialForCalendarServiceSelect(3-17)packages/platform/libraries/index.ts (1)
credentialForCalendarServiceSelect(97-97)packages/types/Calendar.d.ts (1)
SelectedCalendarEventTypeIds(259-259)
🪛 markdownlint-cli2 (0.17.2)
packages/features/calendar-cache/CalendarCache.md
6-6: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
7-7: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
8-8: Unordered list indentation
Expected: 6; Actual: 8
(MD007, ul-indent)
9-9: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
10-10: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (15)
apps/api/v1/test/lib/selected-calendars/_post.test.ts (1)
92-93: Mocks updated correctly for new Outlook fields – nice catch
The additionaloutlookSubscriptionExpirationandoutlookSubscriptionIdkeys keep the test fixtures in sync with the schema change and avoid unexpectedundefinedissues in the future.Also applies to: 139-140
packages/app-store/office365calendar/api/index.ts (1)
3-3: Export looks fine
Adding thewebhookre-export cleanly exposes the new handler without side effects.turbo.json (1)
380-381: AddOUTLOOK_WEBHOOK_URLalongside the token
Office365CalendarServicebuilds the subscription callback URL fromOUTLOOK_WEBHOOK_URLwhen present (mirrors the Google path). OnlyOUTLOOK_WEBHOOK_TOKENwas added toglobalEnv; forgetting the URL will cause mismatches in CI/CD whereNEXT_PUBLIC_WEBAPP_URLdiffers from the production domain."OUTLOOK_WEBHOOK_TOKEN", + "OUTLOOK_WEBHOOK_URL",packages/app-store/office365calendar/lib/__mocks__/office365apis.ts (1)
1-1: Import path may break in Jest without TS path-alias
import type { ISettledResponse } from "office365calendar/lib/CalendarService";If tests run with plain Node resolution (no TS-path mapping), this absolute path can fail. Consider using a relative path (
../CalendarService) or ensuremoduleNameMapper/pathsmapping exists injest.config.ts.packages/app-store/office365calendar/api/webhook.ts (3)
25-39: LGTM! Proper webhook validation handlingThe validation token handling correctly implements Microsoft's webhook verification requirements.
50-54: Good optimization for handling duplicate eventsUsing
uniqueByto process each subscription only once is efficient given Microsoft can send multiple events for the same subscription.
55-94: Well-structured event processing with proper error handlingThe implementation correctly:
- Validates client state for security
- Returns 200 OK with error details to prevent Microsoft Graph retry storms
- Fetches and updates calendar cache appropriately
packages/app-store/office365calendar/zod.ts (1)
17-34: Well-structured schema matching Microsoft's specificationsThe schema correctly implements Microsoft's change notification structure with proper field types and optionality.
packages/app-store/office365calendar/lib/CalendarService.test.ts (1)
304-384: Comprehensive cache testing with proper grouping validationThe test correctly validates that calendars are grouped by eventTypeId for efficient batch fetching, which is crucial for performance.
packages/lib/server/repository/selectedCalendar.ts (3)
31-36: Consistent integration with existing Google Calendar patternThe outlookSubscriptionId filter follows the same pattern as googleChannelId, maintaining consistency.
174-195: Good integration support for both calendar typesThe OR clause properly handles both Google and Outlook calendars with appropriate expiration checks.
316-334: Method follows consistent pattern with Google CalendarThe implementation correctly mirrors
findFirstByGoogleChannelIdwith appropriate naming and structure.packages/app-store/office365calendar/lib/CalendarService.ts (3)
316-356: Well-structured batch API implementationThe refactoring properly handles:
- Batch requests for multiple calendars
- Retry-after headers for rate limiting
- Pagination through nextLink responses
758-758: Method name uses singular form correctlyDespite the past comment, the method name
startWatchingCalendarInOutlookis correct as it operates on a single calendar.
867-942: Robust subscription lifecycle managementThe implementation correctly handles:
- Shared subscriptions between event types
- Cache cleanup and repopulation
- Partial unwatching scenarios
| static async updateManyForEventTypeIds({ | ||
| data, | ||
| eventTypeIds, | ||
| }: { | ||
| data: Prisma.SelectedCalendarUncheckedCreateInput; | ||
| eventTypeIds: SelectedCalendarEventTypeIds; | ||
| }) { | ||
| const userId = data.userId; | ||
| return await Promise.allSettled( | ||
| eventTypeIds.map((eventTypeId) => { | ||
| SelectedCalendarRepository.update({ | ||
| where: { | ||
| userId: data.userId, | ||
| integration: data.integration, | ||
| externalId: data.externalId, | ||
| eventTypeId: data.eventTypeId || null, | ||
| }, | ||
| data: { | ||
| ...data, | ||
| eventTypeId, | ||
| userId, | ||
| integration: data.integration, | ||
| externalId: data.externalId, | ||
| credentialId: data.credentialId, | ||
| }, | ||
| }); | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical bug in eventTypeId assignment
The method uses data.eventTypeId on line 432 instead of the mapped eventTypeId, which will cause incorrect updates.
return await Promise.allSettled(
eventTypeIds.map((eventTypeId) => {
SelectedCalendarRepository.update({
where: {
userId: data.userId,
integration: data.integration,
externalId: data.externalId,
- eventTypeId: data.eventTypeId || null,
+ eventTypeId: eventTypeId || null,
},
data: {
...data,
eventTypeId,
userId,
integration: data.integration,
externalId: data.externalId,
credentialId: data.credentialId,
},
});
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static async updateManyForEventTypeIds({ | |
| data, | |
| eventTypeIds, | |
| }: { | |
| data: Prisma.SelectedCalendarUncheckedCreateInput; | |
| eventTypeIds: SelectedCalendarEventTypeIds; | |
| }) { | |
| const userId = data.userId; | |
| return await Promise.allSettled( | |
| eventTypeIds.map((eventTypeId) => { | |
| SelectedCalendarRepository.update({ | |
| where: { | |
| userId: data.userId, | |
| integration: data.integration, | |
| externalId: data.externalId, | |
| eventTypeId: data.eventTypeId || null, | |
| }, | |
| data: { | |
| ...data, | |
| eventTypeId, | |
| userId, | |
| integration: data.integration, | |
| externalId: data.externalId, | |
| credentialId: data.credentialId, | |
| }, | |
| }); | |
| }) | |
| ); | |
| } | |
| static async updateManyForEventTypeIds({ | |
| data, | |
| eventTypeIds, | |
| }: { | |
| data: Prisma.SelectedCalendarUncheckedCreateInput; | |
| eventTypeIds: SelectedCalendarEventTypeIds; | |
| }) { | |
| const userId = data.userId; | |
| return await Promise.allSettled( | |
| eventTypeIds.map((eventTypeId) => { | |
| return SelectedCalendarRepository.update({ | |
| where: { | |
| userId: data.userId, | |
| integration: data.integration, | |
| externalId: data.externalId, | |
| - eventTypeId: data.eventTypeId || null, | |
| + eventTypeId: eventTypeId || null, | |
| }, | |
| data: { | |
| ...data, | |
| eventTypeId, | |
| userId, | |
| integration: data.integration, | |
| externalId: data.externalId, | |
| credentialId: data.credentialId, | |
| }, | |
| }); | |
| }) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In packages/lib/server/repository/selectedCalendar.ts lines 417 to 445, the
updateManyForEventTypeIds method incorrectly uses data.eventTypeId in the where
clause instead of the current eventTypeId from the map iteration. To fix this,
replace data.eventTypeId with the eventTypeId variable in the where condition to
ensure the correct eventTypeId is used for each update call.
| - A unique constraint covering the columns `[outlookSubscriptionId,eventTypeId]` on the table `SelectedCalendar` will be added. If there are existing duplicate values, this will fail. | ||
|
|
||
| */ | ||
| -- AlterTable | ||
| ALTER TABLE "SelectedCalendar" ADD COLUMN "outlookSubscriptionExpiration" TEXT, | ||
| ADD COLUMN "outlookSubscriptionId" TEXT; | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "SelectedCalendar_outlookSubscriptionId_eventTypeId_key" ON "SelectedCalendar"("outlookSubscriptionId", "eventTypeId"); |
There was a problem hiding this comment.
Unique index contradicts the intended subscription-sharing model
Historical design (see learning notes) deliberately re-uses the same Outlook subscription across many eventTypeIds and even allows multiple SelectedCalendar rows for a single (subscriptionId,eventTypeId) pair during retries. The composite unique constraint will block that pattern and may also fail the migration on existing data.
--- CreateIndex
-CREATE UNIQUE INDEX "SelectedCalendar_outlookSubscriptionId_eventTypeId_key"
- ON "SelectedCalendar"("outlookSubscriptionId", "eventTypeId");
+-- (drop the unique index – leave the columns without a uniqueness constraint)Consider adding a non-unique index instead if query performance is required.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/prisma/migrations/20250502011606_add_outlook_subscription_id_field/migration.sql
around lines 4 to 12, the unique index on (outlookSubscriptionId, eventTypeId)
conflicts with the intended design that allows multiple rows with the same
subscriptionId and eventTypeId. To fix this, change the unique index to a
non-unique index by removing the UNIQUE keyword so it does not enforce
uniqueness but still improves query performance.
| // Used to identify a watched calendar in Outlook | ||
| outlookSubscriptionId String? | ||
| outlookSubscriptionExpiration String? | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Store expiration as DateTime, not String
outlookSubscriptionExpiration mirrors the Google field and is typed as String.
This prevents using efficient range queries (<, >, between) and forces string-to-date casting in application code.
- outlookSubscriptionExpiration String?
+ /// RFC3339 timestamp e.g. "2025-08-30T23:59:59Z"
+ outlookSubscriptionExpiration DateTime?If changing the type is too risky for this PR, at least document the ISO format expectation and add a TODO for a migration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Used to identify a watched calendar in Outlook | |
| outlookSubscriptionId String? | |
| outlookSubscriptionExpiration String? | |
| // Used to identify a watched calendar in Outlook | |
| outlookSubscriptionId String? | |
| /// RFC3339 timestamp e.g. "2025-08-30T23:59:59Z" | |
| outlookSubscriptionExpiration DateTime? |
🤖 Prompt for AI Agents
In packages/prisma/schema.prisma at lines 868 to 871, the field
outlookSubscriptionExpiration is currently typed as String, which limits
efficient date queries and requires casting in application code. Change the type
of outlookSubscriptionExpiration from String to DateTime to enable native date
operations. If this change is too risky for now, add a comment documenting that
the field expects an ISO date string and include a TODO note to migrate this
field to DateTime in the future.
| // Think about introducing a generated unique key ${userId}_${integration}_${externalId}_${eventTypeId} | ||
| @@unique([userId, integration, externalId, eventTypeId]) | ||
| @@unique([googleChannelId, eventTypeId]) | ||
| @@unique([outlookSubscriptionId, eventTypeId]) |
There was a problem hiding this comment.
Remove the unique constraint on (outlookSubscriptionId, eventTypeId)
The Outlook integration intentionally re-uses one subscription across many event types (see past discussions & retrieved learnings).
@@unique([outlookSubscriptionId, eventTypeId]) will break that pattern by disallowing duplicates and cause Unique constraint failed errors when multiple event types share the same subscription.
- @@unique([outlookSubscriptionId, eventTypeId])
+ // DO NOT make this unique – one subscription can belong to many event types.
+ @@index([outlookSubscriptionId, eventTypeId])🤖 Prompt for AI Agents
In packages/prisma/schema.prisma at line 892, remove the unique constraint
declaration @@unique([outlookSubscriptionId, eventTypeId]) because it prevents
reusing one subscription across multiple event types. Deleting this constraint
will allow multiple event types to share the same outlookSubscriptionId without
causing unique constraint errors.
|
Please see comment here #21050 (comment) |
|
/tip 100 @ouwargui |
|
@ouwargui: You've been awarded a $100 by Cal.com, Inc.! 👉 Complete your Algora onboarding to collect the tip. |
3 participants
What does this PR do?
Issue
The core issue is that fetching availability data for Outlook calendars currently involves querying the Microsoft Graph API every time someone views a public booking page that uses an Outlook calendar.
How it was fixed
Used the Google Calendar code as base to implement caching to the Microsoft Graph response and added a webhook to handle calendar change notifications and update the cache.
Also allowed
office365_calendarintegration to be watched and unwatched on thecalendar-cachecronjob.Next steps
We should refactor some duplicate code between
googlecalendar.CalendarServiceandoffice365calendar.CalendarService.Visual Demo (For contributors especially)
https://cap.link/e6g6jgd2t1nztd9
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
OUTLOOK_WEBHOOK_TOKEN: Token to verify incoming webhooks from Microsoft Outlook (Required)OUTLOOK_WEBHOOK_URL: Optional URL to override for tunelling webhooks. Defaults to NEXT_PUBLIC_WEBAPP_URL.calendar-cachefeature flag for your team.SelectedCalendartablecalendar-cachecronjob manually or wait for it to run automaticallyCalendarCachetablehttpsSummary by mrge
Added support for caching Outlook calendar availability and handling Microsoft Graph webhooks to keep cache updated.