Comprehensive workflow reminder management for booking lifecycle events#6
Comprehensive workflow reminder management for booking lifecycle events#6akshayutture-augment 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>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| }, | ||
| data: { | ||
| cancelled: true, | ||
| }, |
There was a problem hiding this comment.
Bug: Incomplete Reminder Deletion Leaves Database Orphans
When immediateDelete is true, deleteScheduledEmailReminder cancels the SendGrid scheduled send but returns without deleting the database record. This causes orphaned workflow reminder records to accumulate in the database indefinitely. The function should delete the database entry after canceling the external send.
| if (reminder.method === WorkflowMethods.EMAIL) { | ||
| deleteScheduledEmailReminder(reminder.id, reminder.referenceId); | ||
| } else if (reminder.method === WorkflowMethods.SMS) { | ||
| deleteScheduledSMSReminder(reminder.id, reminder.referenceId); |
There was a problem hiding this comment.
Bug: Unawaited Promises Cause Race Conditions
The forEach callback is declared async but its promises are not awaited. The deleteScheduledEmailReminder and deleteScheduledSMSReminder calls execute without waiting for completion, potentially causing race conditions where workflow steps are deleted before reminders are properly canceled. Use await Promise.all(remindersToUpdate.map(...)) instead.
| await Promise.all(workflowRemindersToDelete); | ||
| } catch (error) { | ||
| console.log(`Error cancelling scheduled Emails: ${error}`); | ||
| } |
There was a problem hiding this comment.
Bug: Unfiltered Reminders Generate Cancellation Errors
The query for remindersToCancel doesn't filter for non-null referenceId. Reminders created for dates beyond 72 hours have referenceId: null. When these reminders are later marked as cancelled and the CRON job processes them, it attempts SendGrid API calls with batch_id: null, which fail and generate unnecessary error logs. The query should exclude reminders where referenceId is null.
2 participants
Test 6
Replicated from ai-code-review-evaluation/cal.com-cursor#6
Note
Adds cancellable workflow reminders with schema support and updates booking/workflow flows to cancel, clean up, and schedule reminders reliably (email/SMS).
cancelledflag toWorkflowReminder(schema + migration).deleteScheduledEmailReminder(reminderId, referenceId, immediateDelete?)to cancel via SendGrid or markcancelled.deleteScheduledSMSReminder(reminderId, referenceId)to cancel via Twilio and delete DB record.handleCancelBooking): cancel all associated workflow reminders (email/SMS) and scheduled jobs.handleNewBooking, viewer/bookings requestReschedule): cancel reminders from the previous booking before proceeding.scheduleEmailReminders):cancelledwithin the next hour; continue scheduling unscheduled reminders.WorkflowStepContainer):Written by Cursor Bugbot for commit 2ebfcf4. Configure here.