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

[Roles] Fix wording for updating roles toast to match button text #194427

Merged

Conversation

SiddharthMantri
Copy link
Contributor

@SiddharthMantri SiddharthMantri commented Sep 30, 2024

Closes #194406

Summary

Fix wording for Roles update toast to match button when updating an existing role.

Screenshots

image

Release notes

Changes wording for toast message when updating an existing role successfully. The toast now matches the button to say Updated role

@SiddharthMantri SiddharthMantri requested a review from a team as a code owner September 30, 2024 12:44
@SiddharthMantri SiddharthMantri added release_note:enhancement Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Users/Roles/API Keys labels Sep 30, 2024
@elasticmachine
Copy link
Contributor

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

@jeramysoucy jeramysoucy self-requested a review October 1, 2024 13:10
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! Just one nit, which wasn't mentioned by the original reporter.

notifications.toasts.addSuccess(
i18n.translate(
'xpack.security.management.editRole.roleSuccessfullySavedNotificationMessage',
{ defaultMessage: 'Saved role' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the button in the create render of the edit role page reads "Create role". Should we also update the toast for this to match?

Suggested change
{ defaultMessage: 'Saved role' }
{ defaultMessage: 'Created role' }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i think it makes sense to do so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in c86d79c

@jeramysoucy jeramysoucy added backport:all-open Backport to all branches that could still receive a release backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.16.0 backport:skip This commit does not require backporting and removed backport:all-open Backport to all branches that could still receive a release backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 2, 2024
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, just left one last change request. Approving to not hold this up.

Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
@SiddharthMantri SiddharthMantri enabled auto-merge (squash) October 2, 2024 07:15
@SiddharthMantri
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group6.ts / discover/esql discover esql view switch modal should not show switch modal when switching to a data view while a saved search is open
  • [job] [logs] FTR Configs #46 / serverless observability UI Dataset Quality Flyout overview summary panel should show summary KPIs

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 590.8KB 591.0KB +157.0B

History

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

@SiddharthMantri SiddharthMantri merged commit 5ea1ab0 into elastic:main Oct 3, 2024
22 checks passed
tiansivive pushed a commit to tiansivive/kibana that referenced this pull request Oct 7, 2024
…astic#194427)

Closes elastic#194406

## Summary

Fix wording for Roles update toast to match button when updating an
existing role.

### Screenshots

<img width="746" alt="image"
src="https://github.com/user-attachments/assets/674a57f3-5a5a-445a-9266-8b55526aef72">


### Release notes
Changes wording for toast message when updating an existing role
successfully. The toast now matches the button to say `Updated role`

---------

Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update role - match wording of button and toast
5 participants