Users: Do not send an email change notification when there was no previous address#12505
Users: Do not send an email change notification when there was no previous address#12505thisismyurl wants to merge 1 commit into
Conversation
…vious address. The email change notification is addressed to the old email address. When an account has no previous address (for example a user created during a content import), the notification is sent to an empty recipient. Skip the notification in that case, as there is nobody to notify. Adds a regression test asserting the notification path does not run when the prior address is empty. Props thisismyurl. See #47618.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Pull request overview
Adjusts wp_update_user() to avoid preparing/sending the email-change notification when the account had no prior email address (e.g., imported users with an empty user_email), preventing emails with an empty To: recipient.
Changes:
- Add a guard in
wp_update_user()so thesend_email_change_emailfilter (and downstream notification logic) only runs when the old email address is non-empty. - Add a PHPUnit regression test asserting that the
send_email_change_emailfilter is not reached when the old email is empty.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/wp-includes/user.php | Adds a non-empty-old-email guard before applying send_email_change_email in wp_update_user(). |
| tests/phpunit/tests/user.php | Adds a regression test covering the empty-old-email scenario for email-change notifications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Blank the address to reproduce an account imported without an email. | ||
| $wpdb->update( $wpdb->users, array( 'user_email' => '' ), array( 'ID' => $user_id ) ); | ||
| clean_user_cache( $user_id ); | ||
|
|
||
| $this->assertSame( '', get_userdata( $user_id )->user_email, 'The account should start with no email address.' ); | ||
|
|
||
| $change_email_triggered = false; | ||
| add_filter( | ||
| 'send_email_change_email', | ||
| static function ( $send ) use ( &$change_email_triggered ) { | ||
| $change_email_triggered = true; | ||
| return $send; | ||
| } | ||
| ); | ||
|
|
||
| $update = wp_update_user( | ||
| array( | ||
| 'ID' => $user_id, | ||
| 'user_email' => 'new@example.com', | ||
| ) | ||
| ); | ||
|
|
||
| $this->assertSame( $user_id, $update, 'The email address should have been updated.' ); | ||
| $this->assertFalse( $change_email_triggered, 'The email change notification should not be triggered when there is no previous address.' ); |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
The email change notification is addressed to the user's old email address, so that the previous owner of the address learns it was changed. When an account has no previous address (the case in the ticket is a user created during a content import, where the importer never set one), there is nobody at the old address to notify, and the notification is instead sent to an empty recipient. Email loggers show it as a message with an empty
To:field.This adds an empty-address guard so the notification is only prepared when there was a prior address to send it to. The change is one condition on the existing
ifthat gates thesend_email_change_emailfilter, so the filter and everything downstream simply don't run when the old address is empty.I kept the patch scoped to the email-change case from the ticket. The password-change notification a few lines above has the same shape (it is sent to the user's current address, which would also be empty for one of these imported accounts), so it may be worth the same guard, but I left it out to keep this change to what the ticket describes. Happy to fold it in if you'd prefer to cover both here.
The regression test creates a user, blanks their address to reproduce the imported-account state, then updates the email and asserts the
send_email_change_emailfilter is never reached. By construction it fails without the guard (the old condition fires the filter, which would send to the empty address) and passes with it.One disclosure on verification: this box has no Docker/
wp-env, so I could not run the full PHPUnit suite locally. I verified the guard logic with a standalone harness and wrote the committed test to the PHPUnit harness with@ticket 47618; I'd appreciate a CI run confirming it.Trac ticket: https://core.trac.wordpress.org/ticket/47618
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Helping locate the affected code path and draft the regression test. I reviewed, tested, and take responsibility for the final code.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.