[wp-trac] [WordPress Trac] #44467: Ensure meta wrapper functions do not contain additional logic

[WordPress Trac]

#44467: Ensure meta wrapper functions do not contain additional logic
-------------------------------------------------+-------------------------
 Reporter:  flixos90                             |       Owner:  flixos90
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.0
Component:  Options, Meta APIs                   |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  2nd-opinion has-patch needs-unit-    |     Focuses:  multisite,
  tests                                          |  rest-api
-------------------------------------------------+-------------------------

Comment (by flixos90):

 Comparing the two patches, to a large extend they are very similar. Both
 rely on filters to handle all the necessary functionality. Some
 observations / differences:

 * [attachment:"44467.diff"] adds filters for setting the `last_changed`
 value more granularly while [attachment:"44467.2.diff"] is more
 abstracted. Since there is no clear pattern though on whether object types
 even use `last_changed` (core's do, but that doesn't mean all others
 necessarily do), I think we should keep this specific. Furthermore, the
 abstracted approach requires the introduction of additional actions
 (without including `$meta_type` in the name), and I don't particularly
 like that, given that there are already a bunch of actions being triggered
 for the very same thing. Furthermore, it wouldn't make sense having their
 names in present tense (`add_metadata` ...) because the existing actions
 triggered at the same time are named in past tense
 (`added_{$meta_type}_meta` ...)
 * [attachment:"44467.diff"] takes care of the functions that change
 metadata by meta ID. Those aren't handled in [attachment:"44467.2.diff"].
 * When handling term meta, [attachment:"44467.diff"] does not touch the
 current check for a shared term, which returns a `WP_Error`, while
 [attachment:"44467.2.diff"] puts that check in a filter. This would
 generally make sense, however `WP_Error` is not a possible return value of
 any metadata function, and in the patch's implementation would actually be
 cast to a boolean, which would make it `true`. For backward-compatibility
 reasons, that check should remain in the wrapper function.

 Other than that, the patches work quite similar, only have a bit different
 function names etc. For the above reasons I suggest proceeding with
 [attachment:"44467.diff"].

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