[wp-trac] [WordPress Trac] #36904: Add caching to comment feed in WP_Query

[WordPress Trac]

#36904: Add caching to comment feed in WP_Query
------------------------------------------+-----------------------------
 Reporter:  spacedmonkey                  |       Owner:
     Type:  enhancement                   |      Status:  new
 Priority:  normal                        |   Milestone:  Future Release
Component:  Query                         |     Version:  2.2
 Severity:  normal                        |  Resolution:
 Keywords:  needs-unit-tests needs-patch  |     Focuses:  performance
------------------------------------------+-----------------------------
Changes (by boonebgorges):

 * keywords:  needs-unit-tests => needs-unit-tests needs-patch


Comment:

 Thanks for your work on this, @spacedmonkey. I agree that this is a good
 strategy.

 Instead of doing `is_comment_feed()` checks inside of `WP_Comment_Query`,
 I'd prefer to control this using a parameter. Something like
 `do_comment_feed_filters` is ugly sounding, but since it's really a back
 compat argument anyway, I think it's OK. This could take the place of
 `suppress_filters`, which I don't see the need for more generally in
 `WP_Comment_Query`. So, in `WP_Query`:

 {{{
 $comment_args = array(
     // ...
     'do_comment_feed_filters' => ! $q['suppress_filters'],
     // ...
 );
 }}}

 and then in `get_comment_ids()` we just need to check `if (
 $this->query_vars['do_comment_feed_filters'] )`.

 Could you please move the filter docs over to `get_comment_ids()`?

 I'm not convinced, at a glance, that setting `post__in` to the IDs of
 `$this->posts` is going to do the work of the existing SQL. For one thing,
 I'm pretty sure `$this->posts` hasn't yet been set at this point. It's
 possible that `WP_Comment_Query` doesn't have enough internal logic to
 handle things like `WHERE ( post_status = 'publish' OR ( post_status =
 'inherit' && post_type = 'attachment' ) )`. If not, it may be possible to
 add it, either as general purpose parameters, or in the form of new
 parameters that are very specific to the use case of comment feeds. It'd
 be helpful to have tests for this, since it'll demonstrate more clearly
 what the cases are that need to be covered; looking at the code, I have a
 rough sense of what they are (though the generic `$where` is a wildcard),
 but having them explicit will make it easier to see what changes are
 required in `WP_Comment_Query` to make this change possible.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36904#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform