Grouped features for space management #74151

legrego · 2020-08-03T19:04:44Z

Summary

Adds a new `category` property to each registered feature

The feature schema has been updated to require a category, which tells Kibana how to visually organize the various features.

This PR exclusively uses the DEFAULT_APP_CATEGORIES exposed by core for all registered features, but the schema will accept any category so long as it satisfies the underlying AppCategory interface.

Groups features by category for Spaces Management

The spaces management screen has been updated to take advantage of these categories. The old implementation displayed a flat table of features, and the ordering was not always predictable.

The updated interface organizes features according to the category in which they exist. Features can be toggled on/off independently as before, and users can also toggle entire categories.

Out of scope

The role management screen is out-of-scope for this PR, and will be addressed in a followup.
Additional design edits for spaces management are out-of-scope for this PR, and will be addressed in a followup as time permits.

elastic/eui#3881 prevents the categories from rendering an extraAction, which I would otherwise use to indicate that a section has x/y features enabled. I don't consider this a blocker for merge however.

Preview of docs changes: https://kibana_74151.docs-preview.app.elstc.co/guide/en/kibana/master/development-security.html

Relates: #72659
Resolves #54793

legrego

Author's notes for reviewers

legrego · 2020-09-14T13:23:12Z

x-pack/plugins/spaces/public/management/edit_space/customize_space/customize_space.tsx

note: no functional changes to this view. This is a re-org to support future changes to the avatar customization

legrego · 2020-09-14T13:29:21Z

x-pack/plugins/spaces/public/management/spaces_management_app.tsx

note: wrapping in <RedirectAppLinks> allows our links into the Role Management app to use client-side routing instead of forcing a full-page reload.

legrego · 2020-09-14T17:40:25Z

src/core/utils/default_app_categories.ts

Discussed with the design team, and the decision was to add an icon for the Management section to be consistent with the other feature categories.

…atures

sorenlouv · 2020-09-16T13:01:31Z

based on the discussions in that PR, it seems like it's meant to be a part of the "APM" feature, and not a true standalone feature. @sqren am I understanding this correctly? Do you have any suggestions on how we can make this clearer to end users?
@legrego

Client Side Monitoring (CSM) is currently technically located in the APM plugins folder but from a user perspective it's a separate app (still under Observability). In 7.11 we hope to move CSM out of APM so it also technically is a separate app.
Since the CSM app primarily relies on APM data it might make sense to keep it under APM from a permissions-point-of-view.

@shahzad31 WDYT?

shahzad31 · 2020-09-16T13:04:31Z

based on the discussions in that PR, it seems like it's meant to be a part of the "APM" feature, and not a true standalone feature. @sqren am I understanding this correctly? Do you have any suggestions on how we can make this clearer to end users?
@legrego

Client Side Monitoring (CSM) is currently technically located in the APM plugins folder but from a user perspective it's a separate app (still under Observability). In 7.11 we hope to move CSM out of APM so it also technically is a separate app.
Since the CSM app primarily relies on APM data it might make sense to keep it under APM from a permissions-point-of-view.

@shahzad31 WDYT?

@sorantis Yes it will keep following APM permission POV.

Once we have synthetic data it might change but for now. It will remain as is.

jportner · 2020-09-16T13:30:38Z

it seems like it's meant to be a part of the "APM" feature, and not a true standalone feature.

@legrego @sqren Could we consider changing the display string for the "APM" feature to say "APM and Client Side Monitoring"? Then at least the user would see that the one feature is really enabling two applications in the main nav.

x-pack/plugins/apm/server/feature.ts

…atures

legrego · 2020-09-16T20:09:19Z

@elasticmachine merge upstream

legrego · 2020-09-16T20:21:55Z

@jportner CI is struggling right now, but this is otherwise ready for another review.

Note that EUI merged the bugfix I was waiting on, so 2ed14c5 (#74151) re-introduces the extraAction alongside each category with more than a single item.

spong

Security changes LGTM -- thanks @legrego! 🙂

andrewvc

LGTM from Uptime's side

legrego · 2020-09-17T12:45:21Z

@elasticmachine merge upstream

jportner

More feedback and optional nits

I like that you changed the "Enterprise Search" and "Management" categories so they're just a single checkbox!
I like the accordion's extraAction!
We could switch the order of the "Observability" features so they more closely match the order that's in the navigation pane (Logs, then Metrics, then APM and Client Side Monitoring, then Uptime)
It's not immediately clear what disabling "Built-In Alerts" does..?
Disabling "Actions" just disables the "Connectors" tab of the "Alerts and Actions" page... perhaps some of these features could use a tooltip describing what they are?
Disabling the "Management" category entirely (0 / 8 features visible) still leaves the "Stack Management" page visible in the navigation pane (see screenshot below); I understand this is intended, but it's going to be confusing for end-users. Maybe we could use a tooltip for this category as well?

legrego · 2020-09-17T18:20:02Z

We could switch the order of the "Observability" features so they more closely match the order that's in the navigation pane (Logs, then Metrics, then APM and Client Side Monitoring, then Uptime)

Good catch, sorry I missed that in the first review.

It's not immediately clear what disabling "Built-In Alerts" does..?

Hmm. @gmmorris would you expect this toggle to turn off "Index Threshold alerts" for the space?

Disabling "Actions" just disables the "Connectors" tab of the "Alerts and Actions" page... perhaps some of these features could use a tooltip describing what they are?

Not a bad idea. I'd like to place this in the "Additional design edits for spaces management are out-of-scope for this PR, and will be addressed in a followup as time permits." category, at least for now.

I really want to have time to group the features on the role management page in 7.10 as well, so that is my next ux focus for this grouping initiative. If I didn't have role management to work on as well, then I'd definitely consider this now.

Disabling the "Management" category entirely (0 / 8 features visible) still leaves the "Stack Management" page visible in the navigation pane (see screenshot below); I understand this is intended, but it's going to be confusing for end-users. Maybe we could use a tooltip for this category as well?

Also a good idea. I don't yet know how to solve this holistically with my current approach, so I might 🙈 band-aid this for the time being by looking for this category explicitly, and rendering it differently.

…category

…fc/grouped-features

jportner

LGTM

…atures

kibanamachine · 2020-09-18T15:56:58Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a42faae

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
spaces	227	-1	228

page load bundle size

id	value	diff	baseline
core	1.2MB	+28.0B	1.2MB
features	15.8KB	+65.0B	15.8KB
spaces	489.6KB	+995.0B	488.7KB
total		+1.1KB

History

💚 Build #75323 succeeded b31c049
💚 Build #75177 succeeded c5ccddb
💔 Build #74936 failed 2ed14c5
💔 Build #74964 failed a41ff7a
💚 Build #74867 succeeded 0a29cb3

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

legrego · 2020-09-18T16:30:22Z

Doc updates (screenshots, etc) will follow after role management is updated as well

* Grouped features for space management * Apply suggestions from code review Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> * Address PR Feedback * docs changes * updating types/docs * update APM feature name * Reintroduce extraAction following EUI update * change ordering of infra features, and render callout for management category Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Joe Portner <5295965+jportner@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

legrego mentioned this pull request Aug 3, 2020

Design for Solution Grouping on Spaces and Roles Pages #72659

Open

legrego force-pushed the fc/grouped-features branch from 3318288 to dae6500 Compare August 14, 2020 15:52

legrego force-pushed the fc/grouped-features branch 3 times, most recently from 9fdacfa to 5acc79f Compare September 2, 2020 19:05

legrego force-pushed the fc/grouped-features branch 6 times, most recently from 5f35d8a to c392d66 Compare September 14, 2020 17:03

Grouped features for space management

856ef22

legrego force-pushed the fc/grouped-features branch from c392d66 to 856ef22 Compare September 14, 2020 18:29

legrego commented Sep 14, 2020

View reviewed changes

legrego marked this pull request as ready for review September 14, 2020 18:39

legrego requested review from a team as code owners September 14, 2020 18:39

legrego requested review from a team September 14, 2020 18:39

legrego requested a review from a team as a code owner September 14, 2020 18:39

legrego requested a review from a team September 14, 2020 18:39

legrego requested review from a team as code owners September 14, 2020 18:39

legrego added 2 commits September 16, 2020 07:18

updating types/docs

0d577dc

Merge branch 'master' of github.com:elastic/kibana into fc/grouped-fe…

b902099

…atures

sorenlouv reviewed Sep 16, 2020

View reviewed changes

x-pack/plugins/apm/server/feature.ts Outdated Show resolved Hide resolved

legrego added 4 commits September 16, 2020 12:29

update APM feature name

b40014e

Merge branch 'master' of github.com:elastic/kibana into fc/grouped-fe…

0a29cb3

…atures

Merge branch 'master' of github.com:elastic/kibana into fc/grouped-fe…

017cdde

…atures

Reintroduce extraAction following EUI update

2ed14c5

Merge branch 'master' into fc/grouped-features

a41ff7a

spong approved these changes Sep 16, 2020

View reviewed changes

andrewvc approved these changes Sep 16, 2020

View reviewed changes

Merge branch 'master' into fc/grouped-features

c5ccddb

jportner reviewed Sep 17, 2020

View reviewed changes

legrego added 2 commits September 17, 2020 14:45

change ordering of infra features, and render callout for management …

fe0a156

…category

Merge branch 'fc/grouped-features' of github.com:legrego/kibana into …

b31c049

…fc/grouped-features

jportner approved these changes Sep 17, 2020

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into fc/grouped-fe…

a42faae

…atures

legrego merged commit 9f3992f into elastic:master Sep 18, 2020

legrego deleted the fc/grouped-features branch September 18, 2020 16:31

legrego mentioned this pull request Sep 18, 2020

[7.x] Grouped features for space management (#74151) #77925

Merged

legrego mentioned this pull request Sep 25, 2020

Grouped features for role management #78152

Merged

Grouped features for space management #74151

Grouped features for space management #74151

Uh oh!

Conversation

legrego commented Aug 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Adds a new category property to each registered feature

Groups features by category for Spaces Management

Out of scope

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

legrego Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

legrego Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

legrego Sep 14, 2020

Choose a reason for hiding this comment

Uh oh!

sorenlouv commented Sep 16, 2020

Uh oh!

shahzad31 commented Sep 16, 2020

Uh oh!

jportner commented Sep 16, 2020

Uh oh!

Uh oh!

legrego commented Sep 16, 2020

Uh oh!

legrego commented Sep 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spong left a comment

Choose a reason for hiding this comment

Uh oh!

andrewvc left a comment

Choose a reason for hiding this comment

Uh oh!

legrego commented Sep 17, 2020

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

legrego commented Sep 17, 2020

Uh oh!

jportner left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Sep 18, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

page load bundle size

History

Uh oh!

legrego commented Sep 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants

legrego commented Aug 3, 2020 •

edited

Loading

Adds a new `category` property to each registered feature

legrego commented Sep 16, 2020 •

edited

Loading