-
Notifications
You must be signed in to change notification settings - Fork 11.6k
feat: SMS workflow action for form triggers #3 #23673
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
Conversation
…755107037-add-workflow-triggers
…ub.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…755107037-add-workflow-triggers
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (1)
44-47: Typo in example description ("Rounting" → "Routing")Fix spelling to avoid leaking into API docs.
- @ApiPropertyOptional({ description: "Name of the workflow", example: "Rounting-form Test Workflow" }) + @ApiPropertyOptional({ description: "Name of the workflow", example: "Routing-form Test Workflow" })apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (2)
46-53: Swagger: Missing ApiExtraModels for phone step DTOs (schemas may not render)Without registering WorkflowPhoneAttendeeStepDto and WorkflowPhoneNumberStepDto, the $ref targets in oneOf/discriminator can be absent in the OpenAPI output. Add them here.
@ApiExtraModels( OnFormSubmittedTriggerDto, OnFormSubmittedNoEventTriggerDto, WorkflowEmailAddressStepDto, WorkflowEmailAttendeeStepDto, + WorkflowPhoneAttendeeStepDto, + WorkflowPhoneNumberStepDto, RoutingFormWorkflowTriggerDto, WorkflowFormActivationDto )
35-43: Validation and Swagger type bugs in WorkflowFormActivationDto
- ValidateIf checks a non-existent property isActiveOnAllEventTypes. Should be isActiveOnAllRoutingForms.
- Swagger type hints list Number[], but the field is string[].
Fix both to avoid incorrect validation behavior and wrong OpenAPI docs.
@ApiPropertyOptional({ description: "List of routing form IDs the workflow applies to", example: ["abd1-123edf-a213d-123dfwf"], - type: [Number], + type: [String], }) - @ValidateIf((o) => !o.isActiveOnAllEventTypes) + @ValidateIf((o) => !o.isActiveOnAllRoutingForms) @IsOptional() @IsString({ each: true }) activeOnRoutingFormIds: string[] = [];
🧹 Nitpick comments (1)
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (1)
30-42: Optional: Trim unused ApiExtraModels entriesIf WhatsApp steps aren’t allowed for routing-form workflows, drop UpdatePhoneWhatsAppNumberWorkflowStepDto and UpdateWhatsAppAttendeePhoneWorkflowStepDto from ApiExtraModels to keep the OpenAPI clean.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts(3 hunks)apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts
🧬 Code graph analysis (2)
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (1)
apps/api/v2/src/modules/workflows/inputs/workflow-step.input.ts (4)
UpdatePhoneAttendeeWorkflowStepDto(381-389)UpdatePhoneNumberWorkflowStepDto(390-398)SMS_ATTENDEE(10-10)SMS_NUMBER(11-11)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
apps/api/v2/src/modules/workflows/inputs/workflow-step.input.ts (4)
WorkflowPhoneAttendeeStepDto(251-281)WorkflowPhoneNumberStepDto(283-307)SMS_ATTENDEE(10-10)SMS_NUMBER(11-11)
🔇 Additional comments (8)
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (4)
11-12: LGTM: SMS action constants importedConstants are correctly added for discriminator mapping and docs.
75-82: LGTM: Added phone step schemas to steps oneOfPhone step DTOs are correctly referenced in the OpenAPI oneOf for steps.
97-99: LGTM: Discriminator updated for SMS stepsDiscriminator subTypes now include SMS attendee/number actions.
102-107: LGTM: Steps union expanded to include phone stepsType union matches discriminator and oneOf. Good.
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (4)
10-16: LGTM: Introduced SMS constants and phone step DTOsImports are correct and align with added step support.
92-94: LGTM (but see next comment): Phone step schemas referenced in oneOfReferences are correct; however, you must add these DTOs to ApiExtraModels to ensure Swagger generates component schemas.
108-110: LGTM: Discriminator updated for SMS stepsSubtypes include SMS_ATTENDEE and SMS_NUMBER.
113-118: LGTM: Steps union expanded to include phone stepsType union matches the discriminator and oneOf.
ThyMinimalDev
left a comment
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.
LGTM
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.
4 issues found across 12 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="packages/features/ee/workflows/lib/reminders/smsReminderManager.ts">
<violation number="1" location="packages/features/ee/workflows/lib/reminders/smsReminderManager.ts:141">
Rule violated: **Avoid Logging Sensitive Information**
Remove the `reminderPhone` from this warning; phone numbers are PII and must not be logged per the "Avoid Logging Sensitive Information" guideline.</violation>
<violation number="2" location="packages/features/ee/workflows/lib/reminders/smsReminderManager.ts:217">
This mirrors the event path bug: smsMessageWithoutOptOut now contains the opt-out footer, but smsMessage stays unchanged. As a result, SMS messages are sent without the opt-out while fallback emails receive it. Please assign the opt-out-augmented message to smsMessage (or another variable) and keep the original string for bodyWithoutOptOut.</violation>
</file>
<file name="packages/features/ee/workflows/api/scheduleSMSReminders.ts">
<violation number="1" location="packages/features/ee/workflows/api/scheduleSMSReminders.ts:174">
This assigns the opt-out footer to `smsMessageWithoutOptOut` so the SMS body stays unchanged, causing scheduled SMS messages to miss the required opt-out text while the fallback email gains it. Please append the opt-out footer to `message` (the SMS body) and keep `smsMessageWithoutOptOut` equal to the original message.</violation>
</file>
<file name="apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts">
<violation number="1" location="apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts:14">
Please add the new SMS step DTOs to @ApiExtraModels so their schemas are emitted for the getSchemaPath references; otherwise the generated OpenAPI spec points to missing components.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| const isNumberVerified = await getIsNumberVerified(); | ||
|
|
||
| if (!isNumberVerified) { | ||
| log.warn(`Phone number not verified`, safeStringify({ reminderPhone, isNumberVerified })); |
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.
Rule violated: Avoid Logging Sensitive Information
Remove the reminderPhone from this warning; phone numbers are PII and must not be logged per the "Avoid Logging Sensitive Information" guideline.
Prompt for AI agents
Address the following comment on packages/features/ee/workflows/lib/reminders/smsReminderManager.ts at line 141:
<comment>Remove the `reminderPhone` from this warning; phone numbers are PII and must not be logged per the "Avoid Logging Sensitive Information" guideline.</comment>
<file context>
@@ -138,141 +134,224 @@ export const scheduleSMSReminder = async (args: ScheduleTextReminderArgs & { evt
const isNumberVerified = await getIsNumberVerified();
+ if (!isNumberVerified) {
+ log.warn(`Phone number not verified`, safeStringify({ reminderPhone, isNumberVerified }));
+ return;
+ }
</file context>
| log.warn(`Phone number not verified`, safeStringify({ reminderPhone, isNumberVerified })); | |
| log.warn(`Phone number not verified`, safeStringify({ isNumberVerified })); |
| if (process.env.TWILIO_OPT_OUT_ENABLED === "true") { | ||
| message = await WorkflowOptOutService.addOptOutMessage(message, locale || "en"); | ||
| } | ||
| const smsMessageWithoutOptOut = await WorkflowOptOutService.addOptOutMessage(message, locale || "en"); |
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.
This assigns the opt-out footer to smsMessageWithoutOptOut so the SMS body stays unchanged, causing scheduled SMS messages to miss the required opt-out text while the fallback email gains it. Please append the opt-out footer to message (the SMS body) and keep smsMessageWithoutOptOut equal to the original message.
Prompt for AI agents
Address the following comment on packages/features/ee/workflows/api/scheduleSMSReminders.ts at line 174:
<comment>This assigns the opt-out footer to `smsMessageWithoutOptOut` so the SMS body stays unchanged, causing scheduled SMS messages to miss the required opt-out text while the fallback email gains it. Please append the opt-out footer to `message` (the SMS body) and keep `smsMessageWithoutOptOut` equal to the original message.</comment>
<file context>
@@ -171,11 +171,7 @@ export async function handler(req: NextRequest) {
- if (process.env.TWILIO_OPT_OUT_ENABLED === "true") {
- message = await WorkflowOptOutService.addOptOutMessage(message, locale || "en");
- }
+ const smsMessageWithoutOptOut = await WorkflowOptOutService.addOptOutMessage(message, locale || "en");
const scheduledNotification = await scheduleSmsOrFallbackEmail({
</file context>
| WorkflowEmailAddressStepDto, | ||
| WorkflowEmailAttendeeStepDto, | ||
| WorkflowPhoneNumberStepDto, | ||
| WorkflowPhoneAttendeeStepDto, |
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.
Please add the new SMS step DTOs to @ApiExtraModels so their schemas are emitted for the getSchemaPath references; otherwise the generated OpenAPI spec points to missing components.
Prompt for AI agents
Address the following comment on apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts at line 14:
<comment>Please add the new SMS step DTOs to @ApiExtraModels so their schemas are emitted for the getSchemaPath references; otherwise the generated OpenAPI spec points to missing components.</comment>
<file context>
@@ -7,8 +7,12 @@ import {
WorkflowEmailAddressStepDto,
WorkflowEmailAttendeeStepDto,
+ WorkflowPhoneNumberStepDto,
+ WorkflowPhoneAttendeeStepDto,
+ SMS_ATTENDEE,
} from "./workflow-step.input";
</file context>
| } | ||
|
|
||
| if (smsMessage.trim().length > 0) { | ||
| const smsMessageWithoutOptOut = await WorkflowOptOutService.addOptOutMessage( |
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.
This mirrors the event path bug: smsMessageWithoutOptOut now contains the opt-out footer, but smsMessage stays unchanged. As a result, SMS messages are sent without the opt-out while fallback emails receive it. Please assign the opt-out-augmented message to smsMessage (or another variable) and keep the original string for bodyWithoutOptOut.
Prompt for AI agents
Address the following comment on packages/features/ee/workflows/lib/reminders/smsReminderManager.ts at line 217:
<comment>This mirrors the event path bug: smsMessageWithoutOptOut now contains the opt-out footer, but smsMessage stays unchanged. As a result, SMS messages are sent without the opt-out while fallback emails receive it. Please assign the opt-out-augmented message to smsMessage (or another variable) and keep the original string for bodyWithoutOptOut.</comment>
<file context>
@@ -138,141 +134,224 @@ export const scheduleSMSReminder = async (args: ScheduleTextReminderArgs & { evt
+ }
+
+ if (smsMessage.trim().length > 0) {
+ const smsMessageWithoutOptOut = await WorkflowOptOutService.addOptOutMessage(
+ smsMessage,
+ evt.organizer.language.locale
</file context>
E2E results are ready! |
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.
No issues found across 1 file
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.
No issues found across 1 file
* allow routing forms for activeOn * use repository function to get routing forms * remove unnecessary code * adjust logic in update handler * add triggers to api v2 * remvoe unused file * rename to getAcitveOnOptions handler * remove routingFormOptions handler * clean up getActiveOnOptions * refactor WorkflowService * remove logs * remove unused * fix: type check * fix: missed before after events for recurring * fix: calendarEvent handleMarkNoShow * fix error message Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * don't query disabled routing forms * create tasker function * add tasker code * move isFormTrigger function * small adjustments + todo comments * remove email to host action for form triggers * throw trpc error if email to host is added as step * fix dialog on how to use form responses as variables * remove add variable dropdown for form triggers * remove form workfows in event workflows tab * improvements for workflow logic on form submission * review fixes * base setup for seperate schedule functions (evt and form) * add missing BOOKING_PAID workflow trigger * fix pathname * fix: test for BOOKING_REQUESTED * fix activeOn ids * pass hideBranding and smsReminderNumber * adjustments to reminderScheduler * create empty scheduelForForm functions * pass locale and timezone with form user * pass formData instead of responses * pass timeFormat and locale * reusable function for email sending and reminder creation * implement scheduleEmailReminderForForm * remove added editor field from merge conflict * don't support cal.ai action with form triggers * throw bad request if form trigger and cal.ai is combined * add tests for scheduleFormWorkflows * add form submission tests * remove form response varibe info * clean up workflow actions * fixes for getting template options * pass triggerType to getAllWorkflows * move reusable logic to scheduleSMSReminder * add formdata to param type * type fixes for text reminder managers * implement scheduleSMSReminderForForm * fix import * fix isAuthorizedToAddActiveOnIds * disble whatsapp action * implement triggerFormSubmittedNoEventWorkflow * code clean up * Merge branch 'devin/1755107037-add-workflow-triggers' into feat/routing-form-workflow-triggers * fix type errors * remove async from getSubmitterEmail * fix type errors * revert cal.ai changes * fix type error * add sublogger * code clean up * fix type errors * remove label for attendee whatsapp action * code clean up * fixes saving teams on org workflows * fix type error * code improvements for activeOn ids * Revert "code improvements for activeOn ids" This reverts commit 0a3590a. * improve variable name * fix unit tests * small fixes * type fixes * remove unused translation keys * fix merge conflict issues * code clean up * remove SMS action support * remove more SMS code * add missing imports * set custom template for form action * type fixes * fix tasker endpoint * fix duplicate check * fix workfows.test.ts * use repository funciton to getHideBranding * add back SMS action * add back changes to smsReminderManager * code clean up * fix hasDuplicateSubmission * code clean up * select only needed properties * remove repository functions * Revert "remove repository functions" This reverts commit 7aa47b1. * add scheduleWorkflows function * Revert "add scheduleWorkflows function" This reverts commit fe5db4f. * move type to /types * Revert "move type to /types" This reverts commit 91e0152. * revert changes causing type errors * remove import * remove unused import * Revert "remove unused import" This reverts commit 1916768. * revert changed from attempt to fix type errors * pass object to gt all workflows * fix isAuthorized check * trigger filtering * remove form submitted no event booked code * remove form submitted no event from schema * remove more code * remove test * fixes * add getSubmitterEmail function * add trigger * small fixes * add missing workflow DTOs * small fixes * use activeOnWithChildren * fix active on when switching trigger type * remove add variable dropdown * add getAllWorkflowsFromRoutingForm to WorkflowService * fix error caused by undefined evt * fix type error * fix type error * fix tests * code clean up * final fixes and clean up * remove console.log * remove template text form from triggers * add routing form repoditory function * fix bug with key * add comments * fix test * add missing await * use predefined FormSubmissionData type * add .trim() to sms message * pass contextData instead * add missing trigger in update-workflow.input.ts * ForEvt and ForForm function for aiPhoneCallManager * chore: add support for form workflows on api v2 * fixup! chore: add support for form workflows on api v2 * use only repository functions in update handler * move all prisma queries from list.handler * review suggestions * chore: handle workflows api v2 * chore: handle workflows api v2, split in 2 endpoints * fix workflow step creation * remove connect agent and fixes types * add type to workflow * chore: use workflow type in apiv2 WorkflowsOutputService * update worklfow type on update * chore: use workflow type in apiv2 WorkflowsOutputService * fix template body for torm trigger * some UI fixes for email subject/body * resetting email body when changing form triggers * use type field to query workflows * clean up all old active on values * remove responseId from all funciton calls * remove undefined from updateTemplate * refactor: split routing form and event-type workflows code * refactor: split routing form and event-type workflows code * fix template text when adding action * chore: don't rename WorkflowActivationDto to avoid ci blocking * refine update schedule to use only allowed actions * fix type error * don't allow whatsapp action with form trigger * fix type error * return early if activeOn array is empty * fix: from step type in BaseFormWorkflowStepDto * fixup! fix: from step type in BaseFormWorkflowStepDto * api v2 updates * move all prisma calls to repository (service/workflows.ts) * use FORM_TRIGGER_WORKFLOW_EVENTS for form queries * use userRepository * use FORM_TRIGGER_WORKFLOW_EVENTS in isFormTrigger * code clean up * code clean up * use repository functions in formSubmissionValidation.ts * remove action check in update handler * add back trpc import * fix agent repository functions * add SMS actions to allowed form action constants * remove unsued import * fixes for offset api v2 * add missing responseId * fix failing test * fix failing test * remove unused imports * chore: handle sms step action for form worklfow in dtos --------- Co-authored-by: CarinaWolli <[email protected]> Co-authored-by: Amit Sharma <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Udit Takkar <[email protected]> Co-authored-by: Benny Joo <[email protected]> Co-authored-by: cal.com <[email protected]> Co-authored-by: Morgan <[email protected]>
What does this PR do?
On top of #23716
Adds SMS action for form triggers.
How should this be tested?