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.

Sign up for GitHub

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

Jump to bottom

[Roles] Added optional role description #183145

Merged
merged 10 commits into from
May 16, 2024
Merged

[Roles] Added optional role description #183145

merged 10 commits into from
May 16, 2024

Conversation

elena-shostak
Copy link
Contributor

@elena-shostak elena-shostak commented May 10, 2024 •
edited
Loading

Summary

  1. Added optional role description field for Save/Edit Role page.
  2. Added tooltip with description for roles ComboBox that we render on the User and Role Mappings pages.
3. Updated RolesGridPage table responsive setup.
[Before] responsiveBreakpoint={false} [After] responsiveBreakpoint={true}
Before After
Screen.Recording.2024-05-13.at.18.15.21.mov

Checklist

For maintainers

Fixes: #173570

Release note

Added optional role description field for Save/Edit Role page.

@elena-shostak
Copy link
Contributor Author

/ci

2 similar comments
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak added release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Users/Roles/API Keys 8.15 candidate v8.15.0 labels May 13, 2024
@elena-shostak
Copy link
Contributor Author

/ci

@elena-shostak elena-shostak force-pushed the 173570-role-description branch from a1fa00f to 2e458f5 Compare May 13, 2024 16:44
@elena-shostak
Copy link
Contributor Author

/ci

elena-shostak
elena-shostak commented May 13, 2024
View reviewed changes
x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx
@@ -183,7 +183,6 @@ export class RolesGridPage extends Component<Props, State> {

<EuiInMemoryTable
itemId="name"
responsiveBreakpoint={false}
Copy link
Contributor Author

@elena-shostak elena-shostak May 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Without responsive breakpoints looks too dense and sort of broken. See screenshot in the PR description.

cc @legrego to confirm that such UI change would be okay.

Copy link
Member

Choose a reason for hiding this comment

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

Without responsive breakpoints looks too dense and sort of broken. See screenshot in the PR description.

cc @legrego to confirm that such UI change would be okay.

I don't remember why we explicitly disabled this in the past, but I think allowing the responsive breakpoint makes a lot of sense for this screen. I'm happy for you to include this in the PR. Thanks!

@elena-shostak elena-shostak marked this pull request as ready for review May 14, 2024 08:18
@elena-shostak elena-shostak requested a review from a team as a code owner May 14, 2024 08:18
@elasticmachine
Copy link
Contributor

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

@jeramysoucy jeramysoucy self-requested a review May 14, 2024 11:02
jeramysoucy
jeramysoucy requested changes May 14, 2024
View reviewed changes
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

@elena-shostak Nicely done! I am happy to approve as-is, but curious to first get your thoughts on some of these nits/suggestions.

Also, related...
@slobodanadamovic I noticed that the transport_client role didn't get a description.

docs/api/role-management/get-all.asciidoc Outdated Show resolved Hide resolved
docs/api/role-management/get-all.asciidoc Outdated Show resolved Hide resolved
docs/api/role-management/get.asciidoc Outdated Show resolved Hide resolved
docs/api/role-management/put.asciidoc Show resolved Hide resolved
docs/api/role-management/put.asciidoc Outdated Show resolved Hide resolved
docs/api/role-management/put.asciidoc Outdated Show resolved Hide resolved
x-pack/plugins/security/public/management/role_combo_box/role_combo_box.tsx Show resolved Hide resolved
x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx Outdated Show resolved Hide resolved
x-pack/plugins/security/public/management/roles/roles_grid/roles_grid_page.tsx Outdated Show resolved Hide resolved
x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx Outdated Show resolved Hide resolved
@slobodanadamovic
Copy link

slobodanadamovic commented May 14, 2024 •
edited
Loading

@slobodanadamovic I noticed that the transport_client role didn't get a description.

@jeramysoucy Oh snap, I missed this one. Thanks for catching it! I will raise a PR to add it.

Edit: opened elastic/elasticsearch#108633

jeramysoucy
jeramysoucy approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the additions!

@elena-shostak
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 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 582.0KB 583.7KB +1.7KB

History

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

@elena-shostak elena-shostak merged commit 4fade37 into elastic:main May 16, 2024
17 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legrego legrego legrego left review comments

@jeramysoucy jeramysoucy jeramysoucy approved these changes

Assignees
No one assigned
Labels
8.15 candidate backport:skip This commit does not require backporting Feature:Users/Roles/API Keys release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow roles to have an optional description
7 participants
@elena-shostak @elasticmachine @slobodanadamovic @kibana-ci @legrego @jeramysoucy @kibanamachine