Skip to content

Commit

Permalink
Issue opensearch-project#671 fix trigger name validation (opensearch-…
Browse files Browse the repository at this point in the history
…project#794) (opensearch-project#1071)

(cherry picked from commit 83a2b3e)

Signed-off-by: Chenxi Wang <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent ca02f42 commit 83ad06c
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { hasError, isInvalid } from '../../../../utils/validate';
import { DEFAULT_TRIGGER_NAME } 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';
import { SEVERITY_OPTIONS } from '../../../../utils/constants';
Expand Down Expand Up @@ -101,7 +102,7 @@ class DefineCompositeLevelTrigger extends Component {
<FormikFieldText
name={`${formikFieldPath}name`}
fieldProps={{
validate: required,
validate: (val) => validateTriggerName(values?.triggerDefinitions, triggerIndex)(val),
}}
formRow
rowProps={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,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: {} } : {}) }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
});
});

0 comments on commit 83ad06c

Please sign in to comment.