Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.x] Issue #671 fix trigger name validation #841

Merged
merged 1 commit into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, 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';

Expand Down Expand Up @@ -100,7 +101,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 @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} } : {}) }}
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.');
});
});
Loading