Skip to content

Commit

Permalink
Comments: Pass $page as argument to comments functions
Browse files Browse the repository at this point in the history 
Removes query alteration from `build_comment_query_vars_from_block` by introducing a new way to pass the `$page` as argument to functions handling pagination for the comments.

Props cybr, santosguillamot, bernhard-reiter, gziolo.
Fixes #60806.



git-svn-id: https://develop.svn.wordpress.org/trunk@59081 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
@gziolo
gziolo committed Sep 23, 2024
1 parent 805b933 commit fd104ae
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 27 deletions.
5 changes: 0 additions & 5 deletions src/wp-includes/blocks.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2567,11 +2567,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'] );
}
}
}

Expand Down
22 changes: 15 additions & 7 deletions src/wp-includes/link-template.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions tests/phpunit/tests/blocks/renderCommentTemplate.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -253,7 +253,6 @@ public function test_build_comment_query_vars_from_block_sets_cpage_var() {
);
$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' ) );
}

/**
Expand Down
28 changes: 23 additions & 5 deletions tests/phpunit/tests/link/getNextCommentsLink.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand All @@ -28,13 +28,31 @@ 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 );

set_query_var( 'cpage', $old_cpage );

$this->assertStringContainsString( 'cpage=2', $link );
}

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 ) );

// Check setting the query var is ignored.
$old_cpage = get_query_var( 'cpage' );
set_query_var( 'cpage', 2 );

$link = get_next_comments_link( 'Next', 5, 3 );

set_query_var( 'cpage', $old_cpage );

$this->assertStringContainsString( 'cpage=4', $link );
}
}
32 changes: 25 additions & 7 deletions tests/phpunit/tests/link/getPreviousCommentsLink.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,46 @@ 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( 'Next' );
$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( 'Next' );
$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 );
/**
* @ticket 60806
*/
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.
$old_cpage = get_query_var( 'cpage' );
set_query_var( 'cpage', 4 );

$link = get_previous_comments_link( 'Previous', 3 );

set_query_var( 'cpage', $old_cpage );

$this->assertStringContainsString( 'cpage=2', $link );
}
}

0 comments on commit fd104ae

Please sign in to comment.