Code review Change the order of the sections #622

Signed-off-by: Jovan Cvetkovic <[email protected]>
opensearch-project · Jun 19, 2023 · 8c19090 · 8c19090
1 parent dac6581
commit 8c19090
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 33 deletions.
diff --git a/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx b/public/pages/Rules/components/RuleEditor/DetectionVisualEditor.tsx
@@ -110,6 +110,18 @@ export class DetectionVisualEditor extends React.Component<
   DetectionVisualEditorProps,
   DetectionVisualEditorState
 > {
+  /**
+   * Text area editor row height
+   * @private
+   */
+  private textareaRowHeight = 25;
+
+  /**
+   * Text area editor empty space to occupy before filling in the editor
+   * @private
+   */
+  private textareaEmptySpace = 40;
+
   constructor(props: DetectionVisualEditorProps) {
     super(props);
     this.state = {
@@ -136,7 +148,7 @@ export class DetectionVisualEditor extends React.Component<
 
     if (this.props.isInvalid != prevProps.isInvalid) {
       this.validateCondition(this.state.detectionObj.condition);
-      this.validateDatum(this.state.detectionObj.selections);
+      this.validateData(this.state.detectionObj.selections);
     }
   }
 
@@ -208,7 +220,7 @@ export class DetectionVisualEditor extends React.Component<
     return dump(compiledDetection);
   };
 
-  private validateDatum = (selections: Selection[]) => {
+  private validateData = (selections: Selection[]) => {
     const { errors } = this.state;
     selections.map((selection, selIdx) => {
       const fieldNames = new Set<string>();
@@ -276,7 +288,7 @@ export class DetectionVisualEditor extends React.Component<
         },
       },
       () => {
-        this.validateDatum(newSelections);
+        this.validateData(newSelections);
       }
     );
   };
@@ -361,7 +373,7 @@ export class DetectionVisualEditor extends React.Component<
   private updateCondition = (value: string) => {
     value = value.trim();
 
-    const detectionObj = { ...this.state.detectionObj, condition: value } as DetectionObject;
+    const detectionObj: DetectionObject = { ...this.state.detectionObj, condition: value };
     this.setState(
       {
         detectionObj,
@@ -438,7 +450,7 @@ export class DetectionVisualEditor extends React.Component<
   };
 
   private getTextareaHeight = (rowNo: number = 0) => {
-    return `${rowNo * 25 + 40}px`;
+    return `${rowNo * this.textareaRowHeight + this.textareaEmptySpace}px`;
   };
 
   render() {

diff --git a/public/pages/Rules/components/RuleEditor/components/SelectionExpField.tsx b/public/pages/Rules/components/RuleEditor/components/SelectionExpField.tsx
@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useCallback, useEffect, useState } from 'react';
 import {
   EuiPopoverTitle,
   EuiFlexItem,
@@ -149,6 +149,34 @@ export const SelectionExpField: React.FC<SelectionExpFieldProps> = ({
     </div>
   );
 
+  const onSelectionPopup = (e: Event, idx: number) => {
+    e.preventDefault();
+    openPopover(idx);
+  };
+
+  const onRemoveSelection = (idx: number) => {
+    const usedExp = _.cloneDeep(usedExpressions);
+    usedExp.splice(idx, 1);
+    usedExp.length && (usedExp[0].description = '');
+    setUsedExpressions([...usedExp]);
+    onChange(getValue(usedExp));
+  };
+
+  const onAddSelection = useCallback(() => {
+    const usedExp = _.cloneDeep(usedExpressions);
+    const differences = _.differenceBy(selections, usedExp, 'name');
+    const exp = [
+      ...usedExp,
+      {
+        description: usedExpressions.length ? 'AND' : '',
+        isOpen: false,
+        name: differences[0]?.name,
+      },
+    ];
+    setUsedExpressions(exp);
+    onChange(getValue(exp));
+  }, [usedExpressions, selections]);
+
   return (
     <EuiFlexGroup gutterSize="s" data-test-subj={dataTestSubj}>
       {!usedExpressions.length && (
@@ -184,10 +212,7 @@ export const SelectionExpField: React.FC<SelectionExpFieldProps> = ({
                 description={exp.description}
                 value={exp.name}
                 isActive={exp.isOpen}
-                onClick={(e: any) => {
-                  e.preventDefault();
-                  openPopover(idx);
-                }}
+                onClick={(e) => onSelectionPopup(e, idx)}
               />
             }
             isOpen={exp.isOpen}
@@ -200,36 +225,18 @@ export const SelectionExpField: React.FC<SelectionExpFieldProps> = ({
           <EuiButtonIcon
             data-test-subj={`selection-exp-field-item-remove-${idx}`}
             className={'selection-exp-field-item-remove'}
-            onClick={() => {
-              const usedExp = _.cloneDeep(usedExpressions);
-              usedExp.splice(idx, 1);
-              usedExp.length && (usedExp[0].description = '');
-              setUsedExpressions([...usedExp]);
-              onChange(getValue(usedExp));
-            }}
+            onClick={() => onRemoveSelection(idx)}
             color={'danger'}
             iconType="cross"
             aria-label={'Remove condition'}
+            key={idx}
           />
         </EuiFlexItem>
       ))}
       {selections.length > usedExpressions.length && (
         <EuiFlexItem grow={false} key={`selections_add`}>
           <EuiButtonIcon
-            onClick={() => {
-              const usedExp = _.cloneDeep(usedExpressions);
-              const differences = _.differenceBy(selections, usedExp, 'name');
-              const exp = [
-                ...usedExp,
-                {
-                  description: usedExpressions.length ? 'AND' : '',
-                  isOpen: false,
-                  name: differences[0]?.name,
-                },
-              ];
-              setUsedExpressions(exp);
-              onChange(getValue(exp));
-            }}
+            onClick={onAddSelection}
             color={'primary'}
             iconType="plusInCircleFilled"
             aria-label={'Add one more condition'}

diff --git a/public/utils/validation.ts b/public/utils/validation.ts
@@ -14,7 +14,7 @@ export const NAME_REGEX = new RegExp(/^[a-zA-Z0-9 _-]{5,50}$/);
 // numbers 0-9, hyphens, dot, and underscores.
 export const DETECTION_NAME_REGEX = new RegExp(/^[a-zA-Z0-9_.-]{5,50}$/);
 
-export const CONDITION_REGEX = new RegExp(
+export const DETECTION_CONDITION_REGEX = new RegExp(
   /^((not )?[a-zA-Z0-9_]+)?( (and|or|not) ?([a-zA-Z0-9_]+))*(?<!and|or|not)$/
 );
 
@@ -39,7 +39,10 @@ export function validateDetectionFieldName(
   return name.trim().match(regex) !== null;
 }
 
-export function validateCondition(name: string, regex: RegExp = CONDITION_REGEX): boolean {
+export function validateCondition(
+  name: string,
+  regex: RegExp = DETECTION_CONDITION_REGEX
+): boolean {
   return name.trim().match(regex) !== null;
 }