From 4062dab0db539215aedad1ce26b064c53bf68345 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 23 Nov 2020 16:11:23 -0600 Subject: [PATCH 1/5] Prevent error from being displayed when choosing action throttle Addresses #83230. This field was previously refactored to not require a callback prop; simply updating the value via `field.setValue` is sufficient for our use case. This fix removes the errant code that assumed a callback prop, since such a prop does not exist on the underlying component. --- .../components/rules/throttle_select_field/index.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/throttle_select_field/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/throttle_select_field/index.tsx index bf3498b28cd45..f0326913909b5 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/throttle_select_field/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/throttle_select_field/index.tsx @@ -25,14 +25,13 @@ export const DEFAULT_THROTTLE_OPTION = THROTTLE_OPTIONS[0]; type ThrottleSelectField = typeof SelectField; export const ThrottleSelectField: ThrottleSelectField = (props) => { + const { setValue } = props.field; const onChange = useCallback( (e) => { const throttle = e.target.value; - props.field.setValue(throttle); - props.handleChange(throttle); + setValue(throttle); }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [props.field.setValue, props.handleChange] + [setValue] ); const newEuiFieldProps = { ...props.euiFieldProps, onChange }; return ; From 405cd5c54e406b1adb611337475ec3a78f6f3199 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 23 Nov 2020 16:31:28 -0600 Subject: [PATCH 2/5] Fix UI bug on ML Jobs popover EUI links now add an SVG if they're an external link; our use of a div was causing that to wrap. Since the div was only needed to change the text size, a refactor makes this all work. --- .../components/ml_popover/jobs_table/jobs_table.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/ml_popover/jobs_table/jobs_table.tsx b/x-pack/plugins/security_solution/public/common/components/ml_popover/jobs_table/jobs_table.tsx index 5e3045efe1f4d..54a2381ecf587 100644 --- a/x-pack/plugins/security_solution/public/common/components/ml_popover/jobs_table/jobs_table.tsx +++ b/x-pack/plugins/security_solution/public/common/components/ml_popover/jobs_table/jobs_table.tsx @@ -57,9 +57,11 @@ const JobName = ({ id, description, basePath }: JobNameProps) => { return ( - - {id} - + + + {id} + + {description.length > truncateThreshold ? `${description.substring(0, truncateThreshold)}...` From 05b92cd6f63efd51b9e556ddb28cda2a18a112dc Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 23 Nov 2020 17:41:03 -0600 Subject: [PATCH 3/5] Exercise editing of tags in E2E tests These tests were recently skipped due to their improper teardown. Since that's a broader issue across most of these tests, I'm reopening these so we can get the coverage provided here for the time being. --- .../cypress/integration/alerts_detection_rules_custom.spec.ts | 3 +-- x-pack/plugins/security_solution/cypress/objects/rule.ts | 1 + .../security_solution/cypress/screens/create_new_rule.ts | 3 +++ .../plugins/security_solution/cypress/tasks/create_new_rule.ts | 2 ++ 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts b/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts index 596b92d064050..fb1f2920aaceb 100644 --- a/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts @@ -215,8 +215,7 @@ describe('Custom detection rules creation', () => { }); }); -// FLAKY: https://github.com/elastic/kibana/issues/83772 -describe.skip('Custom detection rules deletion and edition', () => { +describe('Custom detection rules deletion and edition', () => { beforeEach(() => { esArchiverLoad('custom_rules'); loginAndWaitForPageWithoutDateRange(DETECTIONS_URL); diff --git a/x-pack/plugins/security_solution/cypress/objects/rule.ts b/x-pack/plugins/security_solution/cypress/objects/rule.ts index 0bb4c8e356091..8ba545e242b47 100644 --- a/x-pack/plugins/security_solution/cypress/objects/rule.ts +++ b/x-pack/plugins/security_solution/cypress/objects/rule.ts @@ -265,4 +265,5 @@ export const editedRule = { ...existingRule, severity: 'Medium', description: 'Edited Rule description', + tags: [...existingRule.tags, 'edited'], }; diff --git a/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts b/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts index 618ddbad9f44a..d802e97363a68 100644 --- a/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts +++ b/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts @@ -146,6 +146,9 @@ export const TAGS_FIELD = export const TAGS_INPUT = '[data-test-subj="detectionEngineStepAboutRuleTags"] [data-test-subj="comboBoxSearchInput"]'; +export const TAGS_CLEAR_BUTTON = + '[data-test-subj="detectionEngineStepAboutRuleTags"] [data-test-subj="comboBoxClearButton"]'; + export const THRESHOLD_FIELD_SELECTION = '.euiFilterSelectItem'; export const THRESHOLD_INPUT_AREA = '[data-test-subj="thresholdInput"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts b/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts index 251a7ccc4b9c9..9db01878ccd5f 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts @@ -64,6 +64,7 @@ import { QUERY_PREVIEW_BUTTON, EQL_QUERY_PREVIEW_HISTOGRAM, EQL_QUERY_VALIDATION_SPINNER, + TAGS_CLEAR_BUTTON, } from '../screens/create_new_rule'; import { NOTIFICATION_TOASTS, TOAST_ERROR_CLASS } from '../screens/shared'; import { TIMELINE } from '../screens/timelines'; @@ -84,6 +85,7 @@ export const fillAboutRule = (rule: CustomRule | MachineLearningRule | Threshold cy.get(RISK_INPUT).clear({ force: true }).type(`${rule.riskScore}`, { force: true }); + cy.get(TAGS_CLEAR_BUTTON).click({ force: true }); rule.tags.forEach((tag) => { cy.get(TAGS_INPUT).type(`${tag}{enter}`, { force: true }); }); From 8563f435f76e9cff21bca8fe4d55faa84f00ecff Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Mon, 23 Nov 2020 17:45:46 -0600 Subject: [PATCH 4/5] useFetchIndex defaults to isLoading: false In the case where no index pattern is provided, the hook exits without doing any work but does not update the loading state; this had the downstream effect of disabling a form field that was waiting for this hook to stop loading. --- .../security_solution/public/common/containers/source/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/common/containers/source/index.tsx b/x-pack/plugins/security_solution/public/common/containers/source/index.tsx index 7e73a40f2f748..f245857f3d0db 100644 --- a/x-pack/plugins/security_solution/public/common/containers/source/index.tsx +++ b/x-pack/plugins/security_solution/public/common/containers/source/index.tsx @@ -126,7 +126,7 @@ export const useFetchIndex = ( const { data, notifications } = useKibana().services; const abortCtrl = useRef(new AbortController()); const previousIndexesName = useRef([]); - const [isLoading, setLoading] = useState(true); + const [isLoading, setLoading] = useState(false); const [state, setState] = useState({ browserFields: DEFAULT_BROWSER_FIELDS, From 130d67c8c106998f5de6581d396386716c69e43d Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 24 Nov 2020 14:12:44 -0600 Subject: [PATCH 5/5] Move situational action into ... the situation We only need to clear existing tags in the case where we're editing the rule (and it has tags); in all other cases, this method fails. This fixes things by moving that conditional logic (clear the tags field) into the test that needs it (editing custom rules). --- .../cypress/integration/alerts_detection_rules_custom.spec.ts | 2 ++ .../plugins/security_solution/cypress/tasks/create_new_rule.ts | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts b/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts index fb1f2920aaceb..dfe984cba3816 100644 --- a/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts @@ -38,6 +38,7 @@ import { SCHEDULE_INTERVAL_AMOUNT_INPUT, SCHEDULE_INTERVAL_UNITS_INPUT, SEVERITY_DROPDOWN, + TAGS_CLEAR_BUTTON, TAGS_FIELD, } from '../screens/create_new_rule'; import { @@ -327,6 +328,7 @@ describe('Custom detection rules deletion and edition', () => { cy.get(ACTIONS_THROTTLE_INPUT).invoke('val').should('eql', 'no_actions'); goToAboutStepTab(); + cy.get(TAGS_CLEAR_BUTTON).click({ force: true }); fillAboutRule(editedRule); saveEditedRule(); diff --git a/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts b/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts index 9db01878ccd5f..251a7ccc4b9c9 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts @@ -64,7 +64,6 @@ import { QUERY_PREVIEW_BUTTON, EQL_QUERY_PREVIEW_HISTOGRAM, EQL_QUERY_VALIDATION_SPINNER, - TAGS_CLEAR_BUTTON, } from '../screens/create_new_rule'; import { NOTIFICATION_TOASTS, TOAST_ERROR_CLASS } from '../screens/shared'; import { TIMELINE } from '../screens/timelines'; @@ -85,7 +84,6 @@ export const fillAboutRule = (rule: CustomRule | MachineLearningRule | Threshold cy.get(RISK_INPUT).clear({ force: true }).type(`${rule.riskScore}`, { force: true }); - cy.get(TAGS_CLEAR_BUTTON).click({ force: true }); rule.tags.forEach((tag) => { cy.get(TAGS_INPUT).type(`${tag}{enter}`, { force: true }); });