feat: form submitted no event booked workflow trigger #2#23716
feat: form submitted no event booked workflow trigger #2#23716CarinaWolli merged 263 commits intomainfrom
Conversation
…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
…755107037-add-workflow-triggers
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 (3)
apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.ts (1)
129-137: Swagger schema mismatch:datais a single object but annotated as array.Change
type: [RoutingFormWorkflowOutput]totype: RoutingFormWorkflowOutput.Apply:
- @ApiProperty({ - description: "workflow", - type: [RoutingFormWorkflowOutput], - }) + @ApiProperty({ + description: "workflow", + type: RoutingFormWorkflowOutput, + })apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
31-39: Fix validation guard to reference the correct flag.Should reference
isActiveOnAllRoutingForms, notisActiveOnAllEventTypes.- @ValidateIf((o) => !o.isActiveOnAllEventTypes) + @ValidateIf((o) => !o.isActiveOnAllRoutingForms)apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (1)
90-93: Extend routing-form enum-to-trigger map to include FORM_SUBMITTED_NO_EVENTexport const ENUM_TO_ROUNTING_FORM_WORKFLOW_TRIGGER = { [WorkflowTriggerEvents.FORM_SUBMITTED]: FORM_SUBMITTED, + [WorkflowTriggerEvents.FORM_SUBMITTED_NO_EVENT]: FORM_SUBMITTED_NO_EVENT, } as const;
🧹 Nitpick comments (5)
apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.ts (1)
37-41: Clarify offset doc for routing-form triggers.BEFORE_EVENT/AFTER_EVENT are event-type triggers, not routing-form. Recommend limiting to FORM_SUBMITTED_NO_EVENT here.
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
31-35: Correct Swaggertypefor routing form IDs.IDs are strings; update
type: [Number]to[String].- type: [Number], + type: [String],apps/api/v2/src/modules/workflows/services/workflows.output.service.ts (2)
217-226: Good offset mapping; consider helper to DRY.Build offset via a small helper to reuse across event-type/routing-form paths.
Example:
const buildOffset = (w: WorkflowType) => ({ value: w.time ?? 1, unit: w.timeUnit ? ENUM_TO_TIME_UNIT[w.timeUnit] : HOUR, });
211-216: Use output Activation DTO for clarity.Prefer
RoutingFormWorkflowActivationOutputDtoover input DTO.- import { WorkflowFormActivationDto } from "@/modules/workflows/inputs/create-form-workflow"; + import { RoutingFormWorkflowActivationOutputDto } from "@/modules/workflows/outputs/routing-form-workflow.output"; ... - const activation: WorkflowFormActivationDto = { + const activation: RoutingFormWorkflowActivationOutputDto = {apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (1)
52-54: Optional: standardize trigger list interpolation.For consistency with create DTO, use
.toString()onFORM_WORKFLOW_TRIGGER_TYPES.- description: `Trigger configuration for the routing-form workflow, allowed triggers are ${FORM_WORKFLOW_TRIGGER_TYPES}`, + description: `Trigger configuration for the routing-form workflow, allowed triggers are ${FORM_WORKFLOW_TRIGGER_TYPES.toString()}`,
📜 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 (7)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts(3 hunks)apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts(2 hunks)apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts(6 hunks)apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.ts(1 hunks)apps/api/v2/src/modules/workflows/services/workflows.input.service.ts(2 hunks)apps/api/v2/src/modules/workflows/services/workflows.output.service.ts(5 hunks)apps/web/public/static/locales/en/common.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/api/v2/src/modules/workflows/services/workflows.input.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.tsapps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.tsapps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/modules/workflows/outputs/routing-form-workflow.output.tsapps/api/v2/src/modules/workflows/inputs/create-form-workflow.tsapps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/workflows/services/workflows.output.service.ts
🧠 Learnings (2)
📓 Common learnings
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:
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.tsapps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts
🧬 Code graph analysis (4)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (5)
OnFormSubmittedNoEventTriggerDto(270-278)OnFormSubmittedTriggerDto(260-268)RoutingFormWorkflowTriggerDto(139-147)FORM_SUBMITTED(14-14)FORM_SUBMITTED_NO_EVENT(15-15)
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (1)
packages/platform/libraries/index.ts (1)
WorkflowTriggerEvents(35-35)
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (2)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
ApiExtraModels(42-106)apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (5)
OnFormSubmittedTriggerDto(260-268)OnFormSubmittedNoEventTriggerDto(270-278)RoutingFormWorkflowTriggerDto(139-147)FORM_SUBMITTED(14-14)FORM_SUBMITTED_NO_EVENT(15-15)
apps/api/v2/src/modules/workflows/services/workflows.output.service.ts (2)
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (7)
OnFormSubmittedNoEventTriggerDto(270-278)WORKFLOW_TRIGGER_TO_ENUM(56-71)FORM_SUBMITTED(14-14)FORM_SUBMITTED_NO_EVENT(15-15)ENUM_TO_WORKFLOW_TRIGGER(73-88)ENUM_TO_TIME_UNIT(108-112)HOUR(94-94)apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
WorkflowFormActivationDto(22-40)
⏰ 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). (7)
- GitHub Check: Tests / E2E App Store
- GitHub Check: Tests / E2E Embed React
- GitHub Check: Tests / E2E API v2
- GitHub Check: Tests / E2E (3/4)
- GitHub Check: Tests / E2E (4/4)
- GitHub Check: Tests / E2E (1/4)
- GitHub Check: Tests / E2E (2/4)
🔇 Additional comments (5)
apps/api/v2/src/modules/workflows/inputs/create-form-workflow.ts (1)
63-81: LGTM: trigger union + discriminator are wired correctly.oneOf + discriminator setup for FORM_SUBMITTED and FORM_SUBMITTED_NO_EVENT looks correct.
apps/api/v2/src/modules/workflows/services/workflows.output.service.ts (2)
205-210: LGTM: routing-form trigger gating includes NO_EVENT.Correctly restricts to ROUTING_FORM with FORM_SUBMITTED/FORM_SUBMITTED_NO_EVENT.
Please confirm defaulting
time ?? 1andtimeUnit || HOURaligns with API defaults and UI expectations.
244-248: LGTM: exclude form-submitted triggers from event-type outputs.Correctly filters out FORM_SUBMITTED and FORM_SUBMITTED_NO_EVENT.
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (1)
14-23: LGTM: NO_EVENT trigger wired into constants, enums, and DTOs.Additions are consistent and correct (constants, both-direction maps, and DTO).
Also applies to: 39-49, 56-66, 73-83, 210-219, 270-278
apps/api/v2/src/modules/workflows/inputs/update-form-workflow.input.ts (1)
52-58: LGTM: trigger union + discriminator updated for NO_EVENT.Schema and typing are correct.
Also applies to: 65-71
E2E results are ready! |
apps/api/v2/src/modules/workflows/services/workflows.input.service.ts
Outdated
Show resolved
Hide resolved
apps/api/v2/src/modules/workflows/services/workflows.input.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/v2/src/modules/workflows/services/workflows.input.service.ts (1)
144-155: Partial updates clear existing offsets; fix fallback and enum/string unit mixing.When
updateDto.triggeris omitted,time/timeUnitbecome null/undefined, wiping existing offset. AlsotimeUnitForZod ?? currentData.timeUnitmixes string units with enum values, leading toTIME_UNIT_TO_ENUM[enum] === undefined.Use the effective trigger (new or current) to decide offset, preserve stored
time/timeUnitwhen not provided, and only map strings viaTIME_UNIT_TO_ENUM.- const timeUnitForZod = this._isOffsetTrigger(updateDto.trigger) - ? updateDto?.trigger?.offset?.unit ?? currentData.timeUnit ?? null - : undefined; - - const time = this._isOffsetTrigger(updateDto.trigger) - ? updateDto?.trigger?.offset?.value ?? currentData?.time ?? null - : null; - - const timeUnit = timeUnitForZod ? TIME_UNIT_TO_ENUM[timeUnitForZod] : null; + const isOffset = this._isOffsetTrigger(updateDto.trigger, currentData.trigger); + + // Prefer incoming DTO values; otherwise, preserve existing stored values + const timeUnitForZod = + isOffset ? updateDto?.trigger?.offset?.unit ?? null : undefined; + + const time = + isOffset ? updateDto?.trigger?.offset?.value ?? currentData?.time ?? null : null; + + // Map string unit to enum when provided; otherwise keep current enum + const timeUnit = + isOffset + ? timeUnitForZod + ? TIME_UNIT_TO_ENUM[timeUnitForZod] + : currentData.timeUnit ?? null + : null;
🧹 Nitpick comments (3)
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts (1)
103-106: Include FORM_SUBMITTED_NO_EVENT in ENUM_TO_ROUNTING_FORM_WORKFLOW_TRIGGER for completeness.The routing-form enum-to-string map only handles
FORM_SUBMITTED. Add the no‑event variant to avoid future inconsistencies.export const ENUM_TO_ROUNTING_FORM_WORKFLOW_TRIGGER = { [WorkflowTriggerEvents.FORM_SUBMITTED]: FORM_SUBMITTED, + [WorkflowTriggerEvents.FORM_SUBMITTED_NO_EVENT]: FORM_SUBMITTED_NO_EVENT, } as const;apps/api/v2/src/modules/organizations/teams/workflows/controllers/org-team-workflows.controller.e2e-spec.ts (1)
476-532: Add an update test for FORM_SUBMITTED_NO_EVENT with offset.Creation is covered. Add a PATCH test updating a routing-form workflow to
FORM_SUBMITTED_NO_EVENTwith an offset to verify offsets persist and serialize correctly on updates.apps/api/v2/src/modules/workflows/services/workflows.output.service.ts (1)
145-158: Minor: prefer Set membership for constant lookups.Replace
.some(...)withSetmembership for O(1) checks and clarity.Example:
const FORM_TRIGGERS = new Set(FORM_WORKFLOW_TRIGGER_TYPES.map(t => WORKFLOW_TRIGGER_TO_ENUM[t])); return FORM_TRIGGERS.has(trigger);
📜 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 (4)
apps/api/v2/src/modules/organizations/teams/workflows/controllers/org-team-workflows.controller.e2e-spec.ts(2 hunks)apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts(6 hunks)apps/api/v2/src/modules/workflows/services/workflows.input.service.ts(3 hunks)apps/api/v2/src/modules/workflows/services/workflows.output.service.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/organizations/teams/workflows/controllers/org-team-workflows.controller.e2e-spec.tsapps/api/v2/src/modules/workflows/services/workflows.input.service.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/organizations/teams/workflows/controllers/org-team-workflows.controller.e2e-spec.tsapps/api/v2/src/modules/workflows/services/workflows.input.service.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.tsapps/api/v2/src/modules/organizations/teams/workflows/controllers/org-team-workflows.controller.e2e-spec.tsapps/api/v2/src/modules/workflows/services/workflows.input.service.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/workflows/services/workflows.input.service.tsapps/api/v2/src/modules/workflows/services/workflows.output.service.ts
🧠 Learnings (2)
📓 Common learnings
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:
apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts
🔇 Additional comments (1)
apps/api/v2/src/modules/workflows/services/workflows.output.service.ts (1)
212-229: Confirm default offset fallback (1 hour) is intended.When
workflow.time/timeUnitare null, output defaults to1andHOUR. Verify this matches product expectations; otherwise, consider omittingoffsetor returning stored values only.Also applies to: 252-261
| export const OffsetTriggerDTOInstances = [ | ||
| OnFormSubmittedNoEventTriggerDto, | ||
| OnBeforeEventTriggerDto, | ||
| OnAfterEventTriggerDto, | ||
| OnAfterCalVideoGuestsNoShowTriggerDto, | ||
| OnAfterEventTriggerDto, | ||
| ]; | ||
| export type OffsetTriggerDTOInstancesType = InstanceType<(typeof OffsetTriggerDTOInstances)[number]>; |
There was a problem hiding this comment.
Fix OffsetTriggerDTOInstances: remove duplicate and include hosts no‑show.
OnAfterEventTriggerDto is listed twice, and OnAfterCalVideoHostsNoShowTriggerDto is missing. This breaks offset detection for the hosts no‑show case.
-export const OffsetTriggerDTOInstances = [
- OnFormSubmittedNoEventTriggerDto,
- OnBeforeEventTriggerDto,
- OnAfterEventTriggerDto,
- OnAfterCalVideoGuestsNoShowTriggerDto,
- OnAfterEventTriggerDto,
-];
+export const OffsetTriggerDTOInstances = [
+ OnFormSubmittedNoEventTriggerDto,
+ OnBeforeEventTriggerDto,
+ OnAfterEventTriggerDto,
+ OnAfterCalVideoGuestsNoShowTriggerDto,
+ OnAfterCalVideoHostsNoShowTriggerDto,
+];📝 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.
| export const OffsetTriggerDTOInstances = [ | |
| OnFormSubmittedNoEventTriggerDto, | |
| OnBeforeEventTriggerDto, | |
| OnAfterEventTriggerDto, | |
| OnAfterCalVideoGuestsNoShowTriggerDto, | |
| OnAfterEventTriggerDto, | |
| ]; | |
| export type OffsetTriggerDTOInstancesType = InstanceType<(typeof OffsetTriggerDTOInstances)[number]>; | |
| export const OffsetTriggerDTOInstances = [ | |
| OnFormSubmittedNoEventTriggerDto, | |
| OnBeforeEventTriggerDto, | |
| OnAfterEventTriggerDto, | |
| OnAfterCalVideoGuestsNoShowTriggerDto, | |
| OnAfterCalVideoHostsNoShowTriggerDto, | |
| ]; | |
| export type OffsetTriggerDTOInstancesType = InstanceType<(typeof OffsetTriggerDTOInstances)[number]>; |
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/workflows/inputs/workflow-trigger.input.ts around
lines 293 to 300, the OffsetTriggerDTOInstances array incorrectly includes
OnAfterEventTriggerDto twice and is missing
OnAfterCalVideoHostsNoShowTriggerDto; update the array to remove the duplicate
OnAfterEventTriggerDto and add OnAfterCalVideoHostsNoShowTriggerDto in the
appropriate position so that host no‑show offset detection is included (no other
changes to the type alias are required).
| private _isOffsetTrigger( | ||
| trigger: UpdateEventTypeWorkflowDto["trigger"] | UpdateFormWorkflowDto["trigger"] | ||
| ): trigger is OffsetTriggerDTOInstancesType { | ||
| return OffsetTriggerDTOInstances.some((Instance) => trigger instanceof Instance); | ||
| } |
There was a problem hiding this comment.
Offset-trigger guard is brittle; also misses hosts no‑show. Prefer enum-based check with fallback.
instanceof on DTOs is unreliable and currently won’t detect OnAfterCalVideoHostsNoShowTriggerDto. Also, when updateDto.trigger is omitted, we should still detect offset via currentData.trigger.
Proposed fix: use enum membership as primary check, fall back to class check, and allow passing the current trigger.
@@
- private _isOffsetTrigger(
- trigger: UpdateEventTypeWorkflowDto["trigger"] | UpdateFormWorkflowDto["trigger"]
- ): trigger is OffsetTriggerDTOInstancesType {
- return OffsetTriggerDTOInstances.some((Instance) => trigger instanceof Instance);
- }
+ private _isOffsetTrigger(
+ trigger: UpdateEventTypeWorkflowDto["trigger"] | UpdateFormWorkflowDto["trigger"] | undefined,
+ currentTrigger?: WorkflowType["trigger"]
+ ): trigger is OffsetTriggerDTOInstancesType {
+ // Prefer discriminator/type when present
+ if (trigger && "type" in (trigger as any)) {
+ const enumType =
+ WORKFLOW_TRIGGER_TO_ENUM[(trigger as any).type as keyof typeof WORKFLOW_TRIGGER_TO_ENUM];
+ return ENUM_OFFSET_WORFLOW_TRIGGERS.includes(enumType);
+ }
+ // Fallback to class check (already transformed cases)
+ if (trigger) {
+ return OffsetTriggerDTOInstances.some((Instance) => trigger instanceof Instance);
+ }
+ // No DTO provided: use current stored trigger
+ if (currentTrigger) {
+ return ENUM_OFFSET_WORFLOW_TRIGGERS.includes(currentTrigger);
+ }
+ return false;
+ }Also add the missing import:
@@
-import {
- OffsetTriggerDTOInstances,
- OffsetTriggerDTOInstancesType,
- TIME_UNIT_TO_ENUM,
- WORKFLOW_TRIGGER_TO_ENUM,
-} from "../inputs/workflow-trigger.input";
+import {
+ OffsetTriggerDTOInstances,
+ OffsetTriggerDTOInstancesType,
+ TIME_UNIT_TO_ENUM,
+ WORKFLOW_TRIGGER_TO_ENUM,
+ ENUM_OFFSET_WORFLOW_TRIGGERS,
+} from "../inputs/workflow-trigger.input";📝 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.
| private _isOffsetTrigger( | |
| trigger: UpdateEventTypeWorkflowDto["trigger"] | UpdateFormWorkflowDto["trigger"] | |
| ): trigger is OffsetTriggerDTOInstancesType { | |
| return OffsetTriggerDTOInstances.some((Instance) => trigger instanceof Instance); | |
| } | |
| // apps/api/v2/src/modules/workflows/services/workflows.input.service.ts | |
| import { | |
| OffsetTriggerDTOInstances, | |
| OffsetTriggerDTOInstancesType, | |
| TIME_UNIT_TO_ENUM, | |
| WORKFLOW_TRIGGER_TO_ENUM, | |
| ENUM_OFFSET_WORFLOW_TRIGGERS, | |
| } from "../inputs/workflow-trigger.input"; | |
| private _isOffsetTrigger( | |
| trigger: UpdateEventTypeWorkflowDto["trigger"] | UpdateFormWorkflowDto["trigger"] | undefined, | |
| currentTrigger?: WorkflowType["trigger"] | |
| ): trigger is OffsetTriggerDTOInstancesType { | |
| // Prefer discriminator/type when present | |
| if (trigger && "type" in (trigger as any)) { | |
| const enumType = | |
| WORKFLOW_TRIGGER_TO_ENUM[(trigger as any).type as keyof typeof WORKFLOW_TRIGGER_TO_ENUM]; | |
| return ENUM_OFFSET_WORFLOW_TRIGGERS.includes(enumType); | |
| } | |
| // Fallback to class check (already transformed cases) | |
| if (trigger) { | |
| return OffsetTriggerDTOInstances.some((Instance) => trigger instanceof Instance); | |
| } | |
| // No DTO provided: use current stored trigger | |
| if (currentTrigger) { | |
| return ENUM_OFFSET_WORFLOW_TRIGGERS.includes(currentTrigger); | |
| } | |
| return false; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/features/bookings/repositories/BookingRepository.ts (1)
389-402: LGTM! Method implementation follows best practices.The method correctly:
- Uses
selectinstead ofinclude(per coding guidelines)- Selects only the minimal required field (
id)- Uses
findFirstappropriately for existence checking- Aligns with the PR objective to detect whether a booking exists for a routing form response
Optional enhancement: Consider adding an explicit return type annotation for clarity:
- async findFirstBookingFromResponse({ responseId }: { responseId: number }) { + async findFirstBookingFromResponse({ + responseId + }: { + responseId: number + }): Promise<{ id: number } | 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 (4)
packages/features/bookings/repositories/BookingRepository.ts(1 hunks)packages/features/ee/workflows/lib/reminders/reminderScheduler.ts(1 hunks)packages/lib/server/service/workflows.test.ts(2 hunks)packages/lib/server/service/workflows.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/lib/server/service/workflows.test.ts
- packages/features/ee/workflows/lib/reminders/reminderScheduler.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/features/bookings/repositories/BookingRepository.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/features/bookings/repositories/BookingRepository.tspackages/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/features/bookings/repositories/BookingRepository.tspackages/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/features/bookings/repositories/BookingRepository.tspackages/lib/server/service/workflows.ts
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
packages/lib/server/service/workflows.ts (1)
packages/platform/libraries/index.ts (1)
WorkflowTriggerEvents(35-35)
🔇 Additional comments (4)
packages/features/bookings/repositories/BookingRepository.ts (1)
389-402: Inconsistency in the AI-generated summary.The AI summary claims this PR "adds two identical asynchronous methods named
findFirstBookingFromResponse", but only one method definition is present in the code. This discrepancy should be noted.packages/lib/server/service/workflows.ts (3)
1-1: LGTM!The new imports are necessary to support the FORM_SUBMITTED_NO_EVENT workflow trigger functionality.
Also applies to: 5-5, 7-7
146-176: LGTM! Day.js usage is appropriate for this context.The implementation correctly:
- Filters workflows by the FORM_SUBMITTED_NO_EVENT trigger
- Computes
scheduledAttimestamps using Day.js with configurable time units and reasonable defaults (15 minutes)- Creates tasks with comprehensive payloads including all necessary context
- Uses
Promise.allto parallelize task creationNote: Day.js is instantiated once per workflow in the map. While technically in a loop, this is not a performance-critical hot path (form submission handlers typically process a small number of workflows), so the usage is appropriate. The fail-fast behavior of
Promise.allensures transactional consistency—if any task creation fails, the entire operation fails.
147-147: Upstream Zod schemas enforce validtimeUnitvalues, so the cast on line 147 is safe.
| responseId, | ||
| }: { | ||
| responseId: number; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that all callers have been updated with the new parameter.
The function signature now requires a responseId parameter. Ensure that all call sites of scheduleFormWorkflows have been updated to provide this value.
Run the following script to verify all call sites:
🏁 Script executed:
#!/bin/bash
# Description: Find all calls to scheduleFormWorkflows and verify they pass responseId
# Search for function calls
ast-grep --pattern 'scheduleFormWorkflows({$$$})'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate all calls to scheduleFormWorkflows across TS/JS files
rg -nP --type=ts --type=tsx --type=js --type=jsx 'scheduleFormWorkflows\s*\(' -nLength of output: 27
🏁 Script executed:
#!/bin/bash
# Find all mentions of scheduleFormWorkflows across TS/JS files
rg -n 'scheduleFormWorkflows' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'Length of output: 1372
🏁 Script executed:
#!/bin/bash
# Inspect call sites for responseId inclusion
echo "=== workflows.test.ts (line ~79) ==="
sed -n '75,90p' packages/lib/server/service/workflows.test.ts
echo
echo "=== workflows.test.ts (line ~126) ==="
sed -n '120,140p' packages/lib/server/service/workflows.test.ts
echo
echo "=== workflows.test.ts (line ~193) ==="
sed -n '190,210p' packages/lib/server/service/workflows.test.ts
echo
echo "=== formSubmissionUtils.ts (line ~223) ==="
sed -n '210,240p' packages/app-store/routing-forms/lib/formSubmissionUtils.tsLength of output: 2373
Include responseId in all scheduleFormWorkflows calls
- In packages/lib/server/service/workflows.test.ts, the invocations at lines ~79 and ~193 omit
responseId; update them to pass a valid number.
🤖 Prompt for AI Agents
In packages/lib/server/service/workflows.ts around lines 89 to 91, the
scheduleFormWorkflows signature requires a responseId:number but the test
invocations in packages/lib/server/service/workflows.test.ts (approx lines 79
and 193) omit it; update both calls to pass a valid numeric responseId (e.g., 1
or a test constant) and adjust any related test setup/mocks to expect/handle
that parameter.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts (2)
20-37: Consider dependency injection for repositories.For improved testability and flexibility, consider accepting repository instances as parameters or through a configuration object rather than instantiating them inline. This would make it easier to mock dependencies in tests.
Example refactor:
export async function shouldTriggerFormSubmittedNoEvent( options: ValidationOptions, dependencies?: { bookingRepository?: BookingRepository; formResponseRepository?: RoutingFormResponseRepository; } ) { const bookingRepository = dependencies?.bookingRepository ?? new BookingRepository(prisma); // ... rest of implementation }
12-15: Remove unused ValidationResult interface. The interface is exported but never referenced; delete it to reduce confusion.
📜 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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts(1 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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts
🧠 Learnings (4)
📓 Common learnings
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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.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/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts
🧬 Code graph analysis (1)
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts (2)
packages/platform/libraries/repositories.ts (1)
BookingRepository(4-4)packages/lib/server/repository/formResponse.ts (1)
RoutingFormResponseRepository(10-162)
⏰ 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: Production builds / Build Docs
🔇 Additional comments (2)
packages/features/tasker/tasks/triggerFormSubmittedNoEvent/formSubmissionValidation.ts (2)
5-10: LGTM!The interface is well-defined and appropriately captures the validation inputs. The
anytype forresponsesmaintains consistency with the existing form response handling patterns.
53-96: Verify response type coverage in duplicate check.
getSubmitterEmailhandles both string and object responses, buthasDuplicateSubmissionskips non-object responses.- Confirm whether
findAllResponsesWithBookingcan return plain-stringresponsevalues.- If so, extend
hasDuplicateSubmissionto detect duplicates in string responses as well.
…m#23716) * refactor: improve _scheduleWorkflowReminders readability and add missing 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> * only show customt emplate for form triggers * filter outside scheduleWorkflowReminder * fix type check * chore: add more tests * test: add comprehensive unit tests for handleMarkNoShow with webhook 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> * Revert "test: add comprehensive unit tests for handleMarkNoShow with webhook and workflow coverage" This reverts commit 7642992. * fix: add new workflow triggers to api/v2 * update swagger docs * fix: e2e * fix type check * fix tests, add test for before after events * fix unit tests * revert confirm.handler.test * fix: unit tests * dummy form variables * add routing forms to active on dropdown * add migration file * Ui fixes for variables dropdown * remove other translation keys * review fixes * allow routing forms for activeOn * use repository function to get routing forms * remove unnecessary code * adjust logic in update handler * add triggers to api v2 * remvoe unused file * rename to getAcitveOnOptions handler * remove routingFormOptions handler * clean up getActiveOnOptions * refactor WorkflowService * remove logs * remove unused * fix: type check * fix: missed before after events for recurring * fix: calendarEvent handleMarkNoShow * fix error message Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * don't query disabled routing forms * create tasker function * add tasker code * move isFormTrigger function * small adjustments + todo comments * remove email to host action for form triggers * throw trpc error if email to host is added as step * fix dialog on how to use form responses as variables * remove add variable dropdown for form triggers * remove form workfows in event workflows tab * improvements for workflow logic on form submission * review fixes * base setup for seperate schedule functions (evt and form) * add missing BOOKING_PAID workflow trigger * fix pathname * fix: test for BOOKING_REQUESTED * fix activeOn ids * pass hideBranding and smsReminderNumber * adjustments to reminderScheduler * create empty scheduelForForm functions * pass locale and timezone with form user * pass formData instead of responses * pass timeFormat and locale * reusable function for email sending and reminder creation * implement scheduleEmailReminderForForm * remove added editor field from merge conflict * don't support cal.ai action with form triggers * throw bad request if form trigger and cal.ai is combined * add tests for scheduleFormWorkflows * add form submission tests * remove form response varibe info * clean up workflow actions * fixes for getting template options * pass triggerType to getAllWorkflows * move reusable logic to scheduleSMSReminder * add formdata to param type * type fixes for text reminder managers * implement scheduleSMSReminderForForm * fix import * fix isAuthorizedToAddActiveOnIds * disble whatsapp action * implement triggerFormSubmittedNoEventWorkflow * code clean up * Merge branch 'devin/1755107037-add-workflow-triggers' into feat/routing-form-workflow-triggers * fix type errors * remove async from getSubmitterEmail * fix type errors * revert cal.ai changes * fix type error * add sublogger * code clean up * fix type errors * remove label for attendee whatsapp action * code clean up * fixes saving teams on org workflows * fix type error * code improvements for activeOn ids * Revert "code improvements for activeOn ids" This reverts commit 0a3590a. * improve variable name * fix unit tests * small fixes * type fixes * remove unused translation keys * fix merge conflict issues * code clean up * remove SMS action support * remove more SMS code * add missing imports * set custom template for form action * type fixes * fix tasker endpoint * fix duplicate check * fix workfows.test.ts * use repository funciton to getHideBranding * code clean up * fix hasDuplicateSubmission * code clean up * select only needed properties * remove repository functions * Revert "remove repository functions" This reverts commit 7aa47b1. * add scheduleWorkflows function * Revert "add scheduleWorkflows function" This reverts commit fe5db4f. * move type to /types * Revert "move type to /types" This reverts commit 91e0152. * revert changes causing type errors * remove import * remove unused import * Revert "remove unused import" This reverts commit 1916768. * revert changed from attempt to fix type errors * pass object to gt all workflows * fix isAuthorized check * trigger filtering * remove form submitted no event booked code * remove form submitted no event from schema * remove more code * remove test * fixes * add getSubmitterEmail function * add trigger * small fixes * add missing workflow DTOs * small fixes * use activeOnWithChildren * fix active on when switching trigger type * remove add variable dropdown * add getAllWorkflowsFromRoutingForm to WorkflowService * fix error caused by undefined evt * fix type error * fix type error * fix tests * code clean up * final fixes and clean up * remove console.log * remove template text form from triggers * add routing form repoditory function * fix bug with key * fix test * add missing trigger in update-workflow.input.ts * ForEvt and ForForm function for aiPhoneCallManager * chore: add support for form workflows on api v2 * fixup! chore: add support for form workflows on api v2 * use only repository functions in update handler * move all prisma queries from list.handler * review suggestions * chore: handle workflows api v2 * chore: handle workflows api v2, split in 2 endpoints * fix workflow step creation * remove connect agent and fixes types * add type to workflow * chore: use workflow type in apiv2 WorkflowsOutputService * update worklfow type on update * chore: use workflow type in apiv2 WorkflowsOutputService * fix template body for torm trigger * some UI fixes for email subject/body * resetting email body when changing form triggers * use type field to query workflows * clean up all old active on values * remove responseId from all funciton calls * remove undefined from updateTemplate * refactor: split routing form and event-type workflows code * refactor: split routing form and event-type workflows code * fix template text when adding action * chore: don't rename WorkflowActivationDto to avoid ci blocking * refine update schedule to use only allowed actions * fix type error * don't allow whatsapp action with form trigger * fix type error * return early if activeOn array is empty * fix: from step type in BaseFormWorkflowStepDto * fixup! fix: from step type in BaseFormWorkflowStepDto * api v2 updates * move all prisma calls to repository (service/workflows.ts) * use FORM_TRIGGER_WORKFLOW_EVENTS for form queries * use userRepository * use FORM_TRIGGER_WORKFLOW_EVENTS in isFormTrigger * code clean up * code clean up * use repository functions in formSubmissionValidation.ts * add back trpc import * fix agent repository functions * remove unsued import * fixes for offset api v2 * add missing responseId * fix failing test --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: amit@cal.com <samit91848@gmail.com> Co-authored-by: CarinaWolli <wollencarina@gmail.com> Co-authored-by: Amit Sharma <74371312+Amit91848@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com> Co-authored-by: Benny Joo <sldisek783@gmail.com> Co-authored-by: cal.com <morgan@cal.com> Co-authored-by: Morgan <33722304+ThyMinimalDev@users.noreply.github.com>
What does this PR do?
Adds new workflow trigger "Form submitted no event booked". That triggers X minutes after a form was submitted if no booking exists for the response.
Fixes CAL-6397
Fixes #23745
How should this be tested?
scheduledAtto a past date and callcurl http://localhost:3000/api/tasks/cronto trigger the task