feat: [Booking flow refactor - 3] Rename booking handlers to service names and remove Prisma import from InstantBookingCreateService#23682
Conversation
WalkthroughThis PR consolidates booking service implementations under lib/service, updates many import/re-export paths, and refactors DI to use ModuleLoader/bindModuleToClassOnToken patterns. It injects PrismaClient into instant-booking flows and exposes moduleLoader objects for several modules. Type surface changes include removal of the BookingResponse alias, addition/reordering of RegularBookingCreateResult, and a new Zod schema export instantMeetingSubscriptionSchema (re-exported by instant-meeting). Tests, client mappers, and platform library re-exports were updated to follow new module and type locations. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
keithwillcode
added
core
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice
branch
from
aaf3293 to
d357430
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
3 times, most recently
from
3b6187c to
4734a13
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice
branch
from
d357430 to
2ac051c
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
4734a13 to
bf970b4
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice
branch
from
f567bd4 to
7546752
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice
branch
from
7546752 to
ea92356
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
bf970b4 to
85969f9
Compare
hariombalhara
changed the title
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice
branch
from
ea92356 to
30558e0
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
85969f9 to
852ac29
Compare
|
|
||
| import { getInstantBookingCreateService } from "@calcom/features/bookings/di/InstantBookingCreateService.container"; | ||
| import type { CreateInstantBookingData } from "../dto/types"; | ||
| import { getInstantBookingCreateService } from "../../di/InstantBookingCreateService.container"; |
There was a problem hiding this comment.
Within the package we should import using relative path.
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
82303a4 to
3f24155
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
269-276: Replaceincludewithselectin Prisma create operation.The
createBookingObjusesinclude: { attendees: true }which violates the coding guideline requiringselectinstead ofincludefor all Prisma operations. This ensures you only retrieve the data you need.Apply this diff to use
select:const createBookingObj = { - include: { - attendees: true, - }, + select: { + id: true, + uid: true, + userId: true, + responses: true, + title: true, + customInputs: true, + attendees: { + select: { + id: true, + email: true, + name: true, + timeZone: true, + locale: true, + phoneNumber: true, + }, + }, + }, data: newBookingData, };As per coding guidelines
♻️ Duplicate comments (2)
packages/features/bookings/lib/dto/schema.ts (1)
5-5: Thez.any()type defeats runtime validation.This issue has already been flagged in previous reviews. The
expirationTimefield should use a specific type (likelyz.number().nullable()for a DOMTimeStamp) orz.unknown()if the type truly varies. Usingz.any()removes the benefits of Zod's type safety and runtime validation.packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts (1)
5-5: Import path correctly updated.The import path has been updated to
../../service/RegularBookingService, which is correct according to the PR objectives. The refactoring renamedhandleNewBooking.ts→RegularBookingService.ts, notBookingCreateService.tsas suggested in the past review comment.
🧹 Nitpick comments (2)
packages/features/bookings/lib/dto/schema.ts (1)
3-10: Consider adding stricter validation for subscription fields.While the current schema is functional, adding validation constraints would improve data quality:
endpointcould validate URL format usingz.string().url()keys.authandkeys.p256dhcould enforce minimum length requirements since they're cryptographic keys (typically base64-encoded with predictable lengths)Apply this diff for stricter validation:
export const instantMeetingSubscriptionSchema = z.object({ - endpoint: z.string(), + endpoint: z.string().url(), expirationTime: z.any().optional(), keys: z.object({ - auth: z.string(), - p256dh: z.string(), + auth: z.string().min(1), + p256dh: z.string().min(1), }), });packages/features/bookings/lib/dto/types.d.ts (1)
46-47: Type inference pattern is sound.The
Awaited<ReturnType<...>>pattern correctly derives the type from the async method. The TODO acknowledges that an explicit definition would improve clarity and reduce coupling.Would you like me to open a tracking issue for defining this type explicitly in the follow-up PR?
📜 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 (20)
apps/web/lib/types/booking.ts(0 hunks)apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts(1 hunks)apps/web/test/utils/bookingScenario/types.d.ts(1 hunks)packages/features/bookings/di/InstantBookingCreateService.module.ts(2 hunks)packages/features/bookings/di/RecurringBookingService.module.ts(1 hunks)packages/features/bookings/di/RegularBookingService.module.ts(2 hunks)packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx(1 hunks)packages/features/bookings/lib/dto/schema.ts(1 hunks)packages/features/bookings/lib/dto/types.d.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(5 hunks)packages/features/bookings/lib/index.ts(1 hunks)packages/features/bookings/lib/service/InstantBookingCreateService.test.ts(6 hunks)packages/features/bookings/lib/service/InstantBookingCreateService.ts(5 hunks)packages/features/bookings/lib/service/RecurringBookingService.ts(1 hunks)packages/features/bookings/lib/service/RegularBookingService.ts(10 hunks)packages/features/di/modules/Cache.ts(1 hunks)packages/features/di/modules/CheckBookingAndDurationLimits.ts(1 hunks)packages/features/instant-meeting/schema.ts(1 hunks)packages/platform/libraries/bookings.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/types/booking.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
- packages/platform/libraries/bookings.ts
- apps/web/test/utils/bookingScenario/types.d.ts
- packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts
- packages/features/instant-meeting/schema.ts
- packages/features/bookings/lib/service/InstantBookingCreateService.test.ts
🧰 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:
packages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/RegularBookingService.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/InstantBookingCreateService.module.ts
📚 Learning: 2025-09-09T08:58:55.970Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/lib/di/containers/InsightsBooking.ts:10-12
Timestamp: 2025-09-09T08:58:55.970Z
Learning: The moduleLoader pattern is being implemented incrementally, focusing only on modules that are directly or indirectly used by Booking modules. Other modules like InsightsBooking are intentionally excluded from this migration phase.
Applied to files:
packages/features/bookings/di/RegularBookingService.module.ts
🧬 Code graph analysis (5)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (2)
packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/features/bookings/lib/dto/types.d.ts (2)
CreateInstantBookingData(16-16)InstantBookingCreateResult(49-56)
packages/features/bookings/lib/dto/types.d.ts (1)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
RegularBookingService(2607-2636)
packages/features/di/modules/CheckBookingAndDurationLimits.ts (3)
packages/features/di/di.ts (2)
bindModuleToClassOnToken(57-113)ModuleLoader(4-4)packages/features/di/tokens.ts (1)
DI_TOKENS(5-62)packages/features/di/modules/CheckBookingLimits.ts (1)
moduleLoader(13-18)
packages/features/di/modules/Cache.ts (3)
packages/features/di/di.ts (2)
bindModuleToClassOnToken(57-113)ModuleLoader(4-4)packages/features/di/tokens.ts (1)
DI_TOKENS(5-62)packages/features/di/modules/Features.ts (1)
moduleLoader(11-16)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
packages/lib/CalEventParser.ts (1)
getVideoCallUrlFromCalEvent(422-439)
⏰ 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 (17)
packages/features/di/modules/Cache.ts (1)
8-21: Helper-based loader wiring looks goodSwitching to
bindModuleToClassOnTokencleanly wires the cache service and auto-loads its dependencies. Nice consistency win across the DI modules.packages/features/di/modules/CheckBookingAndDurationLimits.ts (1)
11-24: Consistent DI helper usage confirmedThe service now uses the shared binding helper with its
checkBookingLimitsServicedependency wired viadepsMap, aligning with the new module-loader pattern.packages/features/bookings/lib/index.ts (1)
5-5: LGTM! Path update aligns with client code reorganization.The import path correctly reflects the relocation of the mapper to
lib/client/, maintaining the same public API surface while organizing client-side code more clearly.apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts (1)
3-3: LGTM!The import path consolidation aligns with the PR's objective to centralize test utility types. The type-only import is appropriate and maintains the same type contracts for the mock request/response utilities.
packages/features/bookings/lib/dto/schema.ts (1)
1-1: LGTM!Standard Zod import with no issues.
packages/features/bookings/di/InstantBookingCreateService.module.ts (1)
14-17: DI wiring looks correct.The
depsMapcorrectly wiresprismaClientto match theIInstantBookingCreateServiceDependenciesinterface in the service file. The TODO comment provides helpful context about the planned refactoring to remove the Prisma dependency in favor of repository injection.packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
31-33: Dependency injection implementation is well-structured.The introduction of
IInstantBookingCreateServiceDependencieswith constructor-based DI and propagation ofprismaClientthrough the handler function is consistent with the established pattern and aligns with the DI module wiring.Also applies to: 164-169, 352-352, 355-355
packages/features/bookings/lib/dto/types.d.ts (2)
10-10: LGTM!The import path update correctly reflects the service relocation to the centralized
lib/service/directory as part of this refactor.
49-56: LGTM!Narrowing the
messagefield to the literal type"Success"is more precise thanstringand correctly reflects that this success result always returns the same message. This refinement improves type safety without breaking consumers since it's an output type.packages/features/bookings/lib/service/RecurringBookingService.ts (1)
6-7: LGTM! Import paths correctly updated.The import paths have been updated to reflect the refactored service structure, moving from
./handleNewBookingto./RegularBookingService. This aligns with the PR's objective to consolidate booking services under thelib/service/directory.packages/features/bookings/di/RecurringBookingService.module.ts (1)
1-2: LGTM! Import path correctly updated.The import path has been updated to the new service location, maintaining consistency with the broader refactoring effort to consolidate services under
lib/service/.packages/features/bookings/di/RegularBookingService.module.ts (2)
2-2: LGTM! Import path correctly updated.The import path has been updated to reflect the new service location at
@calcom/features/bookings/lib/service/RegularBookingService.
22-28: LGTM! Dependency map correctly updated with prismaClient.The
depsMaphas been updated to includeprismaClientviaprismaModuleLoader, and unused dependencies have been removed as noted in previous review feedback. The TODO comment appropriately acknowledges that Prisma dependency injection is temporary and will be replaced with repository-based dependencies in a future PR.packages/features/bookings/lib/service/RegularBookingService.ts (4)
54-124: LGTM! Import paths correctly updated.All import paths have been updated to reflect the refactored structure, with utilities moved to their new locations (e.g.,
../handleNewBooking/*→ relative imports within the restructured hierarchy). The changes maintain consistency across the codebase.
412-419: LGTM! Dependencies cleaned up correctly.The
IBookingServiceDependenciesinterface has been updated to remove unused dependencies (featuresRepository,checkBookingLimitsService,hostRepository,oooRepository,attributeRepository) and now only includes the dependencies actually used byRegularBookingService. This aligns with the DI module configuration inRegularBookingService.module.ts.Based on learnings
482-492: TODO acknowledged: Prisma dependency is temporary.The TODO comment appropriately documents that
prismaClientis a temporary dependency that will be replaced with repository-based dependencies in a future PR. This is a reasonable incremental approach to the refactoring.
1257-1259: LGTM! Formatting changes have no semantic impact.The minor formatting adjustments (line breaks, object literal formatting, constructor body) improve readability without changing any logic or behavior.
Also applies to: 1305-1306, 1648-1648, 1924-1931, 2229-2230, 2292-2294, 2608-2608
|
|
||
| export type InstantBookingCreateResult = { | ||
| message: string; | ||
| message: "Success"; |
There was a problem hiding this comment.
Updated the type because the value is "success" always
| featuresRepository: FeaturesRepository; | ||
| checkBookingLimitsService: CheckBookingLimitsService; | ||
| luckyUserService: LuckyUserService; | ||
| hostRepository: HostRepository; | ||
| oooRepository: OooRepository; | ||
| userRepository: UserRepository; | ||
| attributeRepository: AttributeRepository; |
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
3f24155 to
bf5b05a
Compare
E2E results are ready! |
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
bf5b05a to
9ba3f1d
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
9ba3f1d to
fb0bf72
Compare
| import { BookingActionMap, BookingEmailSmsHandler } from "../BookingEmailSmsHandler"; | ||
| import { getAllCredentialsIncludeServiceAccountKey } from "../getAllCredentialsForUsersOnEvent/getAllCredentials"; | ||
| import { refreshCredentials } from "../getAllCredentialsForUsersOnEvent/refreshCredentials"; | ||
| import getBookingDataSchema from "../getBookingDataSchema"; | ||
| import { LuckyUserService } from "../getLuckyUser"; | ||
| import { addVideoCallDataToEvent } from "../handleNewBooking/addVideoCallDataToEvent"; | ||
| import { checkActiveBookingsLimitForBooker } from "../handleNewBooking/checkActiveBookingsLimitForBooker"; | ||
| import { checkIfBookerEmailIsBlocked } from "../handleNewBooking/checkIfBookerEmailIsBlocked"; | ||
| import { createBooking } from "../handleNewBooking/createBooking"; | ||
| import type { Booking } from "../handleNewBooking/createBooking"; |
There was a problem hiding this comment.
Just import location changes because handleNewBooking now renamed to RegularBookingService is one level deeper
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
269-274: Replaceincludewithselectfor Prisma queries.Line 270 uses
include: { attendees: true }, which violates the coding guideline that requires usingselectto fetch only needed data instead ofinclude.As per coding guidelines.
Apply this diff:
- const createBookingObj = { - include: { - attendees: true, - }, - data: newBookingData, - }; + const createBookingObj = { + select: { + id: true, + uid: true, + userId: true, + responses: true, + title: true, + customInputs: true, + attendees: { + select: { + id: true, + email: true, + name: true, + timeZone: true, + locale: true, + phoneNumber: true, + }, + }, + }, + data: newBookingData, + };Note: Adjust the
selectfields based on which properties ofnewBookingare actually used at lines 316-321 and 332-345.
♻️ Duplicate comments (1)
apps/web/test/utils/bookingScenario/types.d.ts (1)
1-6: Type intersection still creates compatibility issues.The intersections
NextApiRequest & RequestandNextApiResponse & Responseremain problematic as previously flagged. These types have incompatible property shapes (e.g.,headers,method,status(),json()), which will cause TypeScript errors when the test code attempts to satisfy both contracts simultaneously.Even in test scenarios, consider using explicit adapter functions that map only the required fields at the boundary instead of forcing incompatible type intersections.
🧹 Nitpick comments (1)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
1649-1649: Line exceeds recommended length.Line 1649 is very long (combining two enum checks with
||). Consider breaking it for better readability.- (eventType.schedulingType === SchedulingType.ROUND_ROBIN || eventType.schedulingType === SchedulingType.COLLECTIVE) && + (eventType.schedulingType === SchedulingType.ROUND_ROBIN || + eventType.schedulingType === SchedulingType.COLLECTIVE) &&
📜 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 (21)
apps/api/v2/src/lib/services/regular-booking.service.ts(1 hunks)apps/web/lib/types/booking.ts(0 hunks)apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts(1 hunks)apps/web/test/utils/bookingScenario/types.d.ts(1 hunks)packages/features/bookings/di/InstantBookingCreateService.module.ts(2 hunks)packages/features/bookings/di/RecurringBookingService.module.ts(1 hunks)packages/features/bookings/di/RegularBookingService.module.ts(2 hunks)packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx(1 hunks)packages/features/bookings/lib/dto/schema.ts(1 hunks)packages/features/bookings/lib/dto/types.d.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(5 hunks)packages/features/bookings/lib/index.ts(1 hunks)packages/features/bookings/lib/service/InstantBookingCreateService.test.ts(6 hunks)packages/features/bookings/lib/service/InstantBookingCreateService.ts(5 hunks)packages/features/bookings/lib/service/RecurringBookingService.ts(1 hunks)packages/features/bookings/lib/service/RegularBookingService.ts(10 hunks)packages/features/di/modules/Cache.ts(1 hunks)packages/features/di/modules/CheckBookingAndDurationLimits.ts(1 hunks)packages/features/instant-meeting/schema.ts(1 hunks)packages/platform/libraries/bookings.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/types/booking.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts
- apps/api/v2/src/lib/services/regular-booking.service.ts
- packages/platform/libraries/bookings.ts
- packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
- packages/features/bookings/lib/dto/schema.ts
- apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts
- packages/features/instant-meeting/schema.ts
- packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts
🧰 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/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.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/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/lib/index.tspackages/features/bookings/lib/dto/types.d.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
📚 Learning: 2025-10-11T20:10:06.308Z
Learnt from: Udit-takkar
PR: calcom/cal.com#24324
File: packages/lib/server/repository/bookingReport.interface.ts:13-19
Timestamp: 2025-10-11T20:10:06.308Z
Learning: In `packages/lib/server/repository/bookingReport.interface.ts`, the `BookingReportSummary` interface intentionally does not include a `bookingId` field as per the design decision for the report booking feature.
Applied to files:
packages/features/bookings/lib/service/RegularBookingService.ts
📚 Learning: 2025-09-09T08:58:55.970Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/lib/di/containers/InsightsBooking.ts:10-12
Timestamp: 2025-09-09T08:58:55.970Z
Learning: The moduleLoader pattern is being implemented incrementally, focusing only on modules that are directly or indirectly used by Booking modules. Other modules like InsightsBooking are intentionally excluded from this migration phase.
Applied to files:
packages/features/bookings/di/RegularBookingService.module.ts
🧬 Code graph analysis (7)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
packages/features/bookings/lib/dto/types.d.ts (1)
CreateInstantBookingData(16-16)
packages/features/di/modules/CheckBookingAndDurationLimits.ts (3)
packages/features/di/di.ts (2)
bindModuleToClassOnToken(57-113)ModuleLoader(4-4)packages/features/di/tokens.ts (1)
DI_TOKENS(5-62)packages/features/di/modules/CheckBookingLimits.ts (1)
moduleLoader(13-18)
packages/features/bookings/lib/dto/types.d.ts (1)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
RegularBookingService(2608-2637)
packages/features/di/modules/Cache.ts (3)
packages/features/di/di.ts (2)
bindModuleToClassOnToken(57-113)ModuleLoader(4-4)packages/features/di/tokens.ts (1)
DI_TOKENS(5-62)packages/features/di/modules/Features.ts (1)
moduleLoader(11-16)
packages/features/bookings/lib/service/RegularBookingService.ts (2)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)packages/lib/CalEventParser.ts (1)
getVideoCallUrlFromCalEvent(422-439)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (3)
packages/prisma/index.ts (1)
PrismaClient(84-84)packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/features/bookings/lib/dto/types.d.ts (2)
CreateInstantBookingData(16-16)InstantBookingCreateResult(49-56)
🔇 Additional comments (27)
packages/features/bookings/lib/index.ts (1)
5-5: LGTM! Path update aligns with the refactoring.The updated import path correctly reflects the move of client-side code to the
lib/client/directory as stated in the PR objectives. Since this is a re-export in an index file, downstream consumers importing from this index will continue to work without changes.packages/features/di/modules/Cache.ts (3)
2-4: LGTM! Imports support the new DI pattern.The new imports for
bindModuleToClassOnToken,ModuleLoadertype, andfeaturesRepositoryModuleLoaderare necessary for the refactored module loader pattern and align with similar changes across other DI modules.
18-21: LGTM! ModuleLoader export follows established pattern.The exported
moduleLoadercorrectly implements theModuleLoaderinterface and follows the same pattern used in other DI modules (e.g., Features.ts). This enables consumers to load the cache service module along with its dependencies.
8-16: Constructor signature verified—depsMap structure is correct.The
CacheServiceconstructor expectsdependencies: ICacheService, whereICacheServicedefines{ featuresRepository: IFeaturesRepository }. ThedepsMapprovided tobindModuleToClassOnTokenmatches this structure perfectly, ensuring correct dependency injection.packages/features/bookings/di/RegularBookingService.module.ts (1)
1-37: LGTM! Module refactoring follows consistent DI patterns.The file correctly:
- Updates the RegularBookingService import path to the new service location
- Wires dependencies using the moduleLoader pattern with
bindModuleToClassOnToken- Includes appropriate TODO for future Prisma dependency removal
- Exports the moduleLoader and type for downstream consumption
The dependency surface is clean and aligns with the incremental DI migration strategy mentioned in the learnings.
packages/features/di/modules/CheckBookingAndDurationLimits.ts (1)
1-24: LGTM! DI pattern migration completed correctly.The module correctly migrates from inline binding to the
bindModuleToClassOnTokenpattern, which:
- Automatically loads dependencies (checkBookingLimitsService)
- Follows the consistent ModuleLoader structure
- Properly types the exported moduleLoader
This aligns with the incremental DI refactor mentioned in the learnings.
packages/features/bookings/lib/service/RecurringBookingService.ts (1)
6-7: LGTM! Import paths correctly updated.The import path adjustments align with the service module reorganization:
RegularBookingServicemoved from./handleNewBookingto./RegularBookingServiceIBookingServicemoved from./interfaces/to../interfaces/Both are correctly marked as type-only imports, which is good practice.
packages/features/bookings/di/InstantBookingCreateService.module.ts (1)
1-24: LGTM! Prisma dependency injection correctly implemented.The module correctly:
- Updates the InstantBookingCreateService import path to match the new service location
- Injects prismaClient via prismaModuleLoader in the depsMap
- Includes TODO documenting the plan to replace direct Prisma usage with repository injection
- Exports moduleLoader following the consistent pattern
This aligns with the DI refactor strategy across booking services.
packages/features/bookings/di/RecurringBookingService.module.ts (1)
1-2: LGTM! Import path correctly updated.The import path is updated to match the new service module location, consistent with the broader reorganization moving booking handlers to
lib/service/.packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (3)
1-17: LGTM! Test imports and setup correctly refactored.The changes improve test quality by:
- Using strong typing (
CreateInstantBookingData) for test payloads- Updating import paths to match the reorganized service structure
- Removing the unnecessary
NextApiRequestabstraction
88-105: LGTM! Test data properly typed and structured.The mock booking data is now:
- Strongly typed as
CreateInstantBookingData- Includes the required
instant: trueflag- Passes data directly to
createBookinginstead of wrapping in a request bodyThis makes the test cleaner and more maintainable.
159-177: LGTM! Second test case correctly structured.Consistent with the first test, the mock data is properly typed and includes
instant: true. The test correctly verifies the error handling for non-team event types.packages/features/bookings/lib/dto/types.d.ts (3)
10-10: LGTM! Import path correctly updated.The RegularBookingService import path is updated to reflect the new service module location.
46-47: LGTM! Type inference pattern follows best practices.Using
Awaited<ReturnType<RegularBookingService["createBooking"]>>to infer the result type:
- Maintains type safety as the implementation evolves
- Avoids duplication of type definitions
- The TODO comment documents the plan to define the type directly in a follow-up
49-56: LGTM! More precise type for message field.Changing the
messagefield fromstringto the literal"Success"provides stronger type safety and makes the contract more explicit. This aligns with the developer's comment about the value always being "success".packages/features/bookings/lib/service/RegularBookingService.ts (9)
55-61: LGTM! Type imports added for webhook and repository support.The new type-only imports for
EventPayloadType,EventTypeInfo, andBookingRepositorysupport the refactored service structure without changing runtime behavior.
96-125: LGTM! Import paths correctly updated for new module structure.All imports are updated to reflect the reorganized module structure:
- Service utilities moved to appropriate subdirectories
- Import paths adjusted from relative to new locations
- Type-only imports properly marked
The organization improves discoverability and maintains logical grouping.
413-420: LGTM! Dependencies interface correctly reflects reduced surface.The
IBookingServiceDependenciesinterface now includes:
cacheServicecheckBookingAndDurationLimitsServiceprismaClient(with TODO for future removal)bookingRepositoryluckyUserServiceuserRepositoryThis matches the depsMap in the DI module and aligns with the learnings about the interface structure. Based on learnings.
483-493: LGTM! TODO documents planned Prisma removal.The TODO comment clearly documents the plan to:
- Remove direct Prisma dependency
- Inject repositories instead
- Move queries to appropriate repositories
This is a reasonable incremental approach for the refactor.
2228-2232: No indentation issues found.The code follows standard indentation conventions: base indent (2 spaces) → ternary level (4 spaces) → object property level (6 spaces) → closing brace (4 spaces). The indentation is consistent and correct.
1256-1261: No indentation issue found. The multiline ternary’s braces and colon align consistently with other patterns in this file (e.g. lines 2229–2232). No change needed.Likely an incorrect or invalid review comment.
2292-2295: Confirm necessity of theas Prisma.JsonValuecast
Indentation is consistent; verify whethermetadata.appsis already typed asPrisma.JsonValueand remove the cast if it isn’t needed.
1306-1307: Nested ternary indentation is consistent; no changes required.
1924-1932: Indentation is consistent; no formatting adjustments required.Likely an incorrect or invalid review comment.
packages/features/bookings/lib/service/InstantBookingCreateService.ts (3)
25-25: LGTM: Import changes support the DI refactoring.The type import for
PrismaClientand the relocated import forsubscriptionSchemaalign with the PR's goals of dependency injection and code organization.Also applies to: 29-29
31-33: LGTM: Well-defined dependency interface.The
IInstantBookingCreateServiceDependenciesinterface properly encapsulates the service's dependencies and follows TypeScript conventions.
351-356: LGTM: Dependency injection properly implemented.The class constructor correctly accepts and stores dependencies, and the
createBookingmethod properly propagates them to the handler function. This implementation aligns with the PR's goal of introducing DI.
joeauyeung
left a comment
joeauyeung
left a comment
There was a problem hiding this comment.
LGTM and builds properly. Awesome refactor @hariombalhara
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
fb0bf72 to
a9b00d4
Compare
volnei
left a comment
volnei
left a comment
There was a problem hiding this comment.
Important to move away from injecting Prisma directly as you mentioned in follow up PR.
|
@volnei Yeah the PR is ready, but has many file changes. I will open it up for review right after it. -- Edit |
hariombalhara
deleted the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
5 participants
What does this PR do?
This PR is part 3 of the booking flow refactor that renames booking handlers to proper service classes and reorganizes the code structure for better maintainability. This is a pure refactoring with no functional changes to booking behavior.
Key Changes:
Service Renaming & Reorganization:
handleNewBooking.ts→RegularBookingService.tshandleNewRecurringBooking.ts→RecurringBookingService.tshandleInstantMeeting.ts→InstantBookingCreateService.tspackages/features/bookings/lib/service/directoryDependency Injection Updates:
InstantBookingCreateServicenow properly receivesprismaClientvia constructor DIprismaModuleLoaderdependencyType System Improvements:
apps/web/lib/types/booking.tslib/dto/schema.tsfor shared schemas (movedinstantMeetingSubscriptionSchema)Awaited<ReturnType<...>>pattern for better type inferenceCreateSeatedBookingInputtypeCode Organization:
lib/client/subdirectory (booking-to-mutation-input-mapper.tsx)Related PR: This follows the same refactoring pattern established in previous parts of the booking flow refactor series.
Session Info:
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
This is a pure refactoring PR with no functional changes. Testing should focus on verifying that existing functionality still works:
Environment Setup:
Testing Steps:
yarn type-check:ci --forceyarn testExpected Results:
Human Review Checklist
InstantBookingCreateServiceDI module correctly providesprismaClientdependencyAwaited<ReturnType<...>>pattern works correctly for all type consumersChecklist