WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#39947 closed defect (bug) (fixed)

REST API: Sticky post query returns unsticky posts when no posts are sticky

Reported by: ryelle Owned by: jnylen0
Milestone: 4.7.3 Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch has-unit-tests fixed-major
Focuses: Cc:

Description

When getting sticky posts from the API (for example, site.com/wp-json/wp/v2/posts?sticky=true), if there are no sticky posts set, the query just returns all posts — as if sticky was not set. I was expecting an empty array. If there are sticky posts, it behaves as expected, only returning the sticky posts.

The source seems to be this check in WP_REST_Posts_Controller.

Attachments (2)

39947.diff (2.2 KB) - added by ryelle 3 years ago.
39947.2.diff (8.9 KB) - added by jnylen0 3 years ago.
Add more tests; test the generated SQL to catch the IN(0) fix.

Download all attachments as: .zip

Change History (14)

#1 @kadamwhite
3 years ago

  • Milestone changed from Awaiting Review to 4.7.4

#2 @jnylen0
3 years ago

@kadamwhite what do you think about getting this into 4.7.3 if we can get it patched quickly enough?

@ryelle are you available to write a patch and unit tests for this change today?

As for the patch, I would suggest something like this:

<?php
$sticky_posts = get_option( 'sticky_posts', array() );
if ( ! is_array( $sticky_posts ) ) {
    $sticky_posts = array();
}
if ( $request['sticky'] ) {
    // ...

#3 @kadamwhite
3 years ago

@jnylen0 I am +1 on getting this fixed as soon as possible, I milestoned for 4.7.4 because I thought we'd missed the cutoff -- but let's see what we can do :)

#4 @jnylen0
3 years ago

I would say that we need to get it committed to trunk today or tomorrow at the very latest. A bit of a stretch, but potentially doable.

if there are no sticky posts set

I guess we need to know what get_option( 'sticky_posts' ) returns in this case, and what other values it might return.

@ryelle
3 years ago

#5 @ryelle
3 years ago

I've added a patch with the is_array check from this comment, plus some tests. I also changed $args['post__in'] = array( -1 ); to 0, because wp_query runs absint over the post__in array, and so was converting it to post ID 1.

This ticket was mentioned in Slack in #core by jnylen. View the logs.


3 years ago

#7 @swissspidy
3 years ago

  • Keywords has-patch has-unit-tests added

#8 @jnylen0
3 years ago

  • Milestone changed from 4.7.4 to 4.7.3
  • Owner set to jnylen0
  • Status changed from new to assigned

I think the approach looks good, but there are a few more tests that could be added here and I want to look at it more closely. I'll try to land this today for 4.7.3.

@jnylen0
3 years ago

Add more tests; test the generated SQL to catch the IN(0) fix.

#9 @jnylen0
3 years ago

  • Keywords commit added
  • Status changed from assigned to accepted

39947.2.diff adds more test cases and also tests the WHERE clauses generated by WP_Query using a similar approach to #39079. This catches the IN(0) fix on this ticket.

#10 @jnylen0
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 40122:

REST API: Fix behavior of sticky posts filter when no posts are sticky.

Previously, when getting posts from the API with sticky=true, if there were no sticky posts set, the query would return all posts as if the sticky argument was not set. In this situation, the query should return an empty array instead.

A sticky=true query that should return an empty array (in the previous situation, or with include and no intersecting post IDs) was also broken in that it would query the post with ID 1.

Finally, this commit significantly improves test coverage for the sticky filter argument, including direct testing of the WHERE clauses generated by WP_Query.

Props ryelle.
Fixes #39947.

#11 @jnylen0
3 years ago

  • Keywords fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to 4.7 branch. The tests added in [40122] depend on [40037] so that commit should probably be merged here as well.

#12 @ocean90
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 40136:

REST API: Fix behavior of sticky posts filter when no posts are sticky.

Previously, when getting posts from the API with sticky=true, if there were no sticky posts set, the query would return all posts as if the sticky argument was not set. In this situation, the query should return an empty array instead.

A sticky=true query that should return an empty array (in the previous situation, or with include and no intersecting post IDs) was also broken in that it would query the post with ID 1.

Finally, this commit significantly improves test coverage for the sticky filter argument, including direct testing of the WHERE clauses generated by WP_Query.

Merge of [40037] and [40122] to the 4.7 branch.

Props ryelle, jnylen0.
See #39079.
Fixes #39947.

Note: See TracTickets for help on using tickets.