feat: [Booking flow refactor - 0] Add RegularBookingService, RecurringBookingService and InstantBookingCreateService[Skeleton-No Integration]#23277
Conversation
WalkthroughIntroduces framework-agnostic booking DTO types and adds DI tokens, modules, and containers for Regular, Recurring, and Instant booking services. Exposes RegularBookingService and RecurringBookingService that delegate to existing handlers, plus InstantBookingCreateService for instant bookings. Updates recurring booking handler input types to use CreateRecurringBookingData and CreateBookingMeta. Refactors instant meeting handler to a data-driven internal function and new service, keeping the default export wrapper. Adds generic service interfaces (IBookingService, IBookingCreateService). Extends bookingCreateBody schema with an optional creationSource. API routes only reorder imports and add commented scaffolding; no functional path changes. Possibly related PRs
✨ 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
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
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
hariombalhara
changed the title
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
2 times, most recently
from
fb87dab to
ad5e761
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
from
8c39c21 to
e9637a7
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
2 times, most recently
from
bb57189 to
de339f7
Compare
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
from
de339f7 to
83c28e6
Compare
There was a problem hiding this comment.
handleNewBooking is being moved to services in a stacked PR as BookingCreateService
hariombalhara
changed the title
dosubot
bot
added
the
bookings
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
from
67fe765 to
e517ee5
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 (3)
packages/features/instant-meeting/handleInstantMeeting.ts (3)
74-91: Fix: webhook promises are never awaited (map callback doesn’t return).The array built by map contains undefined entries; webhooks may never execute before the function resolves.
Apply:
- const promises = subscribers.map((sub) => { - sendGenericWebhookPayload({ - secretKey: sub.secret, - triggerEvent: eventTrigger, - createdAt: new Date().toISOString(), - webhook: sub, - data: webhookData, - }).catch((e) => { - console.error( - `Error executing webhook for event: ${eventTrigger}, URL: ${sub.subscriberUrl}`, - sub, - e - ); - }); - }); - - await Promise.all(promises); + const promises = subscribers.map((sub) => + sendGenericWebhookPayload({ + secretKey: sub.secret, + triggerEvent: eventTrigger, + createdAt: new Date().toISOString(), + webhook: sub, + data: webhookData, + }).catch((e) => { + logger.error("Instant meeting webhook delivery failed", { + eventTrigger, + webhookId: sub.id, + url: sub.subscriberUrl, + error: e?.message, + }); + }) + ); + + await Promise.all(promises);
82-87: Don’t log secrets or full subscriber object.sub contains secret; avoid leaking in logs. Use structured log with minimal fields.
Already addressed in the diff above; ensure we never log sub or secret directly.
259-267: Prisma: replace include with select (and only fetch needed fields).Guideline: never use include; select only required fields. attendees aren’t used.
- const createBookingObj = { - include: { - attendees: true, - }, - data: newBookingData, - }; - - const newBooking = await prisma.booking.create(createBookingObj); + const newBooking = await prisma.booking.create({ + data: newBookingData, + select: { + id: true, + uid: true, + userId: true, + responses: true, + title: true, + }, + });
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
2480-2496: Bug: bookingMeta.noEmail is ignored by handler; merge into bookingData.handler reads noEmail from bookingData, not top-level input. Current calls drop bookingMeta.noEmail.
Apply:
async createBooking(input: { bookingData: CreateRegularBookingData; bookingMeta?: CreateBookingMeta }) { - // deps to be passed to handler in follow-up PR - return handler({ bookingData: input.bookingData, ...input.bookingMeta }); + // deps to be passed to handler in follow-up PR + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta as { noEmail?: boolean }; + return handler({ bookingData, ...restMeta }); } async rescheduleBooking(input: { bookingData: CreateRegularBookingData; bookingMeta?: CreateBookingMeta }) { - return handler({ bookingData: input.bookingData, ...input.bookingMeta }); + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta as { noEmail?: boolean }; + return handler({ bookingData, ...restMeta }); } async rescheduleBookingForApiV1(input: { bookingData: CreateRegularBookingData; bookingMeta?: CreateBookingMeta; bookingDataSchemaGetter: BookingDataSchemaGetter; }) { - return handler({ bookingData: input.bookingData, ...input.bookingMeta }, input.bookingDataSchemaGetter); + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta as { noEmail?: boolean }; + return handler({ bookingData, ...restMeta }, input.bookingDataSchemaGetter); }
🧹 Nitpick comments (13)
test-container.js (4)
16-20: Dependency mapping is unused by Service2’s constructor.You bind dep1 → SERVICE1, but Service2 takes no args. Either accept the dep or drop the mapping.
Option A — inject dep into Service2:
class Service2 { - constructor() { - console.log("Service2 created"); - } + constructor(dep1) { + console.log("Service2 created"); + this.dep1 = dep1; + } }module2.bind(tokens.SERVICE2).toClass(Service2, { - dep1: tokens.SERVICE1, + dep1: tokens.SERVICE1, });Option B — remove unused mapping:
-module2.bind(tokens.SERVICE2).toClass(Service2, { - dep1: tokens.SERVICE1, -}); +module2.bind(tokens.SERVICE2).toClass(Service2);Also applies to: 29-31
33-34: Duplicate module load for MODULE1.Loading the same module twice can rebind providers or mask bugs. Remove the duplicate unless you’re explicitly testing idempotency.
-container.load(tokens.MODULE1, module1); container.load(tokens.MODULE2, module2);
10-14: Constructor console logs will spam test output.Prefer a logger or guard logs behind NODE_ENV to keep CI clean.
Also applies to: 16-20
35-36: Unused variable.service is assigned but never used. Remove or prefix with _ to satisfy linters.
-const service = container.get(tokens.SERVICE2); -// console.log(service); +// const _service = container.get(tokens.SERVICE2);packages/features/bookings/lib/interfaces/IBookingService.ts (2)
2-2: Typo in comment (“inteface”).Fix spelling.
-// "Any" types are used because this inteface is used by RecurringBookingService which accepts an array of bookingData and RegularBookingService which accepts a single bookingData +// "Any" types are used because this interface is used by RecurringBookingService which accepts an array of bookingData and RegularBookingService which accepts a single bookingData
4-9: Avoid any; make the contract generic.Provide type-safety while staying flexible.
-export interface IBookingService { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - createBooking: (...args: any[]) => Promise<any>; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - rescheduleBooking: (...args: any[]) => Promise<any>; -} +export interface IBookingService< + TCreateInput = unknown, + TCreateResult = unknown, + TRescheduleInput = TCreateInput, + TRescheduleResult = TCreateResult +> { + createBooking: (input: TCreateInput) => Promise<TCreateResult>; + rescheduleBooking: (input: TRescheduleInput) => Promise<TRescheduleResult>; +}packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts (1)
25-36: Minor: make helper signature explicit and exported inline.Small clarity win; no behavior change.
-// Load the dependencies defined for the module above -function loadModuleDeps(container: Container) { +// Load the dependencies defined for the module above +export function loadModuleDeps(container: Container): void { container.load(DI_TOKENS.CACHE_SERVICE_MODULE, cacheModule); container.load( DI_TOKENS.CHECK_BOOKING_AND_DURATION_LIMITS_SERVICE_MODULE, checkBookingAndDurationLimitsModule ); container.load(DI_TOKENS.PRISMA_MODULE, prismaModule); container.load(DI_TOKENS.BOOKING_REPOSITORY_MODULE, bookingRepositoryModule); container.load(DI_TOKENS.FEATURES_REPOSITORY_MODULE, featuresRepositoryModule); container.load(DI_TOKENS.CHECK_BOOKING_LIMITS_SERVICE_MODULE, checkBookingLimitsModule); }packages/features/instant-meeting/handleInstantMeeting.ts (4)
92-94: Use a single logger; drop console.error.Prefer logger.error with structured payload; avoid duplicate logs.
Apply:
- } catch (error) { - console.error("Error executing webhook", error); - logger.error("Error while sending webhook", error); - } + } catch (error) { + logger.error("Error while sending instant meeting webhooks", { error }); + }
299-301: Store Date object, not ISO string.updatedAt likely a DateTime; pass Date for consistency.
- updatedAt: new Date().toISOString(), + updatedAt: new Date(),
303-313: PII risk: webhook payload contains responses/customInputs.Confirm policy for INSTANT_MEETING webhooks; consider a DTO that whitelists fields (align with restricted booking DTOs elsewhere).
I can draft a minimal InstantMeetingWebhookDTO and a mapper if you want.
347-350: Prefer named export over default (follow repo guidance).A named handler improves tree-shaking and refactors. Keep wrapper temporarily if needed, but plan removal.
packages/features/bookings/lib/handleNewRecurringBooking.ts (1)
120-123: Deps interface defined but unused in handler path.Non-blocking; track in follow-up PR to thread deps into handler.
packages/features/bookings/lib/handleNewBooking.ts (1)
2463-2470: DI surface added but not yet used.Skeleton is fine; plan replacement of get*Service calls with deps in a follow-up.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/web/pages/api/book/event.ts(2 hunks)apps/web/pages/api/book/instant-event.ts(1 hunks)apps/web/pages/api/book/recurring-event.ts(1 hunks)packages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/containers/RecurringBookingServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/containers/RegularBookingServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts(1 hunks)packages/features/bookings/lib/di/modules/RecurringBookingServiceModule.ts(1 hunks)packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts(1 hunks)packages/features/bookings/lib/dto/types.d.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(5 hunks)packages/features/bookings/lib/handleNewRecurringBooking.ts(3 hunks)packages/features/bookings/lib/interfaces/IBookingCreateService.ts(1 hunks)packages/features/bookings/lib/interfaces/IBookingService.ts(1 hunks)packages/features/instant-meeting/handleInstantMeeting.ts(6 hunks)packages/lib/di/tokens.ts(1 hunks)packages/prisma/zod/custom/booking.ts(2 hunks)test-container.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts
- packages/features/bookings/lib/di/containers/RecurringBookingServiceContainer.ts
- packages/features/bookings/lib/interfaces/IBookingCreateService.ts
- packages/prisma/zod/custom/booking.ts
- packages/features/bookings/lib/di/modules/RecurringBookingServiceModule.ts
- apps/web/pages/api/book/instant-event.ts
- apps/web/pages/api/book/event.ts
- apps/web/pages/api/book/recurring-event.ts
- packages/lib/di/tokens.ts
- packages/features/bookings/lib/dto/types.d.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
test-container.jspackages/features/bookings/lib/di/containers/RegularBookingServiceContainer.tspackages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.tspackages/features/bookings/lib/interfaces/IBookingService.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/di/modules/RegularBookingServiceModule.tspackages/features/bookings/lib/handleNewRecurringBooking.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/di/containers/RegularBookingServiceContainer.tspackages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.tspackages/features/bookings/lib/interfaces/IBookingService.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/di/modules/RegularBookingServiceModule.tspackages/features/bookings/lib/handleNewRecurringBooking.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/di/containers/RegularBookingServiceContainer.tspackages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.tspackages/features/bookings/lib/interfaces/IBookingService.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/bookings/lib/di/modules/RegularBookingServiceModule.tspackages/features/bookings/lib/handleNewRecurringBooking.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/IBookingService.ts
🧠 Learnings (6)
📓 Common learnings
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.
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 BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
📚 Learning: 2025-08-08T09:12:08.280Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/features/instant-meeting/handleInstantMeeting.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/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.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 BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (6)
packages/features/bookings/lib/di/containers/RegularBookingServiceContainer.ts (3)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-60)packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts (4)
regularBookingServiceModule(14-14)RegularBookingService(39-39)loadModuleDeps(38-38)moduleToken(38-38)packages/features/bookings/lib/handleNewBooking.ts (1)
RegularBookingService(2477-2496)
packages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.ts (3)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-60)packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts (2)
instantBookingCreateServiceModule(6-6)InstantBookingCreateService(11-11)packages/features/instant-meeting/handleInstantMeeting.ts (1)
InstantBookingCreateService(339-345)
packages/features/instant-meeting/handleInstantMeeting.ts (5)
packages/features/bookings/lib/dto/types.d.ts (2)
CreateInstantBookingData(17-17)CreateInstantBookingResponse(44-51)packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
getEventTypesFromDB(16-224)packages/features/bookings/lib/handleNewBooking/getBookingData.ts (1)
getBookingData(86-86)packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts (1)
InstantBookingCreateService(11-11)packages/features/bookings/lib/interfaces/IBookingCreateService.ts (1)
IBookingCreateService(4-7)
packages/features/bookings/lib/handleNewBooking.ts (7)
packages/features/calendar-cache/lib/getShouldServeCache.ts (1)
CacheService(7-15)packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts (1)
CheckBookingAndDurationLimitsService(21-54)packages/prisma/index.ts (1)
PrismaClient(83-83)packages/features/flags/features.repository.ts (1)
FeaturesRepository(18-254)packages/lib/intervalLimits/server/checkBookingLimits.ts (1)
CheckBookingLimitsService(15-111)packages/features/bookings/lib/interfaces/IBookingService.ts (1)
IBookingService(4-9)packages/features/bookings/lib/dto/types.d.ts (3)
CreateRegularBookingData(15-15)CreateBookingMeta(32-38)BookingDataSchemaGetter(13-13)
packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts (8)
packages/lib/di/tokens.ts (1)
DI_TOKENS(1-60)packages/features/bookings/lib/handleNewBooking.ts (1)
RegularBookingService(2477-2496)packages/lib/di/modules/Cache.ts (1)
cacheModule(8-8)packages/lib/di/modules/CheckBookingAndDurationLimits.ts (1)
checkBookingAndDurationLimitsModule(8-8)packages/prisma/prisma.module.ts (1)
prismaModule(7-7)packages/lib/di/modules/Booking.ts (1)
bookingRepositoryModule(6-6)packages/lib/di/modules/Features.ts (1)
featuresRepositoryModule(6-6)packages/lib/di/modules/CheckBookingLimits.ts (1)
checkBookingLimitsModule(6-6)
packages/features/bookings/lib/handleNewRecurringBooking.ts (3)
packages/features/bookings/lib/dto/types.d.ts (2)
CreateRecurringBookingData(19-21)CreateBookingMeta(32-38)packages/features/bookings/lib/handleNewBooking.ts (1)
RegularBookingService(2477-2496)packages/features/bookings/lib/interfaces/IBookingService.ts (1)
IBookingService(4-9)
⏰ 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 (7)
packages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.ts (1)
11-13: Bindings and type export confirmed
The module file already bindsDI_TOKENS.INSTANT_BOOKING_CREATE_SERVICEtoInstantBookingCreateServiceand re-exports its type, satisfying the container’s import. No further changes needed.packages/features/bookings/lib/di/containers/RegularBookingServiceContainer.ts (1)
16-20: LGTM — container loads deps before resolving.Pattern matches other DI containers; using moduleToken decouples from DI_TOKENS at call site.
packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts (1)
16-23: Constructor injection mapping looks correct, but verify parameter shape.RegularBookingService accepts a single deps object. Ensure keys match IBookingServiceDependencies exactly to avoid undefined injections at runtime.
packages/features/instant-meeting/handleInstantMeeting.ts (1)
159-160: LGTM: typed input/response and creationSource wiring.The service surface and return typing look correct and non-breaking.
Also applies to: 171-179, 256-257, 274-275, 333-345
packages/features/bookings/lib/handleNewRecurringBooking.ts (3)
10-12: LGTM: BookingHandlerInput typed to structured recurring data + meta.
108-114: LGTM: simpler for-of over references.Removes non-null assertions and is clearer.
127-147: LGTM: RecurringBookingService DI + methods.Constructor and delegation pattern look good for the skeleton.
| // const booking = await createBookingThroughFactory(); | ||
| return booking; | ||
|
|
||
| // To be added in the follow-up PR |
There was a problem hiding this comment.
Kept it here to quickly test when making change to the new flow.
| }); | ||
|
|
||
| // Load the dependencies defined for the module above | ||
| function loadModuleDeps(container: Container) { |
There was a problem hiding this comment.
By exposing this loadModuleDeps. I am able to specifiy the dependencies from token as well as load modules in those tokens at a single place. It avoids the chances of having a run-time error where a dependency is defined but no module is loaded against the token.
| container.load(DI_TOKENS.CHECK_BOOKING_LIMITS_SERVICE_MODULE, checkBookingLimitsModule); | ||
| } | ||
|
|
||
| export { loadModuleDeps, moduleToken }; |
There was a problem hiding this comment.
Exposing moduleToken allows the container files to just load this token without hardcoding the token for this module there.
hariombalhara
force-pushed
the
hariom/pri-305-bookingcreateservice-skeleton
branch
from
82919c3 to
10ec033
Compare
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 (3)
packages/features/bookings/lib/handleNewRecurringBooking.ts (1)
14-20: Guard against empty bookingData array.Accessing data[0] without validation can throw when bookingData is empty. Return 400 early.
+import { HttpError } from "@calcom/lib/http-error"; @@ export const handleNewRecurringBooking = async (input: BookingHandlerInput): Promise<BookingResponse[]> => { const data = input.bookingData; + if (!Array.isArray(data) || data.length === 0) { + throw new HttpError({ statusCode: 400, message: "recurring_booking_requires_at_least_one_slot" }); + }Also applies to: 27-31
packages/features/instant-meeting/handleInstantMeeting.ts (2)
74-95: Two issues: webhooks not awaited and secret logged.
- The map callback doesn’t return the Promise, so Promise.all waits on undefineds.
- console.error logs the full webhook, including secret. Don’t log secrets; use structured logger with safe fields.
- const promises = subscribers.map((sub) => { - sendGenericWebhookPayload({ - secretKey: sub.secret, - triggerEvent: eventTrigger, - createdAt: new Date().toISOString(), - webhook: sub, - data: webhookData, - }).catch((e) => { - console.error( - `Error executing webhook for event: ${eventTrigger}, URL: ${sub.subscriberUrl}`, - sub, - e - ); - }); - }); + const promises = subscribers.map((sub) => + sendGenericWebhookPayload({ + secretKey: sub.secret, + triggerEvent: eventTrigger, + createdAt: new Date().toISOString(), + webhook: sub, + data: webhookData, + }).catch((e) => { + logger.error("Instant meeting webhook failed", { + eventTrigger, + webhookId: sub.id, + subscriberUrl: sub.subscriberUrl, + error: e instanceof Error ? e.message : String(e), + }); + }) + ); @@ - } catch (error) { - console.error("Error executing webhook", error); - logger.error("Error while sending webhook", error); - } + } catch (error) { + logger.error("Error while sending webhook", error); + }
260-264: Prisma guideline: avoid include; use select or remove if unused.attendees aren’t used later; drop the include to reduce I/O.
- const createBookingObj = { - include: { - attendees: true, - }, - data: newBookingData, - }; + const createBookingObj = { data: newBookingData };
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
2480-2495: Bug: bookingMeta.noEmail is ignored by handler; merge into bookingData.handler reads noEmail from bookingData, not top-level. As-is, passing bookingMeta.noEmail won’t suppress emails.
Apply in all three methods:
async createBooking(input: { bookingData: CreateRegularBookingData; bookingMeta?: CreateBookingMeta }) { // deps to be passed to handler in follow-up PR - return handler({ bookingData: input.bookingData, ...input.bookingMeta }); + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta; + return handler({ bookingData, ...restMeta }); } async rescheduleBooking(input: { bookingData: CreateRegularBookingData; bookingMeta?: CreateBookingMeta }) { - return handler({ bookingData: input.bookingData, ...input.bookingMeta }); + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta; + return handler({ bookingData, ...restMeta }); } async rescheduleBookingForApiV1(input: { @@ }) { - return handler({ bookingData: input.bookingData, ...input.bookingMeta }, input.bookingDataSchemaGetter); + const meta = input.bookingMeta ?? {}; + const bookingData = + typeof meta.noEmail === "boolean" ? { ...input.bookingData, noEmail: meta.noEmail } : input.bookingData; + const { noEmail, ...restMeta } = meta; + return handler({ bookingData, ...restMeta }, input.bookingDataSchemaGetter); }
🧹 Nitpick comments (5)
packages/features/bookings/lib/handleNewBooking.ts (1)
2461-2461: Prefer named exports over default.Project guideline prefers named exports for better tree-shaking/refactors. Consider exporting handler as a named export in a follow-up.
-export default handler; +export { handler };packages/features/bookings/lib/handleNewRecurringBooking.ts (2)
32-41: OK to forward meta; default areCalendarEventsEnabled?If undefined, consider defaulting to true to match non-recurring behavior.
- const handleBookingMeta = { + const handleBookingMeta = { userId: input.userId, @@ - areCalendarEventsEnabled: input.areCalendarEventsEnabled, + areCalendarEventsEnabled: input.areCalendarEventsEnabled ?? true, };
96-101: Minor: avoid redundant type assertion.forcedSlug is already string | undefined in CreateBookingMeta; the cast is unnecessary.
- forcedSlug: input.forcedSlug as string | undefined, + forcedSlug: input.forcedSlug,packages/features/instant-meeting/handleInstantMeeting.ts (2)
256-256: Only set creationSource when provided to keep DB defaults.Prisma ignores undefined, but being explicit is cleaner.
- creationSource: bookingData.creationSource, + ...(bookingData.creationSource ? { creationSource: bookingData.creationSource } : {}),
347-350: Prefer named export; wrapper slated for removal.Since there’s a TODO to remove this wrapper, fine for now. When removing, also switch to named export to align with guidelines.
-export default async function handler(req: NextApiRequest) { - return _handler(req.body); -} +export async function handler(req: NextApiRequest) { + return _handler(req.body); +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
apps/web/pages/api/book/event.ts(2 hunks)apps/web/pages/api/book/instant-event.ts(1 hunks)apps/web/pages/api/book/recurring-event.ts(1 hunks)packages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/containers/RecurringBookingServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/containers/RegularBookingServiceContainer.ts(1 hunks)packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts(1 hunks)packages/features/bookings/lib/di/modules/RecurringBookingServiceModule.ts(1 hunks)packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts(1 hunks)packages/features/bookings/lib/dto/types.d.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(5 hunks)packages/features/bookings/lib/handleNewRecurringBooking.ts(3 hunks)packages/features/bookings/lib/interfaces/IBookingCreateService.ts(1 hunks)packages/features/bookings/lib/interfaces/IBookingService.ts(1 hunks)packages/features/instant-meeting/handleInstantMeeting.ts(6 hunks)packages/lib/di/tokens.ts(1 hunks)packages/prisma/zod/custom/booking.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/pages/api/book/instant-event.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/features/bookings/lib/interfaces/IBookingService.ts
- packages/features/bookings/lib/di/containers/InstantBookingCreateServiceContainer.ts
- packages/features/bookings/lib/di/modules/InstantBookingCreateServiceModule.ts
- packages/prisma/zod/custom/booking.ts
- apps/web/pages/api/book/recurring-event.ts
- packages/features/bookings/lib/interfaces/IBookingCreateService.ts
- packages/features/bookings/lib/di/containers/RecurringBookingServiceContainer.ts
- packages/features/bookings/lib/di/containers/RegularBookingServiceContainer.ts
- apps/web/pages/api/book/event.ts
- packages/features/bookings/lib/di/modules/RecurringBookingServiceModule.ts
- packages/features/bookings/lib/dto/types.d.ts
- packages/features/bookings/lib/di/modules/RegularBookingServiceModule.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/lib/di/tokens.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewRecurringBooking.tspackages/features/bookings/lib/handleNewBooking.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/di/tokens.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewRecurringBooking.tspackages/features/bookings/lib/handleNewBooking.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/lib/di/tokens.tspackages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewRecurringBooking.tspackages/features/bookings/lib/handleNewBooking.ts
🧠 Learnings (6)
📓 Common learnings
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.
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 BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
📚 Learning: 2025-08-08T09:12:08.280Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/features/instant-meeting/handleInstantMeeting.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/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/instant-meeting/handleInstantMeeting.tspackages/features/bookings/lib/handleNewBooking.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/features/bookings/lib/handleNewBooking.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 BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code graph analysis (3)
packages/features/instant-meeting/handleInstantMeeting.ts (4)
packages/features/bookings/lib/dto/types.d.ts (2)
CreateInstantBookingData(17-17)CreateInstantBookingResponse(44-51)packages/features/bookings/lib/handleNewBooking/getEventTypesFromDB.ts (1)
getEventTypesFromDB(16-224)packages/features/bookings/lib/handleNewBooking/getBookingData.ts (1)
getBookingData(86-86)packages/features/bookings/lib/interfaces/IBookingCreateService.ts (1)
IBookingCreateService(4-7)
packages/features/bookings/lib/handleNewRecurringBooking.ts (3)
packages/features/bookings/lib/dto/types.d.ts (3)
BookingHandlerInput(40-42)CreateRecurringBookingData(19-21)CreateBookingMeta(32-38)packages/features/bookings/lib/handleNewBooking.ts (2)
BookingHandlerInput(409-416)RegularBookingService(2477-2496)packages/features/bookings/lib/interfaces/IBookingService.ts (1)
IBookingService(4-9)
packages/features/bookings/lib/handleNewBooking.ts (7)
packages/features/calendar-cache/lib/getShouldServeCache.ts (1)
CacheService(7-15)packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts (1)
CheckBookingAndDurationLimitsService(21-54)packages/lib/server/repository/booking.ts (1)
BookingRepository(117-948)packages/features/flags/features.repository.ts (1)
FeaturesRepository(18-254)packages/lib/intervalLimits/server/checkBookingLimits.ts (1)
CheckBookingLimitsService(15-111)packages/features/bookings/lib/interfaces/IBookingService.ts (1)
IBookingService(4-9)packages/features/bookings/lib/dto/types.d.ts (3)
CreateRegularBookingData(15-15)CreateBookingMeta(32-38)BookingDataSchemaGetter(13-13)
🔇 Additional comments (6)
packages/features/bookings/lib/handleNewBooking.ts (2)
28-34: Type-only imports look good.Using type-only imports here keeps runtime bundles clean. No action needed.
Also applies to: 40-40, 72-72, 82-82, 131-131
2463-2470: DI surface declared but unused.Looks intentional for this skeleton. Ensure upcoming PRs actually thread these deps into handler to remove singletons.
packages/features/bookings/lib/handleNewRecurringBooking.ts (2)
108-114: Nice: null-assertion removed when iterating references.Safer iteration without non-null assertion. LGTM.
127-147: Service DI added and delegation is clear.Constructor now accepts deps; methods build handler input correctly. No issues.
packages/lib/di/tokens.ts (1)
54-60: LGTM: DI tokens added for new services and modules.Consistent naming and symmetry with existing tokens.
packages/features/instant-meeting/handleInstantMeeting.ts (1)
336-345: Service extraction looks good.InstantBookingCreateService cleanly delegates to internal handler. LGTM.
keithwillcode
deleted the
hariom/pri-305-bookingcreateservice-skeleton
branch
3 participants
What does this PR do?
Prepare for moving handleNewBooking to be part of BookingCreateService. No integration is done and thus should have no impact on bookings flow