Issue #671 fix trigger name validation (#794)

Signed-off-by: Chenxi Wang <wangchenxi.us@gmail.com> (cherry picked from commit 83a2b3e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Nov 27, 2023 · 6d233f6 · 6d233f6
1 parent c0691e0
commit 6d233f6
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 41 deletions.
diff --git a/public/pages/CreateTrigger/containers/DefineBucketLevelTrigger/DefineBucketLevelTrigger.js b/public/pages/CreateTrigger/containers/DefineBucketLevelTrigger/DefineBucketLevelTrigger.js
@@ -309,7 +309,14 @@ class DefineBucketLevelTrigger extends Component {
         <div style={{ padding: '0px 10px', paddingTop: '10px' }}>
           <FormikFieldText
             name={`${fieldPath}name`}
-            fieldProps={{ validate: validateTriggerName(triggers, triggerValues, fieldPath) }}
+            fieldProps={{
+              validate: (val) =>
+                validateTriggerName(
+                  triggerValues?.triggerDefinitions,
+                  triggerIndex,
+                  setFlyout != null
+                )(val),
+            }}
             formRow
             rowProps={defaultRowProps}
             inputProps={defaultInputProps}

diff --git a/...pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js b/...pages/CreateTrigger/containers/DefineCompositeLevelTrigger/DefineCompositeLevelTrigger.js
@@ -12,6 +12,7 @@ import { hasError, isInvalid, required } from '../../../../utils/validate';
 import { DEFAULT_TRIGGER_NAME, SEVERITY_OPTIONS } from '../../utils/constants';
 import CompositeTriggerCondition from '../../components/CompositeTriggerCondition/CompositeTriggerCondition';
 import TriggerNotifications from './TriggerNotifications';
+import { validateTriggerName } from '../DefineTrigger/utils/validation';
 import { FORMIK_COMPOSITE_INITIAL_TRIGGER_VALUES } from '../CreateTrigger/utils/constants';
 import { titleTemplate } from '../../../../utils/helpers';
 
@@ -100,7 +101,7 @@ class DefineCompositeLevelTrigger extends Component {
           <FormikFieldText
             name={`${formikFieldPath}name`}
             fieldProps={{
-              validate: required,
+              validate: (val) => validateTriggerName(values?.triggerDefinitions, triggerIndex)(val),
             }}
             formRow
             rowProps={{

diff --git a/...c/pages/CreateTrigger/containers/DefineDocumentLevelTrigger/DefineDocumentLevelTrigger.js b/...c/pages/CreateTrigger/containers/DefineDocumentLevelTrigger/DefineDocumentLevelTrigger.js
@@ -276,7 +276,14 @@ class DefineDocumentLevelTrigger extends Component {
         <div style={{ padding: '0px 10px', paddingTop: '10px' }}>
           <FormikFieldText
             name={`${fieldPath}name`}
-            fieldProps={{ validate: validateTriggerName(triggers, triggerValues, fieldPath) }}
+            fieldProps={{
+              validate: (val) =>
+                validateTriggerName(
+                  triggerValues?.triggerDefinitions,
+                  triggerIndex,
+                  setFlyout !== null
+                )(val),
+            }}
             formRow
             rowProps={defaultRowProps}
             inputProps={defaultInputProps}

diff --git a/public/pages/CreateTrigger/containers/DefineTrigger/DefineTrigger.js b/public/pages/CreateTrigger/containers/DefineTrigger/DefineTrigger.js
@@ -239,7 +239,8 @@ class DefineTrigger extends Component {
       <FormikFieldText
         name={`${fieldPath}name`}
         fieldProps={{
-          validate: validateTriggerName(triggers, triggerValues, fieldPath, flyoutMode),
+          validate: (val) =>
+            validateTriggerName(triggerValues?.triggerDefinitions, triggerIndex, flyoutMode)(val),
         }}
         formRow
         rowProps={{ ...defaultRowProps, ...(flyoutMode ? { style: {} } : {}) }}

diff --git a/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.js b/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.js
@@ -2,29 +2,19 @@
  * Copyright OpenSearch Contributors
  * SPDX-License-Identifier: Apache-2.0
  */
-import _ from 'lodash';
 
-import { FORMIK_INITIAL_TRIGGER_VALUES, TRIGGER_TYPE } from '../../CreateTrigger/utils/constants';
+export const validateTriggerName = (triggers = [], index, isFlyOut = false) => {
+  return (value) => {
+    const trimmedValue = value.trim();
+    if (!trimmedValue) return isFlyOut ? 'Required.' : 'Trigger name is required.';
+    const triggerNameExistWithIndex = triggers.some((trigger, i) => {
+      return i !== index && trimmedValue === trigger.name.trim();
+    });
+    if (triggerNameExistWithIndex) {
+      return 'Trigger name already used.';
+    }
 
-export const validateTriggerName = (triggers, triggerToEdit, fieldPath, isFullText) => (value) => {
-  if (!value) return isFullText ? 'Trigger name is required.' : 'Required.';
-  const nameExists = triggers.filter((trigger) => {
-    const triggerId = _.get(
-      trigger,
-      `${TRIGGER_TYPE.BUCKET_LEVEL}.id`,
-      _.get(trigger, `${TRIGGER_TYPE.QUERY_LEVEL}.id`)
-    );
-    const triggerName = _.get(
-      trigger,
-      `${TRIGGER_TYPE.BUCKET_LEVEL}.name`,
-      _.get(trigger, `${TRIGGER_TYPE.QUERY_LEVEL}.name`, FORMIK_INITIAL_TRIGGER_VALUES.name)
-    );
-    const triggerToEditId = _.get(triggerToEdit, `${fieldPath}id`, triggerToEdit.id);
-    return triggerToEditId !== triggerId && triggerName.toLowerCase() === value.toLowerCase();
-  });
-  if (nameExists.length > 0) {
-    return 'Trigger name already used.';
-  }
-  // TODO: character restrictions
-  // TODO: character limits
+    // TODO: character restrictions
+    // TODO: character limits
+  };
 };
diff --git a/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.test.js b/public/pages/CreateTrigger/containers/DefineTrigger/utils/validation.test.js
@@ -4,28 +4,24 @@
  */
 
 import { validateTriggerName } from './validation';
-import { TRIGGER_TYPE } from '../../CreateTrigger/utils/constants';
 
 describe('validateTriggerName', () => {
   test('returns undefined if no error', () => {
-    expect(validateTriggerName([], {})('valid trigger name')).toBeUndefined();
+    expect(validateTriggerName([], 0)('valid trigger name')).toBeUndefined();
   });
 
   test('returns Required string if falsy value', () => {
-    expect(validateTriggerName([], {})()).toBe('Required.');
-    expect(validateTriggerName([], {})('')).toBe('Required.');
+    expect(validateTriggerName([], 0)('')).toBe('Trigger name is required.');
   });
-  test('returns false if name already exists in monitor while creates new trigger', () => {
-    const triggers = [{ [TRIGGER_TYPE.QUERY_LEVEL]: { id: '123', name: 'Test' } }];
-    expect(validateTriggerName(triggers, { [TRIGGER_TYPE.QUERY_LEVEL]: {} })('Test')).toBe(
-      'Trigger name already used.'
-    );
+  test('returns Required short version string if falsy value', () => {
+    expect(validateTriggerName([], 0, true)('')).toBe('Required.');
   });
-
   test('returns undefined if editing trigger and name is the same', () => {
-    const triggers = [{ id: '123', name: 'Test' }];
-    expect(
-      validateTriggerName(triggers, { [TRIGGER_TYPE.QUERY_LEVEL]: { id: '123' } })('Test')
-    ).toBeUndefined();
+    const triggers = [{ name: 'Test' }];
+    expect(validateTriggerName(triggers, 0)('Test')).toBeUndefined();
+  });
+  test('returns false if name already exists in monitor while creates new trigger', () => {
+    const triggers = [{ name: 'Test' }];
+    expect(validateTriggerName(triggers, 1)('Test')).toBe('Trigger name already used.');
   });
 });