fix: Disable saving threshold check if no threshold selected (#15348)

* Prevent check saving if no thresholds * Add tests * Add changes to changelog * make optional props optional * use false instead of null for boolean
influxdata · Oct 8, 2019 · 95ee116 · 95ee116
1 parent 4911b9d
commit 95ee116
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,11 +1,13 @@
+
 ## v2.0.0-alpha.19 [unreleased]
 
 ### Features
 1. [15313](https://github.com/influxdata/influxdb/pull/15313): Add shortcut for toggling comments in script editor
 
 ### Bug Fixes
 1. [15295](https://github.com/influxdata/influxdb/pull/15295): Ensures users are created with an active status
-2. [15306](https://github.com/influxdata/influxdb/pull/15306): Added missing string values for CacheStatus type
+1. [15306](https://github.com/influxdata/influxdb/pull/15306): Added missing string values for CacheStatus type
+1. [15348](https://github.com/influxdata/influxdb/pull/15348): Disable saving for threshold check if no threshold selected
 
 ## v2.0.0-alpha.18 [2019-09-26]
 

diff --git a/ui/cypress/e2e/checks.test.ts b/ui/cypress/e2e/checks.test.ts
@@ -29,6 +29,12 @@ describe('Checks', () => {
 
     cy.getByTestID(`selector-list ${field}`).click()
 
+    cy.getByTestID('save-cell--button').should('be.disabled')
+
+    cy.getByTestID('checkeo--header alerting-tab').click()
+
+    cy.getByTestID('add-threshold-condition-WARN').click()
+
     cy.getByTestID('save-cell--button').should('be.enabled')
   })
 })
diff --git a/ui/src/alerting/components/CheckEOHeader.tsx b/ui/src/alerting/components/CheckEOHeader.tsx
@@ -86,7 +86,7 @@ const CheckEOHeader: FC<Props> = ({
   }
 
   const saveButtonStatus = () => {
-    if (!isCheckSaveable(draftQueries, check.type)) {
+    if (!isCheckSaveable(draftQueries, check)) {
       return ComponentStatus.Disabled
     }
 
@@ -98,6 +98,10 @@ const CheckEOHeader: FC<Props> = ({
   }
 
   const {singleField, singleAggregateFunc} = isDraftQueryAlertable(draftQueries)
+  const oneOrMoreThresholds =
+    check.type === 'threshold'
+      ? check.thresholds && !!check.thresholds.length
+      : false
 
   return (
     <Page.Header fullWidth={true}>
@@ -130,6 +134,7 @@ const CheckEOHeader: FC<Props> = ({
           checkType={check.type}
           singleField={singleField}
           singleAggregateFunc={singleAggregateFunc}
+          oneOrMoreThresholds={oneOrMoreThresholds}
         />
       </Page.Header.Right>
     </Page.Header>

diff --git a/ui/src/alerting/components/CheckEOSaveButton.tsx b/ui/src/alerting/components/CheckEOSaveButton.tsx
@@ -22,6 +22,7 @@ interface Props {
   checkType: string
   singleField: boolean
   singleAggregateFunc: boolean
+  oneOrMoreThresholds: boolean
 }
 
 const CheckEOSaveButton: FunctionComponent<Props> = ({
@@ -31,6 +32,7 @@ const CheckEOSaveButton: FunctionComponent<Props> = ({
   checkType,
   singleField,
   singleAggregateFunc,
+  oneOrMoreThresholds,
 }) => {
   return (
     <Popover
@@ -42,14 +44,20 @@ const CheckEOSaveButton: FunctionComponent<Props> = ({
       type={PopoverType.Outline}
       contents={() => (
         <div className="query-checklist--popover">
-          <p>{`To create a ${checkType} check, your query must include:`}</p>
+          <p>{`To create a ${checkType} check, you must select:`}</p>
           <ul className="query-checklist--list">
             <QueryChecklistItem text="One field" selected={singleField} />
             {checkType === 'threshold' && (
-              <QueryChecklistItem
-                text="One aggregate function"
-                selected={singleAggregateFunc}
-              />
+              <>
+                <QueryChecklistItem
+                  text="One aggregate function"
+                  selected={singleAggregateFunc}
+                />
+                <QueryChecklistItem
+                  text="One or more thresholds"
+                  selected={oneOrMoreThresholds}
+                />
+              </>
             )}
           </ul>
         </div>

diff --git a/ui/src/alerting/components/builder/ThresholdCondition.tsx b/ui/src/alerting/components/builder/ThresholdCondition.tsx
@@ -113,6 +113,7 @@ const ThresholdCondition: FC<Props> = ({
         color={LEVEL_COMPONENT_COLORS[level]}
         size={ComponentSize.Large}
         onClick={addLevel}
+        testID={`add-threshold-condition-${level}`}
       />
     )
   }

diff --git a/ui/src/shared/components/dashed_button/DashedButton.tsx b/ui/src/shared/components/dashed_button/DashedButton.tsx
@@ -8,23 +8,30 @@ import {ComponentColor, ComponentSize} from '@influxdata/clockface'
 interface Props {
   text: string
   onClick: (e: MouseEvent) => void
-  color: ComponentColor
-  size: ComponentSize
+  color?: ComponentColor
+  size?: ComponentSize
+  testID?: string
 }
 
 const DashedButton: FC<Props> = ({
   text,
   onClick,
   color = ComponentColor.Primary,
   size = ComponentSize.Medium,
+  testID = 'dashed-button',
 }) => {
   const classname = classnames('dashed-button', {
     [`dashed-button__${color}`]: color,
     [`dashed-button__${size}`]: size,
   })
 
   return (
-    <button className={classname} onClick={onClick} type="button">
+    <button
+      className={classname}
+      onClick={onClick}
+      type="button"
+      data-testid={testID}
+    >
       {text}
     </button>
   )

diff --git a/ui/src/timeMachine/utils/queryBuilder.ts b/ui/src/timeMachine/utils/queryBuilder.ts
@@ -1,5 +1,5 @@
 import {get, isEmpty} from 'lodash'
-import {BuilderConfig, DashboardDraftQuery} from 'src/types'
+import {BuilderConfig, DashboardDraftQuery, Check} from 'src/types'
 import {FUNCTIONS} from 'src/timeMachine/constants/queryBuilder'
 import {
   TIME_RANGE_START,
@@ -48,7 +48,7 @@ export const isDraftQueryAlertable = (
 
 export const isCheckSaveable = (
   draftQueries: DashboardDraftQuery[],
-  checkType: string
+  check: Partial<Check>
 ): boolean => {
   const {
     oneQuery,
@@ -57,11 +57,18 @@ export const isCheckSaveable = (
     singleField,
   } = isDraftQueryAlertable(draftQueries)
 
-  if (checkType === 'deadman') {
+  if (check.type === 'deadman') {
     return oneQuery && builderMode && singleField
   }
 
-  return oneQuery && builderMode && singleAggregateFunc && singleField
+  return (
+    oneQuery &&
+    builderMode &&
+    singleAggregateFunc &&
+    singleField &&
+    check.thresholds &&
+    !!check.thresholds.length
+  )
 }
 
 export function buildQuery(builderConfig: BuilderConfig): string {