From 2ebfcf448c7e4136ac1b8b9ef9b526a4424fd155 Mon Sep 17 00:00:00 2001 From: Carina Wollendorfer <30310907+CarinaWolli@users.noreply.github.com> Date: Mon, 20 Feb 2023 12:40:08 -0500 Subject: [PATCH] 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 --- .../bookings/lib/handleCancelBooking.ts | 25 +-- .../features/bookings/lib/handleNewBooking.ts | 28 +++- .../workflows/api/scheduleEmailReminders.ts | 37 +++++ .../components/WorkflowStepContainer.tsx | 126 ++++++++------- .../lib/reminders/emailReminderManager.ts | 45 ++++-- .../lib/reminders/smsReminderManager.ts | 11 +- .../migration.sql | 2 + packages/prisma/schema.prisma | 1 + .../trpc/server/routers/viewer/bookings.tsx | 25 +-- .../trpc/server/routers/viewer/workflows.tsx | 144 ++++++++---------- 10 files changed, 241 insertions(+), 203 deletions(-) create mode 100644 packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql diff --git a/packages/features/bookings/lib/handleCancelBooking.ts b/packages/features/bookings/lib/handleCancelBooking.ts index 2e53fd2b463..acd85d3d128 100644 --- a/packages/features/bookings/lib/handleCancelBooking.ts +++ b/packages/features/bookings/lib/handleCancelBooking.ts @@ -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[] = []; + //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[] = [attendeeDeletes, bookingReferenceDeletes].concat( - remindersToDelete - ); + const prismaPromises: Promise[] = [attendeeDeletes, bookingReferenceDeletes]; await Promise.all(prismaPromises.concat(apiDeletes)); diff --git a/packages/features/bookings/lib/handleNewBooking.ts b/packages/features/bookings/lib/handleNewBooking.ts index 2149b8395d1..58b7ea4483e 100644 --- a/packages/features/bookings/lib/handleNewBooking.ts +++ b/packages/features/bookings/lib/handleNewBooking.ts @@ -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, diff --git a/packages/features/ee/workflows/api/scheduleEmailReminders.ts b/packages/features/ee/workflows/api/scheduleEmailReminders.ts index b1ef72f1f43..5125c9fb1b2 100644 --- a/packages/features/ee/workflows/api/scheduleEmailReminders.ts +++ b/packages/features/ee/workflows/api/scheduleEmailReminders.ts @@ -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[] = []; + + 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: { diff --git a/packages/features/ee/workflows/components/WorkflowStepContainer.tsx b/packages/features/ee/workflows/components/WorkflowStepContainer.tsx index 3fd3e6f7f85..3b13904cdb7 100644 --- a/packages/features/ee/workflows/components/WorkflowStepContainer.tsx +++ b/packages/features/ee/workflows/components/WorkflowStepContainer.tsx @@ -387,81 +387,75 @@ export default function WorkflowStepContainer(props: WorkflowStepProps) { }} /> - {(isPhoneNumberNeeded || isSenderIdNeeded) && ( + {isPhoneNumberNeeded && (
- {isPhoneNumberNeeded && ( + +
+ + 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); + }} + /> + +
+ + {form.formState.errors.steps && + form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo && ( +

+ {form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo?.message || ""} +

+ )} + {numberVerified ? ( +
+ {t("number_verified")} +
+ ) : ( <> - -
- - 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); +
+ { + setVerificationCode(e.target.value); }} + required />
- - {form.formState.errors.steps && - form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo && ( -

- {form.formState?.errors?.steps[step.stepNumber - 1]?.sendTo?.message || ""} -

- )} - {numberVerified ? ( -
- {t("number_verified")} -
- ) : ( - <> -
- { - setVerificationCode(e.target.value); - }} - required - /> - -
- - )} )}
diff --git a/packages/features/ee/workflows/lib/reminders/emailReminderManager.ts b/packages/features/ee/workflows/lib/reminders/emailReminderManager.ts index 14ffc893586..e84373b6907 100644 --- a/packages/features/ee/workflows/lib/reminders/emailReminderManager.ts +++ b/packages/features/ee/workflows/lib/reminders/emailReminderManager.ts @@ -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}`); diff --git a/packages/features/ee/workflows/lib/reminders/smsReminderManager.ts b/packages/features/ee/workflows/lib/reminders/smsReminderManager.ts index fad4d945bcd..6b17c0dbb0b 100644 --- a/packages/features/ee/workflows/lib/reminders/smsReminderManager.ts +++ b/packages/features/ee/workflows/lib/reminders/smsReminderManager.ts @@ -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}`); } diff --git a/packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql b/packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql new file mode 100644 index 00000000000..600081f994e --- /dev/null +++ b/packages/prisma/migrations/20230217230604_add_cancelled_to_workflow_reminder/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "WorkflowReminder" ADD COLUMN "cancelled" BOOLEAN; diff --git a/packages/prisma/schema.prisma b/packages/prisma/schema.prisma index 043f4f16a27..8e34f78988c 100644 --- a/packages/prisma/schema.prisma +++ b/packages/prisma/schema.prisma @@ -641,6 +641,7 @@ model WorkflowReminder { scheduled Boolean workflowStepId Int workflowStep WorkflowStep @relation(fields: [workflowStepId], references: [id], onDelete: Cascade) + cancelled Boolean? } enum WorkflowTemplates { diff --git a/packages/trpc/server/routers/viewer/bookings.tsx b/packages/trpc/server/routers/viewer/bookings.tsx index d3d36bb8374..0d58bbbf819 100644 --- a/packages/trpc/server/routers/viewer/bookings.tsx +++ b/packages/trpc/server/routers/viewer/bookings.tsx @@ -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[] = []; - + //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"); diff --git a/packages/trpc/server/routers/viewer/workflows.tsx b/packages/trpc/server/routers/viewer/workflows.tsx index c3441ea713a..639b2c764e1 100644 --- a/packages/trpc/server/routers/viewer/workflows.tsx +++ b/packages/trpc/server/routers/viewer/workflows.tsx @@ -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[] = []; + //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", }; }