-
Notifications
You must be signed in to change notification settings - Fork 215
fix/dokan-menu-not-visible-for-editor-role #3050
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAn early permission guard is added to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks and finishing touches✅ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/Admin/Menu.php (1)
58-67: Consider removing redundant capability checks.The capability checks on lines 58 and 71 are now redundant since the early return on line 37 ensures that only users with the required capability reach this code. Removing these checks would simplify the logic.
🔎 Proposed refactor
- if ( current_user_can( $capability ) ) { - $submenu[ $slug ][] = [ __( 'Dashboard', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/' ]; - $submenu[ $slug ][] = [ __( 'Withdraw', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/withdraw?status=pending' ]; - $submenu[ $slug ][] = [ __( 'Reverse Withdrawal', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/reverse-withdrawal' ]; - - // if dokan pro not installed or dokan pro is greater than 2.9.14 register the `vendor` sub-menu - if ( ! dokan()->is_pro_exists() || version_compare( DOKAN_PRO_PLUGIN_VERSION, '2.9.14', '>' ) ) { - $submenu[ $slug ][] = [ __( 'Vendors', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/vendors' ]; - } + $submenu[ $slug ][] = [ __( 'Dashboard', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/' ]; + $submenu[ $slug ][] = [ __( 'Withdraw', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/withdraw?status=pending' ]; + $submenu[ $slug ][] = [ __( 'Reverse Withdrawal', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/reverse-withdrawal' ]; + + // if dokan pro not installed or dokan pro is greater than 2.9.14 register the `vendor` sub-menu + if ( ! dokan()->is_pro_exists() || version_compare( DOKAN_PRO_PLUGIN_VERSION, '2.9.14', '>' ) ) { + $submenu[ $slug ][] = [ __( 'Vendors', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/vendors' ]; }And for lines 71-74:
- if ( current_user_can( $capability ) ) { - $submenu[ $slug ][] = [ __( '<span style="color:#f18500">Help</span>', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/help' ]; - $submenu[ $slug ][] = [ __( 'Settings', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/settings' ]; - } + $submenu[ $slug ][] = [ __( '<span style="color:#f18500">Help</span>', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/help' ]; + $submenu[ $slug ][] = [ __( 'Settings', 'dokan-lite' ), $capability, 'admin.php?page=' . $slug . '#/settings' ];Also applies to: 71-74
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Admin/Menu.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (3)
includes/Admin/Menu.php (3)
37-39: LGTM - Early capability check prevents unauthorized menu access.The early return correctly implements the PR objective by preventing menu registration when the current user lacks the required capability. This ensures the Dokan menu won't appear for Editor role users.
32-32: No action needed. The function namedokana_admin_menu_capability()is correctly spelled and is a deliberately defined wrapper function in the codebase (includes/functions.php, line 24) that calls the maindokan_admin_menu_capability()function. This is an intentional design pattern, not a typo.Likely an incorrect or invalid review comment.
69-69: Thedokan_admin_menuaction on line 69 fires unconditionally for all users (before the capability check on line 71) and passes the$capabilityas a parameter. Extensions like Dashboard can receive this parameter and decide what menu items to register based on user permissions. This is not a breaking change—the hook behavior remains the same.
All Submissions:
Changes proposed in this Pull Request:
Check only the current user's capability. If the required capability is missing, return early.
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
*Title: * Dokan menu should not be appeared in admin panel for editor role
Before Changes
After Changes
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.