fix: pbac being enabled when a team has ANY features

[sean-brydon]

What does this PR do?

Currently if a team has ANY features we are checking PBAC permissions in the UI to display the nav items.

How should this be tested?

1. Assign an org two random features (not pbac)

2. notice you do have access to a few of the settings nav items (previously this is hidden)

3. Assign an org two random features (including pbac)

4. Notive everything works as expected

[coderabbitai]

Walkthrough

The settings layout now loads permissions conditionally based on a PBAC feature flag. It first fetches features. If PBAC is enabled, it fetches rolePermissions and organizationPermissions in parallel and applies them. If PBAC is disabled or features are not loaded, it falls back: when features exist, canViewRoles derives from admin/owner status; when features are not loaded, canViewRoles defaults to false. This introduces a feature-flag gate and changes the permission-loading control flow. No exported/public declarations changed.

Possibly related PRs

• chore: add permissions.canUpdateOrganization to settings #24142 — Also updates settings/(settings-layout)/layout.tsx to gate PBAC permission loading and mapping (including canUpdateOrganization).
• fix: issues found in PBAC creation  #22659 — Adjusts PBAC-related permission loading in the same layout file, specifically affecting canViewRoles propagation.
• feat: pbac - org pages check update permissions #24164 — Refactors PBAC-based organization permission checks within settings layout/pages to use validateUserHasOrgPerms/validateUserHasOrg.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: pbac being enabled when a team has ANY features" directly relates to the main change in the changeset. The raw summary indicates the changeset reworks PBAC permission loading to be conditional on the PBAC feature flag, whereas previously permissions were fetched regardless. The title accurately captures the bug being fixed: PBAC permissions were incorrectly being checked whenever a team had any features, rather than only when PBAC was specifically enabled. The title is concise, uses the appropriate "fix:" prefix for a bug fix, and clearly indicates the nature of the problem to anyone reviewing commit history.
Description Check	✅ Passed	The pull request description is directly related to the changeset. It clearly explains the bug ("if a team has ANY features we are checking PBAC permissions"), provides specific test cases to validate the fix (assigning random features with and without PBAC), and aligns with the code changes that make permission loading conditional on the PBAC feature flag. The description includes actionable testing steps that demonstrate understanding of the issue and how to verify the fix works correctly.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/org-admin-nav-items-settings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx (2)

56-62: Fetch features first: correct sequencing; minor perf trade-off noted

Fetching features first prevents PBAC checks from running when not needed. This fixes the bug. If PBAC-enabled orgs are common and latency is critical, consider pre-starting permission fetches and only awaiting/using them when PBAC is true to hide the extra RTT; otherwise current approach reduces unnecessary DB load for PBAC-off orgs.

---

64-73: Proper PBAC gate around permission fetch/use

Gating both the fetch and application of PBAC permissions on features?.pbac === true resolves the “ANY feature enables PBAC” issue.

One follow-up to verify: the repository currently maps slugs to true and ignores the enabled flag (see packages/features/flags/features.repository.ts, Lines 67-89). If a PBAC relation exists but is disabled, features.pbac would still be truthy and keep PBAC enabled here. Recommend filtering on feature.enabled: true in the repository, e.g.:

// In FeaturesRepository.getTeamFeatures
await this.prismaClient.teamFeatures.findMany({
  where: { teamId, feature: { enabled: true } }, // ensure only enabled features
  include: { feature: { select: { slug: true } } }, // enabled no longer needed
});

📜 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 9133bc5 and 0d6578d.

📒 Files selected for processing (1)

• apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Always use t() for text localization in frontend code; direct text embedding should trigger a warning

Files:

• apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx

**/*.{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:

• apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx

**/*.{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:

• apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx

🧬 Code graph analysis (1)

apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx (1)

packages/features/flags/features.repository.ts (1)

• getTeamFeatures (68-90)

⏰ 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). (3)

• GitHub Check: Type check / check-types
• GitHub Check: Linters / lint
• GitHub Check: Tests / Unit

🔇 Additional comments (1)

apps/web/app/(use-page-wrapper)/settings/(settings-layout)/layout.tsx (1)

81-85: Legacy fallback is correct and conservative

When PBAC is off or features missing, falling back to admin/owner for all checks (and false for roles when no features loaded) is safe and matches expected behavior.

Please re-run the two test steps in the PR description to confirm visibility toggles match expectations.

[github-actions]

E2E results are ready!

• Workflow #83570.1 latest results