[wp-trac] [WordPress Trac] #50521: comments_pre_query doesn't align with other pre_query filters

WordPress Trac noreply at wordpress.org
Wed Jul 1 14:29:58 UTC 2020

#50521: comments_pre_query doesn't align with other pre_query filters
 Reporter:  dinhtungdu    |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Comments      |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:

Comment (by adamsilverstein):

 Hello @dinhtungdu thanks for opening this ticket!

 Reviewing the original ticket I see in the discussion the decision to not
 set `$this->comments` was explicit, see this comment:
 https://core.trac.wordpress.org/ticket/45800#comment:18 - I removed
 setting in the patch following that comment.

 The reasoning at the time (which I am happy to revisit) is that the
 function enables passing `count=true` and in this case we would be setting
 $this->comments to the count, not the comments. So if we do assign the
 returned value we need more logic about what/how to assign.

 > For example, a theme/plugin hooks into comments_pre_query, and we use
 WP_Comment_Query to query comments.

 I'm not sure I understand your use case. The `pre_` filter here is
 designed to e''nable skipping the regular WordPress db requests'' for
 getting the data - instead, data could come from a cache or any datastore,
 for example Elasticsearch. If you use `WP_Comment_Query` in your filter
 callback, does that mean you are using a WP db lookup anyway?

 Can you give some wider context for your use case, how you plan to use the
 `pre_` filter in your code?

 > This is inconsistent with other pre_query filters behavior of other
 objects (User, Term, ...). Also, it creates a minor inconvenience and
 possibly a performance issue when querying comments.

 Do these other `pre` filters include the ability to return a count?

 > Note that the doc of WP_Comment_Query is incorrect

 Can you please open a separate ticket with a patch to correct that
 (assuming this is inline doc you are talking about)?

Ticket URL: <https://core.trac.wordpress.org/ticket/50521#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform

More information about the wp-trac mailing list