Fixes that workflow reminders of cancelled and rescheduled bookings are 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>
This commit is contained in:
Carina Wollendorfer 2023-02-20 12:40:08 -05:00 committed by GitHub
parent 235ec0e906
commit 2ebfcf448c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 241 additions and 203 deletions

View File

@ -1,8 +1,6 @@
import {
BookingStatus,
MembershipRole,
Prisma,
PrismaPromise,
WebhookTriggerEvents,
WorkflowMethods,
WorkflowReminder,
@ -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));

View File

@ -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";
@ -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";
@ -759,6 +769,7 @@ async function handler(req: NextApiRequest & { userId?: number | undefined }) {
},
},
payment: true,
workflowReminders: true,
},
});
}
@ -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,

View File

@ -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";
@ -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: {

View File

@ -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>

View File

@ -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,
},
});
await client.request({
url: `/v3/user/scheduled_sends/${referenceId}`,
method: "DELETE",
return;
}
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}`);

View File

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

View File

@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "WorkflowReminder" ADD COLUMN "cancelled" BOOLEAN;

View File

@ -641,6 +641,7 @@ model WorkflowReminder {
scheduled Boolean
workflowStepId Int
workflowStep WorkflowStep @relation(fields: [workflowStepId], references: [id], onDelete: Cascade)
cancelled Boolean?
}
enum WorkflowTemplates {

View File

@ -5,7 +5,6 @@ import type {
Workflow,
WorkflowsOnEventTypes,
WorkflowStep,
PrismaPromise,
} from "@prisma/client";
import {
BookingStatus,
@ -482,30 +481,18 @@ export const bookingsRouter = router({
},
});
// delete scheduled jobs of cancelled bookings
// delete scheduled jobs of previous booking
cancelScheduledJobs(bookingToReschedule);
//cancel workflow reminders
const remindersToDelete: PrismaPromise<Prisma.BatchPayload>[] = [];
//cancel workflow reminders of previous booking
bookingToReschedule.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);
});
await Promise.all(remindersToDelete);
const [mainAttendee] = bookingToReschedule.attendees;
// @NOTE: Should we assume attendees language?
const tAttendees = await getTranslation(mainAttendee.locale ?? "en", "common");

View File

@ -1,11 +1,12 @@
import type { Prisma, PrismaPromise } from "@prisma/client";
import {
PrismaPromise,
WorkflowTemplates,
WorkflowActions,
WorkflowTriggerEvents,
BookingStatus,
WorkflowMethods,
TimeUnit,
Prisma
} from "@prisma/client";
import { z } from "zod";
@ -23,7 +24,6 @@ import {
scheduleEmailReminder,
} from "@calcom/features/ee/workflows/lib/reminders/emailReminderManager";
import {
// BookingInfo,
deleteScheduledSMSReminder,
scheduleSMSReminder,
} from "@calcom/features/ee/workflows/lib/reminders/smsReminderManager";
@ -208,13 +208,12 @@ export const workflowsRouter = router({
},
});
//cancel workflow reminders of deleted workflow
scheduledReminders.forEach((reminder) => {
if (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, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
@ -373,29 +372,15 @@ export const workflowsRouter = router({
const remindersToDelete = await Promise.all(remindersToDeletePromise);
const deleteReminderPromise: PrismaPromise<Prisma.BatchPayload>[] = [];
//cancel workflow reminders for all bookings from event types that got disabled
remindersToDelete.flat().forEach((reminder) => {
//already scheduled reminders
if (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 deleteReminder = ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
booking: {
userId: ctx.user.id,
},
},
});
deleteReminderPromise.push(deleteReminder);
});
await Promise.all(deleteReminderPromise);
//update active on & reminders for new eventTypes
await ctx.prisma.workflowsOnEventTypes.deleteMany({
where: {
@ -529,15 +514,13 @@ export const workflowsRouter = router({
});
//step was deleted
if (!newStep) {
//delete already scheduled reminders
// cancel all workflow reminders from deleted steps
if (remindersFromStep.length > 0) {
remindersFromStep.forEach((reminder) => {
if (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, true);
} else if (reminder.method === WorkflowMethods.SMS) {
deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
}
});
}
@ -586,19 +569,14 @@ export const workflowsRouter = router({
return reminder;
}
});
//cancel all workflow reminders from steps that were edited
remindersToUpdate.forEach(async (reminder) => {
if (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);
}
await ctx.prisma.workflowReminder.deleteMany({
where: {
id: reminder.id,
},
});
});
const eventTypesToUpdateReminders = activeOn.filter((eventTypeId) => {
if (!newEventTypes.includes(eventTypeId)) {
@ -923,63 +901,63 @@ export const workflowsRouter = router({
if (isSMSAction(step.action) /*|| step.action === WorkflowActions.EMAIL_ADDRESS*/ /*) {
const hasTeamPlan = (await ctx.prisma.membership.count({ where: { userId: user.id } })) > 0;
if (!hasTeamPlan) {
throw new TRPCError({ code: "UNAUTHORIZED", message: "Team plan needed" });
throw new TRPCError({ code: "UNAUTHORIZED", message: "Team plan needed" });
}
}
const booking = await ctx.prisma.booking.findFirst({
orderBy: {
createdAt: "desc",
createdAt: "desc",
},
where: {
userId: ctx.user.id,
userId: ctx.user.id,
},
include: {
attendees: true,
user: true,
attendees: true,
user: true,
},
});
let evt: BookingInfo;
if (booking) {
evt = {
uid: booking?.uid,
attendees:
booking?.attendees.map((attendee) => {
return { name: attendee.name, email: attendee.email, timeZone: attendee.timeZone };
}) || [],
organizer: {
language: {
locale: booking?.user?.locale || "",
},
name: booking?.user?.name || "",
email: booking?.user?.email || "",
timeZone: booking?.user?.timeZone || "",
},
startTime: booking?.startTime.toISOString() || "",
endTime: booking?.endTime.toISOString() || "",
title: booking?.title || "",
location: booking?.location || null,
additionalNotes: booking?.description || null,
customInputs: booking?.customInputs,
uid: booking?.uid,
attendees:
booking?.attendees.map((attendee) => {
return { name: attendee.name, email: attendee.email, timeZone: attendee.timeZone };
}) || [],
organizer: {
language: {
locale: booking?.user?.locale || "",
},
name: booking?.user?.name || "",
email: booking?.user?.email || "",
timeZone: booking?.user?.timeZone || "",
},
startTime: booking?.startTime.toISOString() || "",
endTime: booking?.endTime.toISOString() || "",
title: booking?.title || "",
location: booking?.location || null,
additionalNotes: booking?.description || null,
customInputs: booking?.customInputs,
};
} else {
//if no booking exists create an example booking
evt = {
attendees: [{ name: "John Doe", email: "john.doe@example.com", timeZone: "Europe/London" }],
organizer: {
language: {
locale: ctx.user.locale,
},
name: ctx.user.name || "",
email: ctx.user.email,
timeZone: ctx.user.timeZone,
},
startTime: dayjs().add(10, "hour").toISOString(),
endTime: dayjs().add(11, "hour").toISOString(),
title: "Example Booking",
location: "Office",
additionalNotes: "These are additional notes",
attendees: [{ name: "John Doe", email: "john.doe@example.com", timeZone: "Europe/London" }],
organizer: {
language: {
locale: ctx.user.locale,
},
name: ctx.user.name || "",
email: ctx.user.email,
timeZone: ctx.user.timeZone,
},
startTime: dayjs().add(10, "hour").toISOString(),
endTime: dayjs().add(11, "hour").toISOString(),
title: "Example Booking",
location: "Office",
additionalNotes: "These are additional notes",
};
}