Conversation
PeerRich
commented
Sep 26, 2025
PeerRich
commented
📝 WalkthroughWalkthroughAdds Cal AI UI gated by IS_CALCOM (dropdown item and clickable badge navigating to a Cal AI workflow template). Introduces eventTypeId?: string to the workflow/new page props and enforces permission checks and association of created workflows to event types. Adds EventTypeRepository.isPersonalUserEventType and WorkflowRepository.getEventTypeIdsByWorkflowStepId. Extends CalAI phone agent creation APIs to accept userTimeZone (propagated through service layers) and enforces workflow.update authorization when updating workflows from agent creation. SessionStorage access wrapped in broader try/catch blocks. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
keithwillcode
added
core
dosubot
bot
added
event-types
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/modules/event-types/views/event-types-listing-view.tsx(2 hunks)packages/features/shell/navigation/NavigationItem.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/shell/navigation/NavigationItem.tsxapps/web/modules/event-types/views/event-types-listing-view.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/shell/navigation/NavigationItem.tsxapps/web/modules/event-types/views/event-types-listing-view.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/shell/navigation/NavigationItem.tsxapps/web/modules/event-types/views/event-types-listing-view.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
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.
📚 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/shell/navigation/NavigationItem.tsxapps/web/modules/event-types/views/event-types-listing-view.tsx
🧬 Code graph analysis (2)
packages/features/shell/navigation/NavigationItem.tsx (1)
packages/lib/constants.ts (1)
IS_CALCOM(43-49)
apps/web/modules/event-types/views/event-types-listing-view.tsx (2)
packages/lib/constants.ts (1)
IS_CALCOM(43-49)packages/ui/components/dropdown/Dropdown.tsx (2)
DropdownMenuItem(63-71)DropdownItem(161-181)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
| {IS_CALCOM && item.name === "workflows" && ( | ||
| <Link href="/workflow/new?action=calAi&templateWorkflowId=wf-11" > | ||
| <Badge startIcon="sparkles" variant="purple"> | ||
| Cal.ai | ||
| </Badge> | ||
| </Link> |
There was a problem hiding this comment.
Avoid nested links for the workflows badge.
Line 220 renders a Next.js Link inside another Link, creating nested <a> tags. That breaks HTML semantics, hurts accessibility/keyboard focus, and can confuse Radix’s menu handling. Please refactor so the badge triggers navigation without introducing a second anchor, and localize the label while you’re here.
-import Link from "next/link";
-import { usePathname } from "next/navigation";
+import Link from "next/link";
+import { usePathname, useRouter } from "next/navigation";
@@
- const { item, isChild } = props;
+ const { item, isChild } = props;
+ const router = useRouter();
@@
- {t(item.name)}
- {item.badge && item.badge}
+ {t(item.name)}
+ {IS_CALCOM && item.name === "workflows" ? (
+ <Badge
+ startIcon="sparkles"
+ variant="purple"
+ role="link"
+ tabIndex={0}
+ onClick={(event) => {
+ event.preventDefault();
+ event.stopPropagation();
+ router.push("/workflow/new?action=calAi&templateWorkflowId=wf-11");
+ }}
+ onKeyDown={(event) => {
+ if (event.key === "Enter" || event.key === " ") {
+ event.preventDefault();
+ event.stopPropagation();
+ router.push("/workflow/new?action=calAi&templateWorkflowId=wf-11");
+ }
+ }}>
+ {t("cal_ai")}
+ </Badge>
+ ) : (
+ item.badge && item.badge
+ )}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/features/shell/navigation/NavigationItem.tsx around lines 219 to
224, remove the nested Next.js Link by deleting the inner <Link
href="/workflow/new?action=calAi&templateWorkflowId=wf-11"> and instead make the
Badge itself trigger navigation via the router (use
useRouter().push('/workflow/new?action=calAi&templateWorkflowId=wf-11')); ensure
the Badge is accessible by adding role="link", tabIndex={0}, and handling
onKeyDown for Enter to call router.push as well; finally replace the hardcoded
"Cal.ai" label with a localized string (e.g. t('navigation.calAi') or the app's
i18n key) so the badge text is internationalized.
- Remove unused _error parameters from catch blocks - Add meaningful comments to catch blocks to avoid empty block warnings - Remove problematic eslint-disable comments for undefined rule - Fix nested Link component by using clickable div wrapper with proper accessibility Co-Authored-By: peer@cal.com <peer@cal.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
* feat: pass event type * feat: pass event type * refactor: use count * refactor: add event type * refactor: add event type * chore: undo
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
658-665: Implementation looks good; mobile dropdown still needs this item.The desktop dropdown implementation is correct:
- Proper localization using
t()- Appropriate feature gating with
IS_CALCOM- Good URL construction with
eventTypeIdand optionalteamIdHowever, as noted in a previous review, the mobile dropdown (sm:hidden menu) still lacks this Cal AI workflow entry, creating an inconsistent experience for mobile users.
🧹 Nitpick comments (4)
packages/lib/server/repository/eventTypeRepository.ts (1)
1426-1435: ConsiderfindFirstfor better performance.The logic is correct and secure. However,
count()scans all matching rows, whereasfindFirst()stops after finding the first match, making it more efficient for existence checks.Apply this diff to optimize the query:
async isPersonalUserEventType({ userId, eventTypeId }: { userId: number; eventTypeId: number }) { - const count = await this.prismaClient.eventType.count({ + const eventType = await this.prismaClient.eventType.findFirst({ where: { id: eventTypeId, userId, }, + select: { id: true }, }); - return count > 0; + return !!eventType; }apps/web/app/(use-page-wrapper)/workflow/new/page.tsx (2)
77-78: Validate eventTypeId before parsing to avoid silent skipsGuard against non-numeric inputs so invalid values don’t get silently ignored.
Apply this minimal change:
-const parsedEventTypeId = eventTypeId ? parseInt(eventTypeId, 10) : undefined; +const parsedEventTypeId = + typeof eventTypeId === "string" && /^\d+$/.test(eventTypeId) + ? parseInt(eventTypeId, 10) + : undefined;
152-159: Linking: consider idempotency and atomicity
- Prefer an idempotent write (e.g., upsert or ignore-on-duplicate) to avoid unique constraint errors or duplicates on retries.
- Consider a DB transaction to atomically create the workflow and its association; otherwise, a failure here leaves an unattached workflow.
packages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.ts (1)
28-28: LGTM! Timezone propagation looks correct.The change correctly propagates the user's timezone to the agent creation service with appropriate fallback handling.
Consider using UTC as a more neutral default instead of "Europe/London" for a global application, unless there's a specific reason for this choice.
- userTimeZone: ctx.user.timeZone ?? "Europe/London", + userTimeZone: ctx.user.timeZone ?? "UTC",
📜 Review details
Configuration used: CodeRabbit UI
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/web/app/(use-page-wrapper)/workflow/new/page.tsx(4 hunks)apps/web/modules/event-types/views/event-types-listing-view.tsx(2 hunks)packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts(1 hunks)packages/features/calAIPhone/providers/retellAI/services/AgentService.ts(4 hunks)packages/lib/server/repository/eventTypeRepository.ts(1 hunks)packages/lib/server/repository/workflow.ts(3 hunks)packages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*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/eventTypeRepository.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/eventTypeRepository.tspackages/lib/server/repository/workflow.tspackages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.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/eventTypeRepository.tspackages/lib/server/repository/workflow.tsapps/web/app/(use-page-wrapper)/workflow/new/page.tsxpackages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.tsapps/web/modules/event-types/views/event-types-listing-view.tsx
**/*.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:
apps/web/app/(use-page-wrapper)/workflow/new/page.tsxapps/web/modules/event-types/views/event-types-listing-view.tsx
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
🧠 Learnings (9)
📓 Common learnings
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.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/lib/schema.ts:41-43
Timestamp: 2025-08-14T10:30:23.062Z
Learning: In calcom/cal.com workflows with CAL_AI_PHONE_CALL actions, agentId can legitimately be null during initial setup. The workflow is: 1) save workflow to get step ID, 2) use step ID to create and link agent. Therefore, agentId should remain nullable in schemas to support this multi-step setup process (per maintainer Udit-takkar).
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:22:47.013Z
Learning: In calcom/cal.com, the executeAIPhoneCall task in packages/features/tasker/tasks/executeAIPhoneCall.ts is executed internally by the workflow system and not accessible externally, so RBAC checks on eventTypeId within this internal execution path are not required for security (per maintainer Udit-takkar in PR #22995).
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/lib/server/repository/eventTypeRepository.tsapps/web/app/(use-page-wrapper)/workflow/new/page.tsx
📚 Learning: 2025-08-14T10:48:52.586Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/ai/_router.ts:46-84
Timestamp: 2025-08-14T10:48:52.586Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/ai/_router.ts, the voiceId input parameter in the create endpoint is intentionally not forwarded to aiService.createAgent() as voice customization is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.ts
📚 Learning: 2025-09-22T12:47:33.011Z
Learnt from: Udit-takkar
PR: calcom/cal.com#23890
File: packages/features/calAIPhone/providers/retellAI/RetellAIService.ts:185-185
Timestamp: 2025-09-22T12:47:33.011Z
Learning: In calcom/cal.com, the userTimeZone parameter in createInboundAgent() is intentionally not forwarded to aiConfigurationService.setupInboundAIConfiguration() as timezone is not required for inbound agent AI configuration setup (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality.
Applied to files:
packages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.tspackages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-17T22:00:16.329Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts:117-126
Timestamp: 2025-08-17T22:00:16.329Z
Learning: In calcom/cal.com PR #22995, packages/trpc/server/routers/viewer/aiVoiceAgent/_router.ts, the enabled input parameter in the update endpoint is intentionally not forwarded to aiService.updateAgentConfiguration() as the enabled/disabled agent functionality is not required at the moment (per maintainer Udit-takkar). Future reviews should not flag this as missing functionality unless requirements change.
Applied to files:
packages/trpc/server/routers/viewer/aiVoiceAgent/create.handler.ts
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.tspackages/features/calAIPhone/providers/retellAI/services/AgentService.ts
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
packages/features/calAIPhone/providers/retellAI/services/AgentService.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:
apps/web/modules/event-types/views/event-types-listing-view.tsx
🔇 Additional comments (7)
apps/web/modules/event-types/views/event-types-listing-view.tsx (1)
18-18: LGTM!The
IS_CALCOMimport is correctly added and used to gate the Cal AI feature below.packages/lib/server/repository/workflow.ts (1)
264-264: Verify the filter truthiness change doesn't alter query behavior.The change from null-safe checks (possibly
filters.teamIds?.length) to direct truthiness (filters.teamIds) means empty arrays[]are now truthy and will pass the condition. This results inid: { in: [] }clauses, which match nothing in SQL. If the previous logic skipped the filter for empty arrays, this changes query behavior.Run the following script to examine the previous implementation and confirm this change is intentional:
Also applies to: 280-280
apps/web/app/(use-page-wrapper)/workflow/new/page.tsx (1)
41-49: LGTM: eventTypeId plumbing into PageProps/searchParamsThe additional param and wiring look correct for downstream use.
packages/features/calAIPhone/providers/retellAI/RetellAIPhoneServiceProvider.ts (1)
213-219: LGTM! Verify all call sites have been updated.The addition of the required
userTimeZoneparameter aligns with the broader integration for timezone-aware tool updates. The type system enforces that all callers must provide this parameter.Run the following script to verify all call sites have been updated:
packages/features/calAIPhone/providers/retellAI/services/AgentService.ts (3)
5-9: LGTM!The imported utilities are used appropriately in the authorization logic below.
452-468: LGTM!The
userTimeZoneparameter addition enables timezone-aware tool configuration, and theactiveEventTypeIdsinitialization properly supports the conditional workflow logic.
483-503: Authorization check enhances security.The workflow permission check correctly prevents unauthorized agent creation. However, verify that the downstream tool updates maintain proper authorization.
Based on retrieved learnings,
updateToolsFromAgentId()calls require verification of both agent and eventType ownership. Run the following script to confirmupdateToolsFromAgentIdincludes eventType ownership checks:
| if (parsedEventTypeId) { | ||
| const permissionService = new PermissionCheckService(); | ||
| let hasPermission = false; | ||
|
|
||
| if (user.org?.id) { | ||
| hasPermission = await permissionService.checkPermission({ | ||
| userId, | ||
| teamId: user.org?.id, | ||
| permission: "eventType.update", | ||
| fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }); | ||
| } else if (parsedTeamId) { | ||
| hasPermission = await permissionService.checkPermission({ | ||
| userId, | ||
| teamId: parsedTeamId, | ||
| permission: "eventType.update", | ||
| fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER], | ||
| }); | ||
| } else { | ||
| const eventTypeRepo = new EventTypeRepository(prisma); | ||
| hasPermission = await eventTypeRepo.isPersonalUserEventType({ | ||
| userId, | ||
| eventTypeId: parsedEventTypeId, | ||
| }); | ||
| } | ||
|
|
||
| if (!hasPermission) { | ||
| redirect("/workflows?error=unauthorized"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Missing ownership check on eventTypeId enables cross-team linking
Current check verifies the user can update some team/org, but not that the supplied eventTypeId actually belongs to that team/org or to the user. A user could pass an eventTypeId from another team while satisfying permission in their own org/team, leading to unauthorized linking.
Use the eventType’s actual owner (teamId/userId) to scope the permission:
-if (parsedEventTypeId) {
- const permissionService = new PermissionCheckService();
- let hasPermission = false;
-
- if (user.org?.id) {
- hasPermission = await permissionService.checkPermission({
- userId,
- teamId: user.org?.id,
- permission: "eventType.update",
- fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
- });
- } else if (parsedTeamId) {
- hasPermission = await permissionService.checkPermission({
- userId,
- teamId: parsedTeamId,
- permission: "eventType.update",
- fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
- });
- } else {
- const eventTypeRepo = new EventTypeRepository(prisma);
- hasPermission = await eventTypeRepo.isPersonalUserEventType({
- userId,
- eventTypeId: parsedEventTypeId,
- });
- }
-
- if (!hasPermission) {
- redirect("/workflows?error=unauthorized");
- }
-}
+if (parsedEventTypeId !== undefined) {
+ const eventType = await prisma.eventType.findUnique({
+ where: { id: parsedEventTypeId },
+ select: { id: true, teamId: true, userId: true },
+ });
+ if (!eventType) {
+ redirect("/workflows?error=invalidEventType");
+ }
+
+ let hasPermission = false;
+ if (eventType.teamId) {
+ const permissionService = new PermissionCheckService();
+ hasPermission = await permissionService.checkPermission({
+ userId,
+ teamId: eventType.teamId,
+ permission: "eventType.update",
+ fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
+ });
+ } else {
+ // Personal event type
+ hasPermission = eventType.userId === userId;
+ }
+
+ if (!hasPermission) {
+ redirect("/workflows?error=unauthorized");
+ }
+}This ensures the permission check is evaluated against the actual owner of the event type, preventing cross-tenant abuse. Based on learnings
| if (activeEventTypeIds.length > 0) { | ||
| await Promise.all( | ||
| activeEventTypeIds.map(async (eventTypeId) => { | ||
| try { | ||
| await this.updateToolsFromAgentId(agent.providerAgentId, { | ||
| eventTypeId, | ||
| timeZone: userTimeZone ?? "Europe/London", | ||
| userId, | ||
| teamId: teamId || undefined, | ||
| }); | ||
| } catch (error) { | ||
| const message = `${error instanceof Error ? error.message : "Unknown error"}`; | ||
| this.logger.error(message); | ||
| } | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Remove unnecessary nullish coalescing operator.
Since userTimeZone is typed as a required string parameter (line 462), the fallback ?? "Europe/London" at line 526 is unreachable and should be removed for clarity.
Apply this diff:
- timeZone: userTimeZone ?? "Europe/London",
+ timeZone: userTimeZone,The error handling strategy (catching and logging per-event-type failures) is appropriate, allowing partial success when some event types fail to update.
📝 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.
| if (activeEventTypeIds.length > 0) { | |
| await Promise.all( | |
| activeEventTypeIds.map(async (eventTypeId) => { | |
| try { | |
| await this.updateToolsFromAgentId(agent.providerAgentId, { | |
| eventTypeId, | |
| timeZone: userTimeZone ?? "Europe/London", | |
| userId, | |
| teamId: teamId || undefined, | |
| }); | |
| } catch (error) { | |
| const message = `${error instanceof Error ? error.message : "Unknown error"}`; | |
| this.logger.error(message); | |
| } | |
| }) | |
| ); | |
| } | |
| timeZone: userTimeZone, |
🤖 Prompt for AI Agents
In packages/features/calAIPhone/providers/retellAI/services/AgentService.ts
around lines 520 to 536, remove the unreachable nullish coalescing fallback (??
"Europe/London") used with the required string parameter userTimeZone (declared
at line 462); replace any expression that uses userTimeZone ?? "Europe/London"
with just userTimeZone so the code uses the guaranteed required value and the
fallback is not misleading.
| static async getEventTypeIdsByWorkflowStepId({ workflowStepId }: { workflowStepId: number }) { | ||
| const workflowStep = await prisma.workflowStep.findUnique({ | ||
| where: { id: workflowStepId }, | ||
| select: { workflow: { select: { id: true, activeOn: { select: { eventTypeId: true } } } } }, | ||
| }); | ||
|
|
||
| return { | ||
| workflowId: workflowStep?.workflow?.id ?? null, | ||
| eventTypeIds: workflowStep?.workflow?.activeOn.map((activeOn) => activeOn.eventTypeId) ?? [], | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add optional chaining before .map() to prevent TypeError.
Line 429 calls .map() on the result of workflowStep?.workflow?.activeOn without optional chaining. If workflowStep or workflow is null, the optional chain returns undefined, and calling .map() on undefined throws a TypeError before the ?? operator can provide the fallback.
Apply this diff to fix the issue:
return {
workflowId: workflowStep?.workflow?.id ?? null,
- eventTypeIds: workflowStep?.workflow?.activeOn.map((activeOn) => activeOn.eventTypeId) ?? [],
+ eventTypeIds: workflowStep?.workflow?.activeOn?.map((activeOn) => activeOn.eventTypeId) ?? [],
};📝 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.
| static async getEventTypeIdsByWorkflowStepId({ workflowStepId }: { workflowStepId: number }) { | |
| const workflowStep = await prisma.workflowStep.findUnique({ | |
| where: { id: workflowStepId }, | |
| select: { workflow: { select: { id: true, activeOn: { select: { eventTypeId: true } } } } }, | |
| }); | |
| return { | |
| workflowId: workflowStep?.workflow?.id ?? null, | |
| eventTypeIds: workflowStep?.workflow?.activeOn.map((activeOn) => activeOn.eventTypeId) ?? [], | |
| }; | |
| } | |
| return { | |
| workflowId: workflowStep?.workflow?.id ?? null, | |
| eventTypeIds: workflowStep?.workflow?.activeOn?.map((activeOn) => activeOn.eventTypeId) ?? [], | |
| }; |
| static async getEventTypeIdsByWorkflowStepId({ workflowStepId }: { workflowStepId: number }) { | |
| const workflowStep = await prisma.workflowStep.findUnique({ | |
| where: { id: workflowStepId }, | |
| select: { workflow: { select: { id: true, activeOn: { select: { eventTypeId: true } } } } }, | |
| }); | |
| return { | |
| workflowId: workflowStep?.workflow?.id ?? null, | |
| eventTypeIds: workflowStep?.workflow?.activeOn.map((activeOn) => activeOn.eventTypeId) ?? [], | |
| }; | |
| } | |
| return { | |
| workflowId: workflowStep?.workflow?.id ?? null, | |
| eventTypeIds: workflowStep?.workflow?.activeOn?.map((activeOn) => activeOn.eventTypeId) ?? [], | |
| }; |
🤖 Prompt for AI Agents
In packages/lib/server/repository/workflow.ts around lines 421 to 431, the code
calls .map() on workflowStep?.workflow?.activeOn which can be undefined and
cause a TypeError; change the call to use optional chaining before map (e.g.,
workflowStep?.workflow?.activeOn?.map(...)) and keep the existing nullish
fallback (or explicitly fallback to an empty array with ?? []) so .map() is only
invoked when activeOn is defined.
|
This PR is being marked as stale due to inactivity. |
3 participants
