feat: CalendarCache - filter generic calendars from subscription batch #18
Conversation
Add filtering to SelectedCalendarRepository.findNextSubscriptionBatch to exclude generic calendars (holidays, contacts, shared, imported, resources) based on their externalId suffixes. Changes: - Add GENERIC_CALENDAR_SUFFIXES constant mapping providers to their generic calendar suffixes in AdaptersFactory - Add getGenericCalendarSuffixes method to AdapterFactory interface - Update findNextSubscriptionBatch to accept and use genericCalendarSuffixes parameter to filter out calendars with matching externalId suffixes - Update CalendarSubscriptionService.checkForNewSubscriptions to pass generic calendar suffixes from the adapter factory - Update tests to cover the new filtering logic Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
WalkthroughThe PR introduces generic calendar suffix filtering throughout the calendar subscription system. A new constant maps calendar providers to suffix strings to exclude. The adapter factory exposes these suffixes. The calendar subscription service retrieves and passes them to the repository. The repository's Changes
Sequence DiagramsequenceDiagram
participant CS as CalendarSubscriptionService
participant AF as AdapterFactory
participant Repo as SelectedCalendarRepository
participant DB as Database
CS->>AF: getGenericCalendarSuffixes()
AF-->>CS: string[]
CS->>Repo: findNextSubscriptionBatch({<br/> take, teamIds, integrations,<br/> genericCalendarSuffixes<br/>})
Repo->>Repo: Build query with filters
Note over Repo: AND NOT externalId<br/>endsWith suffixes
Repo->>DB: SELECT * WHERE filters
DB-->>Repo: SelectedCalendar[]
Repo-->>CS: filtered results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.ts (1)
3-3: Update import to match the renamed class.The test imports
SelectedCalendarRepository, but the implementation file only exportsPrismaSelectedCalendarRepository. Update line 3 to import the correct class name:import { PrismaSelectedCalendarRepository } from "@calcom/features/selectedCalendar/repositories/SelectedCalendarRepository";Also update line 50 and 53 where the type and instantiation use
SelectedCalendarRepositoryto usePrismaSelectedCalendarRepositoryinstead.
🤖 Fix all issues with AI agents
In
`@packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts`:
- Around line 23-27: Change the parameter type in SelectedCalendarRepository
(the function/method that destructures { take, teamIds, integrations,
genericCalendarSuffixes }) to make integrations optional (integrations?:
string[]) to match SelectedCalendarRepository.interface.ts, and update any logic
inside that function (query building or filters that reference integrations) to
guard for undefined—e.g., check integrations && integrations.length (or
integrations?.length) before using it or build the query branch that handles
integrations being undefined/omitted.
🧹 Nitpick comments (2)
packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
67-71: Consider usingObject.values().flat()for cleaner iteration.The current implementation works correctly, but using
Object.valuesavoids the type assertion.♻️ Suggested simplification
getGenericCalendarSuffixes(): string[] { - return Object.keys(GENERIC_CALENDAR_SUFFIXES).flatMap( - (provider) => GENERIC_CALENDAR_SUFFIXES[provider as CalendarSubscriptionProvider] - ); + return Object.values(GENERIC_CALENDAR_SUFFIXES).flat(); }packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts (1)
41-45: Minor: Redundant optional chaining after truthy check.The
?.ingenericCalendarSuffixes?.mapon line 42 is unnecessary sincegenericCalendarSuffixes?.lengthon line 41 already ensures the array is defined and non-empty.♻️ Suggested cleanup
- AND: genericCalendarSuffixes?.length - ? genericCalendarSuffixes?.map((suffix) => ({ + AND: genericCalendarSuffixes?.length + ? genericCalendarSuffixes.map((suffix) => ({ NOT: { externalId: { endsWith: suffix } }, })) : undefined,
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/features/calendar-subscription/adapters/AdaptersFactory.tspackages/features/calendar-subscription/lib/CalendarSubscriptionService.tspackages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.tspackages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.tspackages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.tspackages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts (1)
packages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.ts (1)
ISelectedCalendarRepository(3-67)
🔇 Additional comments (5)
packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (1)
391-396: LGTM!The integration of
genericCalendarSuffixesinto the subscription batch query is clean and correctly sources the suffixes from the adapter factory.packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts (1)
115-121: LGTM!The mock setup for
getGenericCalendarSuffixescorrectly mirrors the productionGENERIC_CALENDAR_SUFFIXESvalues, and the test expectations throughout properly validate the integration.packages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.ts (1)
202-236: LGTM!The new test case properly validates the suffix filtering behavior, verifying that the
ANDclause contains the correctNOT endsWithconditions for each provided suffix.packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
7-20: LGTM!The
GENERIC_CALENDAR_SUFFIXESconstant is well-documented and properly centralizes the exclusion patterns per provider. The empty array foroffice365_calendarprovides a clear extension point for future additions.packages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.ts (1)
26-36: LGTM!The interface correctly adds
genericCalendarSuffixesas an optional parameter with appropriate JSDoc documentation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| }: { | ||
| take: number; | ||
| teamIds: number[]; | ||
| integrations: string[]; | ||
| genericCalendarSuffixes?: string[]; |
There was a problem hiding this comment.
Type mismatch: integrations is required here but optional in interface.
The interface at SelectedCalendarRepository.interface.ts declares integrations?: string[] as optional, but this implementation declares it as required (integrations: string[]). This mismatch could cause type errors for callers expecting to omit the parameter.
🔧 Suggested fix
}: {
take: number;
teamIds: number[];
- integrations: string[];
+ integrations?: string[];
genericCalendarSuffixes?: string[];
}) {Then update the query to handle undefined:
where: {
- integration: { in: integrations },
+ integration: integrations?.length ? { in: integrations } : undefined,🤖 Prompt for AI Agents
In
`@packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts`
around lines 23 - 27, Change the parameter type in SelectedCalendarRepository
(the function/method that destructures { take, teamIds, integrations,
genericCalendarSuffixes }) to make integrations optional (integrations?:
string[]) to match SelectedCalendarRepository.interface.ts, and update any logic
inside that function (query building or filters that reference integrations) to
guard for undefined—e.g., check integrations && integrations.length (or
integrations?.length) before using it or build the query branch that handles
integrations being undefined/omitted.
Benchmark PR from qodo-benchmark#702
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.