feat: Calendar Cache #23876
feat: Calendar Cache #23876
Conversation
WalkthroughAdds a calendar subscription and caching subsystem: new CalendarSubscriptionService, CalendarSyncService, cache repository/service/wrapper, adapter factory, Google and Office365 adapters, and port/interface types. Introduces ISelectedCalendarRepository and Prisma-backed SelectedCalendarRepository, extends Prisma schema and migrations with channel/sync fields and CalendarCacheEvent model. Adds Next.js webhook and two cron routes (with auth), Vercel cron entries, env/turbo exposure for webhook variables, updates getCalendar to optionally return a cache-wrapped Calendar, introduces feature flags and defaults, and includes extensive unit tests and mocks. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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. Comment |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
volnei
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
volnei
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts (1)
5-5: Consider performance implications of dayjs usage.As per coding guidelines, flag excessive Day.js use in performance-critical code. This file uses dayjs for date calculations in
fetchEventswhich may be called frequently during sync operations.Based on coding guidelines
Consider using native Date operations for simple date arithmetic in hot paths:
- const now = dayjs(); - const pastWindow = now.subtract(1, "month"); - const futureWindow = now.add(3, "month"); + const now = new Date(); + const pastWindow = new Date(now.getTime() - 30 * 24 * 60 * 60 * 1000); + const futureWindow = new Date(now.getTime() + 90 * 24 * 60 * 60 * 1000); - const timeMinISO = pastWindow.toISOString(); - const timeMaxISO = futureWindow.toISOString(); + const timeMinISO = pastWindow.toISOString(); + const timeMaxISO = futureWindow.toISOString();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts(1 hunks)packages/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.tspackages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.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/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.tspackages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.tspackages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
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.
📚 Learning: 2025-08-08T09:12:08.280Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
🧬 Code graph analysis (2)
packages/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.ts (1)
packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
DefaultAdapterFactory(15-44)
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts (2)
packages/features/calendar-subscription/lib/CalendarSubscriptionPort.interface.ts (5)
ICalendarSubscriptionPort(47-83)CalendarCredential(42-42)CalendarSubscriptionResult(8-14)CalendarSubscriptionEvent(36-40)CalendarSubscriptionEventItem(16-34)packages/types/Calendar.d.ts (1)
SelectedCalendar(320-323)
⏰ Context from checks skipped due to timeout of 180000ms. 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 (6)
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts (5)
31-42: Fixed - looks good!The webhook token validation no longer logs the provided token, addressing the security concern from the previous review.
53-85: Fixed - looks good!The debug/console.log statements with sensitive data have been removed.
86-101: Fixed - looks good!The channels.stop request now correctly includes both
idandresourceIdas required by the Google Calendar API.
150-193: Fixed - looks good!The
originalStartDatefield is now correctly mapped fromevent.originalStartTimewith proper date handling for bothdateTimeanddateformats.
110-142: Critical pagination bug still not fixed.The pagination logic still has the same issue where
pageTokenis never applied to the request parameters, creating potential for infinite loops.Apply this fix to resolve the pagination issue:
- const params: calendar_v3.Params$Resource$Events$List = { - calendarId: selectedCalendar.externalId, - pageToken, - singleEvents: true, - }; + const baseParams = { + calendarId: selectedCalendar.externalId, + singleEvents: true, + };do { - const { data }: { data: calendar_v3.Schema$Events } = await client.events.list(params); + const { data }: { data: calendar_v3.Schema$Events } = await client.events.list({ + ...baseParams, + pageToken, + }); syncToken = data.nextSyncToken || syncToken; - pageToken = data.nextPageToken ?? null; + pageToken = data.nextPageToken || undefined; events.push(...(data.items || [])); } while (pageToken);packages/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.ts (1)
1-55: LGTM!Well-structured test suite that thoroughly validates the adapter factory behavior:
- Tests both supported providers (
google_calendar,office365_calendar)- Verifies singleton behavior
- Tests error handling for unsupported providers
- Validates the provider list functionality
The tests follow good practices with proper setup, clear assertions, and comprehensive coverage.
packages/features/calendar-subscription/adapters/__tests__/AdaptersFactory.test.ts
Show resolved
Hide resolved
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/prisma/schema.prisma(3 hunks)turbo.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- turbo.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
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.
📚 Learning: 2025-09-08T07:27:42.903Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
Applied to files:
packages/prisma/schema.prisma
📚 Learning: 2025-07-18T08:47:01.264Z
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/prisma/schema.prisma
📚 Learning: 2025-07-18T08:47:01.264Z
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/prisma/schema.prisma
🔇 Additional comments (1)
packages/prisma/schema.prisma (1)
973-975: Restore channel indices for watch/unwatch pathsThe new provider-agnostic
channel*fields power the scheduler’s watch/unwatch sweepers, but we never reintroduced the composite indices that keep those cron queries off full table scans. As data for non-Google providers grows, those jobs will degrade badly.Please mirror the existing google-specific indices for the new fields and index
channelIdfor webhook lookups:@@index([integration, googleChannelExpiration, error, watchAttempts, maxAttempts], name: "SelectedCalendar_watch_idx") @@index([integration, googleChannelExpiration, error, unwatchAttempts, maxAttempts], name: "SelectedCalendar_unwatch_idx") + @@index([integration, channelExpiration, error, watchAttempts, maxAttempts], name: "SelectedCalendar_watch_v2_idx") + @@index([integration, channelExpiration, error, unwatchAttempts, maxAttempts], name: "SelectedCalendar_unwatch_v2_idx") + @@index([channelId], name: "SelectedCalendar_channelId_idx")Based on learnings
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/lib/server/repository/SelectedCalendarRepository.ts (1)
25-51: Align signature with interface and reduce payload; guard empty integrations.
- Make integrations optional to match the interface.
- Only select id (caller only uses id).
- Avoid passing integration filter when list is empty.
As per coding guidelines
- async findNextSubscriptionBatch({ take, integrations }: { take: number; integrations: string[] }) { + async findNextSubscriptionBatch({ + take, + integrations = [], + }: { + take: number; + integrations?: string[]; + }) { return this.prismaClient.selectedCalendar.findMany({ + select: { id: true }, where: { - integration: { in: integrations }, + ...(integrations.length ? { integration: { in: integrations } } : {}), OR: [{ syncSubscribedAt: null }, { channelExpiration: { lte: new Date() } }], // initially we will run subscription only for teams that have // the feature flags enabled and it should be removed later user: { teams: { some: { team: { features: { some: { OR: [ { featureId: "calendar-subscription-cache" }, { featureId: "calendar-subscription-sync" }, ], }, }, }, }, }, }, }, take, }); }packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (2)
87-92: Clear channel fields on unsubscribe to avoid stale state and auto-resubscribe.Also nullify channel fields so rows don’t look “subscribed” and won’t be re-enqueued unintentionally.
await Promise.all([ calendarSubscriptionAdapter.unsubscribe(selectedCalendar, credential), this.deps.selectedCalendarRepository.updateSubscription(selectedCalendarId, { + channelId: null, + channelResourceId: null, + channelResourceUri: null, + channelKind: null, + channelExpiration: null, syncSubscribedAt: null, }), ]);
94-99: Consider unconditional cache cleanup after unsubscribe.Cleanup should not depend on the global cache flag; after unsubscribe we shouldn’t retain cache entries for that calendar.
Would you like me to update this to always call cleanupCache and add a safety guard inside the cache service?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts(1 hunks)packages/lib/server/repository/SelectedCalendarRepository.ts(1 hunks)packages/lib/server/repository/__tests__/SelectedCalendarRepository.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/server/repository/tests/SelectedCalendarRepository.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*Repository.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/SelectedCalendarRepository.ts
**/*.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/lib/server/repository/SelectedCalendarRepository.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.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/lib/server/repository/SelectedCalendarRepository.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/repository/SelectedCalendarRepository.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.ts
**/*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/features/calendar-subscription/lib/CalendarSubscriptionService.ts
🧠 Learnings (11)
📓 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#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
📚 Learning: 2025-09-08T07:27:42.903Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-09-08T07:27:42.903Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-07-28T11:50:23.946Z
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 : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-08-07T18:42:34.081Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/lib/server/repository/PrismaPhoneNumberRepository.ts:412-417
Timestamp: 2025-08-07T18:42:34.081Z
Learning: In Cal.com codebase, the coding guideline requiring explicit `select` clauses instead of `include` for Prisma queries applies to read operations but not to update operations. Update operations don't need explicit select clauses.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-08-19T14:30:19.998Z
Learnt from: zomars
PR: calcom/cal.com#22708
File: packages/features/calendar-cache-sql/CalendarSubscriptionRepository.ts:181-239
Timestamp: 2025-08-19T14:30:19.998Z
Learning: User zomars reports that Prisma syntax `this.prismaClient.calendarSubscription.fields.maxSyncErrors` in where clause comparisons works locally in the calendar cache SQL system. Need to verify if this is supported in current Prisma versions or specific to testing environment.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-08-05T12:04:29.037Z
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.
Applied to files:
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts
📚 Learning: 2025-07-18T17:57:16.395Z
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/features/calendar-subscription/lib/CalendarSubscriptionService.ts
🧬 Code graph analysis (2)
packages/lib/server/repository/SelectedCalendarRepository.ts (2)
packages/lib/server/repository/SelectedCalendarRepository.interface.ts (1)
ISelectedCalendarRepository(3-62)packages/lib/server/repository/selectedCalendar.ts (1)
SelectedCalendarRepository(42-438)
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (6)
packages/features/calendar-subscription/adapters/AdaptersFactory.ts (2)
AdapterFactory(7-10)CalendarSubscriptionProvider(5-5)packages/lib/server/repository/SelectedCalendarRepository.interface.ts (1)
ISelectedCalendarRepository(3-62)packages/features/flags/features.repository.ts (1)
FeaturesRepository(18-254)packages/features/calendar-subscription/lib/cache/CalendarCacheEventService.ts (1)
CalendarCacheEventService(11-99)packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts (1)
CalendarSyncService(11-98)packages/features/calendar-subscription/lib/CalendarSubscriptionPort.interface.ts (2)
CalendarSubscriptionEvent(36-40)CalendarCredential(42-42)
⏰ Context from checks skipped due to timeout of 180000ms. 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 (2)
packages/lib/server/repository/SelectedCalendarRepository.ts (1)
8-18: Prisma: replace include with select and explicitly whitelist fields.Per guidelines, avoid include and select only what you need; ensure no sensitive fields (like credential.key) can leak. Replace include with select and enumerate required SelectedCalendar fields plus credential.delegationCredential.
As per coding guidelines
return this.prismaClient.selectedCalendar.findUnique({ where: { id }, - include: { - credential: { - select: { - delegationCredential: true, - }, - }, - }, + select: { + id: true, + userId: true, + credentialId: true, + integration: true, + externalId: true, + channelId: true, + channelResourceId: true, + channelResourceUri: true, + channelKind: true, + channelExpiration: true, + syncSubscribedAt: true, + syncToken: true, + syncedAt: true, + syncErrorAt: true, + syncErrorCount: true, + credential: { + select: { + delegationCredential: true, // never select credential.key + }, + }, + }, });packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (1)
168-191: Persist syncToken even when no items; update status after handlers.Early return skips persisting a new syncToken, and updating syncedAt before handlers risks losing events on failure. Process handlers first, then persist token and clear errors. If no items, still persist token and syncedAt.
- if (!events?.items?.length) { - log.debug("No events fetched", { channelId: selectedCalendar.channelId }); - return; - } - - log.debug("Processing events", { channelId: selectedCalendar.channelId, count: events.items.length }); - await this.deps.selectedCalendarRepository.updateSyncStatus(selectedCalendar.id, { - syncToken: events.syncToken || selectedCalendar.syncToken, - syncedAt: new Date(), - syncErrorAt: null, - syncErrorCount: 0, - }); - - // it requires both global and team/user feature cache enabled - if (cacheEnabled && cacheEnabledForUser) { - log.debug("Caching events", { count: events.items.length }); - await this.deps.calendarCacheEventService.handleEvents(selectedCalendar, events.items); - } - - if (syncEnabled) { - log.debug("Syncing events", { count: events.items.length }); - await this.deps.calendarSyncService.handleEvents(selectedCalendar, events.items); - } + const newSyncToken = events?.syncToken || selectedCalendar.syncToken; + const count = events?.items?.length ?? 0; + + if (count > 0) { + log.debug("Processing events", { channelId: selectedCalendar.channelId, count }); + if (cacheEnabled && cacheEnabledForUser) { + log.debug("Caching events", { count }); + await this.deps.calendarCacheEventService.handleEvents(selectedCalendar, events!.items); + } + if (syncEnabled) { + log.debug("Syncing events", { count }); + await this.deps.calendarSyncService.handleEvents(selectedCalendar, events!.items); + } + } else { + log.debug("No events fetched", { channelId: selectedCalendar.channelId }); + } + + await this.deps.selectedCalendarRepository.updateSyncStatus(selectedCalendar.id, { + syncToken: newSyncToken, + syncedAt: new Date(), + syncErrorAt: null, + syncErrorCount: 0, + });
| if (!selectedCalendar) return null; | ||
|
|
There was a problem hiding this comment.
Return type mismatch: don’t return null from a void method.
processWebhook should not return null. Use plain return to satisfy Promise.
- if (!selectedCalendar) return null;
+ if (!selectedCalendar) return;📝 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.
| if (!selectedCalendar) return null; | |
| // before early-exiting when there's no selected calendar | |
| if (!selectedCalendar) return; |
🤖 Prompt for AI Agents
In packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts
around lines 117-118, the processWebhook handler currently uses "return null"
which conflicts with its void/Promise<void> signature; remove the null return
and either use a plain "return" or simply fall through (no value) so the
function resolves as Promise<void>, and ensure the method's signature remains
Promise<void> to match the actual return behavior.
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…cal.com into feat/calendar_sync_cache
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts (6)
5-11: Type the adapter mock toICalendarSubscriptionPortto catch interface driftUse the interface instead of a loose shape; cast the mock object once.
import type { AdapterFactory } from "@calcom/features/calendar-subscription/adapters/AdaptersFactory"; +import type { ICalendarSubscriptionPort } from "@calcom/features/calendar-subscription/lib/lib/CalendarSubscriptionPort.interface"; @@ - let mockAdapter: { - subscribe: ReturnType<typeof vi.fn>; - unsubscribe: ReturnType<typeof vi.fn>; - validate: ReturnType<typeof vi.fn>; - extractChannelId: ReturnType<typeof vi.fn>; - fetchEvents: ReturnType<typeof vi.fn>; - }; + let mockAdapter: ICalendarSubscriptionPort; @@ - mockAdapter = { + mockAdapter = { subscribe: vi.fn().mockResolvedValue(mockSubscriptionResult), unsubscribe: vi.fn().mockResolvedValue(undefined), validate: vi.fn().mockResolvedValue(true), extractChannelId: vi.fn().mockResolvedValue("test-channel-id"), fetchEvents: vi.fn().mockResolvedValue(mockEvents), - }; + } as unknown as ICalendarSubscriptionPort;Also applies to: 95-101, 104-111
263-267: Stabilize feature-flag stubs; avoid order-dependentmockResolvedValueOnceThese tests rely on call order to
checkIfFeatureIsEnabledGloballywhile the SUT calls both flags concurrently. Prefer a predicate-based mock or a small helper to set flags explicitly to reduce flakiness.Example helper:
function setFeatures({ cache, sync, userCache, }: { cache: boolean; sync: boolean; userCache: boolean }) { mockFeaturesRepository.checkIfFeatureIsEnabledGlobally.mockImplementation(async (feature) => { if (feature === "calendar-subscription-cache") return cache; if (feature === "calendar-subscription-sync") return sync; return false; }); mockFeaturesRepository.checkIfUserHasFeature.mockResolvedValue(userCache); }Usage in tests:
setFeatures({ cache: false, sync: true, userCache: true }); // cache off, sync onAlso applies to: 287-292, 299-304, 312-315, 324-328, 343-347
152-168: Assert initial load:subscribeshould triggerprocessEventsVerifies orchestration, not just adapter/repo calls.
const processEventsSpy = vi.spyOn(service, "processEvents"); await service.subscribe("test-calendar-id"); expect(processEventsSpy).toHaveBeenCalledWith(mockSelectedCalendar);
222-231: Assert webhook flow triggers event processingEnsures
processWebhookcallsprocessEventsafter channel resolution.const mockRequest = new Request("http://example.com"); const processEventsSpy = vi.spyOn(service, "processEvents"); await service.processWebhook("google_calendar", mockRequest); expect(processEventsSpy).toHaveBeenCalledWith(mockSelectedCalendar);
268-285: Cover syncToken fallback pathAdd a case where the adapter returns
syncToken: undefinedand assert we fall back to the existing token.mockAdapter.fetchEvents.mockResolvedValueOnce({ ...mockEvents, syncToken: undefined }); await service.processEvents(mockSelectedCalendar); expect(mockSelectedCalendarRepository.updateSyncStatus).toHaveBeenCalledWith( mockSelectedCalendar.id, expect.objectContaining({ syncToken: mockSelectedCalendar.syncToken }) );
191-220: Add unsubscribe edge case: missing credentialWhen
getCredentialForCalendarCacheresolves tonull,unsubscribeshould exit early without adapter calls.const { getCredentialForCalendarCache } = await import("../delegationCredential"); getCredentialForCalendarCache.mockResolvedValueOnce(null); await service.unsubscribe("test-calendar-id"); expect(mockAdapter.unsubscribe).not.toHaveBeenCalled(); expect(mockSelectedCalendarRepository.updateSubscription).not.toHaveBeenCalled();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.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/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
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#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
📚 Learning: 2025-08-05T12:04:29.037Z
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.
Applied to files:
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts
🧬 Code graph analysis (1)
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts (4)
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (1)
CalendarSubscriptionService(19-255)packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
AdapterFactory(7-10)packages/lib/server/repository/SelectedCalendarRepository.interface.ts (1)
ISelectedCalendarRepository(3-62)packages/features/calendar-subscription/lib/__mocks__/delegationCredential.ts (1)
getCredentialForCalendarCache(3-8)
| @@ -0,0 +1,420 @@ | |||
| import "../__mocks__/delegationCredential"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wire the manual mock via vi.mock against the actual module path
Importing the file from __mocks__ doesn't guarantee the SUT uses it. Mock the real module path so CalendarSubscriptionService resolves the manual mock, and switch the dynamic import to the real path as well.
Apply:
- import "../__mocks__/delegationCredential";
+ // Ensure CalendarSubscriptionService uses the manual mock at lib/__mocks__/delegationCredential.ts
+ vi.mock("../delegationCredential");And:
- const { getCredentialForCalendarCache } = await import("../__mocks__/delegationCredential");
+ const { getCredentialForCalendarCache } = await import("../delegationCredential");Also applies to: 148-150
…tween + minor nits
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts (3)
31-40: Rename param to plural to reflect array and fix JSDocThe parameter is an array but named in singular in both the signature and JSDoc. Rename to selectedCalendarIds for clarity and consistency.
- /** - * - * @param selectedCalendarId - * @param start - * @param end - */ - findAllBySelectedCalendarIdsBetween( - selectedCalendarId: string[], + /** + * + * @param selectedCalendarIds + * @param start + * @param end + */ + findAllBySelectedCalendarIdsBetween( + selectedCalendarIds: string[], start: Date, end: Date ): Promise<Pick<CalendarCacheEvent, "start" | "end" | "timeZone">[]>;
11-11: Tighten return types (avoid Promise)Explicit return types make implementations and callers safer. Suggest:
- upsertMany: Promise (or a domain result if you track counts)
- deleteMany: Promise<Prisma.BatchPayload> (Prisma returns { count })
- deleteAllBySelectedCalendarId, deleteStale: Promise<Prisma.BatchPayload> or Promise (pick one consistently)
As per coding guidelines
-import type { CalendarCacheEvent } from "@calcom/prisma/client"; +import type { CalendarCacheEvent, Prisma } from "@calcom/prisma/client"; - upsertMany(events: Partial<CalendarCacheEvent>[]): Promise<unknown>; + upsertMany(events: Partial<CalendarCacheEvent>[]): Promise<void>; - deleteMany(events: Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId">[]): Promise<unknown>; + deleteMany( + events: Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId">[] + ): Promise<Prisma.BatchPayload>; - deleteAllBySelectedCalendarId(selectedCalendarId: string): Promise<unknown>; + deleteAllBySelectedCalendarId(selectedCalendarId: string): Promise<Prisma.BatchPayload>; - deleteStale(): Promise<unknown>; + deleteStale(): Promise<Prisma.BatchPayload>;Also applies to: 17-17, 23-23, 28-28
1-1: Decouple repository API from Prisma modelReturning Pick<CalendarCacheEvent, ...> leaks persistence-layer types into the domain. Define a small DTO for the cache read path to reduce coupling and ease schema changes.
-import type { CalendarCacheEvent, Prisma } from "@calcom/prisma/client"; +import type { Prisma } from "@calcom/prisma/client"; +export type CachedBusySlot = { + start: Date; + end: Date; + timeZone?: string; +}; - ): Promise<Pick<CalendarCacheEvent, "start" | "end" | "timeZone">[]>; + ): Promise<CachedBusySlot[]>;Also applies to: 36-40
packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts (2)
91-105: Early return for empty input and consistent shapeMirror the guard used in getAvailability and keep return types consistent.
async getAvailabilityWithTimeZones?( dateFrom: string, dateTo: string, selectedCalendars: IntegrationCalendar[] // _fallbackToPrimary?: boolean ): Promise<{ start: Date | string; end: Date | string; timeZone: string }[]> { - log.debug("getAvailabilityWithTimeZones from cache", { dateFrom, dateTo, selectedCalendars }); - const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id)); + const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id)); + if (!selectedCalendarIds.length) return []; const result = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween( selectedCalendarIds, new Date(dateFrom), new Date(dateTo) ); return result.map(({ start, end, timeZone }) => ({ start, end, timeZone: timeZone || "UTC" })); }
21-26: Make deps immutableMark deps as readonly to prevent accidental reassignment.
- private deps: { + private readonly deps: { originalCalendar: Calendar; calendarCacheEventRepository: ICalendarCacheEventRepository; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/app/api/cron/calendar-subscriptions-cleanup/route.ts(1 hunks)packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts(1 hunks)packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts(1 hunks)packages/features/calendar-subscription/lib/cache/__tests__/CalendarCacheEventRepository.test.ts(1 hunks)packages/features/calendar-subscription/lib/cache/__tests__/CalendarCacheEventService.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/app/api/cron/calendar-subscriptions-cleanup/route.ts
- packages/features/calendar-subscription/lib/cache/tests/CalendarCacheEventRepository.test.ts
- packages/features/calendar-subscription/lib/cache/tests/CalendarCacheEventService.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/features/calendar-subscription/lib/cache/CalendarCacheWrapper.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.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/features/calendar-subscription/lib/cache/CalendarCacheWrapper.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
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#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
📚 Learning: 2025-08-05T12:04:29.037Z
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.
Applied to files:
packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts
📚 Learning: 2025-09-08T07:27:42.903Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.903Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
Applied to files:
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts
🧬 Code graph analysis (1)
packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts (2)
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts (1)
ICalendarCacheEventRepository(6-41)packages/types/Calendar.d.ts (5)
CalendarServiceEvent(261-263)NewCalendarEventType(71-84)CalendarEvent(163-226)IntegrationCalendar(245-254)SelectedCalendarEventTypeIds(259-259)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| getCredentialId?(): number { | ||
| return this.deps.originalCalendar.getCredentialId ? this.deps.originalCalendar.getCredentialId() : -1; | ||
| } |
There was a problem hiding this comment.
Avoid sentinel -1 for credential id
Returning -1 can leak into downstream logic as a “valid number.” Either delegate when available or omit this optional method to mirror the underlying calendar.
- getCredentialId?(): number {
- return this.deps.originalCalendar.getCredentialId ? this.deps.originalCalendar.getCredentialId() : -1;
- }
+ // Omit this optional method and rely on the underlying calendar's presence.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts
around lines 28-30, the wrapper currently returns a sentinel -1 from
getCredentialId which can be mistaken for a valid id; instead either remove this
optional method from the wrapper so it mirrors the underlying originalCalendar
(no method when original lacks it) or delegate directly and return undefined
when originalCalendar.getCredentialId is absent; update type signature if
necessary to reflect optional/undefined return and remove the -1 fallback.
| async getAvailability( | ||
| dateFrom: string, | ||
| dateTo: string, | ||
| selectedCalendars: IntegrationCalendar[], | ||
| shouldServeCache?: boolean | ||
| // _fallbackToPrimary?: boolean | ||
| ): Promise<EventBusyDate[]> { | ||
| if (!shouldServeCache) { | ||
| return this.deps.originalCalendar.getAvailability(dateFrom, dateTo, selectedCalendars); | ||
| } | ||
|
|
||
| log.debug("getAvailability from cache", { dateFrom, dateTo, selectedCalendars }); | ||
| const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id)); | ||
| if (!selectedCalendarIds.length) { | ||
| return Promise.resolve([]); | ||
| } | ||
| return this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween( | ||
| selectedCalendarIds, | ||
| new Date(dateFrom), | ||
| new Date(dateTo) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Align getAvailability return shape with EventBusyDate
Currently returns repository rows (start, end, timeZone). Map to EventBusyDate to match the Calendar contract.
- return this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
+ const rows = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
selectedCalendarIds,
new Date(dateFrom),
new Date(dateTo)
- );
+ );
+ // If EventBusyDate expects strings, convert to ISO here.
+ return rows.map(({ start, end }) => ({ start, end })) as EventBusyDate[];Please verify the exact EventBusyDate type and adjust to string/Date as required:
🏁 Script executed:
#!/bin/bash
# Inspect EventBusyDate definition and call sites
rg -nP --type=ts -C2 '\b(type|interface)\s+EventBusyDate\b'
rg -nP --type=ts -C2 '\bEventBusyDate\b'Length of output: 35408
🏁 Script executed:
#!/bin/bash
# Show CalendarCacheWrapper.ts snippet for context
sed -n '1,200p' packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts
# Find repository method definition
rg -nP 'findAllBySelectedCalendarIdsBetween' -C3 -n packages/features/calendar-subscription/lib/cacheLength of output: 9848
Align getAvailability output to EventBusyDate
Replace the raw cache return with an awaited call and map to { start, end }:
const rows = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
selectedCalendarIds,
new Date(dateFrom),
new Date(dateTo)
);
return rows.map(({ start, end }) => ({ start, end }));EventBusyDate is defined as { start: Date | string; end: Date | string }.
🤖 Prompt for AI Agents
In packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts
around lines 60 to 81, the method currently returns the raw repository result
instead of an awaited and typed EventBusyDate array; update the code to await
findAllBySelectedCalendarIdsBetween, assign to rows, and return rows.map(({
start, end }) => ({ start, end })) so the output matches EventBusyDate ({ start:
Date | string; end: Date | string }) and preserves async flow.
| async getAvailability( | ||
| dateFrom: string, | ||
| dateTo: string, | ||
| selectedCalendars: IntegrationCalendar[], | ||
| shouldServeCache?: boolean | ||
| // _fallbackToPrimary?: boolean | ||
| ): Promise<EventBusyDate[]> { | ||
| if (!shouldServeCache) { | ||
| return this.deps.originalCalendar.getAvailability(dateFrom, dateTo, selectedCalendars); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm Calendar.getAvailability signature compatibility
You introduce shouldServeCache as the 4th param. Ensure this matches the Calendar interface in packages/types/Calendar.d.ts; otherwise, consider moving this flag to the wrapper constructor or a feature flag check inside.
🏁 Script executed:
#!/bin/bash
# Verify Calendar.getAvailability signature
rg -nP --type=ts -C2 'interface\s+Calendar\b'
rg -nP --type=ts -C2 '\bgetAvailability\s*\('Length of output: 16370
🏁 Script executed:
#!/bin/bash
# Print signature of getAvailability in Calendar interface
sed -n '275,295p' packages/types/Calendar.d.tsLength of output: 730
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.tsLength of output: 4337
Align getAvailability signature with Calendar interface
CalendarCacheWrapper.getAvailability is missing the optional fallbackToPrimary?: boolean parameter declared in the Calendar interface; update its signature to include and forward fallbackToPrimary to originalCalendar.getAvailability.
🤖 Prompt for AI Agents
In packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts
around lines 60 to 69, the getAvailability method signature is missing the
optional fallbackToPrimary?: boolean parameter declared in the Calendar
interface; update the method signature to include fallbackToPrimary?: boolean
and ensure all internal calls to this.deps.originalCalendar.getAvailability pass
through dateFrom, dateTo, selectedCalendars, shouldServeCache (if applicable)
and fallbackToPrimary so the originalCalendar receives the fallback flag
unchanged.
| return this.deps.originalCalendar.getAvailability(dateFrom, dateTo, selectedCalendars); | ||
| } | ||
|
|
||
| log.debug("getAvailability from cache", { dateFrom, dateTo, selectedCalendars }); |
There was a problem hiding this comment.
Reduce PII/noise in logs: log only IDs, not full selectedCalendars
selectedCalendars likely contain emails and other PII; avoid logging the whole objects. Log derived IDs and counts instead.
- log.debug("getAvailability from cache", { dateFrom, dateTo, selectedCalendars });
- const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
+ const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
+ log.debug("getAvailability from cache", {
+ dateFrom,
+ dateTo,
+ selectedCalendarIdCount: selectedCalendarIds.length,
+ selectedCalendarIds,
+ });- log.debug("getAvailabilityWithTimeZones from cache", { dateFrom, dateTo, selectedCalendars });
- const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
+ const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
+ log.debug("getAvailabilityWithTimeZones from cache", {
+ dateFrom,
+ dateTo,
+ selectedCalendarIdCount: selectedCalendarIds.length,
+ selectedCalendarIds,
+ });Also applies to: 97-97
🤖 Prompt for AI Agents
In packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts
around lines 71 and 97, the debug calls currently log full selectedCalendars
(which may contain PII); change them to log only non-PII derived data: e.g., map
selectedCalendars to an array of IDs (or a count if IDs unavailable) and include
that plus dateFrom/dateTo in the log. Ensure you replace the objects with
something like selectedCalendarIds and/or selectedCalendars.length, and keep the
log keys consistent and minimal.
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
🥼 - Let's get this out there! 🔥
🎯 Overview
This PR introduces a comprehensive Calendar Cache and Synchronization system that significantly improves calendar performance and reliability by implementing local caching and real-time sync capabilities for external calendar integrations.
How it works
✨ Key Features
📦 Calendar Event Caching
CalendarCacheWrapper🔄 Real-time Synchronization
🏗️ Architecture
New Database Schema
Service Layer
CalendarSubscriptionService: Manages calendar subscriptions and webhook handlingCalendarCacheEventService: Handles event caching operationsCalendarSyncService: Manages synchronization logicCalendarCacheWrapper: Transparent caching layer for existing Calendar interfaceProvider Adapters
GoogleCalendarSubscriptionAdapter: Google Calendar API integrationOffice365CalendarSubscriptionAdapter: Microsoft Graph API integrationAdaptersFactory: Factory pattern for provider selection🚀 Implementation Details
Feature Flags
Cache Integration
The system integrates transparently with existing calendar operations:
Webhook Handling
X-Goog-Channel-TokenandX-Goog-Channel-IDheaders🔧 Configuration
Environment Variables
GOOGLE_WEBHOOK_TOKEN: Token for validating Google Calendar webhooksGOOGLE_WEBHOOK_URL: Endpoint URL for Google Calendar notificationsRepository Pattern
SelectedCalendarRepository: Data access for calendar selectionsCalendarCacheEventRepository: Data access for cached events📊 Benefits
🚧 Current Status (WIP)
This is a work-in-progress implementation that includes:
🧪 Testing
The implementation follows the existing Cal.com patterns and should be thoroughly tested with:
This PR lays the foundation for a robust calendar caching and synchronization system that will significantly improve user experience and system performance for calendar operations.