Skip to:
Content

BuddyPress.org

Opened 8 months ago

Closed 2 months ago

#8138 closed defect (bug) (fixed)

Confirmation for email change request is sent to the current email

Reported by: imath Owned by: imath
Milestone: 6.0.0 Priority: normal
Severity: normal Version: 2.5.0
Component: Settings Keywords: has-patch commit
Cc:

Description (last modified by imath)

Since [10479], we are using the BP Email API for our notifications / confirmations. To send an email we use the bp_send_email( $email_type, $email_or_user_id, $tokens ) function. When using a user ID as the second parameter, the API will look for the user's current email.

When changing the email from the front end, we shouldn't use a user ID like it's the case since [10479], instead we should use the new email.

This issue comes form this support request: https://buddypress.org/support/topic/activation-email-sent-on-old-adresse-not-the-new-one/

Attachments (2)

8138.patch (1.9 KB) - added by imath 8 months ago.
8138.2.patch (2.0 KB) - added by imath 2 months ago.

Download all attachments as: .zip

Change History (14)

#1 @imath
8 months ago

  • Description modified (diff)

#2 @imath
8 months ago

  • Description modified (diff)

@imath
8 months ago

#3 follow-up: @imath
8 months ago

  • Keywords has-patch added; needs-patch removed

8138.patch is fixing the issue:

  • making sure we to customize the email salutation with the user displayname.
  • behave like WordPress does.

#4 in reply to: ↑ 3 @laudag
8 months ago

I just tested your patch : It’s OK... Except one little thing that could be mistaken by the user : the message in buddypress profile page after changing the email adress is something like that (in french in my web site) : The modification of your email adress for “new mail” is pending. Watch your email “old email” to get activation link, or stop the pending modification.

Even if at last the user will receive the activation in his news mailbox, That could be misunderstood as only sent on the old email and… get support from me whereas it’s not necessary.

What do you think about that ?

Thanks for the patch anyway !

Replying to imath:

8138.patch is fixing the issue:

  • making sure we to customize the email salutation with the user displayname.
  • behave like WordPress does.

#5 follow-up: @imath
8 months ago

Are you sure @laudag because the part of the patch that edits src/bp-settings/bp-settings-template.php should fix this as it’s using the new email

#6 in reply to: ↑ 5 @laudag
8 months ago

Yes, I confirm. I did it again, and the same, even after deleting the browser cache.
The fact is that an other problemoito came out : To try again to change the mail and get the message in the page, I had to cancel the activation in the profile page.
After clic on "cancel pending modification", I get a "non found page", with the URL
https://www.lanautique.com/vie-des-membres/espace-membres/membres/swbclaudag/settings/<Code>new mail</code>/
(swbclaudag, its me)

If I delete the patch line "'<code>' . esc_html( $pending_emailnewemail? ) . '</code>'," in bp-setting-template, I can cancel the pending modification.

Seems that it's not totally steady, no ?

Replying to imath:

Are you sure @laudag because the part of the patch that edits src/bp-settings/bp-settings-template.php should fix this as it’s using the new email

#7 follow-up: @imath
8 months ago

For me, on a fresh install of WordPress & with no other plugins than BuddyPress activated, it behaves as expected, here's a screenshot of the screen notice:

https://cldup.com/FbODvwN8A5.png

#8 in reply to: ↑ 7 @laudag
8 months ago

Well... there must be something in my website which make a conflict. May be BuddyPress Xprofile Custom Field Types. Well, I miss time to check eveythings. At least people can validate their new email as they get the activation message. But this is weird.
Thanks

Replying to imath:

For me, on a fresh install of WordPress & with no other plugins than BuddyPress activated, it behaves as expected, here's a screenshot of the screen notice:

https://cldup.com/FbODvwN8A5.png

#9 @espellcaste
3 months ago

@imath This one seems to be good for 6.0. What do you think? The issue @laudag is mentioning seems to be related to their setup, and not related to your patch.

#10 @imath
3 months ago

  • Keywords needs-testing added

Let's test the patch one more time before committing it.

@imath
2 months ago

#11 @imath
2 months ago

  • Keywords commit added; needs-testing removed

8138.2.patch is a refreshed version of the initial patch. I've just tested it another time and it works as expected. I'm going to commit it asap.

#12 @imath
2 months ago

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

In 12603:

Send the "email change" confirmation message to the new email

To be consistent with how the email change confirmation message is handled by WordPress, we need to send it to the new email and not to the current email.

Props laudag, espellcaste

Fixes #8138

Note: See TracTickets for help on using tickets.