feat: disable woocomerce welcome emails in favor of verification email #1876

dkoo · 2022-08-09T16:48:15Z

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:

Disables WooCommerce's standard, WC-branded welcome emails that are normally sent to new customers, so that we can send our own verification email instead.

Will not send a verification email to readers who register via SSO, which is considered a secure login method and doesn't require verification.

Closes #1872.

How to test the changes in this Pull Request:

Check out this branch.
Register a new email address via the Register block (via direct email input, not SSO) and confirm that you receive a magic link email with a special subject line and message instead of WooCommerce's "Your Newspack account has been created!" email.
Visit the magic link URL in the same session where you registered the email address and confirm that you're redirected to the main My Account dashboard page with a message that your email has been verified. Also confirm that you're able to manage newsletter subscriptions at this point.
In a new session, register a new account via SSO and confirm that you do not receive a magic link email, and that you're immediately able to manage newsletter subscriptions in My Account > Newsletters.

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?

This reverts commit f02f65b.

…for new accounts

dkoo · 2022-08-09T18:09:16Z

@miguelpeixe 23b9809 eschews the use of Newsletters' verification system in favor of magic links with customizable subjects and messages.

miguelpeixe

Thank you for revising the strategy for this! It's working great!

I have another take on the concept here. I think we can preserve the behavior of the magic link by exposing the Magic_Link::generate_url() and not introducing registration logic to it.

With Magic_Link::generate_url(), Reader_Activation can send the email and handle the redirect URL and success notice on its own.

includes/class-magic-link.php

miguelpeixe · 2022-08-09T18:20:04Z

includes/class-magic-link.php

 	 *   Used to build wp_mail().
 	 *
 	 *   @type string $to      The intended recipient - New user email address.
-	 *   @type string $subject The subject of the email.


This may have been by accident?

Yes, I put the @param line in the wrong spot at first.

This line should be restored

Whoops, I misunderstood what you were saying—restored in ae476dd.

Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com>

dkoo · 2022-08-09T19:44:57Z

I have another take on the concept here. I think we can preserve the behavior of the magic link by exposing the Magic_Link::generate_url() and not introducing registration logic to it.

That's a good point, but I'd rather not replicate the logic of sending the actual magic link email in Reader_Activation. How about this? 49d3067 moves all logic related to email verification to Reader_Activation, but still uses methods from Magic_Link to build and send the email, allowing for arbitrarily overriding the subject line and message for the magic link email. That keeps Magic_Link logic abstract and clean from any reader registration/verification logic.

miguelpeixe

not replicate the logic of sending the actual magic link email in Reader_Activation

My proposal is based on the idea that it is not a magic link email, it's a verification email that borrows the magic link as the verifier, hence using just the generate_url() from Magic_Link.

It makes for a more solid understanding that account verification is a core feature and if we, in the future, decide that the URL should not be a magic link it will be more straightforward to detach and maintain.

This is a non-blocker, though! It's working as described 🎉

Left just a couple of comments below.

miguelpeixe · 2022-08-09T20:04:29Z

includes/class-magic-link.php

-		Reader_Activation::set_reader_verified( $user );
-		Reader_Activation::set_current_reader( $user->ID );
-


It's a non-blocker, but I find it confusing that authenticate() doesn't authenticate.

My understanding of hooks is that it should be for side effects and not the primary function of the method.

Hmm, that's a good point. 9d7f3ab moves the Reader_Activation::set_current_reader call back to the Magic_Link::authenticate method, but leaves the set_reader_verified call as a callback on the newspack_magic_link_authenticated hook. In my mind the verification is a side effect of authenticating via magic link, as you suggest.

miguelpeixe · 2022-08-09T20:06:22Z

includes/class-reader-activation.php

+		$user_id = \absint( $user->ID );
+		if ( empty( $user_id ) ) {
+			return new \WP_Error( 'newspack_authenticate_invalid_user_id', __( 'Invalid user id.', 'newspack' ) );
+		}
+


This check is superfluous, the user has already been validated previously.

True, removed in 9d7f3ab.

…ethod

dkoo · 2022-08-09T20:56:48Z

My proposal is based on the idea that it is not a magic link email, it's a verification email that borrows the magic link as the verifier, hence using just the generate_url() from Magic_Link.

It makes for a more solid understanding that account verification is a core feature and if we, in the future, decide that the URL should not be a magic link it will be more straightforward to detach and maintain.

Ah, I think I get what you're saying. I think we should leave it as-is for now and refactor if needed with an entirely new class and set of features if we decide to decouple verification from magic links. Right now it's not so tightly integrated that we can't easily swap the usage of the single Magic_Link::send_email method for another.

miguelpeixe

🙌

#1876) * feat: disable woocomerce welcome emails in favor of our own * refactor: move email verification methods to main plugin * Revert "refactor: move email verification methods to main plugin" This reverts commit f02f65b. * feat: use magic link email instead of Newsletters verification email for new accounts * chore: lint Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com> * refactor: move all reader verification logic to Reader_Activation class * refactor: authenticate reader as a primary function of authenticate method * chore: restore missing doc line Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com>

# [1.89.0-alpha.2](v1.89.0-alpha.1...v1.89.0-alpha.2) (2022-08-12) ### Bug Fixes * **google-auth:** catch and display errors ([#1871](#1871)) ([67cbcfd](67cbcfd)) * **oauth:** csrf token lifespan ([#1869](#1869)) ([52e0f8b](52e0f8b)) * parse CID from _ga cookie if it only contains CID string ([#1874](#1874)) ([dc1fb52](dc1fb52)) * redirecting to My Account after logging in while pre-authed ([#1863](#1863)) ([ddf111e](ddf111e)) * verify reader on google authentication ([#1873](#1873)) ([c9c4eef](c9c4eef)) ### Features * authenticated reader cookie ([#1882](#1882)) ([352316b](352316b)) * better welcome email copy for initial verification ([#1880](#1880)) ([604ebf7](604ebf7)) * cookie reader's preferred auth strategy ([#1875](#1875)) ([fc47f41](fc47f41)) * disable woocomerce welcome emails in favor of verification email ([#1876](#1876)) ([1e470e3](1e470e3)) * lock access to My Account UI until account is verified ([#1877](#1877)) ([a850f48](a850f48))

matticbot · 2022-08-12T17:33:44Z

🎉 This PR is included in version 1.89.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [1.89.0](v1.88.0...v1.89.0) (2022-08-16) ### Bug Fixes * **active-campaign:** legacy contacts detection ([#1858](#1858)) ([67640a5](67640a5)) * **campaigns-wizard:** segmentation wording ([ddf61ad](ddf61ad)) * ensure scroll on smaller height ([#1813](#1813)) ([e234e8b](e234e8b)) * fix fatal error when debug mode active ([#1826](#1826)) ([d9388ee](d9388ee)) * **ga:** cookie parsing ([#1857](#1857)) ([a936abd](a936abd)) * google auth button type ([#1829](#1829)) ([3704d9f](3704d9f)) * **google-auth:** catch and display errors ([#1871](#1871)) ([67cbcfd](67cbcfd)) * **google-auth:** ensure popup on user click event ([#1831](#1831)) ([0af9abf](0af9abf)) * **magic-links:** fix email encoding on sent link ([#1833](#1833)) ([8d4756c](8d4756c)) * **my account:** handle legacy data ([#1823](#1823)) ([6816799](6816799)) * **newsletters:** use international date format ([#1855](#1855)) ([4cda57d](4cda57d)) * **oauth:** csrf token lifespan ([#1869](#1869)) ([52e0f8b](52e0f8b)) * parse CID from _ga cookie if it only contains CID string ([#1874](#1874)) ([dc1fb52](dc1fb52)) * **popups:** use new Campaigns method for creating donation events on new orders ([#1794](#1794)) ([49dc14c](49dc14c)) * **reader-activation:** add metadata to reader registered on donation ([722724c](722724c)) * **reader-activation:** handle modal conflict when auth is triggered from a prompt ([c2a0141](c2a0141)), closes [#1835](#1835) * **reader-activation:** handle no lists config available ([23b0249](23b0249)) * **reader-activation:** reinitialize auth links after DOM load ([#1812](#1812)) ([0a4b499](0a4b499)) * **reader-activation:** remove async prop from library ([#1846](#1846)) ([4131ca6](4131ca6)) * **reader-activation:** username generation handling ([#1789](#1789)) ([17edf2a](17edf2a)) * redirecting to My Account after logging in while pre-authed ([#1863](#1863)) ([ddf111e](ddf111e)) * **registration-block:** don't escape html for sign in labels ([#1834](#1834)) ([871300d](871300d)) * **registration-block:** margin for success message ([#1808](#1808)) ([1bfe546](1bfe546)) * **registration-block:** render on preview ([#1844](#1844)) ([87b9be9](87b9be9)) * tweak arguments for magic link client hash ([#1862](#1862)) ([8dcd45e](8dcd45e)) * verify reader on google authentication ([#1873](#1873)) ([c9c4eef](c9c4eef)) ### Features * **active-campaign:** metadata improvements ([#1851](#1851)) ([48883af](48883af)) * **active-campaigns:** override is-new-contact for legacy contacts ([34dd9a2](34dd9a2)) * **analytics:** send GA events on the server side ([#1828](#1828)) ([3e384e1](3e384e1)) * authenticated reader cookie ([#1882](#1882)) ([352316b](352316b)) * better welcome email copy for initial verification ([#1880](#1880)) ([604ebf7](604ebf7)) * cookie reader's preferred auth strategy ([#1875](#1875)) ([fc47f41](fc47f41)) * disable woocomerce welcome emails in favor of verification email ([#1876](#1876)) ([1e470e3](1e470e3)) * **donations:** remove defaultFrequency from the configuration ([#1814](#1814)) ([b6aa894](b6aa894)) * handle contact update w/out lists selection ([#1816](#1816)) ([67574d1](67574d1)) * handle new frequency options in Campaigns dashbaord ([#1779](#1779)) ([c770a7d](c770a7d)) * if registering an email that already has an account, show different message ([#1849](#1849)) ([bf48bc4](bf48bc4)) * lock access to My Account UI until account is verified ([#1877](#1877)) ([a850f48](a850f48)) * **my-account:** stripe billing portal link ([#1761](#1761)) ([3e69af1](3e69af1)), closes [#1742](#1742) [#1739](#1739) [#1740](#1740) [#1741](#1741) [#1782](#1782) * **reader-activation:** account link and auth form ([#1754](#1754)) ([b163664](b163664)) * **reader-activation:** activecampaign master list ([#1818](#1818)) ([ecbbc47](ecbbc47)) * **reader-activation:** disable 3rd party login buttons initially ([#1806](#1806)) ([c806bfe](c806bfe)) * **reader-activation:** optimistic account link ([#1847](#1847)) ([85c550a](85c550a)) * **reader-activation:** prevent updating user email in my-account ([7d49db4](7d49db4)) * **reader-activation:** registration auth cookie control ([#1787](#1787)) ([aeb0b5b](aeb0b5b)) * **reader-activation:** settings wizard ([#1773](#1773)) ([aaff0de](aaff0de)) * **reader-auth:** make password login the first option, instead of login link ([1fe5ffa](1fe5ffa)), closes [#1809](#1809) * register anonymous single donors ([#1795](#1795)) ([9e4f2f6](9e4f2f6)) * **registration-block:** add success icon ([#1804](#1804)) ([86c38f8](86c38f8)) * **registration-block:** editable success state ([#1785](#1785)) ([7dcea82](7dcea82)), closes [#1768](#1768) * **registration-block:** login with Google ([#1781](#1781)) ([ed79c5c](ed79c5c)), closes [#1774](#1774) * **registration-block:** newsletter subscription ([#1778](#1778)) ([717b5b8](717b5b8)) * reorganise donations wizard and use buttongroup for donation type ([#1824](#1824)) ([f7b58ae](f7b58ae)) * replace WooCommerce’s login form with our own ([#1854](#1854)) ([f5b24c4](f5b24c4)) * **rss:** adds offset feature ([#1790](#1790)) ([321eff5](321eff5)) * send user metadata to AC ([#1793](#1793)) ([03a15ba](03a15ba)) * set client id cookie; reader activation tweaks ([#1780](#1780)) ([96a07ae](96a07ae)) * **stripe:** webhook auto-creation and validation ([365aed9](365aed9)) * tweak registration block styling ([d83448e](d83448e)) ### Reverts * Revert "chore(release): 1.87.0 [skip ci]" ([ca8d55c](ca8d55c))

matticbot · 2022-08-16T18:07:52Z

🎉 This PR is included in version 1.89.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

feat: disable woocomerce welcome emails in favor of our own

c523eab

dkoo added [Type] Enhancement Improved behavior or functionality [Status] Needs Review The issue or pull request needs to be reviewed Reader Activation labels Aug 9, 2022

dkoo self-assigned this Aug 9, 2022

dkoo requested a review from a team as a code owner August 9, 2022 16:48

dkoo mentioned this pull request Aug 9, 2022

feat: send verification email upon new account registration Automattic/newspack-newsletters#924

Closed

6 tasks

dkoo added 3 commits August 9, 2022 11:22

refactor: move email verification methods to main plugin

f02f65b

Revert "refactor: move email verification methods to main plugin"

a6c8a31

This reverts commit f02f65b.

feat: use magic link email instead of Newsletters verification email …

23b9809

…for new accounts

dkoo changed the title ~~feat: disable woocomerce welcome emails in favor of our own~~ feat: disable woocomerce welcome emails in favor of verification email Aug 9, 2022

miguelpeixe requested changes Aug 9, 2022

View reviewed changes

dkoo and others added 2 commits August 9, 2022 13:27

chore: lint

e3d5996

Co-authored-by: Miguel Peixe <miguel.peixe@automattic.com>

refactor: move all reader verification logic to Reader_Activation class

49d3067

dkoo requested a review from miguelpeixe August 9, 2022 19:46

miguelpeixe requested changes Aug 9, 2022

View reviewed changes

refactor: authenticate reader as a primary function of authenticate m…

9d7f3ab

…ethod

dkoo requested a review from miguelpeixe August 9, 2022 20:57

chore: restore missing doc line

ae476dd

miguelpeixe approved these changes Aug 9, 2022

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 Aug 9, 2022

dkoo merged commit 1e470e3 into master Aug 9, 2022

dkoo deleted the feat/disable-woocommerce-welcome-email branch August 9, 2022 21:12

matticbot added the released on @alpha label Aug 12, 2022

matticbot added the released label Aug 16, 2022

		Reader_Activation::set_reader_verified( $user );
		Reader_Activation::set_current_reader( $user->ID );

feat: disable woocomerce welcome emails in favor of verification email #1876

feat: disable woocomerce welcome emails in favor of verification email #1876

Uh oh!

Conversation

dkoo commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

dkoo commented Aug 9, 2022

Uh oh!

miguelpeixe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miguelpeixe Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

dkoo Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

miguelpeixe Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

dkoo Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

dkoo commented Aug 9, 2022

Uh oh!

miguelpeixe left a comment

Choose a reason for hiding this comment

Uh oh!

miguelpeixe Aug 9, 2022 • 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 Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

miguelpeixe Aug 9, 2022 • 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 Aug 9, 2022

Choose a reason for hiding this comment

Uh oh!

dkoo commented Aug 9, 2022

Uh oh!

miguelpeixe left a comment

Choose a reason for hiding this comment

Uh oh!

matticbot commented Aug 12, 2022

Uh oh!

matticbot commented Aug 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dkoo commented Aug 9, 2022 •

edited

Loading

miguelpeixe Aug 9, 2022 •

edited

Loading

miguelpeixe Aug 9, 2022 •

edited

Loading