REST API: Add support for the 'ignore_sticky_posts' argument #8228
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| $response = rest_get_server()->dispatch( $request ); | ||
| $data = $response->get_data(); | ||
|
|
||
| $this->assertSame( $data[0]['id'], $id2, 'Response has no sticky post at the top' ); |
There was a problem hiding this comment.
If I'm reading the code correctly, $data[1] should be $id1 so it would be good to test this.
There was a problem hiding this comment.
I had to double-check; it looks like the setup creates a couple more posts after self::$post_id, so it's not the next recent post.
Probably not relevant for this test. It just checks that the recently created post is the latest and the sticky post isn't prepended.
| @@ -247,6 +247,7 @@ public function get_items( $request ) { | |||
| 'author_exclude' => 'author__not_in', | |||
| 'exclude' => 'post__not_in', | |||
| 'include' => 'post__in', | |||
| 'ignore_sticky' => 'ignore_sticky_posts', | |||
There was a problem hiding this comment.
By using the value from WP_Query is this a little back-to-front? At the moment if the user does not want stickies to stick, the send true, if they do want them to stick they send false.
Should it be massaged for a positive name? (Genuine question, not a passive aggressive change request!)
There was a problem hiding this comment.
We could change it to include_sticky. Happy to address it as requested. I don't have a strong opinion here.
@spacedmonkey, @TimothyBJacobs, is there a naming convention guide for REST API parameters?
There was a problem hiding this comment.
Besides this naming dilemma, do you think this is good to merge?
I've been trying to come up with a better name, but having trouble there. Even include_sticky is too similar to the include argument.
The endpoint already supports the sticky argument, which is basically post__in/post__not_in for sticky posts.
There was a problem hiding this comment.
Besides this naming dilemma, do you think this is good to merge?
Yes, I think it's good to merge putting aside the naming issue. Are you happy to give it a couple of days for others to provide feedback and if nothing comes in, commit it on Friday?
Mamaduka
force-pushed
the
add/rest-posts-controller-sticky-support
branch
from
a89733e to
5b7fb3c
Compare
|
Let's put it in with My one last naming things question is whether it should be plural |
So, merge as it is. Send ' true ' if the user doesn't want sticky posts to stick.
Don't have a strong opinion here, but there's not much difference between |
My thought (bike shedding??) is that the current name is singular whereas the WP_Query is plural with the But the rest api has |
Considering the existing argument, I'm leaning towards |
|
Dancer, works for me!
|
|
Thanks, @peterwilsoncc! |
Gutenberg ticket: WordPress/gutenberg#68970
Trac ticket: https://core.trac.wordpress.org/ticket/35907
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.