test: [Booking Refactor - Preparation - 1] Make all test files import handleNewBooking from a single place#23741
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughIntroduces a new test helper module packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts that exports getNewBookingHandler(), which returns a function that dynamically imports the handleNewBooking default export at call time. Updates numerous test files under packages/features/bookings/lib/handleNewBooking/test/ (including booking-limits, complex-schedules, date-overrides, delegation-credential, dynamic-group-booking, fresh-booking, reschedule, round-robin-no-hosts, workflow-notifications, and team-bookings subfolder tests) and packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts to replace direct dynamic imports with const handleNewBooking = getNewBookingHandler(). Test logic and assertions remain unchanged; changes are constrained to test setup. Possibly related PRs
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
keithwillcode
added
core
This stack of pull requests is managed by Graphite. Learn more about stacking. |
hariombalhara
changed the title
hariombalhara
force-pushed
the
handleNewBooking-tests-import-from-getNewBookingHandler
branch
2 times, most recently
from
4cfef3f to
e7905d5
Compare
hariombalhara
changed the base branch from
move-modules-moduleLoader-pattern-for-RegularBookingService
to
graphite-base/23741
hariombalhara
force-pushed
the
handleNewBooking-tests-import-from-getNewBookingHandler
branch
from
e7905d5 to
e04fc84
Compare
hariombalhara
force-pushed
the
graphite-base/23741
branch
from
3122f8c to
b9074ae
Compare
dosubot
bot
added
automated-tests
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
1-8: Type the helper and cache the imported handler (optional).Gives safer call sites and avoids repeated import() plumbing while preserving lazy load.
+type HandleNewBooking = typeof import("@calcom/features/bookings/lib/handleNewBooking").default; + +let cached: HandleNewBooking | null = null; -async function handler(input: any) { - const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default; - return handleNewBooking(input); -} +async function handler(input: Parameters<HandleNewBooking>[0]) { + if (!cached) { + cached = (await import("@calcom/features/bookings/lib/handleNewBooking")).default; + } + return cached(input); +} -export function getNewBookingHandler() { - return handler; -} +export function getNewBookingHandler(): HandleNewBooking { + return handler as HandleNewBooking; +}packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts (1)
37-37: Align mock path with actual import in handleSeats.test.ts
Changevi.mock("./handleSeats")tovi.mock("../handleSeats")so the mocked module matches theimport * as handleSeatsModule from "../handleSeats"at line 29.packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts (1)
46-46: Ensure helper returns a fresh, correctly-typed handler.Calling getNewBookingHandler() per test is good. Please verify the helper lazily imports the module (not a cached top-level reference) and preserves the production signature (some tests pass extra args like userId). If it currently returns a pre-imported handler, consider this tweak in getNewBookingHandler.ts:
-export function getNewBookingHandler() { - return handler; -} +type HandleNewBooking = typeof import("@calcom/features/bookings/lib/handleNewBooking").default; +export function getNewBookingHandler(): HandleNewBooking { + // Lazy import to avoid stale state leakage and keep types intact + return (async (...args: Parameters<HandleNewBooking>) => { + const mod = await import("@calcom/features/bookings/lib/handleNewBooking"); + return (mod.default as HandleNewBooking)(...args); + }) as HandleNewBooking; +}Also applies to: 142-142, 222-222
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (1)
48-48: Wide adoption of getNewBookingHandler — approved; watch for handler arg shape.Many tests here pass extra properties (e.g., userId). Please ensure getNewBookingHandler’s return type matches the production default export so such calls remain type-safe.
Also applies to: 70-70, 309-309, 515-515, 703-703, 909-909, 1152-1152, 1393-1393, 1603-1603, 1861-1861, 2013-2013, 2173-2173, 2321-2321, 2521-2521, 2678-2678, 2837-2837, 2966-2966
packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (1)
25-25: Round-robin tests updated to shared helper — approved.Import path "../getNewBookingHandler" is correct from this subfolder; repeated per-test retrieval is fine for isolation.
Also applies to: 33-33, 214-214, 375-375, 506-506, 654-654, 785-785
packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts (2)
56-58: Initialize handler in beforeEach to remove duplication belowCreate the handler once per test via beforeEach; it reduces repetition and keeps handler lifecycle explicit.
Apply:
- beforeEach(() => { - resetTestSMS(); - }); + let handleNewBooking: ReturnType<typeof getNewBookingHandler>; + beforeEach(() => { + resetTestSMS(); + handleNewBooking = getNewBookingHandler(); + });
68-68: Remove per-test handler declarations after moving to beforeEachOnce the handler is initialized in beforeEach, drop these local declarations.
Example for the first occurrence:
- const handleNewBooking = getNewBookingHandler();Repeat for the other listed lines.
Also applies to: 250-250, 376-376, 549-549, 782-782, 1019-1019, 1219-1219, 1334-1334, 1514-1514, 1715-1715, 1908-1908, 2102-2102
packages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.ts (3)
156-163: Avoid magic “-1” userIdConsider a named constant to document intent (anonymous user) and prevent drift across tests.
+ const ANONYMOUS_USER_ID = -1; const recurringBookingService = getRecurringBookingService(); const createdBookings = await recurringBookingService.createBooking({ bookingData: bookingDataArray, bookingMeta: { - userId: -1, // Simulating anonymous user like in the API test + userId: ANONYMOUS_USER_ID, // Simulating anonymous user like in the API test }, });
376-385: Fix rejects assertion style for async calls (when unskipping)Use expect(promise).rejects…, not expect(async () => …).rejects…, which treats the function as a value and can yield false positives.
- await expect( - async () => - await recurringBookingService.createBooking({ - bookingData: bookingDataArray, - bookingMeta: { userId: -1 }, - }) - ).rejects.toThrow(ErrorCode.NoAvailableUsersFound); + await expect( + recurringBookingService.createBooking({ + bookingData: bookingDataArray, + bookingMeta: { userId: -1 }, + }) + ).rejects.toThrow(ErrorCode.NoAvailableUsersFound);Also applies to: 551-559
40-40: Rename describe to match the new API surface (nit)Since tests exercise RecurringBookingService, consider renaming for clarity.
- describe("handleNewRecurringBooking", () => { + describe("RecurringBookingService.createBooking", () => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
packages/features/bookings/lib/handleNewBooking/global-booking-limits.test.ts(5 hunks)packages/features/bookings/lib/handleNewBooking/test/booking-limits.test.ts(10 hunks)packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts(3 hunks)packages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.ts(7 hunks)packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts(4 hunks)packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts(36 hunks)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.ts(4 hunks)packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts(17 hunks)packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts(3 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts(13 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts(6 hunks)packages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.ts(7 hunks)packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts(16 hunks)
🧰 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/lib/handleNewBooking/test/round-robin-no-hosts.test.tspackages/features/bookings/lib/handleSeats/test/handleSeats.test.tspackages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.tspackages/features/bookings/lib/handleNewBooking/test/date-overrides.test.tspackages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.tspackages/features/bookings/lib/handleNewBooking/test/booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.tspackages/features/bookings/lib/handleNewBooking/test/reschedule.test.tspackages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.tspackages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.tspackages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.tspackages/features/bookings/lib/handleNewBooking/global-booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.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/handleNewBooking/test/round-robin-no-hosts.test.tspackages/features/bookings/lib/handleSeats/test/handleSeats.test.tspackages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.tspackages/features/bookings/lib/handleNewBooking/test/date-overrides.test.tspackages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.tspackages/features/bookings/lib/handleNewBooking/test/booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.tspackages/features/bookings/lib/handleNewBooking/test/reschedule.test.tspackages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.tspackages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.tspackages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.tspackages/features/bookings/lib/handleNewBooking/global-booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.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/handleNewBooking/test/round-robin-no-hosts.test.tspackages/features/bookings/lib/handleSeats/test/handleSeats.test.tspackages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.tspackages/features/bookings/lib/handleNewBooking/test/date-overrides.test.tspackages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.tspackages/features/bookings/lib/handleNewBooking/test/booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.tspackages/features/bookings/lib/handleNewBooking/test/reschedule.test.tspackages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.tspackages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.tspackages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.tspackages/features/bookings/lib/handleNewBooking/global-booking-limits.test.tspackages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.tspackages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.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-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.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/handleSeats/test/handleSeats.test.tspackages/features/bookings/lib/handleNewBooking/test/date-overrides.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.tspackages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.tspackages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.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/handleNewBooking/test/handleNewRecurringBooking.test.ts
🧬 Code graph analysis (16)
packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)
packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/booking-limits.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/global-booking-limits.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
packages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.ts (1)
packages/lib/di/bookings/containers/RecurringBookingService.container.ts (1)
getRecurringBookingService(27-29)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts (1)
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: team-labels
🔇 Additional comments (17)
packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.ts (2)
27-28: Consolidated handler import looks good.Switching to the shared helper improves consistency across tests.
51-51: LGTM: use of getNewBookingHandler() per-test.Keeps lazy load semantics without duplicating dynamic import code.
packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts (1)
31-32: Refactor to shared handler helper is correct.Import path and per-test retrieval are consistent with the PR objective.
Also applies to: 53-53, 215-215
packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts (1)
31-32: LGTM: centralized handler retrieval.Keeps tests uniform and avoids repeated dynamic import boilerplate.
Also applies to: 53-53
packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts (1)
21-22: Consistent switch to getNewBookingHandler().Paths and usage look correct.
Also applies to: 31-31, 145-145
packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts (2)
28-28: Importing the shared handler helper from handleNewBooking tests is correct.The relative path is right from handleSeats/test to handleNewBooking/test.
37-37: Per-test handler acquisition: OK.All call sites correctly switch to
const handleNewBooking = getNewBookingHandler();.Also applies to: 100-100, 245-245, 387-387, 515-515, 640-640, 749-749, 871-871, 1050-1050, 1253-1253, 1413-1413, 151-151, 171-171, 1846-1846, 2005-2005, 2121-2121, 2388-2388
packages/features/bookings/lib/handleNewBooking/test/booking-limits.test.ts (1)
33-33: LGTM: migrated all usages to the shared helper.Consistent with intent; lazy loading preserved.
Also applies to: 55-55, 173-173, 302-302, 383-383, 472-472, 566-566, 632-632, 701-701, 776-776
packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts (2)
66-66: Single-source import adopted.Matches the consolidation goal.
86-86: All call sites now use getNewBookingHandler(): good.Uniform, readable, and maintains previous behavior.
Also applies to: 268-268, 427-427, 583-583, 704-704, 869-869, 1027-1027, 1119-1119, 1203-1203, 1297-1297, 1384-1384, 1478-1478, 1538-1538, 1624-1624, 1705-1705, 1776-1776, 1861-1861, 1945-1945, 2025-2025, 2123-2123, 2261-2261, 2386-2386, 2522-2522, 2653-2653, 2721-2721, 2846-2846, 3030-3030, 3207-3207, 3355-3355, 3473-3473, 3530-3530, 3593-3593, 3647-3647, 3710-3710, 3771-3771
packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts (1)
32-32: Centralized handler import — LGTM.Switching to a single source of truth via getNewBookingHandler improves consistency across tests.
packages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.ts (1)
29-29: Refactor to shared handler helper — looks good.Imports and per-test retrieval via getNewBookingHandler are consistent and readable. Confirm the helper’s type preserves optional args (e.g., userId) passed by some tests.
Also applies to: 54-54, 165-165, 258-258, 406-406, 545-545, 658-658
packages/features/bookings/lib/handleNewBooking/global-booking-limits.test.ts (1)
20-20: Import path and usage are correct.Using "./test/getNewBookingHandler" from the parent directory is appropriate; handler retrieval per test aligns with the PR goal.
Also applies to: 73-73, 211-211, 299-299, 431-431
packages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.ts (1)
48-48: Consolidated handler acquisition — approved.Pattern is consistent across tests and improves maintainability. No functional changes in assertions.
Also applies to: 166-166, 345-345, 521-521, 714-714, 883-883, 1036-1036
packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts (2)
46-47: Centralized handler import — LGTMImporting from a single helper aligns with the PR objective and improves test consistency.
46-46: Stateless handler confirmed—no action required
getNewBookingHandler()always returns the samehandlerfunction, which merely delegates to the importedhandleNewBookingimplementation. All stateful dependencies are reset in each test viasetupAndTeardown(), so there’s no residual state between tests.packages/features/bookings/lib/handleNewBooking/test/handleNewRecurringBooking.test.ts (1)
26-27: Switch to DI container service — LGTMUsing getRecurringBookingService() improves realism and aligns tests with production wiring.
packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts
Show resolved
Hide resolved
E2E results are ready! |
hariombalhara
force-pushed
the
handleNewBooking-tests-import-from-getNewBookingHandler
branch
from
89db3d7 to
b9e818c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts (3)
46-46: Defer handler acquisition until right before use to avoid premature module import vs. mocks.If
getNewBookingHandler()captures the real module at creation time, calling it this early could import beforemockCalendar/scenario setup. Safer to move it immediately before the first invocation.Apply within this test:
- const handleNewBooking = getNewBookingHandler(); // ... - const createdBooking = await handleNewBooking({ + const handleNewBooking = getNewBookingHandler(); // moved near invocation + const createdBooking = await handleNewBooking({ bookingData: mockBookingData, });
142-142: Same here: acquire handler just before the expect block.Keeps import timing aligned with mocks/fixtures established above.
- const handleNewBooking = getNewBookingHandler(); // ... - await expect( + const handleNewBooking = getNewBookingHandler(); // moved closer to use + await expect( async () => await handleNewBooking({ bookingData: mockBookingData, }) ).rejects.toThrowError(ErrorCode.FixedHostsUnavailableForBooking);
222-222: Repeat: move handler acquisition closer to invocation.Prevents any accidental early module initialization relative to test setup.
- const handleNewBooking = getNewBookingHandler(); // ... - await expect( + const handleNewBooking = getNewBookingHandler(); // moved closer to use + await expect( async () => await handleNewBooking({ bookingData: mockBookingData, }) ).rejects.toThrowError(ErrorCode.FixedHostsUnavailableForBooking);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
packages/features/bookings/lib/handleNewBooking/global-booking-limits.test.ts(5 hunks)packages/features/bookings/lib/handleNewBooking/test/booking-limits.test.ts(10 hunks)packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts(3 hunks)packages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.ts(7 hunks)packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts(4 hunks)packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts(36 hunks)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts(1 hunks)packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts(17 hunks)packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts(3 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts(13 hunks)packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts(6 hunks)packages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.ts(7 hunks)packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts(16 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts
- packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts
- packages/features/bookings/lib/handleNewBooking/global-booking-limits.test.ts
- packages/features/bookings/lib/handleNewBooking/test/workflow-notifications.test.ts
- packages/features/bookings/lib/handleNewBooking/test/booking-limits.test.ts
- packages/features/bookings/lib/handleNewBooking/test/date-overrides.test.ts
- packages/features/bookings/lib/handleNewBooking/test/delegation-credential.test.ts
- packages/features/bookings/lib/handleNewBooking/test/reschedule.test.ts
- packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-scheduling.test.ts
- packages/features/bookings/lib/handleNewBooking/test/fresh-booking.test.ts
- packages/features/bookings/lib/handleNewBooking/test/round-robin-no-hosts.test.ts
- packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts
- packages/features/bookings/lib/handleNewBooking/test/complex-schedules.test.ts
- packages/features/bookings/lib/handleNewBooking/test/team-bookings/collective-reschedule-destination-calendar.test.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/lib/handleNewBooking/test/dynamic-group-booking.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/handleNewBooking/test/dynamic-group-booking.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/handleNewBooking/test/dynamic-group-booking.test.ts
🧬 Code graph analysis (1)
packages/features/bookings/lib/handleNewBooking/test/dynamic-group-booking.test.ts (2)
packages/platform/libraries/index.ts (1)
handleNewBooking(45-45)packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts (1)
getNewBookingHandler(6-8)
⏰ 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/lib/handleNewBooking/test/dynamic-group-booking.test.ts (1)
32-32: Approve — centralized test import verified. Repo-wide search found no tests that directly import or dynamic-import the handleNewBooking default. All handler usage goes through getNewBookingHandler (packages/features/bookings/lib/handleNewBooking/test/getNewBookingHandler.ts dynamically imports the module). A couple tests still import named helpers from ../../handleNewBooking (buildEventForTeamEventType, buildDryRunBooking) — expected for unit testing.
| const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default; | ||
| return handleNewBooking(input); |
There was a problem hiding this comment.
In followup PR. I replace this logic to get the RegularBookingService from the container
hariombalhara
deleted the
handleNewBooking-tests-import-from-getNewBookingHandler
branch
4 participants
What does this PR do?
Replace scattered handleNewBooking imports in test files to use getNewBookingHandler import. It allows us to easily replace the logic to start using RegularBookingService in the followup PR here - #23156
These are changes in just the test files which are verified automatically by the unit tests and thus is safe to merge.