refactor: getUserAvailability into service with DI#22881
Conversation
WalkthroughThis set of changes refactors the user availability logic across multiple packages and services. A new Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Complexity label: Complex Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
keithwillcode
added
core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/api/v2/src/lib/services/user-availability.service.ts (1)
1-27: LGTM! Clean NestJS service implementation with minor formatting suggestions.The service correctly:
- Uses
@Injectable()decorator for dependency injection- Extends the base service from platform libraries
- Injects required repositories through constructor
- Follows NestJS patterns
Consider these minor improvements:
+/** + * NestJS service wrapper for user availability logic. + * Extends the base UserAvailabilityService with injected repositories. + */ @Injectable() export class UserAvailabilityService extends BaseUserAvailabilityService { constructor( oooRepoDependency: PrismaOOORepository, bookingRepository: PrismaBookingRepository, eventTypeRepository: PrismaEventTypeRepository, - - ) { super({ oooRepo: oooRepoDependency, - bookingRepo: bookingRepository, - eventTypeRepo: eventTypeRepository, }); } }packages/trpc/server/routers/viewer/slots/util.ts (1)
821-822: Remove obsolete TODO comment.The TODO comment on line 821 is now obsolete since dependency injection has been implemented for
getUsersAvailability.- // TODO: DI getUsersAvailability const premappedUsersAvailability = await this.dependencies.userAvailabilityService.getUsersAvailability({packages/lib/getUserAvailability.ts (2)
156-161: Fix indentation inconsistency.The
eventTypeRepoproperty has incorrect indentation compared to other properties in the interface.export interface IUserAvailabilityService { - eventTypeRepo: EventTypeRepository; + eventTypeRepo: EventTypeRepository; oooRepo: PrismaOOORepository; bookingRepo: BookingRepository; - }
204-207: Remove unnecessary empty lines.There are unnecessary empty lines after the repository call that should be removed for better code formatting.
const bookings = await this.dependencies.bookingRepo.findAcceptedBookingByEventTypeId({eventTypeId: id, dateFrom: dateFrom.format(), dateTo: dateTo.format()}) - - - return bookings.map((booking) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (21)
apps/api/v1/pages/api/availability/_get.ts(3 hunks)apps/api/v2/package.json(1 hunks)apps/api/v2/src/lib/modules/available-slots.module.ts(2 hunks)apps/api/v2/src/lib/services/available-slots.service.ts(2 hunks)apps/api/v2/src/lib/services/user-availability.service.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts(3 hunks)packages/lib/di/containers/available-slots.ts(2 hunks)packages/lib/di/containers/get-user-availability.ts(1 hunks)packages/lib/di/modules/available-slots.ts(1 hunks)packages/lib/di/modules/get-user-availability.ts(1 hunks)packages/lib/di/tokens.ts(1 hunks)packages/lib/getUserAvailability.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(4 hunks)packages/lib/server/repository/booking.ts(1 hunks)packages/lib/server/repository/eventTypeRepository.ts(1 hunks)packages/lib/server/repository/ooo.ts(1 hunks)packages/platform/libraries/package.json(1 hunks)packages/platform/libraries/schedules.ts(1 hunks)packages/trpc/server/routers/viewer/availability/user.handler.ts(2 hunks)packages/trpc/server/routers/viewer/slots/util.ts(8 hunks)packages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
apps/api/v1/pages/api/availability/_get.tsapps/api/v2/src/lib/services/available-slots.service.tsapps/api/v2/src/lib/modules/available-slots.module.tspackages/lib/di/containers/get-user-availability.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.tspackages/lib/di/modules/get-user-availability.tspackages/trpc/server/routers/viewer/availability/user.handler.tspackages/platform/libraries/schedules.tspackages/lib/di/tokens.tspackages/lib/di/containers/available-slots.tspackages/lib/server/repository/ooo.tspackages/lib/server/repository/booking.tspackages/lib/di/modules/available-slots.tspackages/lib/server/repository/eventTypeRepository.tspackages/trpc/server/routers/viewer/slots/util.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/getUserAvailability.tsapps/api/v2/src/lib/services/user-availability.service.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:
apps/api/v1/pages/api/availability/_get.tsapps/api/v2/src/lib/services/available-slots.service.tsapps/api/v2/src/lib/modules/available-slots.module.tspackages/lib/di/containers/get-user-availability.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.tspackages/lib/di/modules/get-user-availability.tspackages/trpc/server/routers/viewer/availability/user.handler.tspackages/platform/libraries/schedules.tspackages/lib/di/tokens.tspackages/lib/di/containers/available-slots.tspackages/lib/server/repository/ooo.tspackages/lib/server/repository/booking.tspackages/lib/di/modules/available-slots.tspackages/lib/server/repository/eventTypeRepository.tspackages/trpc/server/routers/viewer/slots/util.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/getUserAvailability.tsapps/api/v2/src/lib/services/user-availability.service.ts
**/*.{service,repository}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/lib/services/available-slots.service.tsapps/api/v2/src/lib/services/user-availability.service.ts
**/*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/eventTypeRepository.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
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:
apps/api/v1/pages/api/availability/_get.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.tspackages/trpc/server/routers/viewer/availability/user.handler.tspackages/platform/libraries/schedules.tspackages/lib/server/repository/eventTypeRepository.tspackages/trpc/server/routers/viewer/slots/util.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/getUserAvailability.tsapps/api/v2/src/lib/services/user-availability.service.ts
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
apps/api/v2/src/lib/services/available-slots.service.tspackages/trpc/server/routers/viewer/slots/util.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/lib/server/repository/booking.tspackages/lib/di/modules/available-slots.tspackages/lib/getUserAvailability.ts
📚 Learning: in cal.com's calendar integration, both google calendar and outlook calendar are designed to allow m...
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/lib/server/repository/eventTypeRepository.ts
📚 Learning: the outlook calendar integration in cal.com intentionally reuses subscription ids across multiple ev...
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/lib/server/repository/eventTypeRepository.ts
📚 Learning: applies to **/*.{ts,tsx} : flag excessive day.js use in performance-critical code; prefer native dat...
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,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: applies to **/*repository.ts : repository files must include `repository` suffix, prefix with techno...
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 **/*Repository.ts : Repository files must include `Repository` suffix, prefix with technology if applicable (e.g., `PrismaAppRepository.ts`), and use PascalCase matching the exported class
Applied to files:
packages/lib/getUserAvailability.ts
🧬 Code Graph Analysis (9)
apps/api/v1/pages/api/availability/_get.ts (1)
packages/lib/di/containers/get-user-availability.ts (1)
getUserAvailabilityService(21-23)
apps/api/v2/src/lib/services/available-slots.service.ts (1)
packages/lib/getUserAvailability.ts (1)
UserAvailabilityService(163-614)
apps/api/v2/src/lib/modules/available-slots.module.ts (1)
packages/lib/getUserAvailability.ts (1)
UserAvailabilityService(163-614)
packages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts (1)
packages/lib/di/containers/get-user-availability.ts (1)
getUserAvailabilityService(21-23)
packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts (1)
packages/lib/di/containers/get-user-availability.ts (1)
getUserAvailabilityService(21-23)
packages/trpc/server/routers/viewer/availability/user.handler.ts (1)
packages/lib/di/containers/get-user-availability.ts (1)
getUserAvailabilityService(21-23)
packages/lib/di/modules/available-slots.ts (1)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-35)
packages/lib/server/repository/eventTypeRepository.ts (1)
packages/prisma/zod-utils.ts (1)
EventTypeMetaDataSchema(136-136)
packages/trpc/server/routers/viewer/slots/util.ts (1)
packages/lib/getUserAvailability.ts (1)
UserAvailabilityService(163-614)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (34)
apps/api/v2/src/lib/modules/available-slots.module.ts (2)
15-15: LGTM!The import for
UserAvailabilityServiceis correctly added to support the dependency injection setup.
32-32: LGTM!The
UserAvailabilityServiceis correctly added to the providers array, enabling it to be injected into other services within this module.packages/platform/libraries/package.json (1)
3-3: Verify the version bump strategy.The version jump from "0.0.0" to "9.9.9" is significant. Ensure this aligns with your package versioning strategy and isn't a temporary development version that should be adjusted before merging.
apps/api/v2/package.json (1)
41-41: Dependency update looks correct.The platform-libraries dependency version is correctly updated to match the new version. However, ensure the "9.9.9" version aligns with your versioning strategy as mentioned in the platform-libraries package.json review.
packages/lib/di/containers/available-slots.ts (2)
19-19: LGTM!The import for
getUserAvailabilityModuleis correctly added to support the DI container setup.
35-35: LGTM!The module is correctly loaded into the container using the appropriate DI token, following the established pattern in this file.
packages/lib/di/modules/available-slots.ts (1)
20-20: LGTM!The
userAvailabilityServicedependency is correctly added to the DI module configuration with the appropriate token, maintaining type safety with thesatisfiesconstraint.packages/platform/libraries/schedules.ts (1)
7-7: LGTM! Clean export addition for the new service.The export of
UserAvailabilityServicefollows the existing pattern and supports the dependency injection refactor outlined in the PR objectives.packages/lib/di/tokens.ts (1)
33-34: LGTM! Consistent DI token definitions.The new tokens follow the established naming convention and properly use Symbol constructors with descriptive identifiers for the UserAvailabilityService dependency injection setup.
packages/trpc/server/routers/viewer/availability/user.handler.ts (2)
1-1: LGTM! Clean import refactor to use service factory.The import change from direct function to service factory aligns with the dependency injection refactor.
14-18: LGTM! Proper service instantiation and method usage.The handler correctly instantiates the service and calls the method with the same parameters, maintaining API compatibility while adopting the new service-oriented architecture.
packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts (3)
9-9: LGTM! Import refactor follows established pattern.The import change to use the service factory is consistent with the broader dependency injection refactor.
65-65: LGTM! Efficient service instantiation.Creating the service instance once at the function start is the appropriate approach.
90-110: LGTM! Proper method usage with preserved logic.The call to
getUsersAvailabilityon the service instance maintains the same parameters and logic while adopting the new service architecture. The plural method name correctly reflects the multiple users context.apps/api/v1/pages/api/availability/_get.ts (3)
4-4: LGTM! Import refactor maintains consistency.The import change to use the service factory follows the established dependency injection pattern.
194-204: LGTM! Efficient service usage for single user availability.Creating the service instance once and using it for the single user availability call is clean and maintains the same API behavior.
234-242: LGTM! Proper service reuse in team member loop.The service instance is efficiently reused for each team member's availability calculation, maintaining the same parameters and behavior while adopting the new service architecture.
apps/api/v2/src/lib/services/available-slots.service.ts (1)
15-15: LGTM!The import for
UserAvailabilityServiceis correctly added.packages/lib/di/modules/get-user-availability.ts (1)
1-11: LGTM! Well-structured dependency injection module.The module correctly:
- Binds
UserAvailabilityServiceto its DI token- Maps constructor dependencies to their respective tokens
- Uses
satisfiesfor type safety- Follows proper DI patterns
packages/lib/di/containers/get-user-availability.ts (1)
1-23: LGTM! Properly structured DI container.The container correctly:
- Loads all required modules (Prisma, repositories, and service module)
- Provides a clean factory function to resolve the service
- Maintains proper separation of concerns
packages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts (3)
2-2: LGTM!The import change correctly switches from the standalone function to the service factory pattern.
19-19: LGTM!Service instantiation follows the established pattern using the DI container factory function.
36-45: LGTM!The method call correctly uses the service instance method instead of the standalone function, maintaining all the same parameters and functionality.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (5)
4-4: LGTM! Proper DI container import.The import aligns with the refactoring objective to use dependency injection for user availability services.
89-89: LGTM! Service instantiation follows DI pattern.The service is correctly instantiated at the beginning of the function scope for consistent usage.
109-109: LGTM! Method call through service instance.The
getPeriodStartDatesBetweenmethod is correctly accessed through the service instance instead of direct function import.
166-167: LGTM! Consistent service instantiation pattern.The service instantiation follows the same pattern as in
_getBusyTimesFromBookingLimits.
173-173: LGTM! Consistent method call pattern.The method call through the service instance maintains consistency with the refactoring approach.
packages/lib/server/repository/eventTypeRepository.ts (1)
1303-1390: LGTM! Well-structured repository method for user availability.The method follows consistent patterns with other repository methods and appropriately:
- Uses focused field selection for performance
- Handles null cases properly
- Parses metadata using the imported schema
- Follows the repository naming convention
The field selection is well-tailored for user availability calculations, including booking limits, scheduling details, and team configurations.
packages/trpc/server/routers/viewer/slots/util.ts (3)
23-23: LGTM! Proper dependency injection setup.The import and interface update correctly introduce the
UserAvailabilityServiceas a dependency.Also applies to: 104-104
361-361: Correct refactoring to use injected service.The method call properly uses the dependency-injected
userAvailabilityServicewhile maintaining the same parameters.
396-396: Consistent refactoring pattern applied.All
getPeriodStartDatesBetweencalls are correctly updated to use the injected service with the same parameters.Also applies to: 447-447, 547-547, 595-595
packages/lib/getUserAvailability.ts (2)
166-177: Proper repository usage in _getEventType.The method correctly uses the injected repository and maintains the original metadata parsing logic.
361-363: Static helper usage is intentional and correctThe
getSelectedCalendarsFromUserfunction is declared as astaticmethod onEventTypeRepositorywith no matching instance method. Calling it directly viaEventTypeRepository.getSelectedCalendarsFromUser(...)is valid and consistent with its implementation, so no change is required here.Likely an incorrect or invalid review comment.
| async findAcceptedBookingByEventTypeId({eventTypeId, dateFrom, dateTo}: {eventTypeId?: number, dateFrom: string, dateTo: string}) { | ||
| return this.prismaClient.booking.findMany({ | ||
| where: { | ||
| eventTypeId, | ||
| startTime: { | ||
| gte: dateFrom, | ||
| lte: dateTo, | ||
| }, | ||
| status: BookingStatus.ACCEPTED, | ||
| }, | ||
| select: { | ||
| uid: true, | ||
| startTime: true, | ||
| attendees: { | ||
| select: { | ||
| email: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Address parameter types and method naming issues.
Several concerns with the new repository method:
-
Parameter type inconsistency:
dateFromanddateToare strings, but other methods in this class useDateobjects. This inconsistency could lead to confusion and potential runtime issues. -
Method naming: The method name suggests it returns a single booking (
findAcceptedBookingByEventTypeId) but returns an array. It should befindAcceptedBookingsByEventTypeId. -
Optional parameter handling:
eventTypeIdis optional but used directly in the where clause, which could cause issues ifundefined.
Apply this diff to fix the issues:
-async findAcceptedBookingByEventTypeId({eventTypeId, dateFrom, dateTo}: {eventTypeId?: number, dateFrom: string, dateTo: string}) {
+async findAcceptedBookingsByEventTypeId({eventTypeId, dateFrom, dateTo}: {eventTypeId?: number, dateFrom: Date, dateTo: Date}) {
return this.prismaClient.booking.findMany({
where: {
- eventTypeId,
+ ...(eventTypeId && { eventTypeId }),
startTime: {
gte: dateFrom,
lte: dateTo,
},
status: BookingStatus.ACCEPTED,
},This ensures:
- Consistent parameter types with other repository methods
- Proper handling of optional
eventTypeId - Clear method naming that reflects the return type
📝 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 findAcceptedBookingByEventTypeId({eventTypeId, dateFrom, dateTo}: {eventTypeId?: number, dateFrom: string, dateTo: string}) { | |
| return this.prismaClient.booking.findMany({ | |
| where: { | |
| eventTypeId, | |
| startTime: { | |
| gte: dateFrom, | |
| lte: dateTo, | |
| }, | |
| status: BookingStatus.ACCEPTED, | |
| }, | |
| select: { | |
| uid: true, | |
| startTime: true, | |
| attendees: { | |
| select: { | |
| email: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } | |
| async findAcceptedBookingsByEventTypeId({eventTypeId, dateFrom, dateTo}: {eventTypeId?: number, dateFrom: Date, dateTo: Date}) { | |
| return this.prismaClient.booking.findMany({ | |
| where: { | |
| ...(eventTypeId && { eventTypeId }), | |
| startTime: { | |
| gte: dateFrom, | |
| lte: dateTo, | |
| }, | |
| status: BookingStatus.ACCEPTED, | |
| }, | |
| select: { | |
| uid: true, | |
| startTime: true, | |
| attendees: { | |
| select: { | |
| email: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/lib/server/repository/booking.ts around lines 873 to 893, fix the
method by renaming it to findAcceptedBookingsByEventTypeId to reflect it returns
multiple bookings, change the dateFrom and dateTo parameters from string to Date
to match other methods, and update the where clause to conditionally include
eventTypeId only if it is defined to avoid issues when it is undefined.
| async findUserOOODays({userId, dateTo, dateFrom}: {userId: number, dateTo: string, dateFrom: string}) { | ||
| return this.prismaClient.outOfOfficeEntry.findMany({ | ||
| where: { | ||
| userId, | ||
| OR: [ | ||
| // outside of range | ||
| // (start <= 'dateTo' AND end >= 'dateFrom') | ||
| { | ||
| start: { | ||
| lte: dateTo, | ||
| }, | ||
| end: { | ||
| gte: dateFrom, | ||
| }, | ||
| }, | ||
| // start is between dateFrom and dateTo but end is outside of range | ||
| // (start <= 'dateTo' AND end >= 'dateTo') | ||
| { | ||
| start: { | ||
| lte: dateTo, | ||
| }, | ||
|
|
||
| end: { | ||
| gte: dateTo, | ||
| }, | ||
| }, | ||
| // end is between dateFrom and dateTo but start is outside of range | ||
| // (start <= 'dateFrom' OR end <= 'dateTo') | ||
| { | ||
| start: { | ||
| lte: dateFrom, | ||
| }, | ||
|
|
||
| end: { | ||
| lte: dateTo, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| select: { | ||
| id: true, | ||
| start: true, | ||
| end: true, | ||
| user: { | ||
| select: { | ||
| id: true, | ||
| name: true, | ||
| }, | ||
| }, | ||
| toUser: { | ||
| select: { | ||
| id: true, | ||
| username: true, | ||
| name: true, | ||
| }, | ||
| }, | ||
| reason: { | ||
| select: { | ||
| id: true, | ||
| emoji: true, | ||
| reason: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to reduce code duplication and improve consistency.
The method has several issues that should be addressed:
- Code duplication: The OR condition logic (lines 87-120) is identical to
findManyOOO(lines 20-53) - Inconsistent parameter types: Uses
stringdates whilefindManyOOOusesDateobjects - Formatting inconsistencies: Extra whitespace on lines 104 and 115
Consider extracting the common OR condition logic into a private method:
+ private buildDateRangeConditions(startDate: string | Date, endDate: string | Date) {
+ return [
+ {
+ start: { lte: endDate },
+ end: { gte: startDate },
+ },
+ {
+ start: { lte: endDate },
+ end: { gte: endDate },
+ },
+ {
+ start: { lte: startDate },
+ end: { lte: endDate },
+ },
+ ];
+ }
- async findUserOOODays({userId, dateTo, dateFrom}: {userId: number, dateTo: string, dateFrom: string}) {
+ async findUserOOODays({userId, dateTo, dateFrom}: {userId: number, dateTo: Date, dateFrom: Date}) {
return this.prismaClient.outOfOfficeEntry.findMany({
where: {
userId,
- OR: [
- // duplicated logic...
- ],
+ OR: this.buildDateRangeConditions(dateFrom, dateTo),
},
select: {
// ... existing select
},
});
}Also update findManyOOO to use the same extracted method for consistency.
🤖 Prompt for AI Agents
In packages/lib/server/repository/ooo.ts between lines 83 and 148, the OR
condition logic for date range filtering is duplicated from findManyOOO (lines
20-53) and uses inconsistent parameter types (string dates here vs Date objects
in findManyOOO), plus there are formatting issues with extra whitespace.
Refactor by extracting the common OR condition logic into a private helper
method that accepts consistent Date parameters, then update both findUserOOODays
and findManyOOO to call this helper for the OR condition. Also clean up the
extra whitespace for consistent formatting.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/v2/src/modules/billing/services/billing.service.ts (1)
468-468: Remove unnecessary empty line.The empty line serves no functional purpose and should be removed to maintain code cleanliness.
try { - await this.billingQueue.close();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
apps/api/v2/src/modules/billing/billing.processor.ts(1 hunks)apps/api/v2/src/modules/billing/services/billing.service.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/api/v2/src/modules/billing/billing.processor.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{service,repository}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/billing/services/billing.service.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:
apps/api/v2/src/modules/billing/services/billing.service.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:
apps/api/v2/src/modules/billing/services/billing.service.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
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: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
apps/api/v2/src/modules/billing/services/billing.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (2)
apps/api/v2/src/modules/billing/services/billing.service.ts (2)
407-409: LGTM: E2E bypass logic is correctly implemented.The early exit condition for E2E testing environments is appropriate and prevents actual billing operations during test runs. The return value
truemaintains the expected boolean return type.
434-436: LGTM: Consistent E2E bypass implementation.The early exit condition matches the pattern used in
increaseUsageByUserIdand correctly prevents queue operations during E2E testing. The implementation is consistent and maintains the expected return type.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/lib/server/i18n.ts (1)
32-53: Excellent error handling implementation.The try-catch block provides comprehensive error handling for both network failures and JSON parsing errors. The implementation correctly:
- Checks response status before parsing JSON
- Returns an empty object fallback to maintain function contract
- Uses appropriate timeout for translation fetching
Consider using consistent error logging throughout - either
logger.errororconsole.error, but not both.} catch (err) { - console.error("loadTranslations Error:", err); + logger.error("loadTranslations Error:", err); return {}; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
apps/api/v2/package.json(1 hunks)packages/lib/server/i18n.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/v2/package.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/i18n.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/i18n.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: For large pull requests (>500 lines changed or >10 files touched), advise splitting into smaller, focused PRs by feature, layer, dependency chain, or file/module
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: applies to **/*.tsx : always use `t()` for text localization in frontend code; direct text embedding...
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 **/*.tsx : Always use `t()` for text localization in frontend code; direct text embedding should trigger a warning
Applied to files:
packages/lib/server/i18n.ts
🔇 Additional comments (1)
packages/lib/server/i18n.ts (1)
8-10: Import using require() is appropriateThe
@calcom/config/next-i18next.configmodule is authored as a CommonJS export (module.exports), so usingrequire()(and disabling the lint rule) is necessary and correct in this context.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/v2/src/ee/event-types/event-types_2024_04_15/controllers/event-types.controller.e2e-spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
apps/api/v2/src/ee/event-types/event-types_2024_04_15/controllers/event-types.controller.e2e-spec.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:
apps/api/v2/src/ee/event-types/event-types_2024_04_15/controllers/event-types.controller.e2e-spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| const fields = responseBookingField | ||
| //@ts-ignore | ||
| delete fields.labelAsSafeHtml | ||
| expect(fields).toEqual(bookingFields[0]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve test robustness and avoid mutation of response objects.
This code has several issues:
const fields = responseBookingFieldcreates a reference, sodelete fields.labelAsSafeHtmlmutates the original response object- Using
@ts-ignoresuppresses useful type checking - No null check for
responseBookingField - Mutating response objects can cause side effects in other tests
Apply this diff to fix the issues:
- const fields = responseBookingField
- //@ts-ignore
- delete fields.labelAsSafeHtml
- expect(fields).toEqual(bookingFields[0]);
+ expect(responseBookingField).toBeDefined();
+ const { labelAsSafeHtml, ...fields } = responseBookingField!;
+ expect(fields).toEqual(bookingFields[0]);This approach:
- Adds a null check for safety
- Uses destructuring to exclude
labelAsSafeHtmlwithout mutation - Removes the
@ts-ignoredirective - Preserves the original response object for other potential uses
🤖 Prompt for AI Agents
In
apps/api/v2/src/ee/event-types/event-types_2024_04_15/controllers/event-types.controller.e2e-spec.ts
around lines 293 to 296, avoid mutating the original responseBookingField object
by not deleting properties directly. Instead, add a null check for
responseBookingField, then use destructuring or object spread to create a new
object excluding labelAsSafeHtml, removing the @ts-ignore directive. This
preserves the original object and improves test robustness.
| expect(responseBookingField).toEqual(bookingFields[0]); | ||
| const fields = responseBookingField | ||
| //@ts-ignore | ||
| delete fields.labelAsSafeHtml |
There was a problem hiding this comment.
this field is weirdly formatted making it hard to test, deleting for now until we find better solution
There was a problem hiding this comment.
weird newline and spaces added at the end
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
4 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist