Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable add/remove mailbox buttons based on user permissions #64322

Merged

Conversation

fredrikekelund
Copy link
Contributor

Proposed Changes

For sites with multiple users, only the owner of the Titan/Google Workspace subscription is allowed to add/remove mailboxes. Currently, there is no preemptive warning to users about this. This means that they can click the button to add/remove mailboxes, but the action will fail.

With this change, the "Remove mailbox" button is hidden entirely and the "Add new mailboxes" button is disabled if the current user isn't allowed to perform those actions. We also display an explanation in EmailPlanHeader for why the buttons are disabled.

Screen Shot 2022-06-03 at 10 08 49

Testing Instructions

  1. Have a site with at least two users and a Titan/Google Workspace subscription. The secondary user should be an administrator.
  2. Log in as the secondary user
  3. Navigate to /email/:domain/manage/:site_slug
  4. Verify that the action menu for each mailbox does not contain a "Remove mailbox" button
  5. Verify that the "Add new mailboxes" button is disabled
  6. Verify that there is a warning text in the header that says Only the email subscription owner can add or remove mailboxes.

Related to #

@fredrikekelund fredrikekelund self-assigned this Jun 3, 2022
@fredrikekelund fredrikekelund requested a review from a team as a code owner June 3, 2022 08:43
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 3, 2022
@matticbot
Copy link
Contributor

matticbot commented Jun 3, 2022

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

Sections (~102 bytes added 📈 [gzipped])

name   parsed_size           gzip_size
email       +431 B  (+0.1%)     +102 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.

&:hover {
&.is-selected,
&:hover,
&:focus {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor additional change that I snuck in here. Here are two screenshots that illustrate the difference it makes when the action menu is initially opened.

before

after

@@ -420,10 +422,10 @@
width: 100%;

.email-plan-warnings__warning {
padding-top: 30px;
padding-top: 24px;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that it's now possible that two .email-plan-warnings__warnings are displayed simultaneously, I found it appropriate to reduce the spacing between them slightly. Here's a screenshot of the result:

Screen Shot 2022-06-03 at 10 42 46

@@ -375,7 +382,7 @@ const EmailMailboxActionMenu = ( { account, domain, mailbox } ) => {
visible={ removeTitanMailboxDialogVisible }
/>
) }
<EllipsisMenu position="bottom" className="email-mailbox-action-menu__main">
<EllipsisMenu position="bottom left" className="email-mailbox-action-menu__main">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a styling fix that prevents overflow trouble with the action menu on smaller screens (e.g. laptops). Using bottom left matches the behavior in the DNS editor. Here are two screenshots that illustrate the difference this makes (notice the vertical viewport scrollbar in the first screenshot):

before

after

Copy link
Contributor

@stephanethomas stephanethomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had minor suggestions but otherwise this looks good, and works great.

@fredrikekelund fredrikekelund force-pushed the update/email-management-disable-add-remove-mailbox branch from a608692 to 065b8e8 Compare June 8, 2022 12:12
@fredrikekelund fredrikekelund merged commit 5ca06a9 into trunk Jun 8, 2022
@fredrikekelund fredrikekelund deleted the update/email-management-disable-add-remove-mailbox branch June 8, 2022 12:26
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 8, 2022
@a8ci18n
Copy link

a8ci18n commented Jun 8, 2022

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

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

@a8ci18n
Copy link

a8ci18n commented Jun 16, 2022

Translation for this Pull Request has now been finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants