packages/app-store/office365calendar/lib/CalendarService.ts (1)

348-390: Add input validation for date range and calendar IDs.

The method should validate inputs before processing to prevent unnecessary API calls and potential errors.

Add validation at the beginning of the method:

 async fetchAvailability(
   dateFrom: string,
   dateTo: string,
   calendarIds: string[],
   getCreatedDateTime = false
 ): Promise<EventBusyDate[]> {
+  if (!calendarIds || calendarIds.length === 0) {
+    return [];
+  }
+  
   const dateFromParsed = new Date(dateFrom);
   const dateToParsed = new Date(dateTo);
+  
+  if (isNaN(dateFromParsed.getTime()) || isNaN(dateToParsed.getTime())) {
+    throw new Error("Invalid date format provided");
+  }
+  
+  if (dateFromParsed >= dateToParsed) {
+    throw new Error("dateFrom must be before dateTo");
+  }

packages/app-store/office365calendar/lib/CalendarService.ts (2)

697-699: LGTM! Well-documented subscription scoping.

The implementation correctly scopes subscriptions to specific calendars using /calendars/{calendar_id}/events to reduce notification volume, as documented in the Microsoft Graph API guidelines.

---

700-700: LGTM! Using recommended TTL for calendar subscriptions.

The 3-day TTL aligns with Microsoft's recommended duration for calendar event subscriptions as per the documentation.

packages/app-store/office365calendar/lib/CalendarService.ts (3)

405-436: Consider implementing cache warming strategy.

The cache miss path fetches data but doesn't automatically populate the cache. Consider warming the cache after a miss to benefit subsequent requests.

Consider updating the cache after a miss:

 this.log.debug("[Cache Miss] Fetching availability", { dateFrom, dateTo, calendarIds });
 const data = await this.fetchAvailability(dateFrom, dateTo, calendarIds);
+
+// Optionally warm the cache for future requests
+await this.setAvailabilityInCache(cacheArgs, data).catch((err) => {
+  this.log.warn("Failed to warm cache after miss", err);
+});
+
 return data;

---

712-726: Add error handling for failed subscription deletions.

While the method logs warnings for failed deletions, consider tracking which deletions failed and potentially retrying or escalating critical failures.

Consider tracking failed deletions:

 private async stopWatchingCalendarsInMicrosoft(subscriptions: { subscriptionId: string | null }[]) {
   const uniqueSubscriptions = subscriptions.filter(
     (s, i, arr) => s.subscriptionId && arr.findIndex((x) => x.subscriptionId === s.subscriptionId) === i
   );

-  await Promise.allSettled(
+  const results = await Promise.allSettled(
     uniqueSubscriptions.map(({ subscriptionId }) =>
       subscriptionId
         ? this.fetcher(`/subscriptions/${subscriptionId}`, { method: "DELETE" }).catch((err) => {
             this.log.warn(`Failed to delete subscription ${subscriptionId}`, err);
+            throw err; // Re-throw to capture in allSettled
           })
         : Promise.resolve()
     )
   );
+  
+  const failedDeletions = results.filter(r => r.status === 'rejected');
+  if (failedDeletions.length > 0) {
+    this.log.error(`Failed to delete ${failedDeletions.length} subscriptions`);
+  }
 }

---

728-756: Consider batching cache operations for performance.

The method processes calendars per event type sequentially. For better performance with many event types, consider batching the cache operations.

Consider parallel processing with batching:

-for (const [_eventTypeId, selectedCalendars] of Array.from(selectedCalendarsPerEventType.entries())) {
-  const parsedArgs = {
-    timeMin: getTimeMin(),
-    timeMax: getTimeMax(),
-    items: selectedCalendars.map((sc) => ({ id: sc.externalId })),
-  };
-  const data = await this.fetchAvailability(
-    parsedArgs.timeMin,
-    parsedArgs.timeMax,
-    parsedArgs.items.map((i) => i.id)
-  );
-  await this.setAvailabilityInCache(parsedArgs, data);
-}
+const cacheOperations = Array.from(selectedCalendarsPerEventType.entries()).map(
+  async ([_eventTypeId, selectedCalendars]) => {
+    const parsedArgs = {
+      timeMin: getTimeMin(),
+      timeMax: getTimeMax(),
+      items: selectedCalendars.map((sc) => ({ id: sc.externalId })),
+    };
+    const data = await this.fetchAvailability(
+      parsedArgs.timeMin,
+      parsedArgs.timeMax,
+      parsedArgs.items.map((i) => i.id)
+    );
+    return this.setAvailabilityInCache(parsedArgs, data);
+  }
+);
+
+await Promise.all(cacheOperations);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

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: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.

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.