refactor: schedule SMS reminders via tasker for BEFORE_EVENT/AFTER_EVENT#27942
refactor: schedule SMS reminders via tasker for BEFORE_EVENT/AFTER_EVENT#27942anikdhabal wants to merge 3 commits intomainfrom
Conversation
|
I reviewed this PR thoroughly. The overall approach is solid — mirroring the lazy email scheduling pattern for SMS via Tasker is the right fix. Here's my review: SummaryThe PR correctly identifies and fixes the root cause: The fix routes these reminders through the Tasker instead, which handles scheduling at any future date. Core changes look good
Issues found in
|
There was a problem hiding this comment.
2 issues found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/tasker/tasks/sendWorkflowSMS.ts">
<violation number="1" location="packages/features/tasker/tasks/sendWorkflowSMS.ts:89">
P2: Limit the profile query to only the needed fields (organizationId) instead of fetching the full row.</violation>
<violation number="2" location="packages/features/tasker/tasks/sendWorkflowSMS.ts:169">
P2: The opt-out message is being added to `bodyWithoutOptOut` while the SMS body uses the unmodified message. This causes SMS to miss the opt-out text when enabled and emails to include it. Swap the usage so SMS gets the opt-out version and email uses the plain message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
|
I reviewed both Cubic AI violations and checked their confidence scores:
Neither issue meets the 9/10 confidence threshold required for automated fixes. These should be reviewed manually by a human reviewer if needed. |
Paragon: tests updated4 updated tests generated for this PR. Updated Tests
DetailsUpdated Tests
|
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/workflows/repositories/WorkflowReminderRepository.ts">
<violation number="1" location="packages/features/ee/workflows/repositories/WorkflowReminderRepository.ts:188">
P2: `attendees: true` pulls all attendee columns. The SMS task only uses a handful of attendee fields (email/name/phone/timezone/locale), so this over-fetches data and increases exposure. Select only the fields actually used.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/features/ee/workflows/repositories/WorkflowReminderRepository.ts
Show resolved
Hide resolved
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |


Summary by cubic
Schedules BEFORE_EVENT/AFTER_EVENT SMS reminders via Tasker for reliable queuing and cancellation. Adds lazy SMS scheduling and a Tasker task to send SMS with templates and fallback email while keeping the email path unchanged.
Written for commit c2d5fd5. Summary will update on new commits.