Conversation
Entelligence AI Vulnerability ScannerStatus: No security vulnerabilities found Your code passed our comprehensive security analysis. Analyzed 2 files in total |
Review Summary❌ Rejected Comments (1)
🏷️ Draft Comments (1)
|
🔬 Multi-Approach Review SummaryThis PR was reviewed by 2 different approaches for comparison:
Total: 2 review comments Each comment is labeled with its source approach. This allows you to compare different AI review strategies. 🔒 Security Scan: Run once and shared across all approaches for efficiency. WalkthroughThis PR optimizes organization membership permission checks by introducing an in-memory caching layer and refactoring existing query functions. The refactoring improves database query performance by selecting only necessary fields instead of full membership objects, while adding comprehensive JSDoc documentation for better code maintainability. A new caching mechanism with 5-minute TTL reduces database load for repeated membership lookups, featuring automatic expiration, cache invalidation, and monitoring capabilities. All permission check functions now use consistent boolean conversion patterns, maintaining existing functionality while significantly improving performance and code clarity. Changes
Sequence DiagramThis diagram shows the interactions between components: sequenceDiagram
participant Caller
participant OrgService as Organization Service
participant Prisma as Prisma ORM
participant DB as Database
Note over OrgService: Three membership check functions<br/>with optimized queries
rect rgb(200, 220, 255)
Note over Caller,DB: isOrganisationAdmin(userId, orgId)
Caller->>OrgService: isOrganisationAdmin(userId, orgId)
activate OrgService
OrgService->>Prisma: findFirst(where: userId, teamId, role: ADMIN|OWNER)
Note over Prisma: select: { id, role }
Prisma->>DB: Query membership table
DB-->>Prisma: membership record or null
Prisma-->>OrgService: membership
OrgService->>OrgService: Convert to boolean (!!membership)
OrgService-->>Caller: boolean (admin status)
deactivate OrgService
end
rect rgb(220, 255, 220)
Note over Caller,DB: isOrganisationOwner(userId, orgId)
Caller->>OrgService: isOrganisationOwner(userId, orgId)
activate OrgService
OrgService->>Prisma: findFirst(where: userId, teamId, role: OWNER)
Note over Prisma: select: { id }
Prisma->>DB: Query membership table
DB-->>Prisma: membership record or null
Prisma-->>OrgService: membership
OrgService->>OrgService: Convert to boolean (!!membership)
OrgService-->>Caller: boolean (owner status)
deactivate OrgService
end
rect rgb(255, 240, 220)
Note over Caller,DB: isOrganisationMember(userId, orgId)
Caller->>OrgService: isOrganisationMember(userId, orgId)
activate OrgService
OrgService->>Prisma: findUnique(where: userId_teamId composite key)
Note over Prisma: select: { id }
Prisma->>DB: Query membership table
DB-->>Prisma: membership record or null
Prisma-->>OrgService: membership
OrgService->>OrgService: Convert to boolean (!!membership)
OrgService-->>Caller: boolean (member status)
deactivate OrgService
end
Note over OrgService,Prisma: All queries now use select<br/>to fetch only required fields
🔒 Security Analysis
Note for WindsurfPlease change the default marketplace provider to the following in the windsurf settings:Marketplace Extension Gallery Service URL: https://marketplace.visualstudio.com/_apis/public/gallery Marketplace Gallery Item URL: https://marketplace.visualstudio.com/items Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
Also you can trigger various commands with the bot by doing The current supported commands are
More commands to be added soon. |
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
|
|
||
| // export type OrganisationWithMembers = Awaited<ReturnType<typeof getOrganizationMembers>>; | ||
|
|
||
| // also returns team | ||
| /** | ||
| * Checks if a user is an admin or owner of an organization | ||
| * @param userId - User ID to check | ||
| * @param orgId - Organization/Team ID | ||
| * @returns boolean indicating admin status | ||
| */ | ||
| export async function isOrganisationAdmin(userId: number, orgId: number) { | ||
| return ( | ||
| (await prisma.membership.findFirst({ | ||
| where: { | ||
| userId, | ||
| teamId: orgId, | ||
| OR: [{ role: MembershipRole.ADMIN }, { role: MembershipRole.OWNER }], | ||
| }, | ||
| })) || false | ||
| ); | ||
| const membership = await prisma.membership.findFirst({ | ||
| where: { | ||
| userId, | ||
| teamId: orgId, | ||
| OR: [{ role: MembershipRole.ADMIN }, { role: MembershipRole.OWNER }], | ||
| }, | ||
| select: { | ||
| id: true, | ||
| role: true, | ||
| }, | ||
| }); | ||
|
|
||
| return !!membership; | ||
| } | ||
| /** | ||
| * Checks if a user is an owner of an organization | ||
| * @param userId - User ID to check | ||
| * @param orgId - Organization/Team ID | ||
| * @returns boolean indicating owner status | ||
| */ | ||
| export async function isOrganisationOwner(userId: number, orgId: number) { | ||
| return !!(await prisma.membership.findFirst({ | ||
| const membership = await prisma.membership.findFirst({ | ||
| where: { | ||
| userId, | ||
| teamId: orgId, | ||
| role: MembershipRole.OWNER, | ||
| }, | ||
| })); | ||
| select: { | ||
| id: true, | ||
| }, | ||
| }); | ||
|
|
||
| return !!membership; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a user is a member of an organization | ||
| * @param userId - User ID to check | ||
| * @param orgId - Organization/Team ID | ||
| * @returns boolean indicating membership status | ||
| */ | ||
| export async function isOrganisationMember(userId: number, orgId: number) { | ||
| return !!(await prisma.membership.findUnique({ | ||
| const membership = await prisma.membership.findUnique({ | ||
| where: { | ||
| userId_teamId: { | ||
| userId, | ||
| teamId: orgId, | ||
| }, | ||
| }, | ||
| })); | ||
| select: { | ||
| id: true, | ||
| }, | ||
| }); | ||
|
|
||
| return !!membership; | ||
| } |
There was a problem hiding this comment.
Correctness: 🟠 [LangGraph v3] The async functions isOrganisationAdmin, isOrganisationOwner, and isOrganisationMember lack error handling. Without try/catch blocks, any errors during the database query will be unhandled, potentially causing the application to crash or behave unpredictably. Wrap the async operations in try/catch to handle potential errors gracefully.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // export type OrganisationWithMembers = Awaited<ReturnType<typeof getOrganizationMembers>>; | |
| // also returns team | |
| /** | |
| * Checks if a user is an admin or owner of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating admin status | |
| */ | |
| export async function isOrganisationAdmin(userId: number, orgId: number) { | |
| return ( | |
| (await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId: orgId, | |
| OR: [{ role: MembershipRole.ADMIN }, { role: MembershipRole.OWNER }], | |
| }, | |
| })) || false | |
| ); | |
| const membership = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId: orgId, | |
| OR: [{ role: MembershipRole.ADMIN }, { role: MembershipRole.OWNER }], | |
| }, | |
| select: { | |
| id: true, | |
| role: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } | |
| /** | |
| * Checks if a user is an owner of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating owner status | |
| */ | |
| export async function isOrganisationOwner(userId: number, orgId: number) { | |
| return !!(await prisma.membership.findFirst({ | |
| const membership = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId: orgId, | |
| role: MembershipRole.OWNER, | |
| }, | |
| })); | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } | |
| /** | |
| * Checks if a user is a member of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating membership status | |
| */ | |
| export async function isOrganisationMember(userId: number, orgId: number) { | |
| return !!(await prisma.membership.findUnique({ | |
| const membership = await prisma.membership.findUnique({ | |
| where: { | |
| userId_teamId: { | |
| userId, | |
| teamId: orgId, | |
| }, | |
| }, | |
| })); | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } | |
| import { MembershipRole } from "@calcom/prisma/enums"; | |
| // export type OrganisationWithMembers = Awaited<ReturnType<typeof getOrganizationMembers>>; | |
| /** | |
| * Checks if a user is an admin or owner of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating admin status | |
| */ | |
| export async function isOrganisationAdmin(userId: number, orgId: number) { | |
| try { | |
| const membership = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId: orgId, | |
| OR: [{ role: MembershipRole.ADMIN }, { role: MembershipRole.OWNER }], | |
| }, | |
| select: { | |
| id: true, | |
| role: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } catch (error) { | |
| console.error('Error checking admin status:', error); | |
| return false; | |
| } | |
| } | |
| /** | |
| * Checks if a user is an owner of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating owner status | |
| */ | |
| export async function isOrganisationOwner(userId: number, orgId: number) { | |
| try { | |
| const membership = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| teamId: orgId, | |
| role: MembershipRole.OWNER, | |
| }, | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } catch (error) { | |
| console.error('Error checking owner status:', error); | |
| return false; | |
| } | |
| } | |
| /** | |
| * Checks if a user is a member of an organization | |
| * @param userId - User ID to check | |
| * @param orgId - Organization/Team ID | |
| * @returns boolean indicating membership status | |
| */ | |
| export async function isOrganisationMember(userId: number, orgId: number) { | |
| try { | |
| const membership = await prisma.membership.findUnique({ | |
| where: { | |
| userId_teamId: { | |
| userId, | |
| teamId: orgId, | |
| }, | |
| }, | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!membership; | |
| } catch (error) { | |
| console.error('Error checking membership status:', error); | |
| return false; | |
| } | |
| } |
| import type { MembershipRole } from "@calcom/prisma/enums"; | ||
|
|
||
| /** | ||
| * In-memory cache for organization membership queries | ||
| * This helps reduce database load for frequently accessed membership data | ||
| */ | ||
|
|
||
| interface MembershipCacheEntry { | ||
| userId: number; | ||
| teamId: number; | ||
| role: MembershipRole; | ||
| timestamp: number; | ||
| } | ||
|
|
||
| const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes | ||
| const membershipCache = new Map<string, MembershipCacheEntry>(); | ||
|
|
||
| /** | ||
| * Generates cache key for membership lookup | ||
| */ | ||
| function getCacheKey(userId: number, teamId: number): string { | ||
| return `${userId}:${teamId}`; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if cache entry is still valid | ||
| */ | ||
| function isCacheValid(entry: MembershipCacheEntry): boolean { | ||
| return Date.now() - entry.timestamp <= CACHE_TTL_MS; | ||
| } | ||
|
|
||
| /** | ||
| * Gets cached membership data if available and valid | ||
| */ | ||
| export function getCachedMembership(userId: number, teamId: number): MembershipCacheEntry | null { | ||
| const key = getCacheKey(userId, teamId); | ||
| const entry = membershipCache.get(key); | ||
|
|
||
| if (!entry) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!isCacheValid(entry)) { | ||
| membershipCache.delete(key); | ||
| return null; | ||
| } | ||
|
|
||
| return entry; | ||
| } | ||
|
|
||
| /** | ||
| * Caches membership data | ||
| */ | ||
| export function setCachedMembership( | ||
| userId: number, | ||
| teamId: number, | ||
| role: MembershipRole | ||
| ): void { | ||
| const key = getCacheKey(userId, teamId); | ||
| membershipCache.set(key, { | ||
| userId, | ||
| teamId, | ||
| role, | ||
| timestamp: Date.now(), | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Invalidates cached membership for a user-team pair | ||
| */ | ||
| export function invalidateMembershipCache(userId: number, teamId: number): void { | ||
| const key = getCacheKey(userId, teamId); | ||
| membershipCache.delete(key); | ||
| } | ||
|
|
||
| /** | ||
| * Clears all expired cache entries | ||
| */ | ||
| export function cleanupExpiredCache(): number { | ||
| let removedCount = 0; | ||
|
|
||
| for (const [key, entry] of membershipCache.entries()) { | ||
| if (!isCacheValid(entry)) { | ||
| membershipCache.delete(key); | ||
| removedCount++; | ||
| } | ||
| } | ||
|
|
||
| return removedCount; | ||
| } | ||
|
|
||
| /** | ||
| * Gets current cache statistics | ||
| */ | ||
| export function getCacheStats() { | ||
| return { | ||
| size: membershipCache.size, | ||
| ttlMs: CACHE_TTL_MS, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Correctness: 🟠 [LangGraph v3] The getCacheStats function should have an explicit return type for better type safety and clarity.
1 participant
This PR optimizes organization membership queries by adding selective field fetching, improved documentation, and an in-memory caching layer to reduce database load for frequently accessed membership data.
EntelligenceAI PR Summary
Optimizes organization membership permission checks through query refactoring and introduces an in-memory caching layer to reduce database load.
isOrganisationAdmin,isOrganisationOwner,isOrganisationMember) to select only necessary fields!!) for boolean conversion across all functions