Opened 7 years ago
Last modified 6 years ago
#40477 new defect (bug)
REST API: Does NOT Trigger New User Notifications!
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Users | Keywords: | dev-feedback has-patch needs-testing needs-unit-tests |
Focuses: | rest-api | Cc: |
Description
If you create new users with WP REST API. The notification for new WordPress users via email does NOT get triggered. I tried it on a fresh install. Used the [Email log](https://wordpress.org/plugins/email-log/) WP plugin to test that no emails were sent.
Attachments (3)
Change History (16)
#1
@
7 years ago
- Component changed from General to Users
- Keywords dev-feedback added
- Summary changed from WP REST API — Does NOT Trigger New User Notifications! to REST API: Does NOT Trigger New User Notifications!
- Version changed from 4.7.3 to 4.7
#2
@
7 years ago
- Keywords has-patch needs-testing added
@swissspidy
Added the ticket after discussion with @rmccue
I just created a patch which is now triggering notifications. Calling wp_new_user_notification();
did the trick.
Looking forward!
This ticket was mentioned in Slack in #core-restapi by mrahmadawais. View the logs.
7 years ago
#4
@
7 years ago
Hi @mrahmadawais,
Maybe we should use wp_send_new_user_notifications()
and trigger it after is_wp_error
? What do you think?
Shouldn't this also apply to users created in a multisite or is newuser_notify_siteadmin()
hooked into wpmu_create_user()
sufficient?
#5
@
7 years ago
@websupporter Thanks for pointing out another function. It does what I was doing. Updated the patch.
#6
@
7 years ago
- Keywords needs-unit-tests added
wp_send_new_user_notifications()
is added to a few hooks, for example add_action( 'register_new_user', 'wp_send_new_user_notifications' );
. 40477.2.patch means one cannot opt-out of this.
#7
@
7 years ago
Yeah, I think this should be attached to a hook of some kind. Ideally, this would also be controlled by a param in the POST body. This would also match WP Admin's add new user form.
#8
@
7 years ago
Yes, after doing some research, I think so too. Actually, I think, we should use 'register_new_user'
like in user.php:
/** This action hook is documented in wp-includes/user.php **/
do_action( 'register_new_user', $user_id );
Ref.: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/user.php#L2266
So, if someone dehooked the notice for the normal user registration (on wp-login.php), the notice would also not be send via the REST API. But maybe to register a user via the REST API can be seen as a new action seperate of the user registration on wp-login.php in which case a new action would be sensible.
Or, actually this convinces me more, we see a registration via the REST API more aligned with the action in the wp-admin area. In this case, we should probably use
'edit_user_created_user'
.
/** This action hook is documented in wp-admin/includes/user.php **/
do_action( 'edit_user_created_user', $user_id, $notify );
Ref.: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/user.php#L197
The $notify
in the admin is populated via $_POST
. If you mark the checkbox to send the password to the new user $notify
will be 'both'
, otherwise it will be 'admin'
.
I was thinking about @TimothyBlynJacobs comment
Ideally, this would also be controlled by a param in the POST body.
I think, this would be actually nice, since this means, we could easily "Restify" the "Create New User"-form in the backend 1:1. But I can't figure out a param which on the other side meets the criteria to be general enough for a API param. Somehow I dislike a notify
parameter here.
Right now all params are somehow information about the new user (name, email, locale etc.). A notify
-param is not an additional information but triggers an action and breaks with the existing schema.
Maybe someone comes up with a better idea or my concerns are somehow mislead here, but if not:
A notify
-param would switch the 'admin'
/'both'
-behavior (if we go with the current behavior). If we are not using such a param, we would need to decide, which value $notify
should have.
Coming from the original problem of this ticket, I think we should use both
, otherwise, we would miss the topic :)
The notification for new WordPress users via email does NOT get triggered.
This would be also inline with the standard behavior of wp_send_new_user_notifications()
.
#9
@
7 years ago
@swissspidy Makes sense.
I agree with what @websupporter said, we can have a notify
param that could be set with the REST API. If not set, it could default back to the default WP behavior.
#10
follow-up:
↓ 12
@
7 years ago
I have updated the patch to use
/*
* Trigger default welcome email notifications.
*
* This action hook is documented in wp-admin/includes/user.php
*
* Add second param $notify to override the behavior.
*/
do_action( 'edit_user_created_user', $user_id );
This will favor the default notification behavior as well as provide the ability to opt-out by defining a custom value for $notify
as the second argument.
This ticket was mentioned in Slack in #core-restapi by mrahmadawais. View the logs.
7 years ago
#12
in reply to:
↑ 10
@
7 years ago
Replying to mrahmadawais:
I have updated the patch to use
'edit_user_created_user'
.
This will favor the default notification behavior as well as provide the ability to opt-out by defining a custom value for
$notify
as the second argument.
In 40477.3.patch I'm not seeing any functionality in place to specify $notify
. What do you mean by this?
We discussed a bit more in Slack, and haven't reached a conclusion yet. While I recognize the desire for this functionality, I'm not a big fan of adding a parameter like notify
because it's not RESTful (it doesn't REpresent the State of an object).
A potential solution would be adding a new filter like rest_user_created_notification
that plugins can override. Until then, you can use register_rest_field
to add a suitable parameter here.
I also wish that we had enabled this behavior by default, for both users and comments.
#13
@
6 years ago
This can currently be accomplished without core modification in this way:
<?php function mynamespace_user_update($user, $request, $create) { $user_id = $user->ID; wp_send_new_user_notifications($user_id); } add_action( 'rest_insert_user', 'mynamespace_user_update', 12, 3 );
But, yeah, a patch to make this happen by default seems like it would be nice, especially if there's also a hook to modify it to not send the messages.
Related: #40352.