[wp-trac] [WordPress Trac] #39926: wp_get_object_terms should return WP_Error on wrong fields argument or use a sane default

[WordPress Trac]

#39926: wp_get_object_terms should return WP_Error on wrong fields argument or use
a sane default
------------------------------------+------------------------------
 Reporter:  BjornW                  |       Owner:
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Taxonomy                |     Version:  trunk
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |     Focuses:
------------------------------------+------------------------------
Changes (by BjornW):

 * keywords:   => has-patch dev-feedback


Comment:

 Replying to [comment:2 dd32]:
 > I'm actually OK with the SQL error being generated in this case - it's a
 developer error which they'll otherwise get no notification of, garbage in
 = garbage out type thing.

 I agree with the sentiment of 'garbage in, garbage out'. My initial
 approach towards solving this was to send an explicit error back instead
 of letting the code continue executing and generating invalid SQL. However
 a default fall-back seems more elegant and still conveys the message
 towards the developer that something is not right. Happy to hear you agree
 on this approach :)

 > If [attachment:fix-39926.diff] is the route taken, the `default:` should
 be added to the `all` branch of the `switch` (not duplicating it by
 itself), and it could probably set `$args['fields']` to `'all'` for later
 use?

 I see the appeal of not duplicating lines of code, however mixing explicit
 valid field values with a default 'catch all' does not seem right to me.
 The 'default' in the switch is now explicitly used as a catch all, as
 you'd expect it to do. That's why I opted for adding a default to the
 switch instead of combining it. What do you think about this reasoning?

 I've updated my patch to include settings the fields value to 'all' to
 make sure any further use of the variable will continue as intended.

 ps: Thanks for your quick review & feedback!

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