fix(ui): clarify Unlimited Visibility in RBAC forms#11851
Merged
HugoPBrito merged 11 commits intoJul 8, 2026
Conversation
Contributor
|
✅ All required changelog fragments are present. |
Contributor
|
✅ No Conflicts No conflict markers, and the branch merges cleanly into its base. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors role permission forms to use shared helpers and a reusable hook for ChangesUnlimited Visibility coupling and UI
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RoleForm
participant useManageProvidersUnlimitedVisibility
participant FormState
User->>RoleForm: toggle permission checkbox
RoleForm->>useManageProvidersUnlimitedVisibility: setPermissionValue(field, checked)
useManageProvidersUnlimitedVisibility->>FormState: setValue(field, checked)
User->>RoleForm: toggle "Manage Providers"
RoleForm->>useManageProvidersUnlimitedVisibility: setPermissionValue(manage_providers, checked)
useManageProvidersUnlimitedVisibility->>FormState: setValue(unlimited_visibility, true/false)
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 |
Contributor
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@ui/components/roles/workflow/forms/add-role-form.tsx`:
- Around line 155-181: The UnlimitedVisibilitySection block in AddRoleForm is
duplicated in EditRoleForm, so extract the checkbox plus conditional explanatory
paragraph into a shared component such as UnlimitedVisibilityField. Reuse that
component from both add-role-form.tsx and edit-role-form.tsx, passing the
existing state and handlers like isSelected, isDisabled, and onValueChange so
the compliance text and behavior stay in sync.
In `@ui/components/roles/workflow/forms/role-permissions.ts`:
- Around line 1-14: The shared role-permissions logic should not live under
forms/ because it is used by both add-role-form.tsx and edit-role-form.tsx. Move
the reusable hook useManageProvidersUnlimitedVisibility into a hooks/ module and
the pure helpers getVisiblePermissionFormFields and getUnlimitedVisibilityField
into lib/role-permissions.ts, then update the existing consumers to import from
those new locations while keeping the types and behavior unchanged.
- Around line 41-92: Avoid auto-enabling unlimited_visibility on initial mount;
the useEffect in useManageProvidersUnlimitedVisibility is rewriting existing
form data when manage_providers is already true and unlimited_visibility is
false. Update the effect to skip the first render or only react to user-driven
changes in manage_providers, while keeping setPermissionValue and
setUnlimitedVisibility responsible for syncing the dependent field during
interactions.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 843b49b5-4f86-47a0-9d8a-bfa571b1b1cd
📒 Files selected for processing (7)
ui/components/roles/workflow/forms/add-role-form.test.tsxui/components/roles/workflow/forms/add-role-form.tsxui/components/roles/workflow/forms/edit-role-form.test.tsxui/components/roles/workflow/forms/edit-role-form.tsxui/components/roles/workflow/forms/role-permissions.tsui/components/roles/workflow/forms/unlimited-visibility-section.tsxui/lib/helper.ts
github-actions
Bot
added
the
has-conflicts
…y-unlimited-visibility-rbac # Conflicts: # ui/CHANGELOG.md
…y-unlimited-visibility-rbac # Conflicts: # ui/CHANGELOG.md
github-actions
Bot
removed
the
has-conflicts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/components/roles/workflow/forms/edit-role-form.tsx (1)
206-211: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDrop
{...form.register(field)}from this controlled checkbox.
This checkbox is already controlled viaisSelected+onValueChange, so spreadingregisteradds a second change path and can bypass themanage_providers/unlimited_visibilitysync insetPermissionValue.♻️ Proposed change
<Checkbox - {...form.register(field as keyof FormValues)} isSelected={!!form.watch(field as keyof FormValues)} onValueChange={(checked) => setPermissionValue(field, checked) }🤖 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 `@ui/components/roles/workflow/forms/edit-role-form.tsx` around lines 206 - 211, The Checkbox in edit-role-form is being controlled twice by both form.register and the isSelected/onValueChange pair, which can interfere with the permission sync handled in setPermissionValue. Remove the spread registration from this Checkbox and keep it controlled only through form.watch and setPermissionValue so the manage_providers and unlimited_visibility updates remain the single source of truth.ui/components/roles/workflow/forms/unlimited-visibility-section.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHide decorative icon from assistive tech.
The
Eyeicon is purely decorative next to the "Unlimited Visibility" heading but lacksaria-hidden, unlikeInfoIconinadd-role-form.tsxwhich setsaria-hidden={"true"}. Screen readers may announce it as an unlabeled graphic.♿ Proposed fix
- <Eye className="mt-1 size-5 shrink-0 text-orange-700 dark:text-orange-300" /> + <Eye + aria-hidden="true" + className="mt-1 size-5 shrink-0 text-orange-700 dark:text-orange-300" + />🤖 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 `@ui/components/roles/workflow/forms/unlimited-visibility-section.tsx` at line 14, Add the same accessibility treatment used elsewhere for decorative icons by marking the Eye icon in unlimited-visibility-section.tsx as hidden from assistive tech. Update the Eye usage inside the Unlimited Visibility heading so it includes aria-hidden set to true, mirroring the InfoIcon pattern in add-role-form.tsx, and keep the rest of the heading markup unchanged.
🤖 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.
Outside diff comments:
In `@ui/components/roles/workflow/forms/edit-role-form.tsx`:
- Around line 206-211: The Checkbox in edit-role-form is being controlled twice
by both form.register and the isSelected/onValueChange pair, which can interfere
with the permission sync handled in setPermissionValue. Remove the spread
registration from this Checkbox and keep it controlled only through form.watch
and setPermissionValue so the manage_providers and unlimited_visibility updates
remain the single source of truth.
In `@ui/components/roles/workflow/forms/unlimited-visibility-section.tsx`:
- Line 14: Add the same accessibility treatment used elsewhere for decorative
icons by marking the Eye icon in unlimited-visibility-section.tsx as hidden from
assistive tech. Update the Eye usage inside the Unlimited Visibility heading so
it includes aria-hidden set to true, mirroring the InfoIcon pattern in
add-role-form.tsx, and keep the rest of the heading markup unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a719a0ad-fe4b-41eb-a256-4adfb6c3a609
📒 Files selected for processing (7)
ui/CHANGELOG.mdui/components/roles/workflow/forms/add-role-form.tsxui/components/roles/workflow/forms/edit-role-form.test.tsxui/components/roles/workflow/forms/edit-role-form.tsxui/components/roles/workflow/forms/unlimited-visibility-section.tsxui/hooks/use-manage-providers-unlimited-visibility.tsui/lib/role-permissions.ts
pfe-nazaries
requested changes
Jul 7, 2026
github-actions
Bot
added
the
has-conflicts
…y-unlimited-visibility-rbac # Conflicts: # ui/CHANGELOG.md
github-actions
Bot
removed
the
has-conflicts
- Own useForm, visibility state and server-error handling inside RoleForm - Have add/edit containers pass only defaultValues, submitText and onSubmit - Merge the identical add/edit role schemas into a single roleFormSchema - Replace HeroUI checkbox, divider, tooltip and input with shadcn equivalents
pfe-nazaries
previously approved these changes
Jul 7, 2026
Member
Author
Screen.Recording.2026-07-07.at.15.19.15.mov |
…y-unlimited-visibility-rbac # Conflicts: # ui/CHANGELOG.md
HugoPBrito
dismissed stale reviews from coderabbitai[bot] and pfe-nazaries
via
73c7bb2
pfe-nazaries
approved these changes
Jul 8, 2026
HugoPBrito
added
backport
Collaborator
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
prowler-bot
added
the
was-backported
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
RBAC role forms need clearer in-page guidance for Unlimited Visibility because it controls tenant-wide visibility and affects how role access behaves when provider groups are not enough.
Description
This PR improves the role create/edit forms by:
Steps to review
cd ui && pnpm test:unit components/roles/workflow/forms/add-role-form.test.tsx components/roles/workflow/forms/edit-role-form.test.tsxcd ui && pnpm run typecheckChecklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.