Allow registered alert types to be non-editable by YulNaumenko · Pull Request #65606 · elastic/kibana

YulNaumenko · 2020-05-07T05:48:29Z

Resolve #65549

In the scope of this PR:

Extended UI AlertTypeModel with the additional property: isEditable (boolean). isEditable property is responsible for displaying Edit button on alert details page.
Extended server API AlertType with producer string property, which correcpond to the application owner of the alert type.
Changed Create Alert flyout behavior to display alert type select options based on the equality of the alert consumer and alert type producer. If the alert consumer is alerting Create Alert flyout display all alert types (not depends on the producer value) with isEditable setting true. If alert consumer is some other plugin(application), Create Alert flyout display only alert types with the same producer value.
This changes should handle mixing up alert types between plugins and hide alert types which is not ready to be created/edited from Alert Management UI.

elasticmachine · 2020-05-07T05:48:31Z

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

elasticmachine · 2020-05-07T05:48:33Z

Pinging @elastic/apm-ui (Team:apm)

elasticmachine · 2020-05-07T05:48:35Z

Pinging @elastic/uptime (Team:uptime)

x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts

dgieselaar

LGTM @YulNaumenko! Thank you for getting a fix ready so quick 😀

...s/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.tsx

examples/alerting_example/public/alert_types/always_firing.tsx

…le-edit # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

justinkambic

LGTM for Uptime.

For summary, I reviewed the Uptime code changes and when running this patch locally, I created two Uptime alerts (TLS and monitor status) and attempted to edit them, the UI didn't allow for it. 👍

…mer can display other consumers alert types, but in case if it isEditable

…le-edit

phillipb

Metrics changes look good.

rylnd

SIEM changes look good; I'm still able to CRUD Rule Alerts from within Detections.

I was seeing some errors when viewing Detections Rule Alerts in the Alerting UI:

It was unclear to me whether this was related to this branch, some weird local state of mine, or neither. Regardless, I was able to enable/disable the alert from the Alerting UI, albeit with multiple of those toaster errors.

YulNaumenko · 2020-05-11T18:43:35Z

SIEM changes look good; I'm still able to CRUD Rule Alerts from within Detections.

I was seeing some errors when viewing Detections Rule Alerts in the Alerting UI:

It was unclear to me whether this was related to this branch, some weird local state of mine, or neither. Regardless, I was able to enable/disable the alert from the Alerting UI, albeit with multiple of those toaster errors.

@rylnd , this error message come up because siem alert types is not registered in the alerting UI registry. @mikecote are we planning to show alert details for alert types which doesn't have an alerting UI registry?

mikecote · 2020-05-11T18:45:45Z

@mikecote are we planning to show alert details for alert types which doesn't have an alerting UI registry?

I would say yes unless there's some technical challenges that prevent us from supporting this today.

mikecote

LGTM, just a few suggestions and a comment about isEditable based on our conversation.

x-pack/plugins/alerting/README.md

x-pack/plugins/triggers_actions_ui/public/types.ts

mikecote · 2020-05-12T12:35:47Z

...ins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/index.ts

    iconClass: 'alert',
    alertParamsExpression: IndexThresholdAlertTypeExpression,
    validate: validateExpression,
+    requiresAppContext: true,


I think all the values of requiresAppContext need to be inverted now right? Since index threshold alert doesn't require app context so can be created / edited from management UIs.

…le-edit # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts

kibanamachine · 2020-05-12T20:24:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5c442bf

History

💔 Build #47369 failed aa5cec7
💚 Build #47166 succeeded 09a2911
💔 Build #47146 failed 595eb69
💔 Build #47061 failed 90d7713
💚 Build #46993 succeeded dd2f0e2

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

* Allow registered alert types to be non-editable * Fixed isUiEditEnabled values * Fixed due to comments * fixed failing tests * Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable * fixed tests * Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic * fixed type check * Fixed tests and type checks * Removed error message for non registered plugins * Fixed failing tests * Fixed due to comments * fixed test * - * revert logic for requiresAppContext * Added close toast after saving alert # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts

* Allow registered alert types to be non-editable * Fixed isUiEditEnabled values * Fixed due to comments * fixed failing tests * Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable * fixed tests * Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic * fixed type check * Fixed tests and type checks * Removed error message for non registered plugins * Fixed failing tests * Fixed due to comments * fixed test * - * revert logic for requiresAppContext * Added close toast after saving alert # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts # x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts

* Allow registered alert types to be non-editable * Fixed isUiEditEnabled values * Fixed due to comments * fixed failing tests * Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable * fixed tests * Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic * fixed type check * Fixed tests and type checks * Removed error message for non registered plugins * Fixed failing tests * Fixed due to comments * fixed test * - * revert logic for requiresAppContext * Added close toast after saving alert

* Allow registered alert types to be non-editable * Fixed isUiEditEnabled values * Fixed due to comments * fixed failing tests * Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable * fixed tests * Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic * fixed type check * Fixed tests and type checks * Removed error message for non registered plugins * Fixed failing tests * Fixed due to comments * fixed test * - * revert logic for requiresAppContext * Added close toast after saving alert # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts # x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts

* Allow registered alert types to be non-editable (#65606) * Allow registered alert types to be non-editable * Fixed isUiEditEnabled values * Fixed due to comments * fixed failing tests * Enable alert type selection per alert consumer, only 'alerting' consumer can display other consumers alert types, but in case if it isEditable * fixed tests * Removed consumer property from the client side alert type registry and added server side property producer which purpose is to manage a feature logic * fixed type check * Fixed tests and type checks * Removed error message for non registered plugins * Fixed failing tests * Fixed due to comments * fixed test * - * revert logic for requiresAppContext * Added close toast after saving alert # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts # x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/plugin.ts * Fixed merge issue

Allow registered alert types to be non-editable

6839f41

YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.8.0 labels May 7, 2020

YulNaumenko requested a review from a team May 7, 2020 05:48

YulNaumenko requested review from a team as code owners May 7, 2020 05:48

YulNaumenko self-assigned this May 7, 2020

botelastic bot added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels May 7, 2020

Fixed isUiEditEnabled values

66b1350

Kerry350 reviewed May 7, 2020

View reviewed changes

x-pack/plugins/infra/public/components/alerting/logs/log_threshold_alert_type.ts Outdated Show resolved Hide resolved

dgieselaar approved these changes May 7, 2020

View reviewed changes

dgieselaar reviewed May 7, 2020

View reviewed changes

...s/triggers_actions_ui/public/application/sections/alert_details/components/alert_details.tsx Outdated Show resolved Hide resolved

sorenlouv reviewed May 7, 2020

View reviewed changes

examples/alerting_example/public/alert_types/always_firing.tsx Outdated Show resolved Hide resolved

YulNaumenko added 2 commits May 7, 2020 09:25

Fixed due to comments

b611ee0

YulNaumenko requested review from Kerry350 and sorenlouv May 7, 2020 17:14

fixed failing tests

9062250

justinkambic approved these changes May 7, 2020

View reviewed changes

Enable alert type selection per alert consumer, only 'alerting' consu…

ca0b7ef

…mer can display other consumers alert types, but in case if it isEditable

YulNaumenko requested a review from dgieselaar May 7, 2020 22:03

Merge remote-tracking branch 'upstream/master' into alerting-ui-disab…

de505a1

…le-edit

phillipb approved these changes May 11, 2020

View reviewed changes

rylnd approved these changes May 11, 2020

View reviewed changes

Removed error message for non registered plugins

90d7713

mikecote approved these changes May 11, 2020

View reviewed changes

x-pack/plugins/alerting/README.md Outdated Show resolved Hide resolved

x-pack/plugins/triggers_actions_ui/public/types.ts Outdated Show resolved Hide resolved

YulNaumenko added 4 commits May 11, 2020 15:42

Fixed failing tests

a1efef8

Fixed due to comments

595eb69

fixed test

2446c96

-

09a2911

mikecote reviewed May 12, 2020

View reviewed changes

YulNaumenko added 3 commits May 12, 2020 09:09

revert logic for requiresAppContext

506505c

Merge remote-tracking branch 'upstream/master' into alerting-ui-disab…

aa5cec7

…le-edit # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts

Added close toast after saving alert

5c442bf

YulNaumenko merged commit 5ed5fda into elastic:master May 12, 2020

YulNaumenko mentioned this pull request May 12, 2020

[7.x] Allow registered alert types to be non-editable (#65606) #66318

Closed

This was referenced May 12, 2020

[7.8] Allow registered alert types to be non-editable (#65606) #66319

Closed

Ability to show/hide the option of creating certain AlertTypes based on their display context #63885

Closed

sorenlouv added apm:test-plan-7.8.0 and removed apm:test-plan-7.8.0 labels May 13, 2020

YulNaumenko mentioned this pull request May 13, 2020

[7.x] Allow registered alert types to be non-editable (#65606) #66416

Merged

YulNaumenko mentioned this pull request May 13, 2020

[7.8] Allow registered alert types to be non-editable (#65606) #66499

Merged

spalger mentioned this pull request May 15, 2020

[CI] Replace agent metrics link with the new one #66632

Merged

Conversation

YulNaumenko commented May 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented May 7, 2020

Uh oh!

elasticmachine commented May 7, 2020

Uh oh!

elasticmachine commented May 7, 2020

Uh oh!

Uh oh!

dgieselaar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

justinkambic left a comment

Choose a reason for hiding this comment

Uh oh!

phillipb left a comment

Choose a reason for hiding this comment

Uh oh!

rylnd left a comment

Choose a reason for hiding this comment

Uh oh!

YulNaumenko commented May 11, 2020

Uh oh!

mikecote commented May 11, 2020

Uh oh!

mikecote left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikecote May 12, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented May 12, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

YulNaumenko commented May 7, 2020 •

edited

Loading