-
Notifications
You must be signed in to change notification settings - Fork 651
Fixes #386 roles param get users #2372
Fixes #386 roles param get users #2372
Conversation
Merging commits to keep up to date.
Fixes WP-API#386 roles param for get user. Exposes role querying for GET wp/v2/users endpoint. This currently support an OR relation the default is AND but I thought using the OR operator would match up with what seems to be how people would want to use it. AND didn't seem to make much sense as out of the box WordPress doesn't support multiple roles for a user if I am correct?
|
Example request is authenticate as a user with list_users capability would look like this: For a list of authors and subscribers: Only admins: #review would be awesome! |
tests/test-rest-users-controller.php
Outdated
| $request = new WP_REST_Request( 'GET', '/wp/v2/users' ); | ||
| $request->set_param( 'roles', 'ilovesteak' ); | ||
| $response = $this->server->dispatch( $request ); | ||
| $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); |
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.
Can we add a test that the invalid param is the one we inspected?
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.
Ya, sounds like a good idea.
Few things to clean up |
Removed the custom sanitization/validation added wp_parse_slug_list() into core-integration.php. Cleaned up tests to reflect the changes.
| */ | ||
| public function get_items_permissions_check( $request ) { | ||
| // Check if roles is specified in GET request and if user can list users. | ||
| if ( ! empty( $request['roles'] ) && ! current_user_can( 'list_users' ) ) { |
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.
Is rest_user_cannot_view the right error code?
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.
I don't know haha, I copied it from some of the other permission checks on get requests. For example, if someone put in context=edit they would get this error code if unauthenticated/unauthorized.
Adds another test to reflect what happens when a non existent role is searched currently returns empty set.
…P-API#386-roles-param-get-users
Removing a commented line I accidentally left in.
|
@danielbachhuber Is this ready to go or what else needs to be done? |
Waiting for the final build to pass. |
|
@danielbachhuber Awesome! Thanks for the feedback, @WP-API/amigos are the best! 😃 |
…-users Fixes #386 roles param get users
|
😊 |
|
I'm not able to fetch users based on their roles :-( I just want to fetch contributors list and display it on my homepage, well without using any kind of authentication - my case. |
Yes, it does require authentication.
You'll need to write a bit of custom code for this. |
Could you please help me do that? I'm not an expert in php :-( |
|
@technotip http://v2.wp-api.org/guide/problems/ This has good information on how to authenticate with the API. |
Fixes #386 roles param for get user. Exposes role querying for GET
wp/v2/users endpoint. This currently supports an OR relation the default
is AND but I thought using the OR operator would match up with what
seems to be how people would want to use it. AND didn't seem to make
much sense. if I am correct, out of the box WordPress doesn't support multiple roles
for a user?