[wp-trac] [WordPress Trac] #33587: Notification Functions Should be Replaced by Hooks

[WordPress Trac]

#33587: Notification Functions Should be Replaced by Hooks
-----------------------------------+------------------
 Reporter:  dshanske               |       Owner:
     Type:  enhancement            |      Status:  new
 Priority:  normal                 |   Milestone:  4.4
Component:  Mail                   |     Version:
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |     Focuses:
-----------------------------------+------------------
Changes (by boonebgorges):

 * keywords:  has-patch => has-patch 2nd-opinion


Comment:

 Hi all - So, most of the changes are in. The remaining work has to do with
 the notifications sent to the site admin and new user when the admin
 creates a user through the Dashboard. This happens in a bunch of places
 throughout wp-admin and wp-admin/network. thomaswm's suggestion was to
 introduce a single 'create_user' action in all the necessary places. But
 this seems like a bad action name, for reasons described above.

 jeremyfelt suggested perhaps using the 'user_register' hook. On further
 thought, this can't be done, at least not in a simple way - there are too
 many ways that `wp_insert_user()` is used, and many of them shouldn't
 involve email notifications.

 I've attached two patches with suggested solutions:

 1. [attachment:33587.5.diff] adds different action names in each place.
 The upside: this is pretty straightforward. The downside: it means placing
 a bunch of new action hooks in weird places. Putting `do_action()` calls
 into the business logic of files like `user-new.php` feels all sorts of
 wrong to me.
 2. [attachment:33587.6.diff] uses the 'user_register' hook. In order to
 make sure that email notifications are only sent when we want them, the
 patch adds a `$notify` param - defaulting to `false` - to
 `wpmu_create_user()`, `wp_create_user()`, and `wp_insert_user()`. The
 `$notify` value is then passed to the `'user_register'` hook, and we send
 the email based on this value, in
 `wp_maybe_send_new_user_notifications()`. Then, in all places in core
 where we want to send notifications, we pass `$notify = true`. Upside: we
 don't have to introduce any new actions. Downside: It means adding a very
 ugly param to the `wp_insert_user()` stack. In addition, it'll be hard to
 target specific emails for unhooking or modifying, since they'll all run
 through `'user_register'`; this sorta defeats the purpose of the ticket,
 which is to make it easier to modify specific email notifications.

 Any thoughts on these two strategies?

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