Pass user_roles query parameter in proxy setup URL #1640
Conversation
There was a problem hiding this comment.
This looks good @felixarntz - just a few suggestions to possibly improve, but otherwise 👍
| public function get_user_fields() { | ||
| $current_user = wp_get_current_user(); | ||
|
|
||
| $user_roles = $current_user && $current_user->exists() && ! empty( $current_user->roles ) ? $current_user->roles : array(); |
There was a problem hiding this comment.
This should be safe to simplify here as wp_get_current_user always returns a WP_User instance, which has an empty $roles = array() by default.
| $user_roles = $current_user && $current_user->exists() && ! empty( $current_user->roles ) ? $current_user->roles : array(); | |
| $user_roles = wp_get_current_user()->roles; |
Then the $current_user variable would no longer be necessary 👍
| if ( is_multisite() && current_user_can( 'manage_network' ) ) { | ||
| $user_roles[] = 'network_administrator'; | ||
| } | ||
| $user_roles = array_values( array_unique( $user_roles ) ); |
There was a problem hiding this comment.
Using array_values here should be unnecessary as implode only uses values anyways.
| $user_roles = array_values( array_unique( $user_roles ) ); | |
| $user_roles = array_unique( $user_roles ); |
|
|
||
| // WordPress technically allows for multiple roles, and some plugins | ||
| // make use of that feature - we can just fake it like below. | ||
| wp_get_current_user()->roles[] = 'shop_vendor'; |
There was a problem hiding this comment.
Can we use add_role instead of mutating its property directly?
| wp_get_current_user()->roles[] = 'shop_vendor'; | |
| wp_get_current_user()->add_role( 'shop_vendor' ); |
There was a problem hiding this comment.
The way that WP works here is that we would also need to register shop_vendor as an available role if we want to use WP_User::add_role, otherwise it will be interpreted as a capability (WP is weird). So for testing our behavior specifically I think modifying the roles property is simpler and accurate.
Summary
Addresses issue #1639
Relevant technical choices
Checklist