post__in orderby not working when passed in an array to orderby

[kelvink]

Currently order by post__in only works if you pass it as a string. If you pass it inside an array, it won't orderby post__in at all.

To reproduce:

<?php
$query = new WP_Query( array( 
    'post__in' => array( 1,2,3,4,5,6,7,8 ), 
    'orderby' => array( 'post__in' => 'desc', 'title' => 'asc' ) 
));

[kelvink]

Related: https://core.trac.wordpress.org/changeset/34836

[helen]

Hi @kelvink - thanks for your report. Could you also provide the query set up that does work for you?

Not new to trunk, so moving back to 4.6, though it would be something that's existed longer than that I imagine.

[kelvink]

Replying to helen:

Hi @kelvink - thanks for your report. Could you also provide the query set up that does work for you?

Not new to trunk, so moving back to 4.6, though it would be something that's existed longer than that I imagine.

Works:

<?php
$query = new WP_Query( array( 
    'post__in' => array( 19, 21, 27, 16 ), 
    'orderby' => 'post__in'
));

Does NOT work:

<?php
$query = new WP_Query( array( 
    'post__in' => array( 19, 21, 27, 16 ), 
    'orderby' => array( 'post__in' )
));

My fix makes post__in orderby work if you pass it in an array. So you can use the post__in orderby in conjunction with other ordering parameters.

I can submit a patch for version 4.3 as well, if needed.

[maccast]

Not sure if this has gotten worse or if I'm experiencing a different issue, but I have a site where the post__in sort order is not working anymore when it was in the past. For me it's doesn't work even if i use a string or an array. It was working in the past and I only noticed the issue after upgrading to Wordpress 4.7.2.

Here is my code for reference:

$query_images_args = array( 'post__in' => explode(',',$include), 'post_parent' => $thePost->ID, 'post_type' => 'attachment', 'post_mime_type' =>'image', 'post_status' => 'inherit', 'orderby' => 'post__in' );

$featuredPosts = new WP_Query( $query_images_args );

Another possible difference is I'm getting the gallery attachments associated with the post, but my $include contains the a comma seperatedstring of the gallery image_ids in the order I want them sorted in.

[AllysonSouza]

I'm also having this problem, ordeby post__in not work even in string. That's my code:

<?php
    $membros = array( 156, 159, 153 );
    $args = array(
        'post_type'      => 'time',
        'posts_per_page' => -1,
        'post__in'       => $membros, 
        'orderby'        => 'post__in',
    );
?>

The posts are displayed in the default admin list order, ignoring the post__in.

[boonebgorges]

In 40278:

Tests: Use assertSame() for WP_Query 'orderby' tests.

assertEqualSets() ignores order, so isn't much good for testing 'orderby'.

See #38034.

[boonebgorges]

I'm unable to reproduce the problems described by @AllysonSouza and @maccast. It's possible that they indicate some other issue.

What @kelvink describes is, indeed, a problem, and the approach in the patch looks correct. I'm about to attach a patch that also contains a test demonstrating the issue.

However, the patch causes some other orderby tests to fail. The reason is this. Once you start treating post__in as a "regular" orderby value instead of a special case, it means that you automatically start inheriting the order parameter too. And since the default order param is DESC, WP_Query( array( 'orderby' => 'post__in' ) ) starts returning results in reverse. This is too big a backward compatibility break. So we need to add some logic somewhere that indicates that when you pass orderby=post__in but don't specify order, you intend the order to be ASC.

[maccast]

Thanks of checking this out. Is there possibly some additional information or details I could provide to help determine why @AllysonSouza and I are seeing a different but similar issue to the one being described here?

Replying to boonebgorges:

I'm unable to reproduce the problems described by @AllysonSouza and @maccast. It's possible that they indicate some other issue.

What @kelvink describes is, indeed, a problem, and the approach in the patch looks correct. I'm about to attach a patch that also contains a test demonstrating the issue.

However, the patch causes some other orderby tests to fail. The reason is this. Once you start treating post__in as a "regular" orderby value instead of a special case, it means that you automatically start inheriting the order parameter too. And since the default order param is DESC, WP_Query( array( 'orderby' => 'post__in' ) ) starts returning results in reverse. This is too big a backward compatibility break. So we need to add some logic somewhere that indicates that when you pass orderby=post__in but don't specify order, you intend the order to be ASC.

[boonebgorges]

@maccast The first thing I'd check is the SQL that's being generated in your case by WP_Query ($this->request).

[thefraj]

Hi All,

I came across this thread when I realised that the posts-in operator actually doesn't seem to work at all in the latest versions of Wordpress.

Below is the sort of code I was using, it will not produce any results under any conditions that I can see (even if pages by this Id exist). I also tried casing using foreach ($posts as $p) : ... ... blah blah but have not been able to get any results back

<?php

$args = array(
'post__in' => array(25199, 27, 1448, 6509)
);
$query = new WP_Query( $args );
?>
<?php if ( $query->have_posts() ) : ?>
    <?php while ( $query->have_posts() ) : $query->the_post(); ?>
        <h2><?php the_title(); ?></h2>
        <?php echo the_content(); ?>
 
    <?php endwhile; ?>
<?php endif; ?>
<?php wp_reset_postdata(); ?>

[boonebgorges]

@thefraj post__in is definitely working more generally. See, for instance, the post__in tests in tests/phpunit/tests/query/results.php. In your case, you're probably not passing the proper post_type or post_status to WP_Query. If you have done further debugging and determined that it's definitely a core bug, please post your full analysis, including post type definition and the SQL generated by your WP_Query instance (and, ideally, automated tests demonstrating the failure).

[mgibbs189]

@boonebgorges could we revisit this one? Your included patch looks good (and thanks to @kelvink for first reporting).

[boonebgorges]

@mgibbs189 Yes, I think we could revisit, but my comment above about 'order' still holds. 38034.2.diff​ updates the patch for current trunk, and adds another test that demonstrates the failure. Can someone work on a patch that will account for the proper default value of 'order' when 'orderby=postin'?

[mgibbs189]

@boonebgorges Sure thing, our team will take a closer look :)

[mgibbs189]

@boonebgorges The new patch accounts for the proper order when using post__in, and the order/orderby code itself has been tweaked a bit for clarity.

[boonebgorges]

Thanks, @mgibbs189. Am I correct that lines 1644-1647 (right side) of 38034.3.diff​ are the key changes from 38034.2.diff​ ? I ask because we generally don't mix general code factoring with bug fixes, so they'll have to go in separately. (and in fact, we shy away from doing general code refactoring without cause, as it risks introducing bugs. That said, your changes certainly look ok at a glance :) )

[boonebgorges]

Additionally, I'm seeing some test failures:

1) Tests_Query_Results::test_query_orderby_post_parent__in_with_order_desc
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'child-three'
-    1 => 'child-four'
-    2 => 'child-one'
-    3 => 'child-two'
+    0 => 'child-one'
+    1 => 'child-two'
+    2 => 'child-three'
+    3 => 'child-four'

/home/bgorges/sites/wpdev/tests/phpunit/tests/query/results.php:619

2) Tests_Query_Results::test_query_orderby_post__in_with_order_desc
Failed asserting that Array &0 (
    0 => 1626
    1 => 1628
    2 => 1627
) is identical to Array &0 (
    0 => 1627
    1 => 1628
    2 => 1626
).

/home/bgorges/sites/wpdev/tests/phpunit/tests/query/results.php:679

3) Tests_Query_Results::test_query_orderby_post_name__in_with_order_desc
Failed asserting that Array &0 (
    0 => 'parent-three'
    1 => 'parent-one'
    2 => 'parent-two'
) is identical to Array &0 (
    0 => 'parent-two'
    1 => 'parent-one'
    2 => 'parent-three'
).

/home/bgorges/sites/wpdev/tests/phpunit/tests/query/results.php:714

I haven't looked into these in detail, but they appear to be related. Could you please run grunt copy:files && phpunit --group=post,query to have a look?

[mgibbs189]

@boonebgorges Yes and no. Some of the logic was moved to parse_order(), such as handling of rand and other edge cases.

The code for supporting an array-based orderby had many overlaps to that of a space-separated string orderby (e.g. orderby => 'title menu_order'), so combining it leaves fewer places that need sanitization.

Another patch incoming.

[mgibbs189]

Fixed remaining PHPUnit errors

[boonebgorges]

Thanks, @mgibbs189. 38034.5.diff​ feels like a lot of changes for what seems like a fairly small bit of logic. 38034.6.diff​ is another approach:

• Remove special handling for post__in, post_parent__in, and post_name__in in parse_orderby(), and move the clause generation to the switch block where other orderby values are handled
• For these values of orderby, force order to an empty string. Do this in get_posts() instead of passing the values around and doing it in parse_order(), etc. Note that I'm forcing an empty string rather than respecting the value of order passed to the function: it was decided as part of #39055 that order would be ignored in these cases (a decision I agree with). See https://core.trac.wordpress.org/ticket/39055#comment:13 and [40056]
• Fix a REST API test that was sensitive to the whitespace in the generated SQL clause

Could you have a look to see whether this approach makes sense to you, and whether it works?

[mgibbs189]

@boonebgorges Yes, I like how you moved everything into parse_orderby(), it flows better that way. Thanks for all your help and quick responses thus far.

I do wish that ​this code received a little more TLC, but I understand the hesitation to refactor working code.

[mgibbs189]

@boonebgorges Since your patch looks good, could we get this queued up for core inclusion?

[boonebgorges]

Hi @mgibbs189 - I've got this ticket on my list of things to look at, but please be patient - it's only been Saturday and Sunday since last update :-D

[mgibbs189]

@boonebgorges No worries, just making sure you didn't need anything else from me :)

[boonebgorges]

In 44452:

Query: Standardize treatment of 'orderby' values post__in, post_parent__in, and post_name__in.

Ordering by post__in was introduced in [21776], but the code assumed that
post__in would be a comma-separated string listing post IDs. When an array
of post IDs was passed to the post__in query var, 'orderby=postin' was
not respected. This changeset changes this behavior by handling
'orderby=postin' in the same way as most other values of 'orderby',
which ensures that arrays as well as strings can be properly parsed.

The same treatment is given to the similar post_name__in and
post_parent__in options of 'orderby', so that most query generation for
orderby clauses happens in the same place, instead of in special cases.

A slight change in the resulting SQL (related to the whitespace around
parentheses and commas) necessitates a change to an existing REST API test
that does a string comparison against the SQL query.

Props mgibbs189, kelvink.
Fixes #38034.

[pento]

In 44456:

Query: Fix some code formatting issues introduced in [44452].

See #38034.

[kelvink]

Thank you so much for resolving!