Skip to content
This repository was archived by the owner on Sep 24, 2018. It is now read-only.

Conversation

@BE-Webdesign
Copy link
Member

Fixes #2145, this was put into a new PR to make it cleaner. Original PR is in #2273. This enables tax_query in the filter param and validates that the taxonomies that are queried are show_in_rest => true

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`
@BE-Webdesign BE-Webdesign mentioned this pull request Oct 1, 2016
@BE-Webdesign BE-Webdesign added this to the 2.0 Beta 15 milestone Oct 4, 2016
kadamwhite added a commit to WP-API/node-wpapi that referenced this pull request Oct 5, 2016
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 ) );
Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Member Author

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?

Copy link
Contributor

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

@kadamwhite
Copy link
Contributor

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. tax_query definitely introduces some potential performance overhead, but I'm not sure whether it's our responsibility to limit that artificially in any way.

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 ) );
Copy link
Contributor

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.
Copy link
Member

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();
Copy link
Member

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();
Copy link
Member

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.

@kadamwhite
Copy link
Contributor

The flexibility of completely allowing tax_query was deemed too big a cat to let out of the bag in slack discussion today, so closing this. The ability to set an AND relation for taxonomy IDs included via {taxonomyname}[]=id query parameters is an open question to be addressed another day.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants