Opened 7 years ago
Last modified 7 years ago
#40661 new enhancement
WP_Term_Query->parse_query() needs filter like 'get_terms'
Reported by: | mpol | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Taxonomy | Keywords: | needs-unit-tests needs-patch |
Focuses: | Cc: |
Description
The new class/method WP_Term_Query->parse_query() does not provide a filter like the old function get_terms() does. Using WP_Term_Query directly has thus different functionality from using the old wp_terms function.
Using a plugin like:
https://wordpress.org/support/plugin/custom-taxonomy-order-ne/
with a custom term_order field and sorting based on that, gives the default sort based on name.
Adding a filter to the method like get_terms, provides backwards compatibility for plugin and theme authors, that are switching to WP_Term_Query directly.
This filter gives options to apply a custom order to the list of terms.
Reference on support forum:
https://wordpress.org/support/topic/doesnt-reorder-when-using-wp_term_query/
If adding a filter is somehow not wanted by core devs, then I will have to answer to add this filter manually to the code when support requests are coming. To be clear, I would consider that a valid response.
Attachments (4)
Change History (12)
#1
@
7 years ago
The patch in attachment 1 works for me, but I am definitely no core dev, so you want to look at it critically.
#2
follow-up:
↓ 3
@
7 years ago
- Keywords has-patch needs-refresh needs-unit-tests added
- Type changed from defect (bug) to enhancement
- Version trunk deleted
@mpol Your patch is missing the file name. Also, could you please document the filters?
How do the other ::parse_query()
methods handle this case?
#3
in reply to:
↑ 2
@
7 years ago
For consistency with the ::parse_query()
method in WP_Query
, it might make more sense to implement this similarly to the filter posts_pre_query
. See 40661.diff for my iteration of a terms_pre_query
filter.
This is more a proof of concept than anything else, I'm not sure if it makes sense to leave the burden of handling term caching as well as the hide_empty
and the pad_counts
parameters on the callback function.
#4
@
7 years ago
- Keywords needs-patch added; has-patch needs-refresh removed
Hi all - @mpol Thanks for the report, and @jarocks thanks for the follow-up patch.
For clarification, this appears to have nothing to do with parse_query()
. parse_query()
, in WP_Term_Query
and in other query classes, is where the arguments passed to the query()
method are parsed together with the default arguments and validated. WP_Term_Query::parse_query()
does, in fact, have an action available for plugins to intervene: parse_term_query
.
What's being requested here is a filter on the results of the query. The get_terms
filter still exists, but when WP_Term_Query
was introduced, it was decided to leave that filter in the get_terms()
wrapper function, for naming consistency.
@jarocks The pre-query filter you've suggested here is interesting, but I don't think it serves the purpose of this ticket. custom-taxonomy-order-ne and similar plugins work by modifying the results of the core query; posts_pre_query
is a way of preventing core queries from taking place.
Here's the pattern elsewhere in our query classes:
WP_Query
runs a filterposts_results
on query resultsWP_Comment_Query
runs a filterthe_comments
on query resultsWP_User_Query
has no results filter, either in the query class or in theget_users()
wrapper function
So there's not a great pattern to follow here. term_query_results
seems like a reasonable naming convention. What do others think?
#5
@
7 years ago
@boonebgorges with your comment in mind, I've moved my proposed filter into a separate ticket: 41246
#6
@
7 years ago
Thanks for commenting.
Sorry for being confused with the parse_query
function, I now see that the request should be about the get_terms
function.
I can agree with the naming function. I was hoping to reuse a filter name like get_terms or get_the_terms (there are too many already), but the parameters are different, so it will have to be a new name.
I have a better patch prepared, and will attach it. I added the filter right after the get_results
wpdb query, because caches get updated right away, and I think the filter should be run before the cache gets updated.
It works for me and the usecase of the referenced support topic.
Please rework as you see fit.
#7
@
7 years ago
Thanks, @mpol!
Having the filter placed as in filter-term-query-results.patch means that the cache will be populated with items returned from filters. This seems problematic to me, since an error by a developer could result in incorrectly cached content. Moreover, it means that the filter will run on values coming from a database query *as well as* values coming out of the cache, when the latter will already have been cached on the way into the database. As such, I'd suggest moving the filter so that it runs on what's been pulled from the cache. Does that seem right?
Note that the approach here (especially in light of the preceding paragraph) will probably need revisiting after #37189.
#8
@
7 years ago
Ah, interesting :). I didn't know caches only use raw data.
I am not sure of the implications of #37189, I will wait what happens with that.
For now I attached a new patch (see above).
The filter on line 672 is about getting data from cache, filtering it and returning it.
The same filter on line 804 is about fresh data that is being filtered, after it has been added to cache.
Add filter get_terms to WP_Term_Query->parse_query()