Skip to content

Comments

chore: PBAC routingform entity permissions to favour pbac#24130

Merged
Udit-takkar merged 11 commits intomainfrom
chore/pbac-routing-form-entity-permissions
Sep 30, 2025
Merged

chore: PBAC routingform entity permissions to favour pbac#24130
Udit-takkar merged 11 commits intomainfrom
chore/pbac-routing-form-entity-permissions

Conversation

@sean-brydon
Copy link
Member

What does this PR do?

PR removes entity permissions to rely on new pbac permission system

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

Replaces legacy enum/role helpers with a new public Entity type and PBAC PermissionCheckService-based checks. Signatures changed to canEditEntity(entity: Entity, userId), canAccessEntity(entity: Entity, userId), and canCreateEntity({ targetTeamId, userId }); each calls PermissionCheckService with routingForm permission strings and explicit fallback roles when teamId is present. When no teamId, checks fall back to owner-only (userId === entity.userId). Removed getEntityPermissionLevel, getMembership, withRoleCanCreateEntity, and the ENTITY_PERMISSION_LEVEL enum. entityPrismaWhereClause remains exported. teamsAndUserProfilesQuery.handler now uses a MembershipRole runtime import and a rolesWithWriteAccess array for the non-PBAC path.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately identifies the primary change of migrating entity permission logic to use the PBAC system by referencing entity permissions and PBAC, which aligns with the refactored utilities and permission-check flow in the PR. Although the phrasing is somewhat redundant, it remains focused on the core update of favoring PBAC for entity permissions.
Description Check ✅ Passed The description succinctly states that the PR removes legacy entity permissions in favor of the new PBAC permission system, which directly matches the changeset’s objective of deprecating old utilities and routing permission checks through PBAC.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/pbac-routing-form-entity-permissions

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.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested a review from a team September 29, 2025 08:27
@keithwillcode keithwillcode added consumer core area: core, team members only labels Sep 29, 2025
Comment on lines 15 to 22
if (entity.teamId) {
const permissionService = new PermissionCheckService();
const hasEditPermission = await permissionService.checkPermission({
teamId: entity.teamId,
userId,
permission,
fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now simpily check if the user has permission on this teamId -> if pbac is disabled we use fallbackRoles

Comment on lines 75 to 99
async function getMembership(teamId: number | null, userId: number) {
const { default: prisma } = await import("@calcom/prisma");

const team = teamId
? await prisma.team.findFirst({
where: {
id: teamId,
members: {
some: {
userId,
accepted: true,
},
},
},
include: {
members: {
select: {
userId: true,
role: true,
},
},
},
})
: null;
return team?.members.find((membership) => membership.userId === userId);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used anymore

@dosubot dosubot bot added the platform Anything related to our platform plan label Sep 29, 2025
if (entity.userId === userId) return true;
}

export async function getEntityPermissionLevel(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not used anymore as PBAC logic handles all of this

@graphite-app graphite-app bot requested a review from a team September 29, 2025 08:28
@vercel
Copy link

vercel bot commented Sep 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 30, 2025 11:20am
cal-eu Ignored Ignored Sep 30, 2025 11:20am

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)

7-7: Fix: type-only import used as a value (will fail at runtime).

MembershipRole is imported with import type but used as a value at Lines 117-118. Import it as a value.

-import type { MembershipRole } from "@calcom/prisma/enums";
+import { MembershipRole } from "@calcom/prisma/enums";
🧹 Nitpick comments (6)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (3)

56-61: Prisma: select only what you use.

team.members { userId } and team.parentId are selected but unused in this handler. Drop them to reduce payload, per guidelines.

           metadata: true,
-          parentId: true,
           parent: {
             select: {
               logoUrl: true,
               name: true,
             },
           },
-          members: {
-            select: {
-              userId: true,
-            },
-          },

As per coding guidelines.

Also applies to: 49-49, 50-50


95-115: Simplify permission-filter path.

hasPermissionForFiltered becomes an array of true after filtering; it adds complexity without value. Set readOnly: false directly when withPermission is provided.

-  // Filter teams based on permission if provided
-  let hasPermissionForFiltered: boolean[] = [];
+  // Filter teams based on permission if provided
   if (input?.withPermission) {
     const permissionService = new PermissionCheckService();
     const { permission, fallbackRoles } = input.withPermission;

     const permissionChecks = await Promise.all(
       teamsData.map((membership) =>
         permissionService.checkPermission({
           userId: ctx.user.id,
           teamId: membership.team.id,
           permission: permission as PermissionString,
           fallbackRoles: fallbackRoles ? (fallbackRoles as MembershipRole[]) : [],
         })
       )
     );

-    // Store permission results for teams that passed the filter
-    hasPermissionForFiltered = permissionChecks.filter((hasPermission) => hasPermission);
     teamsData = teamsData.filter((_, index) => permissionChecks[index]);
   }
...
-      readOnly: input?.withPermission
-        ? !hasPermissionForFiltered[index]
-        : !noPbacFallbackRoles.includes(membership.role),
+      readOnly: input?.withPermission ? false : !noPbacFallbackRoles.includes(membership.role),

Also applies to: 137-139


80-81: Optional: safer metadata parsing.

teamMetadataSchema.parse(...) throws on invalid data and fails the entire request. Consider safeParse to skip invalid records instead of aborting.

- metadata: teamMetadataSchema.parse(membership.team.metadata),
+ metadata: (() => {
+   const r = teamMetadataSchema.safeParse(membership.team.metadata);
+   return r.success ? r.data : null;
+ })(),

Also applies to: 90-91

packages/lib/entityPermissionUtils.server.ts (3)

1-2: Minor: mark type-only import to avoid unnecessary runtime dependency.

PermissionString is used in types only; import it as a type.

-import { PermissionString } from "@calcom/features/pbac/domain/types/permission-registry";
+import type { PermissionString } from "@calcom/features/pbac/domain/types/permission-registry";

58-66: Nit: variable naming.

hasEditPermission in canCreateEntity reads oddly. Rename to hasCreatePermission for clarity.

-    const hasEditPermission = await permissionService.checkPermission({
+    const hasCreatePermission = await permissionService.checkPermission({
       teamId: targetTeamId,
       userId,
       permission,
       fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER],
     });
-
-    return hasEditPermission;
+    return hasCreatePermission;

2-2: Optional: reuse a single PermissionCheckService instance.

These functions construct a new service each call. Consider a shared instance to reduce per-call allocations.

+const permissionService = new PermissionCheckService();
...
-    const permissionService = new PermissionCheckService();
     const hasEditPermission = await permissionService.checkPermission({
...
-    const permissionService = new PermissionCheckService();
     const hasReadPermission = await permissionService.checkPermission({
...
-    const permissionService = new PermissionCheckService();
-    const hasEditPermission = await permissionService.checkPermission({
+    const hasCreatePermission = await permissionService.checkPermission({

Also applies to: 15-25, 35-44, 58-66

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between cc8a129 and 74e096e.

📒 Files selected for processing (2)
  • packages/lib/entityPermissionUtils.server.ts (1 hunks)
  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (2 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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
  • packages/lib/entityPermissionUtils.server.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
  • packages/lib/entityPermissionUtils.server.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
  • packages/lib/entityPermissionUtils.server.ts
🧠 Learnings (2)
📚 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/loggedInViewer/teamsAndUserProfilesQuery.handler.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/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
🧬 Code graph analysis (1)
packages/lib/entityPermissionUtils.server.ts (2)
packages/features/pbac/domain/types/permission-registry.ts (1)
  • PermissionString (63-63)
packages/features/pbac/services/permission-check.service.ts (1)
  • PermissionCheckService (19-306)
⏰ 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: Linters / lint
  • GitHub Check: Tests / Unit
🔇 Additional comments (1)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)

4-4: Ripgrep didn’t search any files (filters likely skipped everything)—please manually verify there are no remaining occurrences of withRoleCanCreateEntity in the codebase before removing the import.

Comment on lines 117 to 118
const noPbacFallbackRoles = [MembershipRole.ADMIN, MembershipRole.MEMBER] as MembershipRole[];

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bug: Fallback roles invert intent; OWNER excluded, MEMBER allowed to write.

When input.withPermission is absent, readOnly should align with “can create” fallback (OWNER & ADMIN allowed). Current list [ADMIN, MEMBER] makes OWNER read-only and MEMBER writable.

Align with canCreate/canEdit fallbacks in entityPermissionUtils.server.ts.

-const noPbacFallbackRoles = [MembershipRole.ADMIN, MembershipRole.MEMBER] as MembershipRole[];
+const noPbacFallbackRoles = [MembershipRole.OWNER, MembershipRole.ADMIN] as MembershipRole[];
...
-  readOnly: input?.withPermission
-    ? !hasPermissionForFiltered[index]
-    : !noPbacFallbackRoles.includes(membership.role),
+  readOnly: input?.withPermission
+    ? false // teams already filtered by permission; remaining entries are writable
+    : !noPbacFallbackRoles.includes(membership.role),

Also applies to: 139-140

🤖 Prompt for AI Agents
In
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
around lines 117-118 (and similarly at 139-140), the fallback role list is
inverted: it currently uses [MembershipRole.ADMIN, MembershipRole.MEMBER], which
wrongly treats OWNER as read-only and MEMBER writable; change the
noPbacFallbackRoles to only include MembershipRole.MEMBER so OWNER and ADMIN
remain writable (i.e., set noPbacFallbackRoles = [MembershipRole.MEMBER] in both
places to match the canCreate/canEdit fallbacks).

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2025

E2E results are ready!

}: {
targetTeamId: number | null | undefined;
userId: number;
permission?: PermissionString;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to pass permission argment to a fn that already is used only to check if entity can be created.

I think we can remove permission prop from here and instead directly pass permission:"routingForm.create" when calling checkPermission

export async function canAccessEntity(
entity: EntityPermission,
userId: number,
permission: PermissionString = "routingForm.read"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also, we don't need permission in the argument. canAccessEntity is only used to verify if the form can be read.

We can directly pass permission: "routingForm.read" to checkPermission

Comment on lines 37 to 45
const hasReadPermission = await permissionService.checkPermission({
teamId: entity.teamId,
userId,
permission,
fallbackRoles: [MembershipRole.ADMIN, MembershipRole.OWNER, MembershipRole.MEMBER],
});
const roleForTeamMember = membership?.role;

if (roleForTeamMember) {
//TODO: Remove type assertion
const hasWriteAccessToTeam = (
[MembershipRole.ADMIN, MembershipRole.OWNER] as unknown as MembershipRole
).includes(roleForTeamMember);
if (hasWriteAccessToTeam) {
return ENTITY_PERMISSION_LEVEL.TEAM_WRITE;
} else {
return ENTITY_PERMISSION_LEVEL.TEAM_READ_ONLY;
}
}
}

const ownedByUser = entity.userId === userId;
if (ownedByUser) {
return ENTITY_PERMISSION_LEVEL.USER_ONLY_WRITE;
}
return hasReadPermission;
} else if (entity.userId === userId) return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing the pattern repeating here that permissionService.checkPermission tells about permissions at team level and for a user entity we check if the userId of the entity is same as user taking action on it.

Would it not make sense to handle the user entity permission as well in checkPermission or create an abstraction over checkPermission ?

userId: Parameters<typeof getEntityPermissionLevel>[1]
entity: EntityPermission,
userId: number,
permission: PermissionString = "routingForm.update"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
permission: PermissionString = "routingForm.update"

const hasEditPermission = await permissionService.checkPermission({
teamId: entity.teamId,
userId,
permission,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
permission,
permission: "routingForm.update",

// All members of the team has access to it and user has write access to it
TEAM_WRITE,
}
export type EntityPermission = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Better name could be Entity only as it doesn't have any permission related thing. It just have required props that are needed to derive permissions

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great cleanup, but I have some requests

@github-actions github-actions bot marked this pull request as draft September 30, 2025 04:59
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/lib/entityPermissionUtils.server.ts (1)

9-23: Consider abstracting the repeated permission-check pattern.

All three functions (canEditEntity, canAccessEntity, canCreateEntity) follow the same pattern: instantiate PermissionCheckService, call checkPermission, and fall back to owner-only for personal entities. Consider introducing a helper function to reduce duplication and improve testability:

async function checkEntityPermission({
  entity,
  userId,
  permission,
  fallbackRoles,
}: {
  entity: Entity;
  userId: number;
  permission: PermissionString;
  fallbackRoles: MembershipRole[];
}): Promise<boolean> {
  if (entity.teamId) {
    const permissionService = new PermissionCheckService();
    return permissionService.checkPermission({
      teamId: entity.teamId,
      userId,
      permission,
      fallbackRoles,
    });
  }
  return entity.userId === userId;
}

Then refactor the three exported functions to call this helper with their specific permission strings and fallback roles.

Based on learnings

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between fa28c64 and 7018d09.

📒 Files selected for processing (1)
  • packages/lib/entityPermissionUtils.server.ts (2 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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/lib/entityPermissionUtils.server.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/entityPermissionUtils.server.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/entityPermissionUtils.server.ts
🧬 Code graph analysis (1)
packages/lib/entityPermissionUtils.server.ts (2)
packages/features/pbac/services/permission-check.service.ts (1)
  • PermissionCheckService (19-306)
packages/platform/libraries/index.ts (1)
  • MembershipRole (34-34)
🔇 Additional comments (4)
packages/lib/entityPermissionUtils.server.ts (4)

4-7: LGTM! Clean type definition.

The Entity type is clear and appropriate. The nullable fields correctly model entities that may belong to a user or a team.


25-39: Logic is correct; fallback roles appropriately include MEMBER.

The function correctly uses routingForm.read permission with fallback roles that include MEMBER, which is appropriate for read access (more permissive than edit). The logic is sound, though duplication remains as noted in the previous comment.


41-60: LGTM! Correctly allows personal entity creation.

The function correctly returns true when targetTeamId is absent, allowing any user to create personal entities. When a team is involved, it appropriately checks routingForm.create with fallback roles [ADMIN, OWNER].


62-81: LGTM! Where clause correctly filters by ownership and membership.

The clause correctly ensures that only entities belonging to the user or their accepted team memberships are fetched. The accepted: true check is important for security.

teamsData = teamsData.filter((_, index) => permissionChecks[index]);
}

const noPbacFallbackRoles = [MembershipRole.ADMIN, MembershipRole.MEMBER] as MembershipRole[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest naming it, rolesWithWriteAccess

readOnly: input?.withPermission
? !hasPermissionForFiltered[index]
: !withRoleCanCreateEntity(membership.role),
: !noPbacFallbackRoles.includes(membership.role),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: !noPbacFallbackRoles.includes(membership.role),
: !rolesWithWriteAccess.includes(membership.role),

It now identifies roles with readonly access.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)

116-117: Critical: Wrong roles in rolesWithWriteAccess - excludes OWNER, includes MEMBER.

The current list [MembershipRole.ADMIN, MembershipRole.MEMBER] inverts the intended permission model. This makes team owners read-only while allowing members to write, which contradicts the fallback behavior in canCreateEntity and canEditEntity (both allow OWNER and ADMIN only).

Apply this diff to correct the role list:

-  const rolesWithWriteAccess = [MembershipRole.ADMIN, MembershipRole.MEMBER] as MembershipRole[];
+  const rolesWithWriteAccess = [MembershipRole.OWNER, MembershipRole.ADMIN] as MembershipRole[];

As per coding guidelines

Also applies to: 138-138

🧹 Nitpick comments (1)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)

136-138: Clarify: The permission-path readOnly logic is always false.

When input?.withPermission is provided, line 137 evaluates !hasPermissionForFiltered[index]. However, hasPermissionForFiltered (line 112) only contains true values because it's filtered from permissionChecks. This means !hasPermissionForFiltered[index] is always false, making all permission-checked teams writable.

While this appears to be the intended behavior (teams passing the permission check should be writable), the logic is indirect and could confuse future maintainers.

Consider simplifying to make intent explicit:

      readOnly: input?.withPermission
-        ? !hasPermissionForFiltered[index]
+        ? false // All teams passing permission check are writable
        : !rolesWithWriteAccess.includes(membership.role),
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 7018d09 and fbf823d.

📒 Files selected for processing (1)
  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)
packages/platform/libraries/index.ts (1)
  • MembershipRole (34-34)
⏰ 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
🔇 Additional comments (1)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (1)

6-6: LGTM: Runtime import necessary for array literal.

The change from type-only to runtime import is correct since MembershipRole enum values are now used in the rolesWithWriteAccess array literal at line 116.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between fbf823d and 59798fa.

📒 Files selected for processing (1)
  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.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/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.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). (1)
  • GitHub Check: Production builds / Build Docs
🔇 Additional comments (2)
packages/trpc/server/routers/loggedInViewer/teamsAndUserProfilesQuery.handler.ts (2)

6-6: LGTM: Runtime import is correct.

Changing from a type-only import to a runtime import is necessary since MembershipRole is now used in the rolesWithWriteAccess array at runtime.


116-116: LGTM: Roles correctly define write access.

The rolesWithWriteAccess array correctly includes OWNER and ADMIN, aligning with the PBAC fallback model where these roles have create/edit permissions. This addresses the previous concern about inverted roles.

@Udit-takkar Udit-takkar merged commit 928d6f3 into main Sep 30, 2025
64 of 66 checks passed
@Udit-takkar Udit-takkar deleted the chore/pbac-routing-form-entity-permissions branch September 30, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only platform Anything related to our platform plan ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants