[wp-trac] [WordPress Trac] #43941: Add default value to register meta

WordPress Trac noreply at wordpress.org
Tue Jun 26 22:41:10 UTC 2018


#43941: Add default value to register meta
----------------------------------------+---------------------------
 Reporter:  spacedmonkey                |       Owner:  spacedmonkey
     Type:  enhancement                 |      Status:  assigned
 Priority:  normal                      |   Milestone:  5.0
Component:  Options, Meta APIs          |     Version:  4.6
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:  rest-api
----------------------------------------+---------------------------

Comment (by spacedmonkey):

 Thanks for the feedback @flixos90 . My response

   Only one of the two filters should be applied. Look at sanitize_meta()
 for example to see how it handles those filters. I'd suggest checking !
 empty( $object_subtype ) && has_filter(
 "default_{$object_type}_meta_{$meta_key}_for_{$object_subtype}" ), and if
 so only run the subtype-filter and return immediately. Otherwise, fall
 back to "default_{$object_type}_meta_{$meta_key}".

 I am not sure I like this idea. I personally hate conditional filters in
 core, it is means that developers have to route through code and fine out
 when filters are fired. This is confusing. Firing both filters here does
 the same thing functionally, not sure why we need to change it.

   The function get_metadata_default() would be a lot simpler and more
 flexible if it only executed the filters and didn't touch $wp_meta_keys.
 The actual logic to get the registered default value should be added as a
 filter hook via register_meta() (and also removed again via
 unregister_meta_key(). This would be in line with how default values are
 registered for settings.

 So my original patch did this, but I couldn't get it working without with
 either,
 - Using an an anonymous function, which are not supported in PHP 5.2
 - Using `create_function`, which is not supported in later versions of
 PHP.
 - Create two new functions, that are single use. This feels a little
 pointless and weird. Might just be me. Am I missing something here?

   The argument for the default value should be called default instead of
 default_value. This is in line with how it works for settings.
 I would agree with that. I wanted it to be super clean, it we all believe
 default is fine, I will change it.

   get_metadata_default() as well as the two filters need to be aware of
 the value of $single, in order to allow developers full flexibility in
 handling this. The default value may need to be different, depending on
 how a meta value is requested.

 How do you intend this works? So in register_meta, should it have
 `default_single` and `default_multi`. If so, if `default_multi` isn't set,
 does it fallback to `default_single`.This doesn't feel right. I personally
 think of defaults as defaulting a single value. If anything, I wouldn't
 support defaulting the multiple values.

 Regarding docs, as for unit tests, the docs were not correct at all. Just
 there to help review . Once this patch is in a more final state, as in we
 have locked down the design, I will comment for real.

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


More information about the wp-trac mailing list