feat(my-account): add email change cancellation option by chickenn00dle · Pull Request #3778 · Automattic/newspack-plugin

chickenn00dle · 2025-02-24T17:49:29Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1208993180326452/1209468700585359/f

This PR adds the option to cancel an email change request after one has been submitted:

How to test the changes in this Pull Request:

Ensure the NEWSPACK_EMAIL_CHANGE_ENABLED FF is added in wp-config
As admin, go to Newspack > Engagement > Show Advanced Settings > Transactional Email Content and reset the Change email template
Click edit next to this template and verify the last line in the main body of the email template contains the EMAIL_CANCELLATION_URL template tag
As a logged in reader, go to my account and enter a new email address, then submit
Confirm the page reloads and a Cancel email change button appears next to the Save changes button.
Click this cancel button and verify the page reloads with a notice indicating the email change request was cancelled and the email field has reverted to the old email address
Repeat step 4 and this time check the readers new email for the email change verification email
Verify the email contains a link to cancel the email change request
Click the link and verify the request is cancelled as in step 6

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

dkoo

This is mostly working as described, except for one flow that I detailed below (when clicking a confirm or cancel link while not logged into the reader's account).

dkoo · 2025-02-25T21:34:16Z

+		} else {
+			\wc_add_notice( $error, 'error' );
+		}
+		\wp_safe_redirect( \wc_get_endpoint_url( 'edit-account', '', \wc_get_page_permalink( 'myaccount' ) ) );


I stumbled on this by accident, but if you click on the link while not logged in, it brings you to the site's homepage and doesn't seem to do anything, but the nonce/link then gets invalidated, so after logging in I can no longer click the cancellation link to cancel the request (I just get the "something went wrong" error message).

What if we tweaked this flow to check logged-in state before verifying the nonce? If not logged in, we should redirect to the My Account page so they user gets presented with a login screen, then after logging in, we can proceed with the nonce verification and cancellation logic.

This is also the case for confirming an email change. I think the same flow during logged-out state would work for that, too.

Oooh. Good catch @dkoo! We actually do perform the is_user_logged_in check before verifying the nonce in both handle_verify_email_change and handle_cancel_email_change.

But this issue will still happen since the nonce is tied to the session. So if the url is generated while the reader is logged in, then the reader logs out and logs back in, and finally the reader clicks the link, it will fail.

To get around this case, I've switched from using nonces to wp_hash in cc4111e. This commit also makes it so logged out readers are directed to my account when clicking these links as well.

Let me know if this works!

Actually, I think I'm going to have to take this one out of review since doing away with the nonces opens us up to the issue you pointed out in slack here: https://a8c.slack.com/archives/C055SRT7WN4/p1740519700873229

I'll have to do some more thinking about this later this week. (I'm open to suggestions though!) I'll ping you once this is ready again. Sorry for the confusion @dkoo!

Thanks, @chickenn00dle! If you want to separate out the refactoring of the nonce/logged-out state into another PR, we can proceed with this one as-is. Whatever you prefer!

Alright, I've made a small change to always use a hash of the existing email address, this way only the logged in user with that email can complete the process. I think this should cover us, but let me know what you think @dkoo!

Nice! This is working well with the email hash, but I think we just need one more change for the handling in a logged-out state. Using the following steps:

From My Account, change your email address to get the verification email

Log out of the reader account

Click either the Confirm or Cancel links from the email: you'll get directed to log in at My Account, with the ?verify-email-change=token or ?cancel-email-change=token params still attached

Log into the account using this form and click "Continue" after logging in

At this point, the page reloads and you're logged into the account, but in the reload we lose the params on the URL, so the confirm/cancel request isn't handled. As the reader, I'll need to go back to the email and click the link again.

I think we should let these params pass through after the login so that the reader won't have to take that extra step to complete the intended action. Maybe we can detect the params on the referrer URL on a wp_login hook to handle them?

Great idea! Unfortunately the auth form hijacks the wp_login flow, so I had to make changes to the redirect flow there in ae1217f

Let me know if this looks good to you!

Nice, this accomplishes the task of making the flow more seamless for the reader, so I'm good with it!

dkoo

@chickenn00dle we're almost there! I just have one more suggestion below before merging this. Let me know if you want to discuss further.

dkoo · 2025-02-27T22:15:59Z

+		} else {
+			\wc_add_notice( $error, 'error' );
+		}
+		\wp_safe_redirect( \wc_get_endpoint_url( 'edit-account', '', \wc_get_page_permalink( 'myaccount' ) ) );


Nice! This is working well with the email hash, but I think we just need one more change for the handling in a logged-out state. Using the following steps:

From My Account, change your email address to get the verification email

Log out of the reader account

Click either the Confirm or Cancel links from the email: you'll get directed to log in at My Account, with the ?verify-email-change=token or ?cancel-email-change=token params still attached

Log into the account using this form and click "Continue" after logging in

At this point, the page reloads and you're logged into the account, but in the reload we lose the params on the URL, so the confirm/cancel request isn't handled. As the reader, I'll need to go back to the email and click the link again.

I think we should let these params pass through after the login so that the reader won't have to take that extra step to complete the intended action. Maybe we can detect the params on the referrer URL on a wp_login hook to handle them?

github-actions · 2025-03-03T15:43:55Z

Hey @chickenn00dle, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

# [6.1.0-alpha.1](v6.0.1...v6.1.0-alpha.1) (2025-03-06) ### Bug Fixes * **modal-checkout:** password setup ([8fbfabd](8fbfabd)) * **my-account:** handle email change esp sync errors ([#3792](#3792)) ([3d9f294](3d9f294)) * **my-account:** send change email to old and new emails ([#3786](#3786)) ([710d53d](710d53d)) * **premium-newsletters:** show premium lists in post-checkout signup ([#3788](#3788)) ([ccb1526](ccb1526)) * spacer block handling with registration block ([e9b7beb](e9b7beb)) ### Features * **correction-blocks:** Correction box & Loop item + Template ([#3787](#3787)) ([c215dc2](c215dc2)) * **correction-blocks:** update corrections template ([#3793](#3793)) ([c7aea33](c7aea33)) * **my-account:** add email change cancellation option ([#3778](#3778)) ([600ad61](600ad61)) * **my-account:** sync admin email change with ESP/stripe ([#3799](#3799)) ([7179ffd](7179ffd)) * **my-account:** sync email change with esp ([#3780](#3780)) ([983c087](983c087)) * **my-account:** sync email change with stripe ([#3789](#3789)) ([4f45795](4f45795))

matticbot · 2025-03-06T20:47:50Z

🎉 This PR is included in version 6.1.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [6.1.0](v6.0.5...v6.1.0) (2025-03-18) ### Bug Fixes * **modal-checkout:** endpoint to refresh newsletter lists via REST ([#3841](#3841)) ([2b294e0](2b294e0)) * **modal-checkout:** password setup ([8fbfabd](8fbfabd)) * **my-account:** handle email change esp sync errors ([#3792](#3792)) ([3d9f294](3d9f294)) * **my-account:** send change email to old and new emails ([#3786](#3786)) ([710d53d](710d53d)) * **premium-newsletters:** show premium lists in post-checkout signup ([#3788](#3788)) ([ccb1526](ccb1526)) * spacer block handling with registration block ([e9b7beb](e9b7beb)) ### Features * **correction-blocks:** Correction box & Loop item + Template ([#3787](#3787)) ([c215dc2](c215dc2)) * **correction-blocks:** update corrections template ([#3793](#3793)) ([c7aea33](c7aea33)) * enable email change for newspack users ([#3824](#3824)) ([9c152a8](9c152a8)) * **my-account:** add email change cancellation option ([#3778](#3778)) ([600ad61](600ad61)) * **my-account:** sync admin email change with ESP/stripe ([#3799](#3799)) ([7179ffd](7179ffd)) * **my-account:** sync email change with esp ([#3780](#3780)) ([983c087](983c087)) * **my-account:** sync email change with stripe ([#3789](#3789)) ([4f45795](4f45795)) * **woo-member-commenting:** optional module for member commenting ([#3783](#3783)) ([90746c8](90746c8)) ### Reverts * Revert "refactor(corrections): remove corrections feature flag (#3797)" (#3825) ([afd01f2](afd01f2)), closes [#3797](#3797) [#3825](#3825)

matticbot · 2025-03-18T18:23:25Z

🎉 This PR is included in version 6.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

chickenn00dle requested a review from a team as a code owner February 24, 2025 17:49

chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 24, 2025

dkoo requested changes Feb 25, 2025

View reviewed changes

chickenn00dle requested a review from dkoo February 25, 2025 23:40

chickenn00dle force-pushed the feat/cancel-change-email-requests branch from cc4111e to 94a9bff Compare February 25, 2025 23:42

chickenn00dle removed the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 25, 2025

dkoo requested changes Feb 27, 2025

View reviewed changes

chickenn00dle force-pushed the feat/cancel-change-email-requests branch from dbc893d to ae1217f Compare February 28, 2025 19:22

chickenn00dle requested a review from dkoo February 28, 2025 19:23

chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 28, 2025

dkoo approved these changes Feb 28, 2025

View reviewed changes

github-actions Bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 28, 2025

chickenn00dle added 5 commits March 3, 2025 10:17

feat(my-account): add email change cancellation option

87dec8b

fix: use wp_hash in place of nonce

bca29b0

fix: use hash of existing email

13ace31

fix: re-add params on login

d4a5b97

fix: merge regression

96fd016

chickenn00dle force-pushed the feat/cancel-change-email-requests branch from ae1217f to 96fd016 Compare March 3, 2025 15:40

chickenn00dle merged commit 600ad61 into trunk Mar 3, 2025

chickenn00dle deleted the feat/cancel-change-email-requests branch March 3, 2025 15:43

github-actions Bot added the needs-changelog label Mar 3, 2025

chickenn00dle removed the needs-changelog label Mar 3, 2025

chickenn00dle mentioned this pull request Mar 3, 2025

feat(my-account): sync email change with esp #3780

Merged

6 tasks

matticbot added the released on @alpha label Mar 6, 2025

matticbot added the released label Mar 18, 2025

Conversation

chickenn00dle commented Feb 24, 2025

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

dkoo left a comment

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chickenn00dle Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 28, 2025

Choose a reason for hiding this comment

Uh oh!

dkoo left a comment

Choose a reason for hiding this comment

Uh oh!

dkoo Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 3, 2025

Uh oh!

matticbot commented Mar 6, 2025

Uh oh!

matticbot commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chickenn00dle Feb 25, 2025 •

edited

Loading

chickenn00dle Feb 27, 2025 •

edited

Loading

dkoo Feb 27, 2025 •

edited

Loading

dkoo Feb 27, 2025 •

edited

Loading