Make WordPress Core

Opened 2 months ago

Last modified 11 days ago

#61100 new defect (bug)

'Change role' field on wp-admin/network/site-users.php?id=xxx contains a bug

Reported by: ignatiusjeroe's profile ignatiusjeroe Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch
Focuses: multisite Cc:

Description

when selecting the option '-No role for this site-' on page wp-admin/network/site-users.php?id=xxx triggers an error. This should not be the case. This field is also available on wp-admin/users.php but this doesnt trigger an error.

Bug
See wp-admin/network/site-users.php @ line 140-183. This case 'promote' handles this request. The issue is the value=none of '-No role for this site-', which is not found in $edible_roles. The if-statement on line 145 will cause the error.

Solution
The case 'promote' of wp-admin/users.php @ line 110-170 is almost identical to that of wp-admin/network/site-users.php?id=xxx. But this statement took the 'none' value into account.
wp-admin/network/site-users.php line 145 - 147 should be replaced by wp-admin/users.php lines 125-136

Attachments (1)

Screen Shot 2024-04-30 at 16.32.07.png (165.6 KB) - added by ignatiusjeroe 2 months ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
2 months ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 6.6

This ticket was mentioned in PR #6470 on WordPress/wordpress-develop by @mi5t4n.


2 months ago
#2

  • Keywords has-patch added; needs-patch removed

This pull request resolves the issue encountered when attempting to change a user's role to 'None' using the 'Change Role To...' bulk option within multisite, specifically at the URL wp-admin/network/site-users.php?id=xxx.

Trac ticket: https://core.trac.wordpress.org/ticket/61100

#3 @techpartho
5 weeks ago

Here is my suggestion, To fix this, we should modify the case 'promote' section in wp-admin/network/site-users.php to handle the 'none' role similarly to how it's dealt with in wp-admin/users.php.

Here are the steps to fix this issue:

  1. Add a mock none-role to $editable_roles:
$editable_roles['none'] = array(
    'name' => __( '— No role for this site —' ),
);

  1. Update the if-statement to handle the 'none' role properly:
if ( ! $role || empty( $editable_roles[ $role ] ) ) {
    wp_die( __( 'Sorry, you are not allowed to give users that role.' ), 403 );
}

if ( 'none' === $role ) {
    $role = '';
}

The error will be resolved by implementing these changes, and selecting "-No role for this site-" will work correctly without triggering an error. This solution aligns the behavior of wp-admin/network/site-users.php with wp-admin/users.php.

This ticket was mentioned in Slack in #core by oglekler. View the logs.


3 weeks ago

#5 @oglekler
3 weeks ago

  • Keywords needs-testing added

#6 follow-up: @hmbashar
3 weeks ago

This issue is only for the network site? I've tested on a regular WP, and I don't think there is an issue. https://prnt.sc/ODEyd_h4Kozo

#7 in reply to: ↑ 6 @ignatiusjeroe
3 weeks ago

Replying to hmbashar:

This issue is only for the network site? I've tested on a regular WP, and I don't think there is an issue. https://prnt.sc/ODEyd_h4Kozo

Mate, I provided all links (wp-admin/network/site-users.php) to the exact page. Given instructions on how to duplicate the issues. And here you are completely ignoring all the details of the issue. I suggest you follow instructions more carefully. Its a multisite issue which is blatantly clear in the post details.

#8 @sudipatel007
2 weeks ago

Test Report

Description
I have added patch in my local and it is working fine
Environment

  • WordPress: 6.6-beta2-58420
  • PHP: 8.1.23
  • Server: nginx/1.16.0
  • Database: mysqli (Server: 8.0.16 / Client: mysqlnd 8.1.23)
  • Browser: Chrome 126.0.0.0 (macOS)
  • Theme: Twenty Nineteen 2.8

Steps to Reproduce

  1. Go to Users
  2. Select user
  3. Click on "Chang role to..." and Select "No role for this site"

Expected Results
If you select "No role for this site" for user role, then it should be changed successfully

Actual Results
User Role is changed sucessfully when i change role to "No role for this site"
https://app.screencast.com/yIj1xAwxUxgxq
Patched applied - https://app.screencast.com/7ePeVO35Hw9Wp

Last edited 12 days ago by sudipatel007 (previous) (diff)

#9 @sudipatel007
12 days ago

  • Keywords good-first-bug needs-testing removed

#10 @oglekler
11 days ago

@spacedmonkey are we good to go with this one, or should we need to reschedule it?
@sudipatel007 only one test report is not usually enough.

#11 @spacedmonkey
11 days ago

@oglekler I have no plans to commit this. Not sure why it is in this milestone.

#12 @sudipatel007
11 days ago

@oglekler I think we are good to go. Thanks

This ticket was mentioned in Slack in #core by nhrrob. View the logs.


11 days ago

#14 @nhrrob
11 days ago

  • Milestone changed from 6.6 to 6.7
Note: See TracTickets for help on using tickets.