Add user endpoints #146

Merged
merged 45 commits into from Apr 30, 2014

Conversation

Projects
None yet
3 participants
@rmccue
Member

rmccue commented Apr 20, 2014

This is a continuation of #128 with some cleanups, and adding extra functionality.

Insanely huge props to @tobych on this one; he's done almost all of the heavy lifting here.

Things left to do:

  • Implement /users/me
  • Unify user update/insertion code
  • Add missing fields to update_user
  • Audit everything for security
  • Replace Posts::prepare_author()

Will fix #20.

tobych and others added some commits Mar 29, 2014

Check more appropriate capabilities, including meta caps
Also, make error messages less apologetic. Sorry about that.
Use consistent error messages
This brings permissions error messages in line with the post endpoints.
Don't double-check user ID
get_userdata will check if the user ID is valid, let's not second guess
this ourselves.
Remove HttpStatusCode class (enum)
I like the concept, but it doesn't belong in this PR.
lib/class-wp-json-users.php
+ return new WP_Error( 'json_cannot_delete', __( 'The user cannot be deleted.' ), array( 'status' => 500 ) );
+ }
+ else {
+ // "TODO: return a HTTP 202 here instead"... says the Post endpoint... really? Inappropriate (says tobych)?

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

@tobych Just FYI: the reason the comment is here in the post endpoint is because it's a different check there. In Post::delete_post, this branch is for trashed posts; a 202 would indicate that the request to delete has been received, but the post is still available, just in a trashed state.

@rmccue

rmccue Apr 20, 2014

Member

@tobych Just FYI: the reason the comment is here in the post endpoint is because it's a different check there. In Post::delete_post, this branch is for trashed posts; a 202 would indicate that the request to delete has been received, but the post is still available, just in a trashed state.

@rmccue

This comment has been minimized.

Show comment
Hide comment
@rmccue

rmccue Apr 20, 2014

Member

Looking good so far @tobych, mostly just bringing it in line with all the rest.

Will attack the rest of the bullet points above ASAP.

Member

rmccue commented Apr 20, 2014

Looking good so far @tobych, mostly just bringing it in line with all the rest.

Will attack the rest of the bullet points above ASAP.

@rmccue rmccue added this to the 1.0 milestone Apr 20, 2014

@rmccue rmccue added the Enhancement label Apr 20, 2014

@rmccue rmccue self-assigned this Apr 20, 2014

lib/class-wp-json-server.php
- '/users' => array(
- array( '__return_null', self::READABLE ),
- array( '__return_null', self::CREATABLE | self::ACCEPT_JSON ),
- ),
// /users/me is an alias, and simply redirects to /users/<id>
'/users/me' => array( '__return_null', self::ALLMETHODS ),

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

Still need to handle this one.

@rmccue

rmccue Apr 20, 2014

Member

Still need to handle this one.

lib/class-wp-json-users.php
+// list_users - 3.0
+// add_users - 3.0
+// promote_users - 3.0 (this is about changing a users's level... not sure it's relevant to roles/caps)
+// promote_user (meta)

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

+ $struct[] = $this->prepare_user( $user, $context );
+ }
+
+ return $struct;

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

Should include pagination headers here.

@rmccue

rmccue Apr 20, 2014

Member

Should include pagination headers here.

lib/class-wp-json-users.php
+ $user = apply_filters( 'json_pre_update_user', $user, $id, $data, $_headers );
+
+ // Update the user in the database
+ // http://codex.wordpress.org/Function_Reference/wp_update_user

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

lib/class-wp-json-users.php
+ * @return mixed
+ */
+ public function new_user( $data ) {
+ # TODO: Use WP_User here, and refactor so we're sharing code with edit_user

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

lib/class-wp-json-users.php
+ }
+
+ $user_id = wp_insert_user( $userdata );
+ // TODO: Send appropriate HTTP error codes along with the JSON rendering of the WP_Error we send back

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

lib/class-wp-json-users.php
+
+ $user_id = wp_insert_user( $userdata );
+ // TODO: Send appropriate HTTP error codes along with the JSON rendering of the WP_Error we send back
+ // TODO: I guess we can just add/overwrite the 'status' code in there ourselves... nested WP_Error?

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

I like the thinking here @tobych; this is something we should consider across the project whenever we get a WP_Error back from a core function.

@rmccue

rmccue Apr 20, 2014

Member

I like the thinking here @tobych; this is something we should consider across the project whenever we get a WP_Error back from a core function.

This comment has been minimized.

@rmccue

rmccue Apr 29, 2014

Member

Splitting this into a separate issue: #153

@rmccue

rmccue Apr 29, 2014

Member

Splitting this into a separate issue: #153

lib/class-wp-json-users.php
+ return new WP_Error( 'json_user_invalid_id', __( 'Invalid user ID.' ), array( 'status' => 400 ) );
+ }
+
+ // https://codex.wordpress.org/Function_Reference/wp_delete_user

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

lib/class-wp-json-users.php
+ }
+
+ // https://codex.wordpress.org/Function_Reference/wp_delete_user
+ // TODO: Allow posts to be reassigned (see the docs for wp_delete_user) - use a HTTP parameter?

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

Hmm, good question.

@rmccue

rmccue Apr 20, 2014

Member

Hmm, good question.

lib/class-wp-json-users.php
+ */
+ protected function update_user( $user, $data ) {
+
+ // Won't let them update these fields: ID, login, pass, registered (silently ignored)

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

@rmccue

rmccue Apr 20, 2014

Member

To be removed.

lib/class-wp-json-users.php
+ protected function update_user( $user, $data ) {
+
+ // Won't let them update these fields: ID, login, pass, registered (silently ignored)
+ // TODO: Raise an exception if they try to update those? Always ignore ID though.

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

Good thinking; we've had this problem with people using the content field on Posts::edit_post because it's silently ignored.

@rmccue

rmccue Apr 20, 2014

Member

Good thinking; we've had this problem with people using the content field on Posts::edit_post because it's silently ignored.

lib/class-wp-json-users.php
+
+ // Note that you can pass wp_update_user() an array of fields to
+ // update; we won't bother using it as they don't match the User entity
+ // and it's just one more level of indirection to maintain.

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

Will probably need to in order to keep code consistent between insert/update.

@rmccue

rmccue Apr 20, 2014

Member

Will probably need to in order to keep code consistent between insert/update.

lib/class-wp-json-users.php
+ // ignore avatar - read-only
+ // ignore username - can't change this
+ if ( ! empty( $data['email'] ) ) {
+ $user->user_email = $data['email'];

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

This should be validated for is_email

@rmccue

rmccue Apr 20, 2014

Member

This should be validated for is_email

lib/class-wp-json-users.php
+ $user->user_nicename = $data[ 'slug' ];
+ }
+ if ( ! empty( $data['URL'] ) ) {
+ $user->user_url = $data[ 'URL' ];

This comment has been minimized.

@rmccue

rmccue Apr 20, 2014

Member

This should be validated using parse_url

@rmccue

rmccue Apr 20, 2014

Member

This should be validated using parse_url

rmccue added some commits Apr 29, 2014

Add /users/me endpoint
This returns the data for the current user, and also issues a 302
redirect to the current user's endpoint permalink (e.g. /users/42)
@rmccue

This comment has been minimized.

Show comment
Hide comment
@rmccue

rmccue Apr 29, 2014

Member

@rachelbaker #reviewmerge :)

Member

rmccue commented Apr 29, 2014

@rachelbaker #reviewmerge :)

@rachelbaker

This comment has been minimized.

Show comment
Hide comment
@rachelbaker

rachelbaker Apr 30, 2014

Member

@rmccue This all works great! Merging into trunk.

Member

rachelbaker commented Apr 30, 2014

@rmccue This all works great! Merging into trunk.

rachelbaker added a commit that referenced this pull request Apr 30, 2014

Merge pull request #146 from WP-API/user-endpoints
Add endpoints to handle user management.  Closes #20.

@rachelbaker rachelbaker merged commit b3b79d3 into master Apr 30, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@rachelbaker rachelbaker deleted the user-endpoints branch May 1, 2014

kellbot pushed a commit to kellbot/WP-API that referenced this pull request Aug 1, 2014

Merge pull request #146 from WP-API/user-endpoints
Add endpoints to handle user management.  Closes #20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment