From 6e478380536b0e3f129cb93ab11f474ed3a0aba1 Mon Sep 17 00:00:00 2001 From: Rachel Baker Date: Wed, 26 Oct 2016 21:36:29 +0000 Subject: [PATCH] REST API: Remove experimental `filter` wrapper parameter from the Posts Controller class. Hiding WP_Query params under the filter key (instead of allowing them to be top-level params) was one of our biggest complaints from users of v1 of our REST API. This walks back the re-introduction of the `filter` param during Beta 15, which introduced an "inconsistent mess" and "exposing WP_Query through filter has and will continue to be difficult to support." See https://github.com/WP-API/WP-API/issues/2799. Props websupporter, rachelbaker. Fixes #38378. git-svn-id: https://develop.svn.wordpress.org/trunk@38968 602fd350-edb4-49c9-b593-d223f7449a82 --- .../class-wp-rest-posts-controller.php | 16 ++-------- .../rest-api/rest-attachments-controller.php | 1 - .../tests/rest-api/rest-pages-controller.php | 8 ++--- .../tests/rest-api/rest-posts-controller.php | 29 +++++-------------- 4 files changed, 13 insertions(+), 41 deletions(-) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php index 6812c13dd6..53692d8e3f 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php @@ -111,7 +111,7 @@ class WP_REST_Posts_Controller extends WP_REST_Controller { public function get_items( $request ) { // Make sure a search string is set in case the orderby is set to 'relevance'. - if ( ! empty( $request['orderby'] ) && 'relevance' === $request['orderby'] && empty( $request['search'] ) && empty( $request['filter']['s'] ) ) { + if ( ! empty( $request['orderby'] ) && 'relevance' === $request['orderby'] && empty( $request['search'] ) ) { return new WP_Error( 'rest_no_search_term_defined', __( 'You need to define a search term to order by relevance.' ), array( 'status' => 400 ) ); } @@ -161,11 +161,6 @@ class WP_REST_Posts_Controller extends WP_REST_Controller { $args['date_query'][0]['after'] = $request['after']; } - if ( isset( $registered['filter'] ) && is_array( $request['filter'] ) ) { - $args = array_merge( $args, $request['filter'] ); - unset( $args['filter'] ); - } - // Ensure our per_page parameter overrides any provided posts_per_page filter. if ( isset( $registered['per_page'] ) ) { $args['posts_per_page'] = $request['per_page']; @@ -269,17 +264,13 @@ class WP_REST_Posts_Controller extends WP_REST_Controller { $total_posts = $count_query->found_posts; } - $max_pages = ceil( $total_posts / (int) $query_args['posts_per_page'] ); + $max_pages = ceil( $total_posts / (int) $posts_query->query_vars['posts_per_page'] ); $response = rest_ensure_response( $posts ); $response->header( 'X-WP-Total', (int) $total_posts ); $response->header( 'X-WP-TotalPages', (int) $max_pages ); $request_params = $request->get_query_params(); - if ( ! empty( $request_params['filter'] ) ) { - // Normalize the pagination params. - unset( $request_params['filter']['posts_per_page'], $request_params['filter']['paged'] ); - } $base = add_query_arg( $request_params, rest_url( sprintf( '%s/%s', $this->namespace, $this->rest_base ) ) ); if ( $page > 1 ) { @@ -1910,9 +1901,6 @@ class WP_REST_Posts_Controller extends WP_REST_Controller { 'type' => 'string', 'validate_callback' => array( $this, 'validate_user_can_query_private_statuses' ), ); - $params['filter'] = array( - 'description' => __( 'Use WP Query arguments to modify the response; private query vars require appropriate authorization.' ), - ); $taxonomies = wp_list_filter( get_object_taxonomies( $this->post_type, 'objects' ), array( 'show_in_rest' => true ) ); foreach ( $taxonomies as $taxonomy ) { diff --git a/tests/phpunit/tests/rest-api/rest-attachments-controller.php b/tests/phpunit/tests/rest-api/rest-attachments-controller.php index 86f45a6050..f270d5acb9 100644 --- a/tests/phpunit/tests/rest-api/rest-attachments-controller.php +++ b/tests/phpunit/tests/rest-api/rest-attachments-controller.php @@ -124,7 +124,6 @@ class WP_Test_REST_Attachments_Controller extends WP_Test_REST_Post_Type_Control 'before', 'context', 'exclude', - 'filter', 'include', 'media_type', 'mime_type', diff --git a/tests/phpunit/tests/rest-api/rest-pages-controller.php b/tests/phpunit/tests/rest-api/rest-pages-controller.php index 747069c850..f94bbf6466 100644 --- a/tests/phpunit/tests/rest-api/rest-pages-controller.php +++ b/tests/phpunit/tests/rest-api/rest-pages-controller.php @@ -59,7 +59,6 @@ class WP_Test_REST_Pages_Controller extends WP_Test_REST_Post_Type_Controller_Te 'before', 'context', 'exclude', - 'filter', 'include', 'menu_order', 'offset', @@ -179,11 +178,10 @@ class WP_Test_REST_Pages_Controller extends WP_Test_REST_Post_Type_Controller_Te $page_id = $this->factory->post->create( array( 'post_status' => 'publish', 'post_type' => 'page' ) ); $draft_id = $this->factory->post->create( array( 'post_status' => 'draft', 'post_type' => 'page' ) ); $request = new WP_REST_Request( 'GET', '/wp/v2/pages' ); - $request->set_param( 'filter', array( 'post_status' => 'draft' ) ); + $request->set_param( 'status', 'draft' ); $response = $this->server->dispatch( $request ); - $data = $response->get_data(); - $this->assertCount( 1, $data ); - $this->assertEquals( $page_id, $data[0]['id'] ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + // But they are accessible to authorized users wp_set_current_user( $this->editor_id ); $response = $this->server->dispatch( $request ); diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index dd86f9430c..dfc832ceec 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -67,7 +67,6 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te 'categories', 'context', 'exclude', - 'filter', 'include', 'offset', 'order', @@ -97,10 +96,11 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te public function test_get_items_empty_query() { $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); $request->set_query_params( array( - 'filter' => array( 'year' => 2008 ), + 'author' => REST_TESTS_IMPOSSIBLY_HIGH_NUMBER, ) ); $response = $this->server->dispatch( $request ); - $this->assertEquals( array(), $response->get_data() ); + + $this->assertEmpty( $response->get_data() ); $this->assertEquals( 200, $response->get_status() ); } @@ -309,7 +309,7 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te // Permit stickies $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); - $request->set_param( 'filter', array( 'ignore_sticky_posts' => false ) ); + $request->set_param( 'ignore_sticky_posts', false ); $response = $this->server->dispatch( $request ); $data = $response->get_data(); $this->assertEquals( array( $post_id2, $this->post_id, $post_id3, $post_id1 ), wp_list_pluck( $data, 'id' ) ); @@ -576,16 +576,15 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te $this->assertContains( '<' . $next_link . '>; rel="next"', $headers['Link'] ); } - public function test_get_items_private_filter_query_var() { + public function test_get_items_private_status_query_var() { // Private query vars inaccessible to unauthorized users wp_set_current_user( 0 ); $draft_id = $this->factory->post->create( array( 'post_status' => 'draft' ) ); $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); - $request->set_param( 'filter', array( 'post_status' => 'draft' ) ); + $request->set_param( 'status', 'draft' ); $response = $this->server->dispatch( $request ); - $data = $response->get_data(); - $this->assertCount( 1, $data ); - $this->assertEquals( $this->post_id, $data[0]['id'] ); + $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); + // But they are accessible to authorized users wp_set_current_user( $this->editor_id ); $response = $this->server->dispatch( $request ); @@ -601,18 +600,6 @@ class WP_Test_REST_Posts_Controller extends WP_Test_REST_Post_Type_Controller_Te $this->assertErrorResponse( 'rest_invalid_param', $response, 400 ); } - public function test_get_items_invalid_posts_per_page_ignored() { - // This test ensures that filter[posts_per_page] is ignored, and that -1 - // cannot be used to sidestep per_page's valid range to retrieve all posts - for ( $i = 0; $i < 20; $i++ ) { - $this->factory->post->create( array( 'post_status' => 'publish' ) ); - } - $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); - $request->set_query_params( array( 'filter' => array( 'posts_per_page' => -1 ) ) ); - $response = $this->server->dispatch( $request ); - $this->assertCount( 10, $response->get_data() ); - } - public function test_get_items_invalid_context() { $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); $request->set_param( 'context', 'banana' );