upcoming: [M3-7775] - Disable ability to edit or delete a proxy user via User Profile page #10202

mjac0bs · 2024-02-16T18:50:38Z

Description 📝

In Cloud Manager, a proxy user cannot be deleted, nor can its email address or username be edited once it is provisioned.

This PR is a follow up from #10103 which restricted proxy users from updating their username or email on the Display Settings page (http://localhost:3000/profile/display). There is one additional place that a proxy user could be edited, which is by visiting the User Profile page for the proxy user via URL. (The User Profile button is not visible for a proxy user on the Users & Grants page, http://localhost:3000/account/users.)

Changes 🔄

List any change relevant to the reviewer.

Disables the username and email fields and the buttons on the proxy user's profile. Displays tooltips with reason for disabling.
Separates out tests in change-username.spec.ts into two different files: one for the Display Settings section (display-settings.spec.ts) and another for the User Profile page (user-profile.spec.ts). This was suggested a while ago and included in the AC of another ticket in the backlog, but it was cleaner to make the switch now rather than shoehorn additional test coverage into where it didn't really belong.

Preview 📷

Before	After
ChildAbleToEditProxyUser.mov	ChildUserTryingToEditProxyAccount.mov
ProxyAbleToEditProxyUser.mov	ProxyTryingToEditItsOwnAccount.mov

How to test 🧪

Prerequisites

(How to setup test environment)

Check out this PR and yarn dev.
Make sure Parent/Child feature flag and mocks are on.

Reproduction steps

(How to reproduce the issue, if applicable)

Go to http://localhost:3000/account/users/ParentCompany_a1b2c3d4e5 and observe you can edit the username, email and have the ability to delete the account.
See also: videos.

Verification steps

(How to verify changes)

Change the MSW */profile endpoint to mock the proxy user as the current active user.

const profile = profileFactory.build({
      restricted: false,
      // Parent/Child: switch the `user_type` depending on what account view you need to mock.
      user_type: 'proxy',
      username: 'ParentCompany_a1b2c3d4e5',
    });

Go to http://localhost:3000/account/users/ParentCompany_a1b2c3d4e5) and confirm the username, email, and ability to delete the account are disabled.
Change the MSW */profile endpoint to mock a child user

const profile = profileFactory.build({
      restricted: false,
      // Parent/Child: switch the `user_type` depending on what account view you need to mock.
      user_type: 'child',
    });

Go to http://localhost:3000/account/users/ParentCompany_a1b2c3d4e5) and confirm the username, email, and ability to delete the account are disabled.
Confirm no regressions to User Profile page for other users (default, restricted, current active user).
Confirm tests pass:

yarn cy:run -s "cypress/e2e/core/account/user-profile.spec.ts, cypress/e2e/core/account/display-settings.spec.ts"

As an Author I have considered 🤔

Check all that apply

jaalah-akamai · 2024-02-17T02:02:02Z

packages/manager/src/features/Users/UserProfile.tsx


  const [
    deleteConfirmDialogOpen,
    setDeleteConfirmDialogOpen,
  ] = React.useState<boolean>(false);

+  const isProxyUserProfile = currentUser?.user_type === 'proxy';


Nit: Should this be just isProxyUser?

I used isProxyUserProfile to try to avoid any confusion that the active user is a proxy user -- they don't have to be, just the current user whose profile is being viewed. If it feels more confusing as it currently is, I can change it, but it made more sense in my mind.

jaalah-akamai · 2024-02-17T02:10:09Z

packages/manager/src/features/Users/UserProfile.tsx

+              isProxyUserProfile
+                ? RESTRICTED_FIELD_TOOLTIP
+                : profile?.username !== originalUsername
+                ? 'You can\u{2019}t change another user\u{2019}s email address.'


Nit: I'm not too sure I care for escaped characters but that's a preferential choice. Double quotes would work fine here

I agree. Our IDE may show a squiggly line or something but there is no issue with using plain quotes here and escaping makes finding strings in code harder

This is actually not just for escaping the quote - it's a curly quote, which is a stylistic choice in Cloud Manager. We can revisit with UX whether or not we want to continue to use the curly quote in CM in a cafe, but this change is intended to stay in line with our current standard for the single quote for the new messages and fix any that weren't consistent before.

This is not a very distinction easy to see in this screenshot, but the difference is visible with can't compared to user's:

Prod (straight apostrophe):

This branch (curly apostrophe):

jaalah-akamai

✅ Implementation looks good!

abailly-akamai

UI and code look great ✅
Confirmed the inability to delete an account based on the mock data provided in the PR description ✅

Quick question: I am aware "proxy" is a possible technical value for user_type, but are we going to display that to the user??

abailly-akamai · 2024-02-20T15:18:45Z

packages/manager/cypress/e2e/core/account/user-profile.spec.ts

+        .should('be.visible');
+    });
+  });
+});


abailly-akamai · 2024-02-20T15:21:11Z

packages/manager/src/features/Users/UserProfile.tsx

+              isProxyUserProfile
+                ? RESTRICTED_FIELD_TOOLTIP
+                : profile?.username !== originalUsername
+                ? 'You can\u{2019}t change another user\u{2019}s email address.'


I agree. Our IDE may show a squiggly line or something but there is no issue with using plain quotes here and escaping makes finding strings in code harder

packages/manager/src/features/Users/UserProfile.tsx

mjac0bs · 2024-02-20T17:53:18Z

Quick question: I am aware "proxy" is a possible technical value for user_type, but are we going to display that to the user??

@abailly-akamai - Thank you for catching this! Changed it to business partner in cd22592.

github-actions · 2024-02-20T18:13:51Z

Coverage Report: ✅
Base Coverage: 81.24%
Current Coverage: 81.24%

mjac0bs · 2024-02-20T18:59:20Z

The one test failure in CI was unrelated.

I'm going to go ahead and merge.

mjac0bs added the Parent / Child Account label Feb 16, 2024

mjac0bs self-assigned this Feb 16, 2024

mjac0bs added 3 commits February 16, 2024 12:41

Disable fields and buttons on proxy user's user profile page

720786d

Separate Display and User Profile test specs; add coverage

e3de1ae

Finish test coverage

bef3568

mjac0bs force-pushed the M3-7775-disable-edits-to-proxy-user-profile-page branch from 7f44e74 to bef3568 Compare February 16, 2024 21:32

mjac0bs added 2 commits February 16, 2024 13:35

Clean up

2d507a0

Meant to push the changesets too

6ca7a00

mjac0bs marked this pull request as ready for review February 16, 2024 22:19

mjac0bs requested review from a team as code owners February 16, 2024 22:19

mjac0bs requested review from cliu-akamai, cpathipa, abailly-akamai and jaalah-akamai and removed request for a team February 16, 2024 22:19

mjac0bs added the Ready for Review label Feb 16, 2024

jaalah-akamai reviewed Feb 17, 2024

View reviewed changes

jaalah-akamai approved these changes Feb 17, 2024

View reviewed changes

jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Feb 17, 2024

abailly-akamai approved these changes Feb 20, 2024

View reviewed changes

mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! Ready for Review labels Feb 20, 2024

Remove proxy user language

cd22592

mjac0bs merged commit 9e03e40 into linode:develop Feb 20, 2024
17 of 18 checks passed

This was referenced Mar 1, 2024

Release v1.114.0 - release → staging #10246

Merged

Release v1.114.0 - staging → master #10253

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [M3-7775] - Disable ability to edit or delete a proxy user via User Profile page #10202

upcoming: [M3-7775] - Disable ability to edit or delete a proxy user via User Profile page #10202

mjac0bs commented Feb 16, 2024 •

edited

Loading

jaalah-akamai Feb 17, 2024

mjac0bs Feb 20, 2024

jaalah-akamai Feb 17, 2024

abailly-akamai Feb 20, 2024

mjac0bs Feb 20, 2024 •

edited

Loading

jaalah-akamai left a comment

abailly-akamai left a comment

abailly-akamai Feb 20, 2024

abailly-akamai Feb 20, 2024

mjac0bs commented Feb 20, 2024

github-actions bot commented Feb 20, 2024

mjac0bs commented Feb 20, 2024

upcoming: [M3-7775] - Disable ability to edit or delete a proxy user via User Profile page #10202

upcoming: [M3-7775] - Disable ability to edit or delete a proxy user via User Profile page #10202

Conversation

mjac0bs commented Feb 16, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Prerequisites

Reproduction steps

Verification steps

As an Author I have considered 🤔

jaalah-akamai Feb 17, 2024

Choose a reason for hiding this comment

mjac0bs Feb 20, 2024

Choose a reason for hiding this comment

jaalah-akamai Feb 17, 2024

Choose a reason for hiding this comment

abailly-akamai Feb 20, 2024

Choose a reason for hiding this comment

mjac0bs Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

jaalah-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai Feb 20, 2024

Choose a reason for hiding this comment

abailly-akamai Feb 20, 2024

Choose a reason for hiding this comment

mjac0bs commented Feb 20, 2024

github-actions bot commented Feb 20, 2024

mjac0bs commented Feb 20, 2024

mjac0bs commented Feb 16, 2024 •

edited

Loading

mjac0bs Feb 20, 2024 •

edited

Loading