[wp-trac] [WordPress Trac] #35381: Introduce `WP_Term_Query`

[WordPress Trac]

#35381: Introduce `WP_Term_Query`
--------------------------------------------------+------------------
 Reporter:  boonebgorges                          |       Owner:
     Type:  defect (bug)                          |      Status:  new
 Priority:  normal                                |   Milestone:  4.6
Component:  Taxonomy                              |     Version:
 Severity:  normal                                |  Resolution:
 Keywords:  has-patch needs-testing dev-feedback  |     Focuses:
--------------------------------------------------+------------------

Comment (by boonebgorges):

 I've spent some time over the last few days with this issue.
 [attachment:35381.2.diff] is an attempt to reconcile @flixos90's original
 patch with a more modest approach.

 The largest hurdle in [attachment:35381.diff] is the modification of the
 query so that it fetches only term IDs. `get_terms()` currently needs to
 fetch full term objects, because it does some post-query processing on
 them to handle 'child_of' and empty subcategories. See
 https://core.trac.wordpress.org/browser/tags/4.5.2/src/wp-
 includes/taxonomy.php?marks=1662-1696#L1644. It was the absence of this
 logic that was causing many of the unit test failures I described in my
 comment above.

 Querying for term IDs, rather than full term objects, is something I would
 absolutely like to do. See #34239. But I feel that if we insist on it as a
 prerequisite for `WP_Term_Query`, it's going to hold up the current ticket
 forever. So I've rolled that change out in [attachment:35381.2.diff].

 I changed a couple of other things from the original patch, too.

 * Pagination/`FOUND ROWS` stuff has been removed. We don't expose this
 anywhere in the interface, so I don't think there's a need for it here. If
 someone wants it in the future, we can talk about it then.
 * Since we're still querying for full term objects, there's no need for
 `_prime_term_caches()`. Though see #36814, where it will likely be
 introduced in the context of `get_the_terms()`.
 * I don't think we need or necessarily want 'suppress_filters'. It's been
 removed.
 * I moved the 'get_terms' filter so that it runs only in `get_terms()`.
 The other filters have to stay where they are, for obvious reasons.
 * Some of the bits of argument parsing that you'd left in `get_terms()`, I
 moved to `WP_Term_Query`. I think we need most of it in every case where
 we're fetching terms. If you disagree, I'd be interested to hear the
 reasons.
 * I broke some 'orderby' logic into a new method `parse_orderby_meta()`.
 This is mainly to break up the huge `parse_orderby()` method, which is
 full of conditional statements. Like you, I left the 'get_terms_orderby'
 filter before the meta_query orderby stuff is parsed, but I wonder if
 anything would actually break by moving it afterward - it seems like, in
 fact, it may fix potential bugs.

 Thoughts are welcome. I'll come back to this patch in a few days when I
 have a clearer head to reassess.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35381#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform