Comment meta queries cannot be set in WP_Comment_Query via pre_get_comments

[needle]

This is a follow-up to #30478 (via [31467]) which introduced the $meta_query_clauses local variable to the get_comments() method.

An unintended side effect of pre-processing meta queries into this variable is that one can no longer use the pre_get_comments filter to create meta queries of type WP_Meta_Query to alter the query.

See, for example, this post on the forums:
​https://wordpress.org/support/topic/wordpress-421-pre_get_comments-doesnt-work-after-update

This is a critical issue for plugins which hide/show comments based on a comment meta value - in most cases all previously private, hidden comments will now be public.

[boonebgorges]

Thanks for the report, needle. I have a few thoughts about this.

First, there is definitely a regression for people who were doing modification of the meta_query argument at pre_get_comments.

Second, it's not obvious to me how the example in ​https://wordpress.org/support/topic/wordpress-421-pre_get_comments-doesnt-work-after-update?replies=8 ever would have worked. Since comment meta queries were introduced [22074], parse_query_vars() has been run *before* pre_get_comments. That means that modifying $query->query_vars['meta_key'] should have had no effect.

All this being said, it seems to me to be a mistake that the meta query is initialized at all before pre_get_comments, as it will inevitably lead to cases like this. (For reference, WP_Query sets up its meta query just *after* pre_get_posts https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-includes/query.php?marks=2396,2405,2406#L2385.) Moving the entire meta_query block below pre_get_comments will probably introduce backward compatibility issues for people who are expecting the meta_query to be set up at the hook. So I'm going to suggest 32762.diff​, which reparses the query vars after pre_get_comments, and only then generates the SQL clauses. needle, does this seem right to you?

[needle]

@boonebgorges Thanks for the feedback. I wasn't pointing to the forum post because of the code therein, but rather to show that others had noticed that meta queries built via pre_get_comments were failing ;-)

I've managed to get things working as expected by using the parse_comment_query filter instead, since it fires prior to anything I've highlighted above. However, the pre_get_comments hook seems to be the one that all the discussion I've seen online seems to suggest as the entry point for filtering comments.

Although it looks like the parse_comment_query filter was introduced in [31793], my main concern is how plugins support comment meta query filtering across the different possible WordPress installs that may or may not have this filter. Your patch would seem to address the issue for my purposes, but as you say, may introduce backpat issues for others.

[boonebgorges]

Thanks, needle.

Your patch would seem to address the issue for my purposes, but as you say, may introduce backpat issues for others.

The patch, as written, should not introduce backward compatibility concerns - it's just kinda ugly, because of the need for multiple calls to parse_query_vars(). But let's go with it anyway.

[needle]

Replying to boonebgorges:

The patch, as written, should not introduce backward compatibility concerns

Phew. Sounds good to me, thanks!

[boonebgorges]

In 32911:

In WP_Comment_Query, parse meta_query vars after the pre_get_comments hook.

[31467] included a change that involved generating meta_query SQL before the
pre_get_comments hook, with the result that pre_get_comments callbacks were
no longer able to modify comment meta queries. We fix the problem by moving the
SQL generation to after the hook.

This changeset also includes a second call to meta_query->parse_query_vars(),
to ensure that modifications to metadata-related query vars (such as meta_key
and meta_value) performed in pre_get_comments callbacks have the expected
effect on the comment query.

Fixes #32762.

[side777]

That means that modifying $query->query_vars['meta_key'] should have had no effect.

That's why you had to call meta_query->parse_query_vars() again after altering the meta query. Developers are aware of that. (See the example)

It seems that we now have introduced a third call... but I'm glad it's fixed!

[kasparsd]

#23469 was marked as a duplicate.