Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

Last modified 6 weeks ago

#64325 closed defect (bug) (fixed)

Notes: comment_count field is unintentionally incremented when note are resolved or reopened

Reported by: wildworks's profile wildworks Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.9 Priority: normal
Severity: normal Version: 6.9
Component: Notes Keywords: has-patch dev-reviewed
Focuses: Cc:

Description

Originally reported at https://github.com/WordPress/gutenberg/issues/73667 by @shimotomoki

The comment_count column should show the total number excluding notes. However, when a note is resolved or reopened, the wp_update_comment_count_now function counts comments including notes, and increments comment_count by an unexpected number.

As a result, the comment count may not be displayed correctly in some places.

At the very least, I think the SELECT statement here should filter out notes.

Step-by-step reproduction instructions

  • Create and publish a new post.
  • Open the post on the front-end and add a normal comment to the post.
  • The comment section says One response to “{post_title}”, which is correct.
  • Add a block, add a note to it, resolve the note, and save the post.
  • Open the post on the front-end.
  • The comment section says 3 responses to “Test”, which is incorrect.
  • Access http://localhost:8888/wp-admin/edit-comments.php
  • The "Inresponse to" column shows the number of comments as 3, which is incorrect.

Change History (13)

#1 @gulamdastgir04
6 weeks ago

Reproduction Report

Description

I have tested in WordPress Playground in the 6.9-RC3 Version. I'm able to reproduce this issue.

Environment

  • WordPress: 6.9-RC3
  • PHP: 8.3.28-dev
  • Server: PHP.wasm
  • Database: WP_SQLite_Driver (Server: 8.0.38 / Client: 3.51.0)
  • Browser: Chrome 142.0.0.0
  • OS: Linux
  • Theme: Twenty Twenty-Five 1.3
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1

Actual Results

✅ Error condition occurs (reproduced).

This ticket was mentioned in PR #10572 on WordPress/wordpress-develop by @hbhalodia.


6 weeks ago
#2

  • Keywords has-patch added
  • Fix the comment count for the notes.

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

#3 @hbhalodia
6 weeks ago

Hi @wildworks, Have added the PR to exclude the comment_type as note in the SQL query which is used to show the comment count.

PR - https://github.com/WordPress/wordpress-develop/pull/10572

@wildworks commented on PR #10572:


6 weeks ago
#4

@hbhalodia Thanks for the PR! Can you add a unit test?

  • tests/phpunit/tests/comment/wpUpdateCommentCountNow.php

    diff --git a/tests/phpunit/tests/comment/wpUpdateCommentCountNow.php b/tests/phpunit/tests/comment/wpUpdateCommentCountNow.php
    index 3436934845..07c75d9871 100644
    a b class Tests_Comment_wpUpdateCommentCountNow extends WP_UnitTestCase { 
    4646               remove_filter( 'pre_wp_update_comment_count_now', array( $this, '_return_100' ) );
    4747       }
    4848
     49       /**
     50        * @ticket 64325
     51        */
     52       public function test_only_approved_regular_comments_are_counted() {
     53               $post_id = self::factory()->post->create();
     54
     55               self::factory()->comment->create(
     56                       array(
     57                               'comment_post_ID'  => $post_id,
     58                               'comment_approved' => 0,
     59                       )
     60               );
     61               self::factory()->comment->create(
     62                       array(
     63                               'comment_post_ID'  => $post_id,
     64                               'comment_approved' => 1,
     65                       )
     66               );
     67               self::factory()->comment->create(
     68                       array(
     69                               'comment_post_ID'  => $post_id,
     70                               'comment_type'     => 'note',
     71                               'comment_approved' => 0,
     72                       )
     73               );
     74               self::factory()->comment->create(
     75                       array(
     76                               'comment_post_ID'  => $post_id,
     77                               'comment_type'     => 'note',
     78                               'comment_approved' => 1,
     79                       )
     80               );
     81               $this->assertTrue( wp_update_comment_count_now( $post_id ) );
     82               $this->assertSame( '1', get_comments_number( $post_id ) );
     83       }
     84
    4985       public function _return_100() {
    5086               return 100;
    5187       }

@hbhalodia commented on PR #10572:


6 weeks ago
#5

Yes, on it. Will add the test.

#6 @JeffPaul
6 weeks ago

@gulamdastgir04 are you able to test the PR to see that it resolves the issue as well? I’m on mobile for the next hour or so and won’t be able to test for a bit today and would be great to get confirmation that the PR resolves.

This ticket was mentioned in Slack in #core by akshayar. View the logs.


6 weeks ago

#8 @ellatrix
6 weeks ago

  • Version set to trunk

#9 @SergeyBiryukov
6 weeks ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 61336:

Notes: Avoid incrementing comment_count when notes are resolved or reopened.

Follow-up to [60987].

Props hbhalodia, wildworks, shimotomoki, gulamdastgir04, JeffPaul, ellatrix, SergeyBiryukov.
Fixes #64325.

#10 @SergeyBiryukov
6 weeks ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 6.9 branch.

#11 @ellatrix
6 weeks ago

  • Keywords dev-reviewed added; dev-feedback removed

I haven't followed the Notes work closely, but the fix makes sense at this stage of the release. I guess the comment_count column should be reserved for comments of the type comment, so I wonder if in the future we could explicitly check for that instead of excluding note, but I guess that would be a breaks change an not something we can try for 6.9. 🙂

#12 @ellatrix
6 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 61337:

Notes: Avoid incrementing comment_count when notes are resolved or reopened.

Follow-up to [60987].

Reviewed by ellatrix.
Merges [61336] to the 6.9 branch.

Props hbhalodia, wildworks, shimotomoki, gulamdastgir04, JeffPaul, ellatrix, SergeyBiryukov.
Fixes #64325.

@adamsilverstein commented on PR #10572:


6 weeks ago
#13

Nice catch!

Note: See TracTickets for help on using tickets.