[Security Solution][Detections] Rule forms cleanup

x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts

@@ -58,6 +58,8 @@ import {
   createAndActivateRule,
   fillAboutRuleAndContinue,
   fillDefineCustomRuleWithImportedQueryAndContinue,
+  expectDefineFormToRepopulateAndContinue,
+  expectAboutFormToRepopulateAndContinue,
 } from '../tasks/create_new_rule';
 import { esArchiverLoad, esArchiverUnload } from '../tasks/es_archiver';
 import { loginAndWaitForPageWithoutDateRange } from '../tasks/login';
@@ -82,6 +84,8 @@ describe('Detection rules, custom', () => {
     goToCreateNewRule();
     fillDefineCustomRuleWithImportedQueryAndContinue(newRule);
     fillAboutRuleAndContinue(newRule);
+    expectDefineFormToRepopulateAndContinue(newRule);
+    expectAboutFormToRepopulateAndContinue(newRule);
     createAndActivateRule();
 
     cy.get(CUSTOM_RULES_BTN).invoke('text').should('eql', 'Custom rules (1)');

x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts

@@ -6,6 +6,8 @@
 
 export const ABOUT_CONTINUE_BTN = '[data-test-subj="about-continue"]';
 
+export const ABOUT_EDIT_BUTTON = '[data-test-subj="edit-about-rule"]';
+
 export const ADD_FALSE_POSITIVE_BTN =
   '[data-test-subj="detectionEngineStepAboutRuleFalsePositives"] .euiButtonEmpty__text';
 
@@ -26,6 +28,8 @@ export const CUSTOM_QUERY_INPUT = '[data-test-subj="queryInput"]';
 
 export const DEFINE_CONTINUE_BUTTON = '[data-test-subj="define-continue"]';
 
+export const DEFINE_EDIT_BUTTON = '[data-test-subj="edit-define-rule"]';
+
 export const IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK =
   '[data-test-subj="importQueryFromSavedTimeline"]';
 

x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

@@ -48,6 +48,8 @@ import {
   THRESHOLD_FIELD_SELECTION,
   THRESHOLD_INPUT_AREA,
   THRESHOLD_TYPE,
+  DEFINE_EDIT_BUTTON,
+  ABOUT_EDIT_BUTTON,
 } from '../screens/create_new_rule';
 import { TIMELINE } from '../screens/timeline';
 
@@ -175,6 +177,20 @@ export const fillDefineCustomRuleWithImportedQueryAndContinue = (
   cy.get(CUSTOM_QUERY_INPUT).should('not.exist');
 };
 
+export const expectDefineFormToRepopulateAndContinue = (rule: CustomRule) => {
+  cy.get(DEFINE_EDIT_BUTTON).click();
+  cy.get(CUSTOM_QUERY_INPUT).invoke('text').should('eq', rule.customQuery);
+  cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
+  cy.get(DEFINE_CONTINUE_BUTTON).should('not.exist');
+};
+
+export const expectAboutFormToRepopulateAndContinue = (rule: CustomRule) => {
+  cy.get(ABOUT_EDIT_BUTTON).click();
+  cy.get(RULE_NAME_INPUT).invoke('val').should('eq', rule.name);
+  cy.get(ABOUT_CONTINUE_BTN).should('exist').click({ force: true });
+  cy.get(ABOUT_CONTINUE_BTN).should('not.exist');
+};
+
 export const fillDefineThresholdRuleAndContinue = (rule: ThresholdRule) => {
   const thresholdField = 0;
   const threshold = 1;

x-pack/plugins/security_solution/public/common/lib/kibana/kibana_react.ts

@@ -19,12 +19,11 @@ export interface WithKibanaProps {
   kibana: KibanaContext;
 }
 
-// eslint-disable-next-line react-hooks/rules-of-hooks
-const typedUseKibana = () => useKibana<StartServices>();
+const useTypedKibana = () => useKibana<StartServices>();
 
 export {
   KibanaContextProvider,
-  typedUseKibana as useKibana,
+  useTypedKibana as useKibana,
   useUiSetting,
   useUiSetting$,
   withKibana,

x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.test.tsx

@@ -7,7 +7,7 @@ import React from 'react';
 import { shallow, mount } from 'enzyme';
 
 import {
-  StepRuleDescriptionComponent,
+  StepRuleDescription,
   addFilterStateIfNotThere,
   buildListItems,
   getDescriptionItem,
@@ -52,24 +52,24 @@ describe('description_step', () => {
     mockAboutStep = mockAboutStepRule();
   });
 
-  describe('StepRuleDescriptionComponent', () => {
+  describe('StepRuleDescription', () => {
     test('renders tow columns when "columns" is "multi"', () => {
       const wrapper = shallow(
-        <StepRuleDescriptionComponent columns="multi" data={mockAboutStep} schema={schema} />
+        <StepRuleDescription columns="multi" data={mockAboutStep} schema={schema} />
       );
       expect(wrapper.find('[data-test-subj="listItemColumnStepRuleDescription"]')).toHaveLength(2);
     });
 
     test('renders single column when "columns" is "single"', () => {
       const wrapper = shallow(
-        <StepRuleDescriptionComponent columns="single" data={mockAboutStep} schema={schema} />
+        <StepRuleDescription columns="single" data={mockAboutStep} schema={schema} />
       );
       expect(wrapper.find('[data-test-subj="listItemColumnStepRuleDescription"]')).toHaveLength(1);
     });
 
     test('renders one column with title and description split when "columns" is "singleSplit', () => {
       const wrapper = shallow(
-        <StepRuleDescriptionComponent columns="singleSplit" data={mockAboutStep} schema={schema} />
+        <StepRuleDescription columns="singleSplit" data={mockAboutStep} schema={schema} />
       );
       expect(wrapper.find('[data-test-subj="listItemColumnStepRuleDescription"]')).toHaveLength(1);
       expect(
@@ -299,7 +299,6 @@ describe('description_step', () => {
     describe('queryBar', () => {
       test('returns array of ListItems when queryBar exist', () => {
         const mockQueryBar = {
-          isNew: false,
           queryBar: {
             query: {
               query: 'user.name: root or user.name: admin',
@@ -369,7 +368,6 @@ describe('description_step', () => {
     describe('threshold', () => {
       test('returns threshold description when threshold exist and field is empty', () => {
         const mockThreshold = {
-          isNew: false,
           threshold: {
             field: [''],
             value: 100,
@@ -391,7 +389,6 @@ describe('description_step', () => {
 
       test('returns threshold description when threshold exist and field is set', () => {
         const mockThreshold = {
-          isNew: false,
           threshold: {
             field: ['user.name'],
             value: 100,

x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx

@@ -37,7 +37,6 @@ import {
   buildRuleTypeDescription,
   buildThresholdDescription,
 } from './helpers';
-import { useSecurityJobs } from '../../../../common/components/ml_popover/hooks/use_security_jobs';
 import { buildMlJobDescription } from './ml_job_description';
 import { buildActionsDescription } from './actions_description';
 import { buildThrottleDescription } from './throttle_description';
@@ -52,22 +51,21 @@ const DescriptionListContainer = styled(EuiDescriptionList)`
   }
 `;
 
-interface StepRuleDescriptionProps {
+interface StepRuleDescriptionProps<T> {
   columns?: 'multi' | 'single' | 'singleSplit';
   data: unknown;
   indexPatterns?: IIndexPattern;
-  schema: FormSchema;
+  schema: FormSchema<T>;
 }
 
-export const StepRuleDescriptionComponent: React.FC<StepRuleDescriptionProps> = ({
+export const StepRuleDescriptionComponent = <T,>({
   data,
   columns = 'multi',
   indexPatterns,
   schema,
-}) => {
+}: StepRuleDescriptionProps<T>) => {
   const kibana = useKibana();
   const [filterManager] = useState<FilterManager>(new FilterManager(kibana.services.uiSettings));
-  const { jobs } = useSecurityJobs(false);
 
   const keys = Object.keys(schema);
   const listItems = keys.reduce((acc: ListItems[], key: string) => {
@@ -76,8 +74,7 @@ export const StepRuleDescriptionComponent: React.FC<StepRuleDescriptionProps> =
         ...acc,
         buildMlJobDescription(
           get(key, data) as string,
-          (get(key, schema) as { label: string }).label,
-          jobs
+          (get(key, schema) as { label: string }).label
         ),
       ];
     }
@@ -125,11 +122,13 @@ export const StepRuleDescriptionComponent: React.FC<StepRuleDescriptionProps> =
   );
 };
 
-export const StepRuleDescription = memo(StepRuleDescriptionComponent);
+export const StepRuleDescription = memo(
+  StepRuleDescriptionComponent
+) as typeof StepRuleDescriptionComponent;
 
-export const buildListItems = (
+export const buildListItems = <T,>(
   data: unknown,
-  schema: FormSchema,
+  schema: FormSchema<T>,
   filterManager: FilterManager,
   indexPatterns?: IIndexPattern
 ): ListItems[] =>

x-pack/plugins/security_solution/public/detections/components/rules/description_step/ml_job_description.test.tsx

@@ -14,7 +14,7 @@ jest.mock('../../../../common/lib/kibana');
 
 describe('MlJobDescription', () => {
   it('renders correctly', () => {
-    const wrapper = shallow(<MlJobDescription job={mockOpenedJob} />);
+    const wrapper = shallow(<MlJobDescription jobId={'myJobId'} />);
 
     expect(wrapper.find('[data-test-subj="machineLearningJobId"]')).toHaveLength(1);
   });

x-pack/plugins/security_solution/public/detections/components/rules/description_step/ml_job_description.tsx

@@ -10,6 +10,7 @@ import { EuiBadge, EuiIcon, EuiLink, EuiToolTip } from '@elastic/eui';
 
 import { MlSummaryJob } from '../../../../../../ml/public';
 import { isJobStarted } from '../../../../../common/machine_learning/helpers';
+import { useSecurityJobs } from '../../../../common/components/ml_popover/hooks/use_security_jobs';
 import { useKibana } from '../../../../common/lib/kibana';
 import { ListItems } from './types';
 import { ML_JOB_STARTED, ML_JOB_STOPPED } from './translations';
@@ -69,35 +70,33 @@ const Wrapper = styled.div`
   overflow: hidden;
 `;
 
-const MlJobDescriptionComponent: React.FC<{ job: MlSummaryJob }> = ({ job }) => {
+const MlJobDescriptionComponent: React.FC<{ jobId: string }> = ({ jobId }) => {
+  const { jobs } = useSecurityJobs(false);
   const jobUrl = useKibana().services.application.getUrlForApp(
-    `ml#/jobs?mlManagement=(jobId:${encodeURI(job.id)})`
+    `ml#/jobs?mlManagement=(jobId:${encodeURI(jobId)})`
   );
+  const job = jobs.find(({ id }) => id === jobId);
 
-  return (
+  const jobIdSpan = <span data-test-subj="machineLearningJobId">{jobId}</span>;
+
+  return job != null ? (
     <Wrapper>
       <div>
-        <JobLink data-test-subj="machineLearningJobId" href={jobUrl} target="_blank">
-          {job.id}
+        <JobLink href={jobUrl} target="_blank">
+          {jobIdSpan}
         </JobLink>
         <AuditIcon message={job.auditMessage} />
       </div>
       <JobStatusBadge job={job} />
     </Wrapper>
+  ) : (
+    jobIdSpan
   );
 };
 
 export const MlJobDescription = React.memo(MlJobDescriptionComponent);
 
-export const buildMlJobDescription = (
-  jobId: string,
-  label: string,
-  jobs: MlSummaryJob[]
-): ListItems => {
-  const job = jobs.find(({ id }) => id === jobId);
-
-  return {
-    title: label,
-    description: job ? <MlJobDescription job={job} /> : jobId,
-  };
-};
+export const buildMlJobDescription = (jobId: string, label: string): ListItems => ({
+  title: label,
+  description: <MlJobDescription jobId={jobId} />,
+});

x-pack/plugins/security_solution/public/detections/components/rules/next_step/index.tsx

@@ -9,7 +9,7 @@ import { EuiHorizontalRule, EuiFlexGroup, EuiFlexItem, EuiButton } from '@elasti
 import * as RuleI18n from '../../../pages/detection_engine/rules/translations';
 
 interface NextStepProps {
-  onClick: () => Promise<void>;
+  onClick: () => void;
   isDisabled: boolean;
   dataTestSubj?: string;
 }

x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/data.tsx

@@ -8,10 +8,9 @@ import styled from 'styled-components';
 import { EuiHealth } from '@elastic/eui';
 import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
 import React from 'react';
+import { SeverityValue } from '../../../pages/detection_engine/rules/types';
 import * as I18n from './translations';
 
-export type SeverityValue = 'low' | 'medium' | 'high' | 'critical';
-
 export interface SeverityOptionItem {
   value: SeverityValue;
   inputDisplay: React.ReactElement;

x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/default_value.ts

@@ -21,7 +21,6 @@ export const stepAboutDefaultValue: AboutStepRule = {
   description: '',
   isAssociatedToEndpointList: false,
   isBuildingBlock: false,
-  isNew: true,
   severity: { value: 'low', mapping: fillEmptySeverityMappings([]), isMappingChecked: false },
   riskScore: { value: 50, mapping: [], isMappingChecked: false },
   references: [''],