Adding check to show/hide Avatar form based on whether the user is a … #135743

kc13greiner · 2022-07-05T17:46:33Z

Summary

Removing the UI for Cloud Users to update their avatar (the backend already prohibited Profile updates)

Cloud User:

Non-cloud User:

@MichaelMarcialis I would like to include the changes to the Right Side Nav menu (last point described here.

We probably need to adjust references to the profile page in the Elastic Cloud context (we use Profile, User settings and Preferences words

What are your thoughts?

Resolves #132645

…cloud user or not

kc13greiner · 2022-07-05T17:55:35Z

@elasticmachine merge upstream

…f the AuthorizedUser is a cloud user

MichaelMarcialis · 2022-07-06T20:44:28Z

@MichaelMarcialis I would like to include the changes to the Right Side Nav menu (last point described here.

We probably need to adjust references to the profile page in the Elastic Cloud context (we use Profile, User settings and Preferences words

What are your thoughts?

Based on my initial wireframes (which included cloud flows for user profiles), the hope was that we would offer the following options for cloud users consistently across both the cloud portal and Kibana user menus:

Edit profile and settings
View profile
Organization
Billing
Log out

The hope was that Edit profile and settings would take cloud users directly to their cloud portal user profile page, rather than offering them two links to two different profile pages like we do currently (one for cloud, one for deployment). I don't see the value in taking cloud users to a deployment-level profile page that is empty or immutable. I'd much prefer to take them directly to their cloud portal user profile page if possible and simply not offer an unnecessary deployment-level offshoot.

Additionally, the Edit profile and settings option should likely be reworded to Edit profile for the time being, as the user settings portion is planned future addition. View profile is also a future consideration, so it can be omitted for the time being. Organization, Billing, and Log out are self-explanatory, so I'll skip talking about those.

For the sake of comparison, we would offer non-cloud users the following options in the Kibana user menu:

…le only for non-cloud users

kc13greiner · 2022-07-07T19:45:36Z

Non-Cloud:

Cloud User:

@MichaelMarcialis I removed the 'Preferences' item for cloud and kept the non-cloud as 'Profile' to match the cloud 'Profile' option for a more unified look since we don't have cloud profile editing yet (unless it's ready!)

What do you think?

elasticmachine · 2022-07-07T19:47:20Z

Pinging @elastic/kibana-security (Team:Security)

MichaelMarcialis · 2022-07-08T14:26:35Z

@MichaelMarcialis I removed the 'Preferences' item for cloud and kept the non-cloud as 'Profile' to match the cloud 'Profile' option for a more unified look since we don't have cloud profile editing yet (unless it's ready!)

What do you think?

Thanks, @kc13greiner! I have a couple of questions/comments:

Just to be sure I understand, does this mean the Profile button for the cloud user will continue to take them to their profile page on the cloud portal (i.e. https://cloud.elastic.co/user/settings), and there will be no link in the menu taking cloud users to an empty/immutable deployment profile page? If so, that's great; exactly what I was hoping for!
Assuming my understanding above is correct, can we change the Profile text for both cloud and non-cloud users to Edit profile?
Currently the Account & Billing link for cloud users is actually taking them to the organization page in the cloud portal. Can we 1) change the name of this link to Organization with an icon of users, and 2) add a new link for cloud users called Billing with icon payment that goes to https://cloud.elastic.co/billing? Doing so will make this menu's contents more consistent with the cloud portal user menu:

Regarding some of the additional enhancements I was proposing for the user menu in my wireframes, should that be considered a separate issue? In this case, I'm speaking of the inclusion of the user's avatar, display name, full name (if different than display name), and email address in the menu header. Or should it be part of this PR as well?

kc13greiner · 2022-07-08T18:31:47Z

@MichaelMarcialis

Just to be sure I understand, does this mean the Profile button for the cloud user will continue to take them to their profile page on the cloud portal (i.e. https://cloud.elastic.co/user/settings), and there will be no link in the menu taking cloud users to an empty/immutable deployment profile page? If so, that's great; exactly what I was hoping for!

Exactly that!

Assuming my understanding above is correct, can we change the Profile text for both cloud and non-cloud users to Edit profile?

I will have to check with @azasypkin since the Cloud Profile menu item is in the Cloud directory (Im not sure what the protocol is for changing other teams files yet.)

Currently the Account & Billing link for cloud users is actually taking them to the organization page in the cloud portal. Can we 1) change the name of this link to Organization with an icon of users, and 2) add a new link for cloud users called Billing with icon payment that goes to https://cloud.elastic.co/billing? Doing so will make this menu's contents more consistent with the cloud portal user menu:

Regarding some of the additional enhancements I was proposing for the user menu in my wireframes, should that be considered a separate issue? In this case, I'm speaking of the inclusion of the user's avatar, display name, full name (if different than display name), and email address in the menu header. Or should it be part of this PR as well?

Would it be alright if I handle these in a separate PR?

kc13greiner · 2022-07-08T18:46:00Z

@elasticmachine merge upstream

azasypkin · 2022-07-11T06:58:51Z

Assuming my understanding above is correct, can we change the Profile text for both cloud and non-cloud users to Edit profile?

@MichaelMarcialis sorry for the confusion, we referred to the Profile in the Cloud Console user menu (our assumption was that the goal is to unify Kibana and Cloud Console user menus as much as possible for the Cloud managed users). That's why we kept Profile in Kibana since it's still Profile in the Cloud Console (these lead to the same page), or the Cloud team is planning to rename this menu to Edit profile on their end as well?

MichaelMarcialis · 2022-07-11T14:21:25Z

Currently the Account & Billing link for cloud users is actually taking them to the organization page in the cloud portal. Can we 1) change the name of this link to Organization with an icon of users, and 2) add a new link for cloud users called Billing with icon payment that goes to https://cloud.elastic.co/billing? Doing so will make this menu's contents more consistent with the cloud portal user menu:

Regarding some of the additional enhancements I was proposing for the user menu in my wireframes, should that be considered a separate issue? In this case, I'm speaking of the inclusion of the user's avatar, display name, full name (if different than display name), and email address in the menu header. Or should it be part of this PR as well?

Would it be alright if I handle these in a separate PR?

@kc13greiner: Certainly. Separate PR would be great!

Assuming my understanding above is correct, can we change the Profile text for both cloud and non-cloud users to Edit profile?

@MichaelMarcialis sorry for the confusion, we referred to the Profile in the Cloud Console user menu (our assumption was that the goal is to unify Kibana and Cloud Console user menus as much as possible for the Cloud managed users). That's why we kept Profile in Kibana since it's still Profile in the Cloud Console (these lead to the same page), or the Cloud team is planning to rename this menu to Edit profile on their end as well?

@azasypkin: Regarding the suggestion to change the text of Profile to Edit profile, I was just thinking that it might be good to 1) align the text with what was being proposed in the wireframes and 2) prepare for the future distinction between editing and viewing a user's profile. I'm not aware of any plans on the cloud team's side to make these text changes. That said, given that we don't have to make the distinction between editing and viewing currently, we can always save that change for later if you'd prefer.

azasypkin · 2022-07-11T14:46:16Z

@azasypkin: Regarding the suggestion to change the text of Profile to Edit profile, I was just thinking that it might be good to 1) align the text with what was being proposed in the wireframes and 2) prepare for the future distinction between editing and viewing a user's profile. I'm not aware of any plans on the cloud team's side to make these text changes. That said, given that we don't have to make the distinction between editing and viewing currently, we can always save that change for later if you'd prefer.

Got it, thanks. If we're not concerned about the Profile vs Edit profile difference between Cloud Console and Kibana user menus then it makes sense to stick to the wireframes as you suggested and rename Profile to Edit profile in Kibana in this PR.

azasypkin · 2022-07-12T09:35:11Z

x-pack/plugins/security/public/nav_control/nav_control_component.tsx

    const profileMenuItem: EuiContextMenuPanelItemDescriptor = {
      name: (
        <FormattedMessage
          id="xpack.security.navControlComponent.editProfileLinkText"
-          defaultMessage="{profileOverridden, select, true{Preferences} other{Profile}}"
-          values={{ profileOverridden: hasCustomProfileLinks }}
+          defaultMessage="Edit Profile"


nit: I think you also need to update text here

kibana/x-pack/plugins/cloud/public/user_menu_links.ts

Line 20 in 754fdea

defaultMessage: 'Profile',

to have the same text for all users (either Cloud managed or not).

Resolved in latest commit!

azasypkin · 2022-07-12T09:48:12Z

x-pack/plugins/translations/translations/fr-FR.json

@@ -24061,7 +24061,7 @@
    "xpack.security.management.users.usersTitle": "Utilisateurs",
    "xpack.security.management.usersTitle": "Utilisateurs",
    "xpack.security.navControlComponent.accountMenuAriaLabel": "Menu Compte",
-    "xpack.security.navControlComponent.editProfileLinkText": "{profileOverridden, select, true{Préférences} other{Profil}}",
+    "xpack.security.navControlComponent.editProfileLinkText": "Profil",


nit: if you happened to update translation files manually then it'd make sense to revert these changes and run node scripts/i18n_check.js --fix to remove outdated translations instead. Updated translations will be eventually provided by our translators team.

The translation files have been changed back in the latest commit and re-ran the script

Which looks like it just removed the line entirely, is that correct?

Yep, that's correct!

…ofile link and unit tests

MichaelMarcialis

Thanks for putting this together, @kc13greiner. The non-cloud user menu looks good. However, the cloud user menu appears to still show both profile links (first one leads to cloud profile page, second leads to deployment profile page). Can we remove the link to the deployment profile page for cloud users?

…if there is no custom Edit Profile link passed in by another plugin

MichaelMarcialis

Thanks for making that change, @kc13greiner. LGTM!

azasypkin · 2022-07-19T08:03:29Z

x-pack/plugins/security/public/nav_control/nav_control_component.tsx

+  const isAnonymous = currentUser.value ? isUserAnonymous(currentUser.value) : false;
+  const hasCustomProfileLinks = userMenuLinks.some(({ setAsProfile }) => setAsProfile === true);
+
+  if (!isAnonymous && !hasCustomProfileLinks) {


issue: while testing this change I discovered that we rely on the user's roles (specifically superuser role) to determine whether we should show Cloud specific links or not (added in #97870). If we keep this Cloud plugin logic as is, after this PR non-Cloud managed superusers won't be able to edit their profiles in Kibana (we'll be rendering a single Edit profile link leading to Cloud UI instead of Profile and Preferences links we had previously).

@legrego do I understand correctly that we can now switch this code to rely on elastic_cloud_user user property instead or is there still a legitimate reason for non-Cloud managed superusers to see Cloud specific links?

@legrego do I understand correctly that we can now switch this code to rely on elastic_cloud_user user property instead or is there still a legitimate reason for non-Cloud managed superusers to see Cloud specific links?

That's correct, we should switch this code to rely on elastic_cloud_user instead of the role check. The superuser check was a hard-coded workaround because we didn't have a way to reliably determine which users were cloud users back then.

Great, thanks for confirming, Larry.

…d related unit tests

kibana-ci · 2022-07-19T18:53:26Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`security`	504.4KB	504.5KB	+30.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cloud`	11.4KB	11.3KB	-4.0B
`security`	51.1KB	51.0KB	-103.0B
total			-107.0B

History

💚 Build #58341 succeeded b327af7
💔 Build #58308 failed 9bfd590
💚 Build #57057 succeeded 6b12199
💚 Build #56997 succeeded 43a12fe
💚 Build #56742 succeeded 1067020
💛 Build #56409 was flaky a3dd8c4

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

azasypkin

LGTM, great job!

Adding check to show/hide Avatar form based on whether the user is a …

9291b07

…cloud user or not

kc13greiner added the ci:deploy-cloud label Jul 5, 2022

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

044fae9

kibanamachine and others added 4 commits July 5, 2022 13:55

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

60f7c0c

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

b668d7f

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

5457cc7

Adding tests to verify the avatar doesnt show up in the UserProfile i…

8df287d

…f the AuthorizedUser is a cloud user

kc13greiner requested a review from MichaelMarcialis July 6, 2022 16:04

kc13greiner added 3 commits July 7, 2022 11:11

Changing the name of the link to 'Edit Profile' and making it availab…

45d16fc

…le only for non-cloud users

Adding/Updating unit tests and fixing translation files

af6af23

Removing unused values from FormattedText and related tests

b32c578

kc13greiner marked this pull request as ready for review July 7, 2022 19:45

kc13greiner requested a review from a team as a code owner July 7, 2022 19:45

kc13greiner requested a review from azasypkin July 7, 2022 19:46

kc13greiner added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.4.0 labels Jul 7, 2022

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

af16260

kc13greiner added the release_note:skip Skip the PR/issue when compiling release notes label Jul 7, 2022

kc13greiner marked this pull request as draft July 8, 2022 12:03

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

662482c

kc13greiner and others added 2 commits July 8, 2022 11:09

Updating unit test to work with merge from main

bc54331

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

20adf25

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

a3dd8c4

Updating link to read Edit Profile to match wireframes per PR review

1067020

kc13greiner marked this pull request as ready for review July 11, 2022 17:15

azasypkin reviewed Jul 12, 2022

View reviewed changes

Reverting changes to translation files and changing the cloud Edit Pr…

43a12fe

…ofile link and unit tests

kc13greiner requested a review from a team as a code owner July 12, 2022 11:21

pgayvallet approved these changes Jul 12, 2022

View reviewed changes

Changing capitalization of 'profile' so it follows the naming convention

6b12199

MichaelMarcialis reviewed Jul 12, 2022

View reviewed changes

kc13greiner and others added 2 commits July 18, 2022 10:51

Changing nav menu logic to only render the default Edit Profile link …

9bfd590

…if there is no custom Edit Profile link passed in by another plugin

Merge branch 'main' into feature/hide_avatar_form_for_cloud_user

b327af7

kc13greiner requested review from MichaelMarcialis and azasypkin July 18, 2022 17:03

MichaelMarcialis approved these changes Jul 18, 2022

View reviewed changes

azasypkin reviewed Jul 19, 2022

View reviewed changes

Updating logic to display custom nav links if user is a cloud user an…

e3afd79

…d related unit tests

kc13greiner requested a review from azasypkin July 19, 2022 19:00

azasypkin approved these changes Jul 20, 2022

View reviewed changes

kc13greiner merged commit 2f732ae into elastic:main Jul 20, 2022

kibanamachine added the backport:skip This commit does not require backporting label Jul 20, 2022

kc13greiner deleted the feature/hide_avatar_form_for_cloud_user branch July 20, 2022 11:32

tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding check to show/hide Avatar form based on whether the user is a … #135743

Adding check to show/hide Avatar form based on whether the user is a … #135743

kc13greiner commented Jul 5, 2022 •

edited by legrego

Loading

kc13greiner commented Jul 5, 2022

MichaelMarcialis commented Jul 6, 2022

kc13greiner commented Jul 7, 2022

elasticmachine commented Jul 7, 2022

MichaelMarcialis commented Jul 8, 2022

kc13greiner commented Jul 8, 2022

kc13greiner commented Jul 8, 2022

azasypkin commented Jul 11, 2022

MichaelMarcialis commented Jul 11, 2022

azasypkin commented Jul 11, 2022

azasypkin Jul 12, 2022

kc13greiner Jul 12, 2022

azasypkin Jul 12, 2022

kc13greiner Jul 12, 2022

kc13greiner Jul 12, 2022

azasypkin Jul 12, 2022

MichaelMarcialis left a comment

MichaelMarcialis left a comment

azasypkin Jul 19, 2022

legrego Jul 19, 2022

azasypkin Jul 19, 2022

kibana-ci commented Jul 19, 2022

azasypkin left a comment

Adding check to show/hide Avatar form based on whether the user is a … #135743

Adding check to show/hide Avatar form based on whether the user is a … #135743

Conversation

kc13greiner commented Jul 5, 2022 • edited by legrego Loading

Summary

kc13greiner commented Jul 5, 2022

MichaelMarcialis commented Jul 6, 2022

kc13greiner commented Jul 7, 2022

elasticmachine commented Jul 7, 2022

MichaelMarcialis commented Jul 8, 2022

kc13greiner commented Jul 8, 2022

kc13greiner commented Jul 8, 2022

azasypkin commented Jul 11, 2022

MichaelMarcialis commented Jul 11, 2022

azasypkin commented Jul 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelMarcialis left a comment

Choose a reason for hiding this comment

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Jul 19, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

azasypkin left a comment

Choose a reason for hiding this comment

kc13greiner commented Jul 5, 2022 •

edited by legrego

Loading