Make WordPress Core

Opened 7 years ago

Closed 4 years ago

#28399 closed enhancement (fixed)

Allow WP_Query to query by comment_count column of CPT/posts table

Reported by: ramon fincken Owned by: boonebgorges
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.0
Component: Query Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description (last modified by SergeyBiryukov)

In the current WP_Query, it is possible to order results by comment_count ASC/DESC.
But is it not possible to query the table based on this CPT/posts table column. Therefor it is impossible to (for example) query posts without any comment or posts having over 10 comments.

I propose a diff to use WP_Query in the following way (building on the syntax from meta_query and tax_query)

a) Easy call:
$args['comment_count'] = n; n >= 0

b) Extended call:
$args['comments']['count'] = n; n >= 0

Optional in this call:
$args['comments']['compare'] = Compare; Compare: =, !=, >, >=, <, <=

I will extend this ticket with a patch diff file (trunk) along with a testset (has-patch).

Attachments (5)

comments_query.diff (1.4 KB) - added by ramon fincken 7 years ago.
getCommentsOperator.php (14.6 KB) - added by ramon fincken 7 years ago.
Unit test (props to robert-john van doesburg)
class-wp-query.diff (1.2 KB) - added by ramon fincken 4 years ago.
Patch of class-wp-query.php
commentsQuery.php (10.3 KB) - added by ramon fincken 4 years ago.
New PHPUnit test file
28399.diff (11.6 KB) - added by boonebgorges 4 years ago.

Download all attachments as: .zip

Change History (20)

@ramon fincken
7 years ago


@ramon fincken
7 years ago

Unit test (props to robert-john van doesburg)

#1 @ramon fincken
7 years ago

  • Keywords has-patch needs-testing added

#2 follow-up: @SergeyBiryukov
7 years ago

  • Description modified (diff)

#3 @SergeyBiryukov
7 years ago

  • Focuses ui removed

#4 in reply to: ↑ 2 @ramon fincken
7 years ago

Replying to SergeyBiryukov:
Thanks for adjusting, you beat me to it ;)

#5 @boonebgorges
7 years ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Milestone changed from Awaiting Review to Future Release

I this this is a pretty good idea. A couple of comments (no pun intended):

  • Let's not add two new params ('comment' and 'comment_count'). This seems unnecessary and confusing to me. I suggest having one: 'comment_count'. You can pass an array of 'count' and 'compare', or you can pass it a number, which gets parsed into array( 'count' => $number, 'compare' => '=' ).
  • The unit tests look really comprehensive - thanks so much for that. Let's move them to tests/phpunit/tests/query/commentCount.php, since this is a new feature for WP_Query.
  • In the unit tests: no need to do http_build_query() - just pass an array. No need to set dummy 'post_content' or 'post_type' on the factory posts - just use the defaults ($p1 = $this->factory->post->create(), with no arguments). Testing found_posts is not precise enough - please get the found post IDs and use assertEqualSets to compare with $this->q->posts. I recommend using the following arguments for WP_Query, to minimize overhead:
$this->q->query( array(
    'fields' => 'ids',
    'update_post_meta_cache' => false,
    'update_post_term_cache' => false,
    'comment_count' => array(
        'count' => 5,
        'compare' => '>',
) );

$expected = array( $p1, $p2 );
$this->assertEqualSets( $expected, $this->q->posts );

The functionality itself looks pretty nice, so with some of these changes I think we can put this in. Thanks for the contribution!

#6 @chriscct7
6 years ago

  • Keywords needs-unit-tests added

#7 @ramon fincken
4 years ago

Rewrote this on WordCamp Europe (Paris) to fit with WP 4.8

Uploading the patch and unit tests now

You can use it the integer way:

comment_count = INT

or array

comment_count = ARRAY ( value = INT [, compare = STRING ] )

for instance

                                'comment_count' => 1


                                'comment_count' => array(
                                        'value' => 6,
                                        'compare' => '<'

@ramon fincken
4 years ago

Patch of class-wp-query.php

@ramon fincken
4 years ago

New PHPUnit test file

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

4 years ago

#9 @ramon fincken
4 years ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

4 years ago

#10 @boonebgorges
4 years ago

  • Milestone changed from Future Release to 4.9

Thank you for circling back to this ticket, @ramon fincken. The patch, and especially the tests, are very good.

I've made some modifications in 28399.diff:

  • Added documentation and changed some formatting to match coding standards.
  • In the tests, I've used a few convenience methods to simplify your code (you'd implemented your own versions of WP's wp_list_pluck() and assertEqualSets())
  • Rearranged the way the SQL is assembled to be a bit more readable.

Please review the changes and make sure I've captured everything.

#11 @ramon fincken
4 years ago

I will check this tonight or tomorrow. Quick one yet: if we use prepare, why still use the intval on top?

#12 @boonebgorges
4 years ago

@ramon fincken It's probably redundant :)

#13 follow-up: @ramon fincken
4 years ago

Yes, let's GO :)

The rewritten unit tests are much better readable. Thanks for that.

Do I need to do something at this point?

Last edited 4 years ago by ramon fincken (previous) (diff)

#14 in reply to: ↑ 13 @DrewAPicture
4 years ago

  • Keywords commit added

Replying to ramon fincken:

Do I need to do something at this point?

Nope :-) @boonebgorges will likely give everything another a once over and then commit if everything looks good.

#15 @boonebgorges
4 years ago

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

In 40978:

Introduce $comment_count param for WP_Query.

This parameter allows querying for posts with a specific value of
comment_count. It is also possible to query for posts that match
a comment_count comparison by passing an array with 'value' and
'compare' operators (eg array( 'compare' => '>', 'value' => 5 )).

Props ramon fincken.
Fixes #28399.

Note: See TracTickets for help on using tickets.