Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Harmonize delete_item() method responses #210

Merged

Conversation

@imath
Copy link
Contributor

commented Aug 8, 2019

As discussed during August 7th dev chat, I think it's important we harmonize how we build the responses of the endpoints delete_item() methods.

I've suggested to do just like the WP REST API does for this case. One good reason to do so is: our Members endpoint extends the WP Users one and already uses this specific way to build responses. See the extract of WP_REST_Users_Controller->delete_item() code below:

		$result = wp_delete_user( $id, $reassign );

		if ( ! $result ) {
			return new WP_Error( 'rest_cannot_delete', __( 'The user cannot be deleted.' ), array( 'status' => 500 ) );
		}

		$response = new WP_REST_Response();
		$response->set_data(
			array(
				'deleted'  => true,
				'previous' => $previous->get_data(),
			)
		);

This PR is applying this to all endpoints except for the 2 ones @dcavins is currently working on in #206

@renatonascalves
Copy link
Member

left a comment

Good to me!

@renatonascalves renatonascalves merged commit 9f46520 into buddypress:master Aug 8, 2019

1 check passed

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

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

I think makes sense. Thanks for making the changes!

@imath imath deleted the imath:improve/delete-item-responses branch Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.