From 9209d50562d945190fb24db8b3a0831b799340e8 Mon Sep 17 00:00:00 2001 From: Pete Harverson Date: Wed, 7 Dec 2022 13:11:52 +0000 Subject: [PATCH 1/6] [ML] Use anomaly score explanation for chart tooltip multi-bucket impact (#146866) ## Summary Follow-up to #142999, using the `multi_bucket_impact` field from the `anomaly_score_explanation` data to populate the multi-bucket information in the anomaly chart tooltip using the same visualization style as used in the expanded table row. image The anomaly charts in the Anomaly Explorer and Single Metric Viewer, and the severity cell in the anomalies table now use a new `isMultiBucketAnomaly` check in `ml/common/util/anomaly_utils.ts`m to determine whether to indicate the anomaly as multi-bucket with the plus-shaped symbol. Note that this will result in some changes to whether anomalies are plotted as multi-bucket anomalies. Specifically anomalies with a 'moderate' multi-bucket impact are now likely to be marked with a plus symbol where before they were not: Before: image After: image ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --- .../common/constants/multi_bucket_impact.ts | 19 --- x-pack/plugins/ml/common/types/results.ts | 1 + .../ml/common/util/anomaly_utils.test.ts | 156 ++++++++++++++++-- .../plugins/ml/common/util/anomaly_utils.ts | 72 +++++--- .../anomalies_table_columns.js | 4 +- .../anomalies_table/anomaly_details_utils.tsx | 17 +- .../severity_cell/severity_cell.test.tsx | 4 +- .../severity_cell/severity_cell.tsx | 13 +- .../explorer_chart_single_metric.js | 5 +- .../timeseries_chart/timeseries_chart.js | 6 +- .../timeseriesexplorer_help_popover.tsx | 2 +- .../timeseriesexplorer_utils.js | 10 +- .../ml/public/application/util/chart_utils.js | 23 ++- .../application/util/chart_utils.test.js | 33 +--- .../models/results_service/anomaly_charts.ts | 10 +- .../translations/translations/fr-FR.json | 5 - .../translations/translations/ja-JP.json | 5 - .../translations/translations/zh-CN.json | 5 - 18 files changed, 248 insertions(+), 142 deletions(-) delete mode 100644 x-pack/plugins/ml/common/constants/multi_bucket_impact.ts diff --git a/x-pack/plugins/ml/common/constants/multi_bucket_impact.ts b/x-pack/plugins/ml/common/constants/multi_bucket_impact.ts deleted file mode 100644 index c708f15862163..0000000000000 --- a/x-pack/plugins/ml/common/constants/multi_bucket_impact.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -/** - * Thresholds for indicating the impact of multi-bucket features in an anomaly. - * As a rule-of-thumb, a threshold value T corresponds to the multi-bucket probability - * being 1000^(T/5) times smaller than the single bucket probability. - * So, for example, for HIGH it is 63 times smaller. - */ -export const MULTI_BUCKET_IMPACT = { - HIGH: 3, - MEDIUM: 2, - LOW: 1, - NONE: -5, -}; diff --git a/x-pack/plugins/ml/common/types/results.ts b/x-pack/plugins/ml/common/types/results.ts index 9997bc4751b41..cf25fc6081e16 100644 --- a/x-pack/plugins/ml/common/types/results.ts +++ b/x-pack/plugins/ml/common/types/results.ts @@ -99,6 +99,7 @@ export interface ChartPoint { anomalyScore?: number; actual?: number[]; multiBucketImpact?: number; + isMultiBucketAnomaly?: boolean; typical?: number[]; value?: number | null; entity?: string; diff --git a/x-pack/plugins/ml/common/util/anomaly_utils.test.ts b/x-pack/plugins/ml/common/util/anomaly_utils.test.ts index 24eb06d8bd053..b707a61d5b9bc 100644 --- a/x-pack/plugins/ml/common/util/anomaly_utils.test.ts +++ b/x-pack/plugins/ml/common/util/anomaly_utils.test.ts @@ -12,13 +12,13 @@ import { getEntityFieldList, getEntityFieldName, getEntityFieldValue, - getMultiBucketImpactLabel, getSeverity, getSeverityWithLow, getSeverityColor, isRuleSupported, showActualForFunction, showTypicalForFunction, + isMultiBucketAnomaly, } from './anomaly_utils'; describe('ML - anomaly utils', () => { @@ -260,30 +260,152 @@ describe('ML - anomaly utils', () => { }); }); - describe('getMultiBucketImpactLabel', () => { - test('returns high for 3 <= score <= 5', () => { - expect(getMultiBucketImpactLabel(3)).toBe('high'); - expect(getMultiBucketImpactLabel(5)).toBe('high'); + describe('isMultiBucketAnomaly', () => { + const singleBucketAnomaly: AnomalyRecordDoc = { + job_id: 'farequote_sb', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + anomaly_score_explanation: { + single_bucket_impact: 65, + multi_bucket_impact: 14, + lower_confidence_bound: 94.79879269994528, + typical_value: 100.26620234643129, + upper_confidence_bound: 106.04564690901603, + }, + }; + + const multiBucketAnomaly: AnomalyRecordDoc = { + job_id: 'farequote_mb', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + anomaly_score_explanation: { + single_bucket_impact: 14, + multi_bucket_impact: 65, + lower_confidence_bound: 94.79879269994528, + typical_value: 100.26620234643129, + upper_confidence_bound: 106.04564690901603, + }, + }; + + const multiBucketAnomaly2: AnomalyRecordDoc = { + job_id: 'farequote_mb2', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + anomaly_score_explanation: { + multi_bucket_impact: 65, + lower_confidence_bound: 94.79879269994528, + typical_value: 100.26620234643129, + upper_confidence_bound: 106.04564690901603, + }, + }; + + const noASEAnomaly: AnomalyRecordDoc = { + job_id: 'farequote_ase', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + }; + + const noMBIAnomaly: AnomalyRecordDoc = { + job_id: 'farequote_sbi', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + anomaly_score_explanation: { + single_bucket_impact: 65, + lower_confidence_bound: 94.79879269994528, + typical_value: 100.26620234643129, + upper_confidence_bound: 106.04564690901603, + }, + }; + + const singleBucketAnomaly2: AnomalyRecordDoc = { + job_id: 'farequote_sb2', + result_type: 'record', + probability: 0.0191711, + record_score: 4.38431, + initial_record_score: 19.654, + bucket_span: 300, + detector_index: 0, + is_interim: false, + timestamp: 1454890500000, + function: 'mean', + function_description: 'mean', + field_name: 'responsetime', + anomaly_score_explanation: { + single_bucket_impact: 65, + multi_bucket_impact: 65, + lower_confidence_bound: 94.79879269994528, + typical_value: 100.26620234643129, + upper_confidence_bound: 106.04564690901603, + }, + }; + + test('returns false when single_bucket_impact much larger than multi_bucket_impact', () => { + expect(isMultiBucketAnomaly(singleBucketAnomaly)).toBe(false); + }); + + test('returns true when multi_bucket_impact much larger than single_bucket_impact', () => { + expect(isMultiBucketAnomaly(multiBucketAnomaly)).toBe(true); }); - test('returns medium for 2 <= score < 3', () => { - expect(getMultiBucketImpactLabel(2)).toBe('medium'); - expect(getMultiBucketImpactLabel(2.99)).toBe('medium'); + test('returns true when multi_bucket_impact > 0 and single_bucket_impact undefined', () => { + expect(isMultiBucketAnomaly(multiBucketAnomaly2)).toBe(true); }); - test('returns low for 1 <= score < 2', () => { - expect(getMultiBucketImpactLabel(1)).toBe('low'); - expect(getMultiBucketImpactLabel(1.99)).toBe('low'); + test('returns false when anomaly_score_explanation undefined', () => { + expect(isMultiBucketAnomaly(noASEAnomaly)).toBe(false); }); - test('returns none for -5 <= score < 1', () => { - expect(getMultiBucketImpactLabel(-5)).toBe('none'); - expect(getMultiBucketImpactLabel(0.99)).toBe('none'); + test('returns false when multi_bucket_impact undefined', () => { + expect(isMultiBucketAnomaly(noMBIAnomaly)).toBe(false); }); - test('returns expected label when impact outside normal bounds', () => { - expect(getMultiBucketImpactLabel(10)).toBe('high'); - expect(getMultiBucketImpactLabel(-10)).toBe('none'); + test('returns false when multi_bucket_impact === single_bucket_impact', () => { + expect(isMultiBucketAnomaly(singleBucketAnomaly2)).toBe(false); }); }); diff --git a/x-pack/plugins/ml/common/util/anomaly_utils.ts b/x-pack/plugins/ml/common/util/anomaly_utils.ts index cbbec963b0c3d..31a2a5fe49ca5 100644 --- a/x-pack/plugins/ml/common/util/anomaly_utils.ts +++ b/x-pack/plugins/ml/common/util/anomaly_utils.ts @@ -12,7 +12,6 @@ import { i18n } from '@kbn/i18n'; import { CONDITIONS_NOT_SUPPORTED_FUNCTIONS } from '../constants/detector_rule'; -import { MULTI_BUCKET_IMPACT } from '../constants/multi_bucket_impact'; import { ANOMALY_SEVERITY, ANOMALY_THRESHOLD, SEVERITY_COLORS } from '../constants/anomalies'; import type { AnomaliesTableRecord, AnomalyRecordDoc } from '../types/anomalies'; @@ -119,6 +118,10 @@ function getSeverityTypes() { }); } +/** + * Returns whether the anomaly is in a categorization analysis. + * @param anomaly Anomaly table record + */ export function isCategorizationAnomaly(anomaly: AnomaliesTableRecord): boolean { return anomaly.entityName === 'mlcategory'; } @@ -219,29 +222,54 @@ export function getSeverityColor(normalizedScore: number): string { } /** - * Returns a label to use for the multi-bucket impact of an anomaly - * according to the value of the multi_bucket_impact field of a record, - * which ranges from -5 to +5. - * @param multiBucketImpact - Value of the multi_bucket_impact field of a record, from -5 to +5 + * Returns whether the anomaly record should be indicated in the UI as a multi-bucket anomaly, + * for example in anomaly charts with a cross-shaped marker. + * @param anomaly Anomaly table record */ -export function getMultiBucketImpactLabel(multiBucketImpact: number): string { - if (multiBucketImpact >= MULTI_BUCKET_IMPACT.HIGH) { - return i18n.translate('xpack.ml.anomalyUtils.multiBucketImpact.highLabel', { - defaultMessage: 'high', - }); - } else if (multiBucketImpact >= MULTI_BUCKET_IMPACT.MEDIUM) { - return i18n.translate('xpack.ml.anomalyUtils.multiBucketImpact.mediumLabel', { - defaultMessage: 'medium', - }); - } else if (multiBucketImpact >= MULTI_BUCKET_IMPACT.LOW) { - return i18n.translate('xpack.ml.anomalyUtils.multiBucketImpact.lowLabel', { - defaultMessage: 'low', - }); - } else { - return i18n.translate('xpack.ml.anomalyUtils.multiBucketImpact.noneLabel', { - defaultMessage: 'none', - }); +export function isMultiBucketAnomaly(anomaly: AnomalyRecordDoc): boolean { + if (anomaly.anomaly_score_explanation === undefined) { + return false; } + + const sb = anomaly.anomaly_score_explanation.single_bucket_impact; + const mb = anomaly.anomaly_score_explanation.multi_bucket_impact; + + if (mb === undefined || mb === 0) { + return false; + } + + if (sb !== undefined && sb > mb) { + return false; + } + + if ((sb === undefined || sb === 0) && mb > 0) { + return true; + } + + // Basis of use of 1.7 comes from the backend calculation for + // the single- and multi-bucket impacts + // 1.7 = 5.0/lg(e)/ln(1000) + // with the computation of the logarithm basis changed from e to 10. + if (sb !== undefined && mb > sb) { + return (((mb - sb) * mb) / sb) * 1.7 >= 2; + } + + return false; +} + +/** + * Returns the value on a scale of 1 to 5, from a log based scaled value for an + * anomaly score explanation impact field, such as anomaly_characteristics_impact, + * single_bucket_impact or multi_bucket_impact. + * @param score value from an impact field from the anomaly_score_explanation. + * @returns numeric value on an integer scale of 1 (low) to 5 (high). + */ +export function getAnomalyScoreExplanationImpactValue(score: number): number { + if (score < 2) return 1; + if (score < 4) return 2; + if (score < 7) return 3; + if (score < 12) return 4; + return 5; } /** diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/anomalies_table_columns.js b/x-pack/plugins/ml/public/application/components/anomalies_table/anomalies_table_columns.js index 07f52b03e6221..c8381726ee86a 100644 --- a/x-pack/plugins/ml/public/application/components/anomalies_table/anomalies_table_columns.js +++ b/x-pack/plugins/ml/public/application/components/anomalies_table/anomalies_table_columns.js @@ -27,7 +27,7 @@ import { InfluencersCell } from './influencers_cell'; import { LinksMenu } from './links_menu'; import { checkPermission } from '../../capabilities/check_capabilities'; import { mlFieldFormatService } from '../../services/field_format_service'; -import { isRuleSupported } from '../../../../common/util/anomaly_utils'; +import { isRuleSupported, isMultiBucketAnomaly } from '../../../../common/util/anomaly_utils'; import { formatValue } from '../../formatters/format_value'; import { INFLUENCERS_LIMIT, ANOMALIES_TABLE_TABS } from './anomalies_table_constants'; import { SeverityCell } from './severity_cell'; @@ -133,7 +133,7 @@ export function getColumns( ), render: (score, item) => ( - + ), sortable: true, }, diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/anomaly_details_utils.tsx b/x-pack/plugins/ml/public/application/components/anomalies_table/anomaly_details_utils.tsx index a049ee2c48ea6..0254c27f67906 100644 --- a/x-pack/plugins/ml/public/application/components/anomalies_table/anomaly_details_utils.tsx +++ b/x-pack/plugins/ml/public/application/components/anomalies_table/anomaly_details_utils.tsx @@ -26,7 +26,10 @@ import { import { AnomaliesTableRecord, MLAnomalyDoc } from '../../../../common/types/anomalies'; import { formatValue } from '../../formatters/format_value'; import { ML_JOB_AGGREGATION } from '../../../../common/constants/aggregation_types'; -import { getSeverityColor } from '../../../../common/util/anomaly_utils'; +import { + getAnomalyScoreExplanationImpactValue, + getSeverityColor, +} from '../../../../common/util/anomaly_utils'; const TIME_FIELD_NAME = 'timestamp'; @@ -597,14 +600,6 @@ function getAnomalyType(explanation: MLAnomalyDoc['anomaly_score_explanation']) return explanation.anomaly_type === 'dip' ? dip : spike; } -function getImpactValue(score: number) { - if (score < 2) return 1; - if (score < 4) return 2; - if (score < 7) return 3; - if (score < 12) return 4; - return 5; -} - const impactTooltips = { anomaly_characteristics: { low: i18n.translate( @@ -681,7 +676,7 @@ function getImpactTooltip( score: number, type: 'anomaly_characteristics' | 'single_bucket' | 'multi_bucket' ) { - const value = getImpactValue(score); + const value = getAnomalyScoreExplanationImpactValue(score); if (value < 3) { return impactTooltips[type].low; @@ -698,7 +693,7 @@ const ImpactVisual: FC<{ score: number }> = ({ score }) => { euiTheme: { colors }, } = useEuiTheme(); - const impact = getImpactValue(score); + const impact = getAnomalyScoreExplanationImpactValue(score); const boxPx = '10px'; const emptyBox = colors.lightShade; const fullBox = colors.primary; diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.test.tsx b/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.test.tsx index f58fe20facbf6..e9166c0c6c28c 100644 --- a/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.test.tsx +++ b/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.test.tsx @@ -13,7 +13,7 @@ describe('SeverityCell', () => { test('should render a single-bucket marker with rounded severity score', () => { const props = { score: 75.2, - multiBucketImpact: -2, + isMultiBucketAnomaly: false, }; const { container } = render(); expect(container.textContent).toBe('75'); @@ -24,7 +24,7 @@ describe('SeverityCell', () => { test('should render a multi-bucket marker with low severity score', () => { const props = { score: 0.8, - multiBucketImpact: 4, + isMultiBucketAnomaly: true, }; const { container } = render(); expect(container.textContent).toBe('< 1'); diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.tsx b/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.tsx index b761599a447b7..555110e4a3e4d 100644 --- a/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.tsx +++ b/x-pack/plugins/ml/public/application/components/anomalies_table/severity_cell/severity_cell.tsx @@ -7,7 +7,6 @@ import React, { FC, memo } from 'react'; import { EuiHealth, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { MULTI_BUCKET_IMPACT } from '../../../../../common/constants/multi_bucket_impact'; import { getSeverityColor, getFormattedSeverityScore, @@ -19,21 +18,19 @@ interface SeverityCellProps { */ score: number; /** - * Multi-bucket impact score from –5 to 5. - * Anomalies with a multi-bucket impact value of greater than or equal - * to 2 are indicated with a plus shaped symbol in the cell. + * Flag to indicate whether the anomaly should be displayed in the cell as a + * multi-bucket anomaly with a plus-shaped symbol. */ - multiBucketImpact: number; + isMultiBucketAnomaly: boolean; } /** * Renders anomaly severity score with single or multi-bucket impact marker. */ -export const SeverityCell: FC = memo(({ score, multiBucketImpact }) => { +export const SeverityCell: FC = memo(({ score, isMultiBucketAnomaly }) => { const severity = getFormattedSeverityScore(score); const color = getSeverityColor(score); - const isMultiBucket = multiBucketImpact >= MULTI_BUCKET_IMPACT.MEDIUM; - return isMultiBucket ? ( + return isMultiBucketAnomaly ? ( diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js index 6ce196393b97e..23f1b91910887 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js @@ -24,7 +24,6 @@ import { getFormattedSeverityScore, getSeverityColor, getSeverityWithLow, - getMultiBucketImpactLabel, } from '../../../../common/util/anomaly_utils'; import { LINE_CHART_ANOMALY_RADIUS, @@ -36,6 +35,7 @@ import { removeLabelOverlap, showMultiBucketAnomalyMarker, showMultiBucketAnomalyTooltip, + getMultiBucketImpactTooltipValue, } from '../../util/chart_utils'; import { LoadingIndicator } from '../../components/loading_indicator/loading_indicator'; import { mlFieldFormatService } from '../../services/field_format_service'; @@ -461,6 +461,7 @@ export class ExplorerChartSingleMetric extends React.Component { if (marker.anomalyScore !== undefined) { const score = parseInt(marker.anomalyScore); + tooltipData.push({ label: i18n.translate('xpack.ml.explorer.singleMetricChart.anomalyScoreLabel', { defaultMessage: 'anomaly score', @@ -478,7 +479,7 @@ export class ExplorerChartSingleMetric extends React.Component { label: i18n.translate('xpack.ml.explorer.singleMetricChart.multiBucketImpactLabel', { defaultMessage: 'multi-bucket impact', }), - value: getMultiBucketImpactLabel(marker.multiBucketImpact), + value: getMultiBucketImpactTooltipValue(marker), seriesIdentifier: { key: seriesKey, }, diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js index 8daf2e8f86891..707511c1f22d4 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js @@ -21,7 +21,6 @@ import { i18n } from '@kbn/i18n'; import { getFormattedSeverityScore, getSeverityWithLow, - getMultiBucketImpactLabel, } from '../../../../../common/util/anomaly_utils'; import { formatValue } from '../../../formatters/format_value'; import { @@ -34,6 +33,7 @@ import { numTicksForDateFormat, showMultiBucketAnomalyMarker, showMultiBucketAnomalyTooltip, + getMultiBucketImpactTooltipValue, } from '../../../util/chart_utils'; import { formatHumanReadableDateTimeSeconds } from '../../../../../common/util/date_utils'; import { getTimeBucketsFromCache } from '../../../util/time_buckets'; @@ -1509,12 +1509,12 @@ class TimeseriesChartIntl extends Component { if (showMultiBucketAnomalyTooltip(marker) === true) { tooltipData.push({ label: i18n.translate( - 'xpack.ml.timeSeriesExplorer.timeSeriesChart.multiBucketImpactLabel', + 'xpack.ml.timeSeriesExplorer.timeSeriesChart.multiBucketAnomalyLabel', { defaultMessage: 'multi-bucket impact', } ), - value: getMultiBucketImpactLabel(marker.multiBucketImpact), + value: getMultiBucketImpactTooltipValue(marker), seriesIdentifier: { key: seriesKey, }, diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_help_popover.tsx b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_help_popover.tsx index 4313c9b228825..afd93fd5acee1 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_help_popover.tsx +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_help_popover.tsx @@ -27,7 +27,7 @@ export const TimeSeriesExplorerHelpPopover = () => {

diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js index 5a508cbf4bb41..cd7c12e5c3430 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js @@ -14,6 +14,7 @@ import { each, get, find } from 'lodash'; import moment from 'moment-timezone'; +import { isMultiBucketAnomaly } from '../../../../common/util/anomaly_utils'; import { isTimeSeriesViewJob } from '../../../../common/util/job_utils'; import { parseInterval } from '../../../../common/util/parse_interval'; @@ -193,9 +194,14 @@ export function processDataForFocusAnomalies( } } - if (record.multi_bucket_impact !== undefined) { - chartPoint.multiBucketImpact = record.multi_bucket_impact; + if ( + record.anomaly_score_explanation !== undefined && + record.anomaly_score_explanation.multi_bucket_impact !== undefined + ) { + chartPoint.multiBucketImpact = record.anomaly_score_explanation.multi_bucket_impact; } + + chartPoint.isMultiBucketAnomaly = isMultiBucketAnomaly(record); } } }); diff --git a/x-pack/plugins/ml/public/application/util/chart_utils.js b/x-pack/plugins/ml/public/application/util/chart_utils.js index 4936f80f1911c..ab842fe6f688d 100644 --- a/x-pack/plugins/ml/public/application/util/chart_utils.js +++ b/x-pack/plugins/ml/public/application/util/chart_utils.js @@ -7,7 +7,7 @@ import d3 from 'd3'; import { calculateTextWidth } from './string_utils'; -import { MULTI_BUCKET_IMPACT } from '../../../common/constants/multi_bucket_impact'; +import { getAnomalyScoreExplanationImpactValue } from '../../../common/util/anomaly_utils'; import moment from 'moment'; import { CHART_TYPE } from '../explorer/explorer_constants'; import { ML_PAGES } from '../../../common/constants/locator'; @@ -224,17 +224,22 @@ export async function getExploreSeriesLink(mlLocator, series, timeRange) { } export function showMultiBucketAnomalyMarker(point) { - // TODO - test threshold with real use cases - return ( - point.multiBucketImpact !== undefined && point.multiBucketImpact >= MULTI_BUCKET_IMPACT.MEDIUM - ); + return point.isMultiBucketAnomaly === true; } export function showMultiBucketAnomalyTooltip(point) { - // TODO - test threshold with real use cases - return ( - point.multiBucketImpact !== undefined && point.multiBucketImpact >= MULTI_BUCKET_IMPACT.LOW - ); + return point.isMultiBucketAnomaly === true; +} + +export function getMultiBucketImpactTooltipValue(point) { + const numFilledSquares = + point.multiBucketImpact !== undefined + ? getAnomalyScoreExplanationImpactValue(point.multiBucketImpact) + : 0; + return new Array(5) + .fill('\u25A0 ', 0, numFilledSquares) // Unicode filled square + .fill('\u25A1 ', numFilledSquares) // Unicode hollow square + .join(''); } export function numTicks(axisWidth) { diff --git a/x-pack/plugins/ml/public/application/util/chart_utils.test.js b/x-pack/plugins/ml/public/application/util/chart_utils.test.js index 83c7b58afbefd..f7bc96e1c18d1 100644 --- a/x-pack/plugins/ml/public/application/util/chart_utils.test.js +++ b/x-pack/plugins/ml/public/application/util/chart_utils.test.js @@ -44,7 +44,6 @@ import { showMultiBucketAnomalyTooltip, } from './chart_utils'; -import { MULTI_BUCKET_IMPACT } from '../../../common/constants/multi_bucket_impact'; import { CHART_TYPE } from '../explorer/explorer_constants'; timefilter.setTime({ @@ -159,44 +158,24 @@ describe('ML - chart utils', () => { }); describe('showMultiBucketAnomalyMarker', () => { - test('returns true for points with multiBucketImpact at or above medium impact', () => { - expect(showMultiBucketAnomalyMarker({ multiBucketImpact: MULTI_BUCKET_IMPACT.HIGH })).toBe( - true - ); - expect(showMultiBucketAnomalyMarker({ multiBucketImpact: MULTI_BUCKET_IMPACT.MEDIUM })).toBe( - true - ); + test('returns true for points with isMultiBucketAnomaly=true', () => { + expect(showMultiBucketAnomalyMarker({ isMultiBucketAnomaly: true })).toBe(true); }); test('returns false for points with multiBucketImpact missing or below medium impact', () => { expect(showMultiBucketAnomalyMarker({})).toBe(false); - expect(showMultiBucketAnomalyMarker({ multiBucketImpact: MULTI_BUCKET_IMPACT.LOW })).toBe( - false - ); - expect(showMultiBucketAnomalyMarker({ multiBucketImpact: MULTI_BUCKET_IMPACT.NONE })).toBe( - false - ); + expect(showMultiBucketAnomalyMarker({ isMultiBucketAnomaly: false })).toBe(false); }); }); describe('showMultiBucketAnomalyTooltip', () => { - test('returns true for points with multiBucketImpact at or above low impact', () => { - expect(showMultiBucketAnomalyTooltip({ multiBucketImpact: MULTI_BUCKET_IMPACT.HIGH })).toBe( - true - ); - expect(showMultiBucketAnomalyTooltip({ multiBucketImpact: MULTI_BUCKET_IMPACT.MEDIUM })).toBe( - true - ); - expect(showMultiBucketAnomalyTooltip({ multiBucketImpact: MULTI_BUCKET_IMPACT.LOW })).toBe( - true - ); + test('returns true for points with isMultiBucketAnomaly=true', () => { + expect(showMultiBucketAnomalyTooltip({ isMultiBucketAnomaly: true })).toBe(true); }); test('returns false for points with multiBucketImpact missing or below medium impact', () => { expect(showMultiBucketAnomalyTooltip({})).toBe(false); - expect(showMultiBucketAnomalyTooltip({ multiBucketImpact: MULTI_BUCKET_IMPACT.NONE })).toBe( - false - ); + expect(showMultiBucketAnomalyTooltip({ isMultiBucketAnomaly: false })).toBe(false); }); }); diff --git a/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts b/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts index eb85ec82763e6..dbbf95d4806c2 100644 --- a/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts +++ b/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts @@ -37,6 +37,7 @@ import { aggregationTypeTransform, EntityField, getEntityFieldList, + isMultiBucketAnomaly, } from '../../../common/util/anomaly_utils'; import { InfluencersFilterQuery } from '../../../common/types/es_client'; import { isDefined } from '../../../common/types/guards'; @@ -1101,9 +1102,14 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu } } - if (record.multi_bucket_impact !== undefined) { - chartPoint.multiBucketImpact = record.multi_bucket_impact; + if ( + record.anomaly_score_explanation !== undefined && + record.anomaly_score_explanation.multi_bucket_impact !== undefined + ) { + chartPoint.multiBucketImpact = record.anomaly_score_explanation.multi_bucket_impact; } + + chartPoint.isMultiBucketAnomaly = isMultiBucketAnomaly(record); } }); diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 838ef890f362d..f638218c3ddea 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -19646,10 +19646,6 @@ "xpack.ml.anomalyResultsViewSelector.buttonGroupLegend": "Sélecteur de vue des résultats d'anomalie", "xpack.ml.anomalyResultsViewSelector.singleMetricViewerLabel": "Voir les résultats dans Single Metric Viewer", "xpack.ml.anomalySwimLane.noOverallDataMessage": "Aucune anomalie trouvée dans les résultats de groupe généraux pour cette plage temporelle", - "xpack.ml.anomalyUtils.multiBucketImpact.highLabel": "élevé", - "xpack.ml.anomalyUtils.multiBucketImpact.lowLabel": "bas", - "xpack.ml.anomalyUtils.multiBucketImpact.mediumLabel": "moyen", - "xpack.ml.anomalyUtils.multiBucketImpact.noneLabel": "aucun", "xpack.ml.anomalyUtils.severity.criticalLabel": "critique", "xpack.ml.anomalyUtils.severity.majorLabel": "majeur", "xpack.ml.anomalyUtils.severity.minorLabel": "mineure", @@ -21503,7 +21499,6 @@ "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.actualLabel": "réel", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.lowerBoundsLabel": "limites inférieures", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.upperBoundsLabel": "limites supérieures", - "xpack.ml.timeSeriesExplorer.timeSeriesChart.multiBucketImpactLabel": "impact sur plusieurs compartiments", "xpack.ml.timeSeriesExplorer.timeSeriesChart.typicalLabel": "typique", "xpack.ml.timeSeriesExplorer.timeSeriesChart.valueLabel": "valeur", "xpack.ml.timeSeriesExplorer.timeSeriesChart.withoutAnomalyScore.predictionLabel": "prédiction", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 494c737530cc9..81ada7b9c6f35 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -19627,10 +19627,6 @@ "xpack.ml.anomalyResultsViewSelector.buttonGroupLegend": "異常結果ビューセレクター", "xpack.ml.anomalyResultsViewSelector.singleMetricViewerLabel": "シングルメトリックビューアーで結果を表示", "xpack.ml.anomalySwimLane.noOverallDataMessage": "この時間範囲のバケット結果全体で異常が見つかりませんでした", - "xpack.ml.anomalyUtils.multiBucketImpact.highLabel": "高", - "xpack.ml.anomalyUtils.multiBucketImpact.lowLabel": "低", - "xpack.ml.anomalyUtils.multiBucketImpact.mediumLabel": "中", - "xpack.ml.anomalyUtils.multiBucketImpact.noneLabel": "なし", "xpack.ml.anomalyUtils.severity.criticalLabel": "致命的", "xpack.ml.anomalyUtils.severity.majorLabel": "メジャー", "xpack.ml.anomalyUtils.severity.minorLabel": "マイナー", @@ -21484,7 +21480,6 @@ "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.actualLabel": "実際", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.lowerBoundsLabel": "下の境界", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.upperBoundsLabel": "上の境界", - "xpack.ml.timeSeriesExplorer.timeSeriesChart.multiBucketImpactLabel": "複数バケットの影響", "xpack.ml.timeSeriesExplorer.timeSeriesChart.typicalLabel": "通常", "xpack.ml.timeSeriesExplorer.timeSeriesChart.valueLabel": "値", "xpack.ml.timeSeriesExplorer.timeSeriesChart.withoutAnomalyScore.predictionLabel": "予測", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 9b0d31c174d69..6000d156083ad 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -19657,10 +19657,6 @@ "xpack.ml.anomalyResultsViewSelector.buttonGroupLegend": "异常结果视图选择器", "xpack.ml.anomalyResultsViewSelector.singleMetricViewerLabel": "在 Single Metric Viewer 中查看结果", "xpack.ml.anomalySwimLane.noOverallDataMessage": "此时间范围的总体存储桶中未发现异常", - "xpack.ml.anomalyUtils.multiBucketImpact.highLabel": "高", - "xpack.ml.anomalyUtils.multiBucketImpact.lowLabel": "低", - "xpack.ml.anomalyUtils.multiBucketImpact.mediumLabel": "中", - "xpack.ml.anomalyUtils.multiBucketImpact.noneLabel": "无", "xpack.ml.anomalyUtils.severity.criticalLabel": "紧急", "xpack.ml.anomalyUtils.severity.majorLabel": "重大", "xpack.ml.anomalyUtils.severity.minorLabel": "轻微", @@ -21514,7 +21510,6 @@ "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.actualLabel": "实际", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.lowerBoundsLabel": "下边界", "xpack.ml.timeSeriesExplorer.timeSeriesChart.modelPlotEnabled.upperBoundsLabel": "上边界", - "xpack.ml.timeSeriesExplorer.timeSeriesChart.multiBucketImpactLabel": "多存储桶影响", "xpack.ml.timeSeriesExplorer.timeSeriesChart.typicalLabel": "典型", "xpack.ml.timeSeriesExplorer.timeSeriesChart.valueLabel": "值", "xpack.ml.timeSeriesExplorer.timeSeriesChart.withoutAnomalyScore.predictionLabel": "预测", From 4294e3b17f559a0073f5453a47df7e9dc83c58ba Mon Sep 17 00:00:00 2001 From: Pete Harverson Date: Wed, 7 Dec 2022 13:12:17 +0000 Subject: [PATCH 2/6] [ML] Add API tests for index exists endpoint (#146400) ## Summary Adds tests to check the response of the index exists endpoint: ``` /api/ml/index_exists ``` Also fixed the behavior of the endpoint when passed an index with a wilcard expression so that it no longer returns `true` if an index does not exist matching the expression by adding `allow_no_indices: false` to the query parameters. Part of https://github.com/elastic/kibana/issues/142456 ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- x-pack/plugins/ml/server/routes/system.ts | 1 + .../api_integration/apis/ml/system/index.ts | 1 + .../apis/ml/system/index_exists.ts | 111 ++++++++++++++++++ 3 files changed, 113 insertions(+) create mode 100644 x-pack/test/api_integration/apis/ml/system/index_exists.ts diff --git a/x-pack/plugins/ml/server/routes/system.ts b/x-pack/plugins/ml/server/routes/system.ts index c067b8e3ff438..6ddc325154ed2 100644 --- a/x-pack/plugins/ml/server/routes/system.ts +++ b/x-pack/plugins/ml/server/routes/system.ts @@ -231,6 +231,7 @@ export function systemRoutes( indices.map(async (index) => client.asCurrentUser.indices.exists({ index, + allow_no_indices: false, }) ) ); diff --git a/x-pack/test/api_integration/apis/ml/system/index.ts b/x-pack/test/api_integration/apis/ml/system/index.ts index 68ffd5fa267e9..8b9aef9b813c9 100644 --- a/x-pack/test/api_integration/apis/ml/system/index.ts +++ b/x-pack/test/api_integration/apis/ml/system/index.ts @@ -11,5 +11,6 @@ export default function ({ loadTestFile }: FtrProviderContext) { describe('system', function () { loadTestFile(require.resolve('./capabilities')); loadTestFile(require.resolve('./space_capabilities')); + loadTestFile(require.resolve('./index_exists')); }); } diff --git a/x-pack/test/api_integration/apis/ml/system/index_exists.ts b/x-pack/test/api_integration/apis/ml/system/index_exists.ts new file mode 100644 index 0000000000000..6cd68d82f0b88 --- /dev/null +++ b/x-pack/test/api_integration/apis/ml/system/index_exists.ts @@ -0,0 +1,111 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import expect from '@kbn/expect'; + +import { FtrProviderContext } from '../../../ftr_provider_context'; +import { USER } from '../../../../functional/services/ml/security_common'; +import { COMMON_REQUEST_HEADERS } from '../../../../functional/services/ml/common_api'; + +export default ({ getService }: FtrProviderContext) => { + const esArchiver = getService('esArchiver'); + const supertest = getService('supertestWithoutAuth'); + const ml = getService('ml'); + + const responseBody = { + ft_farequote_small: { exists: true }, + 'ft_farequote_*': { exists: true }, // wildcard + ft_farequote_fail: { exists: false }, + 'ft_farequote_fail_*': { exists: false }, // wildcard + }; + + const testDataList = [ + { + testTitle: 'as ML Poweruser', + user: USER.ML_POWERUSER, + requestBody: { + indices: Object.keys(responseBody), + }, + expected: { + responseCode: 200, + responseBody, + }, + }, + ]; + + const testDataListUnauthorized = [ + { + testTitle: 'as ML Viewer', + user: USER.ML_VIEWER, + requestBody: { + indices: Object.keys(responseBody), + }, + expected: { + responseCode: 403, + error: 'Forbidden', + }, + }, + { + testTitle: 'as ML Unauthorized user', + user: USER.ML_UNAUTHORIZED, + requestBody: { + jobIds: Object.keys(responseBody), + }, + expected: { + responseCode: 403, + error: 'Forbidden', + }, + }, + ]; + + async function runRequest(user: USER, requestBody: object, expectedStatusCode: number) { + const { body, status } = await supertest + .post('/api/ml/index_exists') + .auth(user, ml.securityCommon.getPasswordForUser(user)) + .set(COMMON_REQUEST_HEADERS) + .send(requestBody); + + ml.api.assertResponseStatusCode(expectedStatusCode, status, body); + return body; + } + + describe('POST ml/index_exists', function () { + before(async () => { + await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/ml/farequote_small'); + }); + + describe('should correctly check if indices exist ', function () { + for (const testData of testDataList) { + it(`${testData.testTitle}`, async () => { + const body = await runRequest( + testData.user, + testData.requestBody, + testData.expected.responseCode + ); + const expectedResponse = testData.expected.responseBody; + expect(body).to.eql(expectedResponse); + }); + } + }); + + describe('rejects request', function () { + for (const testData of testDataListUnauthorized) { + describe('fails to check if indices exist', function () { + it(`${testData.testTitle}`, async () => { + const body = await runRequest( + testData.user, + testData.requestBody, + testData.expected.responseCode + ); + + expect(body).to.have.property('error').eql(testData.expected.error); + }); + }); + } + }); + }); +}; From f13f167286ff483de04d8751dac1f100fe44f1fe Mon Sep 17 00:00:00 2001 From: Coen Warmer Date: Wed, 7 Dec 2022 14:16:24 +0100 Subject: [PATCH 3/6] Fix flaky test for Observability > Rules Page (#147175) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves https://github.com/elastic/kibana/issues/146450. ## Summary Check if Create Rule button not only exists, but also is enabled. - [X] Ran the Flaky Test Runner on this branch with 50 iterations: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1634#0184ec8a-d86a-40a9-9e04-47936d9ec9f9 ✅ --- .../apps/observability/pages/rules_page.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/test/observability_functional/apps/observability/pages/rules_page.ts b/x-pack/test/observability_functional/apps/observability/pages/rules_page.ts index 89be0d8ccbd70..900b05f46531a 100644 --- a/x-pack/test/observability_functional/apps/observability/pages/rules_page.ts +++ b/x-pack/test/observability_functional/apps/observability/pages/rules_page.ts @@ -65,14 +65,17 @@ export default ({ getService }: FtrProviderContext) => { }); }); - // FLAKY: https://github.com/elastic/kibana/issues/146450 - describe.skip('Create rule button', () => { + describe('Create rule button', () => { it('Show Create Rule flyout when Create Rule button is clicked', async () => { await observability.alerts.common.navigateToRulesPage(); await retry.waitFor( 'Create Rule button is visible', async () => await testSubjects.exists('createRuleButton') ); + await retry.waitFor( + 'Create Rule button is enabled', + async () => await testSubjects.isEnabled('createRuleButton') + ); await observability.alerts.rulesPage.clickCreateRuleButton(); await retry.waitFor( 'Create Rule flyout is visible', From 031d8a8cead4840659a92e2694ed58603137aab9 Mon Sep 17 00:00:00 2001 From: Alison Goryachev Date: Wed, 7 Dec 2022 08:21:50 -0500 Subject: [PATCH 4/6] [Index Management] Fix component template usage count sort (#147141) --- .../component_template_list.test.ts | 31 ++++++++++++++++++- .../component_template_list.helpers.ts | 12 +++++-- .../component_template_list/table.tsx | 2 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/component_template_list.test.ts b/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/component_template_list.test.ts index a3e9524dcd3ca..ae40fbc9009a7 100644 --- a/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/component_template_list.test.ts +++ b/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/component_template_list.test.ts @@ -46,7 +46,16 @@ describe('', () => { isManaged: false, }; - const componentTemplates = [componentTemplate1, componentTemplate2]; + const componentTemplate3: ComponentTemplateListItem = { + name: 'test_component_template_3', + hasMappings: true, + hasAliases: true, + hasSettings: true, + usedBy: ['test_index_template_1', 'test_index_template_2'], + isManaged: false, + }; + + const componentTemplates = [componentTemplate1, componentTemplate2, componentTemplate3]; httpRequestsMockHelpers.setLoadComponentTemplatesResponse(componentTemplates); @@ -63,6 +72,26 @@ describe('', () => { }); }); + test('should sort "Usage count" column by number', async () => { + const { actions, table } = testBed; + + // Sort ascending + await actions.clickTableColumnSortButton(1); + + const { tableCellsValues: ascTableCellsValues } = + table.getMetaData('componentTemplatesTable'); + const ascUsageCountValues = ascTableCellsValues.map((row) => row[2]); + expect(ascUsageCountValues).toEqual(['Not in use', '1', '2']); + + // Sort descending + await actions.clickTableColumnSortButton(1); + + const { tableCellsValues: descTableCellsValues } = + table.getMetaData('componentTemplatesTable'); + const descUsageCountValues = descTableCellsValues.map((row) => row[2]); + expect(descUsageCountValues).toEqual(['2', '1', 'Not in use']); + }); + test('should reload the component templates data', async () => { const { component, actions } = testBed; diff --git a/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_list.helpers.ts b/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_list.helpers.ts index 9fec92812fd1a..a7087b5b45100 100644 --- a/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_list.helpers.ts +++ b/x-pack/plugins/index_management/public/application/components/component_templates/__jest__/client_integration/helpers/component_template_list.helpers.ts @@ -32,7 +32,7 @@ export type ComponentTemplateListTestBed = TestBed { - const { find } = testBed; + const { find, component } = testBed; /** * User Actions @@ -42,7 +42,7 @@ const createActions = (testBed: TestBed) => { }; const clickComponentTemplateAt = async (index: number) => { - const { component, table, router } = testBed; + const { table, router } = testBed; const { rows } = table.getMetaData('componentTemplatesTable'); const componentTemplateLink = findTestSubject( rows[index].reactWrapper, @@ -57,6 +57,13 @@ const createActions = (testBed: TestBed) => { }); }; + const clickTableColumnSortButton = async (index: number) => { + await act(async () => { + find('tableHeaderSortButton').at(index).simulate('click'); + }); + component.update(); + }; + const clickDeleteActionAt = (index: number) => { const { table } = testBed; @@ -70,6 +77,7 @@ const createActions = (testBed: TestBed) => { clickReloadButton, clickComponentTemplateAt, clickDeleteActionAt, + clickTableColumnSortButton, }; }; diff --git a/x-pack/plugins/index_management/public/application/components/component_templates/component_template_list/table.tsx b/x-pack/plugins/index_management/public/application/components/component_templates/component_template_list/table.tsx index 5b59523ff2548..83bcc171a4873 100644 --- a/x-pack/plugins/index_management/public/application/components/component_templates/component_template_list/table.tsx +++ b/x-pack/plugins/index_management/public/application/components/component_templates/component_template_list/table.tsx @@ -187,7 +187,7 @@ export const ComponentTable: FunctionComponent = ({ name: i18n.translate('xpack.idxMgmt.componentTemplatesList.table.isInUseColumnTitle', { defaultMessage: 'Usage count', }), - sortable: true, + sortable: ({ usedBy }: ComponentTemplateListItem) => usedBy.length, render: (usedBy: string[]) => { if (usedBy.length) { return usedBy.length; From 925666e04b802c45c04e4f7011cf336476b53cd1 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 7 Dec 2022 14:25:31 +0100 Subject: [PATCH 5/6] [RAM] Return rules from bulk delete (#147077) ## Summary In this PR we are returning in response for bulkDelete API actual rules which have being deleted like we do for the bulk editing API. Security solution needs that information as part of the API contract so they can have their stateful rule table still working the way it is. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../server/routes/bulk_delete_rules.test.ts | 2 +- .../server/rules_client/common/index.ts | 1 - .../retry_if_bulk_delete_conflicts.test.ts | 100 -------------- .../common/retry_if_bulk_delete_conflicts.ts | 130 ------------------ .../rules_client/methods/bulk_delete.ts | 50 +++++-- .../rules_client/tests/bulk_delete.test.ts | 82 ++++------- .../rules_client/tests/bulk_disable.test.ts | 53 +++---- .../rules_client/tests/bulk_enable.test.ts | 55 ++++---- .../server/rules_client/tests/test_helpers.ts | 106 ++++++++------ .../group3/tests/alerting/bulk_delete.ts | 119 ++++++++++++++-- 10 files changed, 294 insertions(+), 404 deletions(-) delete mode 100644 x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.test.ts delete mode 100644 x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.ts diff --git a/x-pack/plugins/alerting/server/routes/bulk_delete_rules.test.ts b/x-pack/plugins/alerting/server/routes/bulk_delete_rules.test.ts index 8fc27cfbc0054..c15405be85253 100644 --- a/x-pack/plugins/alerting/server/routes/bulk_delete_rules.test.ts +++ b/x-pack/plugins/alerting/server/routes/bulk_delete_rules.test.ts @@ -26,7 +26,7 @@ beforeEach(() => { describe('bulkDeleteRulesRoute', () => { const bulkDeleteRequest = { filter: '' }; - const bulkDeleteResult = { errors: [], total: 1, taskIdsFailedToBeDeleted: [] }; + const bulkDeleteResult = { rules: [], errors: [], total: 1, taskIdsFailedToBeDeleted: [] }; it('should delete rules with proper parameters', async () => { const licenseState = licenseStateMock.create(); diff --git a/x-pack/plugins/alerting/server/rules_client/common/index.ts b/x-pack/plugins/alerting/server/rules_client/common/index.ts index 6604c417cc639..984c288095d4c 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/index.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/index.ts @@ -8,7 +8,6 @@ export { mapSortField } from './map_sort_field'; export { validateOperationOnAttributes } from './validate_attributes'; export { retryIfBulkEditConflicts } from './retry_if_bulk_edit_conflicts'; -export { retryIfBulkDeleteConflicts } from './retry_if_bulk_delete_conflicts'; export { retryIfBulkDisableConflicts } from './retry_if_bulk_disable_conflicts'; export { retryIfBulkOperationConflicts } from './retry_if_bulk_operation_conflicts'; export { applyBulkEditOperation } from './apply_bulk_edit_operation'; diff --git a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.test.ts b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.test.ts deleted file mode 100644 index 32a18ea7f0984..0000000000000 --- a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { KueryNode } from '@kbn/es-query'; -import { loggingSystemMock } from '@kbn/core/server/mocks'; - -import { retryIfBulkDeleteConflicts } from './retry_if_bulk_delete_conflicts'; -import { RETRY_IF_CONFLICTS_ATTEMPTS } from './wait_before_next_retry'; - -const mockFilter: KueryNode = { - type: 'function', - value: 'mock', -}; - -const mockLogger = loggingSystemMock.create().get(); - -const mockSuccessfulResult = { - apiKeysToInvalidate: ['apiKey1'], - errors: [], - taskIdsToDelete: ['taskId1'], -}; - -const error409 = { - message: 'some fake message', - status: 409, - rule: { - id: 'fake_rule_id', - name: 'fake rule name', - }, -}; - -const getOperationConflictsTimes = (times: number) => { - return async () => { - conflictOperationMock(); - times--; - if (times >= 0) { - return { - apiKeysToInvalidate: [], - taskIdsToDelete: [], - errors: [error409], - }; - } - return mockSuccessfulResult; - }; -}; - -const OperationSuccessful = async () => mockSuccessfulResult; -const conflictOperationMock = jest.fn(); - -describe('retryIfBulkDeleteConflicts', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - - test('should work when operation is successful', async () => { - const result = await retryIfBulkDeleteConflicts(mockLogger, OperationSuccessful, mockFilter); - - expect(result).toEqual(mockSuccessfulResult); - }); - - test('should throw error when operation fails', async () => { - await expect( - retryIfBulkDeleteConflicts( - mockLogger, - async () => { - throw Error('Test failure'); - }, - mockFilter - ) - ).rejects.toThrowError('Test failure'); - }); - - test(`should return conflict errors when number of retries exceeds ${RETRY_IF_CONFLICTS_ATTEMPTS}`, async () => { - const result = await retryIfBulkDeleteConflicts( - mockLogger, - getOperationConflictsTimes(RETRY_IF_CONFLICTS_ATTEMPTS + 1), - mockFilter - ); - - expect(result.errors).toEqual([error409]); - expect(mockLogger.warn).toBeCalledWith('Bulk delete rules conflicts, exceeded retries'); - }); - - for (let i = 1; i <= RETRY_IF_CONFLICTS_ATTEMPTS; i++) { - test(`should work when operation conflicts ${i} times`, async () => { - const result = await retryIfBulkDeleteConflicts( - mockLogger, - getOperationConflictsTimes(i), - mockFilter - ); - - expect(conflictOperationMock.mock.calls.length).toBe(i + 1); - expect(result).toStrictEqual(mockSuccessfulResult); - }); - } -}); diff --git a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.ts b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.ts deleted file mode 100644 index 46a0932253276..0000000000000 --- a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_delete_conflicts.ts +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import pMap from 'p-map'; -import { chunk } from 'lodash'; -import { KueryNode } from '@kbn/es-query'; -import { Logger } from '@kbn/core/server'; -import { convertRuleIdsToKueryNode } from '../../lib'; -import { BulkOperationError } from '../types'; -import { waitBeforeNextRetry, RETRY_IF_CONFLICTS_ATTEMPTS } from './wait_before_next_retry'; - -const MAX_RULES_IDS_IN_RETRY = 1000; - -export type BulkDeleteOperation = (filter: KueryNode | null) => Promise<{ - apiKeysToInvalidate: string[]; - errors: BulkOperationError[]; - taskIdsToDelete: string[]; -}>; - -interface ReturnRetry { - apiKeysToInvalidate: string[]; - errors: BulkOperationError[]; - taskIdsToDelete: string[]; -} - -/** - * Retries BulkDelete requests - * If in response are presents conflicted savedObjects(409 statusCode), this util constructs filter with failed SO ids and retries bulkDelete operation until - * all SO updated or number of retries exceeded - * @param logger - * @param bulkEditOperation - * @param filter - KueryNode filter - * @param retries - number of retries left - * @param accApiKeysToInvalidate - accumulated apiKeys that need to be invalidated - * @param accErrors - accumulated conflict errors - * @param accTaskIdsToDelete - accumulated task ids - * @returns Promise - */ -export const retryIfBulkDeleteConflicts = async ( - logger: Logger, - bulkDeleteOperation: BulkDeleteOperation, - filter: KueryNode | null, - retries: number = RETRY_IF_CONFLICTS_ATTEMPTS, - accApiKeysToInvalidate: string[] = [], - accErrors: BulkOperationError[] = [], - accTaskIdsToDelete: string[] = [] -): Promise => { - try { - const { - apiKeysToInvalidate: currentApiKeysToInvalidate, - errors: currentErrors, - taskIdsToDelete: currentTaskIdsToDelete, - } = await bulkDeleteOperation(filter); - - const apiKeysToInvalidate = [...accApiKeysToInvalidate, ...currentApiKeysToInvalidate]; - const taskIdsToDelete = [...accTaskIdsToDelete, ...currentTaskIdsToDelete]; - const errors = - retries <= 0 - ? [...accErrors, ...currentErrors] - : [...accErrors, ...currentErrors.filter((error) => error.status !== 409)]; - - const ruleIdsWithConflictError = currentErrors.reduce((acc, error) => { - if (error.status === 409) { - return [...acc, error.rule.id]; - } - return acc; - }, []); - - if (ruleIdsWithConflictError.length === 0) { - return { - apiKeysToInvalidate, - errors, - taskIdsToDelete, - }; - } - - if (retries <= 0) { - logger.warn('Bulk delete rules conflicts, exceeded retries'); - - return { - apiKeysToInvalidate, - errors, - taskIdsToDelete, - }; - } - - logger.debug( - `Bulk delete rules conflicts, retrying ..., ${ruleIdsWithConflictError.length} saved objects conflicted` - ); - - await waitBeforeNextRetry(retries); - - // here, we construct filter query with ids. But, due to a fact that number of conflicted saved objects can exceed few thousands we can encounter following error: - // "all shards failed: search_phase_execution_exception: [query_shard_exception] Reason: failed to create query: maxClauseCount is set to 2621" - // That's why we chunk processing ids into pieces by size equals to MAX_RULES_IDS_IN_RETRY - return ( - await pMap( - chunk(ruleIdsWithConflictError, MAX_RULES_IDS_IN_RETRY), - async (queryIds) => - retryIfBulkDeleteConflicts( - logger, - bulkDeleteOperation, - convertRuleIdsToKueryNode(queryIds), - retries - 1, - apiKeysToInvalidate, - errors, - taskIdsToDelete - ), - { - concurrency: 1, - } - ) - ).reduce( - (acc, item) => { - return { - apiKeysToInvalidate: [...acc.apiKeysToInvalidate, ...item.apiKeysToInvalidate], - errors: [...acc.errors, ...item.errors], - taskIdsToDelete: [...acc.taskIdsToDelete, ...item.taskIdsToDelete], - }; - }, - { apiKeysToInvalidate: [], errors: [], taskIdsToDelete: [] } - ); - } catch (err) { - throw err; - } -}; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_delete.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_delete.ts index 66bbd86bf9155..6e581ab94a6d5 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_delete.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_delete.ts @@ -6,14 +6,14 @@ */ import { KueryNode, nodeBuilder } from '@kbn/es-query'; -import { SavedObjectsBulkDeleteObject } from '@kbn/core/server'; +import { SavedObjectsBulkUpdateObject } from '@kbn/core/server'; import { RawRule } from '../../types'; import { convertRuleIdsToKueryNode } from '../../lib'; import { bulkMarkApiKeysForInvalidation } from '../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events'; -import { getAuthorizationFilter, checkAuthorizationAndGetTotal } from '../lib'; +import { getAuthorizationFilter, checkAuthorizationAndGetTotal, getAlertFromRaw } from '../lib'; import { - retryIfBulkDeleteConflicts, + retryIfBulkOperationConflicts, buildKueryNodeFilter, getAndValidateCommonBulkOptions, } from '../common'; @@ -35,11 +35,15 @@ export const bulkDeleteRules = async (context: RulesClientContext, options: Bulk action: 'DELETE', }); - const { apiKeysToInvalidate, errors, taskIdsToDelete } = await retryIfBulkDeleteConflicts( - context.logger, - (filterKueryNode: KueryNode | null) => bulkDeleteWithOCC(context, { filter: filterKueryNode }), - kueryNodeFilterWithAuth - ); + const { rules, errors, accListSpecificForBulkOperation } = await retryIfBulkOperationConflicts({ + action: 'DELETE', + logger: context.logger, + bulkOperation: (filterKueryNode: KueryNode | null) => + bulkDeleteWithOCC(context, { filter: filterKueryNode }), + filter: kueryNodeFilterWithAuth, + }); + + const [apiKeysToInvalidate, taskIdsToDelete] = accListSpecificForBulkOperation; const taskIdsFailedToBeDeleted: string[] = []; const taskIdsSuccessfullyDeleted: string[] = []; @@ -80,7 +84,18 @@ export const bulkDeleteRules = async (context: RulesClientContext, options: Bulk context.unsecuredSavedObjectsClient ); - return { errors, total, taskIdsFailedToBeDeleted }; + const deletedRules = rules.map(({ id, attributes, references }) => { + return getAlertFromRaw( + context, + id, + attributes.alertTypeId as string, + attributes as RawRule, + references, + false + ); + }); + + return { errors, rules: deletedRules, total, taskIdsFailedToBeDeleted }; }; const bulkDeleteWithOCC = async ( @@ -97,7 +112,7 @@ const bulkDeleteWithOCC = async ( } ); - const rules: SavedObjectsBulkDeleteObject[] = []; + const rulesToDelete: Array> = []; const apiKeysToInvalidate: string[] = []; const taskIdsToDelete: string[] = []; const errors: BulkOperationError[] = []; @@ -116,7 +131,7 @@ const bulkDeleteWithOCC = async ( if (rule.attributes.scheduledTaskId) { taskIdToRuleIdMapping[rule.id] = rule.attributes.scheduledTaskId; } - rules.push(rule); + rulesToDelete.push(rule); context.auditLogger?.log( ruleAuditEvent({ @@ -128,7 +143,9 @@ const bulkDeleteWithOCC = async ( } } - const result = await context.unsecuredSavedObjectsClient.bulkDelete(rules); + const result = await context.unsecuredSavedObjectsClient.bulkDelete(rulesToDelete); + + const deletedRuleIds: string[] = []; result.statuses.forEach((status) => { if (status.error === undefined) { @@ -138,6 +155,7 @@ const bulkDeleteWithOCC = async ( if (taskIdToRuleIdMapping[status.id]) { taskIdsToDelete.push(taskIdToRuleIdMapping[status.id]); } + deletedRuleIds.push(status.id); } else { errors.push({ message: status.error.message ?? 'n/a', @@ -149,5 +167,11 @@ const bulkDeleteWithOCC = async ( }); } }); - return { apiKeysToInvalidate, errors, taskIdsToDelete }; + const rules = rulesToDelete.filter((rule) => deletedRuleIds.includes(rule.id)); + + return { + errors, + rules, + accListSpecificForBulkOperation: [apiKeysToInvalidate, taskIdsToDelete], + }; }; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts index e43ee8a37ae7e..19c020f171f3b 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_delete.test.ts @@ -19,6 +19,7 @@ import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; import { getBeforeSetup, setGlobalDate } from './lib'; import { bulkMarkApiKeysForInvalidation } from '../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import { loggerMock } from '@kbn/logging-mocks'; +import { enabledRule1, enabledRule2, returnedRule1, returnedRule2 } from './test_helpers'; jest.mock('../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ bulkMarkApiKeysForInvalidation: jest.fn(), @@ -63,33 +64,9 @@ setGlobalDate(); describe('bulkDelete', () => { let rulesClient: RulesClient; - const existingRule = { - id: 'id1', - type: 'alert', - attributes: {}, - references: [], - version: '123', - }; - const existingDecryptedRule1 = { - ...existingRule, - attributes: { - ...existingRule.attributes, - scheduledTaskId: 'taskId1', - apiKey: Buffer.from('123:abc').toString('base64'), - }, - }; - const existingDecryptedRule2 = { - ...existingRule, - id: 'id2', - attributes: { - ...existingRule.attributes, - scheduledTaskId: 'taskId2', - apiKey: Buffer.from('321:abc').toString('base64'), - }, - }; const mockCreatePointInTimeFinderAsInternalUser = ( - response = { saved_objects: [existingDecryptedRule1, existingDecryptedRule2] } + response = { saved_objects: [enabledRule1, enabledRule2] } ) => { encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest .fn() @@ -103,6 +80,7 @@ describe('bulkDelete', () => { beforeEach(async () => { rulesClient = new RulesClient(rulesClientParams); + mockCreatePointInTimeFinderAsInternalUser(); authorization.getFindAuthorizationFilter.mockResolvedValue({ ensureRuleTypeIsAuthorized() {}, }); @@ -136,7 +114,6 @@ describe('bulkDelete', () => { }); test('should try to delete rules, one successful and one with 500 error', async () => { - mockCreatePointInTimeFinderAsInternalUser(); unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({ statuses: [ { id: 'id1', type: 'alert', success: true }, @@ -157,11 +134,11 @@ describe('bulkDelete', () => { expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledTimes(1); expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledWith([ - existingDecryptedRule1, - existingDecryptedRule2, + enabledRule1, + enabledRule2, ]); expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledTimes(1); - expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['taskId1']); + expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['id1']); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledWith( { apiKeys: ['MTIzOmFiYw=='] }, @@ -169,7 +146,8 @@ describe('bulkDelete', () => { expect.anything() ); expect(result).toStrictEqual({ - errors: [{ message: 'UPS', rule: { id: 'id2', name: 'n/a' }, status: 500 }], + rules: [returnedRule1], + errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 500 }], total: 2, taskIdsFailedToBeDeleted: [], }); @@ -226,19 +204,19 @@ describe('bulkDelete', () => { .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule1, existingDecryptedRule2] }; + yield { saved_objects: [enabledRule1, enabledRule2] }; }, }) .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule2] }; + yield { saved_objects: [enabledRule2] }; }, }) .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule2] }; + yield { saved_objects: [enabledRule2] }; }, }); @@ -246,7 +224,7 @@ describe('bulkDelete', () => { expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledTimes(3); expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledTimes(1); - expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['taskId1']); + expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['id1']); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledWith( { apiKeys: ['MTIzOmFiYw=='] }, @@ -254,7 +232,8 @@ describe('bulkDelete', () => { expect.anything() ); expect(result).toStrictEqual({ - errors: [{ message: 'UPS', rule: { id: 'id2', name: 'n/a' }, status: 409 }], + rules: [returnedRule1], + errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 409 }], total: 2, taskIdsFailedToBeDeleted: [], }); @@ -292,19 +271,19 @@ describe('bulkDelete', () => { .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule1, existingDecryptedRule2] }; + yield { saved_objects: [enabledRule1, enabledRule2] }; }, }) .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule2] }; + yield { saved_objects: [enabledRule2] }; }, }) .mockResolvedValueOnce({ close: jest.fn(), find: function* asyncGenerator() { - yield { saved_objects: [existingDecryptedRule2] }; + yield { saved_objects: [enabledRule2] }; }, }); @@ -312,7 +291,7 @@ describe('bulkDelete', () => { expect(unsecuredSavedObjectsClient.bulkDelete).toHaveBeenCalledTimes(2); expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledTimes(1); - expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['taskId1', 'taskId2']); + expect(taskManager.bulkRemoveIfExist).toHaveBeenCalledWith(['id1', 'id2']); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledWith( { apiKeys: ['MTIzOmFiYw==', 'MzIxOmFiYw=='] }, @@ -320,6 +299,7 @@ describe('bulkDelete', () => { expect.anything() ); expect(result).toStrictEqual({ + rules: [returnedRule1, returnedRule2], errors: [], total: 2, taskIdsFailedToBeDeleted: [], @@ -345,7 +325,6 @@ describe('bulkDelete', () => { }); test('should throw an error if we do not get buckets', async () => { - mockCreatePointInTimeFinderAsInternalUser(); unsecuredSavedObjectsClient.find.mockResolvedValue({ aggregations: { alertTypeId: {}, @@ -363,7 +342,6 @@ describe('bulkDelete', () => { describe('taskManager', () => { test('should return task id if deleting task failed', async () => { - mockCreatePointInTimeFinderAsInternalUser(); unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({ statuses: [ { id: 'id1', type: 'alert', success: true }, @@ -373,12 +351,12 @@ describe('bulkDelete', () => { taskManager.bulkRemoveIfExist.mockImplementation(async () => ({ statuses: [ { - id: 'taskId1', + id: 'id1', type: 'alert', success: true, }, { - id: 'taskId2', + id: 'id2', type: 'alert', success: false, error: { @@ -394,16 +372,13 @@ describe('bulkDelete', () => { expect(logger.debug).toBeCalledTimes(1); expect(logger.debug).toBeCalledWith( - 'Successfully deleted schedules for underlying tasks: taskId1' + 'Successfully deleted schedules for underlying tasks: id1' ); expect(logger.error).toBeCalledTimes(1); - expect(logger.error).toBeCalledWith( - 'Failure to delete schedules for underlying tasks: taskId2' - ); + expect(logger.error).toBeCalledWith('Failure to delete schedules for underlying tasks: id2'); }); test('should not throw an error if taskManager throw an error', async () => { - mockCreatePointInTimeFinderAsInternalUser(); unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({ statuses: [ { id: 'id1', type: 'alert', success: true }, @@ -418,7 +393,7 @@ describe('bulkDelete', () => { expect(logger.error).toBeCalledTimes(1); expect(logger.error).toBeCalledWith( - 'Failure to delete schedules for underlying tasks: taskId1, taskId2. TaskManager bulkRemoveIfExist failed with Error: UPS' + 'Failure to delete schedules for underlying tasks: id1, id2. TaskManager bulkRemoveIfExist failed with Error: UPS' ); }); @@ -433,12 +408,12 @@ describe('bulkDelete', () => { taskManager.bulkRemoveIfExist.mockImplementation(async () => ({ statuses: [ { - id: 'taskId1', + id: 'id1', type: 'alert', success: true, }, { - id: 'taskId2', + id: 'id2', type: 'alert', success: true, }, @@ -449,7 +424,7 @@ describe('bulkDelete', () => { expect(logger.debug).toBeCalledTimes(1); expect(logger.debug).toBeCalledWith( - 'Successfully deleted schedules for underlying tasks: taskId1, taskId2' + 'Successfully deleted schedules for underlying tasks: id1, id2' ); expect(logger.error).toBeCalledTimes(0); }); @@ -459,7 +434,6 @@ describe('bulkDelete', () => { jest.spyOn(auditLogger, 'log').mockImplementation(); test('logs audit event when deleting rules', async () => { - mockCreatePointInTimeFinderAsInternalUser(); unsecuredSavedObjectsClient.bulkDelete.mockResolvedValue({ statuses: [ { id: 'id1', type: 'alert', success: true }, @@ -482,7 +456,6 @@ describe('bulkDelete', () => { }); test('logs audit event when authentication failed', async () => { - mockCreatePointInTimeFinderAsInternalUser(); authorization.ensureAuthorized.mockImplementation(() => { throw new Error('Unauthorized'); }); @@ -499,7 +472,6 @@ describe('bulkDelete', () => { }); test('logs audit event when getting an authorization filter failed', async () => { - mockCreatePointInTimeFinderAsInternalUser(); authorization.getFindAuthorizationFilter.mockImplementation(() => { throw new Error('Error'); }); diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_disable.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_disable.test.ts index 7024844c22879..88699a50195e6 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_disable.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_disable.test.ts @@ -21,16 +21,14 @@ import { loggerMock } from '@kbn/logging-mocks'; import { BulkUpdateTaskResult } from '@kbn/task-manager-plugin/server/task_scheduling'; import { eventLoggerMock } from '@kbn/event-log-plugin/server/mocks'; import { + disabledRule1, disabledRule2, enabledRule1, enabledRule2, savedObjectWith409Error, savedObjectWith500Error, - successfulSavedObject1, - successfulSavedObject2, - successfulSavedObjects, - updatedRule1, - updatedRule2, + returnedDisabledRule1, + returnedDisabledRule2, } from './test_helpers'; jest.mock('../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ @@ -130,7 +128,7 @@ describe('bulkDisableRules', () => { test('should disable two rule', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [disabledRule1, disabledRule2], }); const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -156,14 +154,14 @@ describe('bulkDisableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedDisabledRule1, returnedDisabledRule2], total: 2, }); }); test('should try to disable rules, one successful and one with 500 error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1, savedObjectWith500Error], + saved_objects: [disabledRule1, savedObjectWith500Error], }); const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -183,7 +181,7 @@ describe('bulkDisableRules', () => { expect(result).toStrictEqual({ errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 500 }], - rules: [updatedRule1], + rules: [returnedDisabledRule1], total: 2, }); }); @@ -191,7 +189,7 @@ describe('bulkDisableRules', () => { test('should try to disable rules, one successful and one with 409 error, which will not be deleted with retry', async () => { unsecuredSavedObjectsClient.bulkCreate .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject1, savedObjectWith409Error], + saved_objects: [disabledRule1, savedObjectWith409Error], }) .mockResolvedValueOnce({ saved_objects: [savedObjectWith409Error], @@ -228,7 +226,7 @@ describe('bulkDisableRules', () => { expect(taskManager.bulkDisable).toHaveBeenCalledWith(['id1']); expect(result).toStrictEqual({ errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 409 }], - rules: [updatedRule1], + rules: [returnedDisabledRule1], total: 2, }); }); @@ -236,10 +234,10 @@ describe('bulkDisableRules', () => { test('should try to disable rules, one successful and one with 409 error, which successfully will be disabled with retry', async () => { unsecuredSavedObjectsClient.bulkCreate .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject1, savedObjectWith409Error], + saved_objects: [disabledRule1, savedObjectWith409Error], }) .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject2], + saved_objects: [disabledRule2], }); encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest @@ -269,7 +267,7 @@ describe('bulkDisableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedDisabledRule1, returnedDisabledRule2], total: 2, }); }); @@ -313,7 +311,7 @@ describe('bulkDisableRules', () => { saved_objects: [enabledRule1, disabledRule2], }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [disabledRule1], }); const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -333,7 +331,7 @@ describe('bulkDisableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1], + rules: [returnedDisabledRule1], total: 2, }); }); @@ -341,7 +339,7 @@ describe('bulkDisableRules', () => { describe('taskManager', () => { test('should call task manager bulkDisable', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [disabledRule1, disabledRule2], }); taskManager.bulkDisable.mockResolvedValue({ @@ -375,14 +373,16 @@ describe('bulkDisableRules', () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { - ...successfulSavedObject1, + ...enabledRule1, attributes: { + ...enabledRule1.attributes, scheduledTaskId: 'taskId1', }, } as SavedObject, { - ...successfulSavedObject2, + ...enabledRule2, attributes: { + ...enabledRule1, scheduledTaskId: 'taskId2', }, } as SavedObject, @@ -411,7 +411,7 @@ describe('bulkDisableRules', () => { test('should disable one task if one rule was successfully disabled and one has 500 error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1, savedObjectWith500Error], + saved_objects: [disabledRule1, savedObjectWith500Error], }); taskManager.bulkDisable.mockResolvedValue({ @@ -442,7 +442,7 @@ describe('bulkDisableRules', () => { ], }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [disabledRule1], }); await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -453,7 +453,7 @@ describe('bulkDisableRules', () => { test('should not throw an error if taskManager.bulkDisable throw an error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [disabledRule1, disabledRule2], }); taskManager.bulkDisable.mockImplementation(() => { throw new Error('Something happend during bulkDisable'); @@ -471,8 +471,9 @@ describe('bulkDisableRules', () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { - ...successfulSavedObject1, + ...disabledRule1, attributes: { + ...disabledRule1.attributes, scheduledTaskId: 'taskId1', }, } as SavedObject, @@ -497,7 +498,7 @@ describe('bulkDisableRules', () => { test('logs audit event when disabling rules', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [disabledRule1], }); await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -561,7 +562,7 @@ describe('bulkDisableRules', () => { }); test('should call logEvent', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [disabledRule1, disabledRule2], }); await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); @@ -574,7 +575,7 @@ describe('bulkDisableRules', () => { throw new Error('UPS'); }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [disabledRule1, disabledRule2], }); await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_enable.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_enable.test.ts index 2d2901c6a06c2..a48db94bd13e4 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_enable.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_enable.test.ts @@ -21,14 +21,14 @@ import { BulkUpdateTaskResult } from '@kbn/task-manager-plugin/server/task_sched import { disabledRule1, disabledRule2, + disabledRuleWithAction1, + disabledRuleWithAction2, + enabledRule1, enabledRule2, savedObjectWith409Error, savedObjectWith500Error, - successfulSavedObject1, - successfulSavedObject2, - successfulSavedObjects, - updatedRule1, - updatedRule2, + returnedRule1, + returnedRule2, } from './test_helpers'; jest.mock('../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ @@ -126,7 +126,7 @@ describe('bulkEnableRules', () => { test('should enable two rule', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [enabledRule1, enabledRule2], }); const result = await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); @@ -152,7 +152,7 @@ describe('bulkEnableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedRule1, returnedRule2], total: 2, taskIdsFailedToBeEnabled: [], }); @@ -160,7 +160,7 @@ describe('bulkEnableRules', () => { test('should try to enable rules, one successful and one with 500 error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1, savedObjectWith500Error], + saved_objects: [enabledRule1, savedObjectWith500Error], }); const result = await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); @@ -180,7 +180,7 @@ describe('bulkEnableRules', () => { expect(result).toStrictEqual({ errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 500 }], - rules: [updatedRule1], + rules: [returnedRule1], total: 2, taskIdsFailedToBeEnabled: [], }); @@ -189,7 +189,7 @@ describe('bulkEnableRules', () => { test('should try to enable rules, one successful and one with 409 error, which will not be deleted with retry', async () => { unsecuredSavedObjectsClient.bulkCreate .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject1, savedObjectWith409Error], + saved_objects: [enabledRule1, savedObjectWith409Error], }) .mockResolvedValueOnce({ saved_objects: [savedObjectWith409Error], @@ -224,7 +224,7 @@ describe('bulkEnableRules', () => { expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(3); expect(result).toStrictEqual({ errors: [{ message: 'UPS', rule: { id: 'id2', name: 'fakeName' }, status: 409 }], - rules: [updatedRule1], + rules: [returnedRule1], total: 2, taskIdsFailedToBeEnabled: [], }); @@ -233,10 +233,10 @@ describe('bulkEnableRules', () => { test('should try to enable rules, one successful and one with 409 error, which successfully will be deleted with retry', async () => { unsecuredSavedObjectsClient.bulkCreate .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject1, savedObjectWith409Error], + saved_objects: [enabledRule1, savedObjectWith409Error], }) .mockResolvedValueOnce({ - saved_objects: [successfulSavedObject2], + saved_objects: [enabledRule2], }); encryptedSavedObjects.createPointInTimeFinderDecryptedAsInternalUser = jest @@ -265,13 +265,13 @@ describe('bulkEnableRules', () => { expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(2); expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedRule1, returnedRule2], total: 2, taskIdsFailedToBeEnabled: [], }); }); - test('should thow an error if number of matched rules greater than 10,000', async () => { + test('should throw an error if number of matched rules greater than 10,000', async () => { unsecuredSavedObjectsClient.find.mockResolvedValue({ aggregations: { alertTypeId: { @@ -305,7 +305,10 @@ describe('bulkEnableRules', () => { ); }); - test('should thow if there are actions, but do not have execute permissions', async () => { + test('should throw if there are actions, but do not have execute permissions', async () => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [disabledRuleWithAction1, disabledRuleWithAction2], + }); actionsAuthorization.ensureAuthorized.mockImplementation(() => { throw new Error('UPS'); }); @@ -339,7 +342,7 @@ describe('bulkEnableRules', () => { saved_objects: [disabledRule1, enabledRule2], }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [enabledRule1], }); const result = await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); @@ -359,7 +362,7 @@ describe('bulkEnableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1], + rules: [returnedRule1], total: 2, taskIdsFailedToBeEnabled: [], }); @@ -368,7 +371,7 @@ describe('bulkEnableRules', () => { describe('taskManager', () => { test('should return task id if deleting task failed', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [enabledRule1, enabledRule2], }); taskManager.bulkEnable.mockImplementation( async () => @@ -400,7 +403,7 @@ describe('bulkEnableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedRule1, returnedRule2], total: 2, taskIdsFailedToBeEnabled: ['id2'], }); @@ -408,7 +411,7 @@ describe('bulkEnableRules', () => { test('should not throw an error if taskManager throw an error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [enabledRule1, enabledRule2], }); taskManager.bulkEnable.mockImplementation(() => { throw new Error('UPS'); @@ -423,7 +426,7 @@ describe('bulkEnableRules', () => { expect(result).toStrictEqual({ errors: [], - rules: [updatedRule1, updatedRule2], + rules: [returnedRule1, returnedRule2], taskIdsFailedToBeEnabled: ['id1', 'id2'], total: 2, }); @@ -431,7 +434,7 @@ describe('bulkEnableRules', () => { test('should call task manager bulkEnable for two tasks', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: successfulSavedObjects, + saved_objects: [enabledRule1, enabledRule2], }); await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); @@ -442,7 +445,7 @@ describe('bulkEnableRules', () => { test('should should call task manager bulkEnable only for one task, if one rule have an error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1, savedObjectWith500Error], + saved_objects: [enabledRule1, savedObjectWith500Error], }); await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); @@ -456,7 +459,7 @@ describe('bulkEnableRules', () => { saved_objects: [disabledRule1, enabledRule2], }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [enabledRule1], }); taskManager.bulkEnable.mockImplementation( @@ -485,7 +488,7 @@ describe('bulkEnableRules', () => { test('logs audit event when enabling rules', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ - saved_objects: [successfulSavedObject1], + saved_objects: [enabledRule1], }); await rulesClient.bulkEnableRules({ filter: 'fake_filter' }); diff --git a/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts b/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts index 10c93aa1e88b4..0100242672be9 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts @@ -7,24 +7,6 @@ import type { SavedObject } from '@kbn/core-saved-objects-common'; -export const successfulSavedObject1 = { - id: 'id1', - version: '1', - attributes: { - scheduledTaskId: 'id1', - }, -} as SavedObject; - -export const successfulSavedObject2 = { - id: 'id2', - version: '1', - attributes: { - scheduledTaskId: 'id2', - }, -} as SavedObject; - -export const successfulSavedObjects = [successfulSavedObject1, successfulSavedObject2]; - export const savedObjectWith500Error = { id: 'id2', error: { @@ -53,16 +35,7 @@ export const defaultRule = { consumer: 'fakeConsumer', alertTypeId: 'fakeType', schedule: { interval: '5m' }, - actions: [ - { - group: 'default', - actionTypeId: '1', - actionRef: '1', - params: { - foo: true, - }, - }, - ], + actions: [] as unknown, }, references: [], version: '1', @@ -85,7 +58,7 @@ export const enabledRule2 = { ...defaultRule.attributes, enabled: true, scheduledTaskId: 'id2', - apiKey: Buffer.from('123:abc').toString('base64'), + apiKey: Buffer.from('321:abc').toString('base64'), }, }; @@ -94,7 +67,7 @@ export const disabledRule1 = { attributes: { ...defaultRule.attributes, enabled: false, - scheduledTaskId: 'id2', + scheduledTaskId: 'id1', apiKey: Buffer.from('123:abc').toString('base64'), }, }; @@ -110,33 +83,80 @@ export const disabledRule2 = { }, }; -export const rule2 = { - ...defaultRule, - id: 'id2', +export const disabledRuleWithAction1 = { + ...disabledRule1, attributes: { - ...defaultRule.attributes, - enabled: true, - scheduledTaskId: 'id2', - apiKey: Buffer.from('321:abc').toString('base64'), + ...disabledRule1.attributes, + actions: [ + { + group: 'default', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, + }, + }, + ], + }, +}; + +export const disabledRuleWithAction2 = { + ...disabledRule2, + attributes: { + ...disabledRule2.attributes, + actions: [ + { + group: 'default', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, + }, + }, + ], }, }; -export const updatedRule1 = { +export const returnedRule1 = { actions: [], + alertTypeId: 'fakeType', + apiKey: 'MTIzOmFiYw==', + consumer: 'fakeConsumer', + enabled: true, id: 'id1', + name: 'fakeName', notifyWhen: undefined, params: undefined, - schedule: undefined, - snoozeSchedule: [], + schedule: { + interval: '5m', + }, scheduledTaskId: 'id1', + snoozeSchedule: [], }; -export const updatedRule2 = { +export const returnedRule2 = { actions: [], + alertTypeId: 'fakeType', + apiKey: 'MzIxOmFiYw==', + consumer: 'fakeConsumer', + enabled: true, id: 'id2', + name: 'fakeName', notifyWhen: undefined, params: undefined, - schedule: undefined, - snoozeSchedule: [], + schedule: { + interval: '5m', + }, scheduledTaskId: 'id2', + snoozeSchedule: [], +}; + +export const returnedDisabledRule1 = { + ...returnedRule1, + enabled: false, +}; + +export const returnedDisabledRule2 = { + ...returnedRule2, + enabled: false, }; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts index 91de3084993ff..b69b62e4b4a40 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_delete.ts @@ -10,7 +10,67 @@ import { UserAtSpaceScenarios, SuperuserAtSpace1 } from '../../../scenarios'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; import { getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../../common/lib'; -const defaultSuccessfulResponse = { errors: [], total: 1, taskIdsFailedToBeDeleted: [] }; +const getDefaultRules = (response: any) => ({ + id: response.body.rules[0].id, + apiKey: response.body.rules[0].apiKey, + notifyWhen: 'onThrottleInterval', + enabled: true, + name: 'abc', + tags: ['foo'], + consumer: 'alertsFixture', + throttle: '1m', + alertTypeId: 'test.noop', + apiKeyOwner: response.body.rules[0].apiKeyOwner, + createdBy: 'elastic', + updatedBy: response.body.rules[0].updatedBy, + muteAll: false, + mutedInstanceIds: [], + schedule: { interval: '1m' }, + actions: [], + params: {}, + snoozeSchedule: [], + updatedAt: response.body.rules[0].updatedAt, + createdAt: response.body.rules[0].createdAt, + scheduledTaskId: response.body.rules[0].scheduledTaskId, + executionStatus: response.body.rules[0].executionStatus, + monitoring: response.body.rules[0].monitoring, + ...(response.body.rules[0].nextRun ? { nextRun: response.body.rules[0].nextRun } : {}), + ...(response.body.rules[0].lastRun ? { lastRun: response.body.rules[0].lastRun } : {}), +}); + +const getThreeRules = (response: any) => { + const rules: any[] = []; + for (let i = 0; i < 3; i++) { + rules.push({ + id: response.body.rules[i].id, + apiKey: response.body.rules[i].apiKey, + notifyWhen: 'onThrottleInterval', + enabled: true, + name: 'abc', + tags: ['multiple-rules-delete'], + consumer: 'alertsFixture', + throttle: '1m', + alertTypeId: 'test.noop', + apiKeyOwner: response.body.rules[i].apiKeyOwner, + createdBy: 'elastic', + updatedBy: response.body.rules[i].updatedBy, + muteAll: false, + mutedInstanceIds: [], + schedule: { interval: '1m' }, + actions: [], + params: {}, + snoozeSchedule: [], + updatedAt: response.body.rules[i].updatedAt, + createdAt: response.body.rules[i].createdAt, + scheduledTaskId: response.body.rules[i].scheduledTaskId, + executionStatus: response.body.rules[i].executionStatus, + monitoring: response.body.rules[i].monitoring, + ...(response.body.rules[i].nextRun ? { nextRun: response.body.rules[i].nextRun } : {}), + ...(response.body.rules[i].lastRun ? { lastRun: response.body.rules[i].lastRun } : {}), + }); + } + return rules; +}; // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext) => { @@ -76,7 +136,12 @@ export default ({ getService }: FtrProviderContext) => { case 'superuser at space1': case 'space_1_all at space1': case 'space_1_all_with_restricted_fixture at space1': - expect(response.body).to.eql(defaultSuccessfulResponse); + expect(response.body).to.eql({ + rules: [getDefaultRules(response)], + errors: [], + total: 1, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); try { await getScheduledTask(createdRule1.scheduled_task_id); @@ -147,7 +212,18 @@ export default ({ getService }: FtrProviderContext) => { break; case 'superuser at space1': case 'space_1_all_with_restricted_fixture at space1': - expect(response.body).to.eql(defaultSuccessfulResponse); + expect(response.body).to.eql({ + rules: [ + { + ...getDefaultRules(response), + alertTypeId: 'test.restricted-noop', + consumer: 'alertsRestrictedFixture', + }, + ], + errors: [], + total: 1, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); try { await getScheduledTask(createdRule1.scheduled_task_id); @@ -207,7 +283,12 @@ export default ({ getService }: FtrProviderContext) => { await getScheduledTask(createdRule1.scheduled_task_id); break; case 'superuser at space1': - expect(response.body).to.eql(defaultSuccessfulResponse); + expect(response.body).to.eql({ + rules: [{ ...getDefaultRules(response), alertTypeId: 'test.restricted-noop' }], + errors: [], + total: 1, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); try { await getScheduledTask(createdRule1.scheduled_task_id); @@ -267,7 +348,12 @@ export default ({ getService }: FtrProviderContext) => { case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': case 'space_1_all_with_restricted_fixture at space1': - expect(response.body).to.eql(defaultSuccessfulResponse); + expect(response.body).to.eql({ + rules: [{ ...getDefaultRules(response), consumer: 'alerts' }], + errors: [], + total: 1, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); try { await getScheduledTask(createdRule1.scheduled_task_id); @@ -287,7 +373,7 @@ export default ({ getService }: FtrProviderContext) => { supertest .post(`${getUrlPrefix(space.id)}/api/alerting/rule`) .set('kbn-xsrf', 'foo') - .send(getTestRuleData({ tags: ['multiple-rules-edit'] })) + .send(getTestRuleData({ tags: ['multiple-rules-delete'] })) .expect(200) ) ); @@ -332,7 +418,12 @@ export default ({ getService }: FtrProviderContext) => { case 'superuser at space1': case 'space_1_all at space1': case 'space_1_all_with_restricted_fixture at space1': - expect(response.body).to.eql({ ...defaultSuccessfulResponse, total: 3 }); + expect(response.body).to.eql({ + rules: getThreeRules(response), + errors: [], + total: 3, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); for (const rule of rules) { try { @@ -399,7 +490,12 @@ export default ({ getService }: FtrProviderContext) => { case 'superuser at space1': case 'space_1_all at space1': case 'space_1_all_with_restricted_fixture at space1': - expect(response.body).to.eql({ ...defaultSuccessfulResponse, total: 3 }); + expect(response.body).to.eql({ + rules: getThreeRules(response), + errors: [], + total: 3, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); for (const rule of rules) { try { @@ -431,7 +527,12 @@ export default ({ getService }: FtrProviderContext) => { switch (scenario.id) { // This superuser has more privileges that we think case 'superuser at space1': - expect(response.body).to.eql(defaultSuccessfulResponse); + expect(response.body).to.eql({ + rules: [getDefaultRules(response)], + errors: [], + total: 1, + taskIdsFailedToBeDeleted: [], + }); expect(response.statusCode).to.eql(200); try { await getScheduledTask(createdRule.scheduled_task_id); From f179279e5ce067450d80b70a8ad2581a7bd49422 Mon Sep 17 00:00:00 2001 From: Kevin Delemme Date: Wed, 7 Dec 2022 08:49:53 -0500 Subject: [PATCH 6/6] feat(slo): SLO selector component (#147010) --- .../kbn-storybook/src/lib/default_config.ts | 48 ++++++++++++- .../observability/common/data/sli_list.ts | 45 ++++++++++++ .../slo/slo_selector/slo_selector.stories.tsx | 25 +++++++ .../slo/slo_selector/slo_selector.test.tsx | 46 +++++++++++++ .../shared/slo/slo_selector/slo_selector.tsx | 68 +++++++++++++++++++ .../__storybook_mocks__/use_fetch_slo_list.ts | 16 +++++ .../hooks => hooks/slo}/use_fetch_slo_list.ts | 35 +++++++--- .../public/pages/slos/components/slo_list.tsx | 6 +- .../public/pages/slos/index.test.tsx | 6 +- .../public/pages/slos/mocks/slo_list.ts | 30 ++++++++ .../services/slo/slo_repository.test.ts | 26 ++++++- .../server/services/slo/slo_repository.ts | 8 ++- 12 files changed, 339 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugins/observability/common/data/sli_list.ts create mode 100644 x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.stories.tsx create mode 100644 x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.test.tsx create mode 100644 x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.tsx create mode 100644 x-pack/plugins/observability/public/hooks/slo/__storybook_mocks__/use_fetch_slo_list.ts rename x-pack/plugins/observability/public/{pages/slos/hooks => hooks/slo}/use_fetch_slo_list.ts (67%) diff --git a/packages/kbn-storybook/src/lib/default_config.ts b/packages/kbn-storybook/src/lib/default_config.ts index a2712d3d6f24e..4e497572fdfd8 100644 --- a/packages/kbn-storybook/src/lib/default_config.ts +++ b/packages/kbn-storybook/src/lib/default_config.ts @@ -7,12 +7,16 @@ */ import * as path from 'path'; +import fs from 'fs'; import type { StorybookConfig } from '@storybook/core-common'; -import { Configuration } from 'webpack'; +import webpack, { Configuration } from 'webpack'; import webpackMerge from 'webpack-merge'; import { REPO_ROOT } from './constants'; import { default as WebpackConfig } from '../webpack.config'; +const MOCKS_DIRECTORY = '__storybook_mocks__'; +const EXTENSIONS = ['.ts', '.js']; + export type { StorybookConfig }; const toPath = (_path: string) => path.join(REPO_ROOT, _path); @@ -52,6 +56,48 @@ export const defaultConfig: StorybookConfig = { config.cache = true; } + // This will go over every component which is imported and check its import statements. + // For every import which starts with ./ it will do a check to see if a file with the same name + // exists in the __storybook_mocks__ folder. If it does, use that import instead. + // This allows you to mock hooks and functions when rendering components in Storybook. + // It is akin to Jest's manual mocks (__mocks__). + config.plugins?.push( + new webpack.NormalModuleReplacementPlugin(/^\.\//, async (resource: any) => { + if (!resource.contextInfo.issuer?.includes('node_modules')) { + const mockedPath = path.resolve(resource.context, MOCKS_DIRECTORY, resource.request); + + EXTENSIONS.forEach((ext) => { + const isReplacementPathExists = fs.existsSync(mockedPath + ext); + + if (isReplacementPathExists) { + const newImportPath = './' + path.join(MOCKS_DIRECTORY, resource.request); + resource.request = newImportPath; + } + }); + } + }) + ); + + // Same, but for imports statements which import modules outside of the directory (../) + config.plugins?.push( + new webpack.NormalModuleReplacementPlugin(/^\.\.\//, async (resource: any) => { + if (!resource.contextInfo.issuer?.includes('node_modules')) { + const prs = path.parse(resource.request); + + const mockedPath = path.resolve(resource.context, prs.dir, MOCKS_DIRECTORY, prs.base); + + EXTENSIONS.forEach((ext) => { + const isReplacementPathExists = fs.existsSync(mockedPath + ext); + + if (isReplacementPathExists) { + const newImportPath = prs.dir + '/' + path.join(MOCKS_DIRECTORY, prs.base); + resource.request = newImportPath; + } + }); + } + }) + ); + config.node = { fs: 'empty' }; config.watch = true; config.watchOptions = { diff --git a/x-pack/plugins/observability/common/data/sli_list.ts b/x-pack/plugins/observability/common/data/sli_list.ts new file mode 100644 index 0000000000000..cc630700fcd79 --- /dev/null +++ b/x-pack/plugins/observability/common/data/sli_list.ts @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { SLOList } from '../../public'; + +export const emptySloList: SLOList = { + results: [], + page: 1, + perPage: 25, + total: 0, +}; + +export const sloList: SLOList = { + results: [ + { + id: '1f1c6ee7-433f-4b56-b727-5682262e0d7d', + name: 'latency', + objective: { target: 0.98 }, + summary: { + sliValue: 0.99872, + errorBudget: { + remaining: 0.936, + }, + }, + }, + { + id: 'c0f8d669-9177-4706-9098-f397a88173a6', + name: 'availability', + objective: { target: 0.98 }, + summary: { + sliValue: 0.97, + errorBudget: { + remaining: 0, + }, + }, + }, + ], + page: 1, + perPage: 25, + total: 2, +}; diff --git a/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.stories.tsx b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.stories.tsx new file mode 100644 index 0000000000000..0d1c6372c0513 --- /dev/null +++ b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.stories.tsx @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { ComponentStory } from '@storybook/react'; + +import { SLO } from '../../../../typings'; +import { SloSelector as Component } from './slo_selector'; + +export default { + component: Component, + title: 'app/SLOs/Shared/SloSelector', +}; + +const Template: ComponentStory = () => ( + console.log(slo)} /> +); +const defaultProps = {}; + +export const Default = Template.bind({}); +Default.args = defaultProps; diff --git a/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.test.tsx b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.test.tsx new file mode 100644 index 0000000000000..77da476c1ff16 --- /dev/null +++ b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.test.tsx @@ -0,0 +1,46 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { act, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { render } from '../../../../utils/test_helper'; +import { SloSelector } from './slo_selector'; +import { useFetchSloList } from '../../../../hooks/slo/use_fetch_slo_list'; +import { emptySloList } from '../../../../pages/slos/mocks/slo_list'; +import { wait } from '@testing-library/user-event/dist/utils'; + +jest.mock('../../../../hooks/slo/use_fetch_slo_list'); +const useFetchSloListMock = useFetchSloList as jest.Mock; + +describe('SLO Selector', () => { + const onSelectedSpy = jest.fn(); + beforeEach(() => { + jest.clearAllMocks(); + useFetchSloListMock.mockReturnValue({ loading: true, sloList: emptySloList }); + }); + + it('fetches SLOs asynchronously', async () => { + render(); + + expect(screen.getByTestId('sloSelector')).toBeTruthy(); + expect(useFetchSloListMock).toHaveBeenCalledWith(''); + }); + + it('searches SLOs when typing', async () => { + render(); + + const input = screen.getByTestId('comboBoxInput'); + await act(async () => { + await userEvent.type(input, 'latency', { delay: 1 }); + await wait(310); // debounce delay + }); + + expect(useFetchSloListMock).toHaveBeenCalledWith('latency'); + }); +}); diff --git a/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.tsx b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.tsx new file mode 100644 index 0000000000000..14e36da90fbbd --- /dev/null +++ b/x-pack/plugins/observability/public/components/shared/slo/slo_selector/slo_selector.tsx @@ -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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { EuiComboBox, EuiComboBoxOptionOption } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { debounce } from 'lodash'; +import React, { useEffect, useMemo, useState } from 'react'; + +import { SLO } from '../../../../typings'; +import { useFetchSloList } from '../../../../hooks/slo/use_fetch_slo_list'; + +interface Props { + onSelected: (slo: SLO) => void; +} + +function SloSelector({ onSelected }: Props) { + const [options, setOptions] = useState>>([]); + const [selectedOptions, setSelected] = useState>>(); + const [searchValue, setSearchValue] = useState(''); + const { loading, sloList } = useFetchSloList(searchValue); + + useEffect(() => { + const isLoadedWithData = !loading && sloList !== undefined; + const opts: Array> = isLoadedWithData + ? sloList.results.map((slo) => ({ value: slo.id, label: slo.name })) + : []; + setOptions(opts); + }, [loading, sloList]); + + const onChange = (opts: Array>) => { + setSelected(opts); + if (opts.length === 1) { + const sloId = opts[0].value; + const selectedSlo = sloList.results.find((slo) => slo.id === sloId); + if (selectedSlo !== undefined) { + onSelected(selectedSlo); + } + } + }; + + const onSearchChange = useMemo(() => debounce((value: string) => setSearchValue(value), 300), []); + + return ( + + ); +} + +export { SloSelector }; +export type { Props as SloSelectorProps }; diff --git a/x-pack/plugins/observability/public/hooks/slo/__storybook_mocks__/use_fetch_slo_list.ts b/x-pack/plugins/observability/public/hooks/slo/__storybook_mocks__/use_fetch_slo_list.ts new file mode 100644 index 0000000000000..47f39f55cf895 --- /dev/null +++ b/x-pack/plugins/observability/public/hooks/slo/__storybook_mocks__/use_fetch_slo_list.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { sloList } from '../../../../common/data/sli_list'; +import { UseFetchSloListResponse } from '../use_fetch_slo_list'; + +export const useFetchSloList = (name?: string): UseFetchSloListResponse => { + return { + loading: false, + sloList, + }; +}; diff --git a/x-pack/plugins/observability/public/pages/slos/hooks/use_fetch_slo_list.ts b/x-pack/plugins/observability/public/hooks/slo/use_fetch_slo_list.ts similarity index 67% rename from x-pack/plugins/observability/public/pages/slos/hooks/use_fetch_slo_list.ts rename to x-pack/plugins/observability/public/hooks/slo/use_fetch_slo_list.ts index 059635cb3dbbb..2a9c41d6d3dc4 100644 --- a/x-pack/plugins/observability/public/pages/slos/hooks/use_fetch_slo_list.ts +++ b/x-pack/plugins/observability/public/hooks/slo/use_fetch_slo_list.ts @@ -8,8 +8,8 @@ import { useCallback, useMemo } from 'react'; import { HttpSetup } from '@kbn/core/public'; -import type { SLO, SLOList } from '../../../typings/slo'; -import { useDataFetcher } from '../../../hooks/use_data_fetcher'; +import type { SLO, SLOList } from '../../typings/slo'; +import { useDataFetcher } from '../use_data_fetcher'; const EMPTY_LIST = { results: [], @@ -18,28 +18,42 @@ const EMPTY_LIST = { perPage: 0, }; -export const useFetchSloList = (): [boolean, SLOList] => { - const params = useMemo(() => ({}), []); - const shouldExecuteApiCall = useCallback((apiCallParams: {}) => true, []); +interface SLOListParams { + name?: string; +} + +interface UseFetchSloListResponse { + loading: boolean; + sloList: SLOList; +} - const { loading, data: sloList } = useDataFetcher<{}, SLOList>({ +const useFetchSloList = (name?: string): UseFetchSloListResponse => { + const params: SLOListParams = useMemo(() => ({ name }), [name]); + const shouldExecuteApiCall = useCallback( + (apiCallParams: SLOListParams) => apiCallParams.name === params.name, + [params] + ); + + const { loading, data: sloList } = useDataFetcher({ paramsForApiCall: params, initialDataState: EMPTY_LIST, executeApiCall: fetchSloList, shouldExecuteApiCall, }); - return [loading, sloList]; + return { loading, sloList }; }; const fetchSloList = async ( - params: {}, + params: SLOListParams, abortController: AbortController, http: HttpSetup ): Promise => { try { const response = await http.get>(`/api/observability/slos`, { - query: {}, + query: { + ...(params.name && { name: params.name }), + }, signal: abortController.signal, }); if (response !== undefined) { @@ -78,3 +92,6 @@ function toSLO(result: any): SLO { }, }; } + +export { useFetchSloList }; +export type { UseFetchSloListResponse }; diff --git a/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx b/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx index 2809d780c56d3..93f0f4d24e111 100644 --- a/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx +++ b/x-pack/plugins/observability/public/pages/slos/components/slo_list.tsx @@ -8,14 +8,14 @@ import React from 'react'; import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { useFetchSloList } from '../hooks/use_fetch_slo_list'; +import { useFetchSloList } from '../../../hooks/slo/use_fetch_slo_list'; export function SloList() { - const [isLoading, sloList] = useFetchSloList(); + const { loading, sloList } = useFetchSloList(); return ( - {!isLoading && 

{JSON.stringify(sloList, null, 2)}
} + {!loading && 
{JSON.stringify(sloList, null, 2)}
} ); } diff --git a/x-pack/plugins/observability/public/pages/slos/index.test.tsx b/x-pack/plugins/observability/public/pages/slos/index.test.tsx index 1702cee84311b..2a4bcbbe31c03 100644 --- a/x-pack/plugins/observability/public/pages/slos/index.test.tsx +++ b/x-pack/plugins/observability/public/pages/slos/index.test.tsx @@ -14,14 +14,14 @@ import { useKibana } from '../../utils/kibana_react'; import { kibanaStartMock } from '../../utils/kibana_react.mock'; import { render } from '../../utils/test_helper'; import { SlosPage } from '.'; -import { useFetchSloList } from './hooks/use_fetch_slo_list'; +import { useFetchSloList } from '../../hooks/slo/use_fetch_slo_list'; import { emptySloList } from './mocks/slo_list'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), useParams: jest.fn(), })); -jest.mock('./hooks/use_fetch_slo_list'); +jest.mock('../../hooks/slo/use_fetch_slo_list'); jest.mock('../../utils/kibana_react'); jest.mock('../../hooks/use_breadcrumbs'); @@ -50,7 +50,7 @@ describe('SLOs Page', () => { beforeEach(() => { jest.clearAllMocks(); mockKibana(); - useFetchSloListMock.mockReturnValue([false, emptySloList]); + useFetchSloListMock.mockReturnValue({ loading: false, sloList: emptySloList }); }); it('renders the not found page when the feature flag is not enabled', async () => { diff --git a/x-pack/plugins/observability/public/pages/slos/mocks/slo_list.ts b/x-pack/plugins/observability/public/pages/slos/mocks/slo_list.ts index c2a706057ad90..ff54f0e0a05f7 100644 --- a/x-pack/plugins/observability/public/pages/slos/mocks/slo_list.ts +++ b/x-pack/plugins/observability/public/pages/slos/mocks/slo_list.ts @@ -13,3 +13,33 @@ export const emptySloList: SLOList = { perPage: 25, total: 0, }; + +export const sloList: SLOList = { + results: [ + { + id: '1f1c6ee7-433f-4b56-b727-5682262e0d7d', + name: 'latency', + objective: { target: 0.98 }, + summary: { + sliValue: 0.99872, + errorBudget: { + remaining: 0.936, + }, + }, + }, + { + id: 'c0f8d669-9177-4706-9098-f397a88173a6', + name: 'availability', + objective: { target: 0.98 }, + summary: { + sliValue: 0.97, + errorBudget: { + remaining: 0, + }, + }, + }, + ], + page: 1, + perPage: 25, + total: 2, +}; diff --git a/x-pack/plugins/observability/server/services/slo/slo_repository.test.ts b/x-pack/plugins/observability/server/services/slo/slo_repository.test.ts index a32a97f1f1479..f34b26fc92ccc 100644 --- a/x-pack/plugins/observability/server/services/slo/slo_repository.test.ts +++ b/x-pack/plugins/observability/server/services/slo/slo_repository.test.ts @@ -101,11 +101,11 @@ describe('KibanaSavedObjectsSLORepository', () => { describe('find', () => { const DEFAULT_PAGINATION = { page: 1, perPage: 25 }; - it('includes the filter on name when provided', async () => { + it('includes the filter on name with wildcard when provided', async () => { const repository = new KibanaSavedObjectsSLORepository(soClientMock); soClientMock.find.mockResolvedValueOnce(aFindResponse(SOME_SLO)); - const result = await repository.find({ name: 'availability' }, DEFAULT_PAGINATION); + const result = await repository.find({ name: 'availability*' }, DEFAULT_PAGINATION); expect(result).toEqual({ page: 1, @@ -117,7 +117,27 @@ describe('KibanaSavedObjectsSLORepository', () => { type: SO_SLO_TYPE, page: 1, perPage: 25, - filter: `slo.attributes.name: availability`, + filter: `slo.attributes.name: availability*`, + }); + }); + + it('includes the filter on name with added wildcard when not provided', async () => { + const repository = new KibanaSavedObjectsSLORepository(soClientMock); + soClientMock.find.mockResolvedValueOnce(aFindResponse(SOME_SLO)); + + const result = await repository.find({ name: 'availa' }, DEFAULT_PAGINATION); + + expect(result).toEqual({ + page: 1, + perPage: 25, + total: 1, + results: [SOME_SLO], + }); + expect(soClientMock.find).toHaveBeenCalledWith({ + type: SO_SLO_TYPE, + page: 1, + perPage: 25, + filter: `slo.attributes.name: availa*`, }); }); diff --git a/x-pack/plugins/observability/server/services/slo/slo_repository.ts b/x-pack/plugins/observability/server/services/slo/slo_repository.ts index 12cd3be1c28d1..79edbce1fc751 100644 --- a/x-pack/plugins/observability/server/services/slo/slo_repository.ts +++ b/x-pack/plugins/observability/server/services/slo/slo_repository.ts @@ -96,7 +96,7 @@ export class KibanaSavedObjectsSLORepository implements SLORepository { function buildFilterKuery(criteria: Criteria): string | undefined { const filters: string[] = []; if (!!criteria.name) { - filters.push(`slo.attributes.name: ${criteria.name}`); + filters.push(`slo.attributes.name: ${addWildcardIfAbsent(criteria.name)}`); } return filters.length > 0 ? filters.join(' and ') : undefined; } @@ -113,3 +113,9 @@ function toSLO(storedSLO: StoredSLO): SLO { }, t.identity) ); } + +const WILDCARD_CHAR = '*'; +function addWildcardIfAbsent(value: string): string { + if (value.substring(value.length - 1) === WILDCARD_CHAR) return value; + return `${value}${WILDCARD_CHAR}`; +}