Skip to content

Fixes that workflow reminders of cancelled and rescheduled bookings are still sent #1

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
linxia0415 wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fixes that workflow reminders of cancelled and rescheduled bookings are still sent #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8daa5c4
small UI fix
Feb 16, 2023
b62459d
fix cancelling scheduled emails
Feb 17, 2023
de55c8f
improve comments
Feb 17, 2023
b3c107f
delete reminders for rescheduled bookings
Feb 20, 2023
c416294
add migration file
Feb 20, 2023
2fdb95b
cancel rescheduled bookings immediately
Feb 20, 2023
ad9a0a8
Merge branch 'main' into fix/cancelled-rescheduled-workflow-emails
Feb 20, 2023
6048e2a
remove immediate delete for request reschedule
Feb 20, 2023
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) => {

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🟡 AI 代码审查发现问题

📋 问题概述

handleNewBooking中对workflowReminders的遍历逻辑存在潜在扩展性问题。当前代码只处理了EMAIL和SMS两种方法类型，但如果未来添加新的WorkflowMethods类型（如PUSH_NOTIFICATION），这些类型的提醒将不会被取消，可能导致资源浪费或错误状态。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 966

💬 详细说明：

  • 如果将来添加新的提醒方法类型，这些提醒将不会被正确取消，可能导致系统中残留无效的计划提醒，占用不必要的资源。

📝 问题代码：

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);
        }
      });

💡 修复建议：

应该使用switch语句并包含default分支来处理未知的提醒方法类型，或者在else if分支后添加日志记录未知类型。

✅ 修复示例：

🔗 参考链接

无

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`));
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) {

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🔴 AI 代码审查发现问题

📋 问题概述

deleteScheduledEmailReminder函数在处理referenceId为null的情况时，直接删除数据库记录而不尝试取消外部API调度。这可能导致外部服务仍然按计划发送邮件，而系统认为提醒已被取消。

📍 问题详情

🔴 问题 1 | 严重程度: HIGH | 行号: 203

💬 详细说明：

  • 如果referenceId为null但外部服务实际上已经安排了邮件发送，则无法取消该邮件，可能导致用户收到不应该收到的提醒邮件。

📝 问题代码：

if (!referenceId) {
      await prisma.workflowReminder.delete({
        where: {
          id: reminderId,
        },
      });

      return;
    }

💡 修复建议：

应增加日志记录，说明为什么referenceId为null，并考虑是否需要其他机制来处理这种情况。

✅ 修复示例：

🔗 参考链接

无

await prisma.workflowReminder.delete({
where: {
id: reminderId,
},
});

return;
}

await client.request({
url: `/v3/user/scheduled_sends/${referenceId}`,
method: "DELETE",
if (immediateDelete) {

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🟡 AI 代码审查发现问题

📋 问题概述

当immediateDelete为true时，函数只取消外部API调度但不更新数据库状态。这违反了'先外部后内部'的安全顺序原则，如果后续数据库操作失败，会导致状态不一致。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 213

💬 详细说明：

  • 如果外部API取消成功但数据库更新失败，系统将处于不一致状态：外部服务已取消提醒，但数据库中仍保留记录，可能造成混乱。

📝 问题代码：

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

💡 修复建议：

应先更新数据库状态为已取消，然后再取消外部API调度，确保即使外部调用失败，数据库状态也是正确的。

✅ 修复示例：

🔗 参考链接

无

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}`);

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🟡 AI 代码审查发现问题

📋 问题概述

错误处理不当，使用console.log而不是适当的错误处理。这可能导致错误信息丢失，难以调试问题。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 234

💬 详细说明：

  • 错误信息可能不会被正确记录或监控，使得生产环境中的问题难以排查。

📝 问题代码：

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({

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🔴 AI 代码审查发现问题

📋 问题概述

发现 2 个邻近问题（第 179–182 行）

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 179

💬 详细说明：

  • 如果referenceId为null但外部服务实际上已经安排了短信发送，则无法取消该短信，可能导致用户收到不应该收到的提醒短信。

📝 问题代码：

if (referenceId) {
      await twilio.cancelSMS(referenceId);
    }

💡 修复建议：

应增加日志记录，说明为什么referenceId为null，并考虑是否需要其他机制来处理这种情况。

✅ 修复示例：
🔴 问题 2 | 严重程度: HIGH | 行号: 182

💬 详细说明：

  • 如果Twilio取消成功但数据库删除失败，系统将保留一个已取消的提醒记录，可能导致未来的清理任务无法正确处理。

📝 问题代码：

await prisma.workflowReminder.delete({
      where: {
        id: reminderId,
      },
    });

💡 修复建议：

应考虑添加事务处理或补偿机制，确保两个操作要么都成功，要么都失败。

✅ 修复示例：

🔗 参考链接

无

where: {
id: reminderId,
},
});
} catch (error) {
console.log(`Error canceling reminder with error ${error}`);

@code-hawk-sit code-hawk-sit Bot Apr 7, 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.

🟡 AI 代码审查发现问题

📋 问题概述

错误处理不当，使用console.log而不是适当的错误处理。这可能导致错误信息丢失，难以调试问题。

📍 问题详情

🟡 问题 1 | 严重程度: MEDIUM | 行号: 188

💬 详细说明：

  • 错误信息可能不会被正确记录或监控，使得生产环境中的问题难以排查。

📝 问题代码：

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