Make WordPress Core

Opened 9 years ago

Last modified 3 days ago

#41339 new enhancement

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

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

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 (2)

41339.diff​ (1.7 KB ) - added by pbiron 9 years ago.
allow status__not_in parameter
41339_pr14254_20261013.diff​ (5.7 KB ) - added by sawf1y 3 days ago.
A slightly changed existing patch with code

Download all attachments as: .zip

Change History (7)

@pbiron
9 years ago

allow status__not_in parameter

#1 @pbiron
9 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
9 years ago

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

#3 @boonebgorges
9 years ago

  • Keywords needs-unit-tests added
  • Milestone Awaiting Review → 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
9 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".

This ticket was mentioned in ​PR #14254 on ​WordPress/wordpress-develop by ​@sawf1y.


3 days ago
#5

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

Added tests for new query parameter status__not_in. Also added code with status__not_in. I used already existing code by ​@pbiron with cosmetics changes

Trac ticket: 41339

Use of AI Tools

AI assistance: Yes
Tool(s): Claude
Model(s): Sonnet 5.5
Used for: Consulted how to check that code with new status works fine (as I still not intelligent with WordPress).

@sawf1y
3 days ago

A slightly changed existing patch with code

Note: See TracTickets for help on using tickets.