fix: Missing personal event types in All filter #23343
Conversation
|
@SinghaAnirban005 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughRefactors getEventTypeList in packages/features/insights/server/trpc-router.ts: function is exported and now accepts a Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
consumer
event-types
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/25/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/25/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/insights/server/trpc-router.ts (1)
1073-1104: Fix short-circuit that prevents the isAll branch from ever runningWhen isAll is true (org “All” context), callers commonly pass no teamId and no userId. The early-return guard returns [] before the isAll branch (Line 1103) is evaluated, breaking the PR objective.
Apply this minimal fix so org-wide queries can flow through:
- if (!teamId && !userId) { + // Allow org-wide "All" queries to continue even when both teamId and userId are absent + if (!teamId && !userId && !isAll) { return []; }
🧹 Nitpick comments (3)
packages/features/insights/server/trpc-router.ts (3)
1080-1101: Clarify/validate userId semantics in the “personal-only” branchThis branch ignores the input userId and always uses the authed user.id. If that’s intentional (security), make it explicit and guard against attempts to fetch someone else’s personal event types; otherwise, honor the provided userId with proper authorization.
Option A (make intent explicit and reject mismatched userIds):
- if (userId && !teamId && !isAll) { + if (userId && !teamId && !isAll) { + if (userId !== user.id) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + } const eventTypeResult = await prisma.eventType.findMany({ select: { id: true, slug: true, title: true, teamId: true, userId: true, team: { select: { name: true, }, }, }, where: { - userId: user.id, + userId: user.id, teamId: null, }, }); return eventTypeResult; }Option B (drop dependency on input userId entirely in this branch, since you already scope to the authed user):
- if (userId && !teamId && !isAll) { + if (!teamId && !isAll) { const eventTypeResult = await prisma.eventType.findMany({ ...Please confirm which behavior product expects.
1103-1146: “All” scope likely needs all org members’ personal event types, not just the current user’sYour org-wide (isAll) query adds only the authed user’s personal event types. Upstream, org-level booking filters (see buildBaseWhereCondition Lines 79–127) include personal bookings from all org users via usersFromOrg. If we don’t mirror that here, the Event Type filter won’t list many personal event types that actually show up in org analytics, limiting drill-down/filtering.
If product requirements confirm parity with booking scope, extend the OR to include personal event types of all accepted org members.
Proposed change:
if (isAll && user.organizationId && user.isOwnerAdminOfParentTeam) { const childTeams = await prisma.team.findMany({ where: { parentId: user.organizationId, }, select: { id: true, }, }); if (childTeams.length > 0) { childrenTeamIds = childTeams.map((team) => team.id); } + const teamIds = [user.organizationId, ...childrenTeamIds]; + const usersFromOrg = + teamIds.length > 0 + ? await prisma.membership.findMany({ + where: { + team: { id: { in: teamIds } }, + accepted: true, + }, + select: { userId: true }, + distinct: ["userId"], + }) + : []; const eventTypeResult = await prisma.eventType.findMany({ select: { id: true, slug: true, title: true, teamId: true, userId: true, team: { select: { name: true, }, }, }, where: { OR: [ { teamId: { - in: [user.organizationId, ...childrenTeamIds], + in: teamIds, }, }, - { - userId: user.id, - teamId: null, - }, + ...(usersFromOrg.length > 0 + ? [ + { + teamId: null, + userId: { in: usersFromOrg.map((u) => u.userId) }, + }, + ] + : []), ], }, }); return eventTypeResult; }Note: If privacy policy restricts listing other users’ personal event types to admins only, keep the admin check (already present) and proceed.
1081-1099: Add deterministic ordering to eventType listsUI filters benefit from stable ordering. Ordering by title (and optionally teamId) reduces flicker and improves UX.
Add orderBy to the three queries:
// personal-only branch const eventTypeResult = await prisma.eventType.findMany({ select: { ... }, where: { ... }, + orderBy: [{ title: "asc" }], }); // org-wide branch const eventTypeResult = await prisma.eventType.findMany({ select: { ... }, where: { ... }, + orderBy: [{ title: "asc" }], }); // fallback/member branch const eventTypeResult = await prisma.eventType.findMany({ select: { id: true, slug: true, title: true, teamId: true, userId: true, team: { select: { name: true } }, }, where: eventTypeWhereConditional, + orderBy: [{ title: "asc" }], });Also applies to: 1117-1143, 1181-1194
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- 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/insights/server/trpc-router.ts(3 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/insights/server/trpc-router.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/insights/server/trpc-router.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/insights/server/trpc-router.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-23T13:45:16.490Z
Learnt from: shaun-ak
PR: calcom/cal.com#23233
File: packages/features/users/components/UserTable/EditSheet/EditUserForm.tsx:100-101
Timestamp: 2025-08-23T13:45:16.490Z
Learning: In Cal.com's team structure, the organization team (root team) is identified by having no parentId (!team.parentId), while sub-teams within an organization have a parentId. Use selectedUser?.teams?.find((team) => !team.parentId) to get the organization team, not by matching org?.id.
Applied to files:
packages/features/insights/server/trpc-router.ts
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
Changes look good to me. Could you also add some tests to verify the fix?
SinghaAnirban005
force-pushed
the
bug/events
branch
from
2d45a63 to
e32ad0f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/insights/server/trpc-router.ts (1)
1103-1146: All context should respect selected user filter for personal eventsIn the org-admin “isAll” path, OR currently includes personal events for the authenticated user only. If a user filter is active, include that selected member’s personal event types instead.
Apply this diff:
- if (isAll && user.organizationId && user.isOwnerAdminOfParentTeam) { + if (isAll && user.organizationId && user.isOwnerAdminOfParentTeam) { const childTeams = await prisma.team.findMany({ where: { parentId: user.organizationId, }, select: { id: true, }, }); if (childTeams.length > 0) { childrenTeamIds = childTeams.map((team) => team.id); } const eventTypeResult = await prisma.eventType.findMany({ select: { id: true, slug: true, title: true, teamId: true, userId: true, team: { select: { name: true, }, }, }, where: { OR: [ { teamId: { in: [user.organizationId, ...childrenTeamIds], }, }, { - userId: user.id, + userId: userId ?? user.id, teamId: null, }, ], }, }); return eventTypeResult; }
♻️ Duplicate comments (1)
packages/features/insights/server/trpc-router.ts (1)
1174-1178: Include hosted event types and remove stale TODO (use hosts.some.userId)Event visibility for members should also include event types where the user is a host. This was raised earlier; the TODO should be resolved now.
Apply this diff:
if (isMember) { - eventTypeWhereConditional["OR"] = [{ userId: user.id }, { users: { some: { id: user.id } } }]; - // @TODO this is not working as expected - // hosts: { some: { id: user.id } }, + eventTypeWhereConditional["OR"] = [ + { userId: user.id }, + { users: { some: { id: user.id } } }, + { hosts: { some: { userId: user.id } } }, + ]; }If you want, I can run a verification script to confirm the relation path on EventType/Host.
#!/bin/bash # Verify EventType relations for 'users' and 'hosts', and Host fields rg -n -C2 -g '!**/node_modules/**' -P 'model\s+EventType\b' --type-add 'prisma:*.prisma' --type prisma rg -n -C2 -g '!**/node_modules/**' -P 'model\s+Host\b' --type prisma rg -n -C2 -g '!**/node_modules/**' -P '\bhosts\s*:' --type ts --type tsx
🧹 Nitpick comments (5)
packages/features/insights/server/trpc-router.ts (3)
1153-1158: Avoid querying membership with an empty where (could return arbitrary row)findFirst({}) may return an unrelated membership. Guard the call and also select only what you need.
Apply this diff:
- const membership = await prisma.membership.findFirst({ - where: membershipWhereConditional, - }); + const hasMembershipFilter = Object.keys(membershipWhereConditional).length > 0; + const membership = hasMembershipFilter + ? await prisma.membership.findFirst({ + where: { ...membershipWhereConditional, accepted: true }, + select: { role: true }, + }) + : null;
1180-1194: Deterministic ordering for stable UI (optional)Add an orderBy to stabilize the list for users; e.g., by team then title.
Apply this diff:
const eventTypeResult = await prisma.eventType.findMany({ select: { id: true, slug: true, title: true, teamId: true, userId: true, team: { select: { name: true, }, }, }, + orderBy: [{ teamId: "asc" }, { title: "asc" }], where: eventTypeWhereConditional, });
1056-1072: Minor: clarify the db type/name for readabilityThe parameter is typed as typeof readonlyPrisma but the caller passes ctx.prisma (writable). Consider renaming the argument to db and typing it to the minimal surface used (eventType, team, membership) to avoid confusion.
Example:
type Db = Pick<typeof readonlyPrisma, "eventType" | "team" | "membership">; export async function getEventTypeList({ prisma: db, ... }: { prisma: Db; ... }) { ... }packages/features/insights/server/__tests__/trpc-router.test.ts (2)
101-148: Add assertion for userId precedence in isAll=true pathTo cover the updated behavior, add a case where isAll is true and a different userId is provided; assert that the OR includes that selected userId for personal events.
Here’s a test you can append after the existing “Organization-wide view” test:
+ it("should include selected user's personal events when isAll=true and userId is provided", async () => { + const childTeams = [{ id: 11 }, { id: 12 }]; + vi.mocked(readonlyPrisma.team.findMany).mockResolvedValue(childTeams); + vi.mocked(readonlyPrisma.eventType.findMany).mockResolvedValue(mockEventTypes); + + const ownerUser = { ...mockUser, isOwnerAdminOfParentTeam: true }; + const result = await getEventTypeList({ + prisma: readonlyPrisma, + teamId: null, + userId: 999, + isAll: true, + user: ownerUser, + }); + + expect(readonlyPrisma.eventType.findMany).toHaveBeenCalledWith({ + select: { + id: true, + slug: true, + title: true, + teamId: true, + userId: true, + team: { select: { name: true } }, + }, + where: { + OR: [ + { teamId: { in: [10, 11, 12] } }, + { userId: 999, teamId: null }, + ], + }, + }); + expect(result).toEqual(mockEventTypes); + });
21-25: Optional: verify membership is not queried in contexts that don’t need itAdd expectations like expect(readonlyPrisma.membership.findFirst).not.toHaveBeenCalled() in “Organization-wide view” and the personal-events branch to prevent regressions where membership is queried with an empty where.
I can draft the exact expectations if you confirm the intended behavior.
Also applies to: 185-199
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/insights/server/__tests__/trpc-router.test.ts(1 hunks)packages/features/insights/server/trpc-router.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/insights/server/__tests__/trpc-router.test.tspackages/features/insights/server/trpc-router.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/insights/server/__tests__/trpc-router.test.tspackages/features/insights/server/trpc-router.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/insights/server/__tests__/trpc-router.test.tspackages/features/insights/server/trpc-router.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-23T13:45:16.490Z
Learnt from: shaun-ak
PR: calcom/cal.com#23233
File: packages/features/users/components/UserTable/EditSheet/EditUserForm.tsx:100-101
Timestamp: 2025-08-23T13:45:16.490Z
Learning: In Cal.com's team structure, the organization team (root team) is identified by having no parentId (!team.parentId), while sub-teams within an organization have a parentId. Use selectedUser?.teams?.find((team) => !team.parentId) to get the organization team, not by matching org?.id.
Applied to files:
packages/features/insights/server/trpc-router.ts
🧬 Code graph analysis (1)
packages/features/insights/server/__tests__/trpc-router.test.ts (2)
packages/features/insights/server/trpc-router.ts (1)
getEventTypeList(1056-1197)packages/prisma/index.ts (1)
readonlyPrisma(71-75)
⏰ 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). (2)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (1)
packages/features/insights/server/trpc-router.ts (1)
1118-1130: Good: using select over include and minimizing fieldsNice adherence to the guideline of selecting only required fields from Prisma. Keeps payloads lean and avoids accidental leakage.
Also applies to: 1181-1192
Signed-off-by: Anirban Singha <[email protected]>
Signed-off-by: Anirban Singha <[email protected]>
Signed-off-by: Anirban Singha <[email protected]>
SinghaAnirban005
force-pushed
the
bug/events
branch
from
0448774 to
8d3c15e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/features/insights/server/trpc-router.ts (1)
1174-1178: Include host-linked event types in member view and remove stale TODOEvent types where the user is a host are currently excluded. Add the hosts condition using userId (not id) and remove the TODO.
Apply:
- if (isMember) { - eventTypeWhereConditional["OR"] = [{ userId: user.id }, { users: { some: { id: user.id } } }]; - // @TODO this is not working as expected - // hosts: { some: { id: user.id } }, - } + if (isMember) { + eventTypeWhereConditional["OR"] = [ + { userId: user.id }, + { users: { some: { id: user.id } } }, + { hosts: { some: { userId: user.id } } }, + ]; + }
🧹 Nitpick comments (3)
packages/features/insights/server/trpc-router.ts (3)
1155-1161: Avoid findFirst() with an empty where; it can return unrelated membershipsWhen teamId is undefined and you’re not in the org-admin branch (future edits could make this reachable), membershipWhereConditional is {}. Prisma.findFirst({ where: {} }) returns the first row in the table, which is risky and wastes a query. Guard it.
- const membership = await prisma.membership.findFirst({ - where: membershipWhereConditional, - }); + const hasMembershipFilter = Object.keys(membershipWhereConditional).length > 0; + const membership = hasMembershipFilter + ? await prisma.membership.findFirst({ where: membershipWhereConditional }) + : null;Optionally, replace the generic Error with a TRPCError for consistency with the rest of this module:
- if (!membership && !user.isOwnerAdminOfParentTeam) { - throw new Error("User is not part of a team/org"); - } + if (!membership && !user.isOwnerAdminOfParentTeam) { + throw new TRPCError({ code: "UNAUTHORIZED" }); + }
1082-1093: DRY up the repeated select blocksThree identical select shapes are inlined. Extracting a shared constant reduces drift risk and eases future additions (e.g., adding team.slug later).
Add once (above the first query in this function):
+ const eventTypeSelect = { + id: true, + slug: true, + title: true, + teamId: true, + userId: true, + team: { select: { name: true } }, + } satisfies Prisma.EventTypeSelect;Then replace each inline select:
- select: { - id: true, - slug: true, - title: true, - teamId: true, - userId: true, - team: { select: { name: true } }, - }, + select: eventTypeSelect,Also applies to: 1119-1129, 1181-1192
1080-1101: Add a guard or test for userId ≠ ctx.user.id to avoid UX confusionToday you ignore the input userId for personal events (by design). If a caller passes a different userId, they’ll still get the current user’s personal event types. Consider either:
- returning [] when userId !== user.id, or
- documenting/typing the param to indicate it applies only to “current user.”
Do you want me to add a unit test asserting that passing a different userId still returns only the caller’s personal event types?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/insights/server/__tests__/trpc-router.test.ts(1 hunks)packages/features/insights/server/trpc-router.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/insights/server/tests/trpc-router.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/insights/server/trpc-router.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/insights/server/trpc-router.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/insights/server/trpc-router.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.383Z
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.
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
📚 Learning: 2025-08-26T08:08:23.383Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.383Z
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/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-23T13:45:16.490Z
Learnt from: shaun-ak
PR: calcom/cal.com#23233
File: packages/features/users/components/UserTable/EditSheet/EditUserForm.tsx:100-101
Timestamp: 2025-08-23T13:45:16.490Z
Learning: In Cal.com's team structure, the organization team (root team) is identified by having no parentId (!team.parentId), while sub-teams within an organization have a parentId. Use selectedUser?.teams?.find((team) => !team.parentId) to get the organization team, not by matching org?.id.
Applied to files:
packages/features/insights/server/trpc-router.ts
⏰ 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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (5)
packages/features/insights/server/trpc-router.ts (5)
1056-1072: Good extraction and clear, minimal surface for getEventTypeList()Exporting this helper with an explicit param shape (id, organizationId, isOwnerAdminOfParentTeam) keeps the contract tight and avoids over-exposing ctx.user. Nice.
1073-1075: Early-return guard prevents accidental “full-scan” callsThe empty-scope fast return avoids unnecessary DB hits and ambiguous behavior when no target is provided. Good defensive pattern.
1080-1101: Personal events branch correctly clamps to the authenticated userUsing ctx.user.id (not the input userId) for personal event types is the right security posture here. This aligns with our prior learning and prevents privilege escalation via arbitrary userId injection.
Note: I used the “retrieved learnings” you shared to validate this behavior.
1117-1146: Org “All” scope now includes current user’s personal event types — matches the issue intentThe OR combining org/child team event types with the caller’s personal event types addresses CAL-6297. Select clauses remain minimal and use select (not include), complying with Prisma guidelines.
1180-1196: Prisma usage: minimal select and no include — compliantThe final findMany adheres to our “select-only” guidance and returns just what the UI needs. Looks good.
|
@kart1ka Necessary tests have been added |
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your contribution 🙏.
E2E results are ready! |
| return eventTypeResult; | ||
| } | ||
|
|
||
| if (isAll && user.organizationId && user.isOwnerAdminOfParentTeam) { |
There was a problem hiding this comment.
any reason we don't check teamId here? (compared to the previous code)
never mind. it was not used before.
3 participants
What does this PR do?
This PR ensures that the personal users event types are listed when the "All" context is set
Visual Demo (For contributors especially)
Screencast.from.2025-08-25.22-45-01.webm
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?