Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Conversation

@BE-Webdesign
Copy link
Member

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?

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?
@BE-Webdesign
Copy link
Member Author

Example request is authenticate as a user with list_users capability would look like this:

For a list of authors and subscribers:
GET wp/v2/users?roles=author,subscriber

Only admins:
GET wp/v2/users?roles=administrator

#review would be awesome!

$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 );
Copy link
Member

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?

Copy link
Member Author

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.

@danielbachhuber
Copy link
Member

#review would be awesome!

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' ) ) {
Copy link
Member

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?

Copy link
Member Author

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.
Removing a commented line I accidentally left in.
@danielbachhuber danielbachhuber added this to the 2.0 Beta 13 milestone Mar 18, 2016
@BE-Webdesign
Copy link
Member Author

@danielbachhuber Is this ready to go or what else needs to be done?

@danielbachhuber
Copy link
Member

Is this ready to go or what else needs to be done?

Waiting for the final build to pass.

@BE-Webdesign
Copy link
Member Author

@danielbachhuber Awesome! Thanks for the feedback, @WP-API/amigos are the best! 😃

danielbachhuber added a commit that referenced this pull request Mar 18, 2016
@danielbachhuber danielbachhuber merged commit c96f4d5 into WP-API:develop Mar 18, 2016
@danielbachhuber
Copy link
Member

😊

@technotip
Copy link

I'm not able to fetch users based on their roles :-(
Does that require authentication?

I just want to fetch contributors list and display it on my homepage, well without using any kind of authentication - my case.

@danielbachhuber
Copy link
Member

I'm not able to fetch users based on their roles :-(
Does that require authentication?

Yes, it does require authentication.

I just want to fetch contributors list and display it on my homepage, well without using any kind of authentication - my case.

You'll need to write a bit of custom code for this.

@technotip
Copy link

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 :-(

@BE-Webdesign
Copy link
Member Author

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants