From d2b70acdb05f67c91f03fcbbe9657d1ec47ce762 Mon Sep 17 00:00:00 2001
From: ebb-tide <denizk@gmail.com>
Date: Tue, 3 Apr 2018 11:47:48 -0700
Subject: [PATCH] Implement suggestions from PR review

---
 .../kapacitor/components/KapacitorRules.tsx   | 19 +++++----
 .../containers/KapacitorRulesPage.tsx         | 28 ++++++-------
 ui/src/shared/components/SourceIndicator.tsx  | 41 +++++++++----------
 .../components/KapacitorRules.test.tsx        |  7 ++++
 4 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/ui/src/kapacitor/components/KapacitorRules.tsx b/ui/src/kapacitor/components/KapacitorRules.tsx
index 1f773427f..92d3cf563 100644
--- a/ui/src/kapacitor/components/KapacitorRules.tsx
+++ b/ui/src/kapacitor/components/KapacitorRules.tsx
@@ -7,7 +7,7 @@ import TasksTable from 'src/kapacitor/components/TasksTable'
 
 import {Source, AlertRule} from 'src/types'
 
-interface Props {
+interface KapacitorRulesProps {
   source: Source
   rules: AlertRule[]
   hasKapacitor: boolean
@@ -15,7 +15,8 @@ interface Props {
   onDelete: (rule: AlertRule) => void
   onChangeRuleStatus: (rule: AlertRule) => void
 }
-const KapacitorRules: SFC<Props> = ({
+
+const KapacitorRules: SFC<KapacitorRulesProps> = ({
   source,
   rules,
   hasKapacitor,
@@ -23,7 +24,7 @@ const KapacitorRules: SFC<Props> = ({
   onDelete,
   onChangeRuleStatus,
 }) => {
-  if (loading) {
+  if (loading || !hasKapacitor) {
     return (
       <div>
         <div className="panel-heading">
@@ -34,10 +35,10 @@ const KapacitorRules: SFC<Props> = ({
         </div>
         <div className="panel-body">
           <div className="generic-empty-state">
-            {hasKapacitor ? (
-              <p>Loading Rules...</p>
-            ) : (
+            {!hasKapacitor ? (
               <NoKapacitorError source={source} />
+            ) : (
+              <p>Loading Rules...</p>
             )}
           </div>
         </div>
@@ -45,12 +46,12 @@ const KapacitorRules: SFC<Props> = ({
     )
   }
 
-  const builderRules: AlertRule[] = rules.filter((r: AlertRule) => r.query)
+  const builderRules = rules.filter((r: AlertRule) => r.query)
 
-  const builderHeader: string = `${builderRules.length} Alert Rule${
+  const builderHeader = `${builderRules.length} Alert Rule${
     builderRules.length === 1 ? '' : 's'
   }`
-  const scriptsHeader: string = `${rules.length} TICKscript${
+  const scriptsHeader = `${rules.length} TICKscript${
     rules.length === 1 ? '' : 's'
   }`
 
diff --git a/ui/src/kapacitor/containers/KapacitorRulesPage.tsx b/ui/src/kapacitor/containers/KapacitorRulesPage.tsx
index eed9fba2b..d369f03b4 100644
--- a/ui/src/kapacitor/containers/KapacitorRulesPage.tsx
+++ b/ui/src/kapacitor/containers/KapacitorRulesPage.tsx
@@ -49,20 +49,6 @@ export class KapacitorRulesPage extends PureComponent<Props, State> {
     this.setState({loading: false, hasKapacitor: !!kapacitor})
   }
 
-  public handleDeleteRule = (rule: AlertRule) => {
-    const {actions} = this.props
-
-    actions.deleteRule(rule)
-  }
-
-  public handleRuleStatus = (rule: AlertRule) => {
-    const {actions} = this.props
-    const status = rule.status === 'enabled' ? 'disabled' : 'enabled'
-
-    actions.updateRuleStatus(rule, status)
-    actions.updateRuleStatusSuccess(rule.id, status)
-  }
-
   public render() {
     const {source, rules} = this.props
     const {hasKapacitor, loading} = this.state
@@ -79,6 +65,20 @@ export class KapacitorRulesPage extends PureComponent<Props, State> {
       </PageContents>
     )
   }
+
+  private handleDeleteRule = (rule: AlertRule) => {
+    const {actions} = this.props
+
+    actions.deleteRule(rule)
+  }
+
+  private handleRuleStatus = (rule: AlertRule) => {
+    const {actions} = this.props
+    const status = rule.status === 'enabled' ? 'disabled' : 'enabled'
+
+    actions.updateRuleStatus(rule, status)
+    actions.updateRuleStatusSuccess(rule.id, status)
+  }
 }
 
 interface PageContentsProps {
diff --git a/ui/src/shared/components/SourceIndicator.tsx b/ui/src/shared/components/SourceIndicator.tsx
index c9e6493c3..c2661c259 100644
--- a/ui/src/shared/components/SourceIndicator.tsx
+++ b/ui/src/shared/components/SourceIndicator.tsx
@@ -17,30 +17,27 @@ const SourceIndicator: SFC<Props> = (
 ) => {
   const sourceName: string = _.get(sourceOverride, 'name', name)
   const sourceUrl: string = _.get(sourceOverride, 'url', url)
+  const sourceNameTooltip: string = `<h1>Connected to Source:</h1><p><code>${sourceName} @ ${sourceUrl}</code></p>`
+  const uuidTooltip: string = uuid.v4()
 
-  if (sourceName) {
-    const sourceNameTooltip: string = `<h1>Connected to Source:</h1><p><code>${sourceName} @ ${sourceUrl}</code></p>`
-    const uuidTooltip: string = uuid.v4()
-
-    return (
-      <div
-        className="source-indicator"
-        data-for={uuidTooltip}
-        data-tip={sourceNameTooltip}
-      >
-        <span className="icon server2" />
-        <ReactTooltip
-          id={uuidTooltip}
-          effect="solid"
-          html={true}
-          place="left"
-          class="influx-tooltip"
-        />
-      </div>
-    )
-  }
-  return null
+  return sourceName ? (
+    <div
+      className="source-indicator"
+      data-for={uuidTooltip}
+      data-tip={sourceNameTooltip}
+    >
+      <span className="icon server2" />
+      <ReactTooltip
+        id={uuidTooltip}
+        effect="solid"
+        html={true}
+        place="left"
+        class="influx-tooltip"
+      />
+    </div>
+  ) : null
 }
+
 const {shape} = PropTypes
 
 SourceIndicator.contextTypes = {
diff --git a/ui/test/kapacitor/components/KapacitorRules.test.tsx b/ui/test/kapacitor/components/KapacitorRules.test.tsx
index cb6194fe1..94aa5b624 100644
--- a/ui/test/kapacitor/components/KapacitorRules.test.tsx
+++ b/ui/test/kapacitor/components/KapacitorRules.test.tsx
@@ -51,6 +51,13 @@ describe('Kapacitor.Containers.KapacitorRules', () => {
       expect(noKapacitorError.length).toEqual(1)
     })
 
+    it('renders NoKapacitorError if not loading and no kapacitor configured', () => {
+      const noKapacitorProps = {...props, loading: false, hasKapacitor: false}
+      const wrapper = shallow(<KapacitorRules {...noKapacitorProps} />)
+      const noKapacitorError = wrapper.find(NoKapacitorError)
+      expect(noKapacitorError.length).toEqual(1)
+    })
+
     it('doesnt render NoKapacitorError if loading and kapacitor is configured', () => {
       const noKapacitorProps = {...props, loading: true, hasKapacitor: true}
       const wrapper = shallow(<KapacitorRules {...noKapacitorProps} />)