From 104aa8acb135ae0855d1ef3799933967cfdd03ae Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Thu, 19 Dec 2024 11:40:05 -0800 Subject: [PATCH] adding feature direction and moving suppression rules to each feature Signed-off-by: Amit Galitzky --- .../FormattedFormRow/FormattedFormRow.tsx | 29 +- public/models/types.ts | 16 +- .../AdvancedSettings/AdvancedSettings.tsx | 197 +- .../__tests__/AdvancedSettings.test.tsx | 74 - .../AggregationSelector.tsx | 10 +- .../AggregationSelector.test.tsx.snap | 19 +- .../FeatureAccordion/FeatureAccordion.tsx | 97 +- .../components/FeatureAccordion/styles.scss | 5 + .../components/Features/Features.tsx | 6 + .../SuppressionRules/SuppressionRules.tsx | 395 ++ .../__tests__/SuppressionRules.test.tsx | 113 + .../components/SuppressionRules/index.ts | 12 + .../containers/ConfigureModel.tsx | 53 +- .../ConfigureModel.test.tsx.snap | 584 +- .../pages/ConfigureModel/models/interfaces.ts | 3 +- .../pages/ConfigureModel/utils/constants.tsx | 6 + public/pages/ConfigureModel/utils/helpers.ts | 298 +- .../AdditionalSettings/AdditionalSettings.tsx | 37 +- .../DetectorConfig/containers/Features.tsx | 55 + .../__tests__/DetectorConfig.test.tsx | 4 +- .../containers/__tests__/Features.test.tsx | 221 + .../DetectorConfig.test.tsx.snap | 448 +- .../__snapshots__/Features.test.tsx.snap | 5913 +++++++++++++++++ .../AdditionalSettings/AdditionalSettings.tsx | 42 +- .../ModelConfigurationFields.tsx | 65 +- .../SuppressionRulesModal.tsx | 0 .../components/SuppressionRulesModal/index.ts | 12 + .../__tests__/AdditionalSettings.test.tsx | 41 - .../ModelConfigurationFields.test.tsx | 8 +- .../AdditionalSettings.test.tsx.snap | 86 - .../ModelConfigurationFields.test.tsx.snap | 134 +- .../ReviewAndCreate.test.tsx.snap | 112 +- public/utils/utils.tsx | 1 - 33 files changed, 8226 insertions(+), 870 deletions(-) delete mode 100644 public/pages/ConfigureModel/components/AdvancedSettings/__tests__/AdvancedSettings.test.tsx create mode 100644 public/pages/ConfigureModel/components/SuppressionRules/SuppressionRules.tsx create mode 100644 public/pages/ConfigureModel/components/SuppressionRules/__tests__/SuppressionRules.test.tsx create mode 100644 public/pages/ConfigureModel/components/SuppressionRules/index.ts create mode 100644 public/pages/DetectorConfig/containers/__tests__/Features.test.tsx create mode 100644 public/pages/DetectorConfig/containers/__tests__/__snapshots__/Features.test.tsx.snap rename public/pages/ReviewAndCreate/components/{AdditionalSettings => SuppressionRulesModal}/SuppressionRulesModal.tsx (100%) create mode 100644 public/pages/ReviewAndCreate/components/SuppressionRulesModal/index.ts diff --git a/public/components/FormattedFormRow/FormattedFormRow.tsx b/public/components/FormattedFormRow/FormattedFormRow.tsx index 043e493e..2d8da226 100644 --- a/public/components/FormattedFormRow/FormattedFormRow.tsx +++ b/public/components/FormattedFormRow/FormattedFormRow.tsx @@ -10,7 +10,7 @@ */ import React, { ReactElement, ReactNode } from 'react'; -import { EuiCompressedFormRow, EuiText, EuiLink, EuiIcon } from '@elastic/eui'; +import { EuiCompressedFormRow, EuiText, EuiLink, EuiIcon, EuiToolTip } from '@elastic/eui'; type FormattedFormRowProps = { title?: string; @@ -22,26 +22,25 @@ type FormattedFormRowProps = { fullWidth?: boolean; helpText?: string; hintLink?: string; + linkToolTip?: boolean; }; export const FormattedFormRow = (props: FormattedFormRowProps) => { - let hints; - if (props.hint) { - const hintTexts = Array.isArray(props.hint) ? props.hint : [props.hint]; - hints = hintTexts.map((hint, i) => { - return ( + const hints = props.hint + ? (Array.isArray(props.hint) ? props.hint : [props.hint]).map((hint, i) => ( {hint} - {props.hintLink ? ' ' : null} - {props.hintLink ? ( - - Learn more - - ) : null} + {props.hintLink && ( + <> + {' '} + + Learn more + + + )} - ); - }); - } + )) + : null; const { formattedTitle, ...euiFormRowProps } = props; diff --git a/public/models/types.ts b/public/models/types.ts index 866396cf..4c200413 100644 --- a/public/models/types.ts +++ b/public/models/types.ts @@ -92,6 +92,18 @@ export enum ThresholdType { * (b-a)/|a| is less than or equal to ignoreNearExpectedFromBelowByRatio. */ EXPECTED_OVER_ACTUAL_RATIO = "EXPECTED_OVER_ACTUAL_RATIO", + + /** + * Specifies a threshold for ignoring anomalies based on whether the actual value + * is over the expected value returned from the model. + */ + ACTUAL_IS_OVER_EXPECTED = "ACTUAL_IS_OVER_EXPECTED", + + /** + * Specifies a threshold for ignoring anomalies based on whether the actual value + * is below the expected value returned from the model. + * */ + ACTUAL_IS_BELOW_EXPECTED = "ACTUAL_IS_BELOW_EXPECTED", } // Method to get the description of ThresholdType @@ -113,7 +125,7 @@ export interface Rule { export interface Condition { featureName: string; thresholdType: ThresholdType; - operator: Operator; - value: number; + operator?: Operator; + value?: number; } diff --git a/public/pages/ConfigureModel/components/AdvancedSettings/AdvancedSettings.tsx b/public/pages/ConfigureModel/components/AdvancedSettings/AdvancedSettings.tsx index 3cae536e..2a9b37f6 100644 --- a/public/pages/ConfigureModel/components/AdvancedSettings/AdvancedSettings.tsx +++ b/public/pages/ConfigureModel/components/AdvancedSettings/AdvancedSettings.tsx @@ -21,6 +21,7 @@ import { EuiButtonIcon, EuiCompressedFieldText, EuiToolTip, + EuiButtonEmpty, } from '@elastic/eui'; import { Field, FieldProps, FieldArray } from 'formik'; import React, { useEffect, useState } from 'react'; @@ -48,47 +49,7 @@ export function AdvancedSettings(props: AdvancedSettingsProps) { { value: SparseDataOptionValue.SET_TO_ZERO, text: 'Set to zero' }, { value: SparseDataOptionValue.CUSTOM_VALUE, text: 'Custom value' }, ]; - - const aboveBelowOptions = [ - { value: 'above', text: 'above' }, - { value: 'below', text: 'below' }, - ]; - - function extractArrayError(fieldName: string, form: any): string { - const error = form.errors[fieldName]; - console.log('Error for field:', fieldName, error); // Log the error for debugging - - // Check if the error is an array with objects inside - if (Array.isArray(error) && error.length > 0) { - // Iterate through the array to find the first non-empty error message - for (const err of error) { - if (typeof err === 'object' && err !== null) { - const entry = Object.entries(err).find( - ([_, fieldError]) => fieldError - ); // Find the first entry with a non-empty error message - if (entry) { - const [fieldKey, fieldError] = entry; - - // Replace fieldKey with a more user-friendly name if it matches specific fields - const friendlyFieldName = - fieldKey === 'absoluteThreshold' - ? 'absolute threshold' - : fieldKey === 'relativeThreshold' - ? 'relative threshold' - : fieldKey; // Use the original fieldKey if no match - - return typeof fieldError === 'string' - ? `${friendlyFieldName} ${fieldError.toLowerCase()}` // Format the error message with the friendly field name - : String(fieldError || ''); - } - } - } - } - - // Default case to handle other types of errors - return typeof error === 'string' ? error : String(error || ''); - } - + return ( - - - - {(arrayHelpers) => ( - <> - - {({ field, form }: FieldProps) => ( - <> - - {/* Controls the width of the whole row as FormattedFormRow does not allow that. Otherwise, our row is too packed. */} - - - <> - {form.values.suppressionRules?.map( - (rule, index) => ( - - - - Ignore anomalies for the feature - - - - - {({ field }: FieldProps) => ( - - )} - - - - - when the actual value is no more than - - - - - - {({ field }: FieldProps) => ( - - )} - - - - - or - - - - - {({ field }: FieldProps) => ( -
- - % -
- )} - - - - - - - {({ field }: FieldProps) => ( - - )} - - - - - - the expected value. - - - - - arrayHelpers.remove(index) - } - /> - - - ) - )} - - - - - - )} - - - - arrayHelpers.push({ - fieldName: '', - absoluteThreshold: null, // Set to null to allow empty inputs - relativeThreshold: null, // Set to null to allow empty inputs - aboveBelow: 'above', - }) - } - aria-label="Add rule" - /> - - )} - ) : null} diff --git a/public/pages/ConfigureModel/components/AdvancedSettings/__tests__/AdvancedSettings.test.tsx b/public/pages/ConfigureModel/components/AdvancedSettings/__tests__/AdvancedSettings.test.tsx deleted file mode 100644 index 0d4853ff..00000000 --- a/public/pages/ConfigureModel/components/AdvancedSettings/__tests__/AdvancedSettings.test.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import React from 'react'; -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import { Formik } from 'formik'; -import { AdvancedSettings } from '../AdvancedSettings'; // Adjust the path as necessary - -describe('AdvancedSettings Component', () => { - test('displays error when -1 is entered in suppression rules absolute threshold', async () => { - render( - - {() => } - - ); - - // Open the advanced settings - userEvent.click(screen.getByText('Show')); - - screen.logTestingPlaygroundURL(); - - // Click to add a new suppression rule - const addButton = screen.getByRole('button', { name: /add rule/i }); - fireEvent.click(addButton); - - // Find the absolute threshold input and type -1 - const absoluteThresholdInput = screen.getAllByPlaceholderText('Absolute')[0]; // Select the first absolute threshold input - userEvent.type(absoluteThresholdInput, '-1'); - - // Trigger validation - fireEvent.blur(absoluteThresholdInput); - - // Wait for the error message to appear - await waitFor(() => { - expect(screen.getByText('absolute threshold must be a positive number greater than zero')).toBeInTheDocument(); - }); - }); - test('displays error when -1 is entered in suppression rules relative threshold', async () => { - render( - - {() => } - - ); - - // Open the advanced settings - userEvent.click(screen.getByText('Show')); - - screen.logTestingPlaygroundURL(); - - // Click to add a new suppression rule - const addButton = screen.getByRole('button', { name: /add rule/i }); - fireEvent.click(addButton); - - // Find the relative threshold input and type -1 - const relativeThresholdInput = screen.getAllByPlaceholderText('Relative')[0]; // Select the first absolute threshold input - userEvent.type(relativeThresholdInput, '-1'); - - // Trigger validation - fireEvent.blur(relativeThresholdInput); - - // Wait for the error message to appear - await waitFor(() => { - expect(screen.getByText('relative threshold must be a positive number greater than zero')).toBeInTheDocument(); - }); - }); -}); diff --git a/public/pages/ConfigureModel/components/AggregationSelector/AggregationSelector.tsx b/public/pages/ConfigureModel/components/AggregationSelector/AggregationSelector.tsx index 01144c9d..9cbdb607 100644 --- a/public/pages/ConfigureModel/components/AggregationSelector/AggregationSelector.tsx +++ b/public/pages/ConfigureModel/components/AggregationSelector/AggregationSelector.tsx @@ -26,6 +26,7 @@ import { isInvalid, getError, } from '../../../../utils/utils'; +import { FormattedFormRow } from '../../../../components/FormattedFormRow/FormattedFormRow'; interface AggregationSelectorProps { index?: number; @@ -41,9 +42,10 @@ export const AggregationSelector = (props: AggregationSelectorProps) => { validate={requiredSelectField} > {({ field, form }: FieldProps) => ( - @@ -71,7 +73,7 @@ export const AggregationSelector = (props: AggregationSelectorProps) => { }} data-test-subj="aggregationType" /> - + )} diff --git a/public/pages/ConfigureModel/components/AggregationSelector/__tests__/__snapshots__/AggregationSelector.test.tsx.snap b/public/pages/ConfigureModel/components/AggregationSelector/__tests__/__snapshots__/AggregationSelector.test.tsx.snap index 84f31015..27d1a096 100644 --- a/public/pages/ConfigureModel/components/AggregationSelector/__tests__/__snapshots__/AggregationSelector.test.tsx.snap +++ b/public/pages/ConfigureModel/components/AggregationSelector/__tests__/__snapshots__/AggregationSelector.test.tsx.snap @@ -4,7 +4,9 @@ exports[` spec Empty results renders component with aggre
spec Empty results renders component with aggre class="euiFormLabel euiFormRow__label" for="random_html_id" > - Aggregation method +
+

+ Aggregation method +

+
+
+ The aggregation method determines what constitutes an anomaly. +
+
spec Empty results renders component with aggre class="euiFormHelpText euiFormRow__text" id="random_html_id-help-0" > - The aggregation method determines what constitutes an anomaly. For example, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature. + E.g, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature.
diff --git a/public/pages/ConfigureModel/components/FeatureAccordion/FeatureAccordion.tsx b/public/pages/ConfigureModel/components/FeatureAccordion/FeatureAccordion.tsx index 53fd616c..b77147d3 100644 --- a/public/pages/ConfigureModel/components/FeatureAccordion/FeatureAccordion.tsx +++ b/public/pages/ConfigureModel/components/FeatureAccordion/FeatureAccordion.tsx @@ -21,9 +21,10 @@ import { EuiCompressedFieldText, EuiCompressedCheckbox, EuiButtonIcon, + EuiSpacer, } from '@elastic/eui'; import './styles.scss'; -import { Field, FieldProps } from 'formik'; +import { Field, FieldArray, FieldProps } from 'formik'; import { required, isInvalid, @@ -31,11 +32,16 @@ import { validateFeatureName, } from '../../../../utils/utils'; import { get } from 'lodash'; -import { FEATURE_TYPE_OPTIONS } from '../../utils/constants'; +import { + FEATURE_DIRECTION_OPTIONS, + FEATURE_TYPE_OPTIONS, +} from '../../utils/constants'; import { FEATURE_TYPE } from '../../../../models/interfaces'; import { formikToSimpleAggregation } from '../../utils/helpers'; import { AggregationSelector } from '../AggregationSelector'; import { CustomAggregation } from '../CustomAggregation'; +import { FormattedFormRow } from '../../../../components/FormattedFormRow/FormattedFormRow'; +import { SuppressionRules } from '../SuppressionRules/SuppressionRules'; interface FeatureAccordionProps { onDelete(): void; @@ -109,11 +115,43 @@ export const FeatureAccordion = (props: FeatureAccordionProps) => { ); }; + const handleAnomalyCriteriaSelection = (value, form, featureIndex) => { + const featureName = form.values.featureList[props.index]?.featureName || ''; + + const updatedSuppressionRules = [...(form.values.suppressionRules || [])]; + + if (!updatedSuppressionRules[featureIndex]) { + updatedSuppressionRules[featureIndex] = []; + } + + if (value === 'both') { + // Remove only directionRule for the feature + updatedSuppressionRules[featureIndex] = updatedSuppressionRules[ + featureIndex + ].filter((rule) => !rule.directionRule); + } else { + const newRule = { + featureName: featureName, + absoluteThreshold: 0, + relativeThreshold: 0, + aboveBelow: value, + directionRule: true, + }; + + updatedSuppressionRules[featureIndex] = updatedSuppressionRules[ + featureIndex + ].filter((rule) => !rule.directionRule); + + updatedSuppressionRules[featureIndex].push(newRule); + } + form.setFieldValue('suppressionRules', updatedSuppressionRules); + }; + const deleteAction = (onClick: any) => { if (props.displayMode === 'flyout') { return ( { {({ field, form }: FieldProps) => ( @@ -237,6 +275,57 @@ export const FeatureAccordion = (props: FeatureAccordionProps) => { )} + + {({ field, form }: FieldProps) => ( + + { + const rules = + form.values.suppressionRules?.[props.index]?.filter( + (rule) => rule.directionRule === true + ) || []; + return rules.length > 0 && + typeof rules[0]?.aboveBelow === 'string' + ? rules[0].aboveBelow + : 'both'; + })()} + options={FEATURE_DIRECTION_OPTIONS} + onChange={(e) => { + const selectedValue = e.target.value; + handleAnomalyCriteriaSelection( + selectedValue, + form, + props.index + ); + }} + /> + + )} + + + + +

+ Customize Suppression Rules +

+ + + } + > + + ); }; diff --git a/public/pages/ConfigureModel/components/FeatureAccordion/styles.scss b/public/pages/ConfigureModel/components/FeatureAccordion/styles.scss index 5d819b8a..f2782119 100644 --- a/public/pages/ConfigureModel/components/FeatureAccordion/styles.scss +++ b/public/pages/ConfigureModel/components/FeatureAccordion/styles.scss @@ -1,3 +1,8 @@ .euiFormAccordion_button { padding: 20px 16px 0 0; } + +.euiSuppresionAccordion .euiAccordion__iconWrapper { + font-size: 14.5px !important; + margin-right: 2px; +} \ No newline at end of file diff --git a/public/pages/ConfigureModel/components/Features/Features.tsx b/public/pages/ConfigureModel/components/Features/Features.tsx index af735802..86d9b73e 100644 --- a/public/pages/ConfigureModel/components/Features/Features.tsx +++ b/public/pages/ConfigureModel/components/Features/Features.tsx @@ -69,10 +69,16 @@ export function Features(props: FeaturesProps) { { remove(index); + // delete any leftover suppressionRules as well + const updatedSuppressionRules = props.formikProps.values.suppressionRules.filter( + (_, i) => i !== index + ); + props.formikProps.setFieldValue('suppressionRules', updatedSuppressionRules); }} index={index} feature={feature} handleChange={props.formikProps.handleChange} + rules={props} /> ))} 0 + ) { + for (const arr of suppressionRulesErrors) { + if (Array.isArray(arr) && arr.length > 0) { + for (const err of arr) { + if (typeof err === 'object' && err !== null) { + const entry = Object.entries(err).find( + ([_, fieldError]) => fieldError + ); // Find the first entry with a non-empty error message + if (entry) { + const [fieldKey, fieldError] = entry; + // Replace fieldKey with a more user-friendly name if it matches specific fields + const friendlyFieldName = + fieldKey === 'absoluteThreshold' + ? 'absolute threshold' + : fieldKey === 'relativeThreshold' + ? 'relative threshold' + : fieldKey; + return typeof fieldError === 'string' + ? `${friendlyFieldName} ${fieldError.toLowerCase()}` + : String(fieldError || ''); + } + } + } + } + } + } + return typeof suppressionRulesErrors === 'string' + ? suppressionRulesErrors + : String(suppressionRulesErrors || ''); + } + + return ( + + {(arrayHelpers) => ( + <> + + {({ field, form }: FieldProps) => { + const featureSuppressionRules = + form.values.suppressionRules?.[props.featureIndex] || []; + return ( + <> + + + + <> + + + +

+ Ignore anomalies when the actual value is no more + than: +

+ + {featureSuppressionRules?.map((rule, index) => { + if (rule.directionRule) { + return null; + } + const isPercentage = + rule.isPercentage !== undefined + ? rule.isPercentage + : true; + return ( + + + + {({ field, form }: FieldProps) => { + const currentFeatureName = + props.feature.featureName || ''; + React.useEffect(() => { + if ( + field.value !== currentFeatureName + ) { + form.setFieldValue( + field.name, + currentFeatureName + ); + } + }, [ + field.name, + field.value, + currentFeatureName, + form, + ]); + + return ( + + ); + }} + + +
+ + {({ field }: FieldProps) => { + return ( + { + field.onBlur(e); + form.setFieldTouched( + field.name, + true, + true + ); + }} + value={ + isPercentage + ? form.values + .suppressionRules[ + props.featureIndex + ][index] + .relativeThreshold ?? '' + : form.values + .suppressionRules[ + props.featureIndex + ][index] + .absoluteThreshold ?? '' + } + onChange={(e) => { + const value = e.target.value; + form.setFieldValue( + `suppressionRules.${ + props.featureIndex + }[${index}].${ + isPercentage + ? 'relativeThreshold' + : 'absoluteThreshold' + }`, + value + ? parseFloat(value) + : null + ); + }} + append={ + { + const newValue = + e.target.value; + const currentValue = + form.values + .suppressionRules[ + props.featureIndex + ][index][ + newValue === + 'percentage' + ? 'absoluteThreshold' + : 'relativeThreshold' + ]; + + // Update isPercentage + form.setFieldValue( + `suppressionRules.${props.featureIndex}[${index}].isPercentage`, + newValue === 'percentage' + ); + + // Transfer the current value to the correct field + form.setFieldValue( + `suppressionRules.${ + props.featureIndex + }[${index}].${ + newValue === + 'percentage' + ? 'relativeThreshold' + : 'absoluteThreshold' + }`, + currentValue !== + undefined && + currentValue !== null + ? parseFloat( + currentValue + ) + : null + ); + + // Clear the old field + form.setFieldValue( + `suppressionRules.${ + props.featureIndex + }[${index}].${ + newValue === + 'percentage' + ? 'absoluteThreshold' + : 'relativeThreshold' + }`, + null + ); + }} + /> + } + /> + ); + }} + +
+ + + + + {({ field }: FieldProps) => ( + + + + )} + + + + { + if ( + form.values.suppressionRules[ + props.featureIndex + ].length === 1 + ) { + arrayHelpers.remove(index); + const cleanedSuppressionRules = + form.values.suppressionRules.filter( + (_, i) => i === props.featureIndex + ); + form.setFieldValue( + `suppressionRules.`, + cleanedSuppressionRules + ); + } else { + arrayHelpers.remove(index); + } + }} + /> + + + ); + })} + + + + + + ); + }} + + + { + arrayHelpers.push({ + featureName: props.feature.featureName, + absoluteThreshold: null, // Set to null to allow empty inputs + relativeThreshold: null, // Set to null to allow empty inputs + aboveBelow: 'above', + directionRule: false, + }); + }} + aria-label="Add rule" + style={{ marginTop: '5px' }} + > + Add suppression rule + + + )} + + ); +} diff --git a/public/pages/ConfigureModel/components/SuppressionRules/__tests__/SuppressionRules.test.tsx b/public/pages/ConfigureModel/components/SuppressionRules/__tests__/SuppressionRules.test.tsx new file mode 100644 index 00000000..fbc61aec --- /dev/null +++ b/public/pages/ConfigureModel/components/SuppressionRules/__tests__/SuppressionRules.test.tsx @@ -0,0 +1,113 @@ +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { Formik } from 'formik'; +import { SuppressionRules } from '../SuppressionRules'; + +export const testFeature = { + featureId: '42a50483-59cd-4470-a65e-e85547e1a173', + featureName: 'f1', + featureType: 'simple_aggs', + featureEnabled: true, + importance: 1, + aggregationBy: 'sum', + newFeature: true, + aggregationOf: [ + { + label: 'items_purchased_failure', + type: 'number', + }, + ], +}; + +describe('SuppressionRules Component', () => { + test('displays error when -1 is entered in suppression rules absolute threshold', async () => { + render( + + {() => } + + ); + + screen.logTestingPlaygroundURL(); + + // Click to add a new suppression rule + const addButton = screen.getByRole('button', { name: /add rule/i }); + fireEvent.click(addButton); + + // Find the threshold input and type -1 + const thresholdInput = screen.getAllByPlaceholderText('Threshold')[0]; + userEvent.type(thresholdInput, '-1'); + + // Find the dropdown using the data-test-subj attribute + const unitDropdown = screen.getByTestId('thresholdType-dropdown-0-0'); + expect(unitDropdown).toBeInTheDocument(); + + fireEvent.change(unitDropdown, { target: { value: 'units' } }); + + // Trigger validation + fireEvent.blur(thresholdInput); + + // Wait for the error message to appear + await waitFor(() => { + expect( + screen.getByText( + 'absolute threshold must be a positive number greater than zero' + ) + ).toBeInTheDocument(); + }); + }); + test('displays error when -1 is entered in suppression rules relative threshold', async () => { + render( + + {() => } + + ); + + screen.logTestingPlaygroundURL(); + + // Click to add a new suppression rule + const addButton = screen.getByRole('button', { name: /add rule/i }); + fireEvent.click(addButton); + + // Find the threshold input and type -1 + const thresholdInput = screen.getAllByPlaceholderText('Threshold')[0]; + userEvent.type(thresholdInput, '-1'); + +// // Find the dropdown using the data-test-subj attribute +// const percentageDropdown = screen.getByTestId('thresholdType-dropdown-0-0'); +// expect(percentageDropdown).toBeInTheDocument(); + +// fireEvent.change(percentageDropdown, { target: { value: 'percentage' } }); + + // Trigger validation + fireEvent.blur(thresholdInput); + + // Wait for the error message to appear + await waitFor(() => { + expect( + screen.getByText( + 'relative threshold must be a positive number greater than zero' + ) + ).toBeInTheDocument(); + }); + }); +}); diff --git a/public/pages/ConfigureModel/components/SuppressionRules/index.ts b/public/pages/ConfigureModel/components/SuppressionRules/index.ts new file mode 100644 index 00000000..75792be2 --- /dev/null +++ b/public/pages/ConfigureModel/components/SuppressionRules/index.ts @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +export { SuppressionRules } from './SuppressionRules'; diff --git a/public/pages/ConfigureModel/containers/ConfigureModel.tsx b/public/pages/ConfigureModel/containers/ConfigureModel.tsx index 1c0c0cd3..6427ba50 100644 --- a/public/pages/ConfigureModel/containers/ConfigureModel.tsx +++ b/public/pages/ConfigureModel/containers/ConfigureModel.tsx @@ -228,11 +228,29 @@ export function ConfigureModel(props: ConfigureModelProps) { } }; + const flattenErrorMessages = (errors): string => { + if (Array.isArray(errors)) { + return errors + .flatMap((innerArray) => + Array.isArray(innerArray) + ? innerArray.map((errorObj) => + Object.entries(errorObj || {}) + .map(([key, value]) => `${key}: ${value}`) + .join(", ") + ) + : [] + ) + .join(", "); + } + return typeof errors === "string" ? errors : ""; + }; + + const validateRules = ( formikValues: ModelConfigurationFormikValues, errors: any ) => { - const rules = formikValues.suppressionRules || []; + const suppressionRules = formikValues.suppressionRules || []; // Initialize an array to hold individual error messages const featureNameErrors: string[] = []; @@ -243,11 +261,30 @@ export function ConfigureModel(props: ConfigureModelProps) { .map((feature: FeaturesFormikValues) => feature.featureName); // Validate that each featureName in suppressionRules exists in enabledFeatures - rules.forEach((rule: RuleFormikValues) => { - if (!enabledFeatures.includes(rule.featureName)) { - featureNameErrors.push( - `Feature "${rule.featureName}" in suppression rules does not exist or is not enabled in the feature list.` - ); + suppressionRules.forEach((featureRules: RuleFormikValues[], featureIndex: number) => { + if (featureRules != null && featureRules.length > 0) { + const featureName = featureRules[0]?.featureName; + if (featureName === "" || featureName === undefined) { + featureNameErrors.push( + "Please make sure all features have unique names" + ); + return; + } + + if (!enabledFeatures.includes(featureName)) { + featureNameErrors.push( + `Feature "${featureName}" in suppression rules does not exist or is not enabled in the feature list.` + ); + } + + // Additional validation for each rule if needed + featureRules.forEach((rule: RuleFormikValues, ruleIndex: number) => { + if (rule.absoluteThreshold === null && rule.relativeThreshold === null) { + featureNameErrors.push( + `Rule ${ruleIndex + 1} for feature "${featureName}" must have either an absolute or relative threshold.` + ); + } + }); } }); @@ -271,7 +308,6 @@ export function ConfigureModel(props: ConfigureModelProps) { formikProps.setFieldTouched('shingleSize'); formikProps.setFieldTouched('imputationOption'); formikProps.setFieldTouched('suppressionRules'); - formikProps.validateForm().then((errors) => { // Call the extracted validation method validateImputationOption(formikProps.values, errors); @@ -306,8 +342,9 @@ export function ConfigureModel(props: ConfigureModelProps) { const ruleValueError = get(errors, 'suppressionRules') if (ruleValueError) { + const errorString = flattenErrorMessages(ruleValueError); core.notifications.toasts.addDanger( - ruleValueError + errorString ); focusOnSuppressionRules(); return; diff --git a/public/pages/ConfigureModel/containers/__tests__/__snapshots__/ConfigureModel.test.tsx.snap b/public/pages/ConfigureModel/containers/__tests__/__snapshots__/ConfigureModel.test.tsx.snap index edd26cde..4ad25010 100644 --- a/public/pages/ConfigureModel/containers/__tests__/__snapshots__/ConfigureModel.test.tsx.snap +++ b/public/pages/ConfigureModel/containers/__tests__/__snapshots__/ConfigureModel.test.tsx.snap @@ -273,7 +273,7 @@ exports[` spec creating model configuration renders the compon class="euiFormHelpText euiFormRow__text" id="random_html_id-help-0" > - Enter a descriptive name. The name must be unique within this detector. Feature name must contain 1-64 characters. Valid characters are a-z, A-Z, 0-9, -(hyphen) and _(underscore). + Enter a descriptive, unique name. The name must contain 1-64 characters. Valid characters are a-z, A-Z, 0-9, -(hyphen) and _(underscore). @@ -386,7 +386,9 @@ exports[` spec creating model configuration renders the compon
spec creating model configuration renders the compon class="euiFormLabel euiFormRow__label" for="random_html_id" > - Aggregation method +
+

+ Aggregation method +

+
+
+ The aggregation method determines what constitutes an anomaly. +
+
spec creating model configuration renders the compon class="euiFormHelpText euiFormRow__text" id="random_html_id-help-0" > - The aggregation method determines what constitutes an anomaly. For example, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature. + E.g, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature.
@@ -562,6 +577,276 @@ exports[` spec creating model configuration renders the compon +
+
+ +
+
+
+
+ +
+ + + +
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+ +
+
+
+
+

+ Ignore anomalies when the actual value is no more than: +

+
+
+
+
+
+
+ +
+
+
+
@@ -1255,7 +1540,7 @@ exports[` spec editing model configuration renders the compone class="euiFormHelpText euiFormRow__text" id="random_html_id-help-0" > - Enter a descriptive name. The name must be unique within this detector. Feature name must contain 1-64 characters. Valid characters are a-z, A-Z, 0-9, -(hyphen) and _(underscore). + Enter a descriptive, unique name. The name must contain 1-64 characters. Valid characters are a-z, A-Z, 0-9, -(hyphen) and _(underscore). @@ -1369,7 +1654,9 @@ exports[` spec editing model configuration renders the compone
spec editing model configuration renders the compone class="euiFormLabel euiFormRow__label" for="random_html_id" > - Aggregation method +
+

+ Aggregation method +

+
+
+ The aggregation method determines what constitutes an anomaly. +
+
spec editing model configuration renders the compone class="euiFormHelpText euiFormRow__text" id="random_html_id-help-0" > - The aggregation method determines what constitutes an anomaly. For example, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature. + E.g, if you choose min(), the detector focuses on finding anomalies based on the minimum values of your feature.
@@ -1547,6 +1847,278 @@ exports[` spec editing model configuration renders the compone +
+
+ +
+
+
+
+ +
+ + + +
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+ +
+
+
+
+

+ Ignore anomalies when the actual value is no more than: +

+
+
+
+
+
+
+ +
+
+
+
diff --git a/public/pages/ConfigureModel/models/interfaces.ts b/public/pages/ConfigureModel/models/interfaces.ts index 9bf2a496..824635da 100644 --- a/public/pages/ConfigureModel/models/interfaces.ts +++ b/public/pages/ConfigureModel/models/interfaces.ts @@ -19,7 +19,7 @@ export interface ModelConfigurationFormikValues { categoryField: string[]; shingleSize: number; imputationOption?: ImputationFormikValues; - suppressionRules?: RuleFormikValues[]; + suppressionRules?: Array; } export interface FeaturesFormikValues { @@ -48,4 +48,5 @@ export interface RuleFormikValues { absoluteThreshold?: number; relativeThreshold?: number; aboveBelow: string; + directionRule?: boolean; } diff --git a/public/pages/ConfigureModel/utils/constants.tsx b/public/pages/ConfigureModel/utils/constants.tsx index b0a84b2e..1f8f6248 100644 --- a/public/pages/ConfigureModel/utils/constants.tsx +++ b/public/pages/ConfigureModel/utils/constants.tsx @@ -76,3 +76,9 @@ export enum SparseDataOptionValue { SET_TO_ZERO = 'set_to_zero', CUSTOM_VALUE = 'custom_value', } + +export const FEATURE_DIRECTION_OPTIONS = [ + { text: 'Deviation in any direction (default)', value: "both" }, + { text: 'Rise above expected value', value: "above" }, + { text: 'Drop below expected value', value: "below" } +]; diff --git a/public/pages/ConfigureModel/utils/helpers.ts b/public/pages/ConfigureModel/utils/helpers.ts index 7df261a1..7551d5a7 100644 --- a/public/pages/ConfigureModel/utils/helpers.ts +++ b/public/pages/ConfigureModel/utils/helpers.ts @@ -39,9 +39,7 @@ import { Operator, Action, } from '../../../models/types'; -import { - SparseDataOptionValue -} from './constants' +import { SparseDataOptionValue } from './constants'; export const getFieldOptions = ( allFields: { [key: string]: string[] }, @@ -259,18 +257,25 @@ export function modelConfigurationToFormik( var defaultFillArray: CustomValueFormikValues[] = []; if (SparseDataOptionValue.CUSTOM_VALUE === imputationMethod) { - const defaultFill = get(detector, 'imputationOption.defaultFill', null) as Array<{ featureName: string; data: number }> | null; + const defaultFill = get( + detector, + 'imputationOption.defaultFill', + null + ) as Array<{ featureName: string; data: number }> | null; defaultFillArray = defaultFill - ? defaultFill.map(({ featureName, data }) => ({ - featureName, - data, - })) - : []; + ? defaultFill.map(({ featureName, data }) => ({ + featureName, + data, + })) + : []; } const imputationFormikValues: ImputationFormikValues = { imputationMethod: imputationMethod, - custom_value: SparseDataOptionValue.CUSTOM_VALUE === imputationMethod ? defaultFillArray : undefined, + custom_value: + SparseDataOptionValue.CUSTOM_VALUE === imputationMethod + ? defaultFillArray + : undefined, }; return { @@ -382,27 +387,34 @@ export function formikToSimpleAggregation(value: FeaturesFormikValues) { } } -export function formikToImputationOption(imputationFormikValues?: ImputationFormikValues): ImputationOption | undefined { +export function formikToImputationOption( + imputationFormikValues?: ImputationFormikValues +): ImputationOption | undefined { // Map the formik method to the imputation method; return undefined if method is not recognized. - const method = formikToImputationMethod(imputationFormikValues?.imputationMethod); + const method = formikToImputationMethod( + imputationFormikValues?.imputationMethod + ); if (!method) return undefined; // Convert custom_value array to defaultFill if the method is FIXED_VALUES. - const defaultFill = method === ImputationMethod.FIXED_VALUES - ? imputationFormikValues?.custom_value?.map(({ featureName, data }) => ({ - featureName, - data, - })) - : undefined; + const defaultFill = + method === ImputationMethod.FIXED_VALUES + ? imputationFormikValues?.custom_value?.map(({ featureName, data }) => ({ + featureName, + data, + })) + : undefined; // Construct and return the ImputationOption object. return { method, defaultFill }; } -export function imputationMethodToFormik( - detector: Detector -): string { - var imputationMethod = get(detector, 'imputationOption.method', undefined) as ImputationMethod; +export function imputationMethodToFormik(detector: Detector): string { + var imputationMethod = get( + detector, + 'imputationOption.method', + undefined + ) as ImputationMethod; switch (imputationMethod) { case ImputationMethod.FIXED_VALUES: @@ -433,15 +445,23 @@ export function formikToImputationMethod( } } -export const getCustomValueStrArray = (imputationMethodStr : string, detector: Detector): string[] => { +export const getCustomValueStrArray = ( + imputationMethodStr: string, + detector: Detector +): string[] => { if (SparseDataOptionValue.CUSTOM_VALUE === imputationMethodStr) { - const defaultFill : Array<{ featureName: string; data: number }> = get(detector, 'imputationOption.defaultFill', []); - - return defaultFill - .map(({ featureName, data }) => `${featureName}: ${data}`); + const defaultFill: Array<{ featureName: string; data: number }> = get( + detector, + 'imputationOption.defaultFill', + [] + ); + + return defaultFill.map( + ({ featureName, data }) => `${featureName}: ${data}` + ); } - return [] -} + return []; +}; export const getSuppressionRulesArray = (detector: Detector): string[] => { if (!detector.rules || detector.rules.length === 0) { @@ -453,8 +473,17 @@ export const getSuppressionRulesArray = (detector: Detector): string[] => { return rule.conditions.map((condition) => { const featureName = condition.featureName; const thresholdType = condition.thresholdType; + if (thresholdType === ThresholdType.ACTUAL_IS_OVER_EXPECTED) { + return `Ignore anomalies for feature "${featureName}" when actual value is above the expected value`; + } else if (thresholdType === ThresholdType.ACTUAL_IS_BELOW_EXPECTED) { + return `Ignore anomalies for feature "${featureName}" when actual value is below the expected value`; + } + + let value = condition.value; - const isPercentage = thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO || thresholdType === ThresholdType.EXPECTED_OVER_ACTUAL_RATIO; + const isPercentage = + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO || + thresholdType === ThresholdType.EXPECTED_OVER_ACTUAL_RATIO; // If it is a percentage, multiply by 100 if (isPercentage) { @@ -462,63 +491,153 @@ export const getSuppressionRulesArray = (detector: Detector): string[] => { } // Determine whether it is "above" or "below" based on ThresholdType - const aboveOrBelow = thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN || thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO ? 'above' : 'below'; + const aboveOrBelow = + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN || + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO + ? 'above' + : 'below'; // Construct the formatted string - return `Ignore anomalies for feature "${featureName}" with no more than ${value}${isPercentage ? '%' : ''} ${aboveOrBelow} expected value.`; + return `Ignore anomalies for feature "${featureName}" with no more than ${value}${ + isPercentage ? '%' : '' + } ${aboveOrBelow} expected value.`; }); }); }; +export const getSuppressionRulesArrayForFeature = ( + detector: Detector, + featureName: string +): string[] => { + if (!detector.rules || detector.rules.length === 0) { + return []; // Return an empty array if there are no rules + } + + return detector.rules.flatMap((rule) => { + // Filter conditions based on the specified feature name + const featureConditions = rule.conditions.filter( + (condition) => condition.featureName === featureName + ); + + // Convert each filtered condition to a readable string + return featureConditions.map((condition) => { + const thresholdType = condition.thresholdType; + + if (thresholdType === ThresholdType.ACTUAL_IS_OVER_EXPECTED) { + return `Ignore anomalies for feature "${featureName}" when actual value is above the expected value.`; + } else if (thresholdType === ThresholdType.ACTUAL_IS_BELOW_EXPECTED) { + return `Ignore anomalies for feature "${featureName}" when actual value is below the expected value.`; + } + + let value = condition.value; + const isPercentage = + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO || + thresholdType === ThresholdType.EXPECTED_OVER_ACTUAL_RATIO; + + // If it is a percentage, multiply by 100 + if (isPercentage) { + value *= 100; + } + + // Determine whether it is "above" or "below" based on ThresholdType + const aboveOrBelow = + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN || + thresholdType === ThresholdType.ACTUAL_OVER_EXPECTED_RATIO + ? 'above' + : 'below'; + + // Construct the formatted string + return `Ignore anomalies for feature "${featureName}" with no more than ${value}${ + isPercentage ? '%' : '' + } ${aboveOrBelow} expected value.`; + }); + }); +}; // Convert RuleFormikValues[] to Rule[] -export const formikToRules = (formikValues?: RuleFormikValues[]): Rule[] | undefined => { +export const formikToRules = ( + formikValues?: RuleFormikValues[] +): Rule[] | undefined => { + if (!formikValues || formikValues.length === 0) { return undefined; // Return undefined for undefined or empty input } - return formikValues.map((formikValue) => { + // Flatten the nested array of suppressionRule by feature and filter out null entries + const flattenedSuppressionFormikValues = formikValues.flatMap((nestedArray) => + nestedArray || [] // If null, replace with an empty array + ); + + return flattenedSuppressionFormikValues.map((formikValue) => { const conditions: Condition[] = []; + if (formikValue != null) { + // Determine the threshold type based on aboveBelow and the threshold type (absolute or relative) + const getThresholdType = ( + aboveBelow: string, + isAbsolute: boolean, + directionRule?: boolean + ): ThresholdType => { + if (directionRule) { + return aboveBelow === 'above' + ? ThresholdType.ACTUAL_IS_BELOW_EXPECTED + : ThresholdType.ACTUAL_IS_OVER_EXPECTED; + } else if (isAbsolute) { + return aboveBelow === 'above' + ? ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN + : ThresholdType.EXPECTED_OVER_ACTUAL_MARGIN; + } else { + return aboveBelow === 'above' + ? ThresholdType.ACTUAL_OVER_EXPECTED_RATIO + : ThresholdType.EXPECTED_OVER_ACTUAL_RATIO; + } + }; - // Determine the threshold type based on aboveBelow and the threshold type (absolute or relative) - const getThresholdType = (aboveBelow: string, isAbsolute: boolean): ThresholdType => { - if (isAbsolute) { - return aboveBelow === 'above' - ? ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN - : ThresholdType.EXPECTED_OVER_ACTUAL_MARGIN; - } else { - return aboveBelow === 'above' - ? ThresholdType.ACTUAL_OVER_EXPECTED_RATIO - : ThresholdType.EXPECTED_OVER_ACTUAL_RATIO; + + + if (formikValue.directionRule) { + conditions.push({ + featureName: formikValue.featureName, + thresholdType: getThresholdType(formikValue.aboveBelow, true, formikValue.directionRule), + operator: undefined, + value: undefined, + }); } - }; - // Check if absoluteThreshold is provided, create a condition - if (formikValue.absoluteThreshold !== undefined && formikValue.absoluteThreshold !== 0 && formikValue.absoluteThreshold !== null - && typeof formikValue.absoluteThreshold === 'number' && // Check if it's a number - !isNaN(formikValue.absoluteThreshold) && // Ensure it's not NaN - formikValue.absoluteThreshold > 0 // Check if it's positive - ) { - conditions.push({ - featureName: formikValue.featureName, - thresholdType: getThresholdType(formikValue.aboveBelow, true), - operator: Operator.LTE, - value: formikValue.absoluteThreshold, - }); - } + // Check if absoluteThreshold is provided, create a condition + if ( + formikValue.absoluteThreshold !== undefined && + formikValue.absoluteThreshold !== 0 && + formikValue.absoluteThreshold !== null && + typeof formikValue.absoluteThreshold === 'number' && // Check if it's a number + !isNaN(formikValue.absoluteThreshold) && // Ensure it's not NaN + formikValue.absoluteThreshold > 0 && // Check if it's positive + formikValue?.directionRule != true + ) { + conditions.push({ + featureName: formikValue.featureName, + thresholdType: getThresholdType(formikValue.aboveBelow, true), + operator: Operator.LTE, + value: formikValue.absoluteThreshold, + }); + } - // Check if relativeThreshold is provided, create a condition - if (formikValue.relativeThreshold !== undefined && formikValue.relativeThreshold !== 0 && formikValue.relativeThreshold !== null - && typeof formikValue.relativeThreshold === 'number' && // Check if it's a number - !isNaN(formikValue.relativeThreshold) && // Ensure it's not NaN - formikValue.relativeThreshold > 0 // Check if it's positive - ) { - conditions.push({ - featureName: formikValue.featureName, - thresholdType: getThresholdType(formikValue.aboveBelow, false), - operator: Operator.LTE, - value: formikValue.relativeThreshold / 100, // Convert percentage to decimal, - }); + // Check if relativeThreshold is provided, create a condition + if ( + formikValue.relativeThreshold !== undefined && + formikValue.relativeThreshold !== 0 && + formikValue.relativeThreshold !== null && + typeof formikValue.relativeThreshold === 'number' && // Check if it's a number + !isNaN(formikValue.relativeThreshold) && // Ensure it's not NaN + formikValue.relativeThreshold > 0 && // Check if it's positive + formikValue?.directionRule != true + ) { + conditions.push({ + featureName: formikValue.featureName, + thresholdType: getThresholdType(formikValue.aboveBelow, false), + operator: Operator.LTE, + value: formikValue.relativeThreshold / 100, // Convert percentage to decimal, + }); + } } return { @@ -528,14 +647,16 @@ export const formikToRules = (formikValues?: RuleFormikValues[]): Rule[] | undef }); }; -// Convert Rule[] to RuleFormikValues[] -export const rulesToFormik = (rules?: Rule[]): RuleFormikValues[] => { +// Convert Rule[] to RuleFormikValues[][] +export const rulesToFormik = (rules?: Rule[]): (RuleFormikValues[] | null)[] => { if (!rules || rules.length === 0) { return []; // Return empty array for undefined or empty input } + // Group rules by featureName + const groupedRules: { [featureName: string]: RuleFormikValues[] } = {}; - return rules.map((rule) => { - // Start with default values + rules.forEach((rule) => { + // Start with default values for each rule const formikValue: RuleFormikValues = { featureName: '', absoluteThreshold: undefined, @@ -559,21 +680,42 @@ export const rulesToFormik = (rules?: Rule[]): RuleFormikValues[] => { break; case ThresholdType.ACTUAL_OVER_EXPECTED_RATIO: // *100 to convert to percentage - formikValue.relativeThreshold = condition.value * 100; + formikValue.relativeThreshold = (condition.value ?? 1) * 100; formikValue.aboveBelow = 'above'; break; case ThresholdType.EXPECTED_OVER_ACTUAL_RATIO: // *100 to convert to percentage - formikValue.relativeThreshold = condition.value * 100; + formikValue.relativeThreshold = (condition.value ?? 1) * 100; + formikValue.aboveBelow = 'below'; + break; + case ThresholdType.ACTUAL_IS_BELOW_EXPECTED: + formikValue.relativeThreshold = 0; + formikValue.aboveBelow = 'above'; + formikValue.directionRule = true; + break; + case ThresholdType.ACTUAL_IS_OVER_EXPECTED: + formikValue.relativeThreshold = 0; formikValue.aboveBelow = 'below'; + formikValue.directionRule = true; break; default: break; } }); - return formikValue; + // Add the rule to the grouped object based on featureName + if (!groupedRules[formikValue.featureName]) { + groupedRules[formikValue.featureName] = []; + } + groupedRules[formikValue.featureName].push(formikValue); }); -}; + // Convert grouped object into an array of arrays based on featureList index + const featureList = Object.keys(groupedRules); // Ensure you have a reference to your feature list somewhere + const finalRules: (RuleFormikValues[] | null)[] = featureList.map( + (featureName) => groupedRules[featureName] || null + ); + + return finalRules; +}; diff --git a/public/pages/DetectorConfig/components/AdditionalSettings/AdditionalSettings.tsx b/public/pages/DetectorConfig/components/AdditionalSettings/AdditionalSettings.tsx index ece8466c..f48c9c24 100644 --- a/public/pages/DetectorConfig/components/AdditionalSettings/AdditionalSettings.tsx +++ b/public/pages/DetectorConfig/components/AdditionalSettings/AdditionalSettings.tsx @@ -18,14 +18,13 @@ import { } from '@elastic/eui'; import ContentPanel from '../../../../components/ContentPanel/ContentPanel'; import { convertToCategoryFieldString } from '../../../utils/anomalyResultUtils'; -import { SuppressionRulesModal } from '../../../ReviewAndCreate/components/AdditionalSettings/SuppressionRulesModal'; +import { SuppressionRulesModal } from '../../../ReviewAndCreate/components/SuppressionRulesModal/SuppressionRulesModal'; interface AdditionalSettingsProps { shingleSize: number; categoryField: string[]; imputationMethod: string; customValues: string[]; - suppressionRules: string[]; } export function AdditionalSettings(props: AdditionalSettingsProps) { @@ -39,28 +38,6 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { ); - const [isModalVisible, setIsModalVisible] = useState(false); - const [modalContent, setModalContent] = useState([]); - - const closeModal = () => setIsModalVisible(false); - - const showRulesInModal = (rules: string[]) => { - setModalContent(rules); - setIsModalVisible(true); - }; - - const renderSuppressionRules = (suppressionRules: string[]) => ( -
- {suppressionRules.length > 0 ? ( - showRulesInModal(suppressionRules)}> - {suppressionRules.length} rules - - ) : ( -

-

- )} -
- ); - const tableItems = [ { categoryField: isEmpty(get(props, 'categoryField', [])) @@ -69,7 +46,6 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { shingleSize: props.shingleSize, imputationMethod: props.imputationMethod, customValues: props.customValues, - suppresionRules: props.suppressionRules, }, ]; const tableColumns = [ @@ -79,11 +55,7 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { { name: 'Custom values', field: 'customValues', render: (customValues: string[]) => renderCustomValues(customValues), // Use a custom render function - }, - { name: 'Suppression rules', - field: 'suppresionRules', - render: (suppresionRules: string[]) => renderSuppressionRules(suppresionRules), // Use a custom render function - }, + } ]; return ( @@ -92,11 +64,6 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { items={tableItems} columns={tableColumns} /> - {isModalVisible && ( - - - - )} ); } diff --git a/public/pages/DetectorConfig/containers/Features.tsx b/public/pages/DetectorConfig/containers/Features.tsx index 10897431..87847bd6 100644 --- a/public/pages/DetectorConfig/containers/Features.tsx +++ b/public/pages/DetectorConfig/containers/Features.tsx @@ -17,6 +17,8 @@ import { EuiSmallButton, EuiEmptyPrompt, EuiSpacer, + EuiOverlayMask, + EuiButtonEmpty, } from '@elastic/eui'; import { Detector, @@ -34,7 +36,10 @@ import { imputationMethodToFormik, getCustomValueStrArray, getSuppressionRulesArray, + getSuppressionRulesArrayForFeature, } from '../../ConfigureModel/utils/helpers'; +import { SuppressionRulesModal } from '../../ReviewAndCreate/components/SuppressionRulesModal/SuppressionRulesModal'; +import { Rule } from '../../../models/types'; interface FeaturesProps { detectorId: string; @@ -56,6 +61,34 @@ export const Features = (props: FeaturesProps) => { sortField: 'name', sortDirection: 'asc', }); + const [isRuleModalVisible, setIsRuleModalVisible] = useState(false); + + const [modalContent, setModalContent] = useState([]); + + const closeRuleModal = () => setIsRuleModalVisible(false); + + const showRulesInModal = (rules: string[]) => { + setModalContent(rules); + setIsRuleModalVisible(true); + }; + + const renderSuppressionRules = (suppressionRules: string[], featureIndex: number) => { + return ( +
+ {suppressionRules.length > 0 ? ( + showRulesInModal(suppressionRules)} + > + {suppressionRules.length} rules + + ) : ( +

any

+ )} +
+ ); + }; const closeModal = (index: number) => { const cloneShowCodeModal = [...featuresState.showCodeModel]; @@ -109,6 +142,7 @@ export const Features = (props: FeaturesProps) => { name: feature.featureName, definition: index, state: feature.featureEnabled ? 'Enabled' : 'Disabled', + suppressionRule: index, }) ); @@ -175,6 +209,19 @@ export const Features = (props: FeaturesProps) => { field: 'state', name: 'Feature state', }, + { + field: 'suppressionRule', + name: 'Anomaly Criteria', + render: (featureIndex: number) => { + const feature = featureAttributes[featureIndex]; + return renderSuppressionRules( + getSuppressionRulesArrayForFeature( + props.detector, + feature.featureName + ),featureIndex + ); + }, + }, ]; const getCellProps = () => { @@ -246,6 +293,14 @@ export const Features = (props: FeaturesProps) => { sorting={sorting} onChange={handleTableChange} /> + {isRuleModalVisible && ( + + + + )} spec', () => { + test('renders the component with suppression rules', async () => { + // Mock CoreServicesContext + const coreServicesMock = { + uiSettings: { + get: jest.fn().mockImplementation((key) => { + if (key === 'theme:darkMode') return false; // Mock darkMode + return null; + }), + }, + }; + + // Set up the mock props + const randomDetector = { + ...getRandomDetector(false), + featureAttributes: [ + { + featureName: 'value', + featureEnabled: true, + aggregationQuery: featureQuery1, + }, + { + featureName: 'value2', + featureEnabled: true, + aggregationQuery: featureQuery2, + }, + ] as FeatureAttributes[], + uiMetadata: { + filterType: FILTER_TYPES.SIMPLE, + filters: [], + features: { + value: { + featureType: FEATURE_TYPE.CUSTOM, + } as UiFeature, + value2: { + featureType: FEATURE_TYPE.CUSTOM, + } as UiFeature, + }, + } as UiMetaData, + rules: [ + { + action: Action.IGNORE_ANOMALY, + conditions: [ + { + featureName: 'value', // Matches a feature in featureAttributes + thresholdType: ThresholdType.ACTUAL_OVER_EXPECTED_MARGIN, + operator: Operator.LTE, + value: 5, + }, + { + featureName: 'value2', // Matches another feature in featureAttributes + thresholdType: ThresholdType.EXPECTED_OVER_ACTUAL_RATIO, + operator: Operator.LTE, + value: 10, + }, + ], + }, + ], + }; + + const { container, getByRole, getByText, queryByRole, getByTestId } = + render( + + + {() => ( + + )} + + + ); + + expect(container.firstChild).toMatchSnapshot(); + + const firstButton = getByTestId('suppression-rules-button-0'); + expect(firstButton).toBeInTheDocument(); + fireEvent.click(firstButton); + + // Wait for the modal to appear and check for its content + await waitFor(() => { + expect(screen.getByRole('dialog')).toBeInTheDocument(); // Ensure modal is opened + }); + + getByText('Suppression Rules'); // Modal header + getByText( + 'Ignore anomalies for feature "value" with no more than 5 above expected value.' + ); + + // Close the modal by clicking the close button (X) + const closeButton = getByRole('button', { + name: 'Closes this modal window', + }); + fireEvent.click(closeButton); + + // Ensure the modal is closed + await waitFor(() => { + expect(queryByRole('dialog')).toBeNull(); + }); + + const secondButton = getByTestId('suppression-rules-button-1'); + expect(secondButton).toBeInTheDocument(); + fireEvent.click(secondButton); + + // Wait for the modal to appear and check for its content + await waitFor(() => { + expect(screen.getByRole('dialog')).toBeInTheDocument(); // Ensure modal is opened + }); + + getByText( + 'Ignore anomalies for feature "value2" with no more than 1000% below expected value.' + ); + + const closeSecondButton = getByRole('button', { + name: 'Closes this modal window', + }); + fireEvent.click(closeSecondButton); + + // Ensure the modal is closed + await waitFor(() => { + expect(queryByRole('dialog')).toBeNull(); + }); + }); + test('renders the component without suppression rules', async () => { + // Mock CoreServicesContext + const coreServicesMock = { + uiSettings: { + get: jest.fn().mockImplementation((key) => { + if (key === 'theme:darkMode') return false; // Mock darkMode + return null; + }), + }, + }; + + // Set up the mock props + const randomDetector = { + ...getRandomDetector(false), + featureAttributes: [ + { + featureName: 'value', + featureEnabled: true, + aggregationQuery: featureQuery1, + }, + ] as FeatureAttributes[], + uiMetadata: { + filterType: FILTER_TYPES.SIMPLE, + filters: [], + features: { + value: { + featureType: FEATURE_TYPE.CUSTOM, + } as UiFeature, + }, + } as UiMetaData, + rules: [], + }; + + const { container, queryByText, getByText, queryByRole, getByTestId } = + render( + + + {() => ( + + )} + + + ); + + expect(container.firstChild).toMatchSnapshot(); + const tableCell = screen.getByRole('cell', { name: /anomaly criteria/i }); + const anyText = within(tableCell).getByText('any'); + expect(anyText).toBeInTheDocument(); + }); +}); diff --git a/public/pages/DetectorConfig/containers/__tests__/__snapshots__/DetectorConfig.test.tsx.snap b/public/pages/DetectorConfig/containers/__tests__/__snapshots__/DetectorConfig.test.tsx.snap index 23c72f8d..4e3f05b6 100644 --- a/public/pages/DetectorConfig/containers/__tests__/__snapshots__/DetectorConfig.test.tsx.snap +++ b/public/pages/DetectorConfig/containers/__tests__/__snapshots__/DetectorConfig.test.tsx.snap @@ -843,6 +843,22 @@ exports[` spec renders rules 1`] = ` + + + + Anomaly Criteria + + + @@ -912,6 +928,41 @@ exports[` spec renders rules 1`] = ` + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ spec renders rules 1`] = ` + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ spec renders rules 1`] = ` + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ @@ -1194,22 +1315,6 @@ exports[` spec renders rules 1`] = ` - - - - Suppression rules - - - @@ -1290,38 +1395,6 @@ exports[` spec renders rules 1`] = ` - -
- Suppression rules -
-
-
- -
-
- @@ -2303,6 +2376,23 @@ exports[` spec renders the component 1`] = ` + + + + Anomaly Criteria + + + @@ -2372,6 +2462,28 @@ exports[` spec renders the component 1`] = ` + +
+ Anomaly Criteria +
+
+ +
+

+ any +

+
+ +
+ spec renders the component 1`] = ` + +
+ Anomaly Criteria +
+
+ +
+

+ any +

+
+ +
+ @@ -2591,23 +2725,6 @@ exports[` spec renders the component 1`] = ` - - - - Suppression rules - - - @@ -2688,38 +2805,6 @@ exports[` spec renders the component 1`] = ` - -
- Suppression rules -
-
-
- -
-
- @@ -3732,6 +3817,22 @@ exports[` spec renders the component with 2 custom and 1 simpl + + + + Anomaly Criteria + + + @@ -3804,6 +3905,41 @@ exports[` spec renders the component with 2 custom and 1 simpl + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ spec renders the component with 2 custom and 1 simpl + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ spec renders the component with 2 custom and 1 simpl + +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ @@ -4092,22 +4298,6 @@ exports[` spec renders the component with 2 custom and 1 simpl - - - - Suppression rules - - - @@ -4191,38 +4381,6 @@ exports[` spec renders the component with 2 custom and 1 simpl - -
- Suppression rules -
-
-
- -
-
- diff --git a/public/pages/DetectorConfig/containers/__tests__/__snapshots__/Features.test.tsx.snap b/public/pages/DetectorConfig/containers/__tests__/__snapshots__/Features.test.tsx.snap new file mode 100644 index 00000000..c60c844a --- /dev/null +++ b/public/pages/DetectorConfig/containers/__tests__/__snapshots__/Features.test.tsx.snap @@ -0,0 +1,5913 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` spec renders rules 1`] = ` +
+
+
+
+
+
+

+ Detector settings +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+ Sample Detector +

+
+
+
+
+
+
+
+ +
+
+
+

+ index1 +

+
+
+
+
+
+
+
+ +
+
+
+
+

+ Custom expression: + + +

+
+
+
+
+
+
+
+
+ +
+
+
+

+ 1 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+ detector-1 +

+
+
+
+
+
+
+
+ +
+
+
+

+ A sample detector for testing +

+
+
+
+
+
+
+
+ +
+
+
+

+ timestamp +

+
+
+
+
+
+
+
+ +
+
+
+

+ 04/14/20 12:13 AM +

+
+
+
+
+
+
+
+ +
+
+
+

+ 1 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+
+
+
+
+
+

+ Model configuration +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+

+ Features +   +

+

+ (3) +

+

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + + Feature definition + + + + + + Feature state + + + + + + Anomaly Criteria + + +
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Field: + value +

+

+ Aggregation method: + min +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Field: + value +

+

+ Aggregation method: + min +

+
+ +
+ 		+
+ Feature state +
+
+ + Disabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+ Feature name +
+
+ + value2 + +
+ 		+
+ Feature definition +
+
+ +
+

+ Field: + value2 +

+

+ Aggregation method: + avg +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+
+
+
+
+
+
+
+
+

+ Additional settings +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + Categorical fields + + + + + + Shingle size + + + + + + Imputation method + + + + + + Custom values + + +
+
+ Categorical fields +
+
+ + - + +
+ 		+
+ Shingle size +
+
+ + 8 + +
+ 		+
+ Imputation method +
+
+ + ignore + +
+ 		+
+ Custom values +
+
+
+

+ - +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+ Detector jobs +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+

+ +

+

+
+
+
+
+
+
+
+ +
+
+
+

+ Disabled +

+
+
+
+
+
+
+
+
+
+
+`; + +exports[` spec renders the component 1`] = ` +
+
+
+
+
+
+

+ Detector settings +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+ niihduhifi +

+
+
+
+
+
+
+
+ +
+
+
+

+ logstash-* +

+
+
+
+
+
+
+
+ +
+
+
+
+

+ Custom expression: + + +

+
+
+
+
+
+
+
+
+ +
+
+
+

+ 6 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+

+
+
+
+
+
+
+ +
+
+
+

+ Gegni fe sokvuhi luwegja evumemu dajnidu dangap id baj vejmil linsumi leem. +

+
+
+
+
+
+
+
+ +
+
+
+

+ @timestamp +

+
+
+
+
+
+
+
+ +
+
+
+

+ 04/14/20 12:13 AM +

+
+
+
+
+
+
+
+ +
+
+
+

+ 5 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ Yes +

+
+
+
+
+
+
+
+ +
+
+
+

+ 7 Days +

+
+
+
+
+
+
+
+ +
+
+
+

+ 51200 MB +

+
+
+
+
+
+
+
+ +
+
+
+

+ 60 Days +

+
+
+
+
+
+
+
+
+
+
+
+
+

+ Model configuration +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+

+ Features +   +

+

+ (2) +

+

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + + Feature definition + + + + + + Feature state + + + + + + Anomaly Criteria + + +
+
+ Feature name +
+
+ + HN + +
+ 		+
+ Feature definition +
+
+ +
+

+ Field: + HN +

+

+ Aggregation method: + max +

+
+ +
+ 		+
+ Feature state +
+
+ + Disabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+

+ any +

+
+ +
+
+
+ Feature name +
+
+ + SR + +
+ 		+
+ Feature definition +
+
+ +
+

+ Field: + SR +

+

+ Aggregation method: + min +

+
+ +
+ 		+
+ Feature state +
+
+ + Disabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+

+ any +

+
+ +
+
+
+
+
+
+
+
+
+
+
+

+ Additional settings +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + Categorical fields + + + + + + Shingle size + + + + + + Imputation method + + + + + + Custom values + + +
+
+ Categorical fields +
+
+ + - + +
+ 		+
+ Shingle size +
+
+ + 8 + +
+ 		+
+ Imputation method +
+
+ + previous_value + +
+ 		+
+ Custom values +
+
+
+

+ - +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+ Detector jobs +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+

+

+
+
+ +
+
+ Initializing +
+
+
+

+

+
+
+
+
+
+
+
+ +
+
+
+

+ Disabled +

+
+
+
+
+
+
+
+
+
+
+`; + +exports[` spec renders the component with 2 custom and 1 simple features 1`] = ` +
+
+
+
+
+
+

+ Detector settings +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+ imuovluonp +

+
+
+
+
+
+
+
+ +
+
+
+

+ logstash-* +

+
+
+
+
+
+
+
+ +
+
+
+
+

+ Custom expression: + + +

+
+
+
+
+
+
+
+
+ +
+
+
+

+ 2 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+ 96f53081-6862-5961-8 +

+
+
+
+
+
+
+
+ +
+
+
+

+ Rijivet ub nopfauc weuj noz jinona gaceje kiljev itekep ohohve hipudo mat dutak. +

+
+
+
+
+
+
+
+ +
+
+
+

+ @timestamp +

+
+
+
+
+
+
+
+ +
+
+
+

+ 04/14/20 12:13 AM +

+
+
+
+
+
+
+
+ +
+
+
+

+ 1 Minutes +

+
+
+
+
+
+
+
+ +
+
+
+

+ - +

+
+
+
+
+
+
+
+ +
+
+
+

+ Yes +

+
+
+
+
+
+
+
+ +
+
+
+

+ 7 Days +

+
+
+
+
+
+
+
+ +
+
+
+

+ 51200 MB +

+
+
+
+
+
+
+
+ +
+
+
+

+ 60 Days +

+
+
+
+
+
+
+
+
+
+
+
+
+

+ Model configuration +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+

+ Features +   +

+

+ (3) +

+

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + + Feature definition + + + + + + Feature state + + + + + + Anomaly Criteria + + +
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Disabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+ Feature name +
+
+ + value2 + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+
+
+
+
+
+
+
+
+

+ Additional settings +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + Categorical fields + + + + + + Shingle size + + + + + + Imputation method + + + + + + Custom values + + +
+
+ Categorical fields +
+
+ + - + +
+ 		+
+ Shingle size +
+
+ + 8 + +
+ 		+
+ Imputation method +
+
+ + custom_value + +
+ 		+
+ Custom values +
+
+
+

+ value: 3 +

+

+ value2: 3 +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+ Detector jobs +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+

+

+

+
+
+ +
+
+ Initializing +
+
+
+

+

+
+
+
+
+
+
+
+ +
+
+
+

+ Disabled +

+
+
+
+
+
+
+
+
+
+
+`; + +exports[` spec renders the component with suppression rules 1`] = ` +
+
+
+

+ Model configuration +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+

+ Features +   +

+

+ (2) +

+

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + + Feature definition + + + + + + Feature state + + + + + + Anomaly Criteria + + +
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+ Feature name +
+
+ + value2 + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+
+
+
+
+
+
+
+
+
+

+ Additional settings +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + Categorical fields + + + + + + Shingle size + + + + + + Imputation method + + + + + + Custom values + + +
+
+ Categorical fields +
+
+ + - + +
+ 		+
+ Shingle size +
+
+ + 8 + +
+ 		+
+ Imputation method +
+
+ + set_to_zero + +
+ 		+
+ Custom values +
+
+
+

+ - +

+
+
+
+
+
+
+
+
+
+
+
+
+`; + +exports[` spec renders the component without suppression rules 1`] = ` +
+
+
+

+ Model configuration +

+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+
+
+

+

+ Features +   +

+

+ (1) +

+

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + + + Feature definition + + + + + + Feature state + + + + + + Anomaly Criteria + + +
+
+ Feature name +
+
+ + value + +
+ 		+
+ Feature definition +
+
+ +
+

+ Custom expression: + + +

+
+ +
+ 		+
+ Feature state +
+
+ + Enabled + +
+ 		+
+ Anomaly Criteria +
+
+ +
+

+ any +

+
+ +
+
+
+
+
+
+
+
+
+
+
+

+ Additional settings +

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + + + + + + + + + + + + + + + + +
+
+ + + Categorical fields + + + + + + Shingle size + + + + + + Imputation method + + + + + + Custom values + + +
+
+ Categorical fields +
+
+ + - + +
+ 		+
+ Shingle size +
+
+ + 8 + +
+ 		+
+ Imputation method +
+
+ + ignore + +
+ 		+
+ Custom values +
+
+
+

+ - +

+
+
+
+
+
+
+
+
+
+
+
+
+`; diff --git a/public/pages/ReviewAndCreate/components/AdditionalSettings/AdditionalSettings.tsx b/public/pages/ReviewAndCreate/components/AdditionalSettings/AdditionalSettings.tsx index ac0c1905..e3d27e94 100644 --- a/public/pages/ReviewAndCreate/components/AdditionalSettings/AdditionalSettings.tsx +++ b/public/pages/ReviewAndCreate/components/AdditionalSettings/AdditionalSettings.tsx @@ -9,22 +9,18 @@ * GitHub history for details. */ -import React, { useState } from 'react'; +import React from 'react'; import { get } from 'lodash'; import { - EuiBasicTable, - EuiButtonEmpty, - EuiOverlayMask + EuiBasicTable } from '@elastic/eui'; import ContentPanel from '../../../../components/ContentPanel/ContentPanel'; -import { SuppressionRulesModal } from './SuppressionRulesModal'; interface AdditionalSettingsProps { shingleSize: number; categoryField: string[]; imputationMethod: string; customValues: string[]; - suppressionRules: string[]; } export function AdditionalSettings(props: AdditionalSettingsProps) { @@ -38,35 +34,12 @@ export function AdditionalSettings(props: AdditionalSettingsProps) {
); - const [isModalVisible, setIsModalVisible] = useState(false); - const [modalContent, setModalContent] = useState([]); - - const closeModal = () => setIsModalVisible(false); - - const showRulesInModal = (rules: string[]) => { - setModalContent(rules); - setIsModalVisible(true); - }; - - const renderSuppressionRules = (suppressionRules: string[]) => ( -
- {suppressionRules.length > 0 ? ( - showRulesInModal(suppressionRules)}> - {suppressionRules.length} rules - - ) : ( -

-

- )} -
- ); - const tableItems = [ { categoryField: get(props, 'categoryField.0', '-'), shingleSize: props.shingleSize, imputationMethod: props.imputationMethod, customValues: props.customValues, - suppresionRules: props.suppressionRules, }, ]; const tableColumns = [ @@ -76,11 +49,7 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { { name: 'Custom values', field: 'customValues', render: (customValues: string[]) => renderCustomValues(customValues), // Use a custom render function - }, - { name: 'Suppression rules', - field: 'suppresionRules', - render: (suppressionRules: string[]) => renderSuppressionRules(suppressionRules), // Use a custom render function - }, + } ]; return ( @@ -89,11 +58,6 @@ export function AdditionalSettings(props: AdditionalSettingsProps) { items={tableItems} columns={tableColumns} /> - {isModalVisible && ( - - - - )} ); } diff --git a/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx b/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx index fc92a6ce..9ce27023 100644 --- a/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx +++ b/public/pages/ReviewAndCreate/components/ModelConfigurationFields/ModelConfigurationFields.tsx @@ -19,6 +19,8 @@ import { EuiLoadingSpinner, EuiFlexGroup, EuiText, + EuiButtonEmpty, + EuiOverlayMask, } from '@elastic/eui'; import { Detector, @@ -36,8 +38,10 @@ import { imputationMethodToFormik, getCustomValueStrArray, getSuppressionRulesArray, - } from '../../../ConfigureModel/utils/helpers'; + getSuppressionRulesArrayForFeature, +} from '../../../ConfigureModel/utils/helpers'; import { SORT_DIRECTION } from '../../../../../server/utils/constants'; +import { SuppressionRulesModal } from '../SuppressionRulesModal/SuppressionRulesModal'; interface ModelConfigurationFieldsProps { detector: Detector; @@ -89,6 +93,34 @@ export const ModelConfigurationFields = ( return featuresState.showCodeModel[index]; }; + const [isRuleModalVisible, setIsRuleModalVisible] = useState(false); + + const [modalContent, setModalContent] = useState([]); + + const closeRuleModal = () => setIsRuleModalVisible(false); + + const showRulesInModal = (rules: string[]) => { + setModalContent(rules); + setIsRuleModalVisible(true); + }; + + const renderSuppressionRules = (suppressionRules: string[]) => { + return ( +
+ {suppressionRules.length > 0 ? ( + showRulesInModal(suppressionRules)} + > + {suppressionRules.length} rules + + ) : ( +

any

+ )} +
+ ); + }; + const handleTableChange = (props: any) => { setFeaturesState({ ...featuresState, @@ -120,6 +152,7 @@ export const ModelConfigurationFields = ( name: feature.featureName, definition: index, state: feature.featureEnabled ? 'Enabled' : 'Disabled', + suppressionRule: index, }) ); @@ -186,6 +219,19 @@ export const ModelConfigurationFields = ( field: 'state', name: 'Feature state', }, + { + field: 'suppressionRule', + name: 'Anomaly Criteria', + render: (featureIndex: number) => { + const feature = featureAttributes[featureIndex]; + return renderSuppressionRules( + getSuppressionRulesArrayForFeature( + props.detector, + feature.featureName + ) + ); + }, + }, ]; const getCellProps = () => { @@ -314,9 +360,19 @@ export const ModelConfigurationFields = ( title="Model configuration" titleSize="s" actions={[ - Edit, + + Edit + , ]} > + {isRuleModalVisible && ( + + + + )} {getValidationCallout()}
@@ -324,7 +380,10 @@ export const ModelConfigurationFields = ( shingleSize={shingleSize} categoryField={get(props, 'detector.categoryField', [])} imputationMethod={imputationMethodStr} - customValues={getCustomValueStrArray(imputationMethodStr, props.detector)} + customValues={getCustomValueStrArray( + imputationMethodStr, + props.detector + )} suppressionRules={getSuppressionRulesArray(props.detector)} /> diff --git a/public/pages/ReviewAndCreate/components/AdditionalSettings/SuppressionRulesModal.tsx b/public/pages/ReviewAndCreate/components/SuppressionRulesModal/SuppressionRulesModal.tsx similarity index 100% rename from public/pages/ReviewAndCreate/components/AdditionalSettings/SuppressionRulesModal.tsx rename to public/pages/ReviewAndCreate/components/SuppressionRulesModal/SuppressionRulesModal.tsx diff --git a/public/pages/ReviewAndCreate/components/SuppressionRulesModal/index.ts b/public/pages/ReviewAndCreate/components/SuppressionRulesModal/index.ts new file mode 100644 index 00000000..5ddf42cb --- /dev/null +++ b/public/pages/ReviewAndCreate/components/SuppressionRulesModal/index.ts @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +export { SuppressionRulesModal } from './SuppressionRulesModal'; diff --git a/public/pages/ReviewAndCreate/components/__tests__/AdditionalSettings.test.tsx b/public/pages/ReviewAndCreate/components/__tests__/AdditionalSettings.test.tsx index d2de2682..79e73a13 100644 --- a/public/pages/ReviewAndCreate/components/__tests__/AdditionalSettings.test.tsx +++ b/public/pages/ReviewAndCreate/components/__tests__/AdditionalSettings.test.tsx @@ -41,15 +41,6 @@ describe(' spec', () => { // Assert that multiple elements with the text '-' are present const dashElements = getAllByText('-'); expect(dashElements.length).toBeGreaterThan(1); // Checks that more than one '-' is found - - // Check that the 'Suppression rules' title is present - // Assert that multiple elements with the text '-' are present - const ruleElements = getAllByText('Suppression rules'); - expect(ruleElements.length).toBeGreaterThan(1); // one is table cell title, another is the button - - // Use queryByRole to check that the button link is not present - const button = screen.queryByRole('button', { name: '0 rules' }); - expect(button).toBeNull(); }); test('renders the component with high cardinality enabled', async () => { const { container, getByText, getAllByText, getByRole, queryByRole } = @@ -80,37 +71,5 @@ describe(' spec', () => { // Check for the custom values getByText('denyMax:5'); getByText('denySum:10'); - - // Check for the suppression rules button link - const button = getByRole('button', { name: '2 rules' }); - expect(button).toBeInTheDocument(); - - // Click the button to open the modal - fireEvent.click(button); - - // Wait for the modal to appear and check for its content - await waitFor(() => { - expect(screen.getByRole('dialog')).toBeInTheDocument(); // Ensure modal is opened - }); - - getByText('Suppression Rules'); // Modal header - getByText( - "Ignore anomalies for feature 'CPU Usage' with no more than 5 above expected value." - ); - getByText( - "Ignore anomalies for feature 'Memory Usage' with no more than 10% below expected value." - ); - - // Close the modal by clicking the close button (X) - // Close the modal by clicking the close button (X) - const closeButton = getByRole('button', { - name: 'Closes this modal window', - }); - fireEvent.click(closeButton); - - // Ensure the modal is closed - await waitFor(() => { - expect(queryByRole('dialog')).toBeNull(); - }); }); }); diff --git a/public/pages/ReviewAndCreate/components/__tests__/ModelConfigurationFields.test.tsx b/public/pages/ReviewAndCreate/components/__tests__/ModelConfigurationFields.test.tsx index 98c7ea09..0083d550 100644 --- a/public/pages/ReviewAndCreate/components/__tests__/ModelConfigurationFields.test.tsx +++ b/public/pages/ReviewAndCreate/components/__tests__/ModelConfigurationFields.test.tsx @@ -86,7 +86,7 @@ const testDetector = { describe('ModelConfigurationFields', () => { test('renders the component in create mode (no ID)', async () => { const onEditModelConfiguration = jest.fn(); - const { container, getByText, getByTestId, queryByText, getByRole, queryByRole } = render( + const { container, getByText, getByTestId, queryByText, getAllByRole, queryByRole } = render( { expect(container.firstChild).toMatchSnapshot(); getByText('set_to_zero'); - // Check for the suppression rules button link - const button = getByRole('button', { name: '2 rules' }); - expect(button).toBeInTheDocument(); +// Check for the suppression rules buttons with the name '1 rules' +const buttons = getAllByRole('button', { name: '1 rules' }); +expect(buttons).toHaveLength(2); userEvent.click(getByTestId('viewFeature-0')); await waitFor(() => { diff --git a/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/AdditionalSettings.test.tsx.snap b/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/AdditionalSettings.test.tsx.snap index 4a9e79af..8b2d29a2 100644 --- a/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/AdditionalSettings.test.tsx.snap +++ b/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/AdditionalSettings.test.tsx.snap @@ -143,23 +143,6 @@ exports[` spec renders the component with high cardinality - - - - Suppression rules - - - @@ -240,26 +223,6 @@ exports[` spec renders the component with high cardinality
- -
- Suppression rules -
-
-
-

- - -

-
-
- @@ -414,23 +377,6 @@ exports[` spec renders the component with high cardinality - - - - Suppression rules - - - @@ -514,38 +460,6 @@ exports[` spec renders the component with high cardinality
- -
- Suppression rules -
-
-
- -
-
- diff --git a/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/ModelConfigurationFields.test.tsx.snap b/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/ModelConfigurationFields.test.tsx.snap index d1098678..f34f9e5f 100644 --- a/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/ModelConfigurationFields.test.tsx.snap +++ b/public/pages/ReviewAndCreate/components/__tests__/__snapshots__/ModelConfigurationFields.test.tsx.snap @@ -234,23 +234,6 @@ exports[`ModelConfigurationFields renders the component in create mode (no ID) 1 - - - - Suppression rules - - - @@ -331,38 +314,6 @@ exports[`ModelConfigurationFields renders the component in create mode (no ID) 1
- -
- Suppression rules -
-
-
- -
-
- @@ -579,6 +530,23 @@ exports[`ModelConfigurationFields renders the component in create mode (no ID) 1 + + + + Anomaly Criteria + + + @@ -651,6 +619,40 @@ exports[`ModelConfigurationFields renders the component in create mode (no ID) 1
+ +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+
+ +
+ Anomaly Criteria +
+
+ +
+ +
+ +
+ diff --git a/public/pages/ReviewAndCreate/containers/__tests__/__snapshots__/ReviewAndCreate.test.tsx.snap b/public/pages/ReviewAndCreate/containers/__tests__/__snapshots__/ReviewAndCreate.test.tsx.snap index 2294910a..7b4afb3c 100644 --- a/public/pages/ReviewAndCreate/containers/__tests__/__snapshots__/ReviewAndCreate.test.tsx.snap +++ b/public/pages/ReviewAndCreate/containers/__tests__/__snapshots__/ReviewAndCreate.test.tsx.snap @@ -778,23 +778,6 @@ exports[` spec renders the component, validation loading 1`] - - - - Suppression rules - - - @@ -875,26 +858,6 @@ exports[` spec renders the component, validation loading 1`]
- -
- Suppression rules -
-
-
-

- - -

-
-
- @@ -1111,6 +1074,23 @@ exports[` spec renders the component, validation loading 1`] + + + + Anomaly Criteria + + + @@ -1119,7 +1099,7 @@ exports[` spec renders the component, validation loading 1`] >
- - - - Suppression rules - - - @@ -2176,26 +2139,6 @@ exports[`issue in detector validation issues in feature query 1`] = `
- -
- Suppression rules -
-
-
-

- - -

-
-
- @@ -2413,6 +2356,23 @@ exports[`issue in detector validation issues in feature query 1`] = ` + + + + Anomaly Criteria + + + @@ -2421,7 +2381,7 @@ exports[`issue in detector validation issues in feature query 1`] = ` >
{ return 'Must be a positive integer'; }; -// Validation function for positive decimal numbers export function validatePositiveDecimal(value: any) { // Allow empty, NaN, or non-number values without showing an error if (value === '' || value === null || isNaN(value) || typeof value !== 'number') {