feat: add dry run events for SDK (dryRunBookingSuccessfulV2 and dryRunRescheduleBookingSuccessfulV2)#23072
Conversation
…nRescheduleBookingSuccessfulV2) - Add new event types dryRunBookingSuccessfulV2 and dryRunRescheduleBookingSuccessfulV2 to EventDataMap - Create payload functions that exclude uid field for dry run events - Fire dry run events when isDryRun is true in both regular and recurring booking flows - Add unit tests to verify dry run event payloads are correct and exclude uid field - Ensures dry run mode no longer skips event firing but uses appropriate dry run variants Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughIntroduces a BaseBookingEventPayload type and refactors bookingSuccessfulV2 and rescheduleBookingSuccessfulV2 to extend it (adding uid where applicable). Adds two new public event payloads: dryRunBookingSuccessfulV2 and dryRunRescheduleBookingSuccessfulV2 (both BaseBookingEventPayload). In useBookings, adds internal builders getBaseBookingEventPayload and getBookingSuccessfulEventPayload plus exported aliases getDryRunBookingSuccessfulEventPayload and getDryRunRescheduleBookingSuccessfulEventPayload. Dry-run branches now build and emit dry-run events (including allBookings for recurring) and navigate to /booking/dry-run-successful; non-dry-run behavior is unchanged. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
keithwillcode
added
core
…ogic - Create BaseBookingEventPayload type to eliminate duplication between regular and dry run events - Use intersection types to compose final event types in sdk-action-manager.ts - Create getBaseBookingEventPayload function for common payload generation logic - Export dry run payload functions and import them in tests instead of redefining - Fix missing videoCallUrl field in rescheduleBookingSuccessfulV2 type Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
E2E results are ready! |
dosubot
bot
added
the
bookings
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/14/25)1 reviewer was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/embeds/embed-core/src/sdk-action-manager.ts (1)
12-25: Clarify payload contract (time format and field semantics).Great consolidation. To make this contract self-documenting for SDK consumers, please document the expected time format and semantics for key fields.
Suggested inline JSDoc:
type BaseBookingEventPayload = { - title: string | undefined; - startTime: string | undefined; - endTime: string | undefined; + /** Title as shown to the attendee (eventType name, considering overrides) */ + title: string | undefined; + /** ISO 8601 string. Prefer UTC (toISOString). */ + startTime: string | undefined; + /** ISO 8601 string. Prefer UTC (toISOString). */ + endTime: string | undefined; eventTypeId: number | null | undefined; - status: string | undefined; + /** Booking status (e.g. "PENDING" | "ACCEPTED" | "CANCELLED"). String union upstream. */ + status: string | undefined; paymentRequired: boolean; isRecurring: boolean; /** * This is only used for recurring bookings */ allBookings?: { startTime: string; endTime: string }[]; - videoCallUrl?: string; + /** Absolute URL for the meeting link (Zoom/Google Meet/etc), if applicable */ + videoCallUrl?: string; };packages/features/bookings/Booker/components/hooks/useBookings.ts (3)
59-79: Confirm startTime/endTime are emitted as ISO strings.The builder assumes string inputs. Please confirm upstream booking.startTime/endTime are ISO 8601 strings in all success paths (including dry-run and recurring). If any path returns Date, convert to toISOString before emitting to satisfy the embed-core contract.
If needed, here’s a minimal, non-Day.js conversion pattern:
const getBaseBookingEventPayload = (booking: { - startTime: string; - endTime: string; + startTime: string; + endTime: string; ... }) => { return { - startTime: booking.startTime, - endTime: booking.endTime, + startTime: booking.startTime, + endTime: booking.endTime, ... }; };If you discover Date types in some paths, we can widen the parameter type and normalize to ISO once at the boundary.
217-222: Remove unused validDuration computation in dry-run path.validDuration is computed but never used in the dry-run branch, which can confuse future readers.
Apply this diff to remove it:
- const validDuration = event.data?.isDynamic - ? duration || event.data?.length - : duration && event.data?.metadata?.multipleDuration?.includes(duration) - ? duration - : event.data?.length;
387-411: DRY the allBookings mapping in dry-run recurring branch.Minor duplication: allBookings is computed twice and shadows booking in the map callback. Precompute once and reuse.
Apply this diff:
- if (isRescheduling) { - sdkActionManager?.fire("dryRunRescheduleBookingSuccessfulV2", { - ...getDryRunRescheduleBookingSuccessfulEventPayload({ - ...booking, - isRecurring: true, - }), - allBookings: bookings.map((booking) => ({ - startTime: booking.startTime, - endTime: booking.endTime, - })), - }); - } else { - sdkActionManager?.fire("dryRunBookingSuccessfulV2", { - ...getDryRunBookingSuccessfulEventPayload({ - ...booking, - isRecurring: true, - }), - allBookings: bookings.map((booking) => ({ - startTime: booking.startTime, - endTime: booking.endTime, - })), - }); - } + const allBookings = bookings.map((b) => ({ + startTime: b.startTime, + endTime: b.endTime, + })); + if (isRescheduling) { + sdkActionManager?.fire("dryRunRescheduleBookingSuccessfulV2", { + ...getDryRunRescheduleBookingSuccessfulEventPayload({ + ...booking, + isRecurring: true, + }), + allBookings, + }); + } else { + sdkActionManager?.fire("dryRunBookingSuccessfulV2", { + ...getDryRunBookingSuccessfulEventPayload({ + ...booking, + isRecurring: true, + }), + allBookings, + }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/embeds/embed-core/src/sdk-action-manager.ts(3 hunks)packages/features/bookings/Booker/components/hooks/useBookings.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/embeds/embed-core/src/sdk-action-manager.tspackages/features/bookings/Booker/components/hooks/useBookings.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/embeds/embed-core/src/sdk-action-manager.tspackages/features/bookings/Booker/components/hooks/useBookings.ts
🔇 Additional comments (6)
packages/embeds/embed-core/src/sdk-action-manager.ts (2)
42-45: LGTM: v2 payload now reuses the base type + uid.This removes duplication cleanly and keeps the surface area tight. No concerns here.
62-66: LGTM: reschedule v2 aligned with base + uid and new dry-run variants added.Dry-run event names and payloads are consistent with the non-dry-run counterparts. Inclusion of videoCallUrl via the base type addresses the prior omission.
packages/features/bookings/Booker/components/hooks/useBookings.ts (4)
81-97: LGTM: simple composition keeps base and uid concerns nicely separated.This keeps the uid out of the base and makes reusability straightforward.
98-98: LGTM: alias maintains parity with booking flow.Keeping reschedule equal to booking for the payload builder improves maintainability.
100-102: LGTM: public dry-run payload wrappers are minimal and consistent.Clear surface for consumers; avoids redefining payload shapes in tests and other callers.
223-239: LGTM: dry-run single booking events and isRecurring=false are correctly emitted.Event names match embed-core, and payloads route through the new builders. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
60-80: Normalize startTime/endTime to strings (safely handle Date inputs) in base payload builderDownstream SDK event contracts typically expect string timestamps. Here,
booking.startTime/endTimecan arrive asDate(common in existing code paths), and are passed through as-is. Normalize to ISO strings (or preserve strings) to avoid mixed types being fired across events.Apply this diff:
-const getBaseBookingEventPayload = (booking: { - title?: string; - startTime: string; - endTime: string; - eventTypeId?: number | null; - status?: BookingStatus; - paymentRequired: boolean; - isRecurring: boolean; - videoCallUrl?: string; -}) => { - return { - title: booking.title, - startTime: booking.startTime, - endTime: booking.endTime, - eventTypeId: booking.eventTypeId, - status: booking.status, - paymentRequired: booking.paymentRequired, - isRecurring: booking.isRecurring, - videoCallUrl: booking.videoCallUrl, - }; -}; +const getBaseBookingEventPayload = (booking: { + title?: string; + startTime: string | Date; + endTime: string | Date; + eventTypeId?: number | null; + status?: BookingStatus; + paymentRequired: boolean; + isRecurring: boolean; + videoCallUrl?: string; +}) => { + const toISOStr = (v: string | Date | undefined) => + typeof v === "string" ? v : v?.toISOString?.() ?? (v ? String(v) : undefined); + + return { + title: booking.title, + startTime: toISOStr(booking.startTime), + endTime: toISOStr(booking.endTime), + eventTypeId: booking.eventTypeId, + status: booking.status, + paymentRequired: booking.paymentRequired, + isRecurring: booking.isRecurring, + videoCallUrl: booking.videoCallUrl, + }; +};
🧹 Nitpick comments (3)
packages/features/bookings/Booker/components/hooks/useBookings.ts (3)
82-98: DRY the argument type and align with base builder (accept Date|string)Reuse the param type of
getBaseBookingEventPayloadto keep both builders in sync and avoid repeating field lists. This also carries the widened date/string types from the base builder.Apply this diff:
-const getBookingSuccessfulEventPayload = (booking: { - title?: string; - startTime: string; - endTime: string; - eventTypeId?: number | null; - status?: BookingStatus; - paymentRequired: boolean; - uid?: string; - isRecurring: boolean; - videoCallUrl?: string; -}) => { +const getBookingSuccessfulEventPayload = ( + booking: Parameters<typeof getBaseBookingEventPayload>[0] & { uid?: string } +) => { return { uid: booking.uid, ...getBaseBookingEventPayload(booking), }; };
218-241: Remove unused validDuration computation in dry-run branch
validDurationis computed but not used in this branch, adding noise and minor overhead. Safe to remove.Apply this diff:
- const validDuration = event.data?.isDynamic - ? duration || event.data?.length - : duration && event.data?.metadata?.multipleDuration?.includes(duration) - ? duration - : event.data?.length; - if (isRescheduling) { sdkActionManager?.fire( "dryRunRescheduleBookingSuccessfulV2", getDryRunRescheduleBookingSuccessfulEventPayload({ ...booking, isRecurring: false, }) ); } else { sdkActionManager?.fire( "dryRunBookingSuccessfulV2", getDryRunBookingSuccessfulEventPayload({ ...booking, isRecurring: false, }) ); }
389-412: Avoid recomputing allBookings and normalize timestamps for recurring dry-run eventsCompute
allBookingsonce, normalize to strings (as above), and reuse in both branches. Also avoids shadowing the outerbookingvariable in the.mapcallback.Apply this diff:
if (booking.isDryRun) { - if (isRescheduling) { - sdkActionManager?.fire("dryRunRescheduleBookingSuccessfulV2", { - ...getDryRunRescheduleBookingSuccessfulEventPayload({ - ...booking, - isRecurring: true, - }), - allBookings: bookings.map((booking) => ({ - startTime: booking.startTime, - endTime: booking.endTime, - })), - }); - } else { - sdkActionManager?.fire("dryRunBookingSuccessfulV2", { - ...getDryRunBookingSuccessfulEventPayload({ - ...booking, - isRecurring: true, - }), - allBookings: bookings.map((booking) => ({ - startTime: booking.startTime, - endTime: booking.endTime, - })), - }); - } + const toISOStr = (v: any) => + typeof v === "string" ? v : v?.toISOString?.() ?? (v ? String(v) : undefined); + const allBookings = bookings.map((b) => ({ + startTime: toISOStr(b.startTime), + endTime: toISOStr(b.endTime), + })); + + if (isRescheduling) { + sdkActionManager?.fire("dryRunRescheduleBookingSuccessfulV2", { + ...getDryRunRescheduleBookingSuccessfulEventPayload({ + ...booking, + isRecurring: true, + }), + allBookings, + }); + } else { + sdkActionManager?.fire("dryRunBookingSuccessfulV2", { + ...getDryRunBookingSuccessfulEventPayload({ + ...booking, + isRecurring: true, + }), + allBookings, + }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/Booker/components/hooks/useBookings.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/Booker/components/hooks/useBookings.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/Booker/components/hooks/useBookings.ts
🧬 Code Graph Analysis (1)
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
packages/trpc/server/routers/publicViewer/procedures/event.ts (1)
event(8-12)
⏰ Context from checks skipped due to timeout of 90000ms. 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/Booker/components/hooks/useBookings.ts (1)
101-103: Public dry-run payload helper exports look goodAliases make the surface area explicit while sharing the same builder. Good for DX and future refactors.
…nRescheduleBookingSuccessfulV2) (#23072) * feat: add dry run events for SDK (dryRunBookingSuccessfulV2 and dryRunRescheduleBookingSuccessfulV2) - Add new event types dryRunBookingSuccessfulV2 and dryRunRescheduleBookingSuccessfulV2 to EventDataMap - Create payload functions that exclude uid field for dry run events - Fire dry run events when isDryRun is true in both regular and recurring booking flows - Add unit tests to verify dry run event payloads are correct and exclude uid field - Ensures dry run mode no longer skips event firing but uses appropriate dry run variants Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> * refactor: make dry run events more DRY by reusing types and payload logic - Create BaseBookingEventPayload type to eliminate duplication between regular and dry run events - Use intersection types to compose final event types in sdk-action-manager.ts - Create getBaseBookingEventPayload function for common payload generation logic - Export dry run payload functions and import them in tests instead of redefining - Fix missing videoCallUrl field in rescheduleBookingSuccessfulV2 type Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> * Remove test file --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
4 participants
refactor: make dry run events more DRY by reusing types and payload logic
Summary
This refactoring eliminates code duplication between regular and dry run SDK events by introducing shared base types and payload generation functions. The changes affect the SDK event system that fires when bookings are created or rescheduled.
Key changes:
BaseBookingEventPayloadtype to capture common event structureBaseBookingEventPayload & { uid: string | undefined })getBaseBookingEventPayloadfunction for shared payload generation logicvideoCallUrlfield inrescheduleBookingSuccessfulV2type (was present in dry run version but missing from regular version)The refactoring maintains identical runtime behavior while reducing ~60 lines of duplicated code and improving maintainability.
Review & Testing Checklist for Human
dryRunBookingSuccessfulV2anddryRunRescheduleBookingSuccessfulV2events fire correctly during dry run booking flowsvideoCallUrlin reschedule events)Diagram
%%{ init : { "theme" : "default" }}%% graph TB subgraph Legend L1["Major Edit"]:::major-edit L2["Minor Edit"]:::minor-edit L3["Context/No Edit"]:::context end SDK["sdk-action-manager.ts<br/>Event type definitions"]:::major-edit Hooks["useBookings.ts<br/>Payload generation"]:::major-edit Tests["useBookings.test.ts<br/>Unit tests"]:::minor-edit Base["BaseBookingEventPayload<br/>Shared type"]:::major-edit Events1["bookingSuccessfulV2<br/>rescheduleBookingSuccessfulV2"]:::context Events2["dryRunBookingSuccessfulV2<br/>dryRunRescheduleBookingSuccessfulV2"]:::context SDK --> Base Base --> Events1 Base --> Events2 Hooks --> SDK Tests --> Hooks classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
rescheduleBookingSuccessfulV2was missing thevideoCallUrlfield that was present in the dry run version - this has been fixed