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

[Alerting] License Errors on Alert List View #89920

Merged
merged 28 commits into from
Feb 10, 2021

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Feb 1, 2021

Resolves #85640

Summary

Made the following updates to the alerts list:

  1. Added a tooltip to the alert status (when the status is error) showing the reason message from the alert execution status.
  2. When the error is a license error, updated the status to say License Error
  3. When the error is a license error, added a button link "Fix" that opens a modals asking if user wants to upgrade their license. Clicking "Manage License" opens the license page
  4. At @mdefazio's suggestion, switched the Name and Status columns on the table
Alert with license error Screen Shot 2021-02-01 at 9 31 04 PM
Error reason tooltip Screen Shot 2021-02-01 at 9 40 31 PM
Manage license modal Screen Shot 2021-02-01 at 9 31 12 PM

Checklist

Delete any items that are not applicable to this PR.

@ymao1 ymao1 self-assigned this Feb 1, 2021
@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.12.0 v8.0.0 labels Feb 2, 2021
@ymao1 ymao1 marked this pull request as ready for review February 2, 2021 13:57
@ymao1 ymao1 requested a review from a team as a code owner February 2, 2021 13:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@ymao1 ymao1 requested review from gchaps and mdefazio February 2, 2021 13:58
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

Awesome UX now! Code LGTM.

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

This looks good! Super tiny nit regarding the button parenthesis (but Gail might disagree).
Also, I think it might be worthwhile tweaking the column widths so there's more room for the alert title. Setting the tableLayout to auto might be enough, but I also remember us going through a PR for this previously.

Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

@arisonl Can you please review the licensing text in this PR? There are buttons for "Upgrade license" and "Manage license".

@gchaps
Copy link
Contributor

gchaps commented Feb 3, 2021

@arisonl Could you please advise on the licensing text in the three screenshots shown in the summary?

@arisonl
Copy link
Contributor

arisonl commented Feb 4, 2021

@gchaps thank you for the heads up, this looks good to me

@ymao1
Copy link
Contributor Author

ymao1 commented Feb 8, 2021

@elasticmachine merge upstream

@ryankeairns
Copy link
Contributor

In chatting with @gchaps , we felt the modal copy could use some clarification as there is somewhat of a mismatch currently (i.e. contact adminstrator / Manage license).

Since we can't very easily determine what role a person has - or what they have the ability to do - perhaps the text ought to be more general and we leave the available options up to the License Management page.

The modal title and text would be:

Gold license required

Alert example.people-in-space is disabled because it requires a Gold license. Continue to the License Management page to view upgrade options.

[ Cancel ] [ Manage license ]

@gchaps
Copy link
Contributor

gchaps commented Feb 9, 2021

A few minor tweaks.

Tooltip:

Alert example.people-in-space is disabled because it requires a Gold license. Go to License Management to view upgrade options.

Modal:

Gold license required

Alert example.people-in-space is disabled because it requires a Gold license. Continue to License Management to view upgrade options.

[ Cancel ] [ Manage license ]

@ymao1
Copy link
Contributor Author

ymao1 commented Feb 9, 2021

A few minor tweaks.

Tooltip:

Alert example.people-in-space is disabled because it requires a Gold license. Go to License Management to view upgrade options.

@gchaps Because I have the tooltip showing on any alert execution error and not just license errors, I'm just showing the error from the execution status with no modifications. The current wording is also what shows up in the logs. Would it make sense to update the log message to say "Go to License Management to view upgrade options" as well?

@ymao1
Copy link
Contributor Author

ymao1 commented Feb 9, 2021

@gchaps @ryankeairns Updated modal:
Screen Shot 2021-02-09 at 11 53 53 AM

@ymao1
Copy link
Contributor Author

ymao1 commented Feb 9, 2021

@elasticmachine merge upstream

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM, seems to work as advertised. Made a few comments/notes/questions, no blockers.

@ymao1
Copy link
Contributor Author

ymao1 commented Feb 10, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 337 338 +1

Async chunks

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

id before after diff
triggersActionsUi 1.4MB 1.4MB +2.4KB

History

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

@ymao1 ymao1 merged commit 3e91bc7 into elastic:master Feb 10, 2021
ymao1 added a commit to ymao1/kibana that referenced this pull request Feb 10, 2021
* Adding tooltips to alert list and modal for license upgrade

* Fixing typings

* Custom License Error status. Moving modal to alerts list page

* Adding unit test

* Cleanup

* Unit tests

* Removing tooltip from alert name

* License

* PR fixes

* Updating modal wording

* Updating license state error message

* i18n fix

* Fixing functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 10, 2021
* master: (99 commits)
  [Fleet] Use Fleet Server indices in the search bar (elastic#90835)
  [Search Sessions] added an info flyout to session management (elastic#90559)
  [ILM] Revisit searchable snapshot field after new redesign (elastic#90793)
  [Alerting] License Errors on Alert List View (elastic#89920)
  RFC Improve saved object migrations algorithm (elastic#84333)
  [Lens] (Accessibility) Fix focus on drag and drop actions (elastic#90561)
  Use new shortcut links to Fleet discuss forums. (elastic#90786)
  Do not generate an ephemeral encryption key in production. (elastic#81511)
  [Fleet] Use staging registry for snapshot builds (elastic#90327)
  Actually deleting x-pack/tsconfig.refs.json (elastic#90898)
  Add deprecation warning to all Beats CM pages. (elastic#90741)
  skip flaky suite (elastic#90136)
  Revert "Revert "[Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244)"" (elastic#90889)
  remove ref to removed tsconfig file
  [core.logging] Uses host timezone as default (elastic#90368)
  [Maps] remove maps_file_upload plugin and fold public folder into file_upload plugin (elastic#90292)
  Revert "[Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244)"
  [dev-utils/ci-stats] support disabling ship errors (elastic#90851)
  Prefix with / (elastic#90836)
  [Metrics UI] Add Metrics Anomaly Alert Type (elastic#89244)
  ...
ymao1 added a commit that referenced this pull request Feb 10, 2021
* Adding tooltips to alert list and modal for license upgrade

* Fixing typings

* Custom License Error status. Moving modal to alerts list page

* Adding unit test

* Cleanup

* Unit tests

* Removing tooltip from alert name

* License

* PR fixes

* Updating modal wording

* Updating license state error message

* i18n fix

* Fixing functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@ymao1 ymao1 deleted the alerting/licensing-ux branch March 25, 2021 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alerting UI] Add licensing related enhancements of existing UI.
9 participants