Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exit early if user's email address didn't change #8370 #8371

Merged
merged 2 commits into from
Feb 9, 2021

Conversation

ashleyfae
Copy link
Contributor

@ashleyfae ashleyfae commented Feb 9, 2021

Fixes #8370

Proposed Changes:

The only main change is this extra snippet at the very top:

$user = get_userdata( $user_id );

// Bail if the email address didn't actually change just now.
if ( empty( $user ) || $user->user_email === $old_user_data->user_email ) {
	return;
}

We exit ASAP if the user's email address didn't just change. The following code should only execute if the user's email changed to something else during the current update.

Other changes are minimal while in the area:

  1. Update PhpDocs to include parameters.
  2. Reformat code to exit early instead of a bunch of nested if statements.
  3. Change return false; to return;
  4. Add comments to action hook.
  5. Remove required parameter before optional (PHP 8 error)

This fixed the memory issue for me. We should verify that this hook does still successfully run when actually changing a user's email address.

Copy link
Contributor

@robincornett robincornett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was having trouble testing this and realized that the issue was actually in FES, looks like FES_DB_Vendors->update_vendor_email_on_user_update() will need to be looked at as well.

Once I got that sorted, I was able to reproduce the issue using the code from the original issue post, and this does resolve it.

@ashleyfae ashleyfae merged commit 3e263ef into release/2.10 Feb 9, 2021
@ashleyfae ashleyfae deleted the issue/8370 branch February 9, 2021 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants