chore: [Booking Cancellation Refactor - 0] - Add skeleton for BookingCancelService#24038
Conversation
WalkthroughAdds a bookings cancellation service layer and DI wiring. Introduces a public BookingCancelService class, related DTO/types (CancelRegularBookingData, CancelBookingMeta, HandleCancelBookingResponse), and an IBookingCancelService interface in packages/features/bookings. Adds DI tokens, a BookingCancelService DI module, and a container helper (getBookingCancelService). Adds a NestJS BookingCancelModule and a NestJS BookingCancelService in apps/api/v2 wired to PrismaWriteService. Updates the web cancel route to retrieve server session and use session?.user?.id when invoking cancellation. 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 |
keithwillcode
added
core
hariombalhara
changed the base branch from
graphite-base/24038
to
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
This stack of pull requests is managed by Graphite. Learn more about stacking. |
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
2966093 to
da0457a
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
da0457a to
713a4b5
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
branch
from
80b66bb to
748408d
Compare
hariombalhara
changed the base branch from
hariom/pri-305-bookingcreateservice-rename-handlers-to-services
to
graphite-base/24038
hariombalhara
force-pushed
the
graphite-base/24038
branch
from
748408d to
57150d9
Compare
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
713a4b5 to
fd1bece
Compare
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
fd1bece to
f0e44e1
Compare
| // const bookingCancelService = getBookingCancelService(); | ||
| // const result = await bookingCancelService.cancelBooking({ | ||
| // bookingData: bookingData, | ||
| // bookingMeta: { | ||
| // userId: session?.user?.id || -1, | ||
| // }, | ||
| // }); |
There was a problem hiding this comment.
Example usage, will use in followup PR.
dosubot
bot
added
bookings
![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
🧹 Nitpick comments (1)
apps/web/app/api/cancel/route.ts (1)
35-41: Consider the commented alternative implementation for future migration.The commented code shows the intended future usage of
BookingCancelServicewith proper separation ofbookingDataandbookingMeta. This demonstrates good architectural planning for the service-based approach.Consider adding a TODO comment with a timeline or condition for when this alternative should be activated:
+ // TODO: Migrate to service-based approach once handler export is removed // const bookingCancelService = getBookingCancelService();
📜 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 (8)
apps/api/v2/src/lib/modules/booking-cancel.module.ts(1 hunks)apps/api/v2/src/lib/services/booking-cancel.service.ts(1 hunks)apps/web/app/api/cancel/route.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(4 hunks)packages/features/bookings/lib/interfaces/IBookingCancelService.ts(1 hunks)packages/lib/di/bookings/containers/BookingCancelService.container.ts(1 hunks)packages/lib/di/bookings/modules/BookingCancelService.module.ts(1 hunks)packages/lib/di/bookings/tokens.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*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/interfaces/IBookingCancelService.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:
packages/features/bookings/lib/interfaces/IBookingCancelService.tspackages/lib/di/bookings/tokens.tsapps/api/v2/src/lib/services/booking-cancel.service.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/lib/di/bookings/containers/BookingCancelService.container.tspackages/lib/di/bookings/modules/BookingCancelService.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/web/app/api/cancel/route.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/interfaces/IBookingCancelService.tspackages/lib/di/bookings/tokens.tsapps/api/v2/src/lib/services/booking-cancel.service.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/lib/di/bookings/containers/BookingCancelService.container.tspackages/lib/di/bookings/modules/BookingCancelService.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/web/app/api/cancel/route.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/interfaces/IBookingCancelService.tspackages/lib/di/bookings/tokens.tsapps/api/v2/src/lib/services/booking-cancel.service.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/lib/di/bookings/containers/BookingCancelService.container.tspackages/lib/di/bookings/modules/BookingCancelService.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/web/app/api/cancel/route.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/booking-cancel.service.ts
🧠 Learnings (5)
📓 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/interfaces/IBookingCancelService.tspackages/lib/di/bookings/tokens.tsapps/api/v2/src/lib/services/booking-cancel.service.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/lib/di/bookings/containers/BookingCancelService.container.tspackages/lib/di/bookings/modules/BookingCancelService.module.tspackages/features/bookings/lib/handleCancelBooking.ts
📚 Learning: 2025-08-21T13:55:23.470Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/service/FormWebhookService.ts:0-0
Timestamp: 2025-08-21T13:55:23.470Z
Learning: In the new webhook architecture for Cal.com, schedulePayload is considered legacy code that doesn't fit the clean architecture. The new architecture keeps webhook scheduling logic within the Service layer, specifically through a new method WebhookService.scheduleDelayedWebhooks, rather than using the old centralized schedulePayload helper.
Applied to files:
packages/features/bookings/lib/handleCancelBooking.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
packages/features/bookings/lib/handleCancelBooking.ts
📚 Learning: 2025-09-12T11:23:34.158Z
Learnt from: hariombalhara
PR: calcom/cal.com#23736
File: packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.ts:73-84
Timestamp: 2025-09-12T11:23:34.158Z
Learning: The test file packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.test.ts explicitly documents on line 236 that the current behavior of forceRescheduleForCancelledBooking affecting both CANCELLED and REJECTED bookings is known to be incorrect, but is preserved as "Current Behavior" for backward compatibility. The test comment states the expected behavior should be that REJECTED bookings redirect to booking details even when forceRescheduleForCancelledBooking=true.
Applied to files:
apps/web/app/api/cancel/route.ts
🧬 Code graph analysis (6)
apps/api/v2/src/lib/services/booking-cancel.service.ts (2)
packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(609-620)packages/lib/di/bookings/modules/BookingCancelService.module.ts (1)
BookingCancelService(27-27)
apps/api/v2/src/lib/modules/booking-cancel.module.ts (2)
packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(609-620)packages/lib/di/bookings/modules/BookingCancelService.module.ts (1)
BookingCancelService(27-27)
packages/lib/di/bookings/containers/BookingCancelService.container.ts (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(609-620)
packages/lib/di/bookings/modules/BookingCancelService.module.ts (3)
packages/lib/di/di.ts (1)
bindModuleToClassOnToken(57-113)packages/lib/di/tokens.ts (1)
DI_TOKENS(3-58)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(609-620)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/lib/di/bookings/modules/BookingCancelService.module.ts (1)
BookingCancelService(27-27)packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
IBookingCancelService(2-5)
apps/web/app/api/cancel/route.ts (2)
packages/platform/libraries/index.ts (1)
handleCancelBooking(71-71)packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)
🔇 Additional comments (11)
packages/lib/di/bookings/tokens.ts (1)
4-5: LGTM!The new DI tokens follow the existing naming convention and are properly grouped within the
BOOKING_DI_TOKENSobject. The Symbol usage ensures unique token identity for the DI system.apps/api/v2/src/lib/modules/booking-cancel.module.ts (1)
5-10: LGTM!The NestJS module correctly imports
PrismaModule, provides theBookingCancelService, and exports it for consumption by other modules. The structure follows standard NestJS patterns.apps/web/app/api/cancel/route.ts (2)
28-28: LGTM!Session retrieval is properly implemented using the established pattern with
getServerSessionandbuildLegacyRequest. The session data will be available for extractinguserIdin the cancellation flow.
30-33: LGTM!The
userIdextraction from session follows the correct pattern with a fallback to-1. This maintains backward compatibility while properly integrating session data into the cancellation process.packages/lib/di/bookings/modules/BookingCancelService.module.ts (2)
12-20: LGTM!The DI module correctly uses
bindModuleToClassOnTokenwith proper dependency mapping. TheprismaClientdependency is correctly mapped toprismaModuleLoader, which aligns with theBookingCancelServiceDependenciesinterface.
22-25: LGTM!The exported
moduleLoaderobject provides the standard interface withtokenandloadModulefor the DI container system. This follows the established pattern from other DI modules.apps/api/v2/src/lib/services/booking-cancel.service.ts (1)
6-15: LGTM!The NestJS service correctly extends the base
BookingCancelServiceand properly injects the Prisma client throughPrismaWriteService. The constructor delegation tosuper()with the extractedprismaclient follows the expected DI pattern.packages/features/bookings/lib/handleCancelBooking.ts (3)
32-34: Verify the TODO comment regarding Prisma imports.The TODO suggests that Prisma imports will be handled through DI in a follow-up PR. Ensure this transition plan is tracked properly and the current direct import doesn't conflict with the DI architecture being introduced.
Consider adding a more specific TODO with tracking information:
-// TODO: Prisma import would be used from DI in a followup PR when we remove `handler` export +// TODO: Remove direct prisma import when handler export is removed (tracked in issue/PR #XXXX)
591-600: LGTM!The new type definitions
CancelRegularBookingDataandCancelBookingMetaprovide good separation of concerns.CancelRegularBookingDatacorrectly extracts the booking data type, andCancelBookingMetacontains all optional metadata fields for platform integration.
8-8: No duplicate EventManager import in handleCancelBooking.tsOnly one import exists at packages/features/bookings/lib/handleCancelBooking.ts:8; EventManager is imported in other files as expected — no conflict or import-order issue detected.
packages/lib/di/bookings/containers/BookingCancelService.container.ts (1)
7-13: Confirm idempotent module loading (avoid duplicate bindings/memory leaks).bindModuleToClassOnToken's loadModule calls container.load(moduleToken, module) on every invocation (packages/lib/di/di.ts:101-112), and bookingCancelServiceContainer is created once at module scope while getBookingCancelService() calls loadModule on each use (packages/lib/di/bookings/containers/BookingCancelService.container.ts:7-12). Verify that @evyweb/ioctopus container.load is idempotent / deduplicates by moduleToken; if it re-registers bindings, change to load once (guard flag / API check) or create a per-call container to avoid leaking registrations.
packages/features/bookings/lib/interfaces/IBookingCancelService.ts
Outdated
Show resolved
Hide resolved
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
e9e4963 to
da0a1e8
Compare
|
@hariombalhara can you fix the conflicts? |
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
da0a1e8 to
58eb2d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
603-621: Skeleton implementation is appropriate for this phase.The
BookingCancelServiceclass correctly implements the service pattern:
- Constructor accepts
BookingCancelServiceDependencies(currently onlyprismaClient)cancelBookingmethod mergesbookingDataandbookingMetabefore delegating to the handler- TODO comment (line 618) acknowledges that dependencies aren't passed to the handler yet
This skeleton implementation preserves backward compatibility while establishing the DI infrastructure. The TODO is appropriate for this phase. Past reviewers have noted:
- The interface type mismatch (any types) - acknowledged as follow-up work
- Repository pattern suggestion - acknowledged and addressed in stacked PR #24159
The current implementation achieves the stated goal: "add skeleton for BookingCancelService without integrating into booking flow."
📜 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 (9)
apps/api/v2/src/lib/modules/booking-cancel.module.ts(1 hunks)apps/api/v2/src/lib/services/booking-cancel.service.ts(1 hunks)apps/web/app/api/cancel/route.ts(1 hunks)packages/features/bookings/di/BookingCancelService.container.ts(1 hunks)packages/features/bookings/di/BookingCancelService.module.ts(1 hunks)packages/features/bookings/di/tokens.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(4 hunks)packages/features/bookings/lib/interfaces/IBookingCancelService.ts(1 hunks)packages/features/di/tokens.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/v2/src/lib/modules/booking-cancel.module.ts
- packages/features/bookings/lib/interfaces/IBookingCancelService.ts
- apps/web/app/api/cancel/route.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/di/tokens.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/di/tokens.tspackages/features/bookings/di/BookingCancelService.module.tsapps/api/v2/src/lib/services/booking-cancel.service.tspackages/features/bookings/lib/handleCancelBooking.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/di/tokens.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/di/tokens.tspackages/features/bookings/di/BookingCancelService.module.tsapps/api/v2/src/lib/services/booking-cancel.service.tspackages/features/bookings/lib/handleCancelBooking.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/di/tokens.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/di/tokens.tspackages/features/bookings/di/BookingCancelService.module.tsapps/api/v2/src/lib/services/booking-cancel.service.tspackages/features/bookings/lib/handleCancelBooking.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/booking-cancel.service.ts
🧠 Learnings (2)
📓 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/BookingCancelService.module.tspackages/features/bookings/lib/handleCancelBooking.ts
🧬 Code graph analysis (4)
packages/features/bookings/di/BookingCancelService.container.ts (2)
packages/features/di/di.ts (1)
createContainer(114-114)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(610-621)
packages/features/bookings/di/BookingCancelService.module.ts (3)
packages/features/di/di.ts (1)
bindModuleToClassOnToken(57-113)packages/features/di/tokens.ts (1)
DI_TOKENS(3-63)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(610-621)
apps/api/v2/src/lib/services/booking-cancel.service.ts (2)
packages/features/bookings/di/BookingCancelService.module.ts (1)
BookingCancelService(24-24)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(610-621)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/features/bookings/di/BookingCancelService.module.ts (1)
BookingCancelService(24-24)packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
IBookingCancelService(2-5)
⏰ 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 (4)
packages/features/bookings/di/BookingCancelService.container.ts (1)
8-14: LGTM! Standard DI container pattern.The container creation and module loading follow the established DI pattern in the codebase. The module loader should be idempotent, so repeated calls to
getBookingCancelService()won't cause issues.packages/features/bookings/di/BookingCancelService.module.ts (1)
1-24: LGTM! Module wiring follows established patterns.The DI module correctly:
- Uses tokens from
DI_TOKENS(which should spread fromBOOKING_DI_TOKENSafter fixing the duplication issue)- Binds
BookingCancelServicewithdepsMapcontaining onlyprismaClient, matching theBookingCancelServiceDependenciesinterface- Exports the
moduleLoaderpattern for container integrationBased on learnings from similar services (RegularBookingService), this structure aligns with the established DI architecture.
apps/api/v2/src/lib/services/booking-cancel.service.ts (1)
1-15: LGTM! Clean NestJS service integration.The service correctly:
- Extends the base
BookingCancelServicefrom the features layer- Injects
PrismaWriteServicevia NestJS DI- Passes the Prisma client to the base class constructor
This follows the standard pattern for integrating feature-layer services into the NestJS API layer.
packages/features/bookings/lib/handleCancelBooking.ts (1)
592-601: Type definitions are adequate for skeleton phase.The exported types provide the necessary structure for the service interface. The optional fields in
CancelBookingMetaare appropriate as they represent optional platform-specific metadata.Note: Past reviewer (Udit-takkar) suggested improving type safety by making some fields required. This can be addressed in a follow-up PR once the service is integrated into the booking flow and usage patterns are clearer.
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
58eb2d0 to
06460ee
Compare
|
@Udit-takkar resolved conflicts |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
589-607: Dependencies declared but not used in the skeleton.The
BookingCancelServiceDependenciesare passed to the constructor (line 597) but remain unused incancelBooking(lines 599-606). The method directly invokes the legacyhandlerwithout passingthis.deps(line 605), making thedepsparameter temporarily redundant.This aligns with the PR's skeleton-only scope. The TODO comment (line 604) correctly indicates this will be addressed when migrating away from the exported handler.
📜 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 (10)
apps/api/v2/src/lib/modules/booking-cancel.module.ts(1 hunks)apps/api/v2/src/lib/services/booking-cancel.service.ts(1 hunks)apps/web/app/api/cancel/route.ts(1 hunks)packages/features/bookings/di/BookingCancelService.container.ts(1 hunks)packages/features/bookings/di/BookingCancelService.module.ts(1 hunks)packages/features/bookings/di/tokens.ts(1 hunks)packages/features/bookings/lib/dto/BookingCancel.d.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(3 hunks)packages/features/bookings/lib/interfaces/IBookingCancelService.ts(1 hunks)packages/features/di/tokens.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/features/di/tokens.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/features/bookings/lib/interfaces/IBookingCancelService.ts
- packages/features/bookings/di/BookingCancelService.module.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/app/api/cancel/route.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/di/tokens.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/api/v2/src/lib/services/booking-cancel.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/web/app/api/cancel/route.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/di/tokens.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/api/v2/src/lib/services/booking-cancel.service.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/app/api/cancel/route.tspackages/features/bookings/di/BookingCancelService.container.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/di/tokens.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.tsapps/api/v2/src/lib/services/booking-cancel.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/booking-cancel.service.ts
🧠 Learnings (1)
📚 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/handleCancelBooking.ts
🧬 Code graph analysis (6)
apps/web/app/api/cancel/route.ts (1)
packages/platform/libraries/index.ts (1)
handleCancelBooking(62-62)
packages/features/bookings/di/BookingCancelService.container.ts (3)
packages/features/di/di.ts (1)
createContainer(114-114)packages/features/bookings/di/BookingCancelService.module.ts (1)
BookingCancelService(24-24)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(596-607)
packages/features/bookings/lib/dto/BookingCancel.d.ts (1)
packages/prisma/zod-utils.ts (1)
bookingCancelInput(294-297)
apps/api/v2/src/lib/modules/booking-cancel.module.ts (2)
packages/features/bookings/di/BookingCancelService.module.ts (1)
BookingCancelService(24-24)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(596-607)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
IBookingCancelService(7-12)packages/features/bookings/lib/dto/BookingCancel.d.ts (2)
CancelRegularBookingData(12-12)CancelBookingMeta(17-24)
apps/api/v2/src/lib/services/booking-cancel.service.ts (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(596-607)
🔇 Additional comments (5)
packages/features/bookings/lib/dto/BookingCancel.d.ts (1)
1-35: LGTM! Type definitions are well-structured.The domain types are framework-agnostic and properly documented. The separation of concerns between booking data, metadata, and response is clear.
apps/api/v2/src/lib/modules/booking-cancel.module.ts (1)
1-10: LGTM! NestJS module correctly wired.The module properly imports
PrismaModule, providesBookingCancelService, and exports it for consumption by other modules. The structure follows standard NestJS patterns.apps/web/app/api/cancel/route.ts (1)
28-41: Session retrieval added for future service integration.The session is now retrieved before cancellation (line 28), and the commented-out code (lines 35-41) demonstrates the intended future usage with
bookingCancelService. The current implementation correctly preserves backward compatibility while preparing for the DI-based approach.apps/api/v2/src/lib/services/booking-cancel.service.ts (1)
1-15: LGTM! Service correctly extends base and wires Prisma.The NestJS service properly extends the base
BookingCancelServiceand injectsPrismaWriteServicethrough the constructor, passing the Prisma client to the base class viasuper(). This follows the established DI pattern.packages/features/bookings/di/BookingCancelService.container.ts (1)
8-13: Confirm idempotent behavior ofloadModule:getBookingCancelService()invokescontainer.load(moduleToken, module)on every call—verify that repeated loads viabindModuleToClassOnTokendon’t register the module twice or throw errors.
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
06460ee to
0cb48eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/features/bookings/di/BookingCancelService.container.ts (1)
10-14: Avoid repeated module loading on every invocation.Currently,
loadModuleis called every timegetBookingCancelService()is invoked. Since the container is created once at module scope (line 8), repeatedly loading the same module into the same container could cause issues if the module loader attempts to register the same token multiple times, or is simply inefficient.Consider loading the module once using lazy initialization:
const bookingCancelServiceContainer = createContainer(); +let isModuleLoaded = false; export function getBookingCancelService(): BookingCancelService { - bookingCancelServiceModule.loadModule(bookingCancelServiceContainer); + if (!isModuleLoaded) { + bookingCancelServiceModule.loadModule(bookingCancelServiceContainer); + isModuleLoaded = true; + } return bookingCancelServiceContainer.get<BookingCancelService>(bookingCancelServiceModule.token); }Alternatively, load the module at module scope if immediate initialization is acceptable:
const bookingCancelServiceContainer = createContainer(); +bookingCancelServiceModule.loadModule(bookingCancelServiceContainer); export function getBookingCancelService(): BookingCancelService { - bookingCancelServiceModule.loadModule(bookingCancelServiceContainer); - return bookingCancelServiceContainer.get<BookingCancelService>(bookingCancelServiceModule.token); }
📜 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 (10)
apps/api/v2/src/lib/modules/booking-cancel.module.ts(1 hunks)apps/api/v2/src/lib/services/booking-cancel.service.ts(1 hunks)apps/web/app/api/cancel/route.ts(1 hunks)packages/features/bookings/di/BookingCancelService.container.ts(1 hunks)packages/features/bookings/di/BookingCancelService.module.ts(1 hunks)packages/features/bookings/di/tokens.ts(1 hunks)packages/features/bookings/lib/dto/BookingCancel.d.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(3 hunks)packages/features/bookings/lib/interfaces/IBookingCancelService.ts(1 hunks)packages/features/di/tokens.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/features/di/tokens.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/features/bookings/di/tokens.ts
- apps/web/app/api/cancel/route.ts
- packages/features/bookings/lib/interfaces/IBookingCancelService.ts
- apps/api/v2/src/lib/services/booking-cancel.service.ts
- packages/features/bookings/di/BookingCancelService.module.ts
- packages/features/bookings/lib/dto/BookingCancel.d.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/di/BookingCancelService.container.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.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/di/BookingCancelService.container.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.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/di/BookingCancelService.container.tsapps/api/v2/src/lib/modules/booking-cancel.module.tspackages/features/bookings/lib/handleCancelBooking.ts
🧠 Learnings (1)
📚 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/handleCancelBooking.ts
🧬 Code graph analysis (3)
packages/features/bookings/di/BookingCancelService.container.ts (3)
packages/features/di/di.ts (1)
createContainer(114-114)packages/features/bookings/di/BookingCancelService.module.ts (1)
BookingCancelService(24-24)packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(598-609)
apps/api/v2/src/lib/modules/booking-cancel.module.ts (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
BookingCancelService(598-609)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
IBookingCancelService(7-12)packages/features/bookings/lib/dto/BookingCancel.d.ts (2)
CancelRegularBookingData(12-12)CancelBookingMeta(17-24)
⏰ 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 (2)
apps/api/v2/src/lib/modules/booking-cancel.module.ts (1)
1-10: LGTM! NestJS module structure is correct.The module correctly wires up BookingCancelService with PrismaModule, following standard NestJS dependency injection patterns. The service is both provided and exported, making it available for external use.
packages/features/bookings/lib/handleCancelBooking.ts (1)
591-609: LGTM! Skeleton structure is appropriate for this phase.The
BookingCancelServiceclass correctly implements theIBookingCancelServiceinterface and establishes the DI infrastructure. The TODO comment (Line 606) clearly indicates that dependency injection will be completed in a future phase when the handler export is removed, which aligns with the PR's skeleton-only objectives.
hariombalhara
force-pushed
the
ariom/pri-305-bookingCancelService-skeleton
branch
from
0cb48eb to
406d8fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/features/bookings/lib/dto/BookingCancel.d.ts (1)
17-24: Consider making userId required in future iterations.While all fields being optional in
CancelBookingMetais acceptable for this skeleton implementation, theuserIdfield is critical for authorization. As the service matures beyond the skeleton phase, consider making it required to enforce proper authorization checks.This can be addressed in a follow-up PR when the service is fully integrated:
export type CancelBookingMeta = { - userId?: number; + userId: number; platformClientId?: string; platformRescheduleUrl?: string; platformCancelUrl?: string; platformBookingUrl?: string; arePlatformEmailsEnabled?: boolean; };packages/features/bookings/lib/handleCancelBooking.ts (1)
595-613: Skeleton implementation looks good; track the dependency injection completion.The
BookingCancelServiceclass correctly implements the interface with proper types. The constructor acceptsdepsbut doesn't pass them to the handler (as indicated by the TODO comment), which is acceptable for this skeleton phase.However, to ensure this technical debt is addressed:
Do you want me to open a GitHub issue to track:
- Passing
deps.prismaClientto the handler instead of using the globalprismaimport- Migrating from direct Prisma usage to a Repository pattern (as suggested in previous reviews)
- Removing the exported handler function once all consumers migrate to the service
📜 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 (10)
apps/api/v2/src/lib/modules/booking-cancel.module.ts(1 hunks)apps/api/v2/src/lib/services/booking-cancel.service.ts(1 hunks)apps/web/app/api/cancel/route.ts(1 hunks)packages/features/bookings/di/BookingCancelService.container.ts(1 hunks)packages/features/bookings/di/BookingCancelService.module.ts(1 hunks)packages/features/bookings/di/tokens.ts(1 hunks)packages/features/bookings/lib/dto/BookingCancel.d.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(4 hunks)packages/features/bookings/lib/interfaces/IBookingCancelService.ts(1 hunks)packages/features/di/tokens.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/features/di/tokens.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/features/bookings/di/tokens.ts
- packages/features/bookings/di/BookingCancelService.container.ts
- apps/api/v2/src/lib/services/booking-cancel.service.ts
- apps/api/v2/src/lib/modules/booking-cancel.module.ts
- packages/features/bookings/di/BookingCancelService.module.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/app/api/cancel/route.tspackages/features/bookings/lib/interfaces/IBookingCancelService.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/lib/handleCancelBooking.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/app/api/cancel/route.tspackages/features/bookings/lib/interfaces/IBookingCancelService.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/lib/handleCancelBooking.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/app/api/cancel/route.tspackages/features/bookings/lib/interfaces/IBookingCancelService.tspackages/features/bookings/lib/dto/BookingCancel.d.tspackages/features/bookings/lib/handleCancelBooking.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/interfaces/IBookingCancelService.ts
🧠 Learnings (2)
📚 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/interfaces/IBookingCancelService.tspackages/features/bookings/lib/handleCancelBooking.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/interfaces/IBookingCancelService.ts
🧬 Code graph analysis (4)
apps/web/app/api/cancel/route.ts (2)
packages/platform/libraries/index.ts (1)
handleCancelBooking(62-62)packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)
packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
packages/features/bookings/lib/dto/BookingCancel.d.ts (3)
CancelRegularBookingData(12-12)CancelBookingMeta(17-24)HandleCancelBookingResponse(29-35)
packages/features/bookings/lib/dto/BookingCancel.d.ts (1)
packages/prisma/zod-utils.ts (1)
bookingCancelInput(295-298)
packages/features/bookings/lib/handleCancelBooking.ts (2)
packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
IBookingCancelService(7-12)packages/features/bookings/lib/dto/BookingCancel.d.ts (2)
CancelRegularBookingData(12-12)CancelBookingMeta(17-24)
🔇 Additional comments (4)
packages/features/bookings/lib/interfaces/IBookingCancelService.ts (1)
1-12: LGTM! Clean interface definition with proper types.The interface is well-defined with proper type safety. The previous concern about
anytypes has been resolved.packages/features/bookings/lib/handleCancelBooking.ts (3)
120-125: Approved: Improved condition formatting.The reformatting of the multi-line condition improves readability without changing the logic.
615-615: Default export preserved for backward compatibility.The coding guidelines recommend named exports over default exports for better tree-shaking and refactoring. However, this default export is intentionally preserved for backward compatibility as mentioned in the PR objectives.
As the service integration progresses, consider migrating consumers to the named
BookingCancelServiceexport and eventually removing this default export.
8-8: No duplicate EventManager import Confirmed only a singleEventManagerimport remains inpackages/features/bookings/lib/handleCancelBooking.ts.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
5 participants
What does this PR do?
This PR adds a skeleton implementation for
BookingCancelServicefollowing the same architectural pattern established in PR #23277. The changes include:BookingCancelServiceclass that extends the base service fromhandleCancelBooking.tsIBookingCancelServiceinterface for future implementationshandleCancelBooking.tsto support service-based architectureThis is a skeleton implementation only - the new service is not yet integrated into the booking flow and maintains full backward compatibility with existing functionality.
Link to Devin run: https://app.devin.ai/sessions/998e3ee36c87452785d024698dde8806
PR Description Requested by: @hariombalhara
Visual Demo (For contributors especially)
N/A - This is an architectural refactor with no user-facing changes.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Since this is a skeleton implementation that doesn't modify existing behavior:
Verify existing cancellation flow works unchanged:
Verify DI container setup:
getBookingCancelService()can be instantiated without errorsType checking:
yarn type-check:cito ensure no TypeScript errorsChecklist
Human Review Focus Areas
BookingCancelService.module.tshas all required dependencies and correct token bindingsBookingCancelServiceproperly extends the base class and passes dependencies correctlyIBookingCancelServicewith genericanytypes is appropriate for future implementationshandleCancelBookingexport remains functional and doesn't introduce breaking changes