From 082c5d1534b3fe44ea0b8595f15aaeb83833570c Mon Sep 17 00:00:00 2001 From: Boone Gorges Date: Tue, 8 Jan 2019 03:32:04 +0000 Subject: [PATCH] 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=post__in' was not respected. This changeset changes this behavior by handling 'orderby=post__in' 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. git-svn-id: https://develop.svn.wordpress.org/trunk@44452 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-query.php | 34 +++++++++++++++---- tests/phpunit/tests/post/query.php | 32 +++++++++++++++++ .../tests/rest-api/rest-posts-controller.php | 2 +- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/wp-includes/class-wp-query.php b/src/wp-includes/class-wp-query.php index e569e02ef1..0f38c6479b 100644 --- a/src/wp-includes/class-wp-query.php +++ b/src/wp-includes/class-wp-query.php @@ -1546,6 +1546,9 @@ class WP_Query { 'menu_order', 'comment_count', 'rand', + 'post__in', + 'post_parent__in', + 'post_name__in', ); $primary_meta_key = ''; @@ -1576,6 +1579,8 @@ class WP_Query { return false; } + $orderby_clause = ''; + switch ( $orderby ) { case 'post_name': case 'post_author': @@ -1603,6 +1608,23 @@ class WP_Query { case 'meta_value_num': $orderby_clause = "{$primary_meta_query['alias']}.meta_value+0"; break; + case 'post__in': + if ( ! empty( $this->query_vars['post__in'] ) ) { + $orderby_clause = "FIELD({$wpdb->posts}.ID," . implode( ',', array_map( 'absint', $this->query_vars['post__in'] ) ) . ')'; + } + break; + case 'post_parent__in': + if ( ! empty( $this->query_vars['post_parent__in'] ) ) { + $orderby_clause = "FIELD( {$wpdb->posts}.post_parent," . implode( ', ', array_map( 'absint', $this->query_vars['post_parent__in'] ) ) . ' )'; + } + break; + case 'post_name__in': + if ( ! empty( $this->query_vars['post_name__in'] ) ) { + $post_name__in = array_map( 'sanitize_title_for_query', $this->query_vars['post_name__in'] ); + $post_name__in_string = "'" . implode( "','", $post_name__in ) . "'"; + $orderby_clause = "FIELD( {$wpdb->posts}.post_name," . $post_name__in_string . ' )'; + } + break; default: if ( array_key_exists( $orderby, $meta_clauses ) ) { // $orderby corresponds to a meta_query clause. @@ -2239,6 +2261,12 @@ class WP_Query { $q['order'] = $rand ? '' : $this->parse_order( $q['order'] ); } + // These values of orderby should ignore the 'order' parameter. + $force_asc = array( 'post__in', 'post_name__in', 'post_parent__in' ); + if ( isset( $q['orderby'] ) && in_array( $q['orderby'], $force_asc, true ) ) { + $q['order'] = ''; + } + // Order by. if ( empty( $q['orderby'] ) ) { /* @@ -2252,12 +2280,6 @@ class WP_Query { } } elseif ( 'none' == $q['orderby'] ) { $orderby = ''; - } elseif ( $q['orderby'] == 'post__in' && ! empty( $post__in ) ) { - $orderby = "FIELD( {$wpdb->posts}.ID, $post__in )"; - } elseif ( $q['orderby'] == 'post_parent__in' && ! empty( $post_parent__in ) ) { - $orderby = "FIELD( {$wpdb->posts}.post_parent, $post_parent__in )"; - } elseif ( $q['orderby'] == 'post_name__in' && ! empty( $post_name__in ) ) { - $orderby = "FIELD( {$wpdb->posts}.post_name, $post_name__in )"; } else { $orderby_array = array(); if ( is_array( $q['orderby'] ) ) { diff --git a/tests/phpunit/tests/post/query.php b/tests/phpunit/tests/post/query.php index 3e95e73605..1112c87e04 100644 --- a/tests/phpunit/tests/post/query.php +++ b/tests/phpunit/tests/post/query.php @@ -174,6 +174,38 @@ class Tests_Post_Query extends WP_UnitTestCase { $this->assertSame( $ordered, wp_list_pluck( $q->posts, 'ID' ) ); } + /** + * @ticket 38034 + */ + public function test_orderby_post__in_array() { + $posts = self::factory()->post->create_many( 4 ); + + $ordered = array( $posts[2], $posts[0], $posts[3] ); + + $q = new WP_Query( array( + 'post_type' => 'any', + 'post__in' => $ordered, + 'orderby' => array( 'post__in' => 'ASC' ), + ) ); + $this->assertSame( $ordered, wp_list_pluck( $q->posts, 'ID' ) ); + } + + /** + * @ticket 38034 + */ + public function test_orderby_post__in_array_with_implied_order() { + $posts = self::factory()->post->create_many( 4 ); + + $ordered = array( $posts[2], $posts[0], $posts[3] ); + + $q = new WP_Query( array( + 'post_type' => 'any', + 'post__in' => $ordered, + 'orderby' => 'post__in', + ) ); + $this->assertSame( $ordered, wp_list_pluck( $q->posts, 'ID' ) ); + } + function test_post__in_attachment_ordering() { $post_id = self::factory()->post->create(); $att_ids = array(); diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index 074f5437e7..ee01eb717e 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -291,7 +291,7 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te $data = $response->get_data(); $this->assertEquals( 2, count( $data ) ); $this->assertEquals( $id1, $data[0]['id'] ); - $this->assertPostsOrderedBy( "FIELD( {posts}.ID, $id1,$id3 )" ); + $this->assertPostsOrderedBy( "FIELD({posts}.ID,$id1,$id3)" ); // Invalid include should error $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); $request->set_param( 'include', 'invalid' );