From 2badeeae0159b2b9ef4d765dd911de63db96686d Mon Sep 17 00:00:00 2001 From: Khristinin Nikita Date: Mon, 30 May 2022 18:09:12 +0200 Subject: [PATCH] Security form fix (#132834) **Addresses:** https://github.com/elastic/kibana/issues/130767, https://github.com/elastic/kibana/issues/130770 Related to [this PR](https://github.com/elastic/kibana/pull/130825). @banderror described the reason for the problems in his PR. There was [ask ](https://github.com/elastic/kibana/pull/130825#pullrequestreview-951482981) to not return the `useEffect` to the form lib. The fix of the form clears the values [in this commit](https://github.com/elastic/kibana/pull/132834/commits/df0b7bb9059ae0c1331eb6eec31738ea002ea295) Then I cherry-pick commits from @banderror PR to fix form stripping undefined values and also fixed some tests ### Checklist Delete any items that are not applicable to this PR. - [ ] [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 (cherry picked from commit 52bdf9ad7f81867f2149528790369c93418bd43e) --- .../detection_rules/custom_query_rule.spec.ts | 11 ++- .../indicator_match_rule.spec.ts | 12 ++-- .../eql_query_bar/eql_query_bar.test.tsx | 1 + .../rules/eql_query_bar/eql_query_bar.tsx | 1 + .../components/rules/query_bar/index.tsx | 6 +- .../rules/rule_preview/helpers.test.ts | 27 ++++--- .../rules/rule_preview/index.test.tsx | 2 + .../rules/step_define_rule/index.tsx | 72 +++++++++++++------ .../rules/create/helpers.test.ts | 1 + .../detection_engine/rules/create/helpers.ts | 8 +-- .../detection_engine/rules/create/index.tsx | 16 +++++ .../detection_engine/rules/helpers.test.tsx | 8 +-- .../pages/detection_engine/rules/helpers.tsx | 4 +- .../timeline/query_bar/eql/index.tsx | 2 +- 14 files changed, 111 insertions(+), 60 deletions(-) diff --git a/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts b/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts index 4f62fb199272f..92bd07b8a24ba 100644 --- a/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts @@ -26,12 +26,12 @@ import { SHOWING_RULES_TEXT, } from '../../screens/alerts_detection_rules'; import { - // ABOUT_CONTINUE_BTN, - // ABOUT_EDIT_BUTTON, + ABOUT_CONTINUE_BTN, + ABOUT_EDIT_BUTTON, ACTIONS_THROTTLE_INPUT, CUSTOM_QUERY_INPUT, - // DEFINE_CONTINUE_BUTTON, - // DEFINE_EDIT_BUTTON, + DEFINE_CONTINUE_BUTTON, + DEFINE_EDIT_BUTTON, DEFINE_INDEX_INPUT, DEFAULT_RISK_SCORE_INPUT, RULE_DESCRIPTION_INPUT, @@ -134,7 +134,6 @@ describe('Custom query rules', () => { fillAboutRuleAndContinue(this.rule); fillScheduleRuleAndContinue(this.rule); - /* Commenting this piece of code due to the following issue: https://github.com/elastic/kibana/issues/130767 // expect define step to repopulate cy.get(DEFINE_EDIT_BUTTON).click(); cy.get(CUSTOM_QUERY_INPUT).should('have.value', this.rule.customQuery); @@ -145,7 +144,7 @@ describe('Custom query rules', () => { cy.get(ABOUT_EDIT_BUTTON).click(); cy.get(RULE_NAME_INPUT).invoke('val').should('eql', this.rule.name); cy.get(ABOUT_CONTINUE_BTN).should('exist').click({ force: true }); - cy.get(ABOUT_CONTINUE_BTN).should('not.exist'); */ + cy.get(ABOUT_CONTINUE_BTN).should('not.exist'); createAndEnableRule(); diff --git a/x-pack/plugins/security_solution/cypress/integration/detection_rules/indicator_match_rule.spec.ts b/x-pack/plugins/security_solution/cypress/integration/detection_rules/indicator_match_rule.spec.ts index 1d25e2eb0c508..faa907be8810c 100644 --- a/x-pack/plugins/security_solution/cypress/integration/detection_rules/indicator_match_rule.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/detection_rules/indicator_match_rule.spec.ts @@ -178,13 +178,11 @@ describe('indicator match', () => { selectIndicatorMatchType(); }); - // Unskip once https://github.com/elastic/kibana/issues/130770 is fixed - it.skip('Has a default set of *:*', () => { + it('Has a default set of *:*', () => { getCustomQueryInput().should('have.text', '*:*'); }); - // Unskip once https://github.com/elastic/kibana/issues/1307707 is fixed - it.skip('Shows invalidation text if text is removed', () => { + it('Shows invalidation text if text is removed', () => { getCustomQueryInput().type('{selectall}{del}'); getCustomQueryInvalidationText().should('exist'); }); @@ -401,8 +399,7 @@ describe('indicator match', () => { }); describe('Schedule', () => { - // Unskip once https://github.com/elastic/kibana/issues/1307707 is fixed - it.skip('IM rule has 1h time interval and lookback by default', () => { + it('IM rule has 1h time interval and lookback by default', () => { visitWithoutDateRange(RULE_CREATION); selectIndicatorMatchType(); fillDefineIndicatorMatchRuleAndContinue(getNewThreatIndicatorRule()); @@ -421,8 +418,7 @@ describe('indicator match', () => { deleteAlertsAndRules(); }); - // Unskip once https://github.com/elastic/kibana/issues/1307707 is fixed - it.skip('Creates and enables a new Indicator Match rule', () => { + it('Creates and enables a new Indicator Match rule', () => { visitWithoutDateRange(RULE_CREATION); selectIndicatorMatchType(); fillDefineIndicatorMatchRuleAndContinue(getNewThreatIndicatorRule()); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx index 3b42e99236f87..8a09ff0be0db3 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.test.tsx @@ -48,6 +48,7 @@ describe('EqlQueryBar', () => { query: 'newQuery', language: 'eql', }, + saved_id: null, }; expect(mockField.setValue).toHaveBeenCalledWith(expected); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx index a8f6b81b29041..70f4723c8d92f 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/eql_query_bar/eql_query_bar.tsx @@ -96,6 +96,7 @@ export const EqlQueryBar: FC = ({ query: newQuery, language: 'eql', }, + saved_id: null, }); }, [setValue, onValiditingChange] diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx index 5c8d2643eb445..63e9cb27fc082 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/query_bar/index.tsx @@ -29,7 +29,7 @@ import * as i18n from './translations'; export interface FieldValueQueryBar { filters: Filter[]; query: Query; - saved_id?: string; + saved_id: string | null; } interface QueryBarDefineRuleProps { browserFields: BrowserFields; @@ -176,7 +176,7 @@ export const QueryBarDefineRule = ({ query: '', language: 'kuery', }, - saved_id: undefined, + saved_id: null, }); } } @@ -210,7 +210,7 @@ export const QueryBarDefineRule = ({ ? [...newFilters, getDataProviderFilter(dataProvidersDsl)] : newFilters, query: newQuery, - saved_id: undefined, + saved_id: null, }); }, [browserFields, indexPattern, setFieldValue] diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/helpers.test.ts b/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/helpers.test.ts index 382d0c5473984..8621201a399e7 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/helpers.test.ts @@ -75,7 +75,7 @@ describe('query_preview/helpers', () => { { entries: [{ field: 'test-field', value: 'test-value', type: 'mapping' }] }, ], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -91,7 +91,7 @@ describe('query_preview/helpers', () => { { entries: [{ field: 'test-field', value: 'test-value', type: 'mapping' }] }, ], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -107,7 +107,7 @@ describe('query_preview/helpers', () => { { entries: [{ field: 'test-field', value: 'test-value', type: 'mapping' }] }, ], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -123,7 +123,7 @@ describe('query_preview/helpers', () => { { entries: [{ field: 'test-field', value: 'test-value', type: 'mapping' }] }, ], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -137,7 +137,7 @@ describe('query_preview/helpers', () => { threatIndex: ['threat-*'], threatMapping: [], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -151,7 +151,7 @@ describe('query_preview/helpers', () => { threatIndex: ['threat-*'], threatMapping: [], machineLearningJobId: [], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -165,7 +165,7 @@ describe('query_preview/helpers', () => { threatIndex: ['threat-*'], threatMapping: [], machineLearningJobId: [], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(true); }); @@ -181,7 +181,7 @@ describe('query_preview/helpers', () => { { entries: [{ field: 'test-field', value: 'test-value', type: 'mapping' }] }, ], machineLearningJobId: ['test-ml-job-id'], - queryBar: { filters: [], query: { query: '', language: 'testlang' } }, + queryBar: { filters: [], query: { query: '', language: 'testlang' }, saved_id: null }, }); expect(isDisabled).toEqual(false); }); @@ -195,7 +195,11 @@ describe('query_preview/helpers', () => { threatIndex: ['threat-*'], threatMapping: [], machineLearningJobId: [], - queryBar: { filters: [], query: { query: 'any where true', language: 'testlang' } }, + queryBar: { + filters: [], + query: { query: 'any where true', language: 'testlang' }, + saved_id: null, + }, }); expect(isDisabled).toEqual(false); }); @@ -234,6 +238,7 @@ describe('query_preview/helpers', () => { { query: { query: 'host.name:*', language: 'kuery' }, filters: [{ meta: { alias: '', disabled: false, negate: false } }], + saved_id: null, }, ['foo-*'], 'query' @@ -260,6 +265,7 @@ describe('query_preview/helpers', () => { { query: { query: 'host.name:*', language: 'kuery' }, filters: [{ meta: { alias: '', disabled: false, negate: false } }], + saved_id: null, }, ['foo-*'], 'saved_query' @@ -286,6 +292,7 @@ describe('query_preview/helpers', () => { { query: { query: 'host.name:*', language: 'kuery' }, filters: [{ meta: { alias: '', disabled: false, negate: false } }], + saved_id: null, }, ['foo-*'], 'threshold' @@ -312,6 +319,7 @@ describe('query_preview/helpers', () => { { query: { query: 'file where true', language: 'eql' }, filters: [{ meta: { alias: '', disabled: false, negate: false } }], + saved_id: null, }, ['foo-*'], 'eql' @@ -329,6 +337,7 @@ describe('query_preview/helpers', () => { { query: { query: 'host.name:', language: 'kuery' }, filters: [{ meta: { alias: '', disabled: false, negate: false } }], + saved_id: null, }, ['foo-*'], 'threshold' diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/index.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/index.test.tsx index b142aae860336..29cb0974006aa 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/rule_preview/index.test.tsx @@ -40,10 +40,12 @@ const defaultProps: RulePreviewProps = { query: { filters: [], query: { query: 'file.hash.md5:*', language: 'kuery' }, + saved_id: null, }, threatQuery: { filters: [], query: { query: 'threat.indicator.file.hash.md5:*', language: 'kuery' }, + saved_id: null, }, threshold: { field: ['agent.hostname'], diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx index c4ec9d4653291..931428a076068 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx @@ -10,6 +10,7 @@ import React, { FC, memo, useCallback, useMemo, useState, useEffect } from 'reac import styled from 'styled-components'; import { isEqual, isEmpty } from 'lodash'; import { FieldSpec } from '@kbn/data-views-plugin/common'; +import usePrevious from 'react-use/lib/usePrevious'; import { DEFAULT_INDEX_KEY, @@ -75,12 +76,12 @@ export const stepDefineDefaultValue: DefineStepRule = { queryBar: { query: { query: '', language: 'kuery' }, filters: [], - saved_id: undefined, + saved_id: null, }, threatQueryBar: { query: { query: DEFAULT_THREAT_MATCH_QUERY, language: 'kuery' }, filters: [], - saved_id: undefined, + saved_id: null, }, requiredFields: [], relatedIntegrations: [], @@ -111,6 +112,11 @@ const threatQueryBarDefaultValue: DefineStepRule['queryBar'] = { query: { ...stepDefineDefaultValue.queryBar.query, query: '*:*' }, }; +const defaultCustomQuery = { + forNormalRules: stepDefineDefaultValue.queryBar, + forThreatMatchRules: threatQueryBarDefaultValue, +}; + export const MyLabelButton = styled(EuiButtonEmpty)` height: 18px; font-size: 12px; @@ -195,6 +201,7 @@ const StepDefineRuleComponent: FC = ({ const machineLearningJobId = formMachineLearningJobId ?? initialState.machineLearningJobId; const anomalyThreshold = formAnomalyThreshold ?? initialState.anomalyThreshold; const ruleType = formRuleType || initialState.ruleType; + const previousRuleType = usePrevious(ruleType); const [indexPatternsLoading, { browserFields, indexPatterns }] = useFetchIndex(index); const fields: Readonly = aggregatableFields(browserFields); const [optionsSelected, setOptionsSelected] = useState( @@ -220,36 +227,55 @@ const StepDefineRuleComponent: FC = ({ }, [threatIndex, threatIndicesConfig]); /** - * When a rule type is changed to or from a threat match this will modify the - * default query string to either: - * * from the empty string '' to '*:*' if the rule type is "threatMatchRule" - * * from '*:*' back to the empty string '' if the rule type is not "threatMatchRule" - * This calls queryBar.reset() in both cases to not trigger validation errors as - * the user has not entered data into those areas yet. - * If the user has entered data then through reference compares we can detect reliably if - * the user has changed data. - * * queryBar.value === defaultQueryBar (Has the user changed the input of '' yet?) - * * queryBar.value === threatQueryBarDefaultValue (Has the user changed the input of '*:*' yet?) - * This is a stronger guarantee than "isPristine" off of the forms as that value can be reset - * if you go to step 2) and then back to step 1) or the form is reset in another way. Using - * the reference compare we know factually if the data is changed as the references must change - * in the form libraries form the initial defaults. + * When the user changes rule type to or from "threat_match" this will modify the + * default "Custom query" string to either: + * * from '' to '*:*' if the type is switched to "threat_match" + * * from '*:*' back to '' if the type is switched back from "threat_match" to another one */ useEffect(() => { const { queryBar } = getFields(); - if (queryBar != null) { - const { queryBar: defaultQueryBar } = stepDefineDefaultValue; - if (isThreatMatchRule(ruleType) && queryBar.value === defaultQueryBar) { + if (queryBar == null) { + return; + } + + // NOTE: Below this code does two things that are worth commenting. + + // 1. If the user enters some text in the "Custom query" form field, we want + // to keep it even if the user switched to another rule type. So we want to + // be able to figure out if the field has been modified. + // - The forms library provides properties (isPristine, isModified, isDirty) + // for that but they can't be used in our case: their values can be reset + // if you go to step 2 and then back to step 1 or the form is reset in another way. + // - That's why we compare the actual value of the field with default ones. + // NOTE: It's important to do a deep object comparison by value. + // Don't do it by reference because the forms lib can change it internally. + + // 2. We call queryBar.reset() in both cases to not trigger validation errors + // as the user has not entered data into those areas yet. + + // If the user switched rule type to "threat_match" from any other one, + // but hasn't changed the custom query used for normal rules (''), + // we reset the custom query to the default used for "threat_match" rules ('*:*'). + if (isThreatMatchRule(ruleType) && !isThreatMatchRule(previousRuleType)) { + if (isEqual(queryBar.value, defaultCustomQuery.forNormalRules)) { queryBar.reset({ - defaultValue: threatQueryBarDefaultValue, + defaultValue: defaultCustomQuery.forThreatMatchRules, }); - } else if (queryBar.value === threatQueryBarDefaultValue) { + return; + } + } + + // If the user switched rule type from "threat_match" to any other one, + // but hasn't changed the custom query used for "threat_match" rules ('*:*'), + // we reset the custom query to another default value (''). + if (!isThreatMatchRule(ruleType) && isThreatMatchRule(previousRuleType)) { + if (isEqual(queryBar.value, defaultCustomQuery.forThreatMatchRules)) { queryBar.reset({ - defaultValue: defaultQueryBar, + defaultValue: defaultCustomQuery.forNormalRules, }); } } - }, [ruleType, getFields]); + }, [ruleType, previousRuleType, getFields]); const handleSubmit = useCallback(() => { if (onSubmit) { diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts index 69993cd438189..4cca73b4ffcc2 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.test.ts @@ -364,6 +364,7 @@ describe('helpers', () => { threatQueryBar: { query: { language: 'kql', query: 'threat_host: *' }, filters: threatFilters, + saved_id: null, }, threatMapping, }; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts index 57264ec43e18e..214ec2373c949 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/helpers.ts @@ -296,7 +296,7 @@ export const formatDefineStepData = (defineStepData: DefineStepRule): DefineStep filters: ruleFields.queryBar?.filters, language: ruleFields.queryBar?.query?.language, query: ruleFields.queryBar?.query?.query as string, - saved_id: ruleFields.queryBar?.saved_id, + saved_id: ruleFields.queryBar?.saved_id ?? undefined, ...(ruleType === 'threshold' && { threshold: { field: ruleFields.threshold?.field ?? [], @@ -320,7 +320,7 @@ export const formatDefineStepData = (defineStepData: DefineStepRule): DefineStep filters: ruleFields.queryBar?.filters, language: ruleFields.queryBar?.query?.language, query: ruleFields.queryBar?.query?.query as string, - saved_id: ruleFields.queryBar?.saved_id, + saved_id: ruleFields.queryBar?.saved_id ?? undefined, threat_index: ruleFields.threatIndex, threat_query: ruleFields.threatQueryBar?.query?.query as string, threat_filters: ruleFields.threatQueryBar?.filters, @@ -333,7 +333,7 @@ export const formatDefineStepData = (defineStepData: DefineStepRule): DefineStep filters: ruleFields.queryBar?.filters, language: ruleFields.queryBar?.query?.language, query: ruleFields.queryBar?.query?.query as string, - saved_id: ruleFields.queryBar?.saved_id, + saved_id: ruleFields.queryBar?.saved_id ?? undefined, timestamp_field: ruleFields.eqlOptions?.timestampField, event_category_override: ruleFields.eqlOptions?.eventCategoryField, tiebreaker_field: ruleFields.eqlOptions?.tiebreakerField, @@ -343,7 +343,7 @@ export const formatDefineStepData = (defineStepData: DefineStepRule): DefineStep filters: ruleFields.queryBar?.filters, language: ruleFields.queryBar?.query?.language, query: ruleFields.queryBar?.query?.query as string, - saved_id: ruleFields.queryBar?.saved_id, + saved_id: ruleFields.queryBar?.saved_id ?? undefined, ...(ruleType === 'query' && ruleFields.queryBar?.saved_id && { type: 'saved_query' as Type }), }; diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx index c37cba0e2b57f..42ba9fafd3ea2 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/create/index.tsx @@ -77,6 +77,9 @@ const MyEuiPanel = styled(EuiPanel)<{ MyEuiPanel.displayName = 'MyEuiPanel'; +const isShouldRerenderStep = (step: RuleStep, activeStep: RuleStep) => + activeStep !== step ? '0' : '1'; + const CreateRulePageComponent: React.FC = () => { const [ { @@ -286,6 +289,7 @@ const CreateRulePageComponent: React.FC = () => { }); return null; } + return ( <> @@ -330,6 +334,9 @@ const CreateRulePageComponent: React.FC = () => { setForm={setFormHook} onSubmit={submitStepDefineRule} descriptionColumns="singleSplit" + // We need a key to make this component remount when edit/view mode is toggled + // https://github.com/elastic/kibana/pull/132834#discussion_r881705566 + key={isShouldRerenderStep(RuleStep.defineRule, activeStep)} /> @@ -365,6 +372,9 @@ const CreateRulePageComponent: React.FC = () => { isLoading={isLoading || loading} setForm={setFormHook} onSubmit={() => submitStep(RuleStep.aboutRule)} + // We need a key to make this component remount when edit/view mode is toggled + // https://github.com/elastic/kibana/pull/132834#discussion_r881705566 + key={isShouldRerenderStep(RuleStep.aboutRule, activeStep)} /> @@ -399,6 +409,9 @@ const CreateRulePageComponent: React.FC = () => { isLoading={isLoading || loading} setForm={setFormHook} onSubmit={() => submitStep(RuleStep.scheduleRule)} + // We need a key to make this component remount when edit/view mode is toggled + // https://github.com/elastic/kibana/pull/132834#discussion_r881705566 + key={isShouldRerenderStep(RuleStep.scheduleRule, activeStep)} /> @@ -432,6 +445,9 @@ const CreateRulePageComponent: React.FC = () => { setForm={setFormHook} onSubmit={() => submitStep(RuleStep.ruleActions)} actionMessageParams={actionMessageParams} + // We need a key to make this component remount when edit/view mode is toggled + // https://github.com/elastic/kibana/pull/132834#discussion_r881705566 + key={isShouldRerenderStep(RuleStep.ruleActions, activeStep)} /> diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx index 1a90282555e9d..8ec3ec057ebee 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx @@ -99,7 +99,7 @@ describe('rule helpers', () => { language: '', }, filters: [], - saved_id: undefined, + saved_id: null, }, timeline: { id: '86aa74d0-2136-11ea-9864-ebc8cc1cb8c2', @@ -236,7 +236,7 @@ describe('rule helpers', () => { language: '', }, filters: [], - saved_id: undefined, + saved_id: null, }, timeline: { id: '86aa74d0-2136-11ea-9864-ebc8cc1cb8c2', @@ -269,7 +269,7 @@ describe('rule helpers', () => { language: 'kuery', }, filters: [], - saved_id: undefined, + saved_id: null, }, relatedIntegrations: [], requiredFields: [], @@ -285,7 +285,7 @@ describe('rule helpers', () => { language: '', }, filters: [], - saved_id: undefined, + saved_id: null, }, timeline: { id: '86aa74d0-2136-11ea-9864-ebc8cc1cb8c2', diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx index b1061d758d59b..93e17efd38d5f 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx @@ -87,13 +87,13 @@ export const getDefineStepsData = (rule: Rule): DefineStepRule => ({ threatQueryBar: { query: { query: rule.threat_query ?? '', language: rule.threat_language ?? '' }, filters: (rule.threat_filters ?? []) as Filter[], - saved_id: undefined, + saved_id: null, }, threatMapping: rule.threat_mapping ?? [], queryBar: { query: { query: rule.query ?? '', language: rule.language ?? '' }, filters: (rule.filters ?? []) as Filter[], - saved_id: rule.saved_id, + saved_id: rule.saved_id ?? null, }, relatedIntegrations: rule.related_integrations ?? [], requiredFields: rule.required_fields ?? [], diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/query_bar/eql/index.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/query_bar/eql/index.tsx index fbe7c4cf8a921..7bfb934ecdeca 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/query_bar/eql/index.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/query_bar/eql/index.tsx @@ -37,7 +37,7 @@ const defaultValues = { eqlQueryBar: { query: { query: '', language: 'eql' }, filters: [], - saved_id: undefined, + saved_id: null, }, };