Opened 9 years ago
Closed 9 years ago
#36724 closed defect (bug) (fixed)
who=>authors breaks meta arguments in user queries
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Users | Keywords: | |
Focuses: | Cc: |
Description
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.
Attachments (4)
Change History (14)
#2
@
9 years ago
- Keywords dev-feedback has-patch added; needs-patch removed
Following instructions from @boonebgorges I've added "who" lines to the "meta_query".
#3
@
9 years ago
- Keywords needs-patch added; dev-feedback has-patch removed
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
.
#4
@
9 years ago
- Keywords dev-feedback has-patch added; needs-patch removed
Ups @boonebgorges, you are right! I've fixed this and attached a new diff. Please check it out.
#6
@
9 years ago
- Keywords needs-patch added; dev-feedback has-patch 2nd-opinion removed
- Milestone changed from Awaiting Review to 4.6
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.
#7
@
9 years ago
- Keywords has-patch dev-feedback 2nd-opinion added; needs-patch removed
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 :)
#8
@
9 years ago
- Keywords needs-unit-tests good-first-bug has-patch dev-feedback 2nd-opinion removed
- Owner set to boonebgorges
- Status changed from new to assigned
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.
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.