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>
There was a problem hiding this comment.
Pull request overview
This PR enhances workflow reminder management across the booking lifecycle by implementing a more robust cancellation strategy. Instead of immediately deleting workflow reminders, the system now marks them as cancelled and processes them asynchronously, improving reliability during booking operations.
Changes:
- Introduced a
cancelledfield to theWorkflowRemindermodel for tracking cancelled reminders - Updated reminder deletion functions to accept reminder IDs and handle cancellation logic
- Modified workflow and booking operations to use the new cancellation approach
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/prisma/schema.prisma | Added optional cancelled field to WorkflowReminder model |
| packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql | Database migration adding the cancelled column |
| packages/features/ee/workflows/lib/reminders/emailReminderManager.ts | Enhanced deletion function with immediate/deferred cancellation logic |
| packages/features/ee/workflows/lib/reminders/smsReminderManager.ts | Updated to delete reminder records alongside external cancellation |
| packages/trpc/server/routers/viewer/workflows.tsx | Refactored to use updated cancellation signatures across workflow operations |
| packages/trpc/server/routers/viewer/bookings.tsx | Simplified reschedule cancellation logic using new reminder deletion approach |
| packages/features/bookings/lib/handleCancelBooking.ts | Streamlined booking cancellation to use updated reminder deletion |
| packages/features/bookings/lib/handleNewBooking.ts | Added reminder cancellation for rescheduled bookings |
| packages/features/ee/workflows/api/scheduleEmailReminders.ts | Implemented batch processing of cancelled reminders scheduled within the next hour |
| packages/features/ee/workflows/components/WorkflowStepContainer.tsx | Fixed nesting structure for phone number verification UI |
Comments suppressed due to low confidence (1)
packages/features/ee/workflows/api/scheduleEmailReminders.ts:1
- Sequential API calls within the loop (lines 57-64) could cause performance issues when cancelling multiple reminders. Consider batching the client.request calls or executing them concurrently using Promise.all() to improve throughput.
/* Schedule any workflow reminder that falls within 72 hours for email */
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
|
|
||
| try { | ||
| const workflowRemindersToDelete: Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never>[] = []; |
There was a problem hiding this comment.
The type annotation contains a redundant namespace qualifier. 'Prisma.Prisma__WorkflowReminderClient' should be 'Prisma__WorkflowReminderClient' since Prisma is already imported from '@calcom/prisma/client'.
| const workflowRemindersToDelete: Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never>[] = []; | |
| const workflowRemindersToDelete = []; |
| await client.request({ | ||
| url: "/v3/user/scheduled_sends", | ||
| method: "POST", | ||
| body: { | ||
| batch_id: referenceId, | ||
| status: "cancel", | ||
| }, | ||
| }); | ||
| return; |
There was a problem hiding this comment.
The immediate cancellation logic (lines 214-222) is duplicated in scheduleEmailReminders.ts (lines 57-64). Consider extracting this into a shared helper function to reduce code duplication and improve maintainability.
| await prisma.workflowReminder.update({ | ||
| where: { | ||
| id: reminderId, | ||
| }, | ||
| data: { | ||
| cancelled: true, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The new cancellation flow that marks reminders as cancelled (instead of immediate deletion) lacks test coverage. This is a critical path for the reminder lifecycle and should be covered by tests to ensure the cancelled flag is properly set and processed.
| await client.request({ | ||
| url: "/v3/user/scheduled_sends", | ||
| method: "POST", | ||
| body: { | ||
| batch_id: reminder.referenceId, | ||
| status: "cancel", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Individual reminder cancellation failures will halt processing of remaining reminders. Consider wrapping the client.request call in a try-catch to log failures and continue processing other reminders, ensuring one failure doesn't prevent cancellation of others.
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)*