wp-calypso
wp-calypso copied to clipboard
Emails: Add informative message and actions when user can't add email to the selected domain
Proposed Changes
As shown here 1200182182542585-as-1201225902975505/f we should be more informative to the user when the user is not able to add an email subscription to the domain, when the user is not the owner of the domain.
Testing Instructions
- Transfer a domain to your site, where your user is not the owner, and there are no email subscriptions attached to it
- Go Upgrades -> Emails
- Choose the transferred domain
- Assert that you see this screen:
Assert the following statements:
- The "log in" redirects you to the login page
- The reach-out link is enabled if the domain is registered with us and has privacy enabled.
- Each of the select buttons are disabled
Assert when clicking on reach out link that you see the following screen:
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 #66650
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:
Sections (~740 bytes added 📈 [gzipped])
name parsed_size gzip_size
email +3570 B (+0.5%) +1122 B (+0.6%)
domains +2495 B (+0.2%) +740 B (+0.2%)
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.
The requirements changed a little bit over the course of a discussion in the comments, but we concluded that the Select
button in EmailProvidersStackedCard
should not link to the log in page for unauthorized users, but rather just be disabled.
pbLl1t-1v2-p2#comment-1425
@wongasy The tracks event seems to be working fine for me. I've addressed every other one concern :)
I have a followup pull request (for a different task) incoming that will refactor the message into its own component <EmailNonDomainOwnerMessage domain={ domain } selectedSite={ selectedSite } usePromoCard={ true }>
Making a note that #68156 is related to this PR.
I have a followup pull request (for a different task) incoming that will refactor the message into its own component <EmailNonDomainOwnerMessage domain={ domain } selectedSite={ selectedSite } usePromoCard={ true }>
@AllTerrainDeveloper It would make sense to do the refactoring in a separate PR if we were refactoring code that exists in trunk
. But given that we are introducing new code in this PR anyways, might as well do the refactoring here to avoid potential regression issues that might be introduced in the refactoring.
I have a followup pull request (for a different task) incoming that will refactor the message into its own component <EmailNonDomainOwnerMessage domain={ domain } selectedSite={ selectedSite } usePromoCard={ true }>
@AllTerrainDeveloper It would make sense to do the refactoring in a separate PR if we were refactoring code that exists in
trunk
. But given that we are introducing new code in this PR anyways, might as well do the refactoring here to avoid potential regression issues that might be introduced in the refactoring.
Done :)
@AllTerrainDeveloper I noticed an issue where I'm seeing the non-owner screen in the email upsell step for adding a domain:
![]()
Good catch, fixed.
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/7557365
Thank you @AllTerrainDeveloper for including a screenshot in the description! This is really helpful for our translators.
@AllTerrainDeveloper Other than the minor comment I left regarding tracking, I also found a weird issue, where when user gets redirected to the email comparison page, the Professional Email card is left collapsed. I think source=login-redirect
might be triggering that logic for collapsing the PE card.

@wongasy that issue was a tough one! Due to the nature of the async properties being fetched, the initial state is triggered with the initial loading state (which may or not have finished fetching everything)
I've added a useEffect
hook with a dependency to the flag that is taking time to load (and will also determine the visibility of the notice)
@AllTerrainDeveloper I ran into an issue while reviewing the other PR on the Email Management page changes.
When we remove all mailboxes, the non-owner will see the Email Comparison screen instead of the Email Management screen with the warning messages. This becomes an issue because non-owner admins could end up setting up Email Forwarding, when we have an active email subscription in place. The ideal fix would be to ensure non-owner users land on the Email Management page with the appropriate warning. However, that may involve a move complex fix with routing. As an alternative, we can simply update the Email Comparison screen to hide the Email Forwarding for non-owners.

@AllTerrainDeveloper Confirming that the email forward link has been hidden for non-owners.

When we remove all mailboxes, the non-owner will see the Email Comparison screen instead of the Email Management screen with the warning messages
I wasn't able to reproduce this. Here's what I did:
- On a site with a single domain with Professional Email.
- With the owner account, delete all mailboxes in that subscription.
- With the non-owner account, go to Upgrades > Emails in Calypso.
- I am immediately forwarded to the
titan/set-up-mailbox
page.
Could you outline the steps to reproduce this @wongasy?
Also, with this discussion in mind pbLl1t-1z0-p2, I'm thinking that we maybe shouldn't hide the email forwarding option for non-owner users? Being able to set up email forwarding if there's an existing email subscription sounds like a bug, but right now it looks like non-owners will never see that Start with Email Forwarding
link, right?
Could you outline the steps to reproduce this?
@fredrikekelund Hm... looks like I'm also unable to reproduce this. Initially I thought this could be related to the sandbox store, but I've tried both the regular store and sandbox store, and I was not able to reproduce the issue, which is great!
I'm thinking that we maybe shouldn't hide the email forwarding option for non-owner users? Being able to set up email forwarding if there's an existing email subscription sounds like a bug, but right now it looks like non-owners will never see that Start with Email Forwarding link, right?
If we allow any non-owners to do the initial enabling of email forwarding, then there's a lower chance for the domain-owner to see the Email Comparison page. When they visit Upgrades > Email, they'll see the Email Forwarding management options with only a small button that takes them to the Email Comparison page. From the perspective of promoting a paid service, I think it makes sense to hide the email forwarding option for non-owners. Essentially, only the domain-owner should have the right to choose an email solution (paid or free). And once the email solution is chosen, then non-owner admins can manage the solution, in the case of email forward, they can set up additional forwarding (what we've fixed recently). What do you think?
🤔 On the one hand, the behavior that you describe (only the owner can set up, but other admins can manage) is closest to what we have today. Because right now, if a user isn't allowed to add email, we don't even show the Add Email
link for that domain. On the other hand, I think it could be helpful to let all admin users add email forwarding so long as there aren't any technical obstacles to that (in the same way that ownership of purchased subscriptions is a technical obstacle).
IMO, admin user privileges should typically mean that a user can perform almost all actions on the site. We could also contrast with what user privileges are required to edit DNS records, which is something I believe all site admins can do.
So I think I'd cast my vote for showing the link to all users, simply because it's helpful and the Set up Email Forwarding
link is already quite discrete. The difference in conversion should be slim, and we're already improving it with the other PR's within this same effort.
I do agree that the link for setting up Email Forwarding is so subtle that the potential conversion impact may not actually be a valid concern. In the interest of wrapping this up quickly, @AllTerrainDeveloper I've gone ahead and reverted the change for hiding the Email Forwarding Link.
Translation for this Pull Request has now been finished.