Skip to content

Comments

Fixed default message for index threshold includes both threshold values#60545

Merged
YulNaumenko merged 4 commits intoelastic:masterfrom
YulNaumenko:alerting-ui-not-used-threshold-values
Mar 18, 2020
Merged

Fixed default message for index threshold includes both threshold values#60545
YulNaumenko merged 4 commits intoelastic:masterfrom
YulNaumenko:alerting-ui-not-used-threshold-values

Conversation

@YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Mar 18, 2020 •
edited
Loading

Resolve #60044

Extended Threshold expression with proper validation and added handler for change comparator to set the correct number of threshold values required but its setting 'requiredValues'

@YulNaumenko YulNaumenko added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// labels Mar 18, 2020
@YulNaumenko YulNaumenko requested a review from a team as a code owner March 18, 2020 17:49
@YulNaumenko YulNaumenko self-assigned this Mar 18, 2020
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 18, 2020

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

pmuellr
pmuellr approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@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.

code LGTM but noted an issue with checking a threshold value of 0. Kind of a moot issue, since it appears I can't set a threshold value <= 0 at all - negative numbers should be possible to use here. Suggest we open a new issue for that.

Tried out the code, end-to-end with an index threshold alert and slack action, the {{context.message}} no longer prints two threshold values for the single threshold comparators in the slack message \o/.

mikecote
mikecote approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

LGTM!

...ins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/index.ts Outdated
thresholdComparator &&
builtInComparators[thresholdComparator].requiredValues > 1 &&
(!threshold ||
(threshold && threshold.length < builtInComparators[thresholdComparator!].requiredValues))
Copy link
Contributor

@mikecote mikecote Mar 18, 2020

Choose a reason for hiding this comment

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

nit:

Suggested change
(threshold && threshold.length < builtInComparators[thresholdComparator!].requiredValues))
threshold.length < builtInComparators[thresholdComparator!].requiredValues)

@YulNaumenko
Fixed validation errors with ability to clear input
7dd8cfd
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into alerting-ui-not-u…
549d70f 
…sed-threshold-values

# Conflicts:
#	x-pack/plugins/triggers_actions_ui/public/application/components/builtin_alert_types/threshold/index.ts
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 18, 2020

💚 Build Succeeded

History

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

@YulNaumenko YulNaumenko merged commit 3600f5b into elastic:master Mar 18, 2020
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Mar 18, 2020
@YulNaumenko
Fixed default message for index threshold includes both threshold val…
ba5e38f 
…ues (elastic#60545)

* Fixed default message for index threshold includes both threshold values even if not used

* fixed due to review comments

* Fixed validation errors with ability to clear input
@YulNaumenko YulNaumenko mentioned this pull request Mar 18, 2020
Merged
YulNaumenko added a commit that referenced this pull request Mar 19, 2020
@YulNaumenko
Fixed default message for index threshold includes both threshold val…
9cb2477 
…ues (#60545) (#60582)

* Fixed default message for index threshold includes both threshold values even if not used

* fixed due to review comments

* Fixed validation errors with ability to clear input
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 19, 2020
@gmmorris
Merge branch 'master' into alerting/empty-list
2ef7c4f 
* master: (35 commits)
  [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997)
  Introduce search interceptor (elastic#60523)
  [ML] Add functional tests for file data visualizer (elastic#60413)
  [APM] Optimize service map query (elastic#60412)
  [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces
  Enhancement/update esdocs datasource (elastic#59512)
  [junit] only include stdout in report for failures (elastic#60530)
  Update dependency nock to v12 (elastic#60422)
  upgrade execa to get stdout/stderr in error messages (elastic#60537)
  skip flaky suite (elastic#60471)
  [Ingest] Agent Config Details - Data sources list ui (elastic#60429)
  [SIEM] Create ML Rules (elastic#58053)
  skip flaky suite (elastic#60559)
  fix agent type (elastic#60554)
  Fixed default message for index threshold includes both threshold values (elastic#60545)
  [Ingest] Add support for `yaml` field types (elastic#60440)
  Solved the issue for a GROUP BY expression validation (elastic#60558)
  [Maps] Mark instance state as readonly (elastic#60557)
  Move ui/indices into es_ui_shared plugin. (elastic#60186)
  ServiceNow action improvements (elastic#60052)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 19, 2020
@gmmorris
Merge branch 'master' into alerting/api-constant
8e2d333 
* master: (64 commits)
  [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997)
  Introduce search interceptor (elastic#60523)
  [ML] Add functional tests for file data visualizer (elastic#60413)
  [APM] Optimize service map query (elastic#60412)
  [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces
  Enhancement/update esdocs datasource (elastic#59512)
  [junit] only include stdout in report for failures (elastic#60530)
  Update dependency nock to v12 (elastic#60422)
  upgrade execa to get stdout/stderr in error messages (elastic#60537)
  skip flaky suite (elastic#60471)
  [Ingest] Agent Config Details - Data sources list ui (elastic#60429)
  [SIEM] Create ML Rules (elastic#58053)
  skip flaky suite (elastic#60559)
  fix agent type (elastic#60554)
  Fixed default message for index threshold includes both threshold values (elastic#60545)
  [Ingest] Add support for `yaml` field types (elastic#60440)
  Solved the issue for a GROUP BY expression validation (elastic#60558)
  [Maps] Mark instance state as readonly (elastic#60557)
  Move ui/indices into es_ui_shared plugin. (elastic#60186)
  ServiceNow action improvements (elastic#60052)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 19, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into np-shim/cross…
e2f7a22 
…-cluster-replication

* 'master' of github.com:elastic/kibana: (89 commits)
  Sort by name when fetching alerts and connectors (elastic#60506)
  Make slack param validation handle empty messages (elastic#60468)
  [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997)
  Introduce search interceptor (elastic#60523)
  [ML] Add functional tests for file data visualizer (elastic#60413)
  [APM] Optimize service map query (elastic#60412)
  [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces
  Enhancement/update esdocs datasource (elastic#59512)
  [junit] only include stdout in report for failures (elastic#60530)
  Update dependency nock to v12 (elastic#60422)
  upgrade execa to get stdout/stderr in error messages (elastic#60537)
  skip flaky suite (elastic#60471)
  [Ingest] Agent Config Details - Data sources list ui (elastic#60429)
  [SIEM] Create ML Rules (elastic#58053)
  skip flaky suite (elastic#60559)
  fix agent type (elastic#60554)
  Fixed default message for index threshold includes both threshold values (elastic#60545)
  [Ingest] Add support for `yaml` field types (elastic#60440)
  Solved the issue for a GROUP BY expression validation (elastic#60558)
  [Maps] Mark instance state as readonly (elastic#60557)
  ...

# Conflicts:
#	x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/auto_follow_pattern_form.js
#	x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/follower_index_form/follower_index_form.js
#	x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/components/follower_index_form/follower_index_form.test.js
#	x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/services/auto_follow_pattern_validators.js
#	x-pack/legacy/plugins/cross_cluster_replication/public/np_ready/app/services/input_validation.js
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 19, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into refactor/cons…
3c1d020 
…ole/cleanup

* 'master' of github.com:elastic/kibana: (47 commits)
  [Remote clusters] Update copy (elastic#60382)
  Sort by name when fetching alerts and connectors (elastic#60506)
  Make slack param validation handle empty messages (elastic#60468)
  [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997)
  Introduce search interceptor (elastic#60523)
  [ML] Add functional tests for file data visualizer (elastic#60413)
  [APM] Optimize service map query (elastic#60412)
  [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces
  Enhancement/update esdocs datasource (elastic#59512)
  [junit] only include stdout in report for failures (elastic#60530)
  Update dependency nock to v12 (elastic#60422)
  upgrade execa to get stdout/stderr in error messages (elastic#60537)
  skip flaky suite (elastic#60471)
  [Ingest] Agent Config Details - Data sources list ui (elastic#60429)
  [SIEM] Create ML Rules (elastic#58053)
  skip flaky suite (elastic#60559)
  fix agent type (elastic#60554)
  Fixed default message for index threshold includes both threshold values (elastic#60545)
  [Ingest] Add support for `yaml` field types (elastic#60440)
  Solved the issue for a GROUP BY expression validation (elastic#60558)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 19, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
18dc4ee 
* master: (39 commits)
  [SIEM][CASE]  Configuration page action bar (elastic#60608)
  [Remote clusters] Update copy (elastic#60382)
  Sort by name when fetching alerts and connectors (elastic#60506)
  Make slack param validation handle empty messages (elastic#60468)
  [Alerting] Adds navigation by consumer and alert type to alerting (elastic#58997)
  Introduce search interceptor (elastic#60523)
  [ML] Add functional tests for file data visualizer (elastic#60413)
  [APM] Optimize service map query (elastic#60412)
  [SIEM][Detection Engine] Adds lists feature flag and list values to the REST interfaces
  Enhancement/update esdocs datasource (elastic#59512)
  [junit] only include stdout in report for failures (elastic#60530)
  Update dependency nock to v12 (elastic#60422)
  upgrade execa to get stdout/stderr in error messages (elastic#60537)
  skip flaky suite (elastic#60471)
  [Ingest] Agent Config Details - Data sources list ui (elastic#60429)
  [SIEM] Create ML Rules (elastic#58053)
  skip flaky suite (elastic#60559)
  fix agent type (elastic#60554)
  Fixed default message for index threshold includes both threshold values (elastic#60545)
  [Ingest] Add support for `yaml` field types (elastic#60440)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pmuellr pmuellr pmuellr approved these changes

@mikecote mikecote mikecote approved these changes

Assignees

@YulNaumenko YulNaumenko

Labels

Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v7.7.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Alerting] default message for index threshold includes both threshold values even if not used

5 participants

@YulNaumenko @elasticmachine @kibanamachine @pmuellr @mikecote