Skip to content

Comprehensive workflow reminder management for booking lifecycle events #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ShashankFC wants to merge 1 commit into
base: workflow-queue-base
Choose a base branch
from
Open

Comprehensive workflow reminder management for booking lifecycle events #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 6 additions & 19 deletions packages/features/bookings/lib/handleCancelBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {
BookingStatus,
MembershipRole,
Prisma,
PrismaPromise,
WebhookTriggerEvents,
WorkflowMethods,
WorkflowReminder,
Expand Down Expand Up @@ -483,29 +481,18 @@ async function handler(req: NextApiRequest & { userId?: number }) {
cancelScheduledJobs(booking);
});

//Workflows - delete all reminders for bookings
const remindersToDelete: PrismaPromise<Prisma.BatchPayload>[] = [];
//Workflows - cancel all reminders for cancelled bookings
updatedBookings.forEach((booking) => {
booking.workflowReminders.forEach((reminder) => {
if (reminder.scheduled && reminder.referenceId) {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.referenceId);
}
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
const reminderToDelete = prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
remindersToDelete.push(reminderToDelete);
});
});

const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes].concat(
remindersToDelete
);
const prismaPromises: Promise<unknown>[] = [attendeeDeletes, bookingReferenceDeletes];

await Promise.all(prismaPromises.concat(apiDeletes));

Expand Down
28 changes: 26 additions & 2 deletions packages/features/bookings/lib/handleNewBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import type { App, Credential, EventTypeCustomInput, Prisma } from "@prisma/client";
import { BookingStatus, SchedulingType, WebhookTriggerEvents } from "@prisma/client";
import {
App,
BookingStatus,
Credential,
EventTypeCustomInput,
Prisma,
SchedulingType,
WebhookTriggerEvents,
WorkflowMethods,
} from "@prisma/client";
import async from "async";
import { isValidPhoneNumber } from "libphonenumber-js";
import { cloneDeep } from "lodash";
Expand Down Expand Up @@ -28,7 +36,9 @@ import {
sendScheduledEmails,
sendScheduledSeatsEmails,
} from "@calcom/emails";
import { deleteScheduledEmailReminder } from "@calcom/features/ee/workflows/lib/reminders/emailReminderManager";
import { scheduleWorkflowReminders } from "@calcom/features/ee/workflows/lib/reminders/reminderScheduler";
import { deleteScheduledSMSReminder } from "@calcom/features/ee/workflows/lib/reminders/smsReminderManager";
import getWebhooks from "@calcom/features/webhooks/lib/getWebhooks";
import { isPrismaObjOrUndefined, parseRecurringEvent } from "@calcom/lib";
import { getVideoCallUrl } from "@calcom/lib/CalEventParser";
Expand Down Expand Up @@ -759,6 +769,7 @@ async function handler(req: NextApiRequest & { userId?: number | undefined }) {
},
},
payment: true,
workflowReminders: true,
},
});
}
Expand Down Expand Up @@ -950,6 +961,19 @@ async function handler(req: NextApiRequest & { userId?: number | undefined }) {
let videoCallUrl;

if (originalRescheduledBooking?.uid) {
try {
// cancel workflow reminders from previous rescheduled booking
originalRescheduledBooking.workflowReminders.forEach((reminder) => {
if (reminder.method === WorkflowMethods.EMAIL) {
deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
} catch (error) {
log.error("Error while canceling scheduled workflow reminders", error);
}

// Use EventManager to conditionally use all needed integrations.
const updateManager = await eventManager.reschedule(
evt,
Expand Down
37 changes: 37 additions & 0 deletions packages/features/ee/workflows/api/scheduleEmailReminders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { NextApiRequest, NextApiResponse } from "next";
import dayjs from "@calcom/dayjs";
import { defaultHandler } from "@calcom/lib/server";
import prisma from "@calcom/prisma";
import { Prisma, WorkflowReminder } from "@calcom/prisma/client";
import { bookingMetadataSchema } from "@calcom/prisma/zod-utils";

import customTemplate, { VariablesType } from "../lib/reminders/templates/customTemplate";
Expand Down Expand Up @@ -39,6 +40,42 @@ async function handler(req: NextApiRequest, res: NextApiResponse) {
},
});

//cancel reminders for cancelled/rescheduled bookings that are scheduled within the next hour
const remindersToCancel = await prisma.workflowReminder.findMany({
where: {
cancelled: true,
scheduledDate: {
lte: dayjs().add(1, "hour").toISOString(),
},
},
});

try {
const workflowRemindersToDelete: Prisma.Prisma__WorkflowReminderClient<WorkflowReminder, never>[] = [];

for (const reminder of remindersToCancel) {
await client.request({
url: "/v3/user/scheduled_sends",
method: "POST",
body: {
batch_id: reminder.referenceId,
status: "cancel",
},
});

const workflowReminderToDelete = prisma.workflowReminder.delete({
where: {
id: reminder.id,
},
});

workflowRemindersToDelete.push(workflowReminderToDelete);
}
await Promise.all(workflowRemindersToDelete);
} catch (error) {
console.log(`Error cancelling scheduled Emails: ${error}`);
}

//find all unscheduled Email reminders
const unscheduledReminders = await prisma.workflowReminder.findMany({
where: {
Expand Down
126 changes: 60 additions & 66 deletions packages/features/ee/workflows/components/WorkflowStepContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,81 +387,75 @@ export default function WorkflowStepContainer(props: WorkflowStepProps) {
}}
/>
</div>
{(isPhoneNumberNeeded || isSenderIdNeeded) && (
{isPhoneNumberNeeded && (
<div className="mt-2 rounded-md bg-gray-50 p-4 pt-0">
{isPhoneNumberNeeded && (
<Label className="pt-4">{t("custom_phone_number")}</Label>
<div className="block sm:flex">
<PhoneInput<FormValues>
control={form.control}
name={`steps.${step.stepNumber - 1}.sendTo`}
placeholder={t("phone_number")}
id={`steps.${step.stepNumber - 1}.sendTo`}
className="min-w-fit sm:rounded-tl-md sm:rounded-bl-md sm:border-r-transparent"
required
onChange={() => {
const isAlreadyVerified = !!verifiedNumbers
?.concat([])
.find((number) => number === form.getValues(`steps.${step.stepNumber - 1}.sendTo`));
Comment on lines +402 to +404

@codeant-ai codeant-ai Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The use of optional chaining with verifiedNumbers?.concat([]).find(...) is unsafe: if verifiedNumbers is undefined (e.g. the query has not returned yet), verifiedNumbers?.concat([]) evaluates to undefined, and the subsequent .find(...) call on undefined will throw a runtime TypeError when the phone input changes. [possible bug]

Severity Level: Critical 🚨 
- ❌ SMS workflow step editor crashes while typing phone number.
- ⚠️ Admins may be blocked configuring SMS reminder workflows.
Suggested change
const isAlreadyVerified = !!verifiedNumbers
?.concat([])
.find((number) => number === form.getValues(`steps.${step.stepNumber - 1}.sendTo`));
const currentNumber = form.getValues(`steps.${step.stepNumber - 1}.sendTo`);
const isAlreadyVerified =
Array.isArray(verifiedNumbers) && currentNumber
? verifiedNumbers.includes(currentNumber)
: false;
Steps of Reproduction ✅ 
1. Open the workflows details UI that renders `WorkflowDetailsPage` at
`packages/features/ee/workflows/components/WorkflowDetailsPage.tsx:26-88`, which in turn
renders `WorkflowStepContainer` at lines 132-143 for each step.

2. Ensure there is a workflow step whose `action` is `WorkflowActions.SMS_NUMBER`, so that
`isPhoneNumberNeeded` is `true` per `WorkflowStepContainer` at
`packages/features/ee/workflows/components/WorkflowStepContainer.tsx:68-70` and the phone
number input block at lines 390-407 is rendered.

3. When the page first loads, the TRPC query
`trpc.viewer.workflows.getVerifiedNumbers.useQuery()` at `WorkflowStepContainer.tsx:61-62`
has not yet resolved, so `_verifiedNumbers` is `undefined` and therefore `verifiedNumbers`
is `undefined`.

4. Before the `getVerifiedNumbers` query finishes loading, type into the `PhoneInput`
rendered at `WorkflowStepContainer.tsx:394-407`; its `onChange` handler at lines 401-406
executes, evaluates `verifiedNumbers?.concat([])` to `undefined`, then calls `.find(...)`
on `undefined`, causing a runtime `TypeError` and breaking the React component for the
workflow step editor.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**Line:** 402:404
**Comment:**
	*Possible Bug: The use of optional chaining with `verifiedNumbers?.concat([]).find(...)` is unsafe: if `verifiedNumbers` is undefined (e.g. the query has not returned yet), `verifiedNumbers?.concat([])` evaluates to `undefined`, and the subsequent `.find(...)` call on `undefined` will throw a runtime TypeError when the phone input changes.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
👍 | 👎

setNumberVerified(isAlreadyVerified);
}}
/>
<Button
color="secondary"
disabled={numberVerified || false}
className={classNames(
"-ml-[3px] h-[40px] min-w-fit sm:block sm:rounded-tl-none sm:rounded-bl-none ",
numberVerified ? "hidden" : "mt-3 sm:mt-0"
)}
onClick={() =>
sendVerificationCodeMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
})
}>
{t("send_code")}
</Button>
</div>

{form.formState.errors.steps &&
form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo && (
<p className="mt-1 text-xs text-red-500">
{form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo?.message || ""}
</p>
)}
{numberVerified ? (
<div className="mt-1">
<Badge variant="green">{t("number_verified")}</Badge>
</div>
) : (
<>
<Label className="pt-4">{t("custom_phone_number")}</Label>
<div className="block sm:flex">
<PhoneInput<FormValues>
control={form.control}
name={`steps.${step.stepNumber - 1}.sendTo`}
placeholder={t("phone_number")}
id={`steps.${step.stepNumber - 1}.sendTo`}
className="min-w-fit sm:rounded-tl-md sm:rounded-bl-md sm:border-r-transparent"
required
onChange={() => {
const isAlreadyVerified = !!verifiedNumbers
?.concat([])
.find(
(number) => number === form.getValues(`steps.${step.stepNumber - 1}.sendTo`)
);
setNumberVerified(isAlreadyVerified);
<div className="mt-3 flex">
<TextField
className=" border-r-transparent"
placeholder="Verification code"
value={verificationCode}
onChange={(e) => {
setVerificationCode(e.target.value);
}}
required
/>
<Button
color="secondary"
disabled={numberVerified || false}
className={classNames(
"-ml-[3px] h-[40px] min-w-fit sm:block sm:rounded-tl-none sm:rounded-bl-none ",
numberVerified ? "hidden" : "mt-3 sm:mt-0"
)}
onClick={() =>
sendVerificationCodeMutation.mutate({
className="-ml-[3px] rounded-tl-none rounded-bl-none "
disabled={verifyPhoneNumberMutation.isLoading}
onClick={() => {
verifyPhoneNumberMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
})
}>
{t("send_code")}
code: verificationCode,
});
}}>
Verify
</Button>
</div>

{form.formState.errors.steps &&
form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo && (
<p className="mt-1 text-xs text-red-500">
{form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo?.message || ""}
</p>
)}
{numberVerified ? (
<div className="mt-1">
<Badge variant="green">{t("number_verified")}</Badge>
</div>
) : (
<>
<div className="mt-3 flex">
<TextField
className=" border-r-transparent"
placeholder="Verification code"
value={verificationCode}
onChange={(e) => {
setVerificationCode(e.target.value);
}}
required
/>
<Button
color="secondary"
className="-ml-[3px] rounded-tl-none rounded-bl-none "
disabled={verifyPhoneNumberMutation.isLoading}
onClick={() => {
verifyPhoneNumberMutation.mutate({
phoneNumber: form.getValues(`steps.${step.stepNumber - 1}.sendTo`) || "",
code: verificationCode,
});
}}>
Verify
</Button>
</div>
</>
)}
</>
)}
</div>
Expand Down
45 changes: 33 additions & 12 deletions packages/features/ee/workflows/lib/reminders/emailReminderManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,20 +194,41 @@ export const scheduleEmailReminder = async (
}
};

export const deleteScheduledEmailReminder = async (referenceId: string) => {
export const deleteScheduledEmailReminder = async (
reminderId: number,
referenceId: string | null,
immediateDelete?: boolean
) => {
try {
await client.request({
url: "/v3/user/scheduled_sends",
method: "POST",
body: {
batch_id: referenceId,
status: "cancel",
},
});
if (!referenceId) {
await prisma.workflowReminder.delete({
where: {
id: reminderId,
},
});

return;
}

await client.request({
url: `/v3/user/scheduled_sends/${referenceId}`,
method: "DELETE",
if (immediateDelete) {
await client.request({
url: "/v3/user/scheduled_sends",
method: "POST",
body: {
batch_id: referenceId,
status: "cancel",
},
});
return;
}

await prisma.workflowReminder.update({
where: {
id: reminderId,
},
data: {
cancelled: true,
},
});
} catch (error) {
console.log(`Error canceling reminder with error ${error}`);
Expand Down
11 changes: 9 additions & 2 deletions packages/features/ee/workflows/lib/reminders/smsReminderManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,16 @@ export const scheduleSMSReminder = async (
}
};

export const deleteScheduledSMSReminder = async (referenceId: string) => {
export const deleteScheduledSMSReminder = async (reminderId: number, referenceId: string | null) => {
try {
await twilio.cancelSMS(referenceId);
if (referenceId) {
await twilio.cancelSMS(referenceId);
}
await prisma.workflowReminder.delete({
where: {
id: reminderId,
},
});
} catch (error) {
console.log(`Error canceling reminder with error ${error}`);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "WorkflowReminder" ADD COLUMN "cancelled" BOOLEAN;
1 change: 1 addition & 0 deletions packages/prisma/schema.prisma
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ model WorkflowReminder {
scheduled Boolean
workflowStepId Int
workflowStep WorkflowStep @relation(fields: [workflowStepId], references: [id], onDelete: Cascade)
cancelled Boolean?
}

enum WorkflowTemplates {
Expand Down
Loading