When updating a Comment do not call `wp_update_comment_count` if there is no related Post

[rachelbaker]

As a follow-up to #34954:
In wp_update_comment() we are calling wp_update_comment_count(), but this is unnecessary if the comment_post_ID is 0.

[ambrosey]

I tried to patch this but it was unclear to me due to other inconsistencies in the specific file, whether it's best practice to put

if ( $variable ){
    do_this();
}

or

if ( $variable )
    do_this(); 

I tried to be consistent with the file, but it uses both styles.

[juanfra]

@ambrosey it is recommended to always use braces: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#brace-style

Nice patch!

[ambrosey]

@juanfra would it therefore be reasonable to change this patch to change the rest of the file to braces? Or would that be best served on a separate ticket?

[juanfra]

@ambrosey I'm not sure, let's @rachelbaker respond to that. I can help out with adding braces if the answer is yes.

[rachelbaker]

@juanfra @ambrosey Great questions. With newly written lines we do want to follow the ​WordPress Coding Standards.
To keep an accurate history and reduce code churn we don't reformat entire files for style reasons. Brick by brick, we will get there eventually :)

@ambrosey So it looks like on line 1951 you are just missing a space before the bracket.

[juanfra]

Patch with space.

[juanfra]

@rachelbaker Great, so we'll keep on tracing new tickets for that. And keep improving one line at a time.

Kudos @ambrosey :)

[boonebgorges]

As suggested by @rachelbaker, it probably makes more sense to put the ! $post_id check into wp_update_comment_count() itself. wp_update_comment_count_now() already bails when the $post_id is empty, and there's no reason why we'd ever want to add 0 to the deferred-comment-count array.

[juanfra]

@boonebgorges do you mean inside the wp_update_comment_count() function?

$post_id = (int) $post_id;
if ( ! $post_id ) {
        return false;
}

[rachelbaker]

As I dug more into this I found that this bug had a slightly larger scope. Currently only wp_delete_comment() does check if the comment_post_ID is valid (as in returns a WP_Post object from get_post()) before calling wp_update_comment_count(). Instead of adding a bunch of conditional checks before calling that function, it would be cleaner to do the check in the wp_update_comment_count() function itself, and return early if there is no post.

We will need to be careful when doing this because wp_defer_comment_counting() actually intentionally passes null for the $post_id param.

[juanfra]

Now checking inside the function.

[rachelbaker]

Simplified logic from @juanfra's patch

[rachelbaker]

In 36115:

Comments: Return early from wp_update_comment_count() if there is not a valid post.

Props ambrosey, juanfra.
Fixes #34977