From dec814e70453f1e4f92785e99d620bc734545006 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 1/7] Pass `$page` as argument to comments functions Co-authored-by: DAreRodz --- src/wp-includes/blocks.php | 5 ----- src/wp-includes/link-template.php | 22 +++++++++++++------ .../tests/blocks/renderCommentTemplate.php | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/wp-includes/blocks.php b/src/wp-includes/blocks.php index 1a8f3459c5632..1ebe4f5066025 100644 --- a/src/wp-includes/blocks.php +++ b/src/wp-includes/blocks.php @@ -2564,11 +2564,6 @@ function build_comment_query_vars_from_block( $block ) { $comment_args['paged'] = $max_num_pages; } } - // Set the `cpage` query var to ensure the previous and next pagination links are correct - // when inheriting the Discussion Settings. - if ( 0 === $page && isset( $comment_args['paged'] ) && $comment_args['paged'] > 0 ) { - set_query_var( 'cpage', $comment_args['paged'] ); - } } } diff --git a/src/wp-includes/link-template.php b/src/wp-includes/link-template.php index 08320dc792f4d..a3c285b850236 100644 --- a/src/wp-includes/link-template.php +++ b/src/wp-includes/link-template.php @@ -3109,21 +3109,25 @@ function get_comments_pagenum_link( $pagenum = 1, $max_page = 0 ) { * Retrieves the link to the next comments page. * * @since 2.7.1 + * @since 6.7.0 Added the `page` parameter. * * @global WP_Query $wp_query WordPress Query object. * - * @param string $label Optional. Label for link text. Default empty. - * @param int $max_page Optional. Max page. Default 0. + * @param string $label Optional. Label for link text. Default empty. + * @param int $max_page Optional. Max page. Default 0. + * @param int|null $page Optional. Page number. Default null. * @return string|void HTML-formatted link for the next page of comments. */ -function get_next_comments_link( $label = '', $max_page = 0 ) { +function get_next_comments_link( $label = '', $max_page = 0, $page = null ) { global $wp_query; if ( ! is_singular() ) { return; } - $page = get_query_var( 'cpage' ); + if ( is_null( $page ) ) { + $page = get_query_var( 'cpage' ); + } if ( ! $page ) { $page = 1; @@ -3180,16 +3184,20 @@ function next_comments_link( $label = '', $max_page = 0 ) { * Retrieves the link to the previous comments page. * * @since 2.7.1 + * @since 6.7.0 Added the `page` parameter. * - * @param string $label Optional. Label for comments link text. Default empty. + * @param string $label Optional. Label for comments link text. Default empty. + * @param int|null $page Optional. Page number. Default null. * @return string|void HTML-formatted link for the previous page of comments. */ -function get_previous_comments_link( $label = '' ) { +function get_previous_comments_link( $label = '', $page = null ) { if ( ! is_singular() ) { return; } - $page = get_query_var( 'cpage' ); + if ( is_null( $page ) ) { + $page = get_query_var( 'cpage' ); + } if ( (int) $page <= 1 ) { return; diff --git a/tests/phpunit/tests/blocks/renderCommentTemplate.php b/tests/phpunit/tests/blocks/renderCommentTemplate.php index 0e29dc2241eec..7bd3d4ad332c7 100644 --- a/tests/phpunit/tests/blocks/renderCommentTemplate.php +++ b/tests/phpunit/tests/blocks/renderCommentTemplate.php @@ -219,12 +219,12 @@ public function test_build_comment_query_vars_from_block_pagination_with_no_comm /** * Test that both "Older Comments" and "Newer Comments" are displayed in the correct order * inside the Comment Query Loop when we enable pagination on Discussion Settings. - * In order to do that, it should exist a query var 'cpage' set with the $comment_args['paged'] value. * * @ticket 55505 + * @ticket 60806 * @covers ::build_comment_query_vars_from_block */ - public function test_build_comment_query_vars_from_block_sets_cpage_var() { + public function test_build_comment_query_vars_from_block_sets_max_num_pages() { // This could be any number, we set a fixed one instead of a random for better performance. $comment_query_max_num_pages = 5; From 786e48eb4bc950c78819d8021b36295fe889cb37 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 2/7] Remove `cpage` from test --- tests/phpunit/tests/blocks/renderCommentTemplate.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/phpunit/tests/blocks/renderCommentTemplate.php b/tests/phpunit/tests/blocks/renderCommentTemplate.php index 7bd3d4ad332c7..e1bf6ce85ec3c 100644 --- a/tests/phpunit/tests/blocks/renderCommentTemplate.php +++ b/tests/phpunit/tests/blocks/renderCommentTemplate.php @@ -253,7 +253,6 @@ public function test_build_comment_query_vars_from_block_sets_max_num_pages() { ); $actual = build_comment_query_vars_from_block( $block ); $this->assertSame( $comment_query_max_num_pages, $actual['paged'] ); - $this->assertSame( $comment_query_max_num_pages, get_query_var( 'cpage' ) ); } /** From 76dfb4e260debbed85b26b85c0827cce2fad29ba Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 3/7] Add unit tests for `page` argument --- tests/phpunit/tests/link/getNextCommentsLink.php | 12 ++++++++++++ tests/phpunit/tests/link/getPreviousCommentsLink.php | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/phpunit/tests/link/getNextCommentsLink.php b/tests/phpunit/tests/link/getNextCommentsLink.php index fc18dc3f4c627..35804047584b5 100644 --- a/tests/phpunit/tests/link/getNextCommentsLink.php +++ b/tests/phpunit/tests/link/getNextCommentsLink.php @@ -37,4 +37,16 @@ public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() set_query_var( 'cpage', $cpage ); } + + /** + * @ticket 60806 + */ + public function test_page_should_respect_value_of_page_argument() { + $p = self::factory()->post->create(); + $this->go_to( get_permalink( $p ) ); + + $link = get_next_comments_link( 'Next', 5, 3 ); + + $this->assertStringContainsString( 'cpage=4', $link ); + } } diff --git a/tests/phpunit/tests/link/getPreviousCommentsLink.php b/tests/phpunit/tests/link/getPreviousCommentsLink.php index 596a6eaca0ed1..021b7916302a8 100644 --- a/tests/phpunit/tests/link/getPreviousCommentsLink.php +++ b/tests/phpunit/tests/link/getPreviousCommentsLink.php @@ -35,4 +35,16 @@ public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() set_query_var( 'cpage', $cpage ); } + + /** + * @ticket 60806 + */ + public function test_page_should_respect_value_of_page_argument() { + $p = self::factory()->post->create(); + $this->go_to( get_permalink( $p ) ); + + $link = get_previous_comments_link( 'Next', 3 ); + + $this->assertStringContainsString( 'cpage=2', $link ); + } } From 2d1497aeedee2732e9abacae4d90861b9933a34c Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 4/7] Change "Previous" label --- tests/phpunit/tests/link/getPreviousCommentsLink.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/link/getPreviousCommentsLink.php b/tests/phpunit/tests/link/getPreviousCommentsLink.php index 021b7916302a8..349a7c8c16f36 100644 --- a/tests/phpunit/tests/link/getPreviousCommentsLink.php +++ b/tests/phpunit/tests/link/getPreviousCommentsLink.php @@ -14,7 +14,7 @@ public function test_page_should_respect_value_of_cpage_query_var() { $cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', 3 ); - $link = get_previous_comments_link( 'Next' ); + $link = get_previous_comments_link( 'Previous' ); $this->assertStringContainsString( 'cpage=2', $link ); @@ -28,7 +28,7 @@ public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() $cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', '' ); - $link = get_previous_comments_link( 'Next' ); + $link = get_previous_comments_link( 'Previous' ); // Technically, it returns null here. $this->assertNull( $link ); @@ -43,7 +43,7 @@ public function test_page_should_respect_value_of_page_argument() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); - $link = get_previous_comments_link( 'Next', 3 ); + $link = get_previous_comments_link( 'Previous', 3 ); $this->assertStringContainsString( 'cpage=2', $link ); } From e67471c07144a7ed5b3fc2a2bd1b5e3d32e4f92e Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 5/7] Ensure `$page` argument is respected over `set_query_var` --- tests/phpunit/tests/link/getNextCommentsLink.php | 6 ++++++ tests/phpunit/tests/link/getPreviousCommentsLink.php | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/phpunit/tests/link/getNextCommentsLink.php b/tests/phpunit/tests/link/getNextCommentsLink.php index 35804047584b5..87296b8afd42e 100644 --- a/tests/phpunit/tests/link/getNextCommentsLink.php +++ b/tests/phpunit/tests/link/getNextCommentsLink.php @@ -45,8 +45,14 @@ public function test_page_should_respect_value_of_page_argument() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); + // Check setting the query var is ignored. + $cpage = get_query_var( 'cpage' ); + set_query_var( 'cpage', 2 ); + $link = get_next_comments_link( 'Next', 5, 3 ); $this->assertStringContainsString( 'cpage=4', $link ); + + set_query_var( 'cpage', $cpage ); } } diff --git a/tests/phpunit/tests/link/getPreviousCommentsLink.php b/tests/phpunit/tests/link/getPreviousCommentsLink.php index 349a7c8c16f36..43efd37bdeed8 100644 --- a/tests/phpunit/tests/link/getPreviousCommentsLink.php +++ b/tests/phpunit/tests/link/getPreviousCommentsLink.php @@ -43,8 +43,14 @@ public function test_page_should_respect_value_of_page_argument() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); + // Check setting the query var is ignored. + $cpage = get_query_var( 'cpage' ); + set_query_var( 'cpage', 4 ); + $link = get_previous_comments_link( 'Previous', 3 ); $this->assertStringContainsString( 'cpage=2', $link ); + + set_query_var( 'cpage', $cpage ); } } From 57bc3682303cb7bf8d8bf4c58458f412fc28d8bd Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 6/7] Improve tests readibility --- tests/phpunit/tests/link/getNextCommentsLink.php | 6 +++--- tests/phpunit/tests/link/getPreviousCommentsLink.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/phpunit/tests/link/getNextCommentsLink.php b/tests/phpunit/tests/link/getNextCommentsLink.php index 87296b8afd42e..dcb2aaa779c89 100644 --- a/tests/phpunit/tests/link/getNextCommentsLink.php +++ b/tests/phpunit/tests/link/getNextCommentsLink.php @@ -46,13 +46,13 @@ public function test_page_should_respect_value_of_page_argument() { $this->go_to( get_permalink( $p ) ); // Check setting the query var is ignored. - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', 2 ); $link = get_next_comments_link( 'Next', 5, 3 ); - $this->assertStringContainsString( 'cpage=4', $link ); + set_query_var( 'cpage', $old_cpage ); - set_query_var( 'cpage', $cpage ); + $this->assertStringContainsString( 'cpage=4', $link ); } } diff --git a/tests/phpunit/tests/link/getPreviousCommentsLink.php b/tests/phpunit/tests/link/getPreviousCommentsLink.php index 43efd37bdeed8..f06a37e04db4d 100644 --- a/tests/phpunit/tests/link/getPreviousCommentsLink.php +++ b/tests/phpunit/tests/link/getPreviousCommentsLink.php @@ -44,13 +44,13 @@ public function test_page_should_respect_value_of_page_argument() { $this->go_to( get_permalink( $p ) ); // Check setting the query var is ignored. - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', 4 ); $link = get_previous_comments_link( 'Previous', 3 ); - $this->assertStringContainsString( 'cpage=2', $link ); + set_query_var( 'cpage', $old_cpage ); - set_query_var( 'cpage', $cpage ); + $this->assertStringContainsString( 'cpage=2', $link ); } } From b63b09f3b1f5bab089d7efbff43fa54c9bada8fe Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Fri, 20 Sep 2024 10:01:53 +0200 Subject: [PATCH 7/7] Improve old comments tests --- tests/phpunit/tests/link/getNextCommentsLink.php | 12 ++++++------ tests/phpunit/tests/link/getPreviousCommentsLink.php | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/link/getNextCommentsLink.php b/tests/phpunit/tests/link/getNextCommentsLink.php index dcb2aaa779c89..45ac42e09fdca 100644 --- a/tests/phpunit/tests/link/getNextCommentsLink.php +++ b/tests/phpunit/tests/link/getNextCommentsLink.php @@ -11,14 +11,14 @@ public function test_page_should_respect_value_of_cpage_query_var() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', 3 ); $link = get_next_comments_link( 'Next', 5 ); - $this->assertStringContainsString( 'cpage=4', $link ); + set_query_var( 'cpage', $old_cpage ); - set_query_var( 'cpage', $cpage ); + $this->assertStringContainsString( 'cpage=4', $link ); } /** @@ -28,14 +28,14 @@ public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', '' ); $link = get_next_comments_link( 'Next', 5 ); - $this->assertStringContainsString( 'cpage=2', $link ); + set_query_var( 'cpage', $old_cpage ); - set_query_var( 'cpage', $cpage ); + $this->assertStringContainsString( 'cpage=2', $link ); } /** diff --git a/tests/phpunit/tests/link/getPreviousCommentsLink.php b/tests/phpunit/tests/link/getPreviousCommentsLink.php index f06a37e04db4d..edc08c3b765e6 100644 --- a/tests/phpunit/tests/link/getPreviousCommentsLink.php +++ b/tests/phpunit/tests/link/getPreviousCommentsLink.php @@ -11,29 +11,29 @@ public function test_page_should_respect_value_of_cpage_query_var() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', 3 ); $link = get_previous_comments_link( 'Previous' ); - $this->assertStringContainsString( 'cpage=2', $link ); + set_query_var( 'cpage', $old_cpage ); - set_query_var( 'cpage', $cpage ); + $this->assertStringContainsString( 'cpage=2', $link ); } public function test_page_should_default_to_1_when_no_cpage_query_var_is_found() { $p = self::factory()->post->create(); $this->go_to( get_permalink( $p ) ); - $cpage = get_query_var( 'cpage' ); + $old_cpage = get_query_var( 'cpage' ); set_query_var( 'cpage', '' ); $link = get_previous_comments_link( 'Previous' ); + set_query_var( 'cpage', $old_cpage ); + // Technically, it returns null here. $this->assertNull( $link ); - - set_query_var( 'cpage', $cpage ); } /**