Opened 10 years ago
Last modified 6 weeks ago
#36409 assigned defect (bug)
Comments number is wrong
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.1 | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Comments | Keywords: | has-patch has-unit-tests has-screenshots |
| Focuses: | template | Cc: |
Description
Hi,
the comment number return the number of approved comments, this means all approved comments even those are replies to unapproved comments, for example if a user post comment and people replied to it, then the admin decide to hide this parent comment by disapproving it, the comment number will decrease only by one and keeping counting the hidden replies
So there are two option to fix that :
- Hold/Disapprove all children when disapproving the parent.
- Recalculate the comment number and excluding comments has an unapproved parent.
Regards,
Sidati
Attachments (5)
Change History (48)
#4
@
10 years ago
- Keywords needs-unit-tests added
- Milestone changed from Awaiting Review to Future Release
Hi @sidati - Thanks for the patch. I agree that this is unexpected and incorrect behavior.
Your basic idea for fixing it seems right to me. A couple comments on the patch:
- Could you generate it from the root of your WP installation? It makes it a bit easier to apply and test.
- Your
$depthlogic reads backward to me. I mean, I can see that the patch works, but I would've counted the depth up instead of down. Maybe our minds work differently ;) Anyway, I wonder if the algorithm would be more transparent (and less subject to bugs related to 'thread_comment_depth' if it looked something like this (pseudocode):
$bad_parents = $wpdb->get_col( "SELECT comment_ID from $wpdb->comments WHERE comment_approved != 1" );
$_bad_parents = array();
do {
$_bad_parents = $wpdb->get_col( "SELECT comment_ID from $wpdb->comments WHERE comment_parent IN (" . implode( ',', array_map( 'intval', $bad_parents ) ) . ")";
$bad_parents += $_bad_parents;
} while ( $_bad_parents );
This separates the initial comment_approved query for clarity, and it ensures that we keep walking down the tree as long as bad comments are found.
We will also need unit tests to cover the problem.
#5
@
10 years ago
@boonebgorges;
I like your approach but if you realize the loop is infinite :), so i add a third variable to hold only the found children IDs to be used in the next loop.
You can test it now : 36409-3.patch
#6
in reply to:
↑ description
@
10 years ago
For what it's worth, this looks very similar to #18603. I sort of hope it fixes that one as well. :)
Art
This ticket was mentioned in Slack in #core-comments by sidati. View the logs.
10 years ago
#9
@
10 years ago
I'll create a plugin on github to solve this issue and post it here with unit-tests
This ticket was mentioned in PR #6431 on WordPress/wordpress-develop by @pbearne.
2 years ago
#11
- Keywords has-unit-tests added; needs-unit-tests removed
This update revises the logic used in the wp_update_comment_count_now function. The change ensures that unapproved comments and their children are not included when calculating the comment count for a post. This method involves creating a list of 'bad parents', which represents comments that are not approved, and their associated child comments.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
14 months ago
#17
@
14 months ago
- Milestone changed from 6.8 to 6.9
As per today's bug scrub, I'm moving it to 6.9 for further review/discussion.
This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.
11 months ago
This ticket was mentioned in Slack in #core-test by boniu91. View the logs.
11 months ago
#21
@
11 months ago
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/6431
Environment
- WordPress: 6.8.1
- PHP: 8.2.27
- Server: nginx/1.26.1
- Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.2.27)
- Browser: Firefox 138.0
- OS: macOS
- Theme: Twenty Twenty-Five 1.2
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Actual Results
- ❌ Issue NOT resolved with patch. Comment count is still wrong.
- Raw file downloaded from git has syntactical errors. When I fix that (missing semicolon), the bug is still there.
- When I try to run the PR in playground, playground crashes (MacOS, both Firefox and Safari)
Additional Notes
- None
Supplemental Artifacts
None
#22
follow-up:
↓ 23
@
11 months ago
- Keywords changes-requested added
@pbearne it seems that apart from the issue with the semicolon, the patch is not passing.
Can you give it a go when you find some time?
#23
in reply to:
↑ 22
@
11 months ago
Replying to SirLouen:
@pbearne it seems that apart from the issue with the semicolon, the patch is not passing.
Can you give it a go when you find some time?
Updated
This ticket was mentioned in PR #9277 on WordPress/wordpress-develop by @hbhalodia.
10 months ago
#24
Trac ticket: https://core.trac.wordpress.org/ticket/36409
## Fixes
- Get all the comments related to post and created the lookup based on the comment ID.
- Loop through each comment and check for its child<-->parent and add only if parent comment is approved.
#25
@
10 months ago
Hi Team, I have raised the new PR which uses PHP logic to check for the parent and child comment and update the comment count for the comments that are approved and child comments whose parents are approved.
PR - https://github.com/WordPress/wordpress-develop/pull/9277
Have checked the above patch and it was not wokring as expected, hence worked on the same and created a new PR for it.
Thank You,
@mindctrl commented on PR #9277:
6 months ago
#26
Can we get some phpunit tests here to show this fixes the bug and to prevent regressions?
@hbhalodia commented on PR #9277:
6 months ago
#27
Can we get some phpunit tests here to show this fixes the bug and to prevent regressions?
Hi @mindctrl, I have added the test cases for the updated scenario.
Thank You,
@shailu25 commented on PR #9277:
6 months ago
#28
Can we mention ticket number in unit test?
@wildworks commented on PR #6431:
6 months ago
#30
Closing in favor of #9277
@wildworks commented on PR #9277:
6 months ago
#31
@hbhalodia, Since the failure occurred in the CI, I merged the latest trunk into this pull request.
#32
@
6 months ago
- Milestone changed from 6.9 to 7.0
I've looked at PR 9277, but I'd like to punt this ticket to 7.0. There are two reasons for this.
- We need to investigate further to determine if this change will affect performance. See: https://github.com/WordPress/wordpress-develop/pull/9277/#discussion_r2212676963
- In WordPress 6.9, you can add notes to blocks. This feature is based on the comments API and also utilizes
comment_approved, but these notes should not be displayed in the comment list table. Therefore, we need to ensure that this pull request correctly excludes comments related to the note.
#33
@
4 months ago
- Keywords needs-testing removed
Combined Bug Reproduction and Patch Testing Report
Patch tested: https://github.com/WordPress/wordpress-develop/pull/9277
Steps to Reproduce or Test
- View Hello World post
- Add a couple of replies
- Dashboard > Comments > Unapprove a reply or parent comment
- View post to see comment count
- 🐞 Bug occurs (When I unapprove the parent comment, number of comments reduce by one)
Expected Results
- ✅ If parent comment is unapproved, all child replies should also be uncounted)
Environment
- WordPress: 6.9-beta3-20251107.120557
- PHP: 8.3.29
- Server: PHP.wasm
- Database: WP_SQLite_Driver (Server: 8.0.38 / Client: 3.51.0)
- Browser: Opera
- OS: macOS
- Theme: Twenty Twenty-Five 1.3
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.1
Actual Results
When reproducing a bug/defect:
- ❌ Error condition occurs.
When testing the bugfix patch:
- ✅ Issue resolved with patch.
Screenshots
#35
@
2 months ago
Test Report
Description
This report validates whether the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/9277/
Environment
- WordPress: 6.9-beta3-20260218.111719
- PHP: 8.2.30
- Server: Apache/2.4.66 (Unix) PHP/8.2.30
- Database: mysqli (Server: 10.6.25-MariaDB / Client: mysqlnd 8.2.30)
- Browser: Firefox 148.0
- OS: macOS
- Theme: Twenty Twenty-Five 1.3
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.1
Actual Results
- ✅ When adding notes to blocks, they are not be displayed in the comment list table.
- ✅ When marking a note as resolved, it's not be displayed in the comment list table.
- ✅ When adding a new comment, it's been displayed in the comment list table
Supplemental Artifacts
#36
@
2 months ago
Hi @wildworks, The ticket here is from a while and has a potential PR to go with. If it can be reviewed and share feedbacks would be great to close it out.
Here is the PR - https://github.com/WordPress/wordpress-develop/pull/9277/
Thanks,
#37
follow-up:
↓ 38
@
2 months ago
@hbhalodia Thanks for the update. Can you investigate the cause of the unit test failure? It indicates that the PR may introduce some regressions.
There were 2 failures: 1) Tests_Comment_MetaCache::test_comment_meta_should_be_lazy_loaded_in_comment_feed_queries Failed asserting that 93685 is identical to 93686. /var/www/tests/phpunit/tests/comment/metaCache.php:264 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106 /var/www/vendor/bin/phpunit:118 2) Tests_Comment_MetaCache::test_comment_meta_should_be_lazy_loaded_in_single_post_comment_feed_queries Failed asserting that 93810 is identical to 93811.
See for https://github.com/WordPress/wordpress-develop/actions/runs/22170978215/job/64108595057?pr=9277 more details.
#38
in reply to:
↑ 37
@
2 months ago
Replying to wildworks:
@hbhalodia Thanks for the update. Can you investigate the cause of the unit test failure? It indicates that the PR may introduce some regressions.
There were 2 failures: 1) Tests_Comment_MetaCache::test_comment_meta_should_be_lazy_loaded_in_comment_feed_queries Failed asserting that 93685 is identical to 93686. /var/www/tests/phpunit/tests/comment/metaCache.php:264 phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106 /var/www/vendor/bin/phpunit:118 2) Tests_Comment_MetaCache::test_comment_meta_should_be_lazy_loaded_in_single_post_comment_feed_queries Failed asserting that 93810 is identical to 93811.See for https://github.com/WordPress/wordpress-develop/actions/runs/22170978215/job/64108595057?pr=9277 more details.
Sure @wildworks, I am on it. Will check the cause and update as needed.
#39
@
2 months ago
Hi @wildworks, The tests were failing because, I had one feedback here -- https://github.com/WordPress/wordpress-develop/pull/9277#discussion_r2824097168, to use get_comments function instead of using direct DB query. When I changed that, it looks perfect, but get_comments, egarly loads the comment meta which fails the metaCache test as we see which needs to be lazily loaded.
For now, I have updated the get_comments call to use 'update_comment_meta_cache' => false,, which prevents preload the comment meta and use lazyload mechanism (wp_lazyload_comment_meta) to work properly just like direct SQL query call.
Thanks,
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
7 weeks ago
#41
@
7 weeks ago
As per today's 7.0 pre-RC1 bug scrub:
At a glance, it looks like every feedback were addressed in the PR. Pinging @mukesh27 @westonruter who reviewed the PR so they could have a look, and approve it or not before RC1.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
6 weeks ago
#43
@
6 weeks ago
- Milestone changed from 7.0 to 7.1
@jorbin commented this one during today's bug scrub:
The comments mention that it would be good to take into account notes in the automated tests, that doesn't seem to have been done. I also think i custom comment types in general should be considered.
Therefore, let's move this ticket to 7.1 to give more time for discussion and refining.




Sorry wrong patch