[wp-trac] [WordPress Trac] #50848: Clarify the usage of null for "auto_update_{$type}" filter

[WordPress Trac]

#50848: Clarify the usage of null for "auto_update_{$type}" filter
--------------------------------------------------+-----------------------
 Reporter:  SergeyBiryukov                        |       Owner:  audrasjb
     Type:  defect (bug)                          |      Status:  accepted
 Priority:  normal                                |   Milestone:  5.5.1
Component:  Upgrade/Install                       |     Version:  3.7
 Severity:  normal                                |  Resolution:
 Keywords:  has-patch dev-feedback needs-testing  |     Focuses:  docs
--------------------------------------------------+-----------------------

Comment (by pbiron):

 Following up from the
 [https://wordpress.slack.com/archives/CULBN711P/p1598376129025600 #core-
 auto-updates] meeting this morning, it seems to me that in all of the
 places where `null` is passed, passing `false` would be wrong.

 For example, looking at [https://core.trac.wordpress.org/browser/trunk/src
 /wp-admin/includes/class-wp-plugins-list-table.php#L236 wp-admin/includes
 /class-wp-plugins-list-table.php, L236-240]:

 {{{#!php
 $auto_update_forced = apply_filters( "auto_update_{$type}", null,
 $filter_payload );
 if ( ! is_null( $auto_update_forced ) ) {
         $plugin_data['auto-update-forced'] = $auto_update_forced;
 }
 }}}

 The intention of that code is to detect whether anything has hooked into
 that filter and is returning a truthy or falsey value...so that the UI can
 display "Auto-updates enabled" or "Auto-updates disabled" as plain text
 instead of displaying the enable/disable links.  Hence, passing `false`
 would be the wrong thing to do there.

 The uses in site health are similar: if the filter returns a truthy or
 falsey value (instead of `null`) then that value overrides whether the
 user preference stored (or not) in the site option when constructing the
 text for whether the plugin/theme will auto-update.  And so, passing
 `false` there would be wrong as well.

 And the use in single-site themes.php is also the same.

 So, I think the code needs to stay as it is and the DocBlock for the
 filter needs to be updated to change the type of the `$update` parameter
 to `bool|null` and to describe under what circumstances null will be
 passed.

 I should also point out that the potential complications of using these
 filters for other than their originally intended purpose (which is to
 decide, at the time auto-updates are about to be done, whether they should
 be done or not) was anticipated and discussed, for instance in the
 [https://wordpress.slack.com/archives/CULBN711P/p1588700879253200 #core-
 auto-updates meeting on May 5, 2020].

 In particular, the comments from @azaozz :

 > still feeling somewhat.. "unconfortable" doing that though as it will
 use an existing filter that is pretty popular (used in many plugins) and
 the new use is for the UI, totally unexpected

 and

 > But the use for the UI will definitely be unexpected, so there will
 likely be edge cases

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