-
Notifications
You must be signed in to change notification settings - Fork 651
#2145 tax query in filter param #2748
#2145 tax query in filter param #2748
Conversation
…I#2145-tax-query-in-filter-param
Fixes WP-API#2145, this was put into a new PR to make it cleaner. Original PR is in WP-API#2273. This enables `tax_query` in the `filter` param and validates that the taxonomies that are queried are `show_in_rest => true`
This PR provides a convenience method for setting the `tax_query` filter:
```js
// Return only posts in category 29
site.posts().taxQuery({
taxonomy: 'category',
terms: [ 29 ]
})...
```
Note that if a relation is specified, numeric indices must be provided:
```js
// Return posts in (cats 49 or 29) && in tag 79 && NOT in tag 106
const request = site.posts().taxQuery({
relation: 'AND',
'0': {
taxonomy: 'category',
terms: [ 49, 29 ]
},
'1': {
taxonomy: 'post_tag',
// field: 'slug',
terms: [ 79 ]
},
'2': {
taxonomy: 'post_tag',
terms: [ 106 ],
operator: 'NOT IN'
}
});
```
_If_ "OR" can be used as the topmost relation, an array of tax_query
objects can be provided to the filter method instead; however, any
nested queries must use array indices because the array -> object
parsing is not recursive.
```js
// Implicit OR: this query is, all posts with
// (tag #93 && category #49) || (tag #81 && not tag #116)
site.posts().taxQuery([{
relation: 'AND',
'0': {
taxonomy: 'category',
terms: [49]
},
'1': {
taxonomy: 'post_tag',
terms: [93]
}
}, {
relation: 'AND',
'0': {
taxonomy: 'post_tag',
terms: [81]
},
'1': {
taxonomy: 'post_tag',
terms: 116,
operator: 'NOT IN'
}
}]);
```
| if ( 'taxonomy' === $key ) { | ||
| // If $taxonomies is empty by default set it. On reoccurring calls the function overhead is avoided. | ||
| if ( empty( $taxonomies ) ) { | ||
| $taxonomies = wp_list_filter( get_object_taxonomies( $this->post_type, 'objects' ), array( 'show_in_rest' => true ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could/should this check & assignment go at the top of this block, before the foreach? I doubt there's much perf impact here but since the list of taxonomies that apply to this request's post type won't be changing during this loop, the list of $taxonomies could be declared outside the foreach loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also "subsequent" is probably a better descriptor in that comment than re-occurring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using tax_query in combination with the top level params we have, keeping this call here inside the loop you avoid ever checking against taxonomies if you don't need to. For instance if you were going to do something like ?categories=1,4&filter[tax_query][relation]=AND you would not be checking against taxonomies again that were originally done in the categories=1,4 part of that. If the code doesn't need to run, why run it?
Subsequent sounds better to me too, I can change that. Or maybe "following calls" would be good to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either/or on the wording!
It took me a second to understand what you meant in terms of the ?categories impact, but that makes sense to me. +1
|
This does what it says on the tin; tested it out fairly fully with WP-API/node-wpapi#243 and it's behaving as I would expect. |
| if ( 'taxonomy' === $key ) { | ||
| // If $taxonomies is empty by default set it. On reoccurring calls the function overhead is avoided. | ||
| if ( empty( $taxonomies ) ) { | ||
| $taxonomies = wp_list_filter( get_object_taxonomies( $this->post_type, 'objects' ), array( 'show_in_rest' => true ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either/or on the wording!
It took me a second to understand what you meant in terms of the ?categories impact, but that makes sense to me. +1
| /** | ||
| * Validate tax query. | ||
| * | ||
| * @param mixed $value Value of the parameter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong param name here. Should be $values.
|
|
||
| public function test_get_items_valid_tax_query_in_filter() { | ||
| $term1 = $this->factory->term->create( array( 'taxonomy' => 'category', 'name' => 'basie' ) ); | ||
| $post1 = $this->factory->post->create(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, but no need to assign the $post1 and $post3 variables since you don't use them.
| } | ||
|
|
||
| public function test_get_items_invalid_tax_query_in_filter() { | ||
| $post1 = $this->factory->post->create(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, but no need to assign the $post1 variable, it isn't used.
|
The flexibility of completely allowing |
Fixes #2145, this was put into a new PR to make it cleaner. Original PR is in #2273. This enables
tax_queryin thefilterparam and validates that the taxonomies that are queried areshow_in_rest => true