From 838798d04d7bfb6274fbd6d5dc3654b3132af22c Mon Sep 17 00:00:00 2001 From: Paul Bearne Date: Wed, 18 Dec 2024 12:19:21 -0500 Subject: [PATCH 1/5] This diff introduces a new test, `test_get_items_pagination`, to the `WP-REST-Global-Styles-Revisions-Controller-Test.php` file (previously named `rest-global-styles-revisions-controller.php`). This test verifies the pagination functionality of the REST API endpoint for global styles revisions. Here's a breakdown: 1. **Revision Creation:** The test begins by creating 15 revisions for a global style (using `self::$global_styles_id`). This sets up the scenario for testing pagination. 2. **Offset Test:** It then makes a request to the `/wp/v2/global-styles/{global_style_id}/revisions` endpoint with `offset=5` and `per_page=5`. This should return revisions 6-10. The test asserts: - A 200 OK status code. - 5 items in the response data. - `X-WP-Total` header equals 15 (total revisions). - `X-WP-TotalPages` header equals 3 (given 5 per page). 3. **Paged Test:** Next, it tests "paged" requests, setting `page=2` and `per_page=6`. This should return revisions 7-12. The assertions are similar to the offset test, checking for 6 items and the same total/total pages headers. 4. **Out of Bounds Test:** Finally, the test attempts to fetch an out-of-bounds page (`page=4`, `per_page=6`). It expects a `rest_revision_invalid_page_number` error with a 400 Bad Request status code, confirming proper handling of invalid pagination parameters. In summary, this new test ensures that the global styles revisions endpoint correctly handles `offset`, `page`, and `per_page` parameters, returns the expected number of results, and provides accurate pagination headers (`X-WP-Total` and `X-WP-TotalPages`). It also tests error handling for invalid page numbers. --- ...obal-Styles-Revisions-Controller-Test.php} | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) rename tests/phpunit/tests/rest-api/{rest-global-styles-revisions-controller.php => WP-REST-Global-Styles-Revisions-Controller-Test.php} (94%) diff --git a/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php b/tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php similarity index 94% rename from tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php rename to tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php index b01ada9be28ff..8e189e8c8001a 100644 --- a/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php +++ b/tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php @@ -826,6 +826,47 @@ public function test_get_items_out_of_bounds_page_should_not_error_if_offset() { $this->assertCount( $expected_count, $response->get_data() ); } + + /** + * tests for the pagination + */ + public function test_get_items_pagination() { + + // Create multiple revisions + for ( $i = 0; $i < 15; $i ++ ) { + wp_save_post_revision( self::$global_styles_id ); + } + + // Test offset + $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); + $request->set_param( 'offset', 5 ); + $request->set_param( 'per_page', 5 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 5, $data ); + $this->assertEquals( 15, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 3, $response->get_headers()['X-WP-TotalPages'] ); + + // Test paged + $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); + $request->set_param( 'page', 2 ); + $request->set_param( 'per_page', 6 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 6, $data ); + $this->assertEquals( 15, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 3, $response->get_headers()['X-WP-TotalPages'] ); + + // Test out of bounds + $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); + $request->set_param( 'page', 4 ); + $request->set_param( 'per_page', 6 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_revision_invalid_page_number', $response, 400 ); + } + /** * @doesNotPerformAssertions */ From 3b6b4f3d7e309d94d4d9c61deaa5884f62c225ba Mon Sep 17 00:00:00 2001 From: Paul Bearne Date: Thu, 19 Dec 2024 18:00:06 -0500 Subject: [PATCH 2/5] Refactor pagination logic and add related unit tests. Unified and improved pagination handling in REST API controllers by ensuring 'paged' parameter is checked for existence before casting. Updated tests for global styles, posts, and templates to validate pagination behavior, including edge cases like offsets and out-of-bounds pages. Renamed and reorganized test files for clarity. --- ...est-global-styles-revisions-controller.php | 2 +- .../class-wp-rest-posts-controller.php | 2 +- .../class-wp-rest-revisions-controller.php | 2 +- ...st-global-styles-revisions-controller.php} | 28 ++++++------- .../tests/rest-api/rest-posts-controller.php | 41 +++++++++++++++++++ .../wpRestTemplateRevisionsController.php | 40 ++++++++++++++++++ 6 files changed, 98 insertions(+), 17 deletions(-) rename tests/phpunit/tests/rest-api/{WP-REST-Global-Styles-Revisions-Controller-Test.php => rest-global-styles-revisions-controller.php} (97%) diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-revisions-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-revisions-controller.php index 54285fa560bee..4108f9711c8a7 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-revisions-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-revisions-controller.php @@ -189,7 +189,7 @@ public function get_items( $request ) { $revisions_query = new WP_Query(); $revisions = $revisions_query->query( $query_args ); $offset = isset( $query_args['offset'] ) ? (int) $query_args['offset'] : 0; - $page = (int) $query_args['paged']; + $page = isset( $query_args['paged'] ) ? (int) $query_args['paged'] : 0; $total_revisions = $revisions_query->found_posts; if ( $total_revisions < 1 ) { 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 8852519ec45c9..1a5a7dbeaed84 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 @@ -457,7 +457,7 @@ static function ( $format ) { remove_filter( 'post_password_required', array( $this, 'check_password_required' ) ); } - $page = (int) $query_args['paged']; + $page = isset( $query_args['paged'] ) ? (int) $query_args['paged'] : 0; $total_posts = $posts_query->found_posts; if ( $total_posts < 1 && $page > 1 ) { diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php index fb5fa29231a4f..1dbc611631b9b 100644 --- a/src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php +++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-revisions-controller.php @@ -294,7 +294,7 @@ public function get_items( $request ) { $revisions_query = new WP_Query(); $revisions = $revisions_query->query( $query_args ); $offset = isset( $query_args['offset'] ) ? (int) $query_args['offset'] : 0; - $page = (int) $query_args['paged']; + $page = isset( $query_args['paged'] ) ? (int) $query_args['paged'] : 0; $total_revisions = $revisions_query->found_posts; if ( $total_revisions < 1 ) { diff --git a/tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php b/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php similarity index 97% rename from tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php rename to tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php index 8e189e8c8001a..6f347899d021c 100644 --- a/tests/phpunit/tests/rest-api/WP-REST-Global-Styles-Revisions-Controller-Test.php +++ b/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php @@ -829,35 +829,35 @@ public function test_get_items_out_of_bounds_page_should_not_error_if_offset() { /** * tests for the pagination + * + * @ticket 62292 + * + * @covers WP_REST_Global_Styles_Controller::get_items */ - public function test_get_items_pagination() { - - // Create multiple revisions - for ( $i = 0; $i < 15; $i ++ ) { - wp_save_post_revision( self::$global_styles_id ); - } + public function test_get_global_styles_revisions_pagination() { + wp_set_current_user( self::$admin_id ); // Test offset $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); - $request->set_param( 'offset', 5 ); - $request->set_param( 'per_page', 5 ); + $request->set_param( 'offset', 1 ); + $request->set_param( 'per_page', 1 ); $response = rest_get_server()->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); - $this->assertCount( 5, $data ); - $this->assertEquals( 15, $response->get_headers()['X-WP-Total'] ); + $this->assertCount( 1, $data ); + $this->assertEquals( 3, $response->get_headers()['X-WP-Total'] ); $this->assertEquals( 3, $response->get_headers()['X-WP-TotalPages'] ); // Test paged $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); $request->set_param( 'page', 2 ); - $request->set_param( 'per_page', 6 ); + $request->set_param( 'per_page', 2 ); $response = rest_get_server()->dispatch( $request ); $this->assertEquals( 200, $response->get_status() ); $data = $response->get_data(); - $this->assertCount( 6, $data ); - $this->assertEquals( 15, $response->get_headers()['X-WP-Total'] ); - $this->assertEquals( 3, $response->get_headers()['X-WP-TotalPages'] ); + $this->assertCount( 1, $data ); + $this->assertEquals( 3, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 2, $response->get_headers()['X-WP-TotalPages'] ); // Test out of bounds $request = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id . '/revisions' ); diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index 4574bde83621e..d630bc7e4e4a8 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -5637,6 +5637,45 @@ public function test_multiple_post_format_support() { $this->assertCount( 2, $response->get_data(), 'Two posts are expected to be returned' ); } + /** + * tests for the pagination + * + * @ticket 62292 + * + * @covers WP_REST_Posts_Controller::get_items + */ + public function test_get_posts_with_pagination() { + + // Test offset + $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); + $request->set_param( 'offset', 1 ); + $request->set_param( 'per_page', 1 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 1, $data ); + $this->assertEquals( 30, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 30, $response->get_headers()['X-WP-TotalPages'] ); + + // Test paged + $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); + $request->set_param( 'page', 2 ); + $request->set_param( 'per_page', 2 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 2, $data ); + $this->assertEquals( 30, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 15, $response->get_headers()['X-WP-TotalPages'] ); + + // Test out of bounds + $request = new WP_REST_Request( 'GET', '/wp/v2/posts' ); + $request->set_param( 'page', 4 ); + $request->set_param( 'per_page', 10 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_post_invalid_page_number', $response, 400 ); + } + /** * Internal function used to disable an insert query which * will trigger a wpdb error for testing purposes. @@ -5663,6 +5702,8 @@ public function filter_post_item_schema( $schema ) { return $schema; } + + public function filter_post_item_schema_add_property( $schema ) { $schema['properties']['something_entirely_new'] = array( 'description' => __( 'A new prop added with a the rest_post_item_schema filter.' ), diff --git a/tests/phpunit/tests/rest-api/wpRestTemplateRevisionsController.php b/tests/phpunit/tests/rest-api/wpRestTemplateRevisionsController.php index 014a191f0a13f..d23ec53ba8a53 100644 --- a/tests/phpunit/tests/rest-api/wpRestTemplateRevisionsController.php +++ b/tests/phpunit/tests/rest-api/wpRestTemplateRevisionsController.php @@ -299,6 +299,46 @@ public function test_get_items() { ); } + /** + * tests for the pagination + * + * @ticket 62292 + * + * @covers WP_REST_Template_Revisions_Controller::get_items + */ + public function test_get_template_revisions_pagination() { + wp_set_current_user( self::$admin_id ); + + // Test offset + $request = new WP_REST_Request( 'GET', '/wp/v2/templates/' . self::TEST_THEME . '/' . self::TEMPLATE_NAME . '/revisions' ); + $request->set_param( 'offset', 1 ); + $request->set_param( 'per_page', 1 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 1, $data ); + $this->assertEquals( 4, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 4, $response->get_headers()['X-WP-TotalPages'] ); + + // Test paged + $request = new WP_REST_Request( 'GET', '/wp/v2/templates/' . self::TEST_THEME . '/' . self::TEMPLATE_NAME . '/revisions' ); + $request->set_param( 'page', 2 ); + $request->set_param( 'per_page', 2 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 2, $data ); + $this->assertEquals( 4, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( 2, $response->get_headers()['X-WP-TotalPages'] ); + + // Test out of bounds + $request = new WP_REST_Request( 'GET', '/wp/v2/templates/' . self::TEST_THEME . '/' . self::TEMPLATE_NAME . '/revisions' ); + $request->set_param( 'page', 4 ); + $request->set_param( 'per_page', 6 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_revision_invalid_page_number', $response, 400 ); + } + /** * @covers WP_REST_Template_Revisions_Controller::get_items_permissions_check From 3a2ba222f58265f3fdfbb02bb6c7dab81e625f7c Mon Sep 17 00:00:00 2001 From: Paul Bearne Date: Sun, 29 Dec 2024 17:28:11 -0500 Subject: [PATCH 3/5] Update rest-posts-controller.php --- tests/phpunit/tests/rest-api/rest-posts-controller.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index d630bc7e4e4a8..4f6fa4d5038f9 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -5702,8 +5702,6 @@ public function filter_post_item_schema( $schema ) { return $schema; } - - public function filter_post_item_schema_add_property( $schema ) { $schema['properties']['something_entirely_new'] = array( 'description' => __( 'A new prop added with a the rest_post_item_schema filter.' ), From 9948b9630631f468d2f8c5b7c86eb8bbf4eb8648 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 15 Jan 2025 09:59:35 -0800 Subject: [PATCH 4/5] Fix test method doc blocks. --- .../tests/rest-api/rest-global-styles-revisions-controller.php | 2 +- tests/phpunit/tests/rest-api/rest-posts-controller.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php b/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php index 6f347899d021c..5fd97a7a71abf 100644 --- a/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php +++ b/tests/phpunit/tests/rest-api/rest-global-styles-revisions-controller.php @@ -828,7 +828,7 @@ public function test_get_items_out_of_bounds_page_should_not_error_if_offset() { /** - * tests for the pagination + * Tests for the pagination. * * @ticket 62292 * diff --git a/tests/phpunit/tests/rest-api/rest-posts-controller.php b/tests/phpunit/tests/rest-api/rest-posts-controller.php index d03f59f8f7519..a1b8de0474359 100644 --- a/tests/phpunit/tests/rest-api/rest-posts-controller.php +++ b/tests/phpunit/tests/rest-api/rest-posts-controller.php @@ -5677,7 +5677,7 @@ public function test_multiple_post_format_support() { } /** - * tests for the pagination + * Tests for the pagination. * * @ticket 62292 * From 86170a8446e29edfb9647ad9b9f683f7af4513c2 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 15 Jan 2025 10:07:40 -0800 Subject: [PATCH 5/5] Add test to cover WP_REST_Revisions_Controller directly. --- .../rest-api/rest-revisions-controller.php | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/phpunit/tests/rest-api/rest-revisions-controller.php b/tests/phpunit/tests/rest-api/rest-revisions-controller.php index d2650d31fa852..cdb886c4e47e6 100644 --- a/tests/phpunit/tests/rest-api/rest-revisions-controller.php +++ b/tests/phpunit/tests/rest-api/rest-revisions-controller.php @@ -864,4 +864,44 @@ public function test_get_items_out_of_bounds_page_should_not_error_if_offset() { $response = rest_get_server()->dispatch( $request ); $this->assertCount( $expected_count, $response->get_data() ); } + + /** + * Tests for the pagination. + * + * @ticket 62292 + * + * @covers WP_REST_Revisions_Controller::get_items + */ + public function test_get_template_revisions_pagination() { + wp_set_current_user( self::$editor_id ); + + // Test offset + $request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions' ); + $request->set_param( 'offset', 1 ); + $request->set_param( 'per_page', 1 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 1, $data ); + $this->assertEquals( $this->total_revisions, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( $this->total_revisions, $response->get_headers()['X-WP-TotalPages'] ); + + // Test paged + $request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions' ); + $request->set_param( 'page', 2 ); + $request->set_param( 'per_page', 2 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertEquals( 200, $response->get_status() ); + $data = $response->get_data(); + $this->assertCount( 1, $data ); + $this->assertEquals( $this->total_revisions, $response->get_headers()['X-WP-Total'] ); + $this->assertEquals( (int) ceil( $this->total_revisions / 2 ), $response->get_headers()['X-WP-TotalPages'] ); + + // Test out of bounds + $request = new WP_REST_Request( 'GET', '/wp/v2/posts/' . self::$post_id . '/revisions' ); + $request->set_param( 'page', $this->total_revisions + 1 ); + $request->set_param( 'per_page', 1 ); + $response = rest_get_server()->dispatch( $request ); + $this->assertErrorResponse( 'rest_revision_invalid_page_number', $response, 400 ); + } }