feat(ui) More reasonable aggregate window intervals for selected quer…

…y durations (#16536) * feat(agg-window): Add duration input to aggreagate functionn selector * feat(agg-window): rename duration strings to durations * feat(agg-window): Add window periods for common time ranges * feat(agg-window): Handle set auto select durations correctly * Update changelog * feat(agg-window): Use triple equality:
influxdata · Jan 15, 2020 · d753aa4 · d753aa4
1 parent 93c8a2a
commit d753aa4
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 62 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@
 ### UI Improvements
 
 1. [16203](https://github.com/influxdata/influxdb/pull/16203): Move cloud navigation to top of page instead of within left side navigation
+1. [16536](https://github.com/influxdata/influxdb/pull/16536): Adjust aggregate window periods to be more "reasonable". Use duration input with validation.
+
 
 ## v2.0.0-beta.1 [2020-01-08]
 

diff --git a/ui/src/alerting/components/builder/CheckMetaCard.tsx b/ui/src/alerting/components/builder/CheckMetaCard.tsx
@@ -19,7 +19,7 @@ import {
 
 // Constants
 import {CHECK_OFFSET_OPTIONS} from 'src/alerting/constants'
-import {DURATION_STRINGS} from 'src/timeMachine/constants/queryBuilder'
+import {DURATIONS} from 'src/timeMachine/constants/queryBuilder'
 
 // Types
 import {AppState, CheckTagSet} from 'src/types'
@@ -66,7 +66,7 @@ const CheckMetaCard: FC<Props> = ({
               <Form.Element label="Schedule Every">
                 <DurationInput
                   value={every}
-                  suggestions={DURATION_STRINGS}
+                  suggestions={DURATIONS}
                   onSubmit={onSelectCheckEvery}
                 />
               </Form.Element>

diff --git a/ui/src/alerting/components/notifications/RuleSchedule.tsx b/ui/src/alerting/components/notifications/RuleSchedule.tsx
@@ -7,7 +7,7 @@ import DurationInput from 'src/shared/components/DurationInput'
 
 // Types
 import {RuleState} from './RuleOverlay.reducer'
-import {DURATION_STRINGS} from 'src/timeMachine/constants/queryBuilder'
+import {DURATIONS} from 'src/timeMachine/constants/queryBuilder'
 import {NotificationRuleDraft} from 'src/types'
 import {CHECK_OFFSET_OPTIONS} from 'src/alerting/constants'
 
@@ -26,7 +26,7 @@ const RuleSchedule: FC<Props> = ({rule, onChange}) => {
           <DurationInput
             value={every || ''}
             onSubmit={onChange('every')}
-            suggestions={DURATION_STRINGS}
+            suggestions={DURATIONS}
             placeholder="1d3h30s"
             testID="rule-schedule-every--input"
           />

diff --git a/ui/src/shared/components/DurationInput.tsx b/ui/src/shared/components/DurationInput.tsx
@@ -1,5 +1,5 @@
 // Libraries
-import React, {useState, FC} from 'react'
+import React, {useState, useEffect, FC} from 'react'
 import {
   Input,
   ComponentStatus,
@@ -20,6 +20,7 @@ type Props = {
   submitInvalid?: boolean
   showDivider?: boolean
   testID?: string
+  validFunction?: (input: string) => boolean
 }
 
 const DurationInput: FC<Props> = ({
@@ -30,14 +31,17 @@ const DurationInput: FC<Props> = ({
   submitInvalid = true,
   showDivider = true,
   testID = 'duration-input',
+  validFunction = _ => false,
 }) => {
   const [isFocused, setIsFocused] = useState(false)
 
   const [inputValue, setInputValue] = useState(value)
 
-  const inputStatus = isDurationParseable(inputValue)
-    ? ComponentStatus.Default
-    : ComponentStatus.Error
+  useEffect(() => {
+    if (value != inputValue) {
+      setInputValue(value)
+    }
+  }, [value])
 
   const handleClickSuggestion = (suggestion: string) => {
     setInputValue(suggestion)
@@ -56,9 +60,16 @@ const DurationInput: FC<Props> = ({
     }
   }
 
+  const isValid = (i: string): boolean =>
+    isDurationParseable(i) || validFunction(i)
+
+  const inputStatus = isValid(inputValue)
+    ? ComponentStatus.Default
+    : ComponentStatus.Error
+
   const onChange = (i: string) => {
     setInputValue(i)
-    if (submitInvalid || (!submitInvalid && isDurationParseable(i))) {
+    if (submitInvalid || (!submitInvalid && isValid(i))) {
       onSubmit(i)
     }
   }

diff --git a/ui/src/shared/constants/timeRanges.ts b/ui/src/shared/constants/timeRanges.ts
@@ -11,6 +11,7 @@ export const pastHourTimeRange: SelectableDurationTimeRange = {
   label: 'Past 1h',
   duration: '1h',
   type: 'selectable-duration',
+  windowPeriod: 10000, // 10s
 }
 
 export const pastThirtyDaysTimeRange: SelectableDurationTimeRange = {
@@ -20,6 +21,7 @@ export const pastThirtyDaysTimeRange: SelectableDurationTimeRange = {
   label: 'Past 30d',
   duration: '30d',
   type: 'selectable-duration',
+  windowPeriod: 3600000, // 1h
 }
 
 export const pastFifteenMinTimeRange: SelectableDurationTimeRange = {
@@ -29,6 +31,7 @@ export const pastFifteenMinTimeRange: SelectableDurationTimeRange = {
   label: 'Past 15m',
   duration: '15m',
   type: 'selectable-duration',
+  windowPeriod: 10000, // 10s
 }
 
 export const CUSTOM_TIME_RANGE: {label: string; type: 'custom'} = {
@@ -44,6 +47,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 5m',
     duration: '5m',
     type: 'selectable-duration',
+    windowPeriod: 10000, // 10s
   },
   pastFifteenMinTimeRange,
   pastHourTimeRange,
@@ -54,6 +58,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 6h',
     duration: '6h',
     type: 'selectable-duration',
+    windowPeriod: 60000, // 1m
   },
   {
     seconds: 43200,
@@ -62,6 +67,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 12h',
     duration: '12h',
     type: 'selectable-duration',
+    windowPeriod: 120000, // 2m
   },
   {
     seconds: 86400,
@@ -70,6 +76,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 24h',
     duration: '24h',
     type: 'selectable-duration',
+    windowPeriod: 300000, // 5m
   },
   {
     seconds: 172800,
@@ -78,6 +85,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 2d',
     duration: '2d',
     type: 'selectable-duration',
+    windowPeriod: 600000, // 10m
   },
   {
     seconds: 604800,
@@ -86,6 +94,7 @@ export const SELECTABLE_TIME_RANGES: SelectableDurationTimeRange[] = [
     label: 'Past 7d',
     duration: '7d',
     type: 'selectable-duration',
+    windowPeriod: 1800000, // 30 min
   },
   pastThirtyDaysTimeRange,
 ]

diff --git a/ui/src/timeMachine/components/FunctionSelector.tsx b/ui/src/timeMachine/components/FunctionSelector.tsx
@@ -3,12 +3,17 @@ import React, {PureComponent, ChangeEvent} from 'react'
 import {connect} from 'react-redux'
 
 // Components
-import {Input} from '@influxdata/clockface'
+import {
+  Input,
+  FlexBox,
+  FlexDirection,
+  ComponentSize,
+  TextBlock,
+  InfluxColors,
+} from '@influxdata/clockface'
 import SelectorList from 'src/timeMachine/components/SelectorList'
 import BuilderCard from 'src/timeMachine/components/builderCard/BuilderCard'
-import DurationSelector, {
-  DurationOption,
-} from 'src/shared/components/DurationSelector'
+import DurationInput from 'src/shared/components/DurationInput'
 
 // Actions
 import {
@@ -59,30 +64,52 @@ class FunctionSelector extends PureComponent<Props, State> {
   public state: State = {searchTerm: ''}
 
   public render() {
-    const {
-      selectedFunctions,
-      onSelectAggregateWindow,
-      isInCheckOverlay,
-    } = this.props
+    const {isInCheckOverlay} = this.props
 
     const {searchTerm} = this.state
-
     return (
       <BuilderCard className="function-selector" testID="function-selector">
         <BuilderCard.Header title="Aggregate Functions" />
         <BuilderCard.Menu>
-          <DurationSelector
-            onSelectDuration={onSelectAggregateWindow}
-            selectedDuration={this.duration}
-            durations={this.durations}
-            disabled={!selectedFunctions.length}
-          />
-          <Input
-            className="tag-selector--search"
-            value={searchTerm}
-            onChange={this.handleSetSearchTerm}
-            placeholder="Search functions..."
-          />
+          <FlexBox
+            direction={FlexDirection.Column}
+            margin={ComponentSize.Small}
+          >
+            <FlexBox
+              direction={FlexDirection.Row}
+              margin={ComponentSize.Small}
+              stretchToFitWidth
+            >
+              <FlexBox.Child grow={2} testID="component-spacer--flex-child">
+                <Input
+                  className="tag-selector--search"
+                  value={searchTerm}
+                  onChange={this.handleSetSearchTerm}
+                  placeholder="Search functions..."
+                />
+              </FlexBox.Child>
+            </FlexBox>
+            <FlexBox
+              direction={FlexDirection.Row}
+              margin={ComponentSize.Small}
+              stretchToFitWidth
+              testID="component-spacer"
+            >
+              <TextBlock
+                textColor={InfluxColors.Sidewalk}
+                text="Window period:"
+              />
+              <FlexBox.Child grow={2} testID="component-spacer--flex-child">
+                <DurationInput
+                  onSubmit={this.handleSelectAggregateWindow}
+                  value={this.duration}
+                  suggestions={this.durations}
+                  submitInvalid={false}
+                  validFunction={this.windowInputValid}
+                />
+              </FlexBox.Child>
+            </FlexBox>
+          </FlexBox>
         </BuilderCard.Menu>
         <SelectorList
           items={this.functions}
@@ -94,28 +121,27 @@ class FunctionSelector extends PureComponent<Props, State> {
     )
   }
 
+  private get autoLabel(): string {
+    const {autoWindowPeriod} = this.props
+    return autoWindowPeriod
+      ? `${AGG_WINDOW_AUTO} (${millisecondsToDuration(autoWindowPeriod)})`
+      : AGG_WINDOW_AUTO
+  }
+
   private get duration(): string {
     const {aggregateWindow} = this.props
 
-    return aggregateWindow.period || AGG_WINDOW_AUTO
+    if (!aggregateWindow.period || aggregateWindow.period === AGG_WINDOW_AUTO) {
+      return this.autoLabel
+    }
+
+    return aggregateWindow.period
   }
 
-  private get durations(): DurationOption[] {
+  private get durations(): string[] {
     return this.props.isInCheckOverlay
       ? DURATIONS
-      : [...this.autoNoneDurations, ...DURATIONS]
-  }
-
-  private get autoNoneDurations(): DurationOption[] {
-    const {autoWindowPeriod} = this.props
-    const autoLabel = autoWindowPeriod
-      ? `Auto (${millisecondsToDuration(autoWindowPeriod)})`
-      : 'Auto'
-
-    return [
-      {duration: AGG_WINDOW_AUTO, displayText: autoLabel},
-      {duration: AGG_WINDOW_NONE, displayText: 'None'},
-    ]
+      : [this.autoLabel, AGG_WINDOW_NONE, ...DURATIONS]
   }
 
   private get functions(): string[] {
@@ -140,6 +166,17 @@ class FunctionSelector extends PureComponent<Props, State> {
 
     onSelectFunction(functionName)
   }
+
+  private handleSelectAggregateWindow = (input: string) => {
+    if (input.startsWith(AGG_WINDOW_AUTO)) {
+      this.props.onSelectAggregateWindow(AGG_WINDOW_AUTO)
+      return
+    }
+    this.props.onSelectAggregateWindow(input)
+  }
+
+  private windowInputValid = (input: string): boolean =>
+    input == 'none' || input == this.autoLabel
 }
 
 const mstp = (state: AppState): StateProps => {

diff --git a/ui/src/timeMachine/constants/queryBuilder.ts b/ui/src/timeMachine/constants/queryBuilder.ts
@@ -2,21 +2,6 @@ export const AGG_WINDOW_AUTO = 'auto'
 export const AGG_WINDOW_NONE = 'none'
 
 export const DURATIONS = [
-  {duration: '5s', displayText: 'Every 5 seconds'},
-  {duration: '15s', displayText: 'Every 15 seconds'},
-  {duration: '1m', displayText: 'Every minute'},
-  {duration: '5m', displayText: 'Every 5 minutes'},
-  {duration: '15m', displayText: 'Every 15 minutes'},
-  {duration: '1h', displayText: 'Every hour'},
-  {duration: '6h', displayText: 'Every 6 hours'},
-  {duration: '12h', displayText: 'Every 12 hours'},
-  {duration: '24h', displayText: 'Every 24 hours'},
-  {duration: '2d', displayText: 'Every 2 days'},
-  {duration: '7d', displayText: 'Every 7 days'},
-  {duration: '30d', displayText: 'Every 30 days'},
-]
-
-export const DURATION_STRINGS = [
   '5s',
   '15s',
   '1m',

diff --git a/ui/src/types/queries.ts b/ui/src/types/queries.ts
@@ -24,6 +24,7 @@ export interface SelectableDurationTimeRange {
   label: string
   duration: string
   type: 'selectable-duration'
+  windowPeriod: number
 }
 
 export interface DurationTimeRange {

diff --git a/ui/src/variables/utils/getWindowVars.ts b/ui/src/variables/utils/getWindowVars.ts
@@ -10,6 +10,7 @@ import {WINDOW_PERIOD} from 'src/variables/constants'
 
 // Types
 import {VariableAssignment, Package} from 'src/types/ast'
+import {SELECTABLE_TIME_RANGES} from 'src/shared/constants/timeRanges'
 
 const DESIRED_POINTS_PER_GRAPH = 360
 const FALLBACK_WINDOW_PERIOD = 15000
@@ -63,7 +64,15 @@ export const getWindowPeriod = (
       files: [ast, buildVarsOption(variables)],
     }
 
-    const queryDuration = getMinDurationFromAST(substitutedAST)
+    const queryDuration = getMinDurationFromAST(substitutedAST) // in ms
+
+    const foundDuration = SELECTABLE_TIME_RANGES.find(
+      tr => tr.seconds * 1000 === queryDuration
+    )
+
+    if (foundDuration) {
+      return foundDuration.windowPeriod
+    }
 
     return Math.round(queryDuration / DESIRED_POINTS_PER_GRAPH)
   } catch (error) {