Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces a calendar subscription system. It adds provider-agnostic interfaces, adapters for Google and Microsoft (Office 365), an AdapterFactory, and a CalendarSubscriptionService orchestrating subscribe/unsubscribe, webhook processing, and event handling. It adds cache and sync services, repositories, and Prisma schema/migration for SelectedCalendar fields and a new CalendarCacheEvent model. A Next.js App Router POST endpoint processes webhooks per provider, with comprehensive unit and integration tests. New feature flags and environment variables are added. Minor updates include removing a cron tester call and adjusting VS Code spell checking. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
github-actions
bot
added
❗️ migrations
| "typescript.preferences.importModuleSpecifier": "non-relative", | ||
| "spellright.language": ["en"], | ||
| "spellright.documentTypes": ["markdown", "typescript", "typescriptreact"], | ||
| "spellright.documentTypes": ["markdown", "typescriptreact"], |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
- Add unit tests for CalendarSubscriptionService with webhook processing - Add unit tests for CalendarCacheEventService and Repository - Add unit tests for Google and Microsoft calendar adapters - Add integration tests for webhook processing flow - Add tests for webhook API route handlers - Mock external API calls and database operations using prismock and vi.fn() - Cover error scenarios and edge cases for all components - Fix event filtering in adapters to exclude Cal.com events - Replace all 'any' types with proper TypeScript types for lint compliance - All 62 tests passing with comprehensive coverage Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
♻️ Duplicate comments (1)
.vscode/settings.json (1)
9-9: Spellright scope change looks good.Dropping "typescript" aligns with the prior request. Confirm this is intentional so .ts files aren’t spell‑checked.
🧹 Nitpick comments (30)
apps/web/cron-tester.ts (1)
25-25: Guard against overlapping runs and hung fetches.CronJob doesn’t await your async callback; overlapping executions and indefinite fetch hangs are possible. Add a reentrancy guard and per‑request timeout.
- await Promise.allSettled([fetchCron("/calendar-cache/cron"), fetchCron("/tasks/cron")]); + if (running) { + console.warn("⏭️ Previous cron still running, skipping this tick"); + return; + } + running = true; + try { + await Promise.allSettled([fetchCron("/calendar-cache/cron"), fetchCron("/tasks/cron")]); + } finally { + running = false; + }Outside this hunk, update helpers:
// top-level let running = false; async function fetchCron(endpoint: string) { const apiKey = process.env.CRON_API_KEY; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 10_000); try { const res = await fetch(`http://localhost:3000/api${endpoint}?apiKey=${apiKey}`, { headers: { "Content-Type": "application/json", authorization: `Bearer ${process.env.CRON_SECRET}` }, signal: controller.signal, }); const body = res.headers.get("content-type")?.includes("application/json") ? await res.json() : await res.text(); console.log(endpoint, res.status, body); } finally { clearTimeout(timeout); } }packages/features/calendar-subscription/README.md (1)
1-10: Fill in README with flag names, routes, and setup steps.Add: feature flags (calendar-sync, calendar-subscription-cache, calendar-subscription-sync), required env vars, webhook endpoints per provider, subscription lifecycle, and local dev tunneling instructions.
.env.example (1)
142-149: Tighten formatting and clarify secrets guidance.Remove the extra blank line (dotenv-linter warning) and keep sections contiguous.
GOOGLE_WEBHOOK_URL= - # - MICROSOFT WEBHOOKS *****************************************************************************************Also add a brief “Do not commit real tokens” note under both webhook sections.
Also applies to: 151-159
packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts (1)
28-39: Make cal.com domain filter configurable and gate execution by flag.Parameterize "@cal.com" (env/config) and ensure callers gate handleEvents behind "calendar-subscription-sync".
- const calEvents = calendarSubscriptionEvents.filter((e) => - e.iCalUID?.toLowerCase()?.endsWith("@cal.com") - ); + const domain = process.env.CAL_ICAL_DOMAIN?.toLowerCase() ?? "@cal.com"; + const calEvents = calendarSubscriptionEvents.filter((e) => + e.iCalUID?.toLowerCase()?.endsWith(domain) + );packages/app-store/office365calendar/zod.ts (1)
13-31: clientState verification present; recommend parsing subscriptionExpirationDateTime to Date
- clientState is validated against MICROSOFT_WEBHOOK_TOKEN in packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts (handler ~lines 74–95; subscription creation at line 129). Tests include clientState (apps/web/app/api/webhooks/calendar/[provider]/route.test.ts, packages/features/calendar-subscription/lib/integration.test.ts).
- Keep clientState optional in the zod schema (Graph may omit it). Optionally transform/parse subscriptionExpirationDateTime to a Date upstream (zod transform or parse in the handler) before downstream use.
packages/prisma/schema.prisma (2)
2563-2603: Add query‑pattern indexes for cache reads and incremental sync.Typical access patterns: by selectedCalendarId + time range and by selectedCalendarId + externalUpdatedAt.
@@ @@unique([selectedCalendarId, externalId]) @@index([start, end, status]) @@index([selectedCalendarId, iCalUID]) + @@index([selectedCalendarId, start]) + @@index([selectedCalendarId, externalUpdatedAt])
2568-2570: Make externalEtag nullable (some providers don’t guarantee ETag).If adapters can’t always supply ETag, make it optional to avoid failed upserts.
- externalEtag String + externalEtag String?packages/features/calendar-subscription/lib/integration.test.ts (3)
30-43: Assert feature flag expectations with explicit boolean casts.Minor: vi.fn().mockResolvedValueOnce(true).mockResolvedValueOnce(false) is fine; consider asserting call order with toHaveBeenNthCalledWith for clarity.
Also applies to: 217-247, 331-360
30-42: Optional: tighten repository mock types.If SelectedCalendarRepository is an interface, cast mocks with vi.mocked to get type‑safe spies.
51-71: DRY test setup with a factory for service/adapters.Reduce duplication between Google/Microsoft suites.
Also applies to: 177-185, 293-301
apps/web/app/api/webhooks/calendar/[provider]/route.ts (1)
16-26: Docstring promises 400 for unsupported provider, but code never returns it.Covered by provider allowlist fix above.
packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
1-7: Consistent naming: Microsoft vs Office365.Adapters are named Office365* here; tests import Microsoft*. Pick one convention and stick with it to avoid confusion.
apps/web/app/api/webhooks/calendar/[provider]/route.test.ts (1)
74-74: Add GET validation handshake test for Microsoft GraphGraph sends a GET with
validationTokenduring subscription creation. Please add a test ensuring the route echoes the token with 200 and proper headers.I can draft the test once the GET handler is in place.
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts (1)
11-17: Clarify upsert contract and required keysThe interface says “upsert” with
Partial<CalendarCacheEvent>, but the implementation usescreateManywith full entities. Define the required unique keys (likelyexternalId+selectedCalendarId) and ensure the implementation truly upserts.Would you like me to align the implementation to this contract using transactional per-row upserts?
packages/lib/server/repository/SelectedCalendarRepository.ts (2)
20-25: Consider scoping “not subscribed” to enabled calendarsRight now it pulls all with
syncSubscribedAt: null. IfsyncEnabled/cacheEnabledflags exist, filter here to avoid unnecessary subscribe attempts.- return await this.prismaClient.selectedCalendar.findMany({ - where: { syncSubscribedAt: null }, + return await this.prismaClient.selectedCalendar.findMany({ + where: { syncSubscribedAt: null, syncEnabled: true }, take, });
27-29: Index/channel uniquenessEnsure
channelIdhas an index/unique constraint to keepfindFirst({ where: { channelId } })fast and unambiguous.packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (3)
48-56: Reset error state on successful subscribeAfter a successful subscribe, clear previous sync errors for a clean slate.
await this.deps.selectedCalendarRepository.updateById(selectedCalendarId, { channelId: res?.id, channelResourceId: res?.resourceId, channelResourceUri: res?.resourceUri, channelKind: res?.provider, channelExpiration: res?.expiration, syncSubscribedAt: new Date(), + syncErrorAt: null, + syncErrorCount: 0, });
132-141: Inject cache repository instead of constructing with global prismaDI here improves testability and avoids hard-coding a global client. Pass
calendarCacheEventRepositoryvia ctor deps; the route can wire Prisma-backed impl.If you want, I can provide a small refactor patch across service wiring and route.
100-103: Log message clarityThis branch also hits when features are disabled. Tweak the log to avoid “not found” confusion.
- log.debug("Selected calendar not found", { channelId }); + log.debug("Calendar disabled or missing credential; skipping", { channelId, cacheEnabled, syncEnabled });packages/features/calendar-subscription/lib/cache/CalendarCacheEventService.ts (2)
53-53: Fix inconsistent field mapping between event and cache model.The code maps
event.originalStartDatetooriginalStartTime, but the field name suggests this should beevent.originalStartTimefor consistency.Apply this diff to fix the field mapping:
- originalStartTime: event.originalStartDate, + originalStartTime: event.originalStartTime,
70-73: Consider error handling for Promise.allSettled results.Using
Promise.allSettledis good for resilience, but the code doesn't check if operations failed. Database errors could silently fail without proper monitoring or recovery.Consider logging failures and potentially implementing retry logic:
- await Promise.allSettled([ - this.deps.calendarCacheEventRepository.deleteMany(toDelete), - this.deps.calendarCacheEventRepository.upsertMany(toUpsert), - ]); + const results = await Promise.allSettled([ + this.deps.calendarCacheEventRepository.deleteMany(toDelete), + this.deps.calendarCacheEventRepository.upsertMany(toUpsert), + ]); + + results.forEach((result, index) => { + if (result.status === 'rejected') { + const operation = index === 0 ? 'deleteMany' : 'upsertMany'; + log.error(`Failed to ${operation} cache events`, { + error: result.reason, + count: index === 0 ? toDelete.length : toUpsert.length + }); + } + });packages/app-store/googlecalendar/api/subscriptions/webhook.handler.ts (2)
56-56: Remove unnecessary line.Line 56 contains just
selectedCalendar;which serves no purpose.- selectedCalendar;
92-107: Consider stricter data validation with dedicated error type.The data inconsistency check between
SelectedCalendar.externalIdandCalendarSync.externalCalendarIdthrows a generic Error. Consider using a specific error type for better error handling downstream.+class DataInconsistencyError extends Error { + constructor(message: string, public context: Record<string, unknown>) { + super(message); + this.name = "DataInconsistencyError"; + } +} if (externalCalendarId && externalCalendarId !== calendarSync.externalCalendarId) { log.error( "Data inconsistency: Selected calendar externalId and Synced calendar externalId do not match for the same subscription.", safeStringify({ channelId, resourceId, selectedExternalId: externalCalendarId, syncedExternalId: calendarSync.externalCalendarId, selectedCalendarId: selectedCalendar?.id, calendarSyncId: calendarSync.id, }) ); - throw new Error( - "Data inconsistency: SelectedCalendar.externalId and CalendarSync.externalCalendarId do not match for the same subscription." + throw new DataInconsistencyError( + "Data inconsistency: SelectedCalendar.externalId and CalendarSync.externalCalendarId do not match for the same subscription.", + { + channelId, + resourceId, + selectedExternalId: externalCalendarId, + syncedExternalId: calendarSync.externalCalendarId, + } ); }packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts (1)
56-56: Remove unnecessary standalone statement.Line 56 contains a standalone
selectedCalendar;statement that serves no purpose.log.debug("Attempt to subscribe to Google Calendar", { externalId: selectedCalendar.externalId }); - selectedCalendar;packages/prisma/migrations/20250915124730_add_calendar_sync_and_cache/migration.sql (1)
18-18: Consider making externalEtag nullable.The
externalEtagfield is defined asNOT NULL, but not all calendar providers may provide ETags. Consider making it nullable to support provider flexibility.- "externalEtag" TEXT NOT NULL, + "externalEtag" TEXT,packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts (5)
56-57: Translation needed for Portuguese comment.The comment on line 56 is in Portuguese. For better maintainability and team collaboration, use English for all code comments.
- // duração padrão ~3 dias (Graph ~4230 min) + // default duration ~3 days (Graph ~4230 min)
144-150: Missing error handling for subscription deletion.The
unsubscribemethod should handle cases where the subscription deletion fails (e.g., already deleted, network issues). Currently, any error will bubble up uncaught.async unsubscribe(selectedCalendar: SelectedCalendar, credential: CalendarCredential): Promise<void> { const subId = selectedCalendar.channelResourceId; if (!subId) return; const client = await this.getGraphClient(credential); - await this.request(client, "DELETE", `/subscriptions/${subId}`); + try { + await this.request(client, "DELETE", `/subscriptions/${subId}`); + } catch (error: any) { + // 404 means subscription already deleted, which is acceptable + if (error.message?.includes("404")) { + log.info("Subscription already deleted", { subId }); + return; + } + throw error; + } }
241-245: Sensitive data potentially logged in error messages.The error logging on line 243 logs the full response text which might contain sensitive information like PII or authentication details. Consider sanitizing or limiting the logged content.
if (!res.ok) { const text = await res.text().catch(() => ""); - log.error("Graph API error", { method, endpoint: this.stripBase(url), status: res.status, text }); + // Limit logged text to prevent sensitive data exposure + const truncatedText = text.length > 200 ? text.substring(0, 200) + "..." : text; + log.error("Graph API error", { + method, + endpoint: this.stripBase(url), + status: res.status, + text: truncatedText + }); throw new Error(`Graph ${res.status} ${res.statusText}`); }
234-237: Add request timeout configuration.The fetch request lacks a timeout configuration, which could lead to hanging requests if the Microsoft Graph API is slow or unresponsive.
const init: RequestInit = { method, headers }; if (data && (method === "POST" || method === "PUT" || method === "PATCH")) { init.body = JSON.stringify(data); } + +// Add timeout using AbortController +const controller = new AbortController(); +const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout +init.signal = controller.signal; -const res = await fetch(url, init); +try { + const res = await fetch(url, init); + clearTimeout(timeoutId); + + if (!res.ok) { + const text = await res.text().catch(() => ""); + const truncatedText = text.length > 200 ? text.substring(0, 200) + "..." : text; + log.error("Graph API error", { + method, + endpoint: this.stripBase(url), + status: res.status, + text: truncatedText + }); + throw new Error(`Graph ${res.status} ${res.statusText}`); + } + + if (method === "DELETE" || res.status === 204) return {} as T; + + return (await res.json()) as T; +} catch (error: any) { + clearTimeout(timeoutId); + if (error.name === 'AbortError') { + throw new Error(`Graph API request timeout: ${this.stripBase(url)}`); + } + throw error; +}
73-77: Validate environment variables on initialization.The constructor should validate that required environment variables are present and properly formatted to fail fast rather than at runtime.
constructor(cfg: AdapterConfig = {}) { this.baseUrl = cfg.baseUrl ?? "https://graph.microsoft.com/v1.0"; this.webhookToken = cfg.webhookToken ?? process.env.MICROSOFT_WEBHOOK_TOKEN ?? null; this.webhookUrl = cfg.webhookUrl ?? process.env.MICROSOFT_WEBHOOK_URL ?? null; this.subscriptionTtlMs = cfg.subscriptionTtlMs ?? 3 * 24 * 60 * 60 * 1000; + + // Validate webhook URL format if provided + if (this.webhookUrl) { + try { + new URL(this.webhookUrl); + } catch (error) { + throw new Error(`Invalid MICROSOFT_WEBHOOK_URL format: ${this.webhookUrl}`); + } + } }
📜 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 (24)
.env.example(1 hunks).vscode/settings.json(1 hunks)apps/web/app/api/webhooks/calendar/[provider]/route.test.ts(1 hunks)apps/web/app/api/webhooks/calendar/[provider]/route.ts(1 hunks)apps/web/cron-tester.ts(1 hunks)packages/app-store/googlecalendar/api/subscriptions/webhook.handler.ts(1 hunks)packages/app-store/office365calendar/zod.ts(1 hunks)packages/features/calendar-subscription/README.md(1 hunks)packages/features/calendar-subscription/adapters/AdaptersFactory.ts(1 hunks)packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts(1 hunks)packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts(1 hunks)packages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.ts(1 hunks)packages/features/calendar-subscription/lib/CalendarSubscriptionPort.interface.ts(1 hunks)packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts(1 hunks)packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.ts(1 hunks)packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.ts(1 hunks)packages/features/calendar-subscription/lib/cache/CalendarCacheEventService.ts(1 hunks)packages/features/calendar-subscription/lib/integration.test.ts(1 hunks)packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts(1 hunks)packages/features/flags/config.ts(2 hunks)packages/lib/server/repository/SelectedCalendarRepository.ts(1 hunks)packages/prisma/migrations/20250915124730_add_calendar_sync_and_cache/migration.sql(1 hunks)packages/prisma/schema.prisma(4 hunks)turbo.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
apps/web/app/api/webhooks/calendar/[provider]/route.test.tspackages/features/calendar-subscription/lib/integration.test.tsapps/web/cron-tester.tspackages/features/calendar-subscription/lib/sync/CalendarSyncService.tspackages/features/calendar-subscription/adapters/AdaptersFactory.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.interface.tspackages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.tspackages/lib/server/repository/SelectedCalendarRepository.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.tspackages/features/flags/config.tsapps/web/app/api/webhooks/calendar/[provider]/route.tspackages/features/calendar-subscription/lib/CalendarSubscriptionPort.interface.tspackages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.tspackages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.tspackages/app-store/office365calendar/zod.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventService.tspackages/app-store/googlecalendar/api/subscriptions/webhook.handler.ts
🧠 Learnings (10)
📓 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.
📚 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:
apps/web/cron-tester.tspackages/features/calendar-subscription/lib/sync/CalendarSyncService.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.tspackages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventService.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.tspackages/lib/server/repository/SelectedCalendarRepository.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.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/adapters/Office365CalendarSubscription.adapter.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.tspackages/app-store/office365calendar/zod.tspackages/features/calendar-subscription/lib/cache/CalendarCacheEventService.tspackages/app-store/googlecalendar/api/subscriptions/webhook.handler.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-09-09T03:29:43.025Z
Learnt from: emrysal
PR: calcom/cal.com#23692
File: packages/lib/server/service/InsightsBookingBaseService.ts:16-16
Timestamp: 2025-09-09T03:29:43.025Z
Learning: In the Cal.com codebase, readonlyPrisma is still an instance of PrismaClient, making type changes from `typeof readonlyPrisma` to `PrismaClient` less critical since they are fundamentally compatible types.
Applied to files:
packages/lib/server/repository/SelectedCalendarRepository.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/app-store/office365calendar/zod.ts
📚 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
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.
Applied to files:
packages/prisma/schema.prisma
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 150-150: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
⏰ 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 (12)
packages/features/calendar-subscription/lib/sync/CalendarSyncService.ts (1)
1-1: Verify import source for CalendarSubscriptionEventItem.Confirm the type is exported from CalendarSubscriptionPort.interface (or re-exported). If it lives in CalendarSubscriptionFeatures.interface, update the import to match.
packages/features/flags/config.ts (2)
29-31: Add defaults and docs for new flags.Found declarations/usage in:
- packages/features/flags/config.ts
- packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (feature checks)
- packages/features/calendar-subscription/lib/integration.test.ts
No seed entries or admin UI toggle / access-check registrations were found — add default seed values, register UI toggles (if applicable), and document each flag's purpose/expected behavior.
8-8: ```shell
#!/bin/bash
set -euo pipefailecho "rg matches for 'calendar' or 'calendar-sync' (context lines):"
rg -n --hidden --no-ignore -S -C3 'calendar-sync|calendar' || true
echo
echo "=== packages/features/flags/config.ts ==="
nl -ba packages/features/flags/config.ts || true
echo
echo "=== packages/features/flags/hooks/index.ts ==="
nl -ba packages/features/flags/hooks/index.ts || true</blockquote></details> <details> <summary>packages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.ts (1)</summary><blockquote> `82-86`: **Align credential type for fetchEvents with subscribe/unsubscribe.** subscribe/unsubscribe use CalendarCredential (includes email), but fetchEvents expects CredentialForCalendarService. If adapters need email for delta sync, make this consistent or document the divergence. Option if consistency desired: ```diff - fetchEvents( - selectedCalendar: SelectedCalendar, - credential: CredentialForCalendarService - ): Promise<CalendarSubscriptionEvent>; + fetchEvents( + selectedCalendar: SelectedCalendar, + credential: CalendarCredential + ): Promise<CalendarSubscriptionEvent>;packages/prisma/schema.prisma (2)
928-934: Schema vs service/test field naming mismatch.Schema uses syncToken/syncedAt/syncErrorAt/syncErrorCount but tests/repository stubs use lastSyncToken/lastSyncedAt/lastSyncErrorAt/syncErrorCount. Align naming or map in the repository DTO.
965-965: Confirm change from @id to @unique on BookingTimeStatusDenormalized.Ensure no code relies on @id semantics; Prisma still needs a unique field for views, but switching can affect findById helpers.
packages/features/calendar-subscription/lib/integration.test.ts (2)
186-196: Microsoft validation flow likely uses validationToken in query and requires plaintext echo.Current test places token in body and expects JSON 200. Consider aligning with Graph’s handshake (token in query, 200 with token as text). Adjust tests after route fix.
82-99: SelectedCalendar stub fields don’t match schema.Props like cacheEnabled, lastSyncToken, lastSyncedAt don’t exist in SelectedCalendar schema. Either extend repository DTO or update stubs.
Also applies to: 134-151, 198-215, 250-267, 312-329
apps/web/app/api/webhooks/calendar/[provider]/route.test.ts (1)
62-72: Confirmcontext.paramsshapeYou’re passing
paramsas a Promise. In Next.js App Router handlers,paramsis typically a plain object. Verify the route’s signature and adjust test scaffolding if necessary.packages/prisma/migrations/20250915124730_add_calendar_sync_and_cache/migration.sql (1)
40-46: Well-designed indexes for query performance.The indexes are well-chosen for the expected query patterns:
- Composite index on (start, end, status) for time-range and status filtering
- Index on (selectedCalendarId, iCalUID) for calendar-specific lookups
- Unique constraint on (selectedCalendarId, externalId) prevents duplicate events
packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts (2)
1-11: Consider using a named export instead of default export pattern.While this file uses a named export (which is good), ensure consistency across the codebase by avoiding default exports in other adapter files.
122-122: No change required — toISOString() matches Microsoft Graph's RFC3339 UTC requirement.Microsoft Graph subscription expirationDateTime must be an RFC3339 UTC timestamp (include the trailing "Z"); Date.prototype.toISOString() produces that format, so this line is correct.
| status: event.status, | ||
| isAllDay: !!event.start?.date && !event.start?.dateTime, | ||
| timeZone: event.start?.timeZone || null, | ||
| originalStartTime: event.originalStartTime?.dateTime, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Handle undefined originalStartTime gracefully.
The code references event.originalStartTime?.dateTime but this field might be originalStartDate based on the event structure.
🏁 Script executed:
#!/bin/bash
# Check the Google Calendar API Event schema fields for originalStartTime
rg -A 3 -B 3 "originalStart" --type tsLength of output: 7148
Fix originalStartDate mapping in GoogleCalendarSubscription.adapter.ts
Adapter currently sets originalStartTime: event.originalStartTime?.dateTime, but the internal model and cache use originalStartDate?: Date | null. Replace the line:
originalStartTime: event.originalStartTime?.dateTime,
with:
originalStartDate: event.originalStartTime ? new Date(event.originalStartTime.dateTime ?? event.originalStartTime.date) : null
Files to check: packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts, packages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.ts, packages/features/calendar-subscription/lib/cache/CalendarCacheEventService.ts.
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
around line 163, the adapter incorrectly maps originalStartTime to
event.originalStartTime?.dateTime while the internal model/cache expect
originalStartDate?: Date | null; replace that property mapping to set
originalStartDate to a Date when event.originalStartTime exists (using
event.originalStartTime.dateTime if present, otherwise
event.originalStartTime.date) and to null when it does not; update the adapter
to construct new Date(...) accordingly and then verify and update the
types/usages in
packages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.ts
and
packages/features/calendar-subscription/lib/cache/CalendarCacheEventService.ts
so they reference originalStartDate?: Date | null (not originalStartTime) and
handle null safely.
| }; | ||
| }) | ||
| .filter((e) => !!e.id) // safely remove events with no ID | ||
| .filter((e) => e.start < now); // remove old events |
There was a problem hiding this comment.
Fix filter logic for future events.
The comment says "remove old events" but the filter e.start < now actually removes future events, keeping only past events. This appears to be inverted logic.
})
.filter((e) => !!e.id) // safely remove events with no ID
- .filter((e) => e.start < now); // remove old events
+ .filter((e) => e.start > now); // remove past events, keep future eventsIf the intention is actually to keep past events for caching historical data, then update the comment:
- .filter((e) => e.start < now); // remove old events
+ .filter((e) => e.start < now); // keep only past events for historical cache📝 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.
| .filter((e) => e.start < now); // remove old events | |
| }) | |
| .filter((e) => !!e.id) // safely remove events with no ID | |
| .filter((e) => e.start > now); // remove past events, keep future events |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/adapters/GoogleCalendarSubscription.adapter.ts
around line 169, the filter predicate and comment are inverted: the code
currently uses .filter((e) => e.start < now) but the comment says "remove old
events" while that predicate actually keeps only past events and removes future
ones; fix by changing the predicate to .filter((e) => e.start >= now) to keep
future events (thus removing old ones) and ensure the comment matches ("remove
old events / keep future events"); alternatively, if the intent was to keep past
events for caching, update the comment to reflect that instead of changing the
predicate.
| async validate(context: CalendarSubscriptionWebhookContext): Promise<boolean> { | ||
| // validate handshake | ||
| const validationToken = context?.query?.get("validationToken"); | ||
| if (validationToken) return true; | ||
|
|
||
| // validate notifications | ||
| const clientState = | ||
| context?.headers?.get("clientState") ?? | ||
| (typeof context?.body === "object" && context.body !== null && "clientState" in context.body | ||
| ? (context.body as { clientState?: string }).clientState | ||
| : undefined); | ||
| if (!this.webhookToken) { | ||
| log.warn("MICROSOFT_WEBHOOK_TOKEN missing"); | ||
| return false; | ||
| } | ||
| if (clientState !== this.webhookToken) { | ||
| log.warn("Invalid clientState", { clientState }); | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Security concern: Webhook validation timing attack vulnerability.
The string comparison clientState !== this.webhookToken on line 94 is vulnerable to timing attacks. Use a constant-time comparison function to prevent attackers from inferring the token through response timing analysis.
+import { timingSafeEqual } from "crypto";
+
async validate(context: CalendarSubscriptionWebhookContext): Promise<boolean> {
// validate handshake
const validationToken = context?.query?.get("validationToken");
if (validationToken) return true;
// validate notifications
const clientState =
context?.headers?.get("clientState") ??
(typeof context?.body === "object" && context.body !== null && "clientState" in context.body
? (context.body as { clientState?: string }).clientState
: undefined);
if (!this.webhookToken) {
log.warn("MICROSOFT_WEBHOOK_TOKEN missing");
return false;
}
- if (clientState !== this.webhookToken) {
+ // Use constant-time comparison to prevent timing attacks
+ const clientStateBuffer = Buffer.from(clientState || "");
+ const webhookTokenBuffer = Buffer.from(this.webhookToken);
+ if (clientStateBuffer.length !== webhookTokenBuffer.length ||
+ !timingSafeEqual(clientStateBuffer, webhookTokenBuffer)) {
log.warn("Invalid clientState", { clientState });
return false;
}
return true;
}📝 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.
| async validate(context: CalendarSubscriptionWebhookContext): Promise<boolean> { | |
| // validate handshake | |
| const validationToken = context?.query?.get("validationToken"); | |
| if (validationToken) return true; | |
| // validate notifications | |
| const clientState = | |
| context?.headers?.get("clientState") ?? | |
| (typeof context?.body === "object" && context.body !== null && "clientState" in context.body | |
| ? (context.body as { clientState?: string }).clientState | |
| : undefined); | |
| if (!this.webhookToken) { | |
| log.warn("MICROSOFT_WEBHOOK_TOKEN missing"); | |
| return false; | |
| } | |
| if (clientState !== this.webhookToken) { | |
| log.warn("Invalid clientState", { clientState }); | |
| return false; | |
| } | |
| return true; | |
| } | |
| import { timingSafeEqual } from "crypto"; | |
| async validate(context: CalendarSubscriptionWebhookContext): Promise<boolean> { | |
| // validate handshake | |
| const validationToken = context?.query?.get("validationToken"); | |
| if (validationToken) return true; | |
| // validate notifications | |
| const clientState = | |
| context?.headers?.get("clientState") ?? | |
| (typeof context?.body === "object" && context.body !== null && "clientState" in context.body | |
| ? (context.body as { clientState?: string }).clientState | |
| : undefined); | |
| if (!this.webhookToken) { | |
| log.warn("MICROSOFT_WEBHOOK_TOKEN missing"); | |
| return false; | |
| } | |
| // Use constant-time comparison to prevent timing attacks | |
| const clientStateBuffer = Buffer.from(clientState || ""); | |
| const webhookTokenBuffer = Buffer.from(this.webhookToken); | |
| if (clientStateBuffer.length !== webhookTokenBuffer.length || | |
| !timingSafeEqual(clientStateBuffer, webhookTokenBuffer)) { | |
| log.warn("Invalid clientState", { clientState }); | |
| return false; | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts
around lines 79 to 99, the current direct string comparison clientState !==
this.webhookToken is vulnerable to timing attacks; replace it with a
constant-time comparison: ensure both values are non-null, convert them to
Buffers (or Uint8Array) of equal length (or early-fail if lengths differ) and
use crypto.timingSafeEqual (or a vetted constant-time compare) to compare the
buffers, preserving the existing log behavior on mismatch and returning false on
invalid/missing values.
| if (deltaLink) { | ||
| const path = this.stripBase(deltaLink); | ||
| const r = await this.request<MicrosoftGraphEventsResponse>(client, "GET", path); | ||
| items.push(...r.value); | ||
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | ||
| } else { | ||
| let next: string | null = `/me/calendars/${selectedCalendar.externalId}/events/delta`; | ||
| while (next) { | ||
| const r: MicrosoftGraphEventsResponse = await this.request<MicrosoftGraphEventsResponse>( | ||
| client, | ||
| "GET", | ||
| next | ||
| ); | ||
| items.push(...r.value); | ||
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | ||
| next = r["@odata.nextLink"] ? this.stripBase(r["@odata.nextLink"]) : null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential infinite loop risk in delta pagination.
The while loop on line 168 could potentially run indefinitely if the Graph API keeps returning nextLink values. Consider adding a maximum iteration limit to prevent resource exhaustion.
} else {
let next: string | null = `/me/calendars/${selectedCalendar.externalId}/events/delta`;
+ const MAX_ITERATIONS = 100;
+ let iterations = 0;
while (next) {
+ if (++iterations > MAX_ITERATIONS) {
+ log.error("Max iterations reached while fetching delta events", {
+ calendarId: selectedCalendar.externalId
+ });
+ throw new Error("Too many pages in delta query");
+ }
const r: MicrosoftGraphEventsResponse = await this.request<MicrosoftGraphEventsResponse>(
client,
"GET",
next
);
items.push(...r.value);
deltaLink = r["@odata.deltaLink"] ?? deltaLink;
next = r["@odata.nextLink"] ? this.stripBase(r["@odata.nextLink"]) : null;
}
}📝 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 (deltaLink) { | |
| const path = this.stripBase(deltaLink); | |
| const r = await this.request<MicrosoftGraphEventsResponse>(client, "GET", path); | |
| items.push(...r.value); | |
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | |
| } else { | |
| let next: string | null = `/me/calendars/${selectedCalendar.externalId}/events/delta`; | |
| while (next) { | |
| const r: MicrosoftGraphEventsResponse = await this.request<MicrosoftGraphEventsResponse>( | |
| client, | |
| "GET", | |
| next | |
| ); | |
| items.push(...r.value); | |
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | |
| next = r["@odata.nextLink"] ? this.stripBase(r["@odata.nextLink"]) : null; | |
| } | |
| } | |
| if (deltaLink) { | |
| const path = this.stripBase(deltaLink); | |
| const r = await this.request<MicrosoftGraphEventsResponse>(client, "GET", path); | |
| items.push(...r.value); | |
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | |
| } else { | |
| let next: string | null = `/me/calendars/${selectedCalendar.externalId}/events/delta`; | |
| const MAX_ITERATIONS = 100; | |
| let iterations = 0; | |
| while (next) { | |
| if (++iterations > MAX_ITERATIONS) { | |
| log.error("Max iterations reached while fetching delta events", { | |
| calendarId: selectedCalendar.externalId | |
| }); | |
| throw new Error("Too many pages in delta query"); | |
| } | |
| const r: MicrosoftGraphEventsResponse = await this.request<MicrosoftGraphEventsResponse>( | |
| client, | |
| "GET", | |
| next | |
| ); | |
| items.push(...r.value); | |
| deltaLink = r["@odata.deltaLink"] ?? deltaLink; | |
| next = r["@odata.nextLink"] ? this.stripBase(r["@odata.nextLink"]) : null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts
around lines 161 to 178, the while loop that follows nextLink pagination can run
indefinitely if the Graph API continuously returns @odata.nextLink; add a
max-iteration safety guard (e.g., const MAX_DELTA_PAGES = 1000) and a counter
incremented each loop, break the loop (and log a warning or throw a controlled
error) when the counter exceeds the limit to prevent resource exhaustion, and
ensure you still set deltaLink and collect items before breaking so no data is
lost.
| private parseEvents(events: MicrosoftGraphEvent[]): CalendarSubscriptionEventItem[] { | ||
| return events | ||
| .map((e) => { | ||
| const busy = e.showAs === "busy" || e.showAs === "tentative" || e.showAs === "oof"; | ||
| const start = e.start?.dateTime ? new Date(e.start.dateTime) : new Date(); | ||
| const end = e.end?.dateTime ? new Date(e.end.dateTime) : new Date(); | ||
|
|
||
| return { | ||
| id: e.id, | ||
| iCalUID: e.iCalUId ?? e.id, | ||
| start, | ||
| end, | ||
| busy, | ||
| summary: e.subject, | ||
| description: e.bodyPreview, | ||
| location: e.location?.displayName, | ||
| kind: e.type ?? "microsoftgraph#event", | ||
| status: e.isCancelled ? "cancelled" : "confirmed", | ||
| isAllDay: !!e.isAllDay, | ||
| timeZone: e.start?.timeZone ?? null, | ||
| }; | ||
| }) | ||
| .filter((i: CalendarSubscriptionEventItem) => Boolean(i.id)); | ||
| } |
There was a problem hiding this comment.
Handle missing date fields gracefully.
The parseEvents method defaults to new Date() when start/end dates are missing (lines 191-192), which could lead to incorrect event times. Consider filtering out events with missing dates or using a more appropriate default.
private parseEvents(events: MicrosoftGraphEvent[]): CalendarSubscriptionEventItem[] {
return events
- .map((e) => {
+ .filter((e) => e.start?.dateTime && e.end?.dateTime) // Filter out events without dates
+ .map((e) => {
const busy = e.showAs === "busy" || e.showAs === "tentative" || e.showAs === "oof";
- const start = e.start?.dateTime ? new Date(e.start.dateTime) : new Date();
- const end = e.end?.dateTime ? new Date(e.end.dateTime) : new Date();
+ const start = new Date(e.start!.dateTime);
+ const end = new Date(e.end!.dateTime);
return {
id: e.id,
iCalUID: e.iCalUId ?? e.id,
start,
end,
busy,
summary: e.subject,
description: e.bodyPreview,
location: e.location?.displayName,
kind: e.type ?? "microsoftgraph#event",
status: e.isCancelled ? "cancelled" : "confirmed",
isAllDay: !!e.isAllDay,
timeZone: e.start?.timeZone ?? null,
};
})
.filter((i: CalendarSubscriptionEventItem) => Boolean(i.id));
}📝 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.
| private parseEvents(events: MicrosoftGraphEvent[]): CalendarSubscriptionEventItem[] { | |
| return events | |
| .map((e) => { | |
| const busy = e.showAs === "busy" || e.showAs === "tentative" || e.showAs === "oof"; | |
| const start = e.start?.dateTime ? new Date(e.start.dateTime) : new Date(); | |
| const end = e.end?.dateTime ? new Date(e.end.dateTime) : new Date(); | |
| return { | |
| id: e.id, | |
| iCalUID: e.iCalUId ?? e.id, | |
| start, | |
| end, | |
| busy, | |
| summary: e.subject, | |
| description: e.bodyPreview, | |
| location: e.location?.displayName, | |
| kind: e.type ?? "microsoftgraph#event", | |
| status: e.isCancelled ? "cancelled" : "confirmed", | |
| isAllDay: !!e.isAllDay, | |
| timeZone: e.start?.timeZone ?? null, | |
| }; | |
| }) | |
| .filter((i: CalendarSubscriptionEventItem) => Boolean(i.id)); | |
| } | |
| private parseEvents(events: MicrosoftGraphEvent[]): CalendarSubscriptionEventItem[] { | |
| return events | |
| .filter((e) => e.start?.dateTime && e.end?.dateTime) | |
| .map((e) => { | |
| const busy = e.showAs === "busy" || e.showAs === "tentative" || e.showAs === "oof"; | |
| const start = new Date(e.start!.dateTime); | |
| const end = new Date(e.end!.dateTime); | |
| return { | |
| id: e.id, | |
| iCalUID: e.iCalUId ?? e.id, | |
| start, | |
| end, | |
| busy, | |
| summary: e.subject, | |
| description: e.bodyPreview, | |
| location: e.location?.displayName, | |
| kind: e.type ?? "microsoftgraph#event", | |
| status: e.isCancelled ? "cancelled" : "confirmed", | |
| isAllDay: !!e.isAllDay, | |
| timeZone: e.start?.timeZone ?? null, | |
| }; | |
| }) | |
| .filter((i: CalendarSubscriptionEventItem) => Boolean(i.id)); | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/adapters/Office365CalendarSubscription.adapter.ts
around lines 187 to 210, the current parseEvents implementation uses new Date()
as a fallback for missing start/end dateTime which produces incorrect event
times; instead, filter out events that lack valid start.dateTime or end.dateTime
before mapping (or skip them in the map), so only events with both start and end
dateTime are processed; ensure returned items always have real Date objects
(constructed from the dateTime strings) and adjust the final filter to remove
any items missing id or missing valid start/end.
| async upsertMany(events: CalendarCacheEvent[]): Promise<void> { | ||
| this.prismaClient.calendarCacheEvent.createMany({ data: events }); | ||
| } |
There was a problem hiding this comment.
Not actually upserting + missing await
createManyis not an upsert and will fail on conflicts or silently ignore updates.- The promise is not awaited; the method may return before persistence completes.
Apply a real upsert per unique key (assumes a composite unique on externalId + selectedCalendarId) and await:
- async upsertMany(events: CalendarCacheEvent[]): Promise<void> {
- this.prismaClient.calendarCacheEvent.createMany({ data: events });
- }
+ async upsertMany(events: Partial<CalendarCacheEvent>[]): Promise<void> {
+ const items = events.filter(
+ (e): e is Partial<CalendarCacheEvent> & Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId"> =>
+ !!e.externalId && !!e.selectedCalendarId
+ );
+ if (items.length === 0) return;
+ await this.prismaClient.$transaction(
+ items.map((e) =>
+ this.prismaClient.calendarCacheEvent.upsert({
+ where: {
+ externalId_selectedCalendarId: {
+ externalId: e.externalId,
+ selectedCalendarId: e.selectedCalendarId,
+ },
+ },
+ create: e as any,
+ update: e as any,
+ })
+ )
+ );
+ }If a composite unique isn’t present, I can add one in the migration.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.ts
around lines 9 to 11, the method uses createMany (not an upsert) and does not
await the promise; change it to perform a true upsert per unique key (externalId
+ selectedCalendarId) by iterating over events and calling
prismaClient.calendarCacheEvent.upsert for each record (or use
prisma.$transaction with upserts for batch atomicity), include the appropriate
where clause matching the composite unique, provide update and create payloads,
await the operations (e.g., await Promise.all(...) or await
prisma.$transaction(...)), and if the composite unique constraint does not exist
add a migration to create it before relying on upsert.
| async deleteMany(events: Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId">[]): Promise<void> { | ||
| const conditions = events.filter((c) => c.externalId && c.selectedCalendarId); | ||
| if (conditions.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| this.prismaClient.calendarCacheEvent.deleteMany({ | ||
| where: { | ||
| OR: conditions, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Await writes and guard empty input
deleteMany should be awaited; early-return on empty conditions is good.
- this.prismaClient.calendarCacheEvent.deleteMany({
+ await this.prismaClient.calendarCacheEvent.deleteMany({
where: {
OR: conditions,
},
});📝 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.
| async deleteMany(events: Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId">[]): Promise<void> { | |
| const conditions = events.filter((c) => c.externalId && c.selectedCalendarId); | |
| if (conditions.length === 0) { | |
| return; | |
| } | |
| this.prismaClient.calendarCacheEvent.deleteMany({ | |
| where: { | |
| OR: conditions, | |
| }, | |
| }); | |
| } | |
| async deleteMany(events: Pick<CalendarCacheEvent, "externalId" | "selectedCalendarId">[]): Promise<void> { | |
| const conditions = events.filter((c) => c.externalId && c.selectedCalendarId); | |
| if (conditions.length === 0) { | |
| return; | |
| } | |
| await this.prismaClient.calendarCacheEvent.deleteMany({ | |
| where: { | |
| OR: conditions, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.ts
around lines 13 to 24, the call to
this.prismaClient.calendarCacheEvent.deleteMany is not awaited; keep the
existing early-return for empty conditions but await the deleteMany call so the
method actually waits for the DB write to complete (and propagate errors) before
returning.
| async deleteAllBySelectedCalendarId(selectedCalendarId: string): Promise<void> { | ||
| this.prismaClient.calendarCacheEvent.deleteMany({ | ||
| where: { | ||
| selectedCalendarId, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Await deleteAll
Missing await risks races in callers.
- this.prismaClient.calendarCacheEvent.deleteMany({
+ await this.prismaClient.calendarCacheEvent.deleteMany({
where: {
selectedCalendarId,
},
});📝 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.
| async deleteAllBySelectedCalendarId(selectedCalendarId: string): Promise<void> { | |
| this.prismaClient.calendarCacheEvent.deleteMany({ | |
| where: { | |
| selectedCalendarId, | |
| }, | |
| }); | |
| } | |
| async deleteAllBySelectedCalendarId(selectedCalendarId: string): Promise<void> { | |
| await this.prismaClient.calendarCacheEvent.deleteMany({ | |
| where: { | |
| selectedCalendarId, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/lib/cache/CalendarCacheEventRepository.ts
around lines 26 to 32, the deleteAllBySelectedCalendarId method calls
prismaClient.calendarCacheEvent.deleteMany without awaiting it, which can cause
race conditions for callers; change the implementation to await the deleteMany
call (e.g., await this.prismaClient.calendarCacheEvent.deleteMany({...})) so the
async method only resolves after the deletion completes, preserving the
Promise<void> contract.
| /** | ||
| * Enriches a calendar with data from the cache | ||
| * | ||
| * @param selectedCalendarId calendar to enrich | ||
| * @returns | ||
| */ | ||
| async enrichCalendar({ selectedCalendarId }: { selectedCalendarId: string }): Promise<void> { | ||
| log.debug("enrichCalendar", { selectedCalendarId }); | ||
|
|
||
| const selectedCalendar = await this.deps.selectedCalendarRepository.findById(selectedCalendarId); | ||
| if (!selectedCalendar) { | ||
| log.debug("Selected calendar not found", { selectedCalendarId }); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Incomplete implementation of enrichCalendar method.
The enrichCalendar method fetches the calendar but doesn't perform any enrichment. This appears to be a stub implementation that should either be completed or marked with a TODO.
The method currently only fetches the calendar without performing enrichment. Would you like me to help implement the actual enrichment logic or add a TODO comment to track this?
| export type CalendarSubscriptionProvider = "google" | "microsoft"; | ||
|
|
||
| export type CalendarSubscriptionWebhookContext = { | ||
| headers?: Headers; | ||
| query?: URLSearchParams; | ||
| body?: JsonValue; | ||
| }; | ||
|
|
||
| export type CalendarSubscriptionResult = { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify provider identifiers across the codebase.
Multiple files use different provider keys: this file ("google" | "microsoft"), factory ("google-calendar" | "office365-calendar"), tests ("google_calendar" | "office365_calendar"). This will cause runtime/compile-time breakage and adapter lookups to return undefined.
Standardize to one set (suggest: "google-calendar" | "office365-calendar") and export a single source of truth.
Apply:
- export type CalendarSubscriptionProvider = "google" | "microsoft";
+export const CALENDAR_PROVIDERS = ["google-calendar", "office365-calendar"] as const;
+export type CalendarSubscriptionProvider = (typeof CALENDAR_PROVIDERS)[number];Also applies to: 42-46
🤖 Prompt for AI Agents
In
packages/features/calendar-subscription/lib/CalendarSubscriptionFeatures.interface.ts
around lines 8-16 and 42-46, the provider identifiers are inconsistent across
the codebase; change the union type to use the standardized identifiers
"google-calendar" | "office365-calendar" and export a single source of truth
(e.g., export a constant array of providers and a type derived from it or export
a named type alias) so all consumers import the same provider names; update any
exported types/consts in this file accordingly and ensure the file exports that
single provider constant/type for use by the factory and tests.
|
Closing this PR in favor of #23876 due to an error merging main |
2 participants
What does this PR do?
This PR introduces a comprehensive Calendar Cache and Sync system that enables real-time synchronization and local caching of external calendar events from providers like Google Calendar and Outlook.
Key Features
🔄 Real-time Calendar Synchronization
📦 Calendar Event Caching
🏗️ Modular Architecture
ICalendarSubscriptionPort) for easy extensionTechnical Implementation
New Database Schema:
CalendarCacheEventtable to store cached calendar events with full event metadataSelectedCalendartable with subscription tracking fields (channelId,syncEnabled,cacheEnabled, etc.)Service Architecture:
CalendarSubscriptionService- Manages calendar subscriptions and webhook processingCalendarCacheEventService- Handles event caching operationsCalendarSyncService- Manages bidirectional synchronization (planned)GoogleCalendarSubscriptionAdapter,OutlookCalendarSubscriptionAdapter)Webhook Infrastructure:
/api/webhook/calendar/[provider]/route.tsfor handling provider webhooksConfiguration
New Environment Variables:
Feature Flags:
calendar-subscription-cache- Enable/disable calendar event cachingcalendar-subscription-sync- Enable/disable calendar synchronizationDatabase Changes
20250904193813_add_calendar_cache_and_syncCalendarCacheEventwith comprehensive event metadataSelectedCalendarwith subscription management fieldsFiles Added/Modified
packages/features/calendar-subscription/Future Enhancements
The architecture supports planned features like:
This foundational implementation establishes the infrastructure for advanced calendar integration capabilities while maintaining performance and reliability.