fix: no show webhook triggers not updating #21345
Conversation
|
@kart1ka is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/15/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (05/15/25)1 label was added to this PR based on Keith Williams's automation. |
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts
Show resolved
Hide resolved
|
I have addressed the comments. |
WalkthroughAdds end-to-end "no-show" webhook support: new scheduling and cancellation functions in the webhooks feature, refactors trigger-update logic to handle scheduling and no-show trigger types separately, uses tasker/dayjs to schedule no-show tasks, propagates booking.uid into no-show scheduling calls, and adds calls to cancel no-show tasks in booking lifecycle handlers (new, confirm, reschedule, cancel) and webhook deactivation. Also updates trigger cleanup to accumulate and await multiple deletion/cancellation promises. Assessment against linked issues
Possibly related PRs
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. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (1)
66-67: Good fix! Added referenceUid to task creation.The addition of
referenceUid: booking.uidto both host and guest no-show task creation calls properly addresses the previous review comment and ensures tasks can be correctly associated with bookings for later cancellation.Also applies to: 97-98
packages/features/webhooks/lib/scheduleTrigger.ts (2)
590-628: Consistent implementation with existing patterns.The
cancelNoShowTasksForBookingfunction properly handles task cancellation for both specific bookings (by uid) and bulk operations (by webhook). While bulk deletion could be slow with many bookings, the implementation follows the established pattern used inupdateTriggerForExistingBookings.The function correctly uses
tasker.cancelWithReferencefor specific trigger types and falls back to direct database deletion for general cases.
630-671: Well-implemented no-show task scheduling!The function properly:
- Validates all required fields before proceeding
- Correctly checks that the location is Cal Video (as no-show detection only works for Cal Video)
- Uses dayjs for time calculations with proper type casting
- Creates tasks with all necessary metadata including
referenceUidfor later cancellationThe trigger type validation logic is correct.
🧹 Nitpick comments (1)
packages/features/webhooks/lib/scheduleTrigger.ts (1)
377-470: Excellent refactoring! Extracted booking fetching logic.The
fetchBookingsFromWebhookhelper function cleanly extracts the complex booking fetching logic, making it reusable and improving code maintainability. The function properly handles all webhook configurations including organization, team, and user-level webhooks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/features/bookings/lib/handleCancelBooking.ts(2 hunks)packages/features/bookings/lib/handleConfirmation.ts(2 hunks)packages/features/bookings/lib/handleNewBooking.ts(4 hunks)packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts(3 hunks)packages/features/webhooks/lib/scheduleTrigger.ts(5 hunks)packages/trpc/server/routers/loggedInViewer/connectAndJoin.handler.ts(1 hunks)packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts(2 hunks)packages/trpc/server/routers/viewer/webhook/edit.handler.ts(2 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/trpc/server/routers/loggedInViewer/connectAndJoin.handler.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/webhook/edit.handler.tspackages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.tspackages/features/bookings/lib/handleCancelBooking.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/webhooks/lib/scheduleTrigger.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/trpc/server/routers/loggedInViewer/connectAndJoin.handler.tspackages/trpc/server/routers/viewer/bookings/requestReschedule.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/trpc/server/routers/viewer/webhook/edit.handler.tspackages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.tspackages/features/bookings/lib/handleCancelBooking.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/webhooks/lib/scheduleTrigger.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
packages/trpc/server/routers/loggedInViewer/connectAndJoin.handler.tspackages/features/bookings/lib/handleConfirmation.tspackages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/webhooks/lib/scheduleTrigger.ts
📚 Learning: the office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is spec...
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Applied to files:
packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.tspackages/trpc/server/routers/viewer/webhook/edit.handler.tspackages/features/bookings/lib/handleCancelBooking.tspackages/features/bookings/lib/handleNewBooking.tspackages/features/webhooks/lib/scheduleTrigger.ts
📚 Learning: applies to **/*.{ts,tsx} : flag excessive day.js use in performance-critical code; prefer native dat...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
packages/features/webhooks/lib/scheduleTrigger.ts
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (17)
packages/trpc/server/routers/loggedInViewer/connectAndJoin.handler.ts (1)
242-242: LGTM! Proper integration of booking UID for no-show triggers.The addition of
uid: updatedBooking.uidcorrectly provides the booking's unique identifier to thescheduleNoShowTriggersfunction, enabling proper no-show webhook trigger management.packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts (2)
9-12: LGTM! Proper import of no-show task cancellation function.The import correctly includes
cancelNoShowTasksForBookingalongside the existingdeleteWebhookScheduledTriggersimport, maintaining consistency in the import structure.
160-160: LGTM! Consistent cleanup of no-show tasks during reschedule.The addition of
cancelNoShowTasksForBookingproperly ensures that no-show tasks are cancelled when a booking is rescheduled, maintaining consistency with the overall no-show trigger management system.packages/features/bookings/lib/handleConfirmation.ts (2)
45-45: LGTM! Proper type extension for booking UID.The addition of
uid: stringto the booking object type correctly extends the interface to include the booking's unique identifier, ensuring type safety for no-show trigger management.
448-448: LGTM! Consistent integration of booking UID for no-show triggers.The addition of
uid: booking.uidproperly provides the booking's unique identifier to thescheduleNoShowTriggersfunction, enabling correct no-show webhook trigger scheduling.packages/trpc/server/routers/viewer/webhook/edit.handler.ts (2)
4-4: LGTM! Proper import of no-show task cancellation function.The import correctly includes
cancelNoShowTasksForBookingalongside existing webhook trigger management functions, maintaining import organization consistency.
55-62: LGTM! Proper cleanup of no-show tasks on webhook deactivation.The addition of
cancelNoShowTasksForBookingcorrectly ensures that when a webhook is deactivated, associated no-show tasks are cancelled before deleting scheduled triggers. The webhook context object provides the necessary identifiers for proper task cleanup.packages/features/bookings/lib/handleCancelBooking.ts (2)
12-15: LGTM! Proper import of no-show task cancellation function.The import correctly includes
cancelNoShowTasksForBookingalongsidedeleteWebhookScheduledTriggers, maintaining consistency in the import structure for webhook-related cleanup functions.
516-516: LGTM! Consistent cleanup of no-show tasks during booking cancellation.The addition of
cancelNoShowTasksForBookingproperly ensures that no-show tasks are cancelled when bookings are cancelled, maintaining consistency with the overall no-show trigger management system. The function is correctly added to the webhook trigger promises array.packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts (1)
8-14: LGTM! Type update aligns with no-show trigger requirements.The addition of the
uidproperty to the booking type is necessary for uniquely identifying bookings when scheduling and canceling no-show tasks.packages/features/bookings/lib/handleNewBooking.ts (4)
42-44: LGTM! Import added for no-show task cancellation.The import of
cancelNoShowTasksForBookingis correctly placed alongside related webhook trigger functions.
2081-2096: Good refactoring! Improved concurrent task handling.The change from a single promise to an array of promises allows proper concurrent handling of both webhook trigger deletion and no-show task cancellation during booking rescheduling. This ensures comprehensive cleanup of all scheduled tasks.
2129-2136: Excellent error handling improvement!Combining deletion and scheduling promises into a single Promise.all with proper error catching ensures all operations are attempted and errors are logged appropriately.
2239-2244: LGTM! Booking object properly includes uid.The booking object now correctly includes the
uidproperty required by the updatedscheduleNoShowTriggersfunction signature.packages/features/webhooks/lib/scheduleTrigger.ts (3)
1-8: LGTM! Imports properly organized for new functionality.The imports are correctly updated to support the new no-show trigger functionality, including
taskerfor task management and proper type imports.
22-26: Good constant definition for no-show triggers.The
NO_SHOW_TRIGGERSarray clearly defines the supported no-show webhook events, making the code more maintainable.
472-535: Well-structured refactoring for dual trigger support!The refactored
updateTriggerForExistingBookingsfunction elegantly handles both scheduling and no-show triggers by:
- Separating added/removed triggers for each type
- Using the extracted
fetchBookingsFromWebhookhelper- Efficiently processing all operations with
Promise.allThe early return optimization when no triggers change is a nice touch.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/bookings/lib/handleNewBooking.ts (2)
2236-2237: Type thescheduleTriggerPromisesarray (and consider a more generic name for the delete/cancel bucket)
- Minor type nit: explicitly annotate
scheduleTriggerPromisesasPromise<unknown>[]for consistency withdeleteWebhookScheduledTriggerPromises.- Naming nit:
deleteWebhookScheduledTriggerPromisesnow includes both deletions and no-show cancellations, so a more generic name likecleanupTriggerPromiseswould better reflect contents.Apply this diff to type
scheduleTriggerPromises:- const scheduleTriggerPromises = []; + const scheduleTriggerPromises: Promise<unknown>[] = [];
2284-2291: Use Promise.allSettled and structured error logging instead of .catch on Promise.all
Promise.allwill short-circuit on the first rejection, andJSON.stringify(error)often loses stack/message context.Promise.allSettledlets you log all failures without aborting, and usinggetErrorFromUnknown+safeStringifyyields better diagnostics.Apply this diff:
- await Promise.all([...deleteWebhookScheduledTriggerPromises, ...scheduleTriggerPromises]).catch( - (error) => { - loggerWithEventDetails.error( - "Error while scheduling or canceling webhook triggers", - JSON.stringify({ error }) - ); - } - ); + const results = await Promise.allSettled([ + ...deleteWebhookScheduledTriggerPromises, + ...scheduleTriggerPromises, + ]); + const failures = results.filter((r) => r.status === "rejected") as PromiseRejectedResult[]; + if (failures.length) { + loggerWithEventDetails.error( + "Error while scheduling or canceling webhook triggers", + safeStringify({ errors: failures.map((f) => getErrorFromUnknown(f.reason)) }) + ); + }
📜 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 sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/bookings/lib/handleNewBooking.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/lib/handleNewBooking.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/lib/handleNewBooking.ts
🧬 Code Graph Analysis (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
packages/features/webhooks/lib/scheduleTrigger.ts (2)
deleteWebhookScheduledTriggers(372-375)cancelNoShowTasksForBooking(590-628)
⏰ 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 (3)
packages/features/bookings/lib/handleNewBooking.ts (3)
41-44: LGTM: ImportingcancelNoShowTasksForBookingis the right integration pointWiring the cancel API alongside
deleteWebhookScheduledTriggersandscheduleTriggeraligns with the PR goal to clean up no-show tasks on lifecycle changes.
2241-2251: Good: Cancel old booking’s No-Show tasks on reschedulePushing both:
deleteWebhookScheduledTriggers({ booking: originalRescheduledBooking, ... })andcancelNoShowTasksForBooking({ bookingUid: originalRescheduledBooking.uid })ensures we clean up meeting started/ended scheduling and any no-show tasks for the original booking. This addresses the orphaned no-show task issue on reschedules.
2394-2399: Verified: scheduleNoShowTriggers uses booking.uid as referenceUid — no action requiredConfirmed in packages/features/bookings/lib/handleNewBooking/scheduleNoShowTriggers.ts:
- referenceUid is set from booking.uid when scheduling tasks (matches at lines ~66 and ~97).
- scheduleNoShowTriggers is exported (withReporting wrapper) at ~line 116.
What does this PR do?
The webhook triggers like Meeting Started and Meeting Ended are correctly updating for existing bookings when a webhook is created or updated. However, the No Show (Hosts/Guests) triggers were not being updated in the same way.
This PR fixes that issue.
Visual Demo (For contributors especially)
Could not reproduce the bug for webhook triggers such as Meeting started and Meeting ended.
https://www.loom.com/share/393185653d73437ca95523267b53f961?sid=363c0f88-42c2-4c84-874d-3dcbb342035f
No show webhook triggers fixed:
https://www.loom.com/share/73a5f001faa64b5099072ef4e3e63ea4?sid=de73daec-169c-4ff6-bbed-0836708c7967
Mandatory Tasks (DO NOT REMOVE)
Summary by mrge
Fixed an issue where No Show webhook triggers were not updating for existing bookings when a webhook was created or updated.