feat: CalendarCache - filter generic calendars from subscription batch #34
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 <[email protected]>
WalkthroughThis PR adds calendar suffix-based filtering to exclude generic/system calendars from subscriptions. It introduces provider-specific suffixes via a new constant and factory method, threads this configuration through the service layer, and applies filtering in the repository using a NOT-ENDS-WITH query condition. Changes
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: 2
🤖 Fix all issues with AI agents
In
`@packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts`:
- Around line 23-27: The implementation in SelectedCalendarRepository has a
parameter typed as integrations: string[] while the corresponding interface
marks integrations as optional; update the method signature in
SelectedCalendarRepository to integrations?: string[] (or if integrations must
be required, update the interface instead) and inside the method (e.g., where
integrations is used) handle the undefined case (e.g., default to an empty array
or guard before iterating) so usage is safe; reference the parameter group
containing take, teamIds, integrations, genericCalendarSuffixes in the
SelectedCalendarRepository method.
- Around line 5-6: The exported class name PrismaSelectedCalendarRepository no
longer matches the name imported by the tests (SelectedCalendarRepository),
causing an import error; fix by either renaming the class back to
SelectedCalendarRepository in the file (change PrismaSelectedCalendarRepository
→ SelectedCalendarRepository) or update the test import to use
PrismaSelectedCalendarRepository so both sides reference the same class name
(adjust the import statement in the test that currently imports
SelectedCalendarRepository).
🧹 Nitpick comments (1)
packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
67-71: Consider usingObject.values()for cleaner code.The current implementation works correctly, but using
Object.values()avoids the need for type casting.♻️ Suggested refactor
getGenericCalendarSuffixes(): string[] { - return Object.keys(GENERIC_CALENDAR_SUFFIXES).flatMap( - (provider) => GENERIC_CALENDAR_SUFFIXES[provider as CalendarSubscriptionProvider] - ); + return Object.values(GENERIC_CALENDAR_SUFFIXES).flat(); }
📜 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 (7)
packages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.ts (1)
24-35: LGTM!The interface update is well-documented with JSDoc and correctly types
genericCalendarSuffixesas optional. The signature aligns with the implementation.packages/features/calendar-subscription/adapters/AdaptersFactory.ts (1)
7-20: LGTM!The
GENERIC_CALENDAR_SUFFIXESconstant is well-documented and provides a clear mapping of provider-specific suffixes to exclude. This is a clean way to centralize the configuration.packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts (1)
41-45: LGTM!The suffix filtering logic correctly uses an array of
NOTconditions combined withANDto exclude all calendars matching any of the provided suffixes. When no suffixes are provided,undefinedis correctly passed which Prisma will ignore.packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts (1)
391-396: LGTM!Clean integration of the generic calendar suffix filtering. The suffixes are correctly retrieved from the adapter factory and passed to the repository method.
packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts (1)
115-121: LGTM!The mock correctly returns the same suffixes defined in
GENERIC_CALENDAR_SUFFIXESfor Google Calendar. The test expectations are consistent throughout allcheckForNewSubscriptionstest cases.packages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.ts (2)
202-236: LGTM!Excellent test coverage for the new filtering logic. The test properly validates that:
- The
ANDclause is constructed withNOTconditions for each suffix- The
externalIdendsWithfilter is correctly applied
134-134: Good practice: explicitly testing theAND: undefinedcase.These tests properly verify that when
genericCalendarSuffixesis not provided, theANDclause isundefined, ensuring no unintended filtering occurs.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export class PrismaSelectedCalendarRepository implements ISelectedCalendarRepository { | ||
| constructor(private prismaClient: PrismaClient) {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all exports and re-exports of SelectedCalendarRepository in the selectedCalendar directory
echo "=== Searching for SelectedCalendarRepository in the selectedCalendar directory ==="
rg -n "SelectedCalendarRepository|PrismaSelectedCalendarRepository" packages/features/selectedCalendar/ --type ts
echo ""
echo "=== Checking for index/barrel exports ==="
find packages/features/selectedCalendar -name "index.ts" -o -name "*.test.ts" | head -20Repository: qodo-benchmark/cal.com-combined-coderabbit
Length of output: 1437
Fix class name mismatch between export and test imports.
The class was renamed to PrismaSelectedCalendarRepository but the test file imports the old name SelectedCalendarRepository (line 3 of the test). This will cause an import error. Either rename the class back to SelectedCalendarRepository or update the test imports to use PrismaSelectedCalendarRepository.
🤖 Prompt for AI Agents
In
`@packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts`
around lines 5 - 6, The exported class name PrismaSelectedCalendarRepository no
longer matches the name imported by the tests (SelectedCalendarRepository),
causing an import error; fix by either renaming the class back to
SelectedCalendarRepository in the file (change PrismaSelectedCalendarRepository
→ SelectedCalendarRepository) or update the test import to use
PrismaSelectedCalendarRepository so both sides reference the same class name
(adjust the import statement in the test that currently imports
SelectedCalendarRepository).
| }: { | ||
| 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 declares integrations?: string[] (optional), but this implementation declares integrations: string[] (required). This inconsistency could cause TypeScript errors when calling the method without the integrations parameter.
🔧 Proposed fix
}: {
take: number;
teamIds: number[];
- integrations: string[];
+ integrations?: string[];
genericCalendarSuffixes?: string[];
}) {Note: If integrations should remain required, update the interface instead.
📝 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.
| }: { | |
| take: number; | |
| teamIds: number[]; | |
| integrations: string[]; | |
| genericCalendarSuffixes?: string[]; | |
| }: { | |
| take: number; | |
| teamIds: number[]; | |
| integrations?: string[]; | |
| genericCalendarSuffixes?: string[]; |
🤖 Prompt for AI Agents
In
`@packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts`
around lines 23 - 27, The implementation in SelectedCalendarRepository has a
parameter typed as integrations: string[] while the corresponding interface
marks integrations as optional; update the method signature in
SelectedCalendarRepository to integrations?: string[] (or if integrations must
be required, update the interface instead) and inside the method (e.g., where
integrations is used) handle the undefined case (e.g., default to an empty array
or guard before iterating) so usage is safe; reference the parameter group
containing take, teamIds, integrations, genericCalendarSuffixes in the
SelectedCalendarRepository method.
Benchmark PR from agentic-review-benchmarks#7
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.