Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#54874 closed enhancement (fixed)

[New User Notification] Allow conditional mail supression

Reported by: janthiel's profile janthiel Owned by: johnbillion's profile johnbillion
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch needs-dev-note has-unit-tests
Focuses: Cc:

Description

wp_new_user_notification is a pluggable function allowing some amount of filtering.
The existing filter to allow content filtering but there is no way to fully prevent the mail from being sent.
For example in a case where an external component / plugin handles the notification mail for users with a certain role.

Using the plugable mechanism would require one to copy the complete function just to decide whether to send the mail or not.

This could be easily done by adding a $send flag to the already filterable $wp_new_user_notification_email_admin and $wp_new_user_notification_email.

Based on this a filter function can set this flag to false.
Currently the only way to do this is by emptying the to address in this filter which leads to wp_mail errors due to missing receipient.

Change History (14)

This ticket was mentioned in PR #2202 on WordPress/wordpress-develop by JanThiel.


2 years ago
#1

  • Keywords has-patch added

Allow selective sending of the "New User" Mails to admins and users by extending the existing filters by a send flag.
This allows filters to skip mail sending based on the context and user information.

Trac ticket: https://core.trac.wordpress.org/ticket/54874

#2 follow-up: @costdev
2 years ago

  • Keywords needs-unit-tests needs-dev-note 2nd-opinion added
  • Milestone changed from Awaiting Review to 6.0
  • Severity changed from minor to normal
  • Type changed from enhancement to feature request

Looks like a candidate for a similar filter to send_retrieve_password_email, one of the filters introduced as part of #54690.

  • The admin filter could go on the line just after the 'user' !== $notify condition.
  • The user filter could go just after $key is defined and validated.

I believe these locations would allow for the earliest short-circuit.

Pinging @SergeyBiryukov @johnbillion @audrasjb for 2nd-opinion.

#3 in reply to: ↑ 2 @janthiel
2 years ago

Replying to costdev:

  • The admin filter could go on the line just after the 'user' !== $notify condition.
  • The user filter could go just after $key is defined and validated.

I believe these locations would allow for the earliest short-circuit.

Thanks for your feedback on this :-)

Do I understand correctly that you propose to add additional filters for this purpose?
As I do not see any way to move the existing filters to anywhere earlier as they require the message to be composed to continue to work the way they do.

#4 @costdev
2 years ago

No problem!

That's right, new filters specifically for deciding whether or not to send the emails.

#5 @janthiel
2 years ago

@costdev I added two new filters to do the job and reverted the previous approach. They will use the existing short circuit points to follow the existing code patterns. Feels better and easier to follow than to introduce new checks at other points within the flow.

The two new filters are named a bit more verbose to allow an easier distinction of them:
send_new_user_notification_email_to_admin and send_new_user_notification_email_to_user.

Is that what you had in mind?

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#7 @audrasjb
2 years ago

I updated the PR for better support of WordPress Coding standards.
Looks good to go on my side.

#8 @costdev
2 years ago

@audrasjb, do you think we can go ahead and commit this, then reopen it to add unit tests during Beta 1?

This ticket was mentioned in PR #2586 on WordPress/wordpress-develop by costdev.


2 years ago
#9

  • Keywords has-unit-tests added; needs-unit-tests removed

Update to PR 2202 with @peterwilsoncc's suggestions and unit tests included.

Trac ticket: https://core.trac.wordpress.org/ticket/54874

#10 @costdev
2 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from 6.0 to 6.1
  • Type changed from feature request to enhancement

I think this is pretty much ready for commit.

However, with 6.0 Beta 1 released on Tuesday, I'm moving this to the 6.1 milestone.

#11 @audrasjb
2 years ago

@costdev it looks good to me. I added a commit in the PR to update @since mentions.

Last edited 2 years ago by audrasjb (previous) (diff)

johnbillion commented on PR #2202:


2 years ago
#12

Closing this in favour of #2586 which is ahead. Thanks everyone!

#13 @johnbillion
2 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 53698:

Users: Allow conditional supression of the email notifications that are sent when a new user account is registered.

This introduces the following new filters:

  • wp_send_new_user_notification_to_admin
  • wp_send_new_user_notification_to_user

Props janthiel, costdev, audrasjb, peterwilsoncc

Fixes #54874

Note: See TracTickets for help on using tickets.