who=>authors breaks meta arguments in user queries

[Rarst]

When using who=>authors in user query it silently overrides meta_key, meta_value, and meta_compare arguments.

Among other things this makes it impossible to orderby=>meta_value such query, since it orders by that key.

This behavior is neither explicitly documented or desirable.

[boonebgorges]

Good call. It should be possible to rearrange this so that 'who' is added as a 'meta_query' clause instead of overriding 'meta_key' and 'meta_value'. The roles/role__in/role__not_in params work this way.

[adrianosilvaferreira]

WP User Class diff

[adrianosilvaferreira]

Following instructions from @boonebgorges I've added "who" lines to the "meta_query".

[boonebgorges]

Thanks for the patch, @adrianosilvaferreira! Unfortunately, this approach won't work for a couple reasons. It will create a meta_query parameter that is not well-formed: meta queries must be multi-dimensional arrays, and each key/value pair must be part of its own array:

'meta_query' => array(
    array(
        'key' => 'foo',
        'value' => 'bar',
    )
)

You're missing a level of nesting. Also, if you simply append a clause to the existing meta_query, you may cause problems if the top-level 'relation' is 'OR'.

To make this work right, a new clause will need to be created for the 'who' parameter, and that clause will need to be joined to the existing meta_query using relation=AND.

[adrianosilvaferreira]

Ups @boonebgorges, you are right! I've fixed this and attached a new diff. Please check it out.

[boonebgorges]

Thanks, @adrianosilvaferreira. This patch is looking closer. However, the way you've structured it, a 'who' parameter will override any 'meta_query' passed to the function. You will need to assemble a meta query clause for the 'who' stuff, and then append it to the value of 'meta_query', in the same way that we do with the $role_queries clause in the same method.

We will also need unit tests that demonstrate that 'meta_query' and 'who' work together properly.

[adrianosilvaferreira]

Thanks @boonebgorges for looking into it. I've made a few changes and attached a new patch, I hope that it's correct now, fingers crossed :)

[boonebgorges]

Thank you, @adrianosilvaferreira. This patch is looking pretty good.

In writing a unit test, I found a previous bug that sorta needs to be addressed in order to fix this ticket.

Short version: because of an accident of implementation, it was possible to pass a malformed meta_query when using the who parameter and still have the meta_query recognized.

Long version: https://core.trac.wordpress.org/browser/tags/4.5.1/tests/phpunit/tests/user/query.php?annotate=blame&marks=690-693#L670 shows that a passing test is using improper syntax for meta_query:

'meta_query' => array(
    'key' => 'foo',
    'value' => 'bar',
)

is missing a level of nesting. It should be:

'meta_query' => array(
    array(
        'key' => 'foo',
        'value' => 'bar',
    ),
)

The offending test was introduced in [32207]. In that changeset, 'who' params were translated to meta_value and meta_key before WP_Meta_Query::parse_query_vars() was called. In parse_query_vars(), the presence of a "primary meta query" and "existing meta query" resulted in the two being concatenated in such a way that the invalid meta_query was inadvertantly nested in an array, accidentally turning it into proper syntax. However, I think that the bug was probably originally introduced in [30094], which caused parse_query_vars() to be run both before and after the role parameter.

In the vast majority of cases, the improperly nested syntax described above will simply be ignored by WP_Meta_Query. It's only when combined with who in the context of WP_User_Query that it will accidentally work. So, while it's technically a compatibility break to remove this support (which inadvertently happens with class-wp-user-query.4.diff​), I think it's an acceptable break that is very unlikely to affect anyone.

[boonebgorges]

In 37359:

Tests: Correct 'meta_query' syntax in test related to WP_User_Query 'who' param.

The test, introduced in [32207], used the incorrect syntax for 'meta_query' -
one fewer level of array-nesting than what WP_Meta_Query requires. This
slip uncovered a bug introduced into WP_User_Query in [30094], whereby
an incorrectly formatted 'meta_query' parameter would be properly parsed by
WP_User_Query when passed alongside who=authors.

We need to fix the inconsistent syntax in order to resolve #36724.

See #36724, #32019, #23849, #27026.

[boonebgorges]

In 37360:

Users: The 'who' parameter should not interfere with 'meta_key' + 'meta_value' in WP_User_Query.

Props adrianosilvaferreira.
Fixes #36724.