feat: form submitted no event booked workflow trigger #2 #23716
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: [email protected] <[email protected]>
…webhook and workflow coverage" This reverts commit 7642992.
…ub.com/calcom/cal.com into devin/1755107037-add-workflow-triggers
…755107037-add-workflow-triggers
CarinaWolli
dismissed
Udit-takkar’s stale review
The base branch was changed.
github-actions
bot
added
High priority
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
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