[wp-trac] [WordPress Trac] #37198: Use `WP_Term_Query` for `wp_get_object_terms()`

[WordPress Trac]

#37198: Use `WP_Term_Query` for `wp_get_object_terms()`
-----------------------------------+------------------
 Reporter:  flixos90               |       Owner:
     Type:  enhancement            |      Status:  new
 Priority:  normal                 |   Milestone:  4.7
Component:  Taxonomy               |     Version:  4.6
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:
-----------------------------------+------------------

Comment (by flixos90):

 Replying to [comment:5 boonebgorges]:
 > Your patch is extremely conservative, in the sense that (a) it
 guarantees 100% backward compatibility, and (b) it attempts not to touch
 any existing code beyond what must be touched to refactor
 `wp_get_object_terms()`. I like this as a general approach :) But in this
 case, I think that we can simplify our strategy by looking a bit deeper at
 the back compat issues.

 I'm glad you see it that way. :) I wasn't sure about how backward-
 compatible it should be, but indeed, especially the `post_process_terms()`
 logic in my patch to take care of previous handling in
 `wp_get_object_terms()` was kind of painful, so I like handling it in a
 unified way, plus it's smaller and better readable like you said.

 > IMO, passing 'parent' when the taxonomy is non-hierarchical is not
 meaningful, and it's OK to break it - but I would be interested to hear
 what you think.

 I agree, returning an empty result when passing a `parent` to a non-
 hierarchical taxonomy is actually what I would expect it to return.

 Your other changes make sense to me as well. I'm not exactly sure about
 whether getting rid of the extra sanitization I did could introduce BC
 issues, but you probably have a better overview of that. I think we should
 mention this in a dev-note, and maybe get this in some time soon to see if
 anything breaks.

 One thing I'd like to add back in is only allowing "object_id-related"
 query vars if `object_ids` is passed and not empty. On the one hand,
 providing `fields => all_with_object_id` doesn't make sense without
 passing `object_ids` anyway, but on the other hand we will be more
 foolproof by preventing it entirely. The same applies to sorting by term
 order (this can break the SQL query atm), although I don't like how I did
 it in the first patch (the method is encapsulated otherwise, but for this
 particular reason accesses a class property - passing a bool like
 `$has_object_ids` to the method is structurally better I think).

 Are you planning to add documentation and default params back in? You
 could probably merge a lot of my first patch for that. Looking forward to
 see a comprehensive patch. :)

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