perf: Replace isTeamMember util with an index DB call#24066
Conversation
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 |
hbjORbj
changed the title
isTeamMember utilisTeamMember util with an index DB call
hbjORbj
force-pushed
the
perf/remove-isTeamMember
branch
from
7526044 to
45f02e8
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
dosubot
bot
added
the
teams
![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 (4)
packages/features/ee/teams/lib/queries.ts(0 hunks)packages/lib/server/repository/membership.ts(1 hunks)packages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.ts(2 hunks)packages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/features/ee/teams/lib/queries.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/lib/server/repository/membership.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.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/membership.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.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/lib/server/repository/membership.tspackages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.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.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-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/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts
📚 Learning: 2025-08-26T20:23:28.396Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Applied to files:
packages/trpc/server/routers/viewer/teams/getMemberAvailability.handler.ts
📚 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/trpc/server/routers/viewer/teams/getMemberAvailability.handler.tspackages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.ts (1)
packages/platform/libraries/index.ts (1)
TRPCError(66-66)
⏰ 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: Production builds / Build Web App
- GitHub Check: Production builds / Build API v2
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Production builds / Build API v1
🔇 Additional comments (1)
packages/trpc/server/routers/viewer/teams/getInternalNotesPresets.handler.ts (1)
17-25: Sameaccepted/findUniquebug applies here.This call hits the same repository bug noted in
getMemberAvailability.handler.ts: the helper still performsfindUniquewith anacceptedfilter, which Prisma rejects at runtime. Once the repository is updated to usefindFirst(or an equivalent fix), this guard will work; until then, this handler will throw on every invocation.
| static async findUniqueByUserIdAndTeamId({ | ||
| userId, | ||
| teamId, | ||
| accepted, | ||
| }: { | ||
| userId: number; | ||
| teamId: number; | ||
| accepted?: boolean; | ||
| }) { | ||
| return await prisma.membership.findUnique({ | ||
| where: { | ||
| userId_teamId: { | ||
| userId, | ||
| teamId, | ||
| }, | ||
| accepted, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix the Prisma findUnique misuse with the accepted filter.
prisma.membership.findUnique only accepts a unique selector; adding accepted alongside the userId_teamId composite key makes the query invalid (MembershipWhereUniqueInput rejects non-unique fields). With the current change, any call passing accepted: true will throw an Unknown arg 'accepted' error at runtime. Please switch to a findFirst (or split the check) so the optional accepted predicate is respected.
- return await prisma.membership.findUnique({
- where: {
- userId_teamId: {
- userId,
- teamId,
- },
- accepted,
- },
- });
+ return await prisma.membership.findFirst({
+ where: {
+ userId,
+ teamId,
+ ...(accepted !== undefined ? { accepted } : {}),
+ },
+ });📝 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 findUniqueByUserIdAndTeamId({ | |
| userId, | |
| teamId, | |
| accepted, | |
| }: { | |
| userId: number; | |
| teamId: number; | |
| accepted?: boolean; | |
| }) { | |
| return await prisma.membership.findUnique({ | |
| where: { | |
| userId_teamId: { | |
| userId, | |
| teamId, | |
| }, | |
| accepted, | |
| }, | |
| }); | |
| static async findUniqueByUserIdAndTeamId({ | |
| userId, | |
| teamId, | |
| accepted, | |
| }: { | |
| userId: number; | |
| teamId: number; | |
| accepted?: boolean; | |
| }) { | |
| return await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId, | |
| ...(accepted !== undefined ? { accepted } : {}), | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/lib/server/repository/membership.ts around lines 306 to 323, the
code misuses prisma.membership.findUnique by passing a non-unique field
(accepted) alongside the composite unique userId_teamId key which causes an
"Unknown arg 'accepted'" runtime error; change the call to
prisma.membership.findFirst (or equivalent) and move the selector into a where
object that includes userId, teamId, and the optional accepted predicate so the
optional accepted filter is honored without violating the unique-input contract.
| const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ | ||
| userId: ctx.user.id, | ||
| teamId: input.teamId, | ||
| accepted: true, | ||
| }); | ||
|
|
||
| if (!membership) { | ||
| throw new TRPCError({ code: "UNAUTHORIZED", message: "User is not a member of this team" }); | ||
| } |
There was a problem hiding this comment.
Fix membership lookup: accepted filter breaks findUnique.
MembershipRepository.findUniqueByUserIdAndTeamId currently forwards accepted to prisma.membership.findUnique. Prisma only allows unique fields inside findUnique, so the extra accepted immediately triggers Unknown arg 'accepted' in where.accepted for type MembershipWhereUniqueInput. As a result this guard will now throw for every request, blocking the handler.
Please switch the repository helper to use findFirst (or otherwise add a proper composite unique) before adding the accepted criterion. For example:
- return await prisma.membership.findUnique({
- where: {
- userId_teamId: {
- userId,
- teamId,
- },
- accepted,
- },
- });
+ return await prisma.membership.findFirst({
+ where: {
+ userId,
+ teamId,
+ ...(accepted !== undefined ? { accepted } : {}),
+ },
+ });Until that happens this handler will remain unusable.
Committable suggestion skipped: line range outside the PR's diff.
E2E results are ready! |
…)" This reverts commit 8ed0fb6.
4 participants
What does this PR do?
isTeamMemberutil and useMembershipRepository.findUniquecall insteadMandatory Tasks (DO NOT REMOVE)