diff --git a/ui/src/alerting/actions/checks.ts b/ui/src/alerting/actions/checks.ts index 6d7d0c1ee53..5c142f335ed 100644 --- a/ui/src/alerting/actions/checks.ts +++ b/ui/src/alerting/actions/checks.ts @@ -13,6 +13,10 @@ import * as api from 'src/client' import {getActiveTimeMachine} from 'src/timeMachine/selectors' import {incrementCloneName} from 'src/utils/naming' import {reportError} from 'src/shared/utils/errors' +import {isDurationParseable} from 'src/shared/utils/duration' +import {checkThresholdsValid} from '../utils/checkValidate' +import {createView} from 'src/shared/utils/view' +import {getOrg} from 'src/organizations/selectors' // Actions import { @@ -44,10 +48,6 @@ import { DeadmanCheck, } from 'src/types' -// Utils -import {createView} from 'src/shared/utils/view' -import {getOrg} from 'src/organizations/selectors' - export type Action = | ReturnType | ReturnType @@ -187,6 +187,7 @@ export const saveCheckFromTimeMachine = () => async ( tags, thresholds, } as ThresholdCheck + checkThresholdsValid(thresholds) } else if (check.type === 'deadman') { check = { ...check, @@ -199,6 +200,16 @@ export const saveCheckFromTimeMachine = () => async ( tags, timeSince, } as DeadmanCheck + if (!isDurationParseable(timeSince) || !isDurationParseable(staleTime)) { + throw new Error('Duration fields must contain valid duration') + } + } + + if (!isDurationParseable(offset)) { + throw new Error('Check offset must be a valid duration') + } + if (!isDurationParseable(every)) { + throw new Error('Check every must be a valid duration') } const resp = check.id diff --git a/ui/src/alerting/actions/notifications/rules.ts b/ui/src/alerting/actions/notifications/rules.ts index c227f748f2b..1ce7087d605 100644 --- a/ui/src/alerting/actions/notifications/rules.ts +++ b/ui/src/alerting/actions/notifications/rules.ts @@ -139,6 +139,12 @@ export const createRule = (rule: NotificationRuleDraft) => async ( export const updateRule = (rule: NotificationRuleDraft) => async ( dispatch: Dispatch ) => { + if (rule.offset == '') { + throw new Error('Notification Rule offset field can not be empty') + } + if (rule.every == '') { + throw new Error('Notification Rule every field can not be empty') + } const resp = await api.putNotificationRule({ ruleID: rule.id, data: draftRuleToPostRule(rule), diff --git a/ui/src/alerting/components/builder/CheckMetaCard.tsx b/ui/src/alerting/components/builder/CheckMetaCard.tsx index ccb26feb080..af6e54a7f55 100644 --- a/ui/src/alerting/components/builder/CheckMetaCard.tsx +++ b/ui/src/alerting/components/builder/CheckMetaCard.tsx @@ -6,8 +6,8 @@ import {connect} from 'react-redux' import {Form, ComponentSize, ComponentColor, Grid} from '@influxdata/clockface' import DashedButton from 'src/shared/components/dashed_button/DashedButton' import CheckTagRow from 'src/alerting/components/builder/CheckTagRow' -import DurationSelector from 'src/shared/components/DurationSelector' import BuilderCard from 'src/timeMachine/components/builderCard/BuilderCard' +import DurationInput from 'src/shared/components/DurationInput' // Actions import { @@ -18,7 +18,8 @@ import { } from 'src/alerting/actions/alertBuilder' // Constants -import {CHECK_EVERY_OPTIONS, CHECK_OFFSET_OPTIONS} from 'src/alerting/constants' +import {CHECK_OFFSET_OPTIONS} from 'src/alerting/constants' +import {DURATION_STRINGS} from 'src/timeMachine/constants/queryBuilder' // Types import {AppState, CheckTagSet} from 'src/types' @@ -63,19 +64,19 @@ const CheckMetaCard: FC = ({ - - diff --git a/ui/src/alerting/components/builder/DeadmanConditions.tsx b/ui/src/alerting/components/builder/DeadmanConditions.tsx index be896e4fc3c..2205fbcf33d 100644 --- a/ui/src/alerting/components/builder/DeadmanConditions.tsx +++ b/ui/src/alerting/components/builder/DeadmanConditions.tsx @@ -9,8 +9,6 @@ import { ComponentSize, PanelBody, TextBlock, - InputType, - Input, FlexDirection, InfluxColors, } from '@influxdata/clockface' @@ -25,6 +23,8 @@ import { // Types import {CheckStatusLevel, AppState} from 'src/types' +import DurationInput from 'src/shared/components/DurationInput' +import {CHECK_OFFSET_OPTIONS} from 'src/alerting/constants' interface DispatchProps { onSetStaleTime: typeof setStaleTime @@ -77,14 +77,11 @@ const DeadmanConditions: FC = ({ > - ) => - onSetTimeSince(e.target.value) - } - name="timeSince" - testID="input-field" - type={InputType.Text} + @@ -106,14 +103,11 @@ const DeadmanConditions: FC = ({ text="And stop checking after" /> - ) => - onSetStaleTime(e.target.value) - } - name="staleTime" - testID="input-field" - type={InputType.Text} + diff --git a/ui/src/alerting/components/notifications/EditRuleOverlay.tsx b/ui/src/alerting/components/notifications/EditRuleOverlay.tsx index bfd2f2c0824..bb4dbcafc35 100644 --- a/ui/src/alerting/components/notifications/EditRuleOverlay.tsx +++ b/ui/src/alerting/components/notifications/EditRuleOverlay.tsx @@ -26,7 +26,7 @@ interface StateProps { interface DispatchProps { onNotify: typeof notify - onUpdateRule: (rule: NotificationRuleDraft) => Promise + onUpdateRule: typeof updateRule } type Props = WithRouterProps & StateProps & DispatchProps @@ -84,7 +84,7 @@ const mstp = ({rules}: AppState, {params}: Props): StateProps => { const mdtp = { onNotify: notify, - onUpdateRule: updateRule as any, + onUpdateRule: updateRule, } export default connect( diff --git a/ui/src/alerting/components/notifications/RuleOverlayContents.tsx b/ui/src/alerting/components/notifications/RuleOverlayContents.tsx index 09a60179dc7..a0fd008466d 100644 --- a/ui/src/alerting/components/notifications/RuleOverlayContents.tsx +++ b/ui/src/alerting/components/notifications/RuleOverlayContents.tsx @@ -40,6 +40,15 @@ const RuleOverlayContents: FC = ({saveButtonText, onSave}) => { }) } + const handleChangeParameter = (key: keyof NotificationRuleDraft) => ( + value: string + ) => { + dispatch({ + type: 'UPDATE_RULE', + rule: {...rule, [key]: value} as NotificationRuleDraft, + }) + } + return (
@@ -57,7 +66,7 @@ const RuleOverlayContents: FC = ({saveButtonText, onSave}) => { onChange={handleChange} /> - + diff --git a/ui/src/alerting/components/notifications/RuleSchedule.tsx b/ui/src/alerting/components/notifications/RuleSchedule.tsx index 351db13b972..c84c298d9ff 100644 --- a/ui/src/alerting/components/notifications/RuleSchedule.tsx +++ b/ui/src/alerting/components/notifications/RuleSchedule.tsx @@ -1,15 +1,19 @@ // Libraries -import React, {FC, ChangeEvent} from 'react' +import React, {FC} from 'react' // Components -import {Form, Input, InputType, Grid, Columns} from '@influxdata/clockface' +import {Form, Grid, Columns} from '@influxdata/clockface' +import DurationInput from 'src/shared/components/DurationInput' // Types import {RuleState} from './RuleOverlay.reducer' +import {DURATION_STRINGS} from 'src/timeMachine/constants/queryBuilder' +import {NotificationRuleDraft} from 'src/types' +import {CHECK_OFFSET_OPTIONS} from 'src/alerting/constants' interface Props { rule: RuleState - onChange: (e: ChangeEvent) => void + onChange: (key: keyof NotificationRuleDraft) => (value: string) => void } const RuleSchedule: FC = ({rule, onChange}) => { @@ -19,12 +23,11 @@ const RuleSchedule: FC = ({rule, onChange}) => { - @@ -32,12 +35,11 @@ const RuleSchedule: FC = ({rule, onChange}) => { - diff --git a/ui/src/alerting/components/notifications/utils/index.ts b/ui/src/alerting/components/notifications/utils/index.ts index ffa60ed2027..f43f9083bf4 100644 --- a/ui/src/alerting/components/notifications/utils/index.ts +++ b/ui/src/alerting/components/notifications/utils/index.ts @@ -81,6 +81,7 @@ export const changeStatusRule = ( export const initRuleDraft = (orgID: string): NotificationRuleDraft => ({ type: 'http', every: '10m', + offset: '0s', url: '', orgID, name: '', @@ -130,6 +131,7 @@ export const ruleToDraftRule = ( const tagRules = rule.tagRules || [] return { ...rule, + offset: rule.offset || '', statusRules: statusRules.map(value => ({cid: uuid.v4(), value})), tagRules: tagRules.map(value => ({cid: uuid.v4(), value})), } diff --git a/ui/src/alerting/constants/index.ts b/ui/src/alerting/constants/index.ts index e23e793b746..f0c4b875202 100644 --- a/ui/src/alerting/constants/index.ts +++ b/ui/src/alerting/constants/index.ts @@ -1,9 +1,6 @@ -import {DURATIONS} from 'src/timeMachine/constants/queryBuilder' - import {ThresholdCheck, TagRuleDraft} from 'src/types' import {NotificationEndpoint, CheckStatusLevel} from 'src/client' import {ComponentColor, InfluxColors} from '@influxdata/clockface' -import {DurationOption} from 'src/shared/components/DurationSelector' export const DEFAULT_CHECK_NAME = 'Name this Check' export const DEFAULT_NOTIFICATION_RULE_NAME = 'Name this Notification Rule' @@ -17,22 +14,20 @@ export const DEFAULT_DEADMAN_LEVEL: CheckStatusLevel = 'CRIT' export const DEFAULT_STATUS_MESSAGE = 'Check: ${ r._check_name } is: ${ r._level }' -export const CHECK_EVERY_OPTIONS: DurationOption[] = DURATIONS - -export const CHECK_OFFSET_OPTIONS: DurationOption[] = [ - {duration: '0s', displayText: 'None'}, - {duration: '5s', displayText: '5 seconds'}, - {duration: '15s', displayText: '15 seconds'}, - {duration: '1m', displayText: '1 minute'}, - {duration: '5m', displayText: '5 minutes'}, - {duration: '15m', displayText: '15 minutes'}, - {duration: '1h', displayText: '1 hour'}, - {duration: '6h', displayText: '6 hours'}, - {duration: '12h', displayText: '12 hours'}, - {duration: '24h', displayText: '24 hours'}, - {duration: '2d', displayText: '2 days'}, - {duration: '7d', displayText: '7 days'}, - {duration: '30d', displayText: '30 days'}, +export const CHECK_OFFSET_OPTIONS = [ + '0s', + '5s', + '15s', + '1m', + '5m', + '15m', + '1h', + '6h', + '12h', + '24h', + '2d', + '7d', + '30d', ] export const MONITORING_BUCKET = '_monitoring' diff --git a/ui/src/alerting/utils/checkValidate.ts b/ui/src/alerting/utils/checkValidate.ts new file mode 100644 index 00000000000..485aae034f1 --- /dev/null +++ b/ui/src/alerting/utils/checkValidate.ts @@ -0,0 +1,15 @@ +import {Threshold} from 'src/types' + +export const checkThresholdsValid = (thresholds: Threshold[]) => { + thresholds.forEach(t => { + if (t.type === 'greater' && isNaN(t.value)) { + throw new Error('Threshold must have defined value') + } + if (t.type === 'lesser' && isNaN(t.value)) { + throw new Error('Threshold must have defined value') + } + if (t.type === 'range' && (isNaN(t.min) || isNaN(t.min))) { + throw new Error('Threshold must have defined min and max values') + } + }) +} diff --git a/ui/src/dashboards/utils/time.ts b/ui/src/dashboards/utils/time.ts index aff028a5acc..9bc83c0fcd0 100644 --- a/ui/src/dashboards/utils/time.ts +++ b/ui/src/dashboards/utils/time.ts @@ -2,7 +2,7 @@ import {CustomTimeRange, TimeRange, DurationTimeRange} from 'src/types/queries' import {SELECTABLE_TIME_RANGES} from 'src/shared/constants/timeRanges' import {isDateParseable} from 'src/variables/utils/getTimeRangeVars' -import {isDurationParseable} from 'src/shared/utils/duration' +import {isDurationWithNowParseable} from 'src/shared/utils/duration' interface InputTimeRange { seconds?: number @@ -47,7 +47,7 @@ export const validateAndTypeRange = (timeRange: { } as CustomTimeRange } - if (isDurationParseable(lower)) { + if (isDurationWithNowParseable(lower)) { const selectableTimeRange = SELECTABLE_TIME_RANGES.find( r => r.lower === lower ) diff --git a/ui/src/shared/components/DurationInput.tsx b/ui/src/shared/components/DurationInput.tsx new file mode 100644 index 00000000000..46e1db4a4da --- /dev/null +++ b/ui/src/shared/components/DurationInput.tsx @@ -0,0 +1,100 @@ +// Libraries +import React, {useState, FC} from 'react' +import { + Input, + ComponentStatus, + DropdownMenu, + DropdownDivider, + DropdownItem, + ClickOutside, +} from '@influxdata/clockface' +import {isDurationParseable} from 'src/shared/utils/duration' + +const SUGGESTION_CLASS = 'duration-input--suggestion' + +type Props = { + suggestions: string[] + onSubmit: (input: string) => void + value: string + placeholder?: string + submitInvalid?: boolean + showDivider?: boolean + testID?: string +} + +const DurationInput: FC = ({ + suggestions, + onSubmit, + value, + placeholder, + submitInvalid = true, + showDivider = true, + testID = 'duration-input', +}) => { + const [isFocused, setIsFocused] = useState(false) + + const [inputValue, setInputValue] = useState(value) + + const inputStatus = isDurationParseable(inputValue) + ? ComponentStatus.Default + : ComponentStatus.Error + + const handleClickSuggestion = (suggestion: string) => { + setInputValue(suggestion) + + onSubmit(suggestion) + setIsFocused(false) + } + + const handleClickOutside = e => { + const didClickSuggestion = + e.target.classList.contains(SUGGESTION_CLASS) || + e.target.parentNode.classList.contains(SUGGESTION_CLASS) + + if (!didClickSuggestion) { + setIsFocused(false) + } + } + + const onChange = (i: string) => { + setInputValue(i) + if (submitInvalid || (!submitInvalid && isDurationParseable(i))) { + onSubmit(i) + } + } + + return ( +
+ + onChange(e.target.value)} + onFocus={() => setIsFocused(true)} + testID={testID} + /> + + {isFocused && ( + + {showDivider && } + {suggestions.map(s => ( + + {s} + + ))} + + )} +
+ ) +} + +export default DurationInput diff --git a/ui/src/shared/utils/duration.test.tsx b/ui/src/shared/utils/duration.test.tsx index b8470f7d551..e617207dd69 100644 --- a/ui/src/shared/utils/duration.test.tsx +++ b/ui/src/shared/utils/duration.test.tsx @@ -3,6 +3,7 @@ import { durationToMilliseconds, areDurationsEqual, millisecondsToDuration, + isDurationWithNowParseable, isDurationParseable, } from 'src/shared/utils/duration' import {SELECTABLE_TIME_RANGES} from 'src/shared/constants/timeRanges' @@ -69,18 +70,33 @@ describe('millisecondsToDuration', () => { }) }) +describe('isDurationWithNowParseable', () => { + test('returns false when passed invalid durations', () => { + expect(isDurationWithNowParseable('1h')).toBe(false) + expect(isDurationWithNowParseable('moo')).toBe(false) + expect(isDurationWithNowParseable('123')).toBe(false) + expect(isDurationWithNowParseable('now()')).toBe(false) + }) + + test.each(SELECTABLE_TIME_RANGES)( + 'returns true when passed valid duration', + ({lower}) => { + expect(isDurationWithNowParseable(lower)).toEqual(true) + } + ) +}) + describe('isDurationParseable', () => { test('returns false when passed invalid durations', () => { - expect(isDurationParseable('1h')).toBe(false) expect(isDurationParseable('moo')).toBe(false) expect(isDurationParseable('123')).toBe(false) - expect(isDurationParseable('now()')).toBe(false) + expect(isDurationParseable('now()-1h')).toBe(false) }) - test.each(SELECTABLE_TIME_RANGES)( + test.each(TEST_CASES)( 'returns true when passed valid duration', - ({lower}) => { - expect(isDurationParseable(lower)).toEqual(true) + (input, _) => { + expect(isDurationParseable(input)).toEqual(true) } ) }) diff --git a/ui/src/shared/utils/duration.ts b/ui/src/shared/utils/duration.ts index 78d8e04681b..7d2b4baf5d6 100644 --- a/ui/src/shared/utils/duration.ts +++ b/ui/src/shared/utils/duration.ts @@ -7,7 +7,7 @@ import {TIME_RANGE_FORMAT} from 'src/shared/constants/timeRanges' export const removeSpacesAndNow = (input: string): string => input.replace(/\s/g, '').replace(/now\(\)-/, '') -export const isDurationParseable = (lower: string): boolean => { +export const isDurationWithNowParseable = (lower: string): boolean => { const durationRegExp = /([0-9]+)(y|mo|w|d|h|ms|s|m|us|µs|ns)/g if (!lower || !lower.includes('now()')) { return false @@ -18,6 +18,18 @@ export const isDurationParseable = (lower: string): boolean => { return !!removedLower.match(durationRegExp) } +export const isDurationParseable = (duration: string): boolean => { + if (typeof duration !== 'string') { + return false + } + + const durationRegExp = /^(([0-9]+)(y|mo|w|d|h|ms|s|m|us|µs|ns))+$/g + + // warning! Using string.match(regex) here instead of regex.test(string) because regex.test() modifies the regex object, and can lead to unexpected behavior + + return !!duration.match(durationRegExp) +} + export const parseDuration = (input: string): Duration[] => { const result = [] const durationRegExp = /([0-9]+)(y|mo|w|d|h|ms|s|m|us|µs|ns)/g diff --git a/ui/src/timeMachine/constants/queryBuilder.ts b/ui/src/timeMachine/constants/queryBuilder.ts index 4127ecf528a..4ef5028ebcf 100644 --- a/ui/src/timeMachine/constants/queryBuilder.ts +++ b/ui/src/timeMachine/constants/queryBuilder.ts @@ -16,6 +16,21 @@ export const DURATIONS = [ {duration: '30d', displayText: 'Every 30 days'}, ] +export const DURATION_STRINGS = [ + '5s', + '15s', + '1m', + '5m', + '15m', + '1h', + '6h', + '12h', + '24h', + '2d', + '7d', + '30d', +] + export interface QueryFn { name: string flux: (period?: string) => string