feat(permissions): add useAccessibleOrgs hook and export getPermissionKeys - #1793
Conversation
…nKeys Signed-off-by: Rishi Raj <rishiraj438gt@gmail.com>
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesAccessible organization permissions
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant useAccessibleOrgs
participant useLazyGetUserKeysQuery
participant PermissionMatcher
Caller->>useAccessibleOrgs: provide organizations and permission specification
useAccessibleOrgs->>useLazyGetUserKeysQuery: query eligible organization user keys in parallel
useLazyGetUserKeysQuery-->>useAccessibleOrgs: return fulfilled or failed results
useAccessibleOrgs->>PermissionMatcher: evaluate permission keys and function names
PermissionMatcher-->>useAccessibleOrgs: return accessible organization IDs
useAccessibleOrgs-->>Caller: return accessible organizations and readiness flags
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Signed-off-by: Rishi Raj <180109771+rishiraj38@users.noreply.github.com>
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/custom/useAccessibleOrgs.ts`:
- Around line 95-99: Update the permission-checking flow in useAccessibleOrgs,
including the effect around checkedRef and the request logic around lines
101-137, to associate cached results and each request batch with the current
permissionKey. Clear checkedRef and checkedOrgs when permissionKey changes,
ignore responses belonging to older batches, and ensure isChecking is updated
only for the active batch.
- Around line 125-136: Update the results loop in the setCheckedOrgs callback to
record rejected checks in next using each request’s organization identifier and
a false permission value. Preserve the fulfilled-result handling and ensure
every organization marked as checked before the request reaches a checked state.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 22e283a7-4fd9-48d1-8e90-f6be0776e4b9
📒 Files selected for processing (3)
src/custom/permissions.tsxsrc/custom/useAccessibleOrgs.tssrc/index.tsx
| const [checkedOrgs, setCheckedOrgs] = useState<Map<string, boolean>>(new Map()); | ||
| const [isChecking, setIsChecking] = useState(false); | ||
|
|
||
| // Stable ref to avoid re-triggering the effect on every state update | ||
| const checkedRef = useRef<Set<string>>(new Set()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate permission results when the permission requirement changes.
checkedRef and checkedOrgs are keyed only by organization ID. When permissionKey changes, Line 107 suppresses new requests for already checked organizations. The hook can then return access results for the previous permission requirement.
Associate each request batch with its permission requirement. Clear the result cache when that requirement changes. Ignore results from an older batch after a newer requirement becomes active. Keep isChecking scoped to the active batch.
Also applies to: 101-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/custom/useAccessibleOrgs.ts` around lines 95 - 99, Update the
permission-checking flow in useAccessibleOrgs, including the effect around
checkedRef and the request logic around lines 101-137, to associate cached
results and each request batch with the current permissionKey. Clear checkedRef
and checkedOrgs when permissionKey changes, ignore responses belonging to older
batches, and ensure isChecking is updated only for the active batch.
There was a problem hiding this comment.
Good catch in theory, but in practice permissionKey is set once from the 403 page's permission shield and never changes during the component lifecycle. The useCallback dep array already includes permissionKey so if it did change, the effect re-fires. That said, I'll add a reset of checkedRef when permissionKey changes for correctness doesn't hurt.
There was a problem hiding this comment.
@rishiraj38, agreed. The 403-page lifecycle makes this low risk in the current caller.
If permissionKey can change, reset both checkedRef and checkedOrgs. Also guard in-flight responses so an older request cannot populate the cache after the new requirement becomes active. Resetting only checkedRef can still expose results from the previous requirement.
🐇
✏️ Learnings added
Learnt from: rishiraj38
Repo: layer5io/sistent PR: 1793
File: src/custom/useAccessibleOrgs.ts:95-99
Timestamp: 2026-08-12T19:55:05.061Z
Learning: In layer5io/sistent, `useAccessibleOrgs` is currently used from the 403-page permission shield, where `permissionKey` is normally fixed for the component lifecycle.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: layer5io/sistent PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-04T18:30:58.858Z
Learning: Applies to src/**/*.{ts,tsx} : Use the `Key` interface from `meshery/schemas/permissions`; do not define local permission keys or use/re-export generated `Keys` or `PermissionKeys` maps.
You are interacting with an AI system.
Signed-off-by: Rishi Raj <rishiraj438gt@gmail.com>
|
@rishiraj38, are we importing any RTK client packages from the |
|
I wonder how we are dealing with the balance of performance via caching and stale data via cache invalidation. @coderabbitai, thoughts? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Changed files ---'
git diff --name-only HEAD^ HEAD
printf '%s\n' '--- Hook outline ---'
ast-grep outline src/custom/useAccessibleOrgs.ts --view expanded || true
printf '%s\n' '--- Hook source ---'
cat -n src/custom/useAccessibleOrgs.ts
printf '%s\n' '--- Permission provider outline ---'
ast-grep outline src/custom/PermissionProvider.tsx --view expanded || true
printf '%s\n' '--- Permission query, cache, and hook usages ---'
rg -n -C 4 -i 'useAccessibleOrgs|cache|invalidate|permission.*key|getPermissionKeys|query' \
src test tests __tests__ 2>/dev/null || true
printf '%s\n' '--- Relevant diff ---'
git diff HEAD^ HEAD -- src/custom/useAccessibleOrgs.ts src/custom/PermissionProvider.tsx src/custom/permissions.tsx src/index.tsxLength of output: 191535
The hook clears this cache only when the
A failed request is also cached as I recommend an explicit freshness contract. Add a host-provided Also, callers should memoize For this 403-page path, I would not add polling or a long-lived global cache. An explicit invalidation signal provides predictable correctness with the existing low request volume. You are interacting with an AI system. |
Yes. The hook imports |
Good to hear! |
|
This affects Meshery UI, too, right? |
I shouldn't have used |
3 participants
Description
Add a shared
useAccessibleOrgshook that filters organizations based on whether the user holds a given permission key. This enables both Meshery Cloud and Meshery to show only accessible organizations in the org switcher on 403/permission-denied pages.Key Changes
useAccessibleOrgshook (src/custom/useAccessibleOrgs.ts): Accepts org data and apermissionKeyas parameters, queries/api/identity/orgs/:orgId/users/keysper-org in parallel viauseLazyGetUserKeysQuery, and returns only orgs where the user holds the required permission(s). Generic over the org element type so callers preserve their full typing (e.g.org.name,org.avatar).getPermissionKeysfromPermissionProvider— previously internal-only, now available for consumers that need to extract keys from aPermissionKeySpecwithout going through the CASL evaluator.useAccessibleOrgs,UseAccessibleOrgsOptions, andgetPermissionKeystosrc/index.tsx.Related Issues / PRs
Type of Change
Screen.Recording.2026-08-13.at.1.03.53.AM.mov
Signed commits
Summary by CodeRabbit