WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#18428 closed task (blessed) (fixed)

XML-RPC: wp.getUsers, wp.getUser, wp.getProfile, wp.editProfile

Reported by: nprasath002 Owned by:
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch close
Focuses: Cc:

Description

By default only 50 users are returned. We can filter users by roles

Attachments (11)

wp.getUsers.patch (3.7 KB) - added by nprasath002 4 years ago.
18428.combo.patch (16.2 KB) - added by maxcutler 3 years ago.
18428.combo_unittests.patch (15.7 KB) - added by maxcutler 3 years ago.
18428.combo.2.patch (16.3 KB) - added by maxcutler 3 years ago.
18428.combo_unittests.2.patch (17.1 KB) - added by maxcutler 3 years ago.
18428.alt.patch (9.6 KB) - added by maxcutler 3 years ago.
18428.alt_unittests.patch (14.4 KB) - added by maxcutler 3 years ago.
18428.alt.2.patch (9.5 KB) - added by maxcutler 3 years ago.
18428.alt_unittests.2.patch (14.2 KB) - added by maxcutler 3 years ago.
wp.editProfile.bugfix.patch (905 bytes) - added by maxcutler 3 years ago.
18428.fix_wp_users.diff (1.3 KB) - added by markoheijnen 3 years ago.
Added get_userdata() to wp.getUsers so it can return all fields like wp.getUser

Download all attachments as: .zip

Change History (38)

@nprasath0024 years ago

comment:1 @maxcutler4 years ago

Is there a reason that this method returns only a subset of the fields of the wp.getUser method in #18427?

Would it be easier to maintain if this method delegates to the singular version, as seen in the wp_getMediaLibrary method?

comment:2 @maxcutler4 years ago

  • Cc maxcutler added

comment:3 @nprasath0024 years ago

wp_getUser and wp_getUsers are simply wrappers for function get_user_data and get_users.
So these methods returns the same results of the functions used.

wp_getUser returns more info and costly.
There is a need to return only a subset of information for sites
having large number of users. If we use a singular version retrieving information
about 50 users would be costly

Alternatively we can have three methods like
wp_getUser
wp_getUsers
wp_getUserList

@maxcutler3 years ago

comment:4 @maxcutler3 years ago

Going to use this ticket as the work ticket for #18424 (wp.newUser), #18425 (wp.editUser), #18426 (wp.deleteUser), #18427 (wp.getUser), and #18428 (wp.getUsers).

I've attached a combo patch with those five methods plus wp.getUserInfo (to replace blogger.getUserInfo). Also attached are unit tests for 4/6 methods.

With regards to field names, I chose to deviate from the WP_User names. To an outsider, the WP_User names are terribly inconsistent in format, and I wanted to avoid unnecessary pain. Feedback would be appreciated.

comment:5 @zimbelp3 years ago

Have played with your plugin a little bit and had an additional question/request:

Is there any chance of being able to supply multiple roles to the call (i.e. I want to fetch admins and authors)? Or perhaps return the role names in the response so you can filter the results yourself?

It looked like the call was only expecting a single role name, but maybe I am missing something.

comment:6 @maxcutler3 years ago

Multiple roles might be hard since the API this uses (get_users/WP_User_Query) only supports filtering to a single role.

There is a difference between my plugin and the combo patch, in that the combo patch includes a 'roles' field so that it's easier to filter the response on the client-side. I'll update my plugin at some point in the not-too-distant future.

comment:7 follow-up: @nacin3 years ago

get_users() does provide for 'who' => 'authors' which is going to be more useful to expose than just 'role'.

comment:8 in reply to: ↑ 7 @maxcutler3 years ago

Replying to nacin:

get_users() does provide for 'who' => 'authors' which is going to be more useful to expose than just 'role'.

Perhaps you can shed more light on what exactly the who argument is supposed to do? As far as I can tell, the only value it accepts is author, which does user_level != 0.

Maybe it'd be better to let wp.getUsers' role take authors as a value (in addition to author and other individual roles) and convert that to who?

comment:9 follow-up: @mdawaffe3 years ago

We should be checking

current_user_can( 'edit_user', $user_id )

instead of

current_user_can( 'edit_users' )

where appropriate. The same goes for delete_user.

I also think we should be explicitly casting all input:

$username       = (string) $args[1];
// ...
$user_data['display_name'] = (string) $content_struct['display_name'];

but that's a general issue with all our XML-RPC methods.

It's strange that wp.editUser accepts user_contacts input, but wp.newUser does not.

The post endpoints provide post meta get/set. It'd be cool if the user endpoints provided user meta get/set as well.

comment:10 @daniloercoli3 years ago

  • Cc ercoli@… added

comment:11 in reply to: ↑ 9 @nprasath0023 years ago

Replying to mdawaffe:

We should be checking

current_user_can( 'edit_user', $user_id )

instead of

current_user_can( 'edit_users' )

where appropriate. The same goes for delete_user.

I also think we should be explicitly casting all input:

$username       = (string) $args[1];
// ...
$user_data['display_name'] = (string) $content_struct['display_name'];

but that's a general issue with all our XML-RPC methods.

It's strange that wp.editUser accepts user_contacts input, but wp.newUser does not.

To mimic wp-admin we dropped to support user_contacts when creating a new user. ideally the a user will be created by admin and after that that user will update his contact info.

The post endpoints provide post meta get/set. It'd be cool if the user endpoints provided user meta get/set as well.

@maxcutler3 years ago

comment:12 @maxcutler3 years ago

I refreshed the patch and the unit tests to align with trunk and the refactored test suite.

As discussed in IRC last week, I'd like to replace user_contacts with meta/custom_fields, but ran into a number of questions regarding what APIs to use (get_metadata doesn't return meta_ids, so can't match API of wp.getPost) and which caps to check (no equivalent to edit_post_meta that I could see). I'll keep trying to flag down core devs in IRC to talk this through.

@maxcutler3 years ago

comment:13 @maxcutler3 years ago

I've attached a new patch that covers a revised feature set for the 3.5 release as discussed in IRC (9/5/12 dev chat). Namely, wp.newUser and wp.deleteUser have been removed, and wp.editUser has been changed to wp.editUserInfo, which allows a user to edit his/her own profile fields, not including their password or email address.

@maxcutler3 years ago

comment:14 @maxcutler3 years ago

Refreshed the new patch after a #wcbalt code-review with nacin:

  • Renamed wp.getUserInfo to wp.getProfile and wp.editUserInfo to wp.editProfile.
  • Fixed some doccomment mistakes.
  • Remove overloaded 'role' parameter to wp.getUsers and instead exposed core's who => authors feature.
  • Use list_users cap instead of edit_users cap for wp.getUsers.
  • Removed user_level and capabilities from _prepare_user.

comment:15 follow-up: @nacin3 years ago

If a goal for wp.getUsers is to effectively replace wp.getAuthors, we'll need to allow for fields = 'authors' that matches wp.getAuthor's return values (but paginated, this time) and sticks to an edit_posts cap check, rather than list_users.

comment:16 @nacin3 years ago

In [21824]:

XML-RPC: Introduce wp.getUsers, wp.getUser, wp.getProfile, wp.editProfile.

props maxcutler.
props nprasath002 for earlier patches.

see #18428.

comment:17 @nacin3 years ago

  • Milestone changed from Awaiting Review to 3.5
  • Summary changed from Retrieve a list of user via XML-RPC to XML-RPC: wp.getUsers, wp.getUser, wp.getProfile, wp.editProfile
  • Type changed from feature request to task (blessed)

comment:18 @koke3 years ago

What do you think about adding a filter for $user_fields in _prepare_user? It feels like it could be useful to retrieve other custom user meta added by plugins

comment:19 @maxcutler3 years ago

While writing docs for the codex, I found an inconsistency between what _prepare_user returns and what wp.editProfile accepts (url vs. website). I've attached a patch which resolves this.

comment:20 @nacin3 years ago

In [21959]:

XML-RPC: Accept 'url', not 'website' in wp.editProfile. props maxcutler. see #18428.

comment:21 in reply to: ↑ 15 @nacin3 years ago

Replying to nacin:

If a goal for wp.getUsers is to effectively replace wp.getAuthors, we'll need to allow for fields = 'authors' that matches wp.getAuthor's return values (but paginated, this time) and sticks to an edit_posts cap check, rather than list_users.

This still needs to be dealt with.

@markoheijnen3 years ago

Added get_userdata() to wp.getUsers so it can return all fields like wp.getUser

comment:22 @markoheijnen3 years ago

I just added a patch what adds get_userdata() to wp.getUsers since it can't return things like firstname/lastname. I'm not sure if there is a better way to handle it.

I also added the field option 'authors' but didn't change the cap check. I do was wondering if wp.getAuthors isn't broken since there isn't really a check if the returned users are really authors

comment:23 @nacin3 years ago

In [22134]:

Request WP_User objects when caling get_users() in XML-RPC's wp.getUsers method. see #18428.

comment:24 @nacin3 years ago

What's left here?

comment:25 @nacin3 years ago

  • Keywords close added

comment:26 @markoheijnen3 years ago

Is fine for me. Don't think we need fields = authors support

comment:27 @nacin3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Closing as fixed for 3.5, then.

Note: See TracTickets for help on using tickets.