Backport a bugfix of Comment Template block pagination

[czapla]

Setting paged to 1 (instead of 0) if there are no comments will cause them to fail.

More details in in ​https://github.com/WordPress/gutenberg/issues/40758

This is a backport of the fix for that issue.

This is a backport of ​https://github.com/WordPress/gutenberg/pull/40759

Trac ticket: https://core.trac.wordpress.org/ticket/55658

I'm not quite sure why PHP lint is failing. Locally, running composer run-script lint throws up an error like:

(...)
, Object(PHP_CodeSniffer\Ruleset), Object(PHP_CodeSniffer\Config))
#5 /Users/czapla/Projects/wordpress-develop/vendor/squizlabs/php_codesniffer/src/Runner.php(542): PHP_CodeSniffer\Runner->processChildProcs(Array)
#6 /Users/czapla/Projects/wordpress-develop/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
#7 /Users/czapla/Projects/wordpress-develop/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
#8 {main}
  thrown in /Users/czapla/Projects/wordpress-develop/vendor/squizlabs/php_codesniffer/src/Runner.php on line 606
Script @php ./vendor/squizlabs/php_codesniffer/bin/phpcs --report=summary,source handling the lint event returned with error code 255

Unfortunately, there is no information in the CI on where the mis-formatting is happening.

Ah, indeed! Looks like the GB PR actually also had that (but I didn't notice it since it's covered by the ​GHA workflow for PHP Unit tests, and I only saw the unit test error there):

Ooh, one of the errors is actually visible in _this_ PR's inline diff:

Love those annotations 😄

Here's a diff with the PHP lint fixes on top of this PR (since I can't push to @michalczaplinski's GH repo):

{{{diff
commit 4db5e1840a42c4ecc3bd4e07a5f50f2555d92496
Author: Bernie Reiter <ockham@…>
Date: Tue May 3 10:22:55 2022 +0200

PHP lints

diff --git a/src/wp-includes/blocks.php b/src/wp-includes/blocks.php
index ff15b9568e..a9a7535530 100644
--- a/src/wp-includes/blocks.php
+++ b/src/wp-includes/blocks.php
@@ -1325,8 +1325,8 @@ function build_comment_query_vars_from_block( $block ) {

} elseif ( 'oldest' === $default_page ) {

$comment_argspaged? = 1;

} elseif ( 'newest' === $default_page ) {

• $max_num_pages = (int) ( new WP_Comment_Query( $comment_args ) )->max_num_pages;
• $comment_argspaged? = $max_num_pages === 0 ? 1 : $max_num_pages;

+ $max_num_pages = (int) ( new WP_Comment_Query( $comment_args ) )->max_num_pages;
+ $comment_argspaged? = 0 === $max_num_pages ? 1 : $max_num_pages;

}
Set the cpage query var to ensure the previous and next pagination links are correct
when inheriting the Discussion Settings.

}}}

Using the testing instructions here ​https://github.com/WordPress/gutenberg/issues/40758, I was able to reproduce the SQL syntax error. Applying the fix in this patch resolves the issue.

What is this patch doing? When there are no comments, it sets the 'paged' arg in the comments query to the default value of 1.

Should it be setting the default value? Or should it not set 'paged' arg and then let Core natively handle setting the default value. Setting here means that if the default value changes someday, it'll need to be changed in multiple places. cc @dream-encode what do you think?

This fix is needed for 6.0 as the issue was introduced in 6.0's cycle and its severity is severe. The patch works and can in as-is to fix the issue. The question about whether to set a default value or not set 'paged' arg can be handled separately if more time is needed for discussion and consensus.

@hellofromtonya Looking at the core docs, I think letting Core handle the default is probably safer, as long as no other bugs are introduced. Seems a bit risky to be changing the paged param in other places this late in the cycle. I'd rather use the default, then investigate possibly setting paged in 6.0.1, if it's even needed.

I think that's a safer path @dream-encode. Thank you! I'll make the adjustment and then prep the commit.

And of course, I need to also fix the tests with this PR 🤦‍♀️

[hellofromTonya]

In 53336:

Editor: Sets 'paged' query arg only when there are comments: build_comment_query_vars_from_block().

A SQL syntax error happened when a post has no comments and "Break comments into pages" is checked in Settings > Discussion. The fix sets the 'paged' query arg only when there are comments. When there are no comments, WP_Comment_Query sets the default 'paged' value to 1.

Props bernhard-reiter, luisherranz, czapla, cbravobernal, davidbaumwald, hellofromTonya.

Follow-up to [53142], [53138].
Fixes #55658.

Use Yoda condition checks, you must.
Love those annotations 😄

Admittedly, I didn't realize that this was an actual warning because of how it's worded. I thought this was some inside joke 😅 . TIL ​Yoda conditions are a thing.

@michalczaplinski yeah they are a real thing. And here's where they live in Core's handbook:

• PHP Coding Standard ​https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#yoda-conditions

{{{php
if ( true === $the_force ) {

$victorious = you_will( $be );

}
}}}

Committed via changeset https://core.trac.wordpress.org/changeset/53336.

Thanks for taking this over the finish line, @hellofromtonya and @dream-encode ! 😄

I also resynchronized the changes made in Core back to Gutenberg's compat PR ​https://github.com/WordPress/gutenberg/pull/40759.