Make WordPress Core

Opened 8 years ago

Last modified 8 years ago

#41339 new enhancement

WP_Comments_Query::__construct() should allow a 'status__not_in' parameter

Reported by: pbiron's profile pbiron Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9
Component: Comments Keywords: has-patch needs-unit-tests
Focuses: Cc:

Description

WP_Comments_Query::__construct() (and hence, get_comments()) currently allows a status parameter to get comments with a specific status.

It would be useful to also allow $status__not_in to exclude comments with specific stati.

Related: #41338

Attachments (1)

41339.diff (1.7 KB) - added by pbiron 8 years ago.
allow status__not_in parameter

Download all attachments as: .zip

Change History (5)

@pbiron
8 years ago

allow status__not_in parameter

#1 @pbiron
8 years ago

  • Keywords has-patch added

I went back-and-forth over whether the $status__not_in parameter should accept a space/comma separated list like $status does.

I decided to not allow that because it seems that is a "legacy" construct and $xxx__not_in parameters added more recently (in this class and others) only accept true arrays.

I'm certainly open to suggestions that it should allow a space/comma separated list.

#2 @pbiron
8 years ago

The patch I added does not contain the docblock fix from the related #41338.

#3 @boonebgorges
8 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Thanks for the patch! The idea seems pretty good to me. I agree with your logic that it's only necessary to support arrays.

We will need some tests to support the new parameter. Specifically, I would like to see tests that demonstrate how status__not_in works when used alongside status. It appears that if I pass the same value to both, then there will be no 'status' clause at all (since array_diff( [ 'foo' ], [ 'foo' ] ) is an empty array). I'm not sure what the best behavior is here, but in any case, it should be documented with unit tests.

#4 @pbiron
8 years ago

I'll write some unit tests when I get a chance.

I'll look at how other classes that have both $xxx (or $xxx__in) and $xxx__not_in parameters deal with the case where the two "cancel each other out".

Note: See TracTickets for help on using tickets.