-
Notifications
You must be signed in to change notification settings - Fork 0
SMS workflow reminder retry count tracking #3
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: sms-retry-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 |
|---|---|---|
|
|
@@ -28,10 +28,19 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { | |
| //delete all scheduled sms reminders where scheduled date is past current date | ||
| await prisma.workflowReminder.deleteMany({ | ||
| where: { | ||
| method: WorkflowMethods.SMS, | ||
| scheduledDate: { | ||
| lte: dayjs().toISOString(), | ||
| }, | ||
| OR: [ | ||
| { | ||
| method: WorkflowMethods.SMS, | ||
| scheduledDate: { | ||
| lte: dayjs().toISOString(), | ||
| }, | ||
| }, | ||
| { | ||
| retryCount: { | ||
| gt: 1, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
|
|
@@ -44,8 +53,11 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { | |
| lte: dayjs().add(7, "day").toISOString(), | ||
| }, | ||
| }, | ||
| select, | ||
| })) as PartialWorkflowReminder[]; | ||
| select: { | ||
| ...select, | ||
| retryCount: true, | ||
| }, | ||
| })) as (PartialWorkflowReminder & { retryCount: number })[]; | ||
|
|
||
| if (!unscheduledReminders.length) { | ||
| res.json({ ok: true }); | ||
|
|
@@ -163,9 +175,26 @@ async function handler(req: NextApiRequest, res: NextApiResponse) { | |
| referenceId: scheduledSMS.sid, | ||
|
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.
|
||
| }, | ||
| }); | ||
| } else { | ||
| await prisma.workflowReminder.update({ | ||
| where: { | ||
| id: reminder.id, | ||
| }, | ||
| data: { | ||
| retryCount: reminder.retryCount + 1, | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| await prisma.workflowReminder.update({ | ||
| where: { | ||
| id: reminder.id, | ||
| }, | ||
| data: { | ||
| retryCount: reminder.retryCount + 1, | ||
| }, | ||
| }); | ||
| console.log(`Error scheduling SMS with error ${error}`); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- AlterTable | ||
| ALTER TABLE "WorkflowReminder" ADD COLUMN "retryCount" INTEGER NOT NULL DEFAULT 0; |
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.
🚨 CRITICAL
The new deleteMany predicate is too broad:
retryCount > 1is ORed with the SMS/scheduledDate filter, so any reminder of any method with a retry count above 1 will be deleted, even if it is still scheduled in the future. The root cause is that the retry cleanup condition was added at the top level instead of being scoped to the SMS reminder criteria.re-entry.ai