Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Walkthrough
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
| @@ -1,4 +1,5 @@ | |||
| //import "server-only"; | |||
| import type { LocationObject } from "@calcom/app-store/locations"; | |||
![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 (1)
packages/features/eventtypes/lib/getEventTypesByViewer.ts (1)
293-308: Enforce ‘eventType.read’ PBAC by gating returned eventTypes
Currently teams lackingeventType.readstill get theireventTypesloaded (onlyreadOnlyUI flag is set). Wrap the mapping in a permission check and return an empty list for unauthorized teams, and switch to aSetfor O(1) lookups:const permittedTeamIds = new Set(teamsWithEventTypeReadPermission); const canRead = permittedTeamIds.has(team.id); const eventTypes = canRead ? await Promise.all(team.eventTypes.map(mapEventType)) : []; return { metadata: { membershipCount: team.members.length, readOnly: !canRead, }, eventTypes: canRead ? eventTypes .filter(filterByTeamIds) .filter(evType => evType.userId === null || evType.userId === user.id) .filter(evType => membership.role === MembershipRole.MEMBER ? evType.schedulingType !== SchedulingType.MANAGED : true ) .filter(filterBySchedulingTypes) : [], };Alternatively, if product rules allow, omit the entire team group when
!canRead.
🧹 Nitpick comments (2)
packages/features/eventtypes/lib/getEventTypeById.ts (1)
2-2: Import consolidation looks good; minor type-safety follow‑up.Switching to a single
LocationObjectsource is correct. As a follow‑up, consider removing the double assertion at usage (locations as unknown as LocationObject[]) by tightening the repository return type so this cast isn’t needed.packages/features/eventtypes/lib/getEventTypesByViewer.ts (1)
56-63: Convert to a Set, usePermissionString, and guard empty results
- Wrap the array returned by
getTeamIdsWithPermission(...)innew Set(...)for O(1)haschecks.- Annotate (or build) the permission argument as
PermissionString—for example,${Resource.EventType}.${CrudAction.Read}—to catch typos at compile time.- Add a guard for an empty result (log a warning or fallback) to prevent silently disabling edit controls when no teams are returned.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/eventtypes/lib/getEventTypeById.ts(1 hunks)packages/features/eventtypes/lib/getEventTypesByViewer.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/eventtypes/lib/getEventTypesByViewer.tspackages/features/eventtypes/lib/getEventTypeById.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/eventtypes/lib/getEventTypesByViewer.tspackages/features/eventtypes/lib/getEventTypeById.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/eventtypes/lib/getEventTypesByViewer.tspackages/features/eventtypes/lib/getEventTypeById.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.805Z
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.
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.
📚 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/features/eventtypes/lib/getEventTypesByViewer.ts
📚 Learning: 2025-08-21T13:44:06.805Z
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.805Z
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/eventtypes/lib/getEventTypesByViewer.ts
🧬 Code graph analysis (1)
packages/features/eventtypes/lib/getEventTypesByViewer.ts (1)
packages/features/pbac/services/permission-check.service.ts (1)
PermissionCheckService(19-306)
E2E results are ready! |
3 participants
What does this PR do?
Uses the correct PBAC logic for setting metadata.readOnly