Email: Surface different warning message when the email subscription and domain are owned by different users #68601

wongasy · 2022-10-04T18:33:03Z

Proposed Changes

In the case where the domain and email subscription are owned by different users, our new messaging that suggests the user to contact the owner of the email / domain will not work. Since the user must be both the domain and email subscription owner in order to make a purchase. Therefore, we will need separate messaging to address this scenario, and this PR introduces a new component for this case.

Testing Instructions

Have two different user accounts - User A and User B
Have a Professional Email subscription as User A
Follow the steps here pbLl1t-LC-p2 to transfer the Professional Email subscription to User B
As User A, visit Upgrades > Emails, you should see the new messaging introduced in this PR
As User B, visit Upgrades > Emails, you should see the new messaging introduced in this PR

Pre-merge Checklist

Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to #

github-actions · 2022-10-04T18:33:15Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-45881

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-45881&env=jetpack

matticbot · 2022-10-04T18:33:30Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~145 bytes added 📈 [gzipped])

name   parsed_size           gzip_size
email       +903 B  (+0.1%)     +145 B  (+0.1%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

a8ci18n · 2022-10-04T19:17:50Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7564892

Thank you @wongasy for including a screenshot in the description! This is really helpful for our translators.

stephanethomas

I've left a number of comments but the most important one is probably documenting the differences between canCurrentUserAddEmail and canDomainOwnerAddEmail in a clear way.

client/lib/domains/can-domain-owner-add-email.ts

client/my-sites/email/email-different-domain-owner-message/index.tsx

client/my-sites/email/email-management/home/email-plan-warning-notice.tsx

client/my-sites/email/email-different-domain-owner-message/index.tsx

client/lib/domains/can-domain-owner-add-email.ts

client/lib/domains/index.js

stephanethomas · 2022-10-05T09:47:39Z

client/my-sites/email/email-different-domain-owner-message/index.tsx

+				eventName="calypso_email_providers_nonowner_impression"
+				eventProperties={ { source } }
+			/>
+			<p className="email-non-owner-message__non-owner-message">


I'm not a big fan of reusing the class name of another component. Could we find a way to share styles between those two components in a cleaner way?

It seems that it's typo, since we have styles for className email-non-domain-owner-message__non-owner-message

https://github.com/Automattic/wp-calypso/blob/improve/email-management-for-different-owners/client/my-sites/email/email-non-domain-owner-message/style.scss#L7

nightnei · 2022-10-05T11:33:22Z

@wongasy WDYT about wrapping these two components (email-non-domain-owner-message & email-non-owner-message) with directory e.g. messages inside wp-calypso/client/my-sites/email/messages/...? To group them and have a more structured hierarchy, instead of a flat one.

wongasy · 2022-10-05T12:50:45Z

client/my-sites/email/email-different-domain-owner-message/index.tsx

+	const onClickLink = ( eventType: 'login' | 'support' ) => {
+		const properties = {
+			action: eventType,
+			source,


@AllTerrainDeveloper Let's also send the property context: 'different-owners', we can set the context for the other non-owner and non-domain-owner components that you built as well.

AllTerrainDeveloper · 2022-10-05T13:24:42Z

@wongasy WDYT about wrapping these two components (email-non-domain-owner-message & email-non-owner-message) with directory e.g. messages inside wp-calypso/client/my-sites/email/messages/...? To group them and have a more structured hierarchy, instead of a flat one.

It's a good idea to group them, but I prefer no to do it this in this Pull Request, lets first merge/deploy what we have achieved so far and then better place them in the structure of Calypso

wongasy · 2022-10-05T13:51:42Z

It's a good idea to group them, but I prefer no to do it this in this Pull Request, lets first merge/deploy what we have achieved so far and then better place them in the structure of Calypso

@AllTerrainDeveloper If we already know that they folder structure will be changed, let's start with the restructure in the main PR, instead of circling back to it, so we don't have to do a separate deploy just to move the files around and having to go through regression testing.

AllTerrainDeveloper · 2022-10-06T14:18:13Z

It's a good idea to group them, but I prefer no to do it this in this Pull Request, lets first merge/deploy what we have achieved so far and then better place them in the structure of Calypso

@AllTerrainDeveloper If we already know that they folder structure will be changed, let's start with the restructure in the main PR, instead of circling back to it, so we don't have to do a separate deploy just to move the files around and having to go through regression testing.

I saw this message too late :( anyway it would be more painful as I would have to apply the renaming painfully 3 times with each rebase...

I can do it here or in another PR, I'm fine with both options.

wongasy · 2022-10-06T15:48:22Z

it would be more painful as I would have to apply the renaming painfully 3 times with each rebase...
@AllTerrainDeveloper Cool, let's do it in another PR, but we need to be sure to regression test all scenarios.

olaseni

This is looking good. I have some comments.

client/lib/domains/is-domain-and-email-subscriptions-owned-by-different-users.ts

fredrikekelund

Looks good and works as expected! 👍 I left one very small (but very important) comment that we should address. Other than that, this is good to go!

fredrikekelund · 2022-10-10T15:45:04Z

client/my-sites/email/email-non-domain-owner-message/style.scss

-}
-
-.email-non-domain-owner-message__non-owner-message {
+.email-non-domain-owner-message__non-owner-message-message {


Suggested change

.email-non-domain-owner-message__non-owner-message-message {

.email-non-domain-owner-message__non-owner-message {

DRY 😉

… to the selected domain

…er and use it in the new link buttons

- Move logic to extract the domain user name - Explicit function description to ease the location of the event logged in Tracks - Change the style class name & padding for the notice. - Extract the function that builds the URL login page with email comparison and user login name

…ct component to do not overflow the main render.

…omain owners.

Add source in tracking

Add new styles to the main sheet Keep showing old notices.

… to the selected domain

…er and use it in the new link buttons

- Move logic to extract the domain user name - Explicit function description to ease the location of the event logged in Tracks - Change the style class name & padding for the notice. - Extract the function that builds the URL login page with email comparison and user login name

…ct component to do not overflow the main render.

…on queryString to a higher scope.

…and domain are owned by different users

Co-authored-by: Stéphane Thomas <email@stephanethomas.com>

AllTerrainDeveloper · 2022-10-11T11:19:00Z

Rebased again due to conflicts...

a8ci18n · 2022-10-12T13:30:28Z

Translation for this Pull Request has now been finished.

wongasy requested review from a team October 4, 2022 18:33

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 4, 2022

wongasy added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Oct 4, 2022

stephanethomas changed the title ~~Email: Surface different warning message when the email subscription …~~ Email: Surface different warning message when the email subscription and domain are owned by different users Oct 5, 2022

stephanethomas requested changes Oct 5, 2022

View reviewed changes

wongasy commented Oct 5, 2022

View reviewed changes

AllTerrainDeveloper force-pushed the improve/email-management-for-non-owners branch 2 times, most recently from 54339c3 to 20735b5 Compare October 6, 2022 13:17

AllTerrainDeveloper force-pushed the improve/email-management-for-different-owners branch from c120f97 to 4c0555e Compare October 6, 2022 14:09

olaseni requested changes Oct 6, 2022

View reviewed changes

client/lib/domains/is-domain-and-email-subscriptions-owned-by-different-users.ts Outdated Show resolved Hide resolved

client/lib/domains/is-domain-and-email-subscriptions-owned-by-different-users.ts Show resolved Hide resolved

fredrikekelund mentioned this pull request Oct 10, 2022

For email subscription non-owners, return early from resolveEmailPlanStatus #68839

Merged

5 tasks

fredrikekelund reviewed Oct 10, 2022

View reviewed changes

Base automatically changed from improve/email-management-for-non-owners to trunk October 11, 2022 11:01

AllTerrainDeveloper added 8 commits October 11, 2022 13:03

Emails: Add informative message and actions when user can't add email…

4e1bd2f

… to the selected domain

Emails: Disable select buttons, gather information for the domain own…

26d0804

…er and use it in the new link buttons

[Emails] Make the URL relative to the host.

a027840

Emails: Improve the final wording, build login query correctly, extra…

5da927b

…ct component to do not overflow the main render.

Fix linting errors

2a8aeff

Emails: Refactor the owner notice into a new component

b03e892

Emails: Remove test flag

a220ed4

wongasy and others added 23 commits October 11, 2022 13:08

Show email forwarding link for non-domain-owner admins

bcb6b27

Emails: initial commit

ebc523d

Differentiate between the two codes sent by the backend

010237d

Emails: Fix infinite loop with useMemo

a629cb5

Emails: Add another component in order to simplify the code for non d…

3e7b60e

…omain owners.

Emails: Change the message for email non-owners with a new component

25d7f2c

Save the constant on the constants file

06a1f38

Add source in tracking

Extract logic to a new component

fab0e5a

Add new styles to the main sheet Keep showing old notices.

Emails: Add informative message and actions when user can't add email…

15efe73

… to the selected domain

Emails: Disable select buttons, gather information for the domain own…

dd84145

…er and use it in the new link buttons

Emails: Improve the final wording, build login query correctly, extra…

f29210f

…ct component to do not overflow the main render.

Emails: Refactor the owner notice into a new component

371c721

Emails: Code improvements, rel = external on login link, moved functi…

1c63ea3

…on queryString to a higher scope.

Email: Surface different warning message when the email subscription …

d977819

…and domain are owned by different users

Updated copy

a10aa19

Clarify the purpose of canDomainOwnerAddEmail in the comment

05f5de8

Co-authored-by: Stéphane Thomas <email@stephanethomas.com>

Declare return type for canDomainOwnerAddEmail

a22bc53

Co-authored-by: Stéphane Thomas <email@stephanethomas.com>

Emails: Address feedback and fix build errors

9d2007d

Linting

bfa8355

This was added un-intentionally during rebase.

b1e91d1

Rename const and exit earlier

3422663

Wrong naming due to endless rebase. Fix className

d516aa9

AllTerrainDeveloper force-pushed the improve/email-management-for-different-owners branch from 80bea5d to d516aa9 Compare October 11, 2022 11:18

AllTerrainDeveloper merged commit d5eeff2 into trunk Oct 12, 2022

AllTerrainDeveloper deleted the improve/email-management-for-different-owners branch October 12, 2022 13:01

github-actions bot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Email: Surface different warning message when the email subscription and domain are owned by different users #68601

Email: Surface different warning message when the email subscription and domain are owned by different users #68601

wongasy commented Oct 4, 2022 •

edited by stephanethomas

github-actions bot commented Oct 4, 2022 •

edited by matticbot

matticbot commented Oct 4, 2022 •

edited

a8ci18n commented Oct 4, 2022

stephanethomas left a comment

stephanethomas Oct 5, 2022

nightnei Oct 5, 2022 •

edited

nightnei commented Oct 5, 2022

wongasy Oct 5, 2022

AllTerrainDeveloper commented Oct 5, 2022

wongasy commented Oct 5, 2022

AllTerrainDeveloper commented Oct 6, 2022

wongasy commented Oct 6, 2022

olaseni left a comment

fredrikekelund left a comment

fredrikekelund Oct 10, 2022

AllTerrainDeveloper commented Oct 11, 2022

a8ci18n commented Oct 12, 2022

	.email-non-domain-owner-message__non-owner-message-message {
	.email-non-domain-owner-message__non-owner-message {

Email: Surface different warning message when the email subscription and domain are owned by different users #68601

Email: Surface different warning message when the email subscription and domain are owned by different users #68601

Conversation

wongasy commented Oct 4, 2022 • edited by stephanethomas

Proposed Changes

Testing Instructions

Pre-merge Checklist

github-actions bot commented Oct 4, 2022 • edited by matticbot

matticbot commented Oct 4, 2022 • edited

a8ci18n commented Oct 4, 2022

stephanethomas left a comment

Choose a reason for hiding this comment

stephanethomas Oct 5, 2022

Choose a reason for hiding this comment

nightnei Oct 5, 2022 • edited

Choose a reason for hiding this comment

nightnei commented Oct 5, 2022

wongasy Oct 5, 2022

Choose a reason for hiding this comment

AllTerrainDeveloper commented Oct 5, 2022

wongasy commented Oct 5, 2022

AllTerrainDeveloper commented Oct 6, 2022

wongasy commented Oct 6, 2022

olaseni left a comment

Choose a reason for hiding this comment

fredrikekelund left a comment

Choose a reason for hiding this comment

fredrikekelund Oct 10, 2022

Choose a reason for hiding this comment

AllTerrainDeveloper commented Oct 11, 2022

a8ci18n commented Oct 12, 2022

wongasy commented Oct 4, 2022 •

edited by stephanethomas

github-actions bot commented Oct 4, 2022 •

edited by matticbot

matticbot commented Oct 4, 2022 •

edited

nightnei Oct 5, 2022 •

edited