Expand args for updating comments #1245

rachelbaker · 2015-05-18T02:01:06Z

Consistent support for Comment create and update arguments:

Expands the update comment args to match the create arguments (including special handling for date_gmt)
Define each update argument and set sanitization callbacks
Removes the prepare_item_for_update method in favor of re-using the prepare_item_for_database method
Allow users with moderate_comments to set comment karma
Return an error when comment type is being changed, since wp_update_comment does not support this arg

Some other minor changes:

Each create argument has set default and sanitization callbacks
Return an error if the user is creating a comment tries to set the author to an invalid value
Don't set a default value for the comment_type
Remove double sanitization on create and update comment args

All with test coverage!!!

This sets up #1205 for the Comments endpoints.

…allbacks

…` method

…thor to an invalid value - A user must be able to `moderate_comments` to set the author to another user_id.

WordPress expects an empty comment_type to mean the type is comment.

The input arguments are sanitized in the route args callback.

rachelbaker · 2015-05-18T02:23:55Z

Fixes #940

@WP-API/amigos #reviewmerge

joehoyle · 2015-05-18T14:06:20Z

lib/endpoints/class-wp-rest-comments-controller.php

@rachelbaker we have an inconsistency here in the endpoints - some use the Schema to define all the update_item / create_item arguments, but here we are not doing that. One reason that may be is because we want to specify the sanitize_callback for each argument, which using the Schema does not allow us to do. Welcome to my recurring issue I've been wanting to solve for a while!

@joehoyle Hmm...another issue I can see with using the Schema is that on comment update you can change the 'status' (which you cannot set on create), so that field isn't completely readonly. What do you think we should do here?

@rachelbaker we probably shouldn't mark it as readonly in that case I think.

@joehoyle We also cannot set a default argument in the schema. Feels like setting the endpoint arguments from the schema is too rigid at this point to fit all our routes.

Related: #1205

rachelbaker · 2015-05-19T19:47:54Z

I need to refresh this so we can add handling to accept date_gmt on update, even though wp_update_comment() doesn't accept the parameter.

I will also try to combine the update/create prepare for database methods to reduce the crazy.

…dpoints arguments

…for_database for both create and update

rachelbaker · 2015-06-15T23:55:56Z

Remaining:

Throw error for comments that are trying to change the type.
Add unit test for updating comment date_gmt.

I am still hoping to get this in Beta 3

rachelbaker · 2015-06-17T02:53:50Z

@WP-API/amigos #reviewmerge

Fixes #940

joehoyle · 2015-06-17T03:28:50Z

lib/endpoints/class-wp-rest-comments-controller.php

I think casting shouldn't be needed as this is sanitized with absint

@joehoyle addressed in 15df46e

joehoyle · 2015-06-17T03:29:38Z

+1 from me on this, though I think @rmccue was going to look over it too.

Sanitizing and type casting are handled in the endpoint args callbacks

…anage_comments permissions - Renames `handle_status_update` method to `handle_status_param` - Display an error if a user doesn't have permission to set the status

rmccue · 2015-06-17T23:39:22Z

tests/test-rest-comments-controller.php

Note the smart quotes here in the string, might cause problems, since it relies on editors using UTF-8/etc. Can we use normal ' or escape sequences please? :)

(Also, they have the internet on computers now?)

rmccue · 2015-06-17T23:40:35Z

👍 on merge, just need to fix those couple of strings. :)

Expand args for updating comments

rachelbaker added 8 commits May 17, 2015 20:10

Define each argument for updating comments and set the sanitization c…

089448c

…allbacks

Add handling for update comment arguments in `prepare_item_for_update…

6e069cc

…` method

Allow users with moderate_comments to set comment karma

9694f0e

Return an error if the user is creating a comment tries to set the au…

de1b7bb

…thor to an invalid value - A user must be able to `moderate_comments` to set the author to another user_id.

Don't set a default for the comment_type

6f8fe89

WordPress expects an empty comment_type to mean the type is comment.

Remove double sanitization on create and update comment args

9083a08

The input arguments are sanitized in the route args callback.

Expand update comment tests to include added args.

a3a64e3

Add tests for creating comments karma and author error responses.

15ff9f4

rachelbaker added the Review label May 18, 2015

rachelbaker added this to the 2.0 Beta 2 milestone May 18, 2015

joehoyle reviewed May 18, 2015
View reviewed changes

rachelbaker added Needs Refresh and removed Review labels May 19, 2015

rachelbaker self-assigned this May 19, 2015

rachelbaker mentioned this pull request May 26, 2015

WP_JSON_Comments_Controller::prepare_item_for_update() is too restrictive #940

Closed

rachelbaker modified the milestones: 2.0 Beta 3, 2.0 Beta 2 May 28, 2015

rachelbaker added 7 commits June 6, 2015 12:51

Merge branch 'develop' into fix-940

26e1d72

Merge branch 'develop' into fix-940

750d4c4

Remove duplicate author param setting in create comment arguments

deb8ead

Merge branch 'develop' into fix-940

db1d6b6

Remove duplicate author argument in comment create endpoint.

3050f78

Add missing author_name parameter to the create and edit comment en…

ebb0497

…dpoints arguments

Remove prepare_item_for_update method in favor of using prepare_item_…

3e1e25d

…for_database for both create and update

rachelbaker removed the Needs Refresh label Jun 15, 2015

rachelbaker added 2 commits June 16, 2015 20:55

Add unit test for updating a comment with the date_gmt argument

4c0881c

Return error for comments that try to change type with test

96436e7

rachelbaker added the Review label Jun 17, 2015

joehoyle reviewed Jun 17, 2015
View reviewed changes

rachelbaker added 5 commits June 17, 2015 08:07

Remove type casting from prepare_item_for_database method

15df46e

Sanitizing and type casting are handled in the endpoint args callbacks

Allow the status param to be set when creating comments if user has m…

30988dc

…anage_comments permissions - Renames `handle_status_update` method to `handle_status_param` - Display an error if a user doesn't have permission to set the status

Add tests for setting the status parameter when creating comments

a08d2fb

Add test_ prefix to comment tests where missing

fb95029

Fix bugs in test methods

e1191d0

rmccue reviewed Jun 17, 2015
View reviewed changes

rachelbaker added 2 commits June 17, 2015 18:52

Escape smart quotes in test string

a2728a0

Correct return value on prepare_item_for_database

056d8fe

rmccue added a commit that referenced this pull request Jun 18, 2015

Merge pull request #1245 from WP-API/fix-940

f430099

Expand args for updating comments

rmccue merged commit f430099 into develop Jun 18, 2015

rmccue deleted the fix-940 branch June 18, 2015 01:59

dave-green-uk mentioned this pull request Jun 25, 2016

Change all instances of WP-API in readme #2517

Closed

Expand args for updating comments #1245

Expand args for updating comments #1245

Uh oh!

Conversation

rachelbaker commented May 18, 2015

Uh oh!

rachelbaker commented May 18, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rachelbaker commented May 19, 2015

Uh oh!

rachelbaker commented Jun 15, 2015

Uh oh!

rachelbaker commented Jun 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joehoyle commented Jun 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmccue commented Jun 17, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants