Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Add sticky parameter to the posts endpoint.#2708

Merged
BE-Webdesign merged 7 commits into
developfrom
sticky-posts-query-param
Sep 28, 2016
Merged

Add sticky parameter to the posts endpoint.#2708
BE-Webdesign merged 7 commits into
developfrom
sticky-posts-query-param

Conversation

@joehoyle
Copy link
Copy Markdown
Member

This allows getting posts that are either "only sticky" for "not sticky"
via sticky=true or sticky=false. This can also be used in conjunction
with include and exclude.

This allows getting posts that are either "only sticky" for "not sticky"
via `sticky=true` or `sticky=false`. This can also be used in conjuction
with `include` and `exclude`.
@joehoyle joehoyle changed the base branch from master to develop September 18, 2016 21:44
@joehoyle
Copy link
Copy Markdown
Member Author

Fixes #2705

Comment thread tests/test-rest-posts-controller.php Outdated
$response = $this->server->dispatch( $request );
$new_data = $response->get_data();
$this->assertEquals( 'Hello\\s', $new_data['content']['raw'] );
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test appears to be unrelated to the rest of the PR, can you split it into a new one? :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhhhhh that's why this is failing, doh! Didn't mean to commit this.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 22, 2016

Current coverage is 94.47% (diff: 94.44%)

Merging #2708 into develop will increase coverage by 0.14%

@@            develop      #2708   diff @@
==========================================
  Files            11         11          
  Lines          3667       3690    +23   
  Methods         173        174     +1   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3459       3486    +27   
+ Misses          208        204     -4   
  Partials          0          0          

Powered by Codecov. Last update 93ca44e...213f7aa

if ( 'post' === $this->post_type ) {
$params['sticky'] = array(
'description' => __( 'Limit result set to items that are sticky.' ),
'type' => 'boolean',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is off.

@rmccue
Copy link
Copy Markdown
Member

rmccue commented Sep 22, 2016

Over to KAdam for review.

unset( $args['filter'] );
}

if ( isset( $request['sticky'] ) ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in slack, sticky=false does not work (it's interpreted as true due to string coercion -- suggest using rest_is_boolean introduced in #2630

$posts = $response->get_data();
$post = $posts[0];
$this->assertEquals( $id1, $post['id'] );
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's worth it but I think this test (variant of the one above) will demonstrate whether sticky=false in query string will work:

    public function test_get_items_not_sticky_query_string_boolean() {
        $id1 = $this->post_id;
        $id2 = $this->factory->post->create( array( 'post_status' => 'publish' ) );

        update_option( 'sticky_posts', array( $id2 ) );

        $request = new WP_REST_Request( 'GET', '/wp/v2/posts' );
        $request->set_param( 'sticky', 'false' );

        $response = $this->server->dispatch( $request );
        $this->assertCount( 1, $response->get_data() );

        $posts = $response->get_data();
        $post = $posts[0];
        $this->assertEquals( $id1, $post['id'] );
    }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion w/ @BE-Webdesign & @rmccue testing this case independently is probably overkill once the right sanitize callback is put in place

if ( 'post' === $this->post_type ) {
$params['sticky'] = array(
'description' => __( 'Limit result set to items that are sticky.' ),
'type' => 'boolean',
Copy link
Copy Markdown
Member

@BE-Webdesign BE-Webdesign Sep 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add in the rest_validate_request_arg and rest_sanitize_request_arg for validate and sanitize callbacks and this patch will be da hotness!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will automajically be done based off the schema already.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is not the schema, sorry my bad!

@joehoyle
Copy link
Copy Markdown
Member Author

@rmccue @BE-Webdesign @kadamwhite ok I pushed an update to fix the false in the URL thang.

@kadamwhite
Copy link
Copy Markdown
Contributor

+1 from me, sticky=false now works as expected

@BE-Webdesign BE-Webdesign merged commit aed84ae into develop Sep 28, 2016
@kadamwhite kadamwhite deleted the sticky-posts-query-param branch September 28, 2016 19:35
kadamwhite added a commit to WP-API/node-wpapi that referenced this pull request Sep 29, 2016
This is an anticipatory change for compatibility with the latest develop
branch of the REST API plugin, which added support for a "sticky" param
used to either return only, or else no, sticky posts, sepending on what
boolean value is passed.

See WP-API/WP-API#2708
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants