Skip to content

[CLEAN] Synthetic Benchmark PR #26871 - refactor: migrate MembershipRole usages to PBAC permission checks#565

Open
ofir-frd wants to merge 1 commit intobase_pr_26871_20260120_2393from
clean_pr_26871_20260120_2393
Open

[CLEAN] Synthetic Benchmark PR #26871 - refactor: migrate MembershipRole usages to PBAC permission checks#565
ofir-frd wants to merge 1 commit intobase_pr_26871_20260120_2393from
clean_pr_26871_20260120_2393

Conversation

@ofir-frd
Copy link

Benchmark PR calcom#26871

Type: Clean (correct implementation)

Original PR Title: refactor: migrate MembershipRole usages to PBAC permission checks
Original PR Description: ## What does this PR do?

Refactors direct MembershipRole checks throughout the codebase to use the PBAC (Permission-Based Access Control) system via PermissionCheckService. This is part of the ongoing effort to centralize permission logic and enable fine-grained access control.

Changes include:

  • get.handler.ts: Renamed isTeamAdminOrOwnercanUpdateTeams using PBAC
  • checkForInvalidAppCredentials.ts: Uses getTeamIdsWithPermission for team credential access
  • outOfOffice.utils.ts: Uses ooo.update permission for admin checks
  • checkIfOrgNeedsUpgrade.handler.ts: Uses organization.manageBilling permission
  • getActiveOnOptions.handler.ts: Pre-fetches teams with eventType.update permission
  • WorkflowRepository.ts: Uses workflow.update permission for readOnly determination
  • organization.tsx: Uses organization.update permission for org management access
  • getEventTypesByViewer.ts: Uses eventType.update for managed event type filtering
  • getPublicEvent.ts: Uses team.read permission for private team member visibility

Updates since last revision

  • Fixed variable naming to match permission strings: renamed teamsWithEventTypeManagePermissionteamsWithEventTypeUpdatePermission and canManageEventTypescanUpdateEventTypes to align with the eventType.update permission being checked

Link to Devin run: https://app.devin.ai/sessions/b006afc07b43408e9ecd92da851aaff2
Requested by: @sean-brydon

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - internal refactoring only.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. API Response Change: The /api/trpc/viewer.me endpoint now returns canUpdateTeams instead of isTeamAdminOrOwner. Verify this doesn't break any frontend functionality.

  2. Permission Checks: Test the following scenarios with both PBAC enabled and disabled (fallback mode):

    • Out of office entry creation for team members
    • Workflow list filtering (readOnly status)
    • Event type management (managed event types visibility)
    • Organization settings page access
    • Private team member visibility on public event pages
  3. Fallback Behavior: When PBAC is not enabled for a team, the fallbackRoles should maintain the same behavior as the previous direct role checks.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my changes generate no new warnings

Items for Human Review

  • Breaking Change: isTeamAdminOrOwner renamed to canUpdateTeams in me endpoint - verify no other consumers depend on old name
  • Permission Strings: Verify team.read is the correct permission for private team member visibility (vs team.listMembersPrivate)
  • Fallback Roles: Confirm the fallback roles match the original role requirements in each case
    Original PR URL: refactor: migrate MembershipRole usages to PBAC permission checks calcom/cal.com#26871

@github-actions
Copy link

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "[CLEAN] Synthetic Benchmark PR #26871 - refactor: migrate MembershipRole usages to PBAC permission checks". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active.

@github-actions github-actions bot added the Stale label Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant