Comprehensive workflow reminder management for booking lifecycle events#7
Comprehensive workflow reminder management for booking lifecycle events#7ShashankFC wants to merge 1 commit into
Conversation
…re still sent (#7232) * small UI fix * fix cancelling scheduled emails * improve comments * delete reminders for rescheduled bookings * add migration file * cancel rescheduled bookings immediately * remove immediate delete for request reschedule --------- Co-authored-by: CarinaWolli <wollencarina@gmail.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| }, | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Email reminder DB records orphaned during rescheduling
Medium Severity
When immediateDelete is true and referenceId is non-null, deleteScheduledEmailReminder cancels the SendGrid scheduled send but neither deletes the WorkflowReminder DB record nor marks it as cancelled. In handleNewBooking.ts, this is called during rescheduling where the old booking is only status-changed (not deleted), so no cascade delete cleans up these records. This leaves orphaned WorkflowReminder rows in the database. The other callers using immediateDelete (workflow/step deletion in workflows.tsx) are safe because cascade deletes handle cleanup, but the reschedule path has no such cascade.
Additional Locations (1)
| await Promise.all(workflowRemindersToDelete); | ||
| } catch (error) { | ||
| console.log(`Error cancelling scheduled Emails: ${error}`); | ||
| } |
There was a problem hiding this comment.
Single SendGrid failure blocks all cancelled reminder processing
Medium Severity
The for...of loop cancelling SendGrid scheduled sends shares a single try/catch with the Promise.all for DB deletes. If any one SendGrid cancellation request fails, the entire block exits to catch—reminders already successfully cancelled on SendGrid won't have their DB records deleted, and remaining reminders won't be processed at all. On subsequent cron runs, already-cancelled reminders are retried on SendGrid, likely failing again (not idempotent), creating a persistent blockage.
2 participants
Test 6nn
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#6](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/6)*Note
Medium Risk
Touches booking lifecycle and workflow reminder cancellation logic plus a Prisma migration; incorrect handling could leave reminders sending after cancellations/reschedules or delete the wrong reminder records.
Overview
Booking cancellation/rescheduling now cancels associated workflow reminders instead of directly deleting reminder rows, with updated calls across booking handlers and TRPC booking/workflow routes to pass reminder IDs and handle missing
referenceIds.Introduces a new
WorkflowReminder.cancelledflag (with migration) and updates email/SMS reminder managers: email cancellations can either mark reminders as cancelled, immediately cancel SendGrid batches, or delete DB rows when unscheduled; SMS cancellations now cancel in Twilio when possible and always delete the reminder row.Adds cron-side cleanup in
scheduleEmailRemindersto actively cancel and delete SendGrid scheduled sends for reminders markedcancelledand due soon, and updates the workflow UI’s SMS number verification section flow/visibility logic.Written by Cursor Bugbot for commit 2ebfcf4. Configure here.