feat: add routing form triggers to workflows#23421
feat: add routing form triggers to workflows#23421CarinaWolli wants to merge 162 commits intomainfrom
Conversation
- Add BOOKING_REJECTED, BOOKING_REQUESTED, BOOKING_PAYMENT_INITIATED, BOOKING_PAID, BOOKING_NO_SHOW_UPDATED to WorkflowTriggerEvents enum
- Update workflow constants to include new trigger options
- Implement workflow trigger logic for booking rejected and requested events
- Add translations for new workflow triggers following {enum}_trigger format
- Generate updated Prisma types for new schema changes
Co-Authored-By: amit@cal.com <samit91848@gmail.com>
- Add WorkflowTriggerEvents import to handleNewBooking.ts - Implement workflow trigger logic for BOOKING_REQUESTED in else block - Filter workflows by BOOKING_REQUESTED trigger and call scheduleWorkflowReminders - Use proper calendar event object construction without type casting - Add error handling for workflow reminder scheduling Co-Authored-By: amit@cal.com <samit91848@gmail.com>
…manager.devin.ai/proxy/github.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
- Add proper database includes for user information in handleConfirmation.ts - Fix ExtendedCalendarEvent type structure with correct hosts mapping - Add missing properties to calendar event objects in handleMarkNoShow.ts - Ensure all workflow triggers follow proper type patterns Co-Authored-By: amit@cal.com <samit91848@gmail.com>
- Add workflow configurations for BOOKING_REQUESTED and BOOKING_PAYMENT_INITIATED in fresh-booking.test.ts - Add workflow configuration for BOOKING_REJECTED in confirm.handler.test.ts - Enable previously skipped confirm.handler.test.ts - Remove workflow test assertions temporarily until triggers are fully functional - Maintain webhook test coverage while adding workflow test infrastructure Co-Authored-By: amit@cal.com <samit91848@gmail.com>
….handler.test.ts - Import mockSuccessfulVideoMeetingCreation from bookingScenario utils - Add mock call to BOOKING_REJECTED workflow test case - Resolves ReferenceError that was causing unit test CI failure Co-Authored-By: amit@cal.com <samit91848@gmail.com>
…ing booking trigger events - Extract complex conditional logic into helper functions (isImmediateTrigger, isTimeBased, shouldProcessWorkflow) - Add missing workflow trigger events with immediate execution logic - Update test workflows to use different actions (EMAIL_ATTENDEE, SMS_ATTENDEE) for better differentiation - Fix translation function mock in confirm.handler.test.ts using mockNoTranslations utility - Maintain existing functionality while improving code maintainability Co-Authored-By: amit@cal.com <samit91848@gmail.com>
…manager.devin.ai/proxy/github.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
…and workflow coverage - Create handleMarkNoShow.test.ts following confirm.handler.test.ts pattern - Add expectBookingNoShowUpdatedWebhookToHaveBeenFired utility function - Test both webhook and workflow triggers for BOOKING_NO_SHOW_UPDATED - Cover attendee/host no-show scenarios, multiple attendees, and error cases - All 6 unit tests pass with proper mocking of external dependencies Co-Authored-By: amit@cal.com <samit91848@gmail.com>
…webhook and workflow coverage" This reverts commit 7642992.
…ub.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
This reverts commit 1916768.
| return tasker.create( | ||
| "triggerFormSubmittedNoEventWorkflow", | ||
| { |
There was a problem hiding this comment.
I think we should add routing form id in referenceUid
| export function getSubmitterEmail(responses: any) { | ||
| const submitterEmail = Object.values(responses).find( | ||
| (response): response is { value: string; label: string } => { | ||
| const value = | ||
| typeof response === "object" && response && "value" in response ? response.value : response; | ||
| return typeof value === "string" && value.includes("@"); | ||
| } | ||
| )?.value; | ||
| return submitterEmail; |
There was a problem hiding this comment.
NIT: Although very low change but there could be other response values with @ in it. we should maybe use regex
const emailRegex = /^[^\s@]+@[^\s@]+.[^\s@]+$/;
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts (1)
67-69: Do not log webhook secrets
console.error(..., webhook, ...)emitswebhook.secretin logs. Sanitize to avoid secret leakage.- }).catch((e) => { - console.error(`Error executing FORM_SUBMITTED_NO_EVENT webhook`, webhook, e); - }); + }).catch((e) => { + console.error(`Error executing FORM_SUBMITTED_NO_EVENT webhook`, { subscriberUrl: webhook.subscriberUrl, appId: webhook.appId }, e); + });packages/trpc/server/routers/viewer/routing-forms/response.handler.ts (1)
18-37: Replace include with select in response.handler
Refactor the Prisma findUnique call in packages/trpc/server/routers/viewer/routing-forms/response.handler.ts to use an explicit select clause listing only the fields consumed by getSerializableForm (id, name, description, position, routes, fields, userId, teamId, settings, createdAt, updatedAt, updatedById, disabled) plus relations (team.parentId and user.id, email, timeFormat, locale). Ensure the select set fully matches getSerializableForm’s input requirements.packages/lib/server/repository/PrismaRoutingFormRepository.ts (1)
41-75: Use select instead of include for Prisma query.Keep consistency with defaultSelect and avoid include to prevent overfetching.
- return (await prisma.app_RoutingForms_Form.findUnique({ - where: { - id: formId, - }, - include: { - user: { - select: { - id: true, - username: true, - email: true, - movedToProfileId: true, - metadata: true, - timeFormat: true, - locale: true, - organization: { - select: { - slug: true, - }, - }, - }, - }, - team: { - select: { - parentId: true, - parent: { - select: { - slug: true, - }, - }, - slug: true, - metadata: true, - }, - }, - }, - })) as RoutingFormWithUserTeamAndOrg | null; + return (await prisma.app_RoutingForms_Form.findUnique({ + where: { id: formId }, + select: { + ...defaultSelect, + user: { + select: { + id: true, + username: true, + email: true, + movedToProfileId: true, + metadata: true, + timeFormat: true, + locale: true, + organization: { select: { slug: true } }, + }, + }, + team: { + select: { + parentId: true, + parent: { select: { slug: true } }, + slug: true, + metadata: true, + }, + }, + }, + })) as RoutingFormWithUserTeamAndOrg | null;packages/features/ee/workflows/lib/reminders/whatsappReminderManager.ts (1)
29-49: Do not hard-block WHATSAPP_ATTENDEE on missing verificationEarly-return on Line 46 prevents WHATSAPP_ATTENDEE (which “does not need to be verified”) from ever sending. Gate on actual verification needs instead of unconditionally returning.
Apply:
- if (!verifiedAt) { - log.warn(`Workflow step ${workflowStepId} not verified`); - return; - } + // Only block when number verification is required and not satisfied + async function isAllowedToSend() { + if (action === WorkflowActions.WHATSAPP_ATTENDEE) return true; + const isVerified = await getIsNumberVerified(); + return !!verifiedAt || isVerified; + } + if (!(await isAllowedToSend())) { + log.warn(`Workflow step ${workflowStepId} not verified`); + return; + }packages/features/ee/workflows/components/AddActionDialog.tsx (1)
80-88: Fix uncontrolled Select vs form default mismatch (submits wrong action)The Select shows actionOptions[0] but the form default is EMAIL_HOST. Submitting without user interaction sends EMAIL_HOST, not the visible default.
Apply:
- const form = useForm<AddActionFormValues>({ + const form = useForm<AddActionFormValues>({ mode: "onSubmit", defaultValues: { - action: WorkflowActions.EMAIL_HOST, + action: actionOptions[0]?.value ?? WorkflowActions.EMAIL_HOST, senderId: SENDER_ID, senderName: SENDER_NAME, }, resolver: zodResolver(formSchema), }); @@ - <Controller - name="action" - control={form.control} - render={() => { - return ( - <Select - isSearchable={false} - className="text-sm" - menuPlacement="bottom" - defaultValue={actionOptions[0]} - onChange={handleSelectAction} - options={actionOptions} - /> - ); - }} - /> + <Controller + name="action" + control={form.control} + render={({ field }) => ( + <Select + isSearchable={false} + className="text-sm" + menuPlacement="bottom" + value={actionOptions.find((o) => o.value === field.value) ?? actionOptions[0]} + onChange={(opt) => { + if (opt) { + field.onChange(opt.value); + handleSelectAction(opt as ISelectActionOption); + } + }} + options={actionOptions} + /> + )} + />Also applies to: 161-174
packages/trpc/server/routers/viewer/workflows/list.handler.ts (2)
51-109: Prisma: replace include with select and only project needed fieldsRepo guideline: never use include; use select with minimal fields. This query currently pulls full objects (e.g., team.members). Tighten projections.
- const activeOrgWorkflows = await prisma.workflow.findMany({ - where: { - ...excludeFormTriggersWhereClause, - team: { id: org.id, members: { some: { userId: ctx.user.id, accepted: true } } }, - OR: [{ isActiveOnAll: true }, { activeOnTeams: { some: { team: { OR: [{ id: input.teamId }, { members: { some: { userId: ctx.user.id, accepted: true } } }] } } } }], - }, - include: { - team: { select: { id: true, slug: true, name: true, members: true } }, - activeOnTeams: { select: { team: { select: { id: true, name: true } } } }, - steps: true, - }, - }); + const activeOrgWorkflows = await prisma.workflow.findMany({ + where: { + ...excludeFormTriggersWhereClause, + team: { id: org.id, members: { some: { userId: ctx.user.id, accepted: true } } }, + OR: [ + { isActiveOnAll: true }, + { + activeOnTeams: { + some: { + team: { + OR: [{ id: input.teamId }, { members: { some: { userId: ctx.user.id, accepted: true } } }], + }, + }, + }, + }, + ], + }, + select: { + id: true, + name: true, + userId: true, + teamId: true, + trigger: true, + time: true, + timeUnit: true, + steps: true, + team: { select: { id: true, slug: true, name: true } }, + activeOnTeams: { select: { team: { select: { id: true, name: true } } } }, + }, + });
118-161: Apply the same select-only pattern to the remaining queriesteamWorkflows, userWorkflows, and allWorkflows also use include and overfetch. Replace with select and constrain members to fields used for readOnly computation.
- include: { - team: { select: { id: true, slug: true, name: true, members: true } }, - activeOn: { select: { eventType: { select: { id: true, title: true, parentId: true, _count: { select: { children: true } } } } } }, - steps: true, - }, + select: { + id: true, + name: true, + userId: true, + teamId: true, + trigger: true, + time: true, + timeUnit: true, + steps: true, + team: { + select: { + id: true, + slug: true, + name: true, + members: { select: { userId: true, role: true, accepted: true } }, + }, + }, + activeOn: { + select: { + eventType: { + select: { + id: true, + title: true, + parentId: true, + _count: { select: { children: true } }, + }, + }, + }, + }, + },Replicate for userWorkflows and allWorkflows; drop team.members where not used.
I can push a PR-wide patch converting all include usages under this router to select with minimal projections.
Also applies to: 174-210, 217-264
packages/features/ee/workflows/lib/reminders/smsReminderManager.ts (1)
108-118: Do not hard-block SMS_ATTENDEE on missing verificationSame issue as WhatsApp: early-return blocks attendee flows that should bypass verification. Consolidate the gate with getIsNumberVerified.
- if (!verifiedAt) { - log.warn(`Workflow step ${workflowStepId} not yet verified`); - return; - } + async function isAllowedToSend() { + if (action === WorkflowActions.SMS_ATTENDEE) return true; + const isVerified = await getIsNumberVerified(); + return !!verifiedAt || isVerified; + } + if (!(await isAllowedToSend())) { + log.warn(`Workflow step ${workflowStepId} not yet verified`); + return; + }Also applies to: 128-142
apps/api/v2/src/modules/workflows/inputs/update-workflow.input.ts (1)
211-224: Type union omits the two new form trigger DTOs; add them to UpdateWorkflowDto.trigger.
Without them, TS types don’t reflect the documented/validated surface.Apply:
trigger?: | OnAfterEventTriggerDto | OnBeforeEventTriggerDto | OnCreationTriggerDto | OnRescheduleTriggerDto | OnCancelTriggerDto | OnRejectedTriggerDto | OnRequestedTriggerDto | OnPaidTriggerDto | OnPaymentInitiatedTriggerDto | OnNoShowUpdateTriggerDto | OnAfterCalVideoGuestsNoShowTriggerDto | OnAfterCalVideoHostsNoShowTriggerDto + | OnFormSubmittedTriggerDto + | OnFormSubmittedNoEventTriggerDto;packages/trpc/server/routers/viewer/workflows/util.ts (1)
469-512: Use select, not include; drop unused children.
Align with Prisma query guidelines and reduce payload.Apply:
- const newEventType = await prisma.eventType.findUnique({ - where: { - id, - }, - include: { - users: { - select: { - id: true, - }, - }, - children: true, - }, - }); + const newEventType = await prisma.eventType.findUnique({ + where: { id }, + select: { + teamId: true, + userId: true, + users: { select: { id: true } }, + }, + });apps/api/v2/src/modules/workflows/inputs/create-workflow.input.ts (1)
71-81: Creation API lacks activeOnRoutingFormIdsCreateWorkflowDto/WorkflowActivationDto only accept event-type IDs. For form triggers, clients need to send routing form IDs at creation to avoid a required follow-up update.
Apply this diff to add routing-form targets to the creation payload (validation specifics can be enforced server-side based on trigger type):
@ApiPropertyOptional({ description: "List of event-types IDs the workflow applies to, required if isActiveOnAllEventTypes is false", example: [698191], type: [Number], }) @ValidateIf((o) => !Boolean(o.isActiveOnAllEventTypes)) @IsOptional() @IsNumber({}, { each: true }) activeOnEventTypeIds: number[] = []; + + @ApiPropertyOptional({ + description: "List of routing-form IDs the workflow applies to (for form-triggered workflows)", + example: ["rf_123"], + type: [String], + }) + @IsOptional() + @IsString({ each: true }) + activeOnRoutingFormIds: string[] = [];packages/trpc/server/routers/viewer/workflows/update.handler.ts (1)
711-750: Use select instead of include for Prisma fetchRepo guidelines discourage include; switch to select and trim fields to what’s needed.
Apply this diff:
- const workflow = await ctx.prisma.workflow.findUnique({ - where: { - id, - }, - include: { - activeOn: { - select: { - eventType: true, - }, - }, - activeOnTeams: { - select: { - team: true, - }, - }, - activeOnRoutingForms: { - select: { - routingForm: { - select: { - id: true, - name: true, - }, - }, - }, - }, - team: { - select: { - id: true, - slug: true, - members: true, - name: true, - isOrganization: true, - }, - }, - steps: { - orderBy: { - stepNumber: "asc", - }, - }, - }, - }); + const workflow = await ctx.prisma.workflow.findUnique({ + where: { id }, + select: { + id: true, + activeOn: { select: { eventType: true } }, + activeOnTeams: { select: { team: true } }, + activeOnRoutingForms: { + select: { routingForm: { select: { id: true, name: true } } }, + }, + team: { + select: { + id: true, + slug: true, + members: true, + name: true, + isOrganization: true, + }, + }, + // Note: nested orderBy is supported; keep as-is if required by UI + steps: { orderBy: { stepNumber: "asc" } }, + }, + });
♻️ Duplicate comments (13)
packages/trpc/server/routers/viewer/eventTypes/getActiveOnOptions.handler.ts (1)
196-206: Prisma select is lean — niceOnly selecting id and name aligns with our “select-only-what-you-need” rule. Also addresses the earlier suggestion to drop disabled from select.
packages/trpc/server/routers/viewer/workflows/update.schema.ts (1)
13-14: Enforce trigger-dependent constraints; default empty arraysRight now both arrays are required and cross-field rules aren’t enforced. Add defaults and a schema-level superRefine to prevent mixed targets and to require time/timeUnit for FORM_SUBMITTED_NO_EVENT. Mirrors prior feedback.
export const ZUpdateInputSchema = z.object({ id: z.number(), name: z.string(), - activeOnEventTypeIds: z.number().array(), // also includes team ids - activeOnRoutingFormIds: z.string().array(), + activeOnEventTypeIds: z.array(z.number()).default([]), // also includes team ids + activeOnRoutingFormIds: z.array(z.string()).default([]), @@ - isActiveOnAll: z.boolean().optional(), -}); + isActiveOnAll: z.boolean().optional(), +}).superRefine((data, ctx) => { + const isFormTrigger = + data.trigger === "FORM_SUBMITTED" || data.trigger === "FORM_SUBMITTED_NO_EVENT"; + const hasEventTypeIds = (data.activeOnEventTypeIds?.length ?? 0) > 0; + const hasRoutingFormIds = (data.activeOnRoutingFormIds?.length ?? 0) > 0; + + if (isFormTrigger && !hasRoutingFormIds) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["activeOnRoutingFormIds"], + message: "Required for form triggers", + }); + } + if (!isFormTrigger && !hasEventTypeIds && !data.isActiveOnAll) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["activeOnEventTypeIds"], + message: "Required unless isActiveOnAll", + }); + } + if (hasEventTypeIds && hasRoutingFormIds) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "Provide either eventTypeIds or routingFormIds, not both", + }); + } + if (data.trigger === "FORM_SUBMITTED_NO_EVENT") { + if (data.time == null || data.timeUnit == null) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["time"], + message: "time and timeUnit are required for FORM_SUBMITTED_NO_EVENT", + }); + } + } +});Also applies to: 35-36
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts (1)
53-62: Bug: getSubmitterEmail returns undefined for string responsesPredicate resolves string values but the return path reads
.value, which is undefined for strings. This breaks duplicate detection and follow-up actions when responses store raw strings.-export function getSubmitterEmail(responses: any) { - const submitterEmail = Object.values(responses).find( - (response): response is { value: string; label: string } => { - const value = - typeof response === "object" && response && "value" in response ? response.value : response; - return typeof value === "string" && value.includes("@"); - } - )?.value; - return submitterEmail; -} +export function getSubmitterEmail(responses: any) { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + for (const entry of Object.values(responses ?? {})) { + const value = + typeof entry === "object" && entry && "value" in (entry as any) ? (entry as any).value : entry; + if (typeof value === "string" && emailRegex.test(value)) return value; + } + return undefined; +}packages/trpc/server/routers/viewer/workflows/list.schema.ts (1)
3-7: Make includeOnlyEventTypeWorkflows default to false to avoid breaking callersRequired boolean breaks existing callers omitting the flag. Provide a default.
-export const ZListInputSchema = z.object({ +export const ZListInputSchema = z.object({ teamId: z.number().optional(), userId: z.number().optional(), - includeOnlyEventTypeWorkflows: z.boolean(), + includeOnlyEventTypeWorkflows: z.boolean().default(false), });packages/lib/hideBranding.ts (2)
1-1: Use default Prisma export.This module exports prisma as default. The named import will fail at runtime.
Apply:
-import { prisma } from "@calcom/prisma"; +import prisma from "@calcom/prisma";
74-77: Fix undefined-to-null mismatch for organizationHideBranding (user path).resolveHideBranding expects boolean | null. Coalesce undefined to null.
Apply:
return resolveHideBranding({ entityHideBranding: user.hideBranding, - organizationHideBranding: user.profiles?.[0]?.organization?.hideBranding, + organizationHideBranding: user.profiles?.[0]?.organization?.hideBranding ?? null, });packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWorkflow.ts (1)
32-58: Wrap entire flow in try/catch; log error object; normalize nullable SMS.Parse/validation/eligibility errors currently bubble and fail the task without context; also pass undefined instead of null for optional arg.
-export async function triggerFormSubmittedNoEventWorkflow(payload: string): Promise<void> { - const { responseId, form, responses, smsReminderNumber, hideBranding, workflow, submittedAt } = - ZTriggerFormSubmittedNoEventWorkflowPayloadSchema.parse(JSON.parse(payload)); - - const shouldTrigger = await shouldTriggerFormSubmittedNoEvent({ - formId: form.id, - responseId, - responses, - submittedAt, - }); - - if (!shouldTrigger) return; - - try { +export async function triggerFormSubmittedNoEventWorkflow(payload: string): Promise<void> { + try { + const { responseId, form, responses, smsReminderNumber, hideBranding, workflow, submittedAt } = + ZTriggerFormSubmittedNoEventWorkflowPayloadSchema.parse(JSON.parse(payload)); + + const shouldTrigger = await shouldTriggerFormSubmittedNoEvent({ + formId: form.id, + responseId, + responses, + submittedAt, + }); + if (!shouldTrigger) return; + await scheduleWorkflowReminders({ - smsReminderNumber, + smsReminderNumber: smsReminderNumber ?? undefined, formData: { responses, user: { email: form.user.email, timeFormat: form.user.timeFormat, locale: form.user.locale ?? "en" }, }, hideBranding, workflows: [workflow as Workflow], }); } catch (error) { - log.error("Error while triggering form submitted no event workflows", JSON.stringify({ error })); + log.error({ err: error }, "Error while triggering form submitted no event workflows"); } }apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (1)
234-242: Offset now supported on no-event trigger.Extending TriggerOffsetDTO resolves the earlier gap for delay configuration.
packages/trpc/server/routers/viewer/workflows/util.ts (1)
514-561: Authorization bug: returns true on first authorized routing form; must require all IDs to pass.
This reintroduces the earlier issue; ensure the loop only returns true after fully validating every id.Apply:
for (const id of newRoutingFormIds) { @@ - if (!routingForm) return false; + if (!routingForm) return false; @@ - if (teamId && teamId !== routingForm.teamId) { - return false; - } + if (teamId && teamId !== routingForm.teamId) return false; + + // Personal ownership passes + if (!teamId && userId && routingForm.userId === userId) continue; @@ - if (routingForm.teamId && routingForm.team) { - const isTeamMember = routingForm.team.members.some( - (member) => member.userId === userId && member.accepted - ); - if (isTeamMember) { - return true; - } - } + if (routingForm.teamId && routingForm.team) { + const isTeamMember = routingForm.team.members.some( + (m) => m.userId === userId && m.accepted + ); + if (isTeamMember) continue; + } @@ - if ( - !teamId && - userId && - routingForm.userId !== userId && - !routingForm?.team?.members.some((member) => member.userId === userId && member.accepted) - ) { - return false; - } + // Not owned and not a member of the form's team ⇒ unauthorized + if (!teamId && userId) return false; } return true;packages/lib/server/service/workflows.ts (1)
141-169: Idempotency/dedup: add referenceUid to tasker job (reuse formId+responseId).
Prevents duplicate enqueueing and eases tracing.If tasker.create supports a referenceUid option, consider:
- return tasker.create( + return tasker.create( "triggerFormSubmittedNoEventWorkflow", { responseId, responses, smsReminderNumber, hideBranding, form: { id: form.id, userId: form.userId, teamId: form.teamId ?? undefined, user: { email: form.user.email, timeFormat: form.user.timeFormat, locale: form.user.locale ?? "en", }, }, workflow, submittedAt: new Date(), }, - { scheduledAt } + { scheduledAt, referenceUid: `routingForm:${form.id}:response:${responseId}` } );Would you like me to scan the repo for tasker.create’s options to confirm the exact property name?
apps/api/v2/src/modules/workflows/services/workflows.input.service.ts (1)
32-34: Offset DTO mismatch resolvedThe earlier concern about OnFormSubmittedNoEventTriggerDto lacking offset is fixed by extending TriggerOffsetDTO.
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (1)
86-119: Action filtering aligns with current support scope.
- Hides SMS/Cal.AI for form triggers.
- Always hides WhatsApp.
- Excludes Cal.AI for org and select-all.
Note: per prior maintainer comment, Cal.AI for form triggers will come in a follow-up PR.
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts (1)
150-169: The date-based limitation logic was pre-existingI can see this code segment with the hardcoded date was already present in the codebase before your changes. Your PR just reorganized it within the new conditional structure to support form-based workflows.
🧹 Nitpick comments (43)
packages/trpc/server/routers/viewer/workflows/getAllActiveWorkflows.schema.ts (1)
21-32: verifiedAt: replace z.coerce.date() with z.date() – since TRPC is configured with superjson (transformer: superjson in both server and client), Dates already round-trip correctly; usez.date().nullable().optional()instead of coercion.packages/trpc/server/routers/viewer/eventTypes/getActiveOnOptions.schema.ts (1)
3-6: Constrain teamId to integer > 0Tighten validation to avoid fractional/negative IDs.
export const ZGetActiveOnOptionsSchema = z.object({ - teamId: z.number().optional(), + teamId: z.number().int().positive().optional(), isOrg: z.boolean().default(false), });packages/trpc/server/routers/viewer/eventTypes/getActiveOnOptions.handler.ts (3)
287-296: Avoid throwing on malformed metadataparse throws on bad metadata and can break option building. Use safeParse and default-open behavior.
- ?.filter((evType) => { - const metadata = EventTypeMetaDataSchema.parse(evType.metadata); - return ( - !metadata?.managedEventConfig || - !!metadata?.managedEventConfig.unlockedFields?.workflows || - !!teamId - ); - }) + ?.filter((evType) => { + const r = EventTypeMetaDataSchema.safeParse(evType.metadata); + const metadata = r.success ? r.data : undefined; + return ( + !metadata?.managedEventConfig || + !!metadata?.managedEventConfig.unlockedFields?.workflows || + !!teamId + ); + })
176-185: Potential duplicate team optionsIf listOtherTeamHandler returns a team the user is already in, you can get duplicates. Deduplicate by id.
- return profileTeamsOptions.concat(otherTeamsOptions); + const seen = new Set<string>(); + return [...profileTeamsOptions, ...otherTeamsOptions].filter((o) => { + if (seen.has(o.value)) return false; + seen.add(o.value); + return true; + });
263-266: Prefer NOT_FOUND over INTERNAL_SERVER_ERRORMissing profile is a data/lookup miss, not a server fault.
- if (!profile) { - throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" }); - } + if (!profile) { + throw new TRPCError({ code: "NOT_FOUND" }); + }packages/app-store/routing-forms/lib/handleResponse.test.ts (2)
84-86: Null timeFormat/locale in mock — verify fallback pathsGood to include these fields. Please confirm downstream email/template formatters gracefully handle nulls. Consider adding one test with non-null values (e.g., locale "en", timeFormat 12) to cover both paths.
217-217: Fix typo in test title"quueueFormResponse" → "queueFormResponse".
-it("should send formResponse with id=0 and quueueFormResponse isn't true", async () => { +it("should send formResponse with id=0 and queueFormResponse isn't true", async () => {packages/lib/server/repository/team.ts (1)
402-414: LGTM — minimal, selective query fits the use-caseNice targeted select of hideBranding and parent.hideBranding with findUnique. Consider adding an explicit return type to lock the shape for callers.
- async findTeamWithParentHideBranding({ teamId }: { teamId: number }) { + async findTeamWithParentHideBranding({ teamId }: { teamId: number }): Promise<{ + hideBranding: boolean; + parent: { hideBranding: boolean } | null; + } | null> {packages/features/ee/workflows/lib/actionHelperFunctions.ts (1)
145-150: Use single source of truth for form triggersLeverage FORM_TRIGGER_WORKFLOW_EVENTS to avoid duplicating trigger lists.
-import { WorkflowActions, WorkflowTemplates, WorkflowTriggerEvents } from "@calcom/prisma/enums"; +import { WorkflowActions, WorkflowTemplates, WorkflowTriggerEvents } from "@calcom/prisma/enums"; +import { FORM_TRIGGER_WORKFLOW_EVENTS } from "./constants"; @@ -export function isFormTrigger(trigger: WorkflowTriggerEvents) { - return ( - trigger === WorkflowTriggerEvents.FORM_SUBMITTED || - trigger === WorkflowTriggerEvents.FORM_SUBMITTED_NO_EVENT - ); -} +export function isFormTrigger(trigger: WorkflowTriggerEvents) { + return FORM_TRIGGER_WORKFLOW_EVENTS.includes(trigger); +}packages/prisma/schema.prisma (1)
1203-1204: Schema additions look correct; mind migration behavior for new enum values
- Bridge model WorkflowsOnRoutingForms with CASCADE FKs and unique (workflowId, routingFormId) is solid.
- Added Workflow.activeOnRoutingForms relation and enum values FORM_SUBMITTED / FORM_SUBMITTED_NO_EVENT align with app logic.
- Please ensure the migration adds enum values safely in Postgres (ALTER TYPE ... ADD VALUE … not in a transaction) and that generated client is updated.
Also applies to: 1351-1352, 1394-1408, 1446-1456
apps/web/public/static/locales/en/common.json (2)
1425-1425: Unify casing with adjacent action labelsNearby keys use sentence case (“send email to attendees”). Match that style for consistency.
- "email_attendee_action_form": "Send email to submitted email address", + "email_attendee_action_form": "send email to submitted email address",
3598-3601: Match trigger label casing with existing onesOther trigger labels (e.g.,
new_event_trigger,reschedule_event_trigger) use lowercase “when …”. Align for consistency.- "form_submitted_trigger": "When routing form is submitted", - "form_submitted_no_event_trigger": "When routing form is submitted and no booking is created", + "form_submitted_trigger": "when routing form is submitted", + "form_submitted_no_event_trigger": "when routing form is submitted and no booking is created",packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts (4)
18-33: Annotate return typeMake the intent explicit and help TS catch mismatches.
-export async function shouldTriggerFormSubmittedNoEvent(options: ValidationOptions) { +export async function shouldTriggerFormSubmittedNoEvent( + options: ValidationOptions +): Promise<boolean> {
10-13: Remove unused ValidationResult or return it
ValidationResultisn’t used. Drop it or switch the function to return{ skip, reason }.-export interface ValidationResult { - skip: boolean; - reason?: string; -}
86-104: Reuse getSubmitterEmail for consistency and robustnessLeverage the helper to cover both object and string cases; simplifies the scan and avoids type assumptions.
const recentResponses = await prisma.app_RoutingForms_FormResponse.findMany({ @@ }); // Check if there's a duplicate email in recent responses - return recentResponses.some((response) => { - if (!response.response || typeof response.response !== "object") return false; - - return Object.values(response.response as Record<string, { value: string; label: string }>).some( - (field) => { - return ( - typeof field === "object" && - field && - "value" in field && - typeof field.value === "string" && - field.value.toLowerCase() === submitterEmail.toLowerCase() - ); - } - ); - }); + return recentResponses.some((r) => { + const email = getSubmitterEmail((r as any).response); + return typeof email === "string" && email.toLowerCase() === submitterEmail.toLowerCase(); + });Also applies to: 106-121
90-92: Time window upper bound: rely on submittedAt to avoid over-broad windowsUsing
lt: new Date()widens the window when the task runs significantly after submission. Consider bounding by the providedsubmittedAt(ordate) for a strict ±60m window relative to the submission being evaluated.- lt: new Date(), + lt: date,Would you like me to adjust tests or add one to cover late execution cases (e.g., delayed cron) to lock this behavior?
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts (1)
78-89: Minor: compute email once and guard missing handlersAvoid repeated extraction and skip unknown action types.
- if (incompleteBookingActions) { - for (const incompleteBookingAction of incompleteBookingActions) { - const actionType = incompleteBookingAction.actionType; - - // Get action function - const bookingActionFunction = incompleteBookingActionFunctions[actionType]; - - const emailValue = getSubmitterEmail(responses); - if (emailValue) { - await bookingActionFunction(incompleteBookingAction, emailValue); - } - } - } + if (incompleteBookingActions) { + const emailValue = getSubmitterEmail(responses); + for (const incompleteBookingAction of incompleteBookingActions) { + const actionType = incompleteBookingAction.actionType; + const bookingActionFunction = incompleteBookingActionFunctions[actionType]; + if (!bookingActionFunction || !emailValue) continue; + await bookingActionFunction(incompleteBookingAction, emailValue); + } + }packages/trpc/server/routers/viewer/routing-forms/response.handler.ts (1)
59-59: Drop the default export; keep the named export only.Prefer named exports for better tree-shaking and refactors.
-export default responseHandler; +// export default removed; use named export abovepackages/lib/server/repository/user.ts (1)
1071-1087: Add an explicit return type and clarify org nullability.Tiny DX/typing nit: declare the return shape to stabilize consumers, and note that profiles[].organization can be null for personal profiles.
- async findUserWithHideBranding({ userId }: { userId: number }) { - return this.prismaClient.user.findUnique({ + async findUserWithHideBranding({ + userId, + }: { + userId: number; + }): Promise< + | { + hideBranding: boolean; + profiles: { organization: { hideBranding: boolean } | null }[]; + } + | null + > { + return await this.prismaClient.user.findUnique({ where: { id: userId }, select: { hideBranding: true, profiles: { select: { organization: { select: { hideBranding: true, }, }, }, }, }, }); }packages/features/eventtypes/components/tabs/workflows/EventWorkfowsTab.tsx (1)
121-126: Localize the "Untitled" fallback.Avoid hardcoded English; use t("untitled") or an appropriate key.
- {workflow.name - ? workflow.name - : `Untitled (${`${t(`${workflow.steps[0].action.toLowerCase()}_action`)}` + {workflow.name + ? workflow.name + : `${t("untitled")} (${`${t(`${workflow.steps[0].action.toLowerCase()}_action`)}` .charAt(0) .toUpperCase()}${`${t(`${workflow.steps[0].action.toLowerCase()}_action`)}`.slice(1)})`}packages/features/ee/workflows/lib/reminders/whatsappReminderManager.ts (1)
54-56: timeUnit typing: prefer a string-union over enum for lowercased unitsUsing a lowercased string cast against an enum-typed alias is brittle. Export a literal union (e.g., type TimeUnitLowerCase = "minute" | "hour" | "day") from smsReminderManager and use that here.
Also applies to: 72-76
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWorkflow.test.ts (1)
119-176: Optional: assert validation contractAlso assert shouldTriggerFormSubmittedNoEvent was called with formId, responseId, responses, and submittedAt to lock the contract.
- await triggerFormSubmittedNoEventWorkflow(payloadString); + await triggerFormSubmittedNoEventWorkflow(payloadString); + expect(mockShouldTriggerFormSubmittedNoEvent).toHaveBeenCalledWith( + expect.objectContaining({ + formId: payload.form.id, + responseId: payload.responseId, + responses: payload.responses, + submittedAt: payload.submittedAt, + }) + );packages/lib/server/service/workflows.test.ts (1)
126-156: Mock return type mismatch for tasker.createTasker.create returns Promise, but the mock returns an object. Return a string to match the interface.
- mockTasker.create.mockResolvedValue({ id: "task-123" }); + mockTasker.create.mockResolvedValue("task-123");packages/features/ee/workflows/components/AddActionDialog.tsx (1)
123-124: Redundant null-guardactionOptions is a required prop; this guard can be removed. If you keep it, prefer an explicit empty-state UI over returning null.
packages/features/ee/workflows/lib/reminders/smsReminderManager.ts (1)
29-34: Prefer a string-union type for lowercased timeUnitUsing an enum named like a type and casting lowercased strings to it is confusing. Replace with a literal union type (e.g., type TimeUnitLowerCase = "day" | "minute" | "year") and use consistently.
Also applies to: 75-90
packages/app-store/routing-forms/lib/formSubmissionUtils.test.ts (3)
58-67: Add top-level userId to mockForm to match WorkflowService contract.scheduleFormWorkflows relies on form.userId (see workflows.ts). The current mock omits it; add userId to avoid silent regressions.
Apply:
const mockForm = { id: "form-1", name: "Test Form", + userId: 1, fields: [ { id: "field-1", identifier: "email", label: "Email", type: "email" }, { id: "field-2", identifier: "name", label: "Name", type: "text" }, ], user: { id: 1, email: "test@example.com", timeFormat: 12, locale: "en" }, teamId: null, settings: { emailOwnerOnSubmission: true }, };
35-40: Harden the WorkflowService mock to resist API drift.Mock only the method you need while preserving any real exports to avoid future breakage if the class gains more static members.
Example:
-vi.mock("@calcom/lib/server/service/workflows", () => ({ - WorkflowService: { - scheduleFormWorkflows: vi.fn(() => Promise.resolve()), - }, -})); +vi.mock("@calcom/lib/server/service/workflows", async (importActual) => { + const actual = await importActual<typeof import("@calcom/lib/server/service/workflows")>(); + return { + ...actual, + WorkflowService: { + ...actual.WorkflowService, + scheduleFormWorkflows: vi.fn(() => Promise.resolve()), + }, + }; +});
186-193: Avoid coupling test expectations to field.identifier._onFormSubmission uses getFieldIdentifier; identifiers may be derived. Consider asserting identifier: expect.any(String) or adding a separate unit test for getFieldIdentifier derivation.
Also applies to: 238-244
packages/features/ee/workflows/lib/getOptions.ts (1)
48-60: Tighten typing for option value; avoid any.Keep value typed as WorkflowTemplates for safer consumers.
Apply:
function convertToTemplateOptions( t: TFunction, hasPaidPlan: boolean, templates: readonly WorkflowTemplates[] ) { - return templates.map((template) => { - return { - label: t(`${template.toLowerCase()}`), - value: template, - needsTeamsUpgrade: !hasPaidPlan, - } as { label: string; value: any; needsTeamsUpgrade: boolean }; - }); + return templates.map((template): { label: string; value: WorkflowTemplates; needsTeamsUpgrade: boolean } => ({ + label: t(`${template.toLowerCase()}`), + value: template, + needsTeamsUpgrade: !hasPaidPlan, + })); }packages/app-store/routing-forms/lib/formSubmissionUtils.ts (1)
118-126: Ensure upstream provides user.timeFormat/locale.The stricter type now requires these fields. Verify the loader that builds form.user selects timeFormat and locale; locale is later defaulted to "en" but timeFormat may be null—confirm consumers handle null.
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWorkflow.ts (1)
12-30: Tighten payload typing for responses when feasible.responses: z.any() is permissive; if possible, narrow to the FORM_SUBMITTED_WEBHOOK_RESPONSES shape (record of response/value) to catch payload drift early.
packages/trpc/server/routers/viewer/workflows/util.ts (2)
456-468: Good: consolidated signature to an options object.
Helps readability at call sites. Consider renaming teamId to orgId when isOrg=true to avoid ambiguity.
1013-1061: Form workflows fetch looks good; consider unifying select shape.
To avoid drift with workflowSelect used in getAllWorkflows, consider reusing that projection if accessible, or create a shared constant.packages/prisma/migrations/20250820131340_add_form_triggers_to_workflows/migration.sql (1)
9-10: Guard enum additions in migration scripts
Add idempotent checks around eachALTER TYPE … ADD VALUEto prevent failures on PostgreSQL ≤ 11 and rerun errors. (github.com)-ALTER TYPE "WorkflowTriggerEvents" ADD VALUE 'FORM_SUBMITTED'; -ALTER TYPE "WorkflowTriggerEvents" ADD VALUE 'FORM_SUBMITTED_NO_EVENT'; +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_enum e + JOIN pg_type t ON t.oid = e.enumtypid + WHERE t.typname = 'WorkflowTriggerEvents' AND e.enumlabel = 'FORM_SUBMITTED' + ) THEN + ALTER TYPE "WorkflowTriggerEvents" ADD VALUE 'FORM_SUBMITTED'; + END IF; +END$$; + +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_enum e + JOIN pg_type t ON t.oid = e.enumtypid + WHERE t.typname = 'WorkflowTriggerEvents' AND e.enumlabel = 'FORM_SUBMITTED_NO_EVENT' + ) THEN + ALTER TYPE "WorkflowTriggerEvents" ADD VALUE 'FORM_SUBMITTED_NO_EVENT'; + END IF; +END$$;For strict PG ≤ 11 support, split these into separate migrations so each addition runs in its own file. (github.com)
packages/trpc/server/routers/viewer/workflows/update.handler.ts (2)
349-360: Avoid no-op scheduling for form triggersscheduleWorkflowNotifications early-returns for non BEFORE/AFTER triggers; skip the call entirely to reduce DB load on form updates.
Apply this diff:
- await scheduleWorkflowNotifications({ - activeOn: newActiveOn, - isOrg, - workflowSteps: userWorkflow.steps, // use old steps here, edited and deleted steps are handled below - time, - timeUnit, - trigger, - userId: user.id, - teamId: userWorkflow.teamId, - alreadyScheduledActiveOnIds: activeOnEventTypeIds.filter((activeOn) => !newActiveOn.includes(activeOn)), // alreadyScheduledActiveOnIds - }); + if (!isFormTrigger(trigger)) { + await scheduleWorkflowNotifications({ + activeOn: newActiveOn, + isOrg, + workflowSteps: userWorkflow.steps, // use old steps here, edited and deleted steps are handled below + time, + timeUnit, + trigger, + userId: user.id, + teamId: userWorkflow.teamId, + alreadyScheduledActiveOnIds: activeOnEventTypeIds.filter((a) => !newActiveOn.includes(a)), + }); + }
796-821: Skip AI phone fields maintenance for form triggersThese upserts/removals only matter for event-type workflows; guard with !isFormTrigger(trigger) to avoid unnecessary queries.
Apply this diff:
- // Remove or add attendeePhoneNumber field for AI phone call actions - const aiPhoneCallStepsNeeded = - activeOnEventTypeIds.length && steps.some((s) => s.action === WorkflowActions.CAL_AI_PHONE_CALL); + // Remove or add attendeePhoneNumber field for AI phone call actions (event triggers only) + if (!isFormTrigger(trigger)) { + const aiPhoneCallStepsNeeded = + activeOnEventTypeIds.length && steps.some((s) => s.action === WorkflowActions.CAL_AI_PHONE_CALL); @@ - await removeAIAgentCallPhoneNumberFieldForEventTypes({ - activeOnToRemove: removedActiveOnIds, - workflowId: id, - isOrg, - activeOn: activeOnEventTypeIds, - }); + await removeAIAgentCallPhoneNumberFieldForEventTypes({ + activeOnToRemove: removedActiveOnIds, + workflowId: id, + isOrg, + activeOn: activeOnEventTypeIds, + }); @@ - if (!aiPhoneCallStepsNeeded) { + if (!aiPhoneCallStepsNeeded) { await removeAIAgentCallPhoneNumberFieldForEventTypes({ activeOnToRemove: activeOnWithChildren, workflowId: id, isOrg, }); - } else { + } else { await upsertAIAgentCallPhoneNumberFieldForEventTypes({ activeOn: activeOnWithChildren, workflowId: id, isAIAgentCallPhoneNumberRequired: steps.some((s) => s.action === WorkflowActions.CAL_AI_PHONE_CALL), isOrg, }); - } + } + }packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
1067-1105: Avoid double writes and tighten typing in template select.
You call field.onChange(value) and also setValue on the same field; keep one. Also declare the option type to use WorkflowTemplates for value.-const value = val.value as WorkflowTemplates; +const value = val.value as WorkflowTemplates; ... - field.onChange(value); - form.setValue(`steps.${step.stepNumber - 1}.template`, value); + field.onChange(value);And for isOptionDisabled:
- isOptionDisabled={(option: { - label: string; - value: string; - needsTeamsUpgrade: boolean; - }) => option.needsTeamsUpgrade} + isOptionDisabled={(option: { + label: string; + value: WorkflowTemplates; + needsTeamsUpgrade: boolean; + }) => option.needsTeamsUpgrade}Also applies to: 1119-1121
1223-1232: Hide booking-question variables for form triggers — OK for now.
Once form-response variables ship, re-enable with correct variable set.I can wire the form variable list into this section when back-end exposure lands. Want me to prep that patch behind a feature flag?
packages/features/ee/workflows/lib/getAllWorkflows.ts (1)
51-67: Consider simplifying the trigger type filtering logicThe current implementation uses two separate if statements to build the where clause. This could be simplified using a ternary operator or a more concise approach.
- let triggerTypeWhereClause: Prisma.WorkflowWhereInput = {}; - - if (triggerType === "routingForm") { - triggerTypeWhereClause = { - trigger: { - in: FORM_TRIGGER_WORKFLOW_EVENTS, - }, - }; - } - - if (triggerType === "eventType") { - triggerTypeWhereClause = { - trigger: { - not: { - in: FORM_TRIGGER_WORKFLOW_EVENTS, - }, - }, - }; - } + const triggerTypeWhereClause: Prisma.WorkflowWhereInput = + triggerType === "routingForm" + ? { trigger: { in: FORM_TRIGGER_WORKFLOW_EVENTS } } + : { trigger: { not: { in: FORM_TRIGGER_WORKFLOW_EVENTS } } };packages/features/ee/workflows/lib/reminders/reminderScheduler.ts (2)
103-106: Consider logging the skipped action for debuggingWhen SMS actions are skipped for form triggers, it would be helpful to log this for debugging purposes, especially since the PR mentions SMS support will be added in a follow-up.
if (isSMSAction(step.action)) { if (!evt) { - // SMS action not not yet supported for form triggers + // SMS action not yet supported for form triggers + log.debug(`SMS action ${step.action} not yet supported for form triggers - skipping`); return; }
194-197: Fix typo in commentThere's a typo in the comment: "not not yet" should be "not yet".
- // Whatsapp action not not yet supported for form triggers + // WhatsApp action not yet supported for form triggerspackages/features/ee/workflows/lib/reminders/emailReminderManager.ts (2)
367-368: Add variable support for form-based emailsThe TODO comment indicates that variables are not yet implemented for form-based emails. This could limit the usefulness of form-triggered email workflows.
Would you like me to help implement the variable support for form-based emails? This could include extracting form field values and making them available as template variables similar to the event-based path.
394-394: Document the null scheduledDate behaviorThe comment mentions that
FORM_SUBMITTED_NO_EVENTis scheduled via tasker, but it would be helpful to clarify that passingnullhere means the email is sent immediately.- scheduledDate: null, //FORM_SUBMITTED_NO_EVENT is scheduled via tasker; this function runs only when it should trigger + scheduledDate: null, // null = send immediately. FORM_SUBMITTED_NO_EVENT workflows are scheduled via tasker, and this function is called when they should trigger
| updateDto.trigger instanceof OnBeforeEventTriggerDto || | ||
| updateDto.trigger instanceof OnAfterEventTriggerDto | ||
| updateDto.trigger instanceof OnAfterEventTriggerDto || | ||
| updateDto.trigger instanceof OnFormSubmittedNoEventTriggerDto | ||
| ? updateDto?.trigger?.offset?.unit ?? currentData.timeUnit ?? null | ||
| : undefined; | ||
|
|
There was a problem hiding this comment.
Don’t null-out time/timeUnit when trigger isn’t provided
If updateDto.trigger is undefined, current code sets time/timeUnit to null, unintentionally clearing persisted offsets and triggering reschedules.
Apply this diff to preserve existing values unless the trigger changes to a non-offset variant:
- const timeUnitForZod =
- updateDto.trigger instanceof OnBeforeEventTriggerDto ||
- updateDto.trigger instanceof OnAfterEventTriggerDto ||
- updateDto.trigger instanceof OnFormSubmittedNoEventTriggerDto
- ? updateDto?.trigger?.offset?.unit ?? currentData.timeUnit ?? null
- : undefined;
+ const isDtoOffsetBased =
+ updateDto.trigger instanceof OnBeforeEventTriggerDto ||
+ updateDto.trigger instanceof OnAfterEventTriggerDto ||
+ updateDto.trigger instanceof OnFormSubmittedNoEventTriggerDto;
+
+ const timeUnitForZod =
+ updateDto.trigger === undefined
+ ? currentData.timeUnit ?? null
+ : isDtoOffsetBased
+ ? updateDto.trigger.offset?.unit ?? currentData.timeUnit ?? null
+ : null;
@@
- time:
- updateDto.trigger instanceof OnBeforeEventTriggerDto ||
- updateDto.trigger instanceof OnAfterEventTriggerDto ||
- updateDto.trigger instanceof OnFormSubmittedNoEventTriggerDto
- ? updateDto?.trigger?.offset?.value ?? currentData?.time ?? null
- : null,
+ time:
+ updateDto.trigger === undefined
+ ? currentData.time ?? null
+ : isDtoOffsetBased
+ ? updateDto.trigger.offset?.value ?? currentData.time ?? null
+ : null,Also applies to: 156-160
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/workflows/services/workflows.input.service.ts around
lines 139-144 (and similarly at 156-160), the ternary unconditionally evaluates
to null when updateDto.trigger is undefined, which clears persisted offset
values; change the logic so undefined trigger leaves the existing value
untouched: first check whether updateDto.trigger is strictly undefined and
return undefined (no change); otherwise, if the provided trigger is one of the
offset-bearing DTOs (OnBeforeEventTriggerDto, OnAfterEventTriggerDto,
OnFormSubmittedNoEventTriggerDto) compute the unit/value using the updateDto
trigger or fallback to currentData, and if the trigger is a non-offset variant
explicitly set null to clear offsets when the trigger actually changed to a
non-offset type.
| await Promise.all(promises); | ||
|
|
||
| const workflows = await getAllWorkflowsFromRoutingForm(form); | ||
|
|
||
| await WorkflowService.scheduleFormWorkflows({ | ||
| workflows, | ||
| responses: fieldResponsesByIdentifier, | ||
| responseId, | ||
| form: { | ||
| ...form, | ||
| fields: form.fields.map((field) => ({ | ||
| type: field.type, | ||
| identifier: getFieldIdentifier(field), | ||
| })), | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guarantee field identifiers for phone field or warn/fallback.
smsReminderNumber extraction later depends on a phone field having a stable identifier. If getFieldIdentifier ever returns undefined, SMS detection silently fails.
Apply a defensive mapping and warning:
- await WorkflowService.scheduleFormWorkflows({
+ // Build slim field list with identifiers; warn if phone lacks identifier
+ const slimFields = form.fields.map((field) => ({
+ type: field.type,
+ identifier: getFieldIdentifier(field),
+ }));
+ if (slimFields.some((f) => f.type === "phone" && !f.identifier)) {
+ moduleLogger.warn("Routing form phone field missing identifier; SMS number extraction may fail");
+ }
+ await WorkflowService.scheduleFormWorkflows({
workflows,
responses: fieldResponsesByIdentifier,
responseId,
form: {
...form,
- fields: form.fields.map((field) => ({
- type: field.type,
- identifier: getFieldIdentifier(field),
- })),
+ fields: slimFields,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await Promise.all(promises); | |
| const workflows = await getAllWorkflowsFromRoutingForm(form); | |
| await WorkflowService.scheduleFormWorkflows({ | |
| workflows, | |
| responses: fieldResponsesByIdentifier, | |
| responseId, | |
| form: { | |
| ...form, | |
| fields: form.fields.map((field) => ({ | |
| type: field.type, | |
| identifier: getFieldIdentifier(field), | |
| })), | |
| }, | |
| }); | |
| await Promise.all(promises); | |
| const workflows = await getAllWorkflowsFromRoutingForm(form); | |
| // Build slim field list with identifiers; warn if phone lacks identifier | |
| const slimFields = form.fields.map((field) => ({ | |
| type: field.type, | |
| identifier: getFieldIdentifier(field), | |
| })); | |
| if (slimFields.some((f) => f.type === "phone" && !f.identifier)) { | |
| moduleLogger.warn( | |
| "Routing form phone field missing identifier; SMS number extraction may fail" | |
| ); | |
| } | |
| await WorkflowService.scheduleFormWorkflows({ | |
| workflows, | |
| responses: fieldResponsesByIdentifier, | |
| responseId, | |
| form: { | |
| ...form, | |
| fields: slimFields, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/app-store/routing-forms/lib/formSubmissionUtils.ts around lines
221–237, mapping the form.fields to {type, identifier} can produce undefined
identifiers (getFieldIdentifier may return undefined) which breaks later SMS
detection; update the mapping to guarantee a non-undefined identifier for phone
fields by supplying a deterministic fallback (e.g. `${field.type}_${index}` or
`${getFieldIdentifier(field) ?? 'phone_'+i}`), and emit a warning (console.warn
or the module logger) whenever you use the fallback so failures are visible;
ensure the same fallback key is used when building fieldResponsesByIdentifier so
lookups remain consistent.
| const templateOptions = getWorkflowTemplateOptions(t, step?.action, hasActiveTeamPlan, trigger); | ||
|
|
||
| const hasAiAction = hasCalAIAction(form.getValues("steps")); | ||
| const hasSMSAction = form.getValues("steps").some((step) => isSMSAction(step.action)); | ||
|
|
||
| const filteredTriggerOptions = triggerOptions.filter( | ||
| (option) => !(isFormTrigger(option.value) && (hasAiAction || hasSMSAction)) | ||
| ); | ||
|
|
There was a problem hiding this comment.
Also guard against existing WhatsApp steps before showing form triggers.
Currently you hide form triggers only if SMS or Cal.AI steps exist. Workflows with WhatsApp steps are also unsupported for form triggers per this PR; allow-listing them can leave an invalid state.
Apply:
-const hasAiAction = hasCalAIAction(form.getValues("steps"));
-const hasSMSAction = form.getValues("steps").some((step) => isSMSAction(step.action));
+const hasAiAction = hasCalAIAction(form.getValues("steps"));
+const hasSMSAction = form.getValues("steps").some((step) => isSMSAction(step.action));
+const hasWhatsappAction = form.getValues("steps").some((step) => isWhatsappAction(step.action));
-const filteredTriggerOptions = triggerOptions.filter(
- (option) => !(isFormTrigger(option.value) && (hasAiAction || hasSMSAction))
-);
+const filteredTriggerOptions = triggerOptions.filter(
+ (option) => !(isFormTrigger(option.value) && (hasAiAction || hasSMSAction || hasWhatsappAction))
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const templateOptions = getWorkflowTemplateOptions(t, step?.action, hasActiveTeamPlan, trigger); | |
| const hasAiAction = hasCalAIAction(form.getValues("steps")); | |
| const hasSMSAction = form.getValues("steps").some((step) => isSMSAction(step.action)); | |
| const filteredTriggerOptions = triggerOptions.filter( | |
| (option) => !(isFormTrigger(option.value) && (hasAiAction || hasSMSAction)) | |
| ); | |
| const templateOptions = getWorkflowTemplateOptions(t, step?.action, hasActiveTeamPlan, trigger); | |
| const hasAiAction = hasCalAIAction(form.getValues("steps")); | |
| const hasSMSAction = form.getValues("steps").some((step) => isSMSAction(step.action)); | |
| const hasWhatsappAction = form.getValues("steps").some((step) => isWhatsappAction(step.action)); | |
| const filteredTriggerOptions = triggerOptions.filter( | |
| (option) => | |
| !( | |
| isFormTrigger(option.value) && | |
| (hasAiAction || hasSMSAction || hasWhatsappAction) | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In packages/features/ee/workflows/components/WorkflowStepContainer.tsx around
lines 257 to 265, the filter for form triggers only checks for Cal.AI and SMS
actions, but workflows with WhatsApp steps must also block form triggers; add a
check for WhatsApp steps (e.g. const hasWhatsAppAction =
form.getValues("steps").some(step => isWhatsAppAction(step.action))) and include
it in the predicate so filteredTriggerOptions excludes form triggers when
hasAiAction || hasSMSAction || hasWhatsAppAction.
| export const getAllWorkflows = async ({ | ||
| entityWorkflows, | ||
| userId, | ||
| teamId, | ||
| orgId, | ||
| workflowsLockedForUser, | ||
| triggerType, | ||
| }: { | ||
| entityWorkflows: Workflow[]; | ||
| userId?: number | null; | ||
| teamId?: number | null; | ||
| orgId?: number | null; | ||
| workflowsLockedForUser?: boolean; | ||
| triggerType: "eventType" | "routingForm"; | ||
| }) => { |
There was a problem hiding this comment.
Breaking change: Consider maintaining backward compatibility
The function signature change from positional parameters to an options object is a breaking change that will affect all existing callers. While the new signature is cleaner, this could break existing code.
Consider adding a backward-compatible overload or migration path:
+// Backward compatible overload (deprecated)
+export async function getAllWorkflows(
+ eventTypeWorkflows: Workflow[],
+ userId?: number | null,
+ teamId?: number | null,
+ orgId?: number | null,
+ workflowsLockedForUser?: boolean
+): Promise<Workflow[]>;
+
+// New signature
+export async function getAllWorkflows(options: {
+ entityWorkflows: Workflow[];
+ userId?: number | null;
+ teamId?: number | null;
+ orgId?: number | null;
+ workflowsLockedForUser?: boolean;
+ triggerType: "eventType" | "routingForm";
+}): Promise<Workflow[]>;
+
export const getAllWorkflows = async ({
entityWorkflows,
userId,
teamId,
orgId,
workflowsLockedForUser,
triggerType,
}: {
entityWorkflows: Workflow[];
userId?: number | null;
teamId?: number | null;
orgId?: number | null;
workflowsLockedForUser?: boolean;
triggerType: "eventType" | "routingForm";
}) => {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/features/ee/workflows/lib/getAllWorkflows.ts around lines 32 to 46,
the function was changed from positional parameters to a single options object
which is a breaking API change for existing callers; restore backward
compatibility by adding a backward-compatible wrapper or overload: implement an
overloaded function signature that accepts the old positional args and maps them
to the new options object (or detect argument types at runtime and coerce
positional arguments into the options shape), keep the core implementation using
the options object, and mark the positional variant deprecated with a comment so
callers can migrate without immediate breakage.
| const scheduleEmailReminderForForm = async ( | ||
| args: scheduleEmailReminderArgs & { | ||
| formData: FormSubmissionData; | ||
| } | ||
| ) => { | ||
| const { | ||
| formData, | ||
| triggerEvent, | ||
| sender, | ||
| workflowStepId, | ||
| sendTo, | ||
| emailSubject = "", | ||
| emailBody = "", | ||
| hideBranding, | ||
| } = args; | ||
|
|
||
| await sendOrScheduleWorkflowEmails({ | ||
| ...mailData, | ||
| to: sendTo, | ||
| sendAt: scheduledDate?.toDate(), | ||
| referenceUid: reminderUid ?? undefined, | ||
| const emailContent = { | ||
| emailSubject, | ||
| emailBody: `<body style="white-space: pre-wrap;">${emailBody}</body>`, | ||
| }; | ||
|
|
||
| if (emailBody) { | ||
| const timeFormat = getTimeFormatStringFromUserTimeFormat(formData.user.timeFormat); | ||
| //todo: add variables | ||
| const emailSubjectTemplate = customTemplate(emailSubject, {}, formData.user.locale, timeFormat); | ||
| emailContent.emailSubject = emailSubjectTemplate.text; | ||
| emailContent.emailBody = customTemplate( | ||
| emailBody, | ||
| {}, | ||
| formData.user.locale, | ||
| timeFormat, | ||
| hideBranding | ||
| ).html; | ||
| } | ||
|
|
||
| // Allows debugging generated email content without waiting for sendgrid to send emails | ||
| log.debug(`Sending Email for trigger ${triggerEvent}`, JSON.stringify(emailContent)); | ||
|
|
||
| const mailData = { | ||
| subject: emailContent.emailSubject, | ||
| html: emailContent.emailBody, | ||
| sender, | ||
| }; | ||
|
|
||
| await sendOrScheduleWorkflowEmailWithReminder({ | ||
| mailData, | ||
| sendTo, | ||
| triggerEvent, | ||
| workflowStepId, | ||
| scheduledDate: null, //FORM_SUBMITTED_NO_EVENT is scheduled via tasker; this function runs only when it should trigger | ||
| }); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding error handling for missing email
The form-based email scheduling doesn't validate that sendTo contains valid email addresses. If getSubmitterEmail returns undefined in the caller, this could lead to sending emails to empty recipients.
🏁 Script executed:
#!/bin/bash
# Check if there's validation for sendTo in the calling code
rg -B 5 -A 5 'scheduleEmailReminderForForm|scheduleEmailReminder.*formData' --type ts
# Check if sendOrScheduleWorkflowEmails validates the to field
ast-grep --pattern 'export async function sendOrScheduleWorkflowEmails($_) {
$$$
}'Length of output: 4340
🏁 Script executed:
#!/bin/bash
# Locate the implementation of sendOrScheduleWorkflowEmailWithReminder to inspect validation of the recipients field
rg -C3 'sendOrScheduleWorkflowEmailWithReminder' -g '*.ts'Length of output: 1940
🏁 Script executed:
#!/bin/bash
# Retrieve the scheduleEmailReminderArgs type to inspect sendTo definition
rg -nR "interface scheduleEmailReminderArgs" -n --type ts
# Show the implementation of sendOrScheduleWorkflowEmailWithReminder to check sendTo validation
rg -nP 'const sendOrScheduleWorkflowEmailWithReminder\s*=\s*async' -n -C50 packages/features/ee/workflows/lib/reminders/emailReminderManager.tsLength of output: 3153
Validate sendTo before scheduling emails
In packages/features/ee/workflows/lib/reminders/emailReminderManager.ts (in scheduleEmailReminderForForm and scheduleEmailReminderForEvt), add a pre-flight check to ensure sendTo is a non-empty array of valid email strings. If sendTo is empty or contains invalid entries (e.g. undefined), throw or log an error and abort scheduling to prevent silent failures.
| if (formData) { | ||
| const submitterEmail = getSubmitterEmail(formData.responses); | ||
| if (submitterEmail) { | ||
| sendTo = [submitterEmail]; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify email extraction logic for form submissions
The code relies on getSubmitterEmail to extract the email from form responses. Ensure this function handles various email formats and validates the extracted email properly.
🏁 Script executed:
#!/bin/bash
# Check the implementation of getSubmitterEmail
ast-grep --pattern 'export function getSubmitterEmail($_) {
$$$
}'
# Check for email validation in the function
rg -A 10 'getSubmitterEmail' packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.tsLength of output: 2023
Enforce robust email validation in getSubmitterEmail
Current logic only checks value.includes("@"), which can accept invalid strings. Replace this with a full email‐format check (e.g. a well‐tested regex or a library such as validator.js’s isEmail) to ensure only valid addresses are returned.
🤖 Prompt for AI Agents
In packages/features/ee/workflows/lib/reminders/reminderScheduler.ts around
lines 171 to 176, the current getSubmitterEmail logic only uses
value.includes("@") which allows invalid addresses; replace that check with a
proper email validation (preferably use validator.js isEmail or a well-tested
regex), trim and normalize the candidate string before validating, and only
return the address when the validator confirms it is a valid email; update
imports if using validator.js and add unit tests or edge-case checks to ensure
invalid inputs are rejected.
| const watchedTrigger = useWatch({ | ||
| control: form.control, | ||
| name: "trigger", | ||
| }); | ||
|
|
There was a problem hiding this comment.
Reset activeOn when switching between form and non-form triggers.
If a user switches triggers, stale selections (event types vs routing forms) can be submitted under the wrong array (ids vs formIds), causing server errors.
Apply:
+ useEffect(() => {
+ // Clear selections on trigger mode change to prevent id-type mismatch
+ if (watchedTrigger !== undefined) {
+ setSelectedOptions([]);
+ form.setValue("activeOn", []);
+ form.setValue("selectAll", false);
+ }
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, [watchedTrigger]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const watchedTrigger = useWatch({ | |
| control: form.control, | |
| name: "trigger", | |
| }); | |
| const watchedTrigger = useWatch({ | |
| control: form.control, | |
| name: "trigger", | |
| }); | |
| useEffect(() => { | |
| // Clear selections on trigger mode change to prevent id-type mismatch | |
| if (watchedTrigger !== undefined) { | |
| setSelectedOptions([]); | |
| form.setValue("activeOn", []); | |
| form.setValue("selectAll", false); | |
| } | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [watchedTrigger]); |
🤖 Prompt for AI Agents
In packages/features/ee/workflows/pages/workflow.tsx around lines 74–78, the
watchedTrigger value must reset the activeOn selection when switching between
form-based and non-form triggers to avoid submitting stale ids into the wrong
array; add a useEffect that tracks watchedTrigger changes (compare previous and
current trigger type or presence of formId vs id) and when the trigger type
flips, call form.setValue('activeOn', []) (or the appropriate empty structure)
to clear prior selections so event types and routing form selections can't be
submitted into the wrong field.
packages/trpc/server/routers/viewer/eventTypes/getActiveOnOptions.handler.ts
Show resolved
Hide resolved
| return routingForms.map((form) => ({ | ||
| value: form.id, | ||
| label: form.name, | ||
| })); |
There was a problem hiding this comment.
Type mismatch: Option.value must be string
routingFormOptions returns value as number; Option.value is string. Convert to String and add a safe label fallback.
- return routingForms.map((form) => ({
- value: form.id,
- label: form.name,
- }));
+ return routingForms.map((form) => ({
+ value: String(form.id),
+ label: form.name ?? "Untitled form",
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return routingForms.map((form) => ({ | |
| value: form.id, | |
| label: form.name, | |
| })); | |
| return routingForms.map((form) => ({ | |
| value: String(form.id), | |
| label: form.name ?? "Untitled form", | |
| })); |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/eventTypes/getActiveOnOptions.handler.ts
around lines 239-242, the returned options set value as a number but the
Option.type expects a string and label can be undefined; convert form.id to a
string (e.g., String(form.id)) for value and provide a safe fallback for label
(e.g., form.name ?? 'Unnamed') so every option has a string value and a
non-empty label.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/server/routers/viewer/workflows/util.ts (1)
456-513: Refactor Prisma calls, guard missing event types, handle child event types, and clarify parameter naming
In
isAuthorizedToAddActiveOnIds(util.ts lines 456–513), replace theinclude-basedfindUniquewith aselectofselect: { id: true, teamId: true, userId: true, users: { select: { id: true } }, parent: { select: { id: true, teamId: true } }, }then add
if (!newEventType) return false; const effectiveTeamId = newEventType.teamId ?? newEventType.parent?.teamId ?? null; if (teamId && teamId !== effectiveTeamId) return false;Rename
newEventTypeIds(or add a JSDoc) to reflect that whenisOrgistrueit contains team IDs (e.g.newActiveOnIds) to eliminate confusion.
🧹 Nitpick comments (2)
packages/trpc/server/routers/viewer/workflows/util.ts (2)
456-513: Optional: batch DB reads for scalabilityLooping with await per ID is O(n) round-trips. You can fetch all referenced event types in a single findMany and validate in-memory; same for routing forms. Keep readability if n is small, but flagging for large orgs.
987-1033: Limit selected step fields to what schedulers consumesteps: true may fetch unnecessary data. Narrow the selection to reduce payload and memory.
- select: { + select: { id: true, name: true, trigger: true, time: true, timeUnit: true, userId: true, teamId: true, - steps: true, + steps: { + select: { + id: true, + action: true, + sender: true, + sendTo: true, + reminderBody: true, + emailSubject: true, + template: true, + verifiedAt: true, + }, + }, team: { select: { id: true, name: true, slug: true, }, }, },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/workflows/util.ts(4 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:
packages/trpc/server/routers/viewer/workflows/util.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:
packages/trpc/server/routers/viewer/workflows/util.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:
packages/trpc/server/routers/viewer/workflows/util.ts
🧠 Learnings (3)
📚 Learning: 2025-08-26T20:23:28.396Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Applied to files:
packages/trpc/server/routers/viewer/workflows/util.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/trpc/server/routers/viewer/workflows/util.ts
📚 Learning: 2025-08-17T22:00:16.329Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts:117-126
Timestamp: 2025-08-17T22:00:16.329Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts, the enabled input parameter in the update endpoint is intentionally not forwarded to aiService.updateAgentConfiguration() as the enabled/disabled agent functionality is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/trpc/server/routers/viewer/workflows/util.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/workflows/util.ts (2)
packages/features/ee/workflows/lib/getAllWorkflows.ts (1)
getAllWorkflows(32-160)packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
packages/trpc/server/routers/viewer/workflows/util.ts (2)
612-613: Guard looks correct; prevents scheduling on non-booking triggersThis early return avoids scheduling for form-triggered workflows. Matches intent.
975-985: Good use of trigger scoping in getAllWorkflowsDelegating with triggerType: "eventType" ensures correct filtering downstream.
| // Check authorization for routing form IDs | ||
| for (const id of newRoutingFormIds) { | ||
| // For routing forms, check if user has access to the form | ||
| const routingForm = await prisma.app_RoutingForms_Form.findUnique({ | ||
| where: { | ||
| id: String(id), | ||
| }, | ||
| select: { | ||
| userId: true, | ||
| teamId: true, | ||
| }, | ||
| }); | ||
|
|
||
| if (!routingForm) return false; | ||
|
|
||
| if (teamId && teamId !== routingForm.teamId) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!teamId && userId && routingForm.userId !== userId) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Support org-scope authorization for routing forms (child-team forms) and avoid String() cast
- When isOrg is true, allow forms whose team’s parent is the org team (mirrors the team check above).
- Using String(id) is unnecessary; id is already a string.
- const routingForm = await prisma.app_RoutingForms_Form.findUnique({
- where:
- { id: String(id) },
- select: { userId: true, teamId: true },
- });
+ const routingForm = await prisma.app_RoutingForms_Form.findUnique({
+ where: { id },
+ select: { userId: true, teamId: true },
+ });
if (!routingForm) return false;
- if (teamId && teamId !== routingForm.teamId) {
- return false;
- }
+ if (isOrg && teamId) {
+ const childTeam = routingForm.teamId
+ ? await prisma.team.findUnique({
+ where: { id: routingForm.teamId },
+ select: { parent: { select: { id: true } } },
+ })
+ : null;
+ if (!childTeam || childTeam.parent?.id !== teamId) return false;
+ continue;
+ }
+ if (teamId && teamId !== routingForm.teamId) return false;
if (!teamId && userId && routingForm.userId !== userId) {
return false;
}Is attaching routing forms at org-level expected in this PR? If not, keep the simple teamId match and drop the org block above. If yes, the above change is required.
🏁 Script executed:
#!/bin/bash
# Locate the util.ts file in the workflows util directory
UTIL=$(fd --type f util.ts | grep "packages/trpc/server/routers/viewer/workflows/util.ts")
echo "Util file: $UTIL"
# Dump the context around lines 450–600
sed -n '450,600p' "$UTIL"
# Search for isOrg usage in that file
grep -RIn "isOrg" "$UTIL"
# Find the async function signature that contains this loop
grep -RIn "async " -n "$UTIL" | head -n 10
# Locate the Prisma schema file
SCHEMA=$(fd --type f schema.prisma)
echo "Schema file: $SCHEMA"
# Print the Team model to confirm parent relation
grep -R "model Team" -n "$SCHEMA" -A10
# Print the app_RoutingForms_Form model to confirm fields
grep -R "model app_RoutingForms_Form" -n "$SCHEMA" -A10Length of output: 5335
🏁 Script executed:
#!/bin/bash
# List all .prisma schema files in the repo
echo "Listing .prisma files:"
find . -type f -name "*.prisma"
# Inspect each schema file for Team and app_RoutingForms_Form definitions
PRISMA_FILES=$(find . -type f -name "*.prisma")
for f in $PRISMA_FILES; do
echo -e "\n---- Schema File: $f ----"
# Print Team model
grep -RIn "model Team" -n "$f" -A10
# Print app_RoutingForms_Form model
grep -RIn "model app_RoutingForms_Form" -n "$f" -A10
doneLength of output: 1277
🏁 Script executed:
#!/bin/bash
# Extract the Team model including its parent relation
sed -n '/model Team {/,/}/p' packages/prisma/schema.prisma
# Extract the app_RoutingForms_Form model and its fields
sed -n '/model app_RoutingForms_Form {/,/}/p' packages/prisma/schema.prismaLength of output: 4309
🏁 Script executed:
#!/bin/bash
# Extract the App_RoutingForms_Form model definition
sed -n '/model App_RoutingForms_Form {/,/}/p' packages/prisma/schema.prismaLength of output: 1768
Implement org-scope authorization for routing forms and remove unnecessary String() cast
When isOrg is true, allow routing forms whose team’s parent matches teamId (mirroring the event‐type logic), and drop the redundant String(id) cast.
- const routingForm = await prisma.app_RoutingForms_Form.findUnique({
- where: { id: String(id) },
- select: { userId: true, teamId: true },
- });
+ const routingForm = await prisma.app_RoutingForms_Form.findUnique({
+ where: { id },
+ select: { userId: true, teamId: true },
+ });
if (!routingForm) return false;
- if (teamId && teamId !== routingForm.teamId) {
- return false;
- }
+ if (isOrg && teamId) {
+ const childTeam = routingForm.teamId
+ ? await prisma.team.findUnique({
+ where: { id: routingForm.teamId },
+ select: { parent: { select: { id: true } } },
+ })
+ : null;
+ if (!childTeam || childTeam.parent?.id !== teamId) return false;
+ continue;
+ }
+ if (teamId && teamId !== routingForm.teamId) return false;
if (!teamId && userId && routingForm.userId !== userId) {
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check authorization for routing form IDs | |
| for (const id of newRoutingFormIds) { | |
| // For routing forms, check if user has access to the form | |
| const routingForm = await prisma.app_RoutingForms_Form.findUnique({ | |
| where: { | |
| id: String(id), | |
| }, | |
| select: { | |
| userId: true, | |
| teamId: true, | |
| }, | |
| }); | |
| if (!routingForm) return false; | |
| if (teamId && teamId !== routingForm.teamId) { | |
| return false; | |
| } | |
| if (!teamId && userId && routingForm.userId !== userId) { | |
| return false; | |
| } | |
| } | |
| // Check authorization for routing form IDs | |
| for (const id of newRoutingFormIds) { | |
| // For routing forms, check if user has access to the form | |
| const routingForm = await prisma.app_RoutingForms_Form.findUnique({ | |
| where: { id }, | |
| select: { userId: true, teamId: true }, | |
| }); | |
| if (!routingForm) return false; | |
| if (isOrg && teamId) { | |
| const childTeam = routingForm.teamId | |
| ? await prisma.team.findUnique({ | |
| where: { id: routingForm.teamId }, | |
| select: { parent: { select: { id: true } } }, | |
| }) | |
| : null; | |
| if (!childTeam || childTeam.parent?.id !== teamId) return false; | |
| continue; | |
| } | |
| if (teamId && teamId !== routingForm.teamId) { | |
| return false; | |
| } | |
| if (!teamId && userId && routingForm.userId !== userId) { | |
| return false; | |
| } | |
| } |
There was a problem hiding this comment.
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 (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
868-876: Localize “Verification code” placeholders.Per TSX guidelines, avoid hardcoded UI strings; wrap in t().
- placeholder="Verification code" + placeholder={t("verification_code")}Also applies to: 1024-1032
1348-1389: Fix typo in class names: “test-sm” → “text-sm”.Visual styles won’t apply with the misspelled utility class.
- <p className="test-sm font-medium">{t("format")}</p> + <p className="text-sm font-medium">{t("format")}</p> - <div className="test-sm text-default col-span-5 ltr:mr-2 rtl:ml-2"> + <div className="text-sm text-default col-span-5 ltr:mr-2 rtl:ml-2"> - <div className="test-sm text-emphasis col-span-7">{t("company_size")}</div> + <div className="text-sm text-emphasis col-span-7">{t("company_size")}</div> - <div className="test-sm text-default col-span-5 w-full">{t("variable")}</div> + <div className="text-sm text-default col-span-5 w-full">{t("variable")}</div> - <div className="test-sm text-emphasis col-span-7 break-words"> + <div className="text-sm text-emphasis col-span-7 break-words"> - <p className="test-sm w-full font-medium">{t("example_2")}</p> + <p className="text-sm w-full font-medium">{t("example_2")}</p> - <div className="test-sm text-default col-span-5 ltr:mr-2 rtl:ml-2"> + <div className="text-sm text-default col-span-5 ltr:mr-2 rtl:ml-2"> - <div className="test-sm text-default col-span-5">{t("variable")}</div> + <div className="text-sm text-default col-span-5">{t("variable")}</div> - <div className="test-sm text-emphasis col-span-7 break-words"> + <div className="text-sm text-emphasis col-span-7 break-words">
♻️ Duplicate comments (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
264-272: Also guard WhatsApp steps; don’t (yet) block on EMAIL_HOST.Form triggers should be hidden if any Cal.AI, SMS, or WhatsApp steps exist. Currently you block on EMAIL_HOST instead and miss WhatsApp. Replace EMAIL_HOST gate with WhatsApp. If EMAIL_HOST must be disallowed for form triggers, please confirm product intent and handle at action filtering rather than hiding triggers.
-const hasAiAction = hasCalAIAction(steps); -const hasSMSAction = steps.some((s) => isSMSAction(s.action)); -const hasEmailToHostAction = steps.some((s) => s.action === WorkflowActions.EMAIL_HOST); - -const disallowFormTriggers = hasAiAction || hasSMSAction || hasEmailToHostAction; +const hasAiAction = hasCalAIAction(steps); +const hasSMSAction = steps.some((s) => isSMSAction(s.action)); +const hasWhatsappAction = steps.some((s) => isWhatsappAction(s.action)); + +const disallowFormTriggers = hasAiAction || hasSMSAction || hasWhatsappAction;
🧹 Nitpick comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
1116-1118: Avoid mixing controlled and uncontrolled props in Select.You pass both defaultValue and value; keep it controlled and drop defaultValue.
- defaultValue={selectedTemplate} value={selectedTemplate}
1209-1230: Hide “Include calendar event” for form triggers.Form triggers have no booking; this toggle should not appear.
- {isEmailSubjectNeeded && ( + {isEmailSubjectNeeded && !isFormTrigger(trigger) && ( <div className="mt-2"> <Controller name={`steps.${step.stepNumber - 1}.includeCalendarEvent`}
1346-1346: Trim stray leading space in heading.- <h1 className="w-full text-xl font-semibold"> {t("how_booking_questions_as_variables")}</h1> + <h1 className="w-full text-xl font-semibold">{t("how_booking_questions_as_variables")}</h1>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx(17 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{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:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{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:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (4)
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.622Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.622Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:07:30.058Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/workflows/update.handler.ts:738-763
Timestamp: 2025-08-15T00:07:30.058Z
Learning: In calcom/cal.com workflows, Cal AI phone call actions (CAL_AI_PHONE_CALL) intentionally always require the phone number field when the action is present, unlike SMS/WhatsApp actions which respect the step.numberRequired flag. This is the intended behavior per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
packages/features/ee/workflows/lib/getOptions.ts (2)
getWorkflowTriggerOptions(33-46)getWorkflowTemplateOptions(62-80)packages/features/ee/workflows/lib/actionHelperFunctions.ts (5)
hasCalAIAction(152-154)isSMSAction(25-27)isFormTrigger(145-150)getTemplateBodyForAction(118-143)shouldScheduleEmailReminder(17-19)
🔇 Additional comments (2)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (2)
476-485: Force CUSTOM template on form triggers: LGTM.Ensures no incompatible prebuilt templates leak into form-trigger flows.
256-258: Template options depend on trigger: LGTM.Passing trigger into getWorkflowTemplateOptions aligns UI with form-trigger constraints.
5 participants
What does this PR do?
Adds two new triggers to workflows:
This PR only supporrt email actions. Cal.ai and SMS actions will be added in a follow up. Whatsapp actions need new templates first.
Follow up PRs:
SMS action support feat: SMS workflow action for form triggers #3 #23673
Cal.ai action support
Workflow variables
FORM_SUBMITTED_NO_EVENTtrigger: a variable for the routed event typeWhatsApp actions
How should this be tested?
scheduledAtto a past date and callcurl http://localhost:3000/api/tasks/cronto trigger the task