-
Notifications
You must be signed in to change notification settings - Fork 11.6k
feat: form submitted workflow triggers #1 #23704
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
- 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: [email protected] <[email protected]>
- 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: [email protected] <[email protected]>
…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: [email protected] <[email protected]>
- 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: [email protected] <[email protected]>
….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: [email protected] <[email protected]>
…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: [email protected] <[email protected]>
…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: [email protected] <[email protected]>
…webhook and workflow coverage" This reverts commit 7642992.
…ub.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
| const workflowsToTrigger: Workflow[] = []; | ||
|
|
||
| workflowsToTrigger.push( | ||
| ...workflows.filter((workflow) => workflow.trigger === WorkflowTriggerEvents.FORM_SUBMITTED) |
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.
FORM_WORKFLOW_TRIGGERS.includes(WorkflowTriggerEvents.FORM_SUBMITTED)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.
FORM_SUBMITTED_NO_EVENT that's introduced in a follow up PR is handled differently, see here: https://github.com/calcom/cal.com/pull/23716/files#diff-62e7317f884fe7df32c555141a1953f2a212f818dc8c8b0da77e4e75cf3a351fR190-R192
So I think it's better to check for the specific FORM_SUBMITTED trigger here
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
🧹 Nitpick comments (2)
packages/lib/server/service/workflows.ts (2)
46-65: Reuse the TeamRepository instance.The
TeamRepositoryis instantiated twice (lines 47 and 61) within the same method. Reuse the first instance to eliminate redundant instantiation.Apply this diff:
static async deleteWorkflowRemindersOfRemovedTeam(teamId: number) { const teamRepository = new TeamRepository(prisma); const team = await teamRepository.findById({ id: teamId }); if (team?.parentId) { const activeWorkflowsOnTeam = await WorkflowRepository.findActiveWorkflowsOnTeam({ parentTeamId: team.parentId, teamId: team.id, }); for (const workflow of activeWorkflowsOnTeam) { const workflowSteps = workflow.steps; let remainingActiveOnIds = []; if (workflow.isActiveOnAll) { - const teamRepository = new TeamRepository(prisma); const allRemainingOrgTeams = await teamRepository.findOrgTeamsExcludingTeam({ parentId: team.parentId, excludeTeamId: team.id, });
82-134: Add early return after filtering workflows.The method correctly extracts phone numbers, computes branding, and schedules reminders. However, for consistency with
scheduleWorkflowsForNewBooking(line 173) and to avoid unnecessary work, add an early return if no workflows match theFORM_SUBMITTEDtrigger.Apply this diff after line 107:
workflowsToTrigger.push( ...workflows.filter((workflow) => workflow.trigger === WorkflowTriggerEvents.FORM_SUBMITTED) ); + + if (workflowsToTrigger.length === 0) return; let smsReminderNumber: string | null = null;
📜 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 (3)
packages/lib/server/repository/team.ts(2 hunks)packages/lib/server/repository/workflow.ts(8 hunks)packages/lib/server/service/workflows.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/lib/server/repository/team.ts
- packages/lib/server/repository/workflow.ts
🧰 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/lib/server/service/workflows.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/lib/server/service/workflows.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/lib/server/service/workflows.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/lib/server/service/workflows.ts
🧬 Code graph analysis (1)
packages/lib/server/service/workflows.ts (7)
packages/lib/server/repository/workflow.ts (1)
WorkflowRepository(90-862)packages/lib/getOrgIdFromMemberOrTeamId.ts (1)
getOrgIdFromMemberOrTeamId(47-58)packages/features/ee/workflows/lib/getAllWorkflows.ts (1)
getAllWorkflows(31-139)packages/lib/server/repository/team.ts (1)
TeamRepository(170-488)packages/app-store/routing-forms/lib/formSubmissionUtils.ts (1)
FORM_SUBMITTED_WEBHOOK_RESPONSES(23-35)packages/lib/hideBranding.ts (1)
getHideBranding(51-81)packages/features/ee/workflows/lib/reminders/reminderScheduler.ts (1)
scheduleWorkflowReminders(416-419)
⏰ 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). (8)
- GitHub Check: Production builds / Build Docs
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build API v1
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Production builds / Build API v2
- GitHub Check: Linters / lint
- GitHub Check: Production builds / Build Atoms
🔇 Additional comments (1)
packages/lib/server/service/workflows.ts (1)
21-44: LGTM!The method correctly fetches workflows active on a routing form, derives the organization ID, and aggregates all applicable workflows using the established
getAllWorkflowspattern withWorkflowType.ROUTING_FORM.
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: 1
📜 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 (1)
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts(9 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/features/ee/workflows/lib/reminders/reminderScheduler.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/features/ee/workflows/lib/reminders/reminderScheduler.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/features/ee/workflows/lib/reminders/reminderScheduler.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: In Cal.com codebase, the pattern of using `string.includes("@")` for email detection is consistently used across workflow-related features, including schema validation, UI components, and reminder scheduling. The getSubmitterEmail function follows this established pattern rather than introducing new validation logic.
Applied to files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: In Cal.com codebase, the getSubmitterEmail function uses simple "@" presence checking for email extraction from form responses, which maintains consistency with the existing inline logic that was already in production before being refactored into a separate function.
Applied to files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts
📚 Learning: 2025-09-10T13:05:45.975Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Applied to files:
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts
🧬 Code graph analysis (1)
packages/features/ee/workflows/lib/reminders/reminderScheduler.ts (6)
packages/app-store/routing-forms/lib/formSubmissionUtils.ts (1)
FORM_SUBMITTED_WEBHOOK_RESPONSES(23-35)packages/lib/formatCalendarEvent.ts (1)
formatCalEventExtended(41-49)packages/features/ee/workflows/lib/reminders/smsReminderManager.ts (3)
BookingInfo(46-73)scheduleSMSReminder(90-278)ScheduleTextReminderAction(75-78)packages/features/ee/workflows/lib/actionHelperFunctions.ts (3)
isSMSAction(25-27)isWhatsappAction(29-31)isCalAIAction(37-39)packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts (1)
getSubmitterEmail(18-23)packages/features/ee/workflows/lib/reminders/emailReminderManager.ts (1)
scheduleEmailReminder(107-119)
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: 3
♻️ Duplicate comments (2)
packages/lib/server/repository/workflow.ts (2)
537-577: Replaceincludewith explicitselectThis method uses
includeinstead ofselect, which inflates payloads and violates the coding guideline. As noted in previous reviews, this should be refactored to use explicitselectfor only the needed fields.[As per coding guidelines]
579-649: Replaceincludewith explicitselectThis method uses
includeinstead ofselect, violating the coding guideline. As noted in previous reviews, please refactor to use explicitselectfor only the required fields.[As per coding guidelines]
🧹 Nitpick comments (1)
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (1)
58-92: Prefer extracting complex filter conditions into named variables.The filtering logic in lines 61-69 would benefit from extracting the conditions into descriptive boolean variables, improving readability and maintainability.
Based on a previous review suggestion, consider this refactor:
const transformedActionOptions = baseActionOptions ? baseActionOptions .filter((option) => { - const isFormWorkflowWithInvalidSteps = - isFormTrigger(form.getValues("trigger")) && - !ALLOWED_FORM_WORKFLOW_ACTIONS.some((action) => action === option.value); - - const isSelectAllCalAiAction = isCalAIAction(option.value) && form.watch("selectAll"); - - const isOrgCalAiAction = isCalAIAction(option.value) && isOrg; - - if (isFormWorkflowWithInvalidSteps || isSelectAllCalAiAction || isOrgCalAiAction) { - return false; - } - return true; + const isFormWorkflowWithInvalidSteps = + isFormTrigger(form.getValues("trigger")) && + !ALLOWED_FORM_WORKFLOW_ACTIONS.some((action) => action === option.value); + const isSelectAllCalAiAction = isCalAIAction(option.value) && form.watch("selectAll"); + const isOrgCalAiAction = isCalAIAction(option.value) && isOrg; + return !(isFormWorkflowWithInvalidSteps || isSelectAllCalAiAction || isOrgCalAiAction); }) .map((option) => { let label = option.label; // Transform labels for form triggers if (isFormTrigger(form.getValues("trigger"))) { if (option.value === WorkflowActions.EMAIL_ATTENDEE) { label = t("email_attendee_action_form"); } } return { ...option, label, creditsTeamId: teamId, isOrganization: isOrg, isCalAi: isCalAIAction(option.value), }; }) : [];
📜 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 (3)
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx(8 hunks)packages/features/ee/workflows/lib/actionHelperFunctions.ts(2 hunks)packages/lib/server/repository/workflow.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/ee/workflows/lib/actionHelperFunctions.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/WorkflowDetailsPage.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/WorkflowDetailsPage.tsxpackages/lib/server/repository/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:
packages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/lib/server/repository/workflow.ts
**/*.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/lib/server/repository/workflow.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
📚 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/WorkflowDetailsPage.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
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/WorkflowDetailsPage.tsx
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/lib/server/repository/workflow.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`
Applied to files:
packages/lib/server/repository/workflow.ts
📚 Learning: 2025-09-10T13:05:45.975Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Applied to files:
packages/lib/server/repository/workflow.ts
🧬 Code graph analysis (2)
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (3)
packages/trpc/react/trpc.ts (1)
trpc(54-138)packages/features/ee/workflows/lib/actionHelperFunctions.ts (2)
isFormTrigger(146-148)isCalAIAction(38-40)packages/features/ee/workflows/lib/constants.ts (1)
ALLOWED_FORM_WORKFLOW_ACTIONS(103-106)
packages/lib/server/repository/workflow.ts (2)
packages/features/ee/workflows/lib/constants.ts (1)
FORM_TRIGGER_WORKFLOW_EVENTS(101-101)packages/features/ee/workflows/lib/reminders/emailReminderManager.ts (1)
deleteScheduledEmailReminder(397-452)
🔇 Additional comments (15)
packages/lib/server/repository/workflow.ts (7)
4-4: LGTM!The imports, constant
excludeFormTriggersWhereClause, and helpergetWorkflowTypeare correctly implemented and align with the workflow trigger classification logic.Also applies to: 30-43
124-133: LGTM!The addition of
activeOnRoutingFormsto theselectclause is consistent with the coding guidelines and properly fetches only the required fields.
464-464: LGTM!The refactor to use the imported
deleteScheduledEmailReminderfunction improves consistency and maintainability across reminder deletion methods.
488-515: LGTM!The method correctly uses
selectinstead ofincludeand fetches only the necessary fields for update operations, adhering to the coding guidelines.[As per coding guidelines]
517-535: LGTM!The method correctly derives the workflow type from the trigger using
getWorkflowTypeand updates the workflow accordingly.
808-835: LGTM!This method correctly uses
selectinstead ofincludeand fetches only the necessary fields, adhering to the coding guidelines.[As per coding guidelines]
837-860: LGTM!This method correctly uses
selectinstead ofincludeand fetches only the necessary fields, adhering to the coding guidelines.[As per coding guidelines]
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (8)
9-10: LGTM!The new imports support the form workflow trigger feature correctly.
19-21: LGTM!The imported helpers, constants, and template function are appropriately used throughout the component.
44-44: LGTM!Adding
i18nto the destructuring is necessary for locale-aware template generation.
50-50: LGTM!The
updateTemplatestate enables template synchronization between parent and child workflow step components.
120-150: LGTM! Template generation correctly handles form vs. event triggers.The conditional template logic properly:
- Uses
CUSTOMtemplate for form triggers (with null body/subject, as routing form variables will be added in a follow-up PR)- Uses
REMINDERtemplate for event triggers with generated content- Passes appropriate locale and time format to the template function
Based on learnings.
197-199: LGTM!The new props correctly enable the trigger step to access filtered action options and manage template state synchronization.
272-274: LGTM!Consistent prop passing to action steps ensures uniform behavior across all workflow steps.
304-304: LGTM!Passing
actionOptionstoAddActionDialogcorrectly enables the dialog to display filtered actions appropriate for the current trigger type.
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
♻️ Duplicate comments (4)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
377-390: GuarduseWatchresult before calling.someOn first render
useWatch({ name: "steps" })returnsundefined, sosteps.some(...)andhasCalAIAction(steps)throw a TypeError and crash the page. Default the watched value to an array (e.g.const watchedSteps = useWatch(...) ?? []; const steps = Array.isArray(watchedSteps) ? watchedSteps : [];) before running these predicates.packages/trpc/server/routers/viewer/workflows/update.schema.ts (2)
29-40: Prefer a discriminated union keyed by trigger (forms vs events)Model two shapes so we validate the right fields early and prevent incompatible payloads (as requested earlier).
Example approach:
- For form triggers: require
activeOnRoutingFormIds(non-empty), forcetime/timeUnitto null, and disallowactiveOnEventTypeIds.- For event triggers: require
activeOnEventTypeIds(orisActiveOnAll), disallowactiveOnRoutingFormIds.If helpful, I can draft a z.discriminatedUnion version. Based on past review comments.
33-35: Default activeOn arrays to [] to avoid undefined crashes in handlerWithout defaults, these can be undefined and break
.length/.filterin update.handler.ts (e.g., Lines 87, 239, 468, 547).Apply:
- activeOnEventTypeIds: z.number().array(), // also includes team ids - activeOnRoutingFormIds: z.string().array(), + activeOnEventTypeIds: z.number().array().default([]), // also includes team ids + activeOnRoutingFormIds: z.string().array().default([]),packages/trpc/server/routers/viewer/workflows/update.handler.ts (1)
214-223: Event → Form trigger switch may leave orphan reminders; populate oldActiveOnIds before deleteWhen previous trigger was event-based and new is form,
oldActiveOnIdsis empty here, so nothing is deleted.Apply:
if (!isFormTrigger(userWorkflow.trigger)) { + // If switching to a form trigger, ensure oldActiveOnIds reflects prior event types (+children) + if (isFormTrigger(trigger) && oldActiveOnIds.length === 0) { + const eventTypeRepo = new EventTypeRepository(ctx.prisma); + const prevEventTypes = userWorkflow.isActiveOnAll + ? (userWorkflow.teamId + ? await eventTypeRepo.findAllIncludingChildrenByTeamId({ teamId: userWorkflow.teamId! }) + : await eventTypeRepo.findAllIncludingChildrenByUserId({ userId: userWorkflow.userId! })) + : (await workflowRelationsRepository.findActiveOnEventTypes(id)).map((r) => ({ + id: r.eventTypeId, + children: r.eventType.children, + })); + oldActiveOnIds = prevEventTypes.flatMap((et) => [et.id, ...et.children.map((c) => c.id)]); + } // Delete all existing reminders before rescheduling await deleteRemindersOfActiveOnIds({ removedActiveOnIds: oldActiveOnIds, workflowSteps: userWorkflow.steps, isOrg, });
🧹 Nitpick comments (6)
packages/trpc/server/routers/viewer/workflows/update.schema.ts (1)
12-27: Optional: add per‑action field requirements at schema levelEnforce inputs per step:
- EMAIL_ADDRESS requires non-empty valid
sendTo.- SMS_NUMBER/WHATSAPP_NUMBER require
numberRequiredandsendTowhen applicable.- CAL_AI_PHONE_CALL steps can gate
agentId/inboundAgentId.This reduces handler-time checks and bad states.
packages/trpc/server/routers/viewer/workflows/update.handler.ts (5)
239-249: Schedule only newly added event types (children-aware) when trigger/time didn’t changeUsing
newActiveOn(parents) can miss children. Use a children-aware diff.Apply:
- await scheduleWorkflowNotifications({ - activeOn: newActiveOn, + await scheduleWorkflowNotifications({ + activeOn: activeOnWithChildren.filter((id) => !oldActiveOnIds.includes(id)), 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 + alreadyScheduledActiveOnIds: activeOnWithChildren.filter((id) => oldActiveOnIds.includes(id)), });
465-476: Edited-step scheduling should use children-aware setCurrently uses
activeOnEventTypeIds(parents). Switch toactiveOnWithChildren.- await scheduleWorkflowNotifications({ - activeOn: activeOnEventTypeIds, + await scheduleWorkflowNotifications({ + activeOn: activeOnWithChildren, isOrg, workflowSteps: [newStep], time, timeUnit, trigger, userId: user.id, teamId: userWorkflow.teamId, });
545-555: Added-step scheduling should use children-aware setUse
activeOnWithChildrento cover child event types.- await scheduleWorkflowNotifications({ - activeOn: activeOnEventTypeIds, + await scheduleWorkflowNotifications({ + activeOn: activeOnWithChildren, isOrg, workflowSteps: createdSteps, time, timeUnit, trigger, userId: user.id, teamId: userWorkflow.teamId, });
579-584: Boolean gate: coercelengthto boolean explicitly
activeOnWithChildren.length && ...yieldsnumber | boolean. Use> 0.- const smsReminderNumberNeeded = - activeOnWithChildren.length && + const smsReminderNumberNeeded = + activeOnWithChildren.length > 0 && steps.some( (step) => step.action === WorkflowActions.SMS_ATTENDEE || step.action === WorkflowActions.WHATSAPP_ATTENDEE );
613-616: Boolean gate for AI steps: uselength > 0Ensure a boolean, not
number | boolean.- const aiPhoneCallStepsNeeded = - activeOnWithChildren.length && steps.some((s) => s.action === WorkflowActions.CAL_AI_PHONE_CALL); + const aiPhoneCallStepsNeeded = + activeOnWithChildren.length > 0 && steps.some((s) => s.action === WorkflowActions.CAL_AI_PHONE_CALL);
📜 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 (8)
apps/web/public/static/locales/en/common.json(4 hunks)packages/features/ee/workflows/components/WorkflowDetailsPage.tsx(7 hunks)packages/features/ee/workflows/components/WorkflowStepContainer.tsx(40 hunks)packages/features/ee/workflows/pages/workflow.tsx(9 hunks)packages/lib/server/repository/PrismaAgentRepository.ts(1 hunks)packages/prisma/schema.prisma(5 hunks)packages/trpc/server/routers/viewer/workflows/update.handler.ts(14 hunks)packages/trpc/server/routers/viewer/workflows/update.schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/server/repository/PrismaAgentRepository.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/pages/workflow.tsxpackages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/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/pages/workflow.tsxpackages/trpc/server/routers/viewer/workflows/update.schema.tspackages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/features/ee/workflows/components/WorkflowStepContainer.tsxpackages/trpc/server/routers/viewer/workflows/update.handler.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/features/ee/workflows/pages/workflow.tsxpackages/trpc/server/routers/viewer/workflows/update.schema.tspackages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/features/ee/workflows/components/WorkflowStepContainer.tsxpackages/trpc/server/routers/viewer/workflows/update.handler.ts
**/*.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/update.schema.tspackages/trpc/server/routers/viewer/workflows/update.handler.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
📚 Learning: 2025-09-10T13:35:21.197Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
Applied to files:
packages/features/ee/workflows/pages/workflow.tsxpackages/features/ee/workflows/components/WorkflowDetailsPage.tsxpackages/features/ee/workflows/components/WorkflowStepContainer.tsxpackages/trpc/server/routers/viewer/workflows/update.handler.ts
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
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/WorkflowDetailsPage.tsxpackages/features/ee/workflows/components/WorkflowStepContainer.tsxpackages/trpc/server/routers/viewer/workflows/update.handler.ts
📚 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/WorkflowDetailsPage.tsxpackages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T09:12:08.280Z
Learnt from: hariombalhara
PR: calcom/cal.com#22968
File: packages/features/auth/lib/next-auth-options.ts:327-327
Timestamp: 2025-08-08T09:12:08.280Z
Learning: In packages/features/auth/lib/next-auth-options.ts, do not log credentials in authorize() handlers (e.g., the "saml-idp" CredentialsProvider). Remove accidental console.log statements and avoid including credential contents in logs; prefer either no logging or structured logs without sensitive data.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-10T13:05:45.975Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 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/update.handler.ts
🧬 Code graph analysis (5)
packages/features/ee/workflows/pages/workflow.tsx (1)
packages/features/ee/workflows/lib/actionHelperFunctions.ts (1)
isFormTrigger(146-148)
packages/trpc/server/routers/viewer/workflows/update.schema.ts (2)
packages/features/ee/workflows/lib/constants.ts (5)
WORKFLOW_ACTIONS(19-28)WORKFLOW_TEMPLATES(32-39)WORKFLOW_TRIGGER_EVENTS(3-17)TIME_UNIT(30-30)ALLOWED_FORM_WORKFLOW_ACTIONS(103-106)packages/features/ee/workflows/lib/actionHelperFunctions.ts (1)
isFormTrigger(146-148)
packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (3)
packages/trpc/react/trpc.ts (1)
trpc(54-138)packages/features/ee/workflows/lib/actionHelperFunctions.ts (2)
isFormTrigger(146-148)isCalAIAction(38-40)packages/features/ee/workflows/lib/constants.ts (1)
ALLOWED_FORM_WORKFLOW_ACTIONS(103-106)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (4)
packages/features/ee/workflows/lib/getOptions.ts (1)
getWorkflowTemplateOptions(63-81)packages/features/ee/workflows/lib/actionHelperFunctions.ts (7)
hasCalAIAction(150-152)isSMSAction(26-28)isWhatsappAction(30-32)isFormTrigger(146-148)getTemplateBodyForAction(119-144)shouldScheduleEmailReminder(18-20)isCalAIAction(38-40)packages/ui/components/editor/Editor.tsx (1)
Editor(59-119)packages/features/ee/workflows/lib/constants.ts (1)
DYNAMIC_TEXT_VARIABLES(56-79)
packages/trpc/server/routers/viewer/workflows/update.handler.ts (9)
packages/lib/server/repository/workflow.ts (1)
WorkflowRepository(89-861)packages/lib/server/repository/workflowRelations.ts (1)
WorkflowRelationsRepository(3-94)packages/lib/server/repository/team.ts (1)
TeamRepository(170-488)packages/trpc/server/routers/viewer/workflows/util.ts (3)
isAuthorizedToAddActiveOnIds(456-538)deleteRemindersOfActiveOnIds(540-555)scheduleWorkflowNotifications(591-626)packages/features/ee/workflows/lib/actionHelperFunctions.ts (1)
isFormTrigger(146-148)packages/lib/server/repository/eventTypeRepository.ts (1)
EventTypeRepository(82-1489)packages/lib/server/repository/workflowStep.ts (1)
WorkflowStepRepository(3-22)packages/features/ee/workflows/lib/repository/workflowReminder.ts (1)
WorkflowReminderRepository(4-50)packages/lib/server/repository/PrismaAgentRepository.ts (1)
PrismaAgentRepository(31-642)
⏰ 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). (8)
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build Docs
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Production builds / Build API v2
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Production builds / Build API v1
- GitHub Check: Tests / Unit
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: 1
♻️ Duplicate comments (1)
packages/lib/server/repository/PrismaAgentRepository.ts (1)
616-621: Past review comment partially addressed.The method was renamed from
findAgentWithProviderIdtofindProviderAgentIdByIdas requested, but the parameter style still uses a plain string (agentId: string) instead of an object parameter ({ id }: { id: string }).Apply this diff to complete the requested changes:
- async findProviderAgentIdById(agentId: string) { + async findProviderAgentIdById({ id }: { id: string }) { return await this.prismaClient.agent.findUnique({ - where: { id: agentId }, + where: { id }, select: { providerAgentId: true }, }); }Also update the call site in
packages/trpc/server/routers/viewer/workflows/update.handler.ts(line 648) to:const agent = await PrismaAgentRepository.findProviderAgentIdById({ id: step.agentId });
📜 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)
packages/lib/server/repository/PrismaAgentRepository.ts(4 hunks)packages/trpc/server/routers/viewer/workflows/update.handler.ts(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/trpc/server/routers/viewer/workflows/update.handler.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*Repository.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Repository files must include
Repositorysuffix, prefix with technology if applicable (e.g.,PrismaAppRepository.ts), and use PascalCase matching the exported class
Files:
packages/lib/server/repository/PrismaAgentRepository.ts
**/*.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/lib/server/repository/PrismaAgentRepository.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/lib/server/repository/PrismaAgentRepository.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/lib/server/repository/PrismaAgentRepository.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
| async findAgentWithPhoneNumbers(agentId: string) { | ||
| return await this.prismaClient.agent.findUnique({ | ||
| where: { id: agentId }, | ||
| select: { | ||
| id: true, | ||
| outboundPhoneNumbers: { | ||
| select: { | ||
| id: true, | ||
| phoneNumber: true, | ||
| subscriptionStatus: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Use object parameter for consistency.
This method uses a plain string parameter, while other methods in this repository consistently use object parameters (e.g., findById({ id }), findByProviderAgentId({ providerAgentId })).
Apply this diff to match the established pattern:
- async findAgentWithPhoneNumbers(agentId: string) {
+ async findAgentWithPhoneNumbers({ id }: { id: string }) {
return await this.prismaClient.agent.findUnique({
- where: { id: agentId },
+ where: { id },
select: {
id: true,
outboundPhoneNumbers: {
select: {
id: true,
phoneNumber: true,
subscriptionStatus: true,
},
},
},
});
}Don't forget to update call sites to pass { id: agentId } instead of just agentId.
📝 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.
| async findAgentWithPhoneNumbers(agentId: string) { | |
| return await this.prismaClient.agent.findUnique({ | |
| where: { id: agentId }, | |
| select: { | |
| id: true, | |
| outboundPhoneNumbers: { | |
| select: { | |
| id: true, | |
| phoneNumber: true, | |
| subscriptionStatus: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } | |
| async findAgentWithPhoneNumbers({ id }: { id: string }) { | |
| return await this.prismaClient.agent.findUnique({ | |
| where: { id }, | |
| select: { | |
| id: true, | |
| outboundPhoneNumbers: { | |
| select: { | |
| id: true, | |
| phoneNumber: true, | |
| subscriptionStatus: true, | |
| }, | |
| }, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/lib/server/repository/PrismaAgentRepository.ts around lines 600 to
614, the method findAgentWithPhoneNumbers(agentId: string) uses a plain string
parameter while the repository pattern in this file expects object parameters;
change the signature to findAgentWithPhoneNumbers({ id }: { id: string }) and
update the prisma call to use where: { id } and return the same select shape,
then update all call sites to pass an object (e.g., { id: agentId }) instead of
a bare string to maintain consistency with other repository methods.
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
🧹 Nitpick comments (2)
packages/lib/server/repository/user.ts (2)
1072-1088: Add explicit return type annotation for type safety.The method lacks an explicit return type. Adding one improves type safety, IDE support, and makes the contract clearer for callers.
Apply this diff:
- async findUserWithHideBranding({ userId }: { userId: number }) { + async findUserWithHideBranding({ userId }: { userId: number }): Promise<{ + hideBranding: boolean; + profiles: { + organization: { + hideBranding: boolean; + } | null; + }[]; + } | null> { return this.prismaClient.user.findUnique({
1077-1085: Consider documenting the profiles array usage.The method returns all profiles, but per the AI summary, the caller in
packages/lib/hideBranding.tsaccessesuser.profiles[0].organization.hideBranding. If the expectation is that only the first profile matters for branding resolution, consider adding a comment explaining this behavior or limiting the query to fetch only one profile.
📜 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)
packages/app-store/routing-forms/lib/handleResponse.test.ts(1 hunks)packages/lib/server/repository/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-store/routing-forms/lib/handleResponse.test.ts
🧰 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/lib/server/repository/user.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/lib/server/repository/user.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/lib/server/repository/user.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/emailReminderManager.ts:344-0
Timestamp: 2025-09-10T13:05:45.975Z
Learning: In Cal.com PR #23704 (form submitted workflow triggers), routing form variables for email templates are intentionally left as empty objects ({}) in the initial implementation. The routing form variables (like submission time, form name, form responses) will be added in a planned follow-up PR as part of the workflow variables feature.
Learnt from: CarinaWolli
PR: calcom/cal.com#23704
File: packages/features/ee/workflows/lib/reminders/reminderScheduler.ts:171-176
Timestamp: 2025-09-10T13:35:21.197Z
Learning: The getSubmitterEmail function in packages/features/tasker/tasks/triggerFormSubmittedNoEvent/triggerFormSubmittedNoEventWebhook.ts was extracted from existing inline logic that was already in production use for webhook processing and duplicate detection, and is now being reused in the workflow feature for consistency.
What does this PR do?
Adds a new workflow trigger "When routing form is submitted". For now only email actions are support "Send email to submitted email address" and "Send email to specific email address"
Follow ups PRs:
FORM_SUBMITTED_NO_EVENTtrigger: a variable for the routed event typeFixes CAL-6398
Fixes #23746
Fixes CAL-6396
Fixes #23744
How should this be tested?