Fix: project settings pages permissions#6649
Conversation
WalkthroughThis pull request adds user permissions checks to two components. In the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant L as LabelDndHOC Component
participant P as useUserPermissions Hook
U->>L: Load component
L->>P: Retrieve user permissions
P-->>L: Return permission level (e.g., admin or not)
alt User is editable
L->>L: Initialize drag-and-drop functionality
L->>L: Allow onDrop events
else User is not editable
L->>L: Skip drag-and-drop initialization
end
sequenceDiagram
participant U as User
participant G as GroupItem Component
U->>G: Click the Plus icon/button
G->>G: Check if `isEditable` (user permission)
alt Button is enabled
G->>G: Execute click event handler for state creation
else Button is disabled
G-->>U: No action (disabled state)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/project-states/group-item.tsx (1)
78-87: Great improvement in accessibility and permission handling!The changes enhance both accessibility and permission handling by:
- Using a semantic
buttonelement instead of adiv.- Properly implementing the disabled state based on user permissions.
- Providing visual feedback through cursor and color changes.
Consider adding an
aria-labelto improve screen reader experience:<button className={cn( "flex-shrink-0 w-6 h-6 rounded flex justify-center items-center overflow-hidden transition-colors hover:bg-custom-background-80 cursor-pointer text-custom-primary-100/80 hover:text-custom-primary-100", !isEditable && "cursor-not-allowed text-custom-text-400 hover:text-custom-text-400" )} onClick={() => !createState && setCreateState(true)} disabled={!isEditable} + aria-label="Add new state" > <Plus className="w-4 h-4" /> </button>web/core/components/labels/label-drag-n-drop-HOC.tsx (1)
70-70: Clean implementation of permission-based early return!The early return in
useEffectefficiently prevents drag-and-drop initialization when the user lacks edit permissions.Consider adding a debug log to help troubleshoot permission issues:
- if (!element || !isEditable) return; + if (!element || !isEditable) { + if (process.env.NODE_ENV === 'development' && !isEditable) { + console.debug('Label drag-and-drop disabled: User lacks admin permissions'); + } + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/labels/label-drag-n-drop-HOC.tsx(2 hunks)web/core/components/project-states/group-item.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
web/core/components/labels/label-drag-n-drop-HOC.tsx (1)
63-64: Well-implemented permission check!The permission check is correctly implemented using the
useUserPermissionshook and appropriate permission constants.
* fix: Handled workspace switcher closing on click * fix: permissions for labels dnd + issue state creation
Description
Summary by CodeRabbit