-
Notifications
You must be signed in to change notification settings - Fork 0
Comprehensive workflow reminder management for booking lifecycle events #6
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.
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
base: workflow-queue-base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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"; | ||
|
|
@@ -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); | ||
| } | ||
|
Comment on lines
963
to
+975
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Async reminder deletions on reschedule are not awaited In the reschedule path, async helpers are called inside 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);
}
});Issues:
Refactor to collect and await the promises: - 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);
- }
+ try {
+ // cancel workflow reminders from previous rescheduled booking
+ await Promise.all(
+ originalRescheduledBooking.workflowReminders.map((reminder) => {
+ if (reminder.method === WorkflowMethods.EMAIL) {
+ return deleteScheduledEmailReminder(reminder.id, reminder.referenceId, true);
+ }
+ if (reminder.method === WorkflowMethods.SMS) {
+ return deleteScheduledSMSReminder(reminder.id, reminder.referenceId);
+ }
+ return Promise.resolve();
+ })
+ );
+ } catch (error) {
+ log.error("Error while canceling scheduled workflow reminders", error);
+ }🤖 Prompt for AI Agents |
||
|
|
||
| // Use EventManager to conditionally use all needed integrations. | ||
| const updateManager = await eventManager.reschedule( | ||
| evt, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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> | ||
|
Comment on lines
435
to
457
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use an error toast for wrong codes and add minimal client‑side validation In Recommend:
🤖 Prompt for AI Agents |
||
| </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> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- AlterTable | ||
| ALTER TABLE "WorkflowReminder" ADD COLUMN "cancelled" BOOLEAN; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Async reminder cancellations during booking cancel are not awaited
The new reminder‑cancellation loop:
calls async helpers without awaiting them, so:
Promise.all(prismaPromises.concat(apiDeletes))does not wait for reminder cancellations.Refactor to collect and await the reminder promises:
Also applies to: 495-497
🤖 Prompt for AI Agents