feat(my-account): sync admin email change with external connections#3799
Conversation
6442228 to
a3493cc
Compare
miguelpeixe
left a comment
There was a problem hiding this comment.
Works as described! Just a question regarding the guard clause.
| if ( ! is_admin() || ! self::is_email_change_enabled() ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Should this check be preventing an email update? Bailing here just means the email change happened without the required syncs.
There was a problem hiding this comment.
I believe so. We bail here if the request is not from admin since non-admin requests are handled via a different hook, or if the feature is not enabled to maintain the existing behavior.
There was a problem hiding this comment.
Oops, just realized I didn't directly answer your first question:
Should this check be preventing an email update?
No. At least we aren't doing this currently, so my assumption is this is expected.
There was a problem hiding this comment.
Thanks for explaining! I believe we could go without this clause. is_email_change_enabled() is not whether the sync should happen, it's whether we allow a reader to change their email. Non-blocking, though!
There was a problem hiding this comment.
I believe we could go without this clause. is_email_change_enabled() is not whether the sync should happen, it's whether we allow a reader to change their email. Non-blocking, though!
I think you're correct, although the goal here is to make sure the behavior remains consistent when this feature is not enabled, so we'd need this to make sure admins can still change email addresses without triggering the sync.
I'll keep it for now, but can remove if it causes issues down the line!
|
Hey @chickenn00dle, good job getting this PR merged! 🎉 Now, the 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))
|
🎉 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)
|
🎉 This PR is included in version 6.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Closes https://app.asana.com/0/1208993180326452/1209589552372839
This PR triggers a sync with both Stripe and the site ESP when an email address is updated by an admin
How to test the changes in this Pull Request:
NEWSPACK_EMAIL_CHANGE_ENABLEDconstant is defined and true in wp-configNote: error cases for ESP sync are being tackled in another PR, so be sure to only update to an email address that doesn't exist in the ESP.
Other information: