Merge pull request #1087 from influxdata/bugfix/1003-hosts-loading-me…

…ssage Bugfix/1003: Hosts Loading Messaging
influxdata · Mar 28, 2017 · e7e7a45 · e7e7a45
2 parents 660d764 + 4312297
commit e7e7a45
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,15 +1,12 @@
 ## v1.2.0 [unreleased]
 
-### Bug Fixes
-### Features
-### UI Improvements
-
 ## v1.2.0-beta6 [2017-03-24]
 
 ### Bug Fixes
   1. [#1065](https://github.com/influxdata/chronograf/pull/1065): Add functionality to the `save` and `cancel` buttons on editable dashboards
   2. [#1069](https://github.com/influxdata/chronograf/pull/1069): Make graphs on pre-created dashboards un-editable
   3. [#1085](https://github.com/influxdata/chronograf/pull/1085): Make graphs resizable again
+  4. [#1087](https://github.com/influxdata/chronograf/pull/1087): Hosts page now displays proper loading, host count, and error messages.
 
 ### Features
   1. [#1056](https://github.com/influxdata/chronograf/pull/1056): Add ability to add a dashboard cell

diff --git a/ui/src/hosts/components/HostsTable.js b/ui/src/hosts/components/HostsTable.js
@@ -3,17 +3,27 @@ import shallowCompare from 'react-addons-shallow-compare';
 import {Link} from 'react-router';
 import _ from 'lodash';
 
+const {
+  arrayOf,
+  bool,
+  number,
+  shape,
+  string,
+} = PropTypes
+
 const HostsTable = React.createClass({
   propTypes: {
-    hosts: PropTypes.arrayOf(PropTypes.shape({
-      name: PropTypes.string,
-      cpu: PropTypes.number,
-      load: PropTypes.number,
-      apps: PropTypes.arrayOf(PropTypes.string.isRequired),
+    hosts: arrayOf(shape({
+      name: string,
+      cpu: number,
+      load: number,
+      apps: arrayOf(string.isRequired),
     })),
-    source: PropTypes.shape({
-      id: PropTypes.string.isRequired,
-      name: PropTypes.string.isRequired,
+    hostsLoading: bool,
+    hostsError: string,
+    source: shape({
+      id: string.isRequired,
+      name: string.isRequired,
     }).isRequired,
   },
 
@@ -81,18 +91,23 @@ const HostsTable = React.createClass({
   },
 
   render() {
-    const {searchTerm, sortKey, sortDirection} = this.state;
-    const {hosts, source} = this.props;
-    const sortedHosts = this.sort(this.filter(hosts, searchTerm), sortKey, sortDirection);
-    const hostCount = sortedHosts.length;
-
-    let hostsTitle;
-    if (hosts.length === 0) {
-      hostsTitle = `Loading Hosts...`;
+    const {searchTerm, sortKey, sortDirection} = this.state
+    const {hosts, hostsLoading, hostsError, source} = this.props
+    const sortedHosts = this.sort(this.filter(hosts, searchTerm), sortKey, sortDirection)
+    const hostCount = sortedHosts.length
+
+    let hostsTitle
+
+    if (hostsLoading) {
+      hostsTitle = `Loading Hosts...`
+    } else if (hostsError.length) {
+      hostsTitle = `There was a problem loading hosts`
+    } else if (hosts.length === 0) {
+      hostsTitle = `No hosts found`
     } else if (hostCount === 1) {
-      hostsTitle = `${hostCount} Host`;
+      hostsTitle = `${hostCount} Host`
     } else {
-      hostsTitle = `${hostCount} Hosts`;
+      hostsTitle = `${hostCount} Hosts`
     }
 
     return (

diff --git a/ui/src/hosts/containers/HostsPage.js b/ui/src/hosts/containers/HostsPage.js
@@ -21,7 +21,8 @@ export const HostsPage = React.createClass({
   getInitialState() {
     return {
       hosts: {},
-      up: {},
+      hostsLoading: true,
+      hostsError: '',
     };
   },
 
@@ -30,22 +31,43 @@ export const HostsPage = React.createClass({
     Promise.all([
       getCpuAndLoadForHosts(source.links.proxy, source.telegraf),
       getMappings(),
-    ]).then(([hosts, {data: {mappings}}, up]) => {
-      this.setState({hosts, up});
+      new Promise((resolve) => {
+        this.setState({hostsLoading: true})
+        resolve()
+      }),
+    ]).then(([hosts, {data: {mappings}}]) => {
+      this.setState({
+        hosts,
+        hostsLoading: false,
+      })
       getAppsForHosts(source.links.proxy, hosts, mappings, source.telegraf).then((newHosts) => {
-        this.setState({hosts: newHosts});
+        this.setState({
+          hosts: newHosts,
+          hostsError: '',
+          hostsLoading: false,
+        })
       }).catch(() => {
-        addFlashMessage({type: 'error', text: 'Unable to get apps for hosts'});
+        const reason = 'Unable to get apps for hosts'
+        addFlashMessage({type: 'error', text: reason})
+        this.setState({
+          hostsError: reason,
+          hostsLoading: false,
+        })
       });
     }).catch((reason) => {
+      this.setState({
+        hostsError: reason.toString(),
+        hostsLoading: false,
+      })
       // TODO: this isn't reachable at the moment, because getCpuAndLoadForHosts doesn't fail when it should.
       // (like with a bogus proxy link). We should provide better messaging to the user in this catch after that's fixed.
       console.error(reason); // eslint-disable-line no-console
     });
   },
 
   render() {
-    const {source} = this.props;
+    const {source} = this.props
+    const {hosts, hostsLoading, hostsError} = this.state
     return (
       <div className="page">
         <div className="page-header">
@@ -64,7 +86,12 @@ export const HostsPage = React.createClass({
           <div className="container-fluid">
             <div className="row">
               <div className="col-md-12">
-                <HostsTable source={source} hosts={_.values(this.state.hosts)} up={this.state.up} />
+                <HostsTable
+                  source={source}
+                  hosts={_.values(hosts)}
+                  hostsLoading={hostsLoading}
+                  hostsError={hostsError}
+                />
               </div>
             </div>
           </div>