Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Pass $page as argument to comments functions #7275

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from
+65 −27
Open

Pass $page as argument to comments functions #7275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dec814e
Pass `$page` as argument to comments functions
SantosGuillamot Sep 20, 2024
786e48e
Remove `cpage` from test
SantosGuillamot Sep 20, 2024
76dfb4e
Add unit tests for `page` argument
SantosGuillamot Sep 20, 2024
2d1497a
Change "Previous" label
SantosGuillamot Sep 20, 2024
e67471c
Ensure `$page` argument is respected over `set_query_var`
SantosGuillamot Sep 20, 2024
57bc368
Improve tests readibility
SantosGuillamot Sep 20, 2024
b63b09f
Improve old comments tests
SantosGuillamot Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'] );
}
}
}

Expand Down
22 changes: 15 additions & 7 deletions src/wp-includes/link-template.php
View file
Open in desktop
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
Open in desktop
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() {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this unit test is valuable anymore.


// 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
Open in desktop
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
Open in desktop
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 );
}
}
Loading