Add tests to ensure post controller accepts the same values it returns when creating a post #2763
Conversation
… creating a new draft post
|
Fixes #2696 |
| } | ||
| // Post slug. | ||
| if ( ! empty( $schema['properties']['slug'] ) && isset( $request['slug'] ) ) { | ||
| if ( ! empty( $schema['properties']['slug'] ) && isset( $request['slug'] ) && ! empty( $request['slug'] ) && ! is_object( $request['slug'] ) ) { |
There was a problem hiding this comment.
A blank slug sent to the api winds up here as an empty object which was passing the empty test, I didn’t figure out why. Without this change, the test_update_draft_post_with_returned_draft_post_data test below fails.
There was a problem hiding this comment.
Hmm @adamsilverstein did you figure out where this empty object is coming from?
There was a problem hiding this comment.
Not yet, I'll dig into it @joehoyle. I would expect a blank string here.
There was a problem hiding this comment.
@rmccue @joehoyle I tracked this down to our use of the sanitize_title to sanitize the slugs. sanitize_title takes an optional second parameter as a 'fallback'. in class-wp-rest-controller:L807 in sanitize_params where we use call_user_func, we are passing the value, then also passing the request object and the argument key.
In this case sanitize_title sees an empty string as failing and returns the fallback, the request object, and the post update fails.
Do you know why the request and key are added to the sanitization callback? can those be removed? Going to dig into when they were added.
There was a problem hiding this comment.
ps. I removed the is_object test and now my test_update_draft_post_with_empty_slug test fails
kadamwhite
changed the title
…_title’, passing only the slug
# Conflicts: # lib/endpoints/class-wp-rest-posts-controller.php
|
@joehoyle I think we need to change our sanitize callback to fix this issue. In a3c95b5 I added a new |
plugin.php
Outdated
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Hrmmm I feel like we already added this somewhere, @rmccue is that true?
There was a problem hiding this comment.
Indeed; it's in the base controller as WP_REST_Controller->sanitize_slug
There was a problem hiding this comment.
Which, in fact, is already used above, so not sure why this PR is changing it?
# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
After updating develop, everything checks out for this issue. I'm leaving the test in place for consideration because they validate that everything works as expected. |
|
@adamsilverstein The tests are failing here; does this need |
rmccue
changed the title
|
@rmccue not sure its my branch? I can refresh patch if needed, I'm seeing this in Travis failure: |
prepare_item_for_database, preventing empty slug objects values from being added to the post