From c272a1b7dd732c3b4deb300403600506ea1e5f44 Mon Sep 17 00:00:00 2001 From: Pete Harverson Date: Thu, 13 Sep 2018 15:54:11 +0100 Subject: [PATCH] [ML] Add links to rule editor for quick edit of value or filter (#22990) * [ML] Add links to rule editor for quick edit of value or filter * [ML] Updates to rule editor quick links following review --- .../rule_editor_flyout.test.js.snap | 30 ++- .../components/rule_editor/__tests__/utils.js | 134 ++++++++++ .../detector_description_list.test.js.snap | 34 ++- .../detector_description_list.js | 25 +- .../detector_description_list.test.js | 43 +++- .../rule_editor/rule_editor_flyout.js | 52 +++- .../add_to_filter_list_link.test.js.snap | 14 ++ .../edit_condition_link.test.js.snap | 160 ++++++++++++ .../rule_action_panel.test.js.snap | 126 ++++++---- .../add_to_filter_list_link.js | 38 +++ .../add_to_filter_list_link.test.js | 33 +++ .../select_rule_action/edit_condition_link.js | 106 ++++++++ .../edit_condition_link.test.js | 68 +++++ .../select_rule_action/rule_action_panel.js | 234 ++++++++++++++---- .../rule_action_panel.test.js | 97 +++++--- .../select_rule_action/select_rule_action.js | 13 +- .../components/rule_editor/styles/main.less | 21 +- .../ml/public/components/rule_editor/utils.js | 54 +++- .../ml/public/explorer/explorer_controller.js | 5 +- .../public/services/ml_api_service/filters.js | 17 +- .../ml/server/models/filter/filter_manager.js | 17 +- 21 files changed, 1159 insertions(+), 162 deletions(-) create mode 100644 x-pack/plugins/ml/public/components/rule_editor/__tests__/utils.js create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/add_to_filter_list_link.test.js.snap create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/edit_condition_link.test.js.snap create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.js create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.test.js create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.js create mode 100644 x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.test.js diff --git a/x-pack/plugins/ml/public/components/rule_editor/__snapshots__/rule_editor_flyout.test.js.snap b/x-pack/plugins/ml/public/components/rule_editor/__snapshots__/rule_editor_flyout.test.js.snap index bb7b79c4a3de..8fea0d1cf247 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/__snapshots__/rule_editor_flyout.test.js.snap +++ b/x-pack/plugins/ml/public/components/rule_editor/__snapshots__/rule_editor_flyout.test.js.snap @@ -31,6 +31,15 @@ exports[`RuleEditorFlyout renders the flyout after adding a condition to a rule diff --git a/x-pack/plugins/ml/public/components/rule_editor/__tests__/utils.js b/x-pack/plugins/ml/public/components/rule_editor/__tests__/utils.js new file mode 100644 index 000000000000..bde9ebe4df3c --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/__tests__/utils.js @@ -0,0 +1,134 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + +import expect from 'expect.js'; +import { + isValidRule, + buildRuleDescription, + getAppliesToValueFromAnomaly, +} from '../utils'; +import { + ACTION, + APPLIES_TO, + OPERATOR, + FILTER_TYPE, +} from '../../../../common/constants/detector_rule'; + +describe('ML - rule editor utils', () => { + + const ruleWithCondition = { + actions: [ACTION.SKIP_RESULT], + conditions: [ + { + applies_to: APPLIES_TO.ACTUAL, + operator: OPERATOR.GREATER_THAN, + value: 10 + } + ] + }; + + const ruleWithScope = { + actions: [ACTION.SKIP_RESULT], + scope: { + instance: { + filter_id: 'test_aws_instances', + filter_type: FILTER_TYPE.INCLUDE, + enabled: true + } + } + }; + + const ruleWithConditionAndScope = { + actions: [ACTION.SKIP_RESULT], + conditions: [ + { + applies_to: APPLIES_TO.TYPICAL, + operator: OPERATOR.LESS_THAN, + value: 100 + } + ], + scope: { + instance: { + filter_id: 'test_aws_instances', + filter_type: FILTER_TYPE.EXCLUDE, + enabled: true + } + } + }; + + describe('isValidRule', () => { + + it('returns true for a rule with an action and a condition', () => { + expect(isValidRule(ruleWithCondition)).to.be(true); + }); + + it('returns true for a rule with an action and scope', () => { + expect(isValidRule(ruleWithScope)).to.be(true); + }); + + it('returns true for a rule with an action, scope and condition', () => { + expect(isValidRule(ruleWithConditionAndScope)).to.be(true); + }); + + it('returns false for a rule with no action', () => { + const ruleWithNoAction = { + actions: [], + conditions: [ + { + applies_to: APPLIES_TO.TYPICAL, + operator: OPERATOR.LESS_THAN, + value: 100 + } + ], + }; + + expect(isValidRule(ruleWithNoAction)).to.be(false); + }); + + it('returns false for a rule with no scope or conditions', () => { + const ruleWithNoScopeOrCondition = { + actions: [ACTION.SKIP_RESULT], + }; + + expect(isValidRule(ruleWithNoScopeOrCondition)).to.be(false); + }); + + }); + + describe('buildRuleDescription', () => { + + it('returns expected rule descriptions', () => { + expect(buildRuleDescription(ruleWithCondition)).to.be( + 'skip result when actual is greater than 10'); + expect(buildRuleDescription(ruleWithScope)).to.be( + 'skip result when instance is in test_aws_instances'); + expect(buildRuleDescription(ruleWithConditionAndScope)).to.be( + 'skip result when typical is less than 100 AND instance is not in test_aws_instances'); + }); + }); + + describe('getAppliesToValueFromAnomaly', () => { + + const anomaly = { + actual: [210], + typical: [1.23], + }; + + it('returns expected actual value from an anomaly', () => { + expect(getAppliesToValueFromAnomaly(anomaly, APPLIES_TO.ACTUAL)).to.be(210); + }); + + it('returns expected typical value from an anomaly', () => { + expect(getAppliesToValueFromAnomaly(anomaly, APPLIES_TO.TYPICAL)).to.be(1.23); + }); + + it('returns expected diff from typical value from an anomaly', () => { + expect(getAppliesToValueFromAnomaly(anomaly, APPLIES_TO.DIFF_FROM_TYPICAL)).to.be(208.77); + }); + }); + +}); diff --git a/x-pack/plugins/ml/public/components/rule_editor/components/detector_description_list/__snapshots__/detector_description_list.test.js.snap b/x-pack/plugins/ml/public/components/rule_editor/components/detector_description_list/__snapshots__/detector_description_list.test.js.snap index 6a64df207466..69d936a56796 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/components/detector_description_list/__snapshots__/detector_description_list.test.js.snap +++ b/x-pack/plugins/ml/public/components/rule_editor/components/detector_description_list/__snapshots__/detector_description_list.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`DetectorDescriptionList render for farequote detector 1`] = ` +exports[`DetectorDescriptionList render for detector with anomaly values 1`] = ` +`; + +exports[`DetectorDescriptionList render for population detector with no anomaly values 1`] = ` + { - test('render for farequote detector', () => { + test('render for detector with anomaly values', () => { const props = { job: { - job_id: 'farequote' + job_id: 'responsetimes' }, detector: { detector_description: 'mean response time' - } + }, + anomaly: { + actual: [50], + typical: [1.23], + source: { function: 'mean' }, + }, + }; + + const component = shallow( + + ); + + expect(component).toMatchSnapshot(); + + }); + + test('render for population detector with no anomaly values', () => { + + const props = { + job: { + job_id: 'population' + }, + detector: { + detector_description: 'count by status over clientip' + }, + anomaly: { + source: { function: 'count' }, + causes: [ + { + actual: [50], + typical: [1.01] + }, + { + actual: [60], + typical: [1.2] + }, + ], + }, }; const component = shallow( diff --git a/x-pack/plugins/ml/public/components/rule_editor/rule_editor_flyout.js b/x-pack/plugins/ml/public/components/rule_editor/rule_editor_flyout.js index 8cc57db20d24..f58a18b6e95b 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/rule_editor_flyout.js +++ b/x-pack/plugins/ml/public/components/rule_editor/rule_editor_flyout.js @@ -43,7 +43,8 @@ import { getNewConditionDefaults, isValidRule, saveJobRule, - deleteJobRule + deleteJobRule, + addItemToFilter, } from './utils'; import { ACTION, CONDITIONS_NOT_SUPPORTED_FUNCTIONS } from '../../../common/constants/detector_rule'; @@ -130,7 +131,7 @@ export class RuleEditorFlyout extends Component { }); if (this.partitioningFieldNames.length > 0 && this.canGetFilters) { - // Load the current list of filters. + // Load the current list of filters. These are used for configuring rule scope. ml.filters.filters() .then((filters) => { const filterListIds = filters.map(filter => filter.filter_id); @@ -305,19 +306,33 @@ export class RuleEditorFlyout extends Component { saveEdit = () => { const { - job, - anomaly, rule, ruleIndex } = this.state; + this.updateRuleAtIndex(ruleIndex, rule); + } + + updateRuleAtIndex = (ruleIndex, editedRule) => { + const { + job, + anomaly, + } = this.state; + const jobId = job.job_id; const detectorIndex = anomaly.detectorIndex; - saveJobRule(job, detectorIndex, ruleIndex, rule) + saveJobRule(job, detectorIndex, ruleIndex, editedRule) .then((resp) => { if (resp.success) { - toastNotifications.addSuccess(`Changes to ${jobId} detector rules saved`); + toastNotifications.add( + { + title: `Changes to ${jobId} detector rules saved`, + color: 'success', + iconType: 'check', + text: 'Note that changes will take effect for new results only.' + } + ); this.closeFlyout(); } else { toastNotifications.addDanger(`Error saving changes to ${jobId} detector rules`); @@ -356,6 +371,27 @@ export class RuleEditorFlyout extends Component { }); } + addItemToFilterList = (item, filterId, closeFlyoutOnAdd) => { + addItemToFilter(item, filterId) + .then(() => { + if (closeFlyoutOnAdd === true) { + toastNotifications.add( + { + title: `Added ${item} to ${filterId}`, + color: 'success', + iconType: 'check', + text: 'Note that changes will take effect for new results only.' + } + ); + this.closeFlyout(); + } + }) + .catch((error) => { + console.log(`Error adding ${item} to filter ${filterId}:`, error); + toastNotifications.addDanger(`An error occurred adding ${item} to filter ${filterId}`); + }); + } + render() { const { isFlyoutVisible, @@ -392,9 +428,10 @@ export class RuleEditorFlyout extends Component { @@ -442,6 +479,7 @@ export class RuleEditorFlyout extends Component { diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/add_to_filter_list_link.test.js.snap b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/add_to_filter_list_link.test.js.snap new file mode 100644 index 000000000000..83f7b8976df2 --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/add_to_filter_list_link.test.js.snap @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`AddToFilterListLink renders the add to filter list link for a value 1`] = ` + + Add + elastic.co + to + safe_domains + +`; diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/edit_condition_link.test.js.snap b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/edit_condition_link.test.js.snap new file mode 100644 index 000000000000..1347debe0a1f --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/edit_condition_link.test.js.snap @@ -0,0 +1,160 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`EditConditionLink renders for a condition using actual 1`] = ` + + + + Update rule condition from + 5 + to + + + + + + + + Update + + + +`; + +exports[`EditConditionLink renders for a condition using diff from typical 1`] = ` + + + + Update rule condition from + 5 + to + + + + + + + + Update + + + +`; + +exports[`EditConditionLink renders for a condition using typical 1`] = ` + + + + Update rule condition from + 5 + to + + + + + + + + Update + + + +`; diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/rule_action_panel.test.js.snap b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/rule_action_panel.test.js.snap index e17c666053d1..2dfdae85be42 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/rule_action_panel.test.js.snap +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/__snapshots__/rule_action_panel.test.js.snap @@ -16,6 +16,32 @@ exports[`RuleActionPanel renders panel for rule with a condition 1`] = ` "description": "skip result when actual is less than 1", "title": "rule", }, + Object { + "description": , + "title": "actions", + }, Object { "description": Edit rule , - "title": "actions", + "title": "", }, Object { "description": , "title": "", @@ -41,7 +67,7 @@ exports[`RuleActionPanel renders panel for rule with a condition 1`] = ` `; -exports[`RuleActionPanel renders panel for rule with a condition and scope 1`] = ` +exports[`RuleActionPanel renders panel for rule with a condition and scope, value not in filter list 1`] = ` , + "title": "actions", + }, Object { "description": Edit rule , - "title": "actions", - }, - Object { - "description": , "title": "", }, - ] - } - textStyle="normal" - type="column" - /> - -`; - -exports[`RuleActionPanel renders panel for rule with scope 1`] = ` - - - Edit rule - , - "title": "actions", - }, Object { "description": , + "title": "", + }, + ] + } + textStyle="normal" + type="column" + /> + +`; + +exports[`RuleActionPanel renders panel for rule with scope, value in filter list 1`] = ` + + + Edit rule + , + "title": "actions", + }, + Object { + "description": , "title": "", diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.js new file mode 100644 index 000000000000..a6e3116157da --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.js @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + +/* + * React component for quick addition of a partitioning field value + * to a filter list used in the scope part of a rule. + */ + +import PropTypes from 'prop-types'; +import React from 'react'; + +import { + EuiLink, +} from '@elastic/eui'; + +export function AddToFilterListLink({ + fieldValue, + filterId, + addItemToFilterList, +}) { + + return ( + addItemToFilterList(fieldValue, filterId, true)} + > + Add {fieldValue} to {filterId} + + ); +} +AddToFilterListLink.propTypes = { + fieldValue: PropTypes.string.isRequired, + filterId: PropTypes.string.isRequired, + addItemToFilterList: PropTypes.func.isRequired, +}; diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.test.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.test.js new file mode 100644 index 000000000000..096294826ee9 --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/add_to_filter_list_link.test.js @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + +import { shallow } from 'enzyme'; +import React from 'react'; + +import { AddToFilterListLink } from './add_to_filter_list_link'; + +describe('AddToFilterListLink', () => { + + test(`renders the add to filter list link for a value`, () => { + const addItemToFilterList = jest.fn(() => {}); + + const wrapper = shallow( + + ); + + expect(wrapper).toMatchSnapshot(); + + wrapper.find('EuiLink').simulate('click'); + wrapper.update(); + expect(addItemToFilterList).toHaveBeenCalled(); + }); + +}); diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.js new file mode 100644 index 000000000000..d99674177e0f --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.js @@ -0,0 +1,106 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + +/* + * React component for quick edit of the numeric condition part of a rule, + * containing a number field input for editing the condition value. + */ + +import PropTypes from 'prop-types'; +import React, { + Component, +} from 'react'; + +import { + EuiFieldNumber, + EuiFlexGroup, + EuiFlexItem, + EuiLink, + EuiText, +} from '@elastic/eui'; + +import { APPLIES_TO } from '../../../../common/constants/detector_rule'; +import { formatValue } from '../../../formatters/format_value'; +import { + getAppliesToValueFromAnomaly, +} from '../utils'; + +export class EditConditionLink extends Component { + constructor(props) { + super(props); + + // Initialize value to anomaly value, if it exists. + // Do rounding at this initialization stage. Then if the user + // really wants to define to higher precision they can. + // Format based on magnitude of value at this stage, rather than using the + // Kibana field formatter (if set) which would add complexity converting + // the entered value to / from e.g. bytes. + let value = ''; + const anomaly = this.props.anomaly; + const anomalyValue = getAppliesToValueFromAnomaly(anomaly, props.appliesTo); + if (anomalyValue !== undefined) { + value = +formatValue(anomalyValue, anomaly.source.function); + } + + this.state = { value }; + } + + onChangeValue = (event) => { + const enteredValue = event.target.value; + this.setState({ + value: (enteredValue !== '') ? +enteredValue : '', + }); + } + + onUpdateClick = () => { + const { conditionIndex, updateConditionValue } = this.props; + updateConditionValue(conditionIndex, this.state.value); + } + + render() { + const value = this.state.value; + return ( + + + + Update rule condition from {this.props.conditionValue} to + + + + + + {value !== '' && + + this.onUpdateClick()} + > + Update + + + } + + ); + } +} +EditConditionLink.propTypes = { + conditionIndex: PropTypes.number.isRequired, + conditionValue: PropTypes.number.isRequired, + appliesTo: PropTypes.oneOf([ + APPLIES_TO.ACTUAL, + APPLIES_TO.TYPICAL, + APPLIES_TO.DIFF_FROM_TYPICAL + ]), + anomaly: PropTypes.object.isRequired, + updateConditionValue: PropTypes.func.isRequired, +}; diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.test.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.test.js new file mode 100644 index 000000000000..bd720bd0ef49 --- /dev/null +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/edit_condition_link.test.js @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +jest.mock('../../../services/job_service.js', () => 'mlJobService'); + +import { shallow } from 'enzyme'; +import React from 'react'; + +import { EditConditionLink } from './edit_condition_link'; +import { APPLIES_TO } from '../../../../common/constants/detector_rule'; + +function prepareTest(updateConditionValueFn, appliesTo) { + + const anomaly = { + actual: [210], + typical: [1.23], + detectorIndex: 0, + source: { + function: 'mean', + airline: ['AAL'], + }, + }; + + const props = { + conditionIndex: 0, + conditionValue: 5, + appliesTo, + anomaly, + updateConditionValue: updateConditionValueFn, + }; + + const wrapper = shallow( + + ); + + return wrapper; +} + +describe('EditConditionLink', () => { + + const updateConditionValue = jest.fn(() => {}); + + test(`renders for a condition using actual`, () => { + const wrapper = prepareTest(updateConditionValue, APPLIES_TO.ACTUAL); + expect(wrapper).toMatchSnapshot(); + }); + + test(`renders for a condition using typical`, () => { + const wrapper = prepareTest(updateConditionValue, APPLIES_TO.TYPICAL); + expect(wrapper).toMatchSnapshot(); + }); + + test(`renders for a condition using diff from typical`, () => { + const wrapper = prepareTest(updateConditionValue, APPLIES_TO.DIFF_FROM_TYPICAL); + expect(wrapper).toMatchSnapshot(); + }); + + test('calls updateConditionValue on clicking update link', () => { + const wrapper = prepareTest(updateConditionValue, APPLIES_TO.ACTUAL); + const instance = wrapper.instance(); + instance.onUpdateClick(); + wrapper.update(); + expect(updateConditionValue).toHaveBeenCalledWith(0, 210); + }); +}); diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.js index f73f06d42174..40c43b725ca7 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.js +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.js @@ -10,7 +10,9 @@ */ import PropTypes from 'prop-types'; -import React from 'react'; +import React, { + Component, +} from 'react'; import { EuiDescriptionList, @@ -18,72 +20,202 @@ import { EuiPanel, } from '@elastic/eui'; +import { cloneDeep } from 'lodash'; + +import { AddToFilterListLink } from './add_to_filter_list_link'; import { DeleteRuleModal } from './delete_rule_modal'; +import { EditConditionLink } from './edit_condition_link'; import { buildRuleDescription } from '../utils'; +import { ml } from '../../../services/ml_api_service'; -function getEditRuleLink(ruleIndex, setEditRuleIndex) { - return ( - setEditRuleIndex(ruleIndex)} - > - Edit rule - - ); -} -function getDeleteRuleLink(ruleIndex, deleteRuleAtIndex) { - return ( - - ); -} +export class RuleActionPanel extends Component { -export function RuleActionPanel({ - job, - detectorIndex, - ruleIndex, - setEditRuleIndex, - deleteRuleAtIndex, -}) { - const detector = job.analysis_config.detectors[detectorIndex]; - const rules = detector.custom_rules; - if (rules === undefined || ruleIndex >= rules.length) { - return null; + constructor(props) { + super(props); + + const { + job, + anomaly, + ruleIndex } = this.props; + + const detector = job.analysis_config.detectors[anomaly.detectorIndex]; + const rules = detector.custom_rules; + if (rules !== undefined && ruleIndex < rules.length) { + this.rule = rules[ruleIndex]; + } + + this.state = { + showAddToFilterListLink: false, + }; } - const rule = rules[ruleIndex]; + componentDidMount() { + // If the rule has a scope section with a single partitioning field key, + // load the filter list to check whether to add a link to add the + // anomaly partitioning field value to the filter list. + const scope = this.rule.scope; + if (scope !== undefined && Object.keys(scope).length === 1) { + const partitionFieldName = Object.keys(scope)[0]; + const partitionFieldValue = this.props.anomaly.source[partitionFieldName]; + + if (scope[partitionFieldName] !== undefined && + partitionFieldValue !== undefined && + partitionFieldValue.length === 1 && + partitionFieldValue[0].length > 0) { + + const filterId = scope[partitionFieldName].filter_id; + ml.filters.filters({ filterId }) + .then((filter) => { + const filterItems = filter.items; + if (filterItems.indexOf(partitionFieldValue[0]) === -1) { + this.setState({ showAddToFilterListLink: true }); + } + }) + .catch((resp) => { + console.log(`Error loading filter ${filterId}:`, resp); + }); + } - const descriptionListItems = [ - { - title: 'rule', - description: buildRuleDescription(rule), - }, - { - title: 'actions', - description: getEditRuleLink(ruleIndex, setEditRuleIndex), - }, - { - title: '', - description: getDeleteRuleLink(ruleIndex, deleteRuleAtIndex) } - ]; + } - return ( - - { + const { ruleIndex, setEditRuleIndex } = this.props; + return ( + setEditRuleIndex(ruleIndex)} + > + Edit rule + + ); + } + + getDeleteRuleLink = () => { + const { ruleIndex, deleteRuleAtIndex } = this.props; + return ( + - - ); + ); + } + + getQuickEditConditionLink = () => { + // Returns the link to adjust the numeric value of a condition + // if the rule has a single numeric condition. + const conditions = this.rule.conditions; + let link = null; + if (this.rule.conditions !== undefined && conditions.length === 1) { + link = ( + + ); + } + + return link; + } + + getQuickAddToFilterListLink = () => { + // Returns the link to add the partitioning field value of the anomaly to the filter + // list used in the scope part of the rule. + + // Note componentDidMount performs the checks for the existence of scope and partitioning fields. + const { anomaly, addItemToFilterList } = this.props; + const scope = this.rule.scope; + const partitionFieldName = Object.keys(scope)[0]; + const partitionFieldValue = anomaly.source[partitionFieldName]; + const filterId = scope[partitionFieldName].filter_id; + + // Partitioning field values stored under named field in anomaly record will be an array. + return ( + + ); + } + + updateConditionValue = (conditionIndex, value) => { + const { + job, + anomaly, + ruleIndex, + updateRuleAtIndex } = this.props; + + const detector = job.analysis_config.detectors[anomaly.detectorIndex]; + const editedRule = cloneDeep(detector.custom_rules[ruleIndex]); + + const conditions = editedRule.conditions; + if (conditionIndex < conditions.length) { + conditions[conditionIndex].value = value; + } + + updateRuleAtIndex(ruleIndex, editedRule); + } + + render() { + if (this.rule === undefined) { + return null; + } + + // Add items for the standard Edit and Delete links. + const descriptionListItems = [ + { + title: 'rule', + description: buildRuleDescription(this.rule, this.props.anomaly), + }, + { + title: '', + description: this.getEditRuleLink(), + }, + { + title: '', + description: this.getDeleteRuleLink() + } + ]; + + // Insert links if applicable for quick edits to a numeric condition + // or to the safe list used by the scope. + const quickConditionLink = this.getQuickEditConditionLink(); + if (quickConditionLink !== null) { + descriptionListItems.splice(1, 0, { + title: '', description: quickConditionLink + }); + } + + if (this.state.showAddToFilterListLink === true) { + const quickAddToFilterListLink = this.getQuickAddToFilterListLink(); + descriptionListItems.splice(descriptionListItems.length - 2, 0, { + title: '', description: quickAddToFilterListLink + }); + } + + descriptionListItems[1].title = 'actions'; + + return ( + + + + ); + } } RuleActionPanel.propTypes = { job: PropTypes.object.isRequired, - detectorIndex: PropTypes.number.isRequired, + anomaly: PropTypes.object.isRequired, ruleIndex: PropTypes.number.isRequired, setEditRuleIndex: PropTypes.func.isRequired, + updateRuleAtIndex: PropTypes.func.isRequired, deleteRuleAtIndex: PropTypes.func.isRequired, + addItemToFilterList: PropTypes.func.isRequired, }; diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.test.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.test.js index 8212b30a5308..53027738790b 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.test.js +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/rule_action_panel.test.js @@ -6,6 +6,28 @@ jest.mock('../../../services/job_service.js', () => 'mlJobService'); +// Mock the call for loading a filter. +// The mock is hoisted to the top, so need to prefix the filter variable +// with 'mock' so it can be used lazily. +const mockTestFilter = { + filter_id: 'eu-airlines', + description: 'List of European airlines', + items: ['ABA', 'AEL'], + used_by: { + detectors: ['mean response time'], + jobs: ['farequote'] + }, +}; +jest.mock('../../../services/ml_api_service', () => ({ + ml: { + filters: { + filters: () => { + return Promise.resolve(mockTestFilter); + } + } + } +})); + import { shallow } from 'enzyme'; import React from 'react'; @@ -38,7 +60,7 @@ describe('RuleActionPanel', () => { ACTION.SKIP_MODEL_UPDATE ], scope: { - instance: { + airline: { filter_id: 'eu-airlines', filter_type: 'exclude' } @@ -49,7 +71,7 @@ describe('RuleActionPanel', () => { ACTION.SKIP_MODEL_UPDATE ], scope: { - instance: { + airline: { filter_id: 'eu-airlines', filter_type: 'exclude' } @@ -69,51 +91,70 @@ describe('RuleActionPanel', () => { }, }; + const anomaly = { + actual: [50], + typical: [1.23], + detectorIndex: 0, + source: { + function: 'mean', + airline: ['AAL'], + }, + }; + + const setEditRuleIndex = jest.fn(() => {}); + const updateRuleAtIndex = jest.fn(() => {}); + const deleteRuleAtIndex = jest.fn(() => {}); + const addItemToFilterList = jest.fn(() => {}); + + const requiredProps = { + job, + anomaly, + detectorIndex: 0, + setEditRuleIndex, + updateRuleAtIndex, + deleteRuleAtIndex, + addItemToFilterList, + }; + test('renders panel for rule with a condition', () => { + const props = { + ...requiredProps, + ruleIndex: 0, + }; const component = shallow( - {}} - deleteRuleAtIndex={() => {}} - /> + ); expect(component).toMatchSnapshot(); }); - test('renders panel for rule with scope ', () => { + test('renders panel for rule with scope, value in filter list', () => { + const props = { + ...requiredProps, + ruleIndex: 1, + }; const component = shallow( - {}} - deleteRuleAtIndex={() => {}} - /> + ); expect(component).toMatchSnapshot(); }); - test('renders panel for rule with a condition and scope ', () => { + test('renders panel for rule with a condition and scope, value not in filter list', () => { + const props = { + ...requiredProps, + ruleIndex: 1, + }; - const component = shallow( - {}} - deleteRuleAtIndex={() => {}} - /> + const wrapper = shallow( + ); - - expect(component).toMatchSnapshot(); + wrapper.setState({ showAddToFilterListLink: true }); + expect(wrapper).toMatchSnapshot(); }); diff --git a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/select_rule_action.js b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/select_rule_action.js index 4b1d78da0c2b..ae5ad07b53b7 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/select_rule_action.js +++ b/x-pack/plugins/ml/public/components/rule_editor/select_rule_action/select_rule_action.js @@ -25,10 +25,12 @@ import { RuleActionPanel } from './rule_action_panel'; export function SelectRuleAction({ job, anomaly, - detectorIndex, setEditRuleIndex, - deleteRuleAtIndex }) { + updateRuleAtIndex, + deleteRuleAtIndex, + addItemToFilterList }) { + const detectorIndex = anomaly.detectorIndex; const detector = job.analysis_config.detectors[detectorIndex]; const rules = detector.custom_rules || []; let ruleActionPanels; @@ -38,11 +40,12 @@ export function SelectRuleAction({ @@ -57,6 +60,7 @@ export function SelectRuleAction({ {ruleActionPanels} @@ -78,7 +82,8 @@ export function SelectRuleAction({ SelectRuleAction.propTypes = { job: PropTypes.object.isRequired, anomaly: PropTypes.object.isRequired, - detectorIndex: PropTypes.number.isRequired, setEditRuleIndex: PropTypes.func.isRequired, + updateRuleAtIndex: PropTypes.func.isRequired, deleteRuleAtIndex: PropTypes.func.isRequired, + addItemToFilterList: PropTypes.func.isRequired, }; diff --git a/x-pack/plugins/ml/public/components/rule_editor/styles/main.less b/x-pack/plugins/ml/public/components/rule_editor/styles/main.less index de96dcdf6d29..b3770f9b31c9 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/styles/main.less +++ b/x-pack/plugins/ml/public/components/rule_editor/styles/main.less @@ -6,20 +6,29 @@ } .select-rule-action-panel { - padding-top:10px; + padding:10px 0px; .euiDescriptionList { .euiDescriptionList__title { flex-basis: 15%; + padding: 0px 16px; } .euiDescriptionList__description { flex-basis: 85%; } + .euiDescriptionList__title:nth-child(1), .euiDescriptionList__description:nth-child(2) { color: #1a1a1a; font-weight: 600; + border-bottom : 1px solid #d9d9d9; + padding-bottom: 12px; + } + + .euiDescriptionList__title:nth-child(3), + .euiDescriptionList__description:nth-child(4) { + padding-top: 6px; } } @@ -52,6 +61,16 @@ font-size: 12px; } + .condition-edit-value-field { + width: 170px; + height: 28px; + margin: 0px 2px; + + input { + height: 28px; + } + } + .euiExpressionButton.disabled { pointer-events: none; diff --git a/x-pack/plugins/ml/public/components/rule_editor/utils.js b/x-pack/plugins/ml/public/components/rule_editor/utils.js index d60e50cd6353..e98314d85f94 100644 --- a/x-pack/plugins/ml/public/components/rule_editor/utils.js +++ b/x-pack/plugins/ml/public/components/rule_editor/utils.js @@ -12,6 +12,7 @@ import { } from '../../../common/constants/detector_rule'; import { cloneDeep } from 'lodash'; +import { ml } from '../../services/ml_api_service'; import { mlJobService } from '../../services/job_service'; export function getNewConditionDefaults() { @@ -157,6 +158,25 @@ export function updateJobRules(job, detectorIndex, rules) { }); } +// Updates an ML filter used in the scope part of a rule, +// adding an item to the filter with the specified ID. +export function addItemToFilter(item, filterId) { + return new Promise((resolve, reject) => { + ml.filters.updateFilter( + filterId, + undefined, + [item], + undefined + ) + .then((updatedFilter) => { + resolve(updatedFilter); + }) + .catch((error) => { + reject(error); + }); + }); +} + export function buildRuleDescription(rule) { const { actions, conditions, scope } = rule; let description = 'skip '; @@ -181,7 +201,7 @@ export function buildRuleDescription(rule) { description += ' AND '; } - description += `${condition.applies_to} is ${operatorToText(condition.operator)} ${condition.value}`; + description += `${appliesToText(condition.applies_to)} is ${operatorToText(condition.operator)} ${condition.value}`; }); } @@ -250,3 +270,35 @@ export function operatorToText(operator) { return (operator !== undefined) ? operator : ''; } } + +// Returns the value of the selected 'applies_to' field from the +// selected anomaly i.e. the actual, typical or diff from typical. +export function getAppliesToValueFromAnomaly(anomaly, appliesTo) { + let actualValue; + let typicalValue; + + const actual = anomaly.actual; + if (actual !== undefined) { + actualValue = Array.isArray(actual) ? actual[0] : actual; + } + + const typical = anomaly.typical; + if (typical !== undefined) { + typicalValue = Array.isArray(typical) ? typical[0] : typical; + } + + switch (appliesTo) { + case APPLIES_TO.ACTUAL: + return actualValue; + + case APPLIES_TO.TYPICAL: + return typicalValue; + + case APPLIES_TO.DIFF_FROM_TYPICAL: + if (actual !== undefined && typical !== undefined) { + return Math.abs(actualValue - typicalValue); + } + } + + return undefined; +} diff --git a/x-pack/plugins/ml/public/explorer/explorer_controller.js b/x-pack/plugins/ml/public/explorer/explorer_controller.js index e7113f411a77..f7b93edf7b40 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_controller.js +++ b/x-pack/plugins/ml/public/explorer/explorer_controller.js @@ -922,9 +922,8 @@ module.controller('MlExplorerController', function ( anomaly.source.function_description); // For detectors with rules, add a property with the rule count. - const customRules = detector.custom_rules; - if (customRules !== undefined) { - anomaly.rulesLength = customRules.length; + if (detector !== undefined && detector.custom_rules !== undefined) { + anomaly.rulesLength = detector.custom_rules.length; } // Add properties used for building the links menu. diff --git a/x-pack/plugins/ml/public/services/ml_api_service/filters.js b/x-pack/plugins/ml/public/services/ml_api_service/filters.js index b4b66e014533..7f07e227e416 100644 --- a/x-pack/plugins/ml/public/services/ml_api_service/filters.js +++ b/x-pack/plugins/ml/public/services/ml_api_service/filters.js @@ -50,14 +50,21 @@ export const filters = { addItems, removeItems ) { + const data = {}; + if (description !== undefined) { + data.description = description; + } + if (addItems !== undefined) { + data.addItems = addItems; + } + if (removeItems !== undefined) { + data.removeItems = removeItems; + } + return http({ url: `${basePath}/filters/${filterId}`, method: 'PUT', - data: { - description, - addItems, - removeItems - } + data }); }, diff --git a/x-pack/plugins/ml/server/models/filter/filter_manager.js b/x-pack/plugins/ml/server/models/filter/filter_manager.js index 2395a22393ea..9912191d4acb 100644 --- a/x-pack/plugins/ml/server/models/filter/filter_manager.js +++ b/x-pack/plugins/ml/server/models/filter/filter_manager.js @@ -101,14 +101,21 @@ export class FilterManager { addItems, removeItems) { try { + const body = {}; + if (description !== undefined) { + body.description = description; + } + if (addItems !== undefined) { + body.add_items = addItems; + } + if (removeItems !== undefined) { + body.remove_items = removeItems; + } + // Returns the newly updated filter. return await this.callWithRequest('ml.updateFilter', { filterId, - body: { - description, - add_items: addItems, - remove_items: removeItems - } + body }); } catch (error) { return Boom.badRequest(error);