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: |
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 |
aaf3293 to
d357430
Compare
3b6187c to
4734a13
Compare
d357430 to
2ac051c
Compare
4734a13 to
bf970b4
Compare
f567bd4 to
7546752
Compare
7546752 to
ea92356
Compare
bf970b4 to
85969f9
Compare
ea92356 to
30558e0
Compare
85969f9 to
852ac29
Compare
de2c60a to
34dfdce
Compare
d4e4120 to
5171538
Compare
5171538 to
0680f44
Compare
34dfdce to
80b66bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
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)
268-273: Replaceincludewithselectfor Prisma query.The code uses
include: { attendees: true }which violates the coding guidelines that mandate usingselectinstead ofincludefor all Prisma queries.As per coding guidelines
Apply this diff to use
select:const createBookingObj = { - include: { - attendees: true, - }, + select: { + id: true, + uid: true, + responses: true, + title: true, + startTime: true, + endTime: true, + description: true, + customInputs: true, + status: true, + metadata: true, + userId: true, + attendees: { + select: { + id: true, + email: true, + name: true, + timeZone: true, + locale: true, + phoneNumber: true, + }, + }, + }, data: newBookingData, };Note: Ensure you select all fields that are actually used in the return value and subsequent logic.
🧹 Nitpick comments (1)
packages/features/instant-meeting/schema.ts (1)
1-3: LGTM! Schema centralized to eliminate duplication.Re-exporting the schema from a central location follows the DRY principle and ensures consistency across the codebase.
📜 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 (18)
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(1 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(3 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(1 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(4 hunks)packages/features/bookings/types.ts(2 hunks)packages/features/instant-meeting/schema.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/types/booking.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/index.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.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:
packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/index.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.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:
packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/index.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
**/*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/RegularBookingService.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.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/di/RegularBookingService.module.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.ts
🧬 Code graph analysis (6)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
packages/features/bookings/types.ts (2)
packages/features/bookings/lib/dto/types.d.ts (1)
InstantBookingCreateResult(53-53)packages/platform/libraries/bookings.ts (1)
InstantBookingCreateResult(8-8)
packages/features/bookings/lib/dto/types.d.ts (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
InstantBookingCreateService(350-356)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
packages/features/bookings/lib/dto/types.d.ts (1)
CreateInstantBookingData(18-18)
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(18-18)InstantBookingCreateResult(53-53)
🔇 Additional comments (17)
apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts (1)
3-3: LGTM! Good refactoring.Moving the type imports to a local "./types" module improves code organization and maintainability by centralizing type definitions.
apps/web/test/utils/bookingScenario/types.d.ts (1)
4-6: Verify type intersection compatibility
Install TypeScript and runnpx tsc --noEmit(or validate in your IDE) to ensure combiningNextApiRequest & RequestandNextApiResponse & Responsedoesn’t produce conflicting property types.packages/features/bookings/di/RegularBookingService.module.ts (1)
1-44: LGTM! DI wiring properly updated.The module correctly:
- Updates the import path to the new service location
- Adds the required repository and prisma loaders
- Extends the depsMap to include oooRepository, userRepository, and prismaClient
The dependency injection setup aligns with the service refactoring.
packages/features/bookings/lib/service/RegularBookingService.ts (2)
1-128: LGTM! Import paths normalized.The import path reorganization reflects the new module structure. All imports correctly resolve to their new locations without changing runtime behavior.
1491-1494: LGTM! Boolean expression reformatted without logic change.The reformatting of the
changedOrganizerexpression maintains the same evaluation logic, just presented on a single line for clarity.packages/features/bookings/types.ts (1)
10-10: LGTM! Type alias maintains consistent public API.The new
InstantBookingResponsetype alias follows the same pattern asBookingResponse, providing a clear and consistent interface for instant booking results.Also applies to: 101-101
packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx (1)
9-10: LGTM! Import paths updated to reflect new structure.The import paths correctly reference types from their new locations in the refactored module structure.
packages/features/bookings/lib/dto/types.d.ts (1)
10-11: LGTM! Service imports updated and type derivation improved.The import path updates reflect the new service locations, and deriving
InstantBookingCreateResultfrom the service method's return type establishes a single source of truth, improving maintainability.Also applies to: 53-53
packages/features/bookings/di/InstantBookingCreateService.module.ts (1)
1-23: LGTM! DI wiring properly adds prismaClient dependency.The module correctly:
- Updates the import path to the new service location
- Adds the prismaModuleLoader import
- Configures the depsMap to inject prismaClient
This aligns with the service's updated dependency requirements.
packages/features/bookings/di/RecurringBookingService.module.ts (1)
1-1: LGTM! Import path updated correctly.The import path change from
handleNewRecurringBookingtoservice/RecurringBookingServicealigns with the PR's objective of organizing booking services under proper module paths.packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (2)
88-105: LGTM! Test updated correctly for new API.The test changes properly reflect the new service API where
bookingDatais passed directly as a typed parameter instead of being extracted from a request object. The addition ofinstant: trueis appropriate for instant booking scenarios.
159-177: LGTM! Error test case updated consistently.The error test case follows the same pattern as the success test, ensuring consistency in how the new API is used across different test scenarios.
packages/features/bookings/lib/service/RecurringBookingService.ts (1)
6-7: LGTM! Import paths updated correctly.The import paths have been properly updated to reflect the new module organization, maintaining consistency with the broader refactoring effort.
packages/features/bookings/lib/index.ts (1)
5-5: LGTM! Export path updated correctly.The export path has been updated to reflect the new module location under
client/booking-event-form/, maintaining consistency with the repository's module organization.packages/features/bookings/lib/service/InstantBookingCreateService.ts (3)
31-33: LGTM! Proper dependency injection interface.The
IInstantBookingCreateServiceDependenciesinterface correctly defines the dependencies needed by the service, following good DI practices.
164-168: Verify that exporting the handler is intentional.The
handlerfunction is now exported and has a new signature that acceptsdepsas a parameter. While this improves testability through dependency injection, exporting internal handlers can expose implementation details.Confirm whether this export is intended for:
- Direct testing of the handler
- Use by DI containers
- Other internal service composition
If the export is not needed externally, consider keeping it as an internal implementation detail.
351-355: LGTM! Service class properly implements DI pattern.The
InstantBookingCreateServiceclass correctly implements dependency injection by accepting dependencies through the constructor and passing them to the handler. This improves testability and follows best practices.
| import { BookingStatus, CreationSource } from "@calcom/prisma/enums"; | ||
|
|
||
| import { buildDryRunBooking } from "../../handleNewBooking"; | ||
| import { buildDryRunBooking } from "../../service/BookingCreateService"; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Verify consistent migration across the codebase.
The import path update from ../../handleNewBooking to ../../service/BookingCreateService correctly reflects the refactoring objective of this PR. The use of a named export aligns with coding guidelines.
Run the following script to confirm all references have been migrated:
🏁 Script executed:
#!/bin/bash
# Verify that no other files still import from the old handleNewBooking path
# and that the new BookingCreateService path exists and exports buildDryRunBooking
# Check for lingering imports from old path
echo "=== Checking for remaining imports from old handleNewBooking path ==="
rg -n --type=ts --type=tsx 'from.*handleNewBooking' -g '!*.test.ts' -g '!*.spec.ts'
# Verify the new service module exists and exports buildDryRunBooking
echo -e "\n=== Verifying BookingCreateService exports buildDryRunBooking ==="
fd -e ts -e tsx 'BookingCreateService' --exec rg -nP 'export.*buildDryRunBooking'
# Check if old handleNewBooking file still exists (should be removed or renamed)
echo -e "\n=== Checking if old handleNewBooking module still exists ==="
fd -e ts -e tsx 'handleNewBooking' -x echo "Found: {}"Length of output: 226
🏁 Script executed:
#!/bin/bash
echo "=== Checking for lingering imports from old handleNewBooking path ==="
rg -nP "from ['\"]\.\./\.\./handleNewBooking" -g '*.ts' -g '*.tsx' -g '!*.test.ts' -g '!*.spec.ts'
echo -e "\n=== Verifying existence of BookingCreateService file ==="
fd -e ts -e tsx BookingCreateService --type f -x echo "Found: {}"
echo -e "\n=== Checking BookingCreateService for buildDryRunBooking export ==="
rg -nP "export\s+(?:const|function)\s+buildDryRunBooking" -g '*BookingCreateService.ts' -g '*BookingCreateService.tsx'Length of output: 1404
Migrate all remaining handleNewBooking imports and ensure BookingCreateService export
- Update these files to import from
lib/service/BookingCreateServiceinstead of../../handleNewBooking/...:
• packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
• packages/features/bookings/lib/handleSeats/lib/lastAttendeeDeleteBooking.ts
• packages/features/bookings/lib/handleSeats/reschedule/rescheduleSeatedBooking.ts
• packages/features/bookings/lib/handleSeats/create/createNewSeat.ts - Add or rename
packages/features/bookings/lib/service/BookingCreateService.tsso it exportsbuildDryRunBooking. - Remove the old
handleNewBookingmodule once all references are migrated.
🤖 Prompt for AI Agents
In
packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts
around line 5, the test currently imports buildDryRunBooking from the old
location; update the import to point to the new service module
(packages/features/bookings/lib/service/BookingCreateService) and ensure that
file exports buildDryRunBooking (add or rename BookingCreateService.ts to export
it). After updating this test import, migrate the other listed files to import
from lib/service/BookingCreateService and remove the old handleNewBooking module
once all references are updated.
packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (4)
78-95: Bug: webhook promises not returned; Promise.all doesn’t await anythingsubscribers.map callback doesn’t return the promise, so Promise.all(promises) resolves immediately and webhooks aren’t awaited.
Apply this diff:
- const promises = subscribers.map((sub) => { - sendGenericWebhookPayload({ + const promises = subscribers.map((sub) => { + return sendGenericWebhookPayload({ secretKey: sub.secret, triggerEvent: eventTrigger, createdAt: new Date().toISOString(), webhook: sub, data: webhookData, - }).catch((e) => { + }).catch((e) => { console.error( `Error executing webhook for event: ${eventTrigger}, URL: ${sub.subscriberUrl}`, sub, e ); }); }); - await Promise.all(promises); + await Promise.all(promises);
169-175: Guard against missing event type before spreadingIf getEventTypesFromDB returns null/undefined, spreading with ...eventType will throw at runtime.
Apply this diff:
- let eventType = await getEventTypesFromDB(bookingData.eventTypeId); + let eventType = await getEventTypesFromDB(bookingData.eventTypeId); + if (!eventType) { + throw new Error("Event type not found"); + } const isOrgTeamEvent = !!eventType?.team && !!eventType?.team?.parentId; eventType = { ...eventType, bookingFields: getBookingFieldsWithSystemFields({ ...eventType, isOrgTeamEvent }), };
269-276: Prisma guideline: avoid include; select only required fieldsinclude: { attendees: true } is unused, and we should select only fields we need.
Apply this diff:
- const createBookingObj = { - include: { - attendees: true, - }, - data: newBookingData, - }; + const createBookingObj = { + data: newBookingData, + select: { + id: true, + uid: true, + userId: true, + responses: true, + customInputs: true, + }, + };As per coding guidelines
101-162: JSON.parse can throw and PII leakage in logs
- JSON.parse(subscription) can throw and crash the whole mapping before Promise.allSettled.
- Logging JSON.stringify(sub.user) may leak subscription endpoint and keys.
Apply this diff:
- const promises = subscribers.map((sub) => { - const subscription = sub.user?.NotificationsSubscriptions?.[0]?.subscription; + const promises = subscribers.map((sub) => { + const subscription = sub.user?.NotificationsSubscriptions?.[0]?.subscription; if (!subscription) return Promise.resolve(); - - const parsedSubscription = subscriptionSchema.safeParse(JSON.parse(subscription)); + let raw: unknown = subscription; + try { + raw = typeof subscription === "string" ? JSON.parse(subscription) : subscription; + } catch (e) { + logger.warn("Skipping invalid notification subscription JSON", { userId: sub.user?.id }); + return Promise.resolve(); + } + const parsedSubscription = subscriptionSchema.safeParse(raw); if (!parsedSubscription.success) { - logger.error("Invalid subscription", parsedSubscription.error, JSON.stringify(sub.user)); + logger.error("Invalid subscription", { + userId: sub.user?.id, + error: parsedSubscription.error?.message, + }); return Promise.resolve(); } return sendNotification({ subscription: { endpoint: parsedSubscription.data.endpoint, keys: { auth: parsedSubscription.data.keys.auth, p256dh: parsedSubscription.data.keys.p256dh, }, }, title: title, body: "User is waiting for you to join. Click to Connect", url: connectAndJoinUrl, type: "INSTANT_MEETING", requireInteraction: false, }); });
- Consider also removing console.error usage above and rely on logger for consistency.
🧹 Nitpick comments (3)
packages/features/bookings/lib/dto/schema.ts (1)
1-10: Consider using a more specific type instead ofz.any()forexpirationTime.The
expirationTimefield usesz.any().optional(), which bypasses type validation. For better type safety and runtime validation, consider using a more specific type likez.number(),z.string(), orz.date()depending on the expected format.Apply this diff to improve type safety (assuming it's a timestamp):
export const instantMeetingSubscriptionSchema = z.object({ endpoint: z.string(), - expirationTime: z.any().optional(), + expirationTime: z.number().optional(), keys: z.object({ auth: z.string(), p256dh: z.string(), }), });If the format is different, adjust to
z.string()orz.date()as appropriate.packages/features/bookings/lib/service/InstantBookingCreateService.ts (2)
293-310: Prisma create should select only needed fieldsinstantMeetingToken.create returns the full record; you only use id and expires.
Apply this diff:
- const instantMeetingToken = await prisma.instantMeetingToken.create({ - data: { + const instantMeetingToken = await prisma.instantMeetingToken.create({ + data: { token, // current time + offset Seconds expires: new Date(new Date().getTime() + 1000 * instantMeetingExpiryTimeOffsetInSeconds), team: { connect: { id: eventType.team.id, }, }, booking: { connect: { id: newBooking.id, }, }, updatedAt: new Date().toISOString(), - }, + }, + select: { id: true, expires: true }, });As per coding guidelines
41-43: DI consistency: avoid global Prisma in getOrgIdFromMemberOrTeamIdgetOrgIdFromMemberOrTeamId uses a global prisma instance (see packages/lib/getOrgIdFromMemberOrTeamId.ts). Pass deps.prismaClient to it or add an overload to accept PrismaClient to ensure single connection source and easier testing.
Based on learnings
📜 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 (18)
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(1 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(3 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(1 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(4 hunks)packages/features/bookings/types.ts(2 hunks)packages/features/instant-meeting/schema.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/types/booking.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RegularBookingService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/instant-meeting/schema.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/types.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RegularBookingService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/instant-meeting/schema.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/types.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RegularBookingService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/instant-meeting/schema.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/types.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
🧠 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/service/RecurringBookingService.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/di/InstantBookingCreateService.module.tspackages/features/bookings/lib/service/RegularBookingService.ts
🧬 Code graph analysis (6)
packages/features/bookings/lib/dto/types.d.ts (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
InstantBookingCreateService(350-356)
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
packages/features/bookings/lib/dto/types.d.ts (1)
CreateInstantBookingData(18-18)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
packages/features/bookings/types.ts (2)
packages/features/bookings/lib/dto/types.d.ts (1)
InstantBookingCreateResult(53-53)packages/platform/libraries/bookings.ts (1)
InstantBookingCreateResult(8-8)
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(18-18)InstantBookingCreateResult(53-53)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)
🔇 Additional comments (24)
packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts (1)
5-5: Import path update looks good.The test now targets the renamed
BookingCreateServicemodule as expected after the handler→service refactor.packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts (1)
6-6: Confirm tests pass with the updated import path
Please run:npx vitest run packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tsto verify the import path change doesn’t break the tests.
apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts (1)
3-3: LGTM! Import path correctly updated.The import path change from a long external path to the local relative
"./types"is correct and aligns with the type relocation to the new types.d.ts file.apps/web/test/utils/bookingScenario/types.d.ts (1)
1-6: No staleCustomNextApiRequest/CustomNextApiResponseimports foundpackages/features/bookings/di/RecurringBookingService.module.ts (1)
1-2: LGTM! Import path updated for service reorganization.The import path change from
handleNewRecurringBookingtoservice/RecurringBookingServicealigns with the broader module reorganization. The DI wiring remains unchanged, preserving runtime behavior.packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx (1)
9-10: LGTM! Import paths updated for new file location.The relative import paths have been correctly adjusted to reflect the file's new location in the
client/subdirectory. The import depth changes are consistent with the directory structure reorganization.packages/features/bookings/lib/service/RecurringBookingService.ts (1)
6-7: LGTM! Import paths reorganized for cleaner module structure.The import paths have been updated to reflect the new service organization:
IBookingServicemoved to a parent-level interfaces directoryRegularBookingServicesimplified from a nested path to a direct service referenceThese changes maintain the same functionality while improving module organization.
packages/features/bookings/di/InstantBookingCreateService.module.ts (3)
3-4: LGTM! Service import path updated and Prisma client DI added.The import path has been correctly updated to the new service location, and the
prismaModuleLoaderimport enables proper dependency injection of the Prisma client.
20-23: LGTM! Module loader export enables proper DI wiring.The exported
moduleLoaderobject follows the established pattern for DI module wiring, providing both the token and load function for dependency resolution.
14-16: Constructor signature matches injected prismaClient
InstantBookingCreateService’s constructor accepts anIInstantBookingCreateServiceDependenciesobject which includesprismaClient, matching thedepsMapconfiguration.packages/features/bookings/types.ts (2)
10-10: LGTM! Import added for new type alias.The import for
InstantBookingCreateResultsupports the new public type alias being introduced in this file.
101-102: LGTM! New public type alias follows established pattern.The
InstantBookingResponsealias follows the same naming convention as the existingBookingResponse(line 99), providing a clear, consumer-friendly type for instant booking results.packages/features/bookings/di/RegularBookingService.module.ts (3)
1-2: LGTM! Import path updated for service reorganization.The import path change from
handleNewBookingtoservice/RegularBookingServicealigns with the broader module structure refactor.
13-16: LGTM! New repository and infrastructure dependencies added.The additional imports (
oooRepositoryModuleLoader,userRepositoryModuleLoader,DI_TOKENS,prismaModuleLoader) expand the service's capabilities with OOO, user, and Prisma client dependencies.
26-38: Constructor signature matches updated depsMap
TheRegularBookingServiceconstructor takes a singleIBookingServiceDependenciesobject, which now defines all 11 dependencies. No changes required.packages/features/bookings/lib/index.ts (1)
5-5: LGTM! Export path updated to reflect file reorganization.The export path has been correctly updated to reference the new location in the
client/subdirectory. The exported symbols remain unchanged, preserving the public API.packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (3)
88-101: Strong typing for booking data looks goodUsing CreateInstantBookingData and explicit instant: true improves safety and clarity.
103-105: Service call signature update LGTMPassing { bookingData } to createBooking aligns with the new API.
159-172: Negative-path test updated correctlyTyped bookingData and expected error string match the service behavior.
Also applies to: 175-177
packages/features/instant-meeting/schema.ts (1)
1-3: Re-exporting the schema is fineAlias import and re-export keep a single source of truth.
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
350-355: Constructor DI and method wiring look correctInstantBookingCreateService forwards deps to handler; interface naming is consistent.
packages/features/bookings/lib/service/RegularBookingService.ts (3)
51-56: Type‑only import of webhook payload types is goodReduces runtime bundle and clarifies intent.
99-129: Import path reorg and type-only imports LGTMShifts to module aliases and type-only imports without behavior change.
1491-1494: Minor formatting change preserved semanticsConditional remains correct; no issues.
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 (3)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
268-273: Useselectinstead ofincludefor Prisma queries.The
createBookingObjusesinclude: { attendees: true }which violates the coding guideline: "For Prisma queries, only select data you need; never useinclude, always useselect".As per coding guidelines
Apply this diff to use
selectinstead ofinclude:const createBookingObj = { - include: { - attendees: true, + select: { + id: true, + uid: true, + userId: true, + responses: true, + title: true, + startTime: true, + endTime: true, + description: true, + customInputs: true, + status: true, + metadata: true, + attendees: { + select: { + id: true, + email: true, + name: true, + timeZone: true, + locale: true, + phoneNumber: true, + }, + }, }, data: newBookingData, };Ensure you select only the fields actually needed for the return value at line 337-344 and any subsequent usage of
newBooking.packages/features/bookings/lib/service/RegularBookingService.ts (2)
1173-1175: Avoid logging raw guest emails (PII).Logs currently include guest emails. This is a privacy/compliance risk. Log counts or hashed values instead.
Apply:
- if (guestsRemoved.length > 0) { - log.info("Removed guests from the booking", guestsRemoved); - } + if (guestsRemoved.length > 0) { + log.info("Removed guests from the booking", { count: guestsRemoved.length }); + // Optionally at debug level: log.debug("Removed guests (hashed)", guestsRemoved.map(anonymize)); + }Optionally add a small anonymizer:
const anonymize = (e: string) => `${e.slice(0,2)}***@***${e.slice(e.lastIndexOf('.'))}`;As per coding guidelines (compliance/privacy).
1218-1223: Prisma read withoutselect(returning all columns).Select only needed fields to reduce data surface and match repository guidelines.
Apply:
- const organizerOrganizationProfile = await prisma.profile.findFirst({ - where: { - userId: organizerUser.id, - }, - }); + const organizerOrganizationProfile = await prisma.profile.findFirst({ + where: { userId: organizerUser.id }, + select: { organizationId: true, username: true }, + });As per coding guidelines (Prisma: always use
select).
🧹 Nitpick comments (4)
packages/features/bookings/lib/service/RegularBookingService.ts (4)
2005-2013: Reduce payload on Prismaupdatecalls.You don’t use the returned record; add a minimal
selectto avoid fetching full rows.Apply:
await prisma.booking.update({ where: { id: booking.id, }, data: { iCalUID: evt.iCalUID || booking.iCalUID, }, + select: { id: true }, });and
await prisma.booking.update({ where: { uid: booking.uid, }, data: { location: evt.location, metadata: { ...(typeof booking.metadata === "object" && booking.metadata), ...metadata }, references: { createMany: { data: referencesToCreate, }, }, }, + select: { id: true }, });As per coding guidelines (Prisma: select only what you need).
Also applies to: 2336-2350
301-306: Use enum values instead of string literals forSchedulingType.Mixing string literals and enum increases drift risk. Prefer the enum consistently.
Apply:
- if (schedulingType === "COLLECTIVE" && user.destinationCalendar) { + if (schedulingType === SchedulingType.COLLECTIVE && user.destinationCalendar) { - const isTeamEventType = - !!eventType.schedulingType && ["COLLECTIVE", "ROUND_ROBIN"].includes(eventType.schedulingType); + const isTeamEventType = + !!eventType.schedulingType && + [SchedulingType.COLLECTIVE, SchedulingType.ROUND_ROBIN].includes(eventType.schedulingType); - teamName: eventType.schedulingType === "COLLECTIVE" || users.length > 1 ? eventType.team?.name : null, + teamName: + eventType.schedulingType === SchedulingType.COLLECTIVE || users.length > 1 ? eventType.team?.name : null,Also applies to: 626-627, 1203-1204
807-832: Reduce repeated Day.js TZ conversions in loops.
.tz()is relatively heavy. Precompute helpers or formatted values outside loops to cut overhead.Example:
const toTzIso = (d: string) => dayjs(d).tz(reqBody.timeZone).format(); // then use toTzIso(start) / toTzIso(end) in ensureAvailableUsers loopsAs per coding guidelines (flag excessive Day.js use in hot paths).
Also applies to: 986-1006
99-129: Confirm handleSeats default exporthandleSeats is exported as default (line 164), so the import is valid. Optional: refactor to a named export to align with project guidelines.
📜 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 (18)
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(1 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(3 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(1 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(4 hunks)packages/features/bookings/types.ts(2 hunks)packages/features/instant-meeting/schema.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/types/booking.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/InstantBookingCreateService.module.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/InstantBookingCreateService.module.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/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/lib/index.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/types.tspackages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsxpackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/dto/types.d.tspackages/features/instant-meeting/schema.tspackages/features/bookings/lib/service/RecurringBookingService.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/di/RecurringBookingService.module.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/InstantBookingCreateService.module.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
**/*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/InstantBookingCreateService.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-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/test/buildEventForTeamEventType.test.ts
📚 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/RegularBookingService.module.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/RecurringBookingService.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/di/InstantBookingCreateService.module.ts
🧬 Code graph analysis (6)
packages/features/bookings/types.ts (2)
packages/features/bookings/lib/dto/types.d.ts (1)
InstantBookingCreateResult(53-53)packages/platform/libraries/bookings.ts (1)
InstantBookingCreateResult(8-8)
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
packages/features/bookings/lib/dto/types.d.ts (1)
CreateInstantBookingData(18-18)
packages/features/bookings/lib/dto/types.d.ts (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
InstantBookingCreateService(350-356)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
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(18-18)InstantBookingCreateResult(53-53)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)
🔇 Additional comments (18)
packages/features/instant-meeting/schema.ts (1)
1-3: LGTM! Clean schema consolidation.This change properly centralizes the subscription schema definition and maintains backward compatibility through aliasing.
packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts (1)
6-6: LGTM! Import path updated correctly.The function import has been correctly relocated to the new
BookingCreateServicemodule as part of the service restructuring.packages/features/bookings/types.ts (1)
10-10: LGTM! Consistent type alias pattern.The new
InstantBookingResponsetype alias follows the same pattern as the existingBookingResponse, providing a cleaner public API name while maintaining the connection to the underlying service result type.Also applies to: 101-101
apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts (1)
3-3: LGTM! Improved import structure.Moving the type imports to a local types module improves code organization and makes the module boundaries clearer.
packages/features/bookings/lib/service/RecurringBookingService.ts (1)
6-7: LGTM! Import paths correctly updated.The relative import paths are now cleaner and properly reflect the new service module structure.
packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx (1)
9-10: LGTM! Import paths correctly adjusted.The type import paths have been properly updated to reflect the new module structure without affecting runtime behavior.
packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts (1)
5-5: LGTM! Import path updated correctly.The function import has been correctly relocated to the new
BookingCreateServicemodule, maintaining test functionality while aligning with the service restructuring.packages/features/bookings/di/InstantBookingCreateService.module.ts (1)
3-4: LGTM! DI wiring for Prisma client is properly configured.The addition of
prismaModuleLoaderand its wiring throughdepsMapfollows the established DI pattern. The service import path relocation to the service-specific module aligns with the broader refactor.Also applies to: 14-16
packages/features/bookings/di/RecurringBookingService.module.ts (1)
1-2: LGTM! Import path update aligns with service consolidation.The relocation of
RecurringBookingServiceimport to the service-specific module path is consistent with the broader refactor pattern.packages/features/bookings/lib/index.ts (1)
5-5: LGTM! Export path updated to reflect directory restructure.The relocation of booking-to-mutation-input-mapper to the client subdirectory maintains the same exports while aligning with the new module organization.
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
16-17: LGTM! Test updates align with service refactor.The test correctly:
- Uses
CreateInstantBookingDatatype for type safety- Obtains the service from the DI container
- Passes
bookingDatainstead of a mock request body- Explicitly sets
instant: truefor clarityThese changes properly reflect the service's updated signature and DI wiring.
Also applies to: 88-101, 159-172
apps/web/test/utils/bookingScenario/types.d.ts (1)
1-6: LGTM! Test utility types are well-defined.The intersection types combining Next.js API types with Express types provide a unified interface for test mocking purposes. The type definitions are clear and appropriate for the test utilities.
packages/features/bookings/lib/dto/types.d.ts (1)
10-11: LGTM! Deriving types from service implementations improves type safety.The change to derive
InstantBookingCreateResultfrom the service's return type (line 53) ensures consistency between the service implementation and its type definition. The updated import paths correctly point to the service implementations.Also applies to: 53-53
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
25-25: LGTM! Dependency injection implementation is correct.The introduction of
IInstantBookingCreateServiceDependenciesand proper propagation ofprismaClientthrough the handler, helper functions, and service class follows best practices for dependency injection. All Prisma usage now comes from the injected client rather than a global instance.Also applies to: 31-33, 164-168, 351-354
packages/features/bookings/lib/service/RegularBookingService.ts (4)
51-51: Good: type-only import for webhook payload types.Keeps runtime lean and avoids unnecessary JS.
1491-1495: Condition restructuring LGTM.Readability improved; semantics preserved.
414-426: DI wiring includes all new dependencies –depsMapinRegularBookingService.module.tsbinds all 11 properties fromIBookingServiceDependencies; no further action required.
2101-2120: No external exposure of credential.key detected The selectedkeyis used solely for internal payment processing in RegularBookingService and isn’t returned via any tRPC handlers or serialized in logs.
There was a problem hiding this comment.
Actionable comments posted: 1
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/service/InstantBookingCreateService.ts (3)
78-95: Bug: promises not returned from map (webhooks may not be awaited)subscribers.map callback doesn’t return the promise. Promise.all will resolve immediately without awaiting webhook calls.
Apply this diff:
- const promises = subscribers.map((sub) => { - sendGenericWebhookPayload({ + const promises = subscribers.map((sub) => { + return 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 ); }); });
138-145: Guard JSON.parse and avoid logging PII/subscription secretsJSON.parse can throw and currently isn’t caught; also logger.error includes the raw subscription payload via stringified user object.
Apply this diff:
- const parsedSubscription = subscriptionSchema.safeParse(JSON.parse(subscription)); - - if (!parsedSubscription.success) { - logger.error("Invalid subscription", parsedSubscription.error, JSON.stringify(sub.user)); - return Promise.resolve(); - } + let parsedRaw: unknown; + try { + parsedRaw = JSON.parse(subscription); + } catch (e) { + logger.warn("Invalid subscription JSON", { + userId: sub.user?.id, + subscriptionId: sub.user?.NotificationsSubscriptions?.[0]?.id, + }); + return Promise.resolve(); + } + const parsedSubscription = subscriptionSchema.safeParse(parsedRaw); + if (!parsedSubscription.success) { + logger.error("Invalid subscription", { + error: parsedSubscription.error?.message, + userId: sub.user?.id, + subscriptionId: sub.user?.NotificationsSubscriptions?.[0]?.id, + }); + return Promise.resolve(); + }
- Rationale: Prevents crashes on bad JSON and avoids logging endpoints/keys. As per coding guidelines (privacy/compliance).
268-276: Prisma: replace include with select (never use include; select minimal fields)Use select to return only required fields; attendees aren’t used here.
As per coding guidelines
- const createBookingObj = { - include: { - attendees: true, - }, - data: newBookingData, - }; + const createBookingObj = { + select: { + id: true, + uid: true, + responses: true, + customInputs: true, + userId: true, + title: true, + }, + data: newBookingData, + };
♻️ Duplicate comments (3)
packages/features/bookings/di/RegularBookingService.module.ts (1)
33-37: Remove unused injected dependencies
hostRepository,oooRepository,userRepository, andattributeRepositoryare declared in the deps map but do not appear to be used byRegularBookingService. Remove these to avoid unnecessary dependency injection overhead.packages/features/bookings/lib/dto/types.d.ts (2)
24-25: Import missing base types
CreateSeatedBookingInputreferencesBaseCreateBookingDataandMasterCreateBookingData, but neither type is defined or imported in this file. Add the necessary imports to resolve the TypeScript compilation error.
53-53: Fix method name in type definitionThe type references
InstantBookingCreateService["create"], but the service exposescreateBooking(as shown in the relevant code snippet at packages/features/bookings/lib/service/InstantBookingCreateService.ts:349-355). Update the type to use the correct method name.Apply this diff:
-export type InstantBookingCreateResult = Awaited<ReturnType<InstantBookingCreateService["create"]>>; +export type InstantBookingCreateResult = Awaited<ReturnType<InstantBookingCreateService["createBooking"]>>;
📜 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 (18)
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(1 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(3 hunks)packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts(1 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(4 hunks)packages/features/bookings/types.ts(2 hunks)packages/features/instant-meeting/schema.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 (4)
- packages/features/bookings/lib/index.ts
- packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
- packages/features/bookings/lib/service/RecurringBookingService.ts
- packages/features/instant-meeting/schema.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/di/InstantBookingCreateService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/types.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RecurringBookingService.module.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/InstantBookingCreateService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/types.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RecurringBookingService.module.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/InstantBookingCreateService.module.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/bookings/types.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/dto/schema.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/createMockNextJsRequest.tspackages/features/bookings/lib/service/InstantBookingCreateService.test.tspackages/features/bookings/di/RecurringBookingService.module.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/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/di/InstantBookingCreateService.module.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/dto/types.d.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/bookings/lib/service/RegularBookingService.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/service/InstantBookingCreateService.test.ts
🧬 Code graph analysis (6)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
packages/features/bookings/types.ts (2)
packages/features/bookings/lib/dto/types.d.ts (1)
InstantBookingCreateResult(53-53)packages/platform/libraries/bookings.ts (1)
InstantBookingCreateResult(8-8)
packages/features/bookings/lib/dto/types.d.ts (2)
packages/platform/libraries/bookings.ts (2)
InstantBookingCreateResult(8-8)InstantBookingCreateService(6-6)packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
InstantBookingCreateService(350-356)
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(18-18)InstantBookingCreateResult(53-53)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (1)
packages/features/bookings/lib/dto/types.d.ts (1)
CreateInstantBookingData(18-18)
🔇 Additional comments (16)
packages/features/bookings/lib/dto/schema.ts (1)
1-10: Schema structure is sound; ensure the flaggedz.any()concern is resolved.The schema correctly models a Web Push subscription object using named exports as per guidelines. Previous reviews have already identified that line 5's
z.any()should be replaced with a more specific type (likelyz.number()for a DOMTimeStamp). Ensure that concern is addressed before merging.apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts (1)
3-3: LGTM! Import path change aligns with the refactoring.The updated import path correctly references the newly created local type definitions file. This improves code organization by centralizing test type definitions.
Note: Ensure that the type definitions in
./types.d.tsare compatible (see concerns raised for that file).packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts (1)
6-6: LGTM!The import path correctly points to
RegularBookingService, resolving the previous review concern about the broken import. The test logic remains unchanged, confirming this is a pure refactoring aligned with the PR's objective.packages/features/bookings/types.ts (1)
10-11: LGTM!The new
InstantBookingResponsetype alias provides a stable public API name for external consumers, following the same pattern asBookingResponseon line 99. The import source is correct and aligns with the PR's refactoring objectives.packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts (1)
5-5: LGTM!The import path correctly references
RegularBookingServicefrom the new service location, addressing the previous review concern. The test logic remains unchanged, confirming this is a pure refactoring.packages/features/bookings/di/InstantBookingCreateService.module.ts (2)
3-4: LGTM!The import path update and addition of
prismaModuleLoadercorrectly wire the DI forInstantBookingCreateService, which now receivesprismaClientvia its constructor as shown in the service implementation.
14-16: LGTM!The
depsMapcorrectly providesprismaClienttoInstantBookingCreateService, aligning with the new dependency injection pattern introduced in this PR.packages/features/bookings/di/RecurringBookingService.module.ts (1)
1-2: LGTM!The import path update correctly points to the new service location, consistent with the PR's refactoring objectives. The empty line improves readability.
packages/features/bookings/lib/service/InstantBookingCreateService.test.ts (4)
1-1: LGTM!The import path correctly reflects the deeper directory structure after the test file reorganization.
16-17: LGTM!The new imports correctly reference the updated module locations and introduce the
CreateInstantBookingDatatype for better type safety.
88-104: LGTM!The test correctly refactored from using a mock request object to passing
bookingDatadirectly, aligning with the new service interface. The addition ofinstant: trueis appropriate for instant booking tests.
159-176: LGTM!Consistent with the first test case, this correctly uses the new service interface with direct
bookingDatapassing instead of mock request objects.packages/features/bookings/lib/dto/types.d.ts (1)
10-11: LGTM!The import path updates correctly reference the new service module locations, aligning with the PR's refactoring objectives.
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
351-355: DI wiring looks goodConstructor-based deps threading and passing prisma via handler is clean and consistent with the refactor goals.
packages/features/bookings/lib/service/RegularBookingService.ts (2)
99-129: Imports re-org LGTMPaths updated to new service structure; named imports preserved. No behavior change.
1491-1494: Conditional consolidation is fineSchedulingType check reads clearer; semantics unchanged.
| featuresRepository: featuresRepositoryModuleLoader, | ||
| checkBookingLimitsService: checkBookingLimitsModuleLoader, | ||
| luckyUserService: luckyUserServiceModuleLoader, | ||
| hostRepository: hostRepositoryModuleLoader, | ||
| oooRepository: oooRepositoryModuleLoader, | ||
| userRepository: userRepositoryModuleLoader, | ||
| attributeRepository: attributeRepositoryModuleLoader, |
There was a problem hiding this comment.
These are not used yet. will add them when they start getting used.
Some of these were needed because downstream code was getting them from container without actually loading the module. I have fixed that by moving the faulty modules(Cache and CheckBookingAndDurationLimits) to use bindModuleToClassOnToken which automatically fixes the issue
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (2)
packages/features/bookings/di/RegularBookingService.module.ts(1 hunks)packages/features/bookings/lib/service/RegularBookingService.ts(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/bookings/lib/service/RegularBookingService.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/RegularBookingService.module.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/RegularBookingService.module.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/RegularBookingService.module.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/di/RegularBookingService.module.ts
⏰ 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 (1)
packages/features/bookings/di/RegularBookingService.module.ts (1)
31-32: BothluckyUserServiceanduserRepositoryare actively used in RegularBookingService and should be retained.Verification confirms:
luckyUserServiceis used at line 1006 to retrieve lucky user datauserRepositoryis used at line 1203 to fetch users with email verification settingsThe previous concern about
userRepositorybeing unused is not supported by the current implementation. No over-injection issue exists.
| const loadModule = bindModuleToClassOnToken({ | ||
| module: checkBookingAndDurationLimitsModule, | ||
| moduleToken, | ||
| token, | ||
| loadModule: (container: Container) => { | ||
| container.load(moduleToken, checkBookingAndDurationLimitsModule); | ||
| classs: CheckBookingAndDurationLimitsService, | ||
| depsMap: { | ||
| checkBookingLimitsService: checkBookingLimitsModuleLoader, |
There was a problem hiding this comment.
Moving to bindModuleToClassOnToken ensures that dependencies (checkBookingLimitsService) are automatically loaded.
| const loadModule = bindModuleToClassOnToken({ | ||
| module: cacheModule, | ||
| moduleToken, | ||
| token, | ||
| loadModule: (container: Container) => { | ||
| container.load(moduleToken, cacheModule); | ||
| classs: CacheService, | ||
| depsMap: { | ||
| featuresRepository: featuresRepositoryModuleLoader, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
It ensures that dependencies are automatically loaded.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/test/utils/bookingScenario/types.d.ts (1)
4-6: Type intersection incompatibility persists.As flagged in the previous review, intersecting
NextApiRequest & RequestandNextApiResponse & Responsecreates type conflicts because Next.js and Express define incompatible request/response models. Since these types were moved (not newly introduced), consider addressing this technical debt by replacing intersections with adapter functions that map only the required fields at test boundaries.
🧹 Nitpick comments (1)
packages/features/bookings/lib/service/InstantBookingCreateService.ts (1)
35-40: Verify consistent Prisma parameter naming.Both
handleInstantMeetingWebhookTriggerandtriggerBrowserNotificationsacceptprismaClientas a parameter but destructure it asprisma. Consider using consistent naming throughout (eitherprismaClientorprisma) to reduce cognitive load.Example refactor to use
prismaconsistently:- prismaClient: PrismaClient; + prisma: PrismaClient;And update the parameter names in both functions:
- prismaClient: PrismaClient; + prisma: PrismaClient;Also applies to: 101-107
📜 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/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(1 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/bookings/types.ts(2 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 (12)
- apps/web/test/utils/bookingScenario/createMockNextJsRequest.ts
- packages/features/instant-meeting/schema.ts
- packages/features/bookings/lib/dto/schema.ts
- packages/features/bookings/di/InstantBookingCreateService.module.ts
- packages/features/bookings/lib/client/booking-event-form/booking-to-mutation-input-mapper.tsx
- packages/platform/libraries/bookings.ts
- packages/features/bookings/lib/index.ts
- packages/features/bookings/lib/service/RecurringBookingService.ts
- packages/features/bookings/types.ts
- packages/features/bookings/di/RecurringBookingService.module.ts
- packages/features/bookings/lib/dto/types.d.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/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/handleNewBooking/test/buildDryRunBooking.test.tsapps/web/test/utils/bookingScenario/types.d.tspackages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.tspackages/features/di/modules/Cache.tspackages/features/bookings/lib/service/InstantBookingCreateService.tspackages/features/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/RegularBookingService.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/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-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/test/buildEventForTeamEventType.test.ts
📚 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/di/modules/CheckBookingAndDurationLimits.tspackages/features/bookings/di/RegularBookingService.module.tspackages/features/bookings/lib/service/RegularBookingService.ts
🧬 Code graph analysis (6)
apps/web/test/utils/bookingScenario/types.d.ts (1)
packages/platform/types/api.ts (1)
Response(31-31)
packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts (1)
packages/features/bookings/lib/service/RegularBookingService.ts (1)
buildEventForTeamEventType(264-351)
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/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(19-19)InstantBookingCreateResult(52-52)
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/service/RegularBookingService.ts (2)
packages/platform/libraries/index.ts (1)
SchedulingType(29-29)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). (3)
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
🔇 Additional comments (11)
packages/features/bookings/lib/service/RegularBookingService.ts (2)
412-419: LGTM! Dependency interface updated appropriately.The
IBookingServiceDependenciesinterface has been refined to includeluckyUserServiceanduserRepositorywhile removingfeaturesRepositoryandcheckBookingLimitsService. This aligns with the refactoring objectives and the TODO comment (lines 482-485) correctly documents the future goal of further abstracting Prisma access through repositories.Based on learnings
2607-2608: LGTM! Constructor follows DI best practices.The empty constructor body using TypeScript's parameter properties pattern is appropriate for dependency injection. The
private readonlymodifier automatically assigns and constrains thedepsparameter.packages/features/bookings/lib/handleNewBooking/test/buildDryRunBooking.test.ts (1)
5-5: LGTM! Import path correctly updated to service location.The import path has been updated to reference
RegularBookingServicefrom the new service directory, consistent with the PR's refactoring objectives and addressing previous review feedback.packages/features/bookings/lib/handleNewBooking/test/buildEventForTeamEventType.test.ts (2)
6-6: LGTM! Import path updated to service location.Consistent with the refactoring, the import now references
RegularBookingServicefrom the new service directory.
83-83: Nice improvement: replacedanywith explicit types.The inline type annotations
(m: { email: string })replace the unsafeanytype, improving type safety without requiring a separate interface definition. This pattern is appropriate for these simple map operations.Also applies to: 126-126, 145-145
packages/features/bookings/lib/service/InstantBookingCreateService.ts (3)
31-33: LGTM! Dependency injection interface introduced.The
IInstantBookingCreateServiceDependenciesinterface properly defines the Prisma dependency, enabling testability and aligning with the DI pattern used inRegularBookingService. This addresses the PR objective of removing direct Prisma imports from services.
164-168: LGTM! Handler signature updated for dependency injection.The handler function now accepts
depsas a parameter and destructures the Prisma client, enabling proper dependency injection. This change maintains the same runtime behavior while improving testability and architectural cleanliness.
350-355: LGTM! Service class properly wired for DI.The
InstantBookingCreateServiceconstructor correctly accepts and stores the dependencies, then passes them to the handler. This completes the dependency injection refactoring for this service.packages/features/bookings/di/RegularBookingService.module.ts (3)
1-11: LGTM! Import path updated correctly.The import path change from the old handler location to
@calcom/features/bookings/lib/service/RegularBookingServicealigns with the PR objective. All module loader imports match thedepsMapentries below.
31-36: LGTM! Export pattern consistent with DI conventions.The
moduleLoaderexport and type re-export follow the standard pattern used across other DI modules in this refactor.
21-28: Dependencies are in use
BothluckyUserServiceanduserRepositoryare referenced inRegularBookingService(e.g., lines 1010, 1207), so their inclusion indepsMapis justified.
|
|
||
| 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.
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; |
E2E results are ready! |
| 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
There was a problem hiding this comment.
LGTM and builds properly. Awesome refactor @hariombalhara
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 |
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