[sql lab] improvements to the left panel (#2709)

* [sql lab] improvements to the left panel * better error handling with error notifications * table is added instantly with a loading image * Fixing tests * Fixed tests
apache · May 9, 2017 · a471afe · a471afe
1 parent 5d0a01d
commit a471afe
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 113 deletions.
diff --git a/superset/assets/javascripts/SqlLab/actions.js b/superset/assets/javascripts/SqlLab/actions.js
@@ -247,6 +247,17 @@ export function mergeTable(table, query) {
 
 export function addTable(query, tableName, schemaName) {
   return function (dispatch) {
+    let table = {
+      dbId: query.dbId,
+      queryEditorId: query.id,
+      schema: schemaName,
+      name: tableName,
+      isMetadataLoading: true,
+      isExtraMetadataLoading: true,
+      expanded: false,
+    };
+    dispatch(mergeTable(table));
+
     let url = `/superset/table/${query.dbId}/${tableName}/${schemaName}/`;
     $.get(url, (data) => {
       const dataPreviewQuery = {
@@ -260,35 +271,21 @@ export function addTable(query, tableName, schemaName) {
         ctas: false,
       };
       // Merge table to tables in state
-      dispatch(mergeTable(
-        Object.assign(data, {
-          dbId: query.dbId,
-          queryEditorId: query.id,
-          schema: schemaName,
-          expanded: true,
-        }), dataPreviewQuery),
-      );
+      table = Object.assign({}, table, data, {
+        expanded: true,
+        isMetadataLoading: false,
+      });
+      dispatch(mergeTable(table, dataPreviewQuery));
       // Run query to get preview data for table
       dispatch(runQuery(dataPreviewQuery));
     })
     .fail(() => {
-      dispatch(
-        addAlert({
-          msg: 'Error occurred while fetching metadata',
-          bsStyle: 'danger',
-        }),
-      );
+      notify.error('Error occurred while fetching table metadata');
     });
 
     url = `/superset/extra_table_metadata/${query.dbId}/${tableName}/${schemaName}/`;
     $.get(url, (data) => {
-      const table = {
-        dbId: query.dbId,
-        queryEditorId: query.id,
-        schema: schemaName,
-        name: tableName,
-      };
-      Object.assign(table, data);
+      table = Object.assign({}, table, data, { isExtraMetadataLoading: false });
       dispatch(mergeTable(table));
     });
   };

diff --git a/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx b/superset/assets/javascripts/SqlLab/components/SqlEditorLeftBar.jsx
@@ -1,3 +1,4 @@
+/* global notify */
 import React from 'react';
 import PropTypes from 'prop-types';
 import { Button } from 'react-bootstrap';
@@ -82,6 +83,10 @@ class SqlEditorLeftBar extends React.PureComponent {
           tableOptions: data.options,
           tableLength: data.tableLength,
         });
+      })
+      .fail(() => {
+        this.setState({ tableLoading: false, tableOptions: [], tableLength: 0 });
+        notify.error('Error while fetching table list');
       });
     } else {
       this.setState({ tableLoading: false, tableOptions: [], filterOptions: null });
@@ -104,11 +109,7 @@ class SqlEditorLeftBar extends React.PureComponent {
       this.props.actions.queryEditorSetSchema(this.props.queryEditor, schemaName);
       this.fetchTables(this.props.queryEditor.dbId, schemaName);
     }
-    this.setState({ tableLoading: true });
-    // TODO: handle setting the tableLoading state depending on success or
-    //       failure of the addTable async call in the action.
     this.props.actions.addTable(this.props.queryEditor, tableName, schemaName);
-    this.setState({ tableLoading: false });
   }
   changeSchema(schemaOpt) {
     const schema = (schemaOpt) ? schemaOpt.value : null;
@@ -122,8 +123,11 @@ class SqlEditorLeftBar extends React.PureComponent {
       const url = `/superset/schemas/${actualDbId}/`;
       $.get(url, (data) => {
         const schemaOptions = data.schemas.map(s => ({ value: s, label: s }));
-        this.setState({ schemaOptions });
-        this.setState({ schemaLoading: false });
+        this.setState({ schemaOptions, schemaLoading: false });
+      })
+      .fail(() => {
+        this.setState({ schemaLoading: false, schemaOptions: [] });
+        notify.error('Error while fetching schema list');
       });
     }
   }
@@ -145,6 +149,7 @@ class SqlEditorLeftBar extends React.PureComponent {
               '_od_DatabaseView=asc'
             }
             onChange={this.onDatabaseChange.bind(this)}
+            onAsyncError={() => notify.error('Error while fetching database list')}
             value={this.props.queryEditor.dbId}
             databaseId={this.props.queryEditor.dbId}
             actions={this.props.actions}

diff --git a/superset/assets/javascripts/SqlLab/components/TableElement.jsx b/superset/assets/javascripts/SqlLab/components/TableElement.jsx
@@ -8,6 +8,7 @@ import CopyToClipboard from '../../components/CopyToClipboard';
 import Link from './Link';
 import ColumnElement from './ColumnElement';
 import ModalTrigger from '../../components/ModalTrigger';
+import Loading from '../../components/Loading';
 
 const propTypes = {
   table: PropTypes.object,
@@ -62,7 +63,7 @@ class TableElement extends React.PureComponent {
     this.props.actions.removeTable(this.props.table);
   }
 
-  renderHeader() {
+  renderWell() {
     const table = this.props.table;
     let header;
     if (table.partitions) {
@@ -97,7 +98,89 @@ class TableElement extends React.PureComponent {
     }
     return header;
   }
-  renderMetadata() {
+  renderControls() {
+    let keyLink;
+    const table = this.props.table;
+    if (table.indexes && table.indexes.length > 0) {
+      keyLink = (
+        <ModalTrigger
+          modalTitle={
+            <div>
+              Keys for table <strong>{table.name}</strong>
+            </div>
+          }
+          modalBody={table.indexes.map((ix, i) => (
+            <pre key={i}>{JSON.stringify(ix, null, '  ')}</pre>
+          ))}
+          triggerNode={
+            <Link
+              className="fa fa-key pull-left m-l-2"
+              tooltip={`View keys & indexes (${table.indexes.length})`}
+            />
+          }
+        />
+      );
+    }
+    return (
+      <ButtonGroup className="ws-el-controls pull-right">
+        {keyLink}
+        <Link
+          className={
+            `fa fa-sort-${!this.state.sortColumns ? 'alpha' : 'numeric'}-asc ` +
+            'pull-left sort-cols m-l-2'}
+          onClick={this.toggleSortColumns.bind(this)}
+          tooltip={
+            !this.state.sortColumns ?
+            'Sort columns alphabetically' :
+            'Original table column order'}
+          href="#"
+        />
+        {table.selectStar &&
+          <CopyToClipboard
+            copyNode={
+              <a className="fa fa-clipboard pull-left m-l-2" />
+            }
+            text={table.selectStar}
+            shouldShowText={false}
+            tooltipText="Copy SELECT statement to clipboard"
+          />
+        }
+        <Link
+          className="fa fa-times table-remove pull-left m-l-2"
+          onClick={this.removeTable.bind(this)}
+          tooltip="Remove table preview"
+          href="#"
+        />
+      </ButtonGroup>
+    );
+  }
+  renderHeader() {
+    const table = this.props.table;
+    return (
+      <div className="clearfix">
+        <div className="pull-left">
+          <a
+            href="#"
+            className="table-name"
+            onClick={(e) => { this.toggleTable(e); }}
+          >
+            <strong>{table.name}</strong>
+            <small className="m-l-5">
+              <i className={`fa fa-${table.expanded ? 'minus' : 'plus'}-square-o`} />
+            </small>
+          </a>
+        </div>
+        <div className="pull-right">
+          {table.isMetadataLoading || table.isExtraMetadataLoading ?
+            <Loading size={20} />
+            :
+            this.renderControls()
+          }
+        </div>
+      </div>
+    );
+  }
+  renderBody() {
     const table = this.props.table;
     let cols;
     if (table.columns) {
@@ -112,7 +195,7 @@ class TableElement extends React.PureComponent {
         timeout={this.props.timeout}
       >
         <div>
-          {this.renderHeader()}
+          {this.renderWell()}
           <div className="table-columns">
             {cols && cols.map(col => (
               <ColumnElement column={col} key={col.name} />
@@ -126,28 +209,6 @@ class TableElement extends React.PureComponent {
   }
 
   render() {
-    const table = this.props.table;
-    let keyLink;
-    if (table.indexes && table.indexes.length > 0) {
-      keyLink = (
-        <ModalTrigger
-          modalTitle={
-            <div>
-              Keys for table <strong>{table.name}</strong>
-            </div>
-          }
-          modalBody={table.indexes.map((ix, i) => (
-            <pre key={i}>{JSON.stringify(ix, null, '  ')}</pre>
-          ))}
-          triggerNode={
-            <Link
-              className="fa fa-key pull-left m-l-2"
-              tooltip={`View keys & indexes (${table.indexes.length})`}
-            />
-          }
-        />
-      );
-    }
     return (
       <Collapse
         in={this.state.expanded}
@@ -156,54 +217,9 @@ class TableElement extends React.PureComponent {
         onExited={this.removeFromStore.bind(this)}
       >
         <div className="TableElement">
-          <div className="clearfix">
-            <div className="pull-left">
-              <a
-                href="#"
-                className="table-name"
-                onClick={(e) => { this.toggleTable(e); }}
-              >
-                <strong>{table.name}</strong>
-                <small className="m-l-5">
-                  <i className={`fa fa-${table.expanded ? 'minus' : 'plus'}-square-o`} />
-                </small>
-              </a>
-            </div>
-            <div className="pull-right">
-              <ButtonGroup className="ws-el-controls pull-right">
-                {keyLink}
-                <Link
-                  className={
-                    `fa fa-sort-${!this.state.sortColumns ? 'alpha' : 'numeric'}-asc ` +
-                    'pull-left sort-cols m-l-2'}
-                  onClick={this.toggleSortColumns.bind(this)}
-                  tooltip={
-                    !this.state.sortColumns ?
-                    'Sort columns alphabetically' :
-                    'Original table column order'}
-                  href="#"
-                />
-                {table.selectStar &&
-                  <CopyToClipboard
-                    copyNode={
-                      <a className="fa fa-clipboard pull-left m-l-2" />
-                    }
-                    text={table.selectStar}
-                    shouldShowText={false}
-                    tooltipText="Copy SELECT statement to clipboard"
-                  />
-                }
-                <Link
-                  className="fa fa-trash table-remove pull-left m-l-2"
-                  onClick={this.removeTable.bind(this)}
-                  tooltip="Remove table preview"
-                  href="#"
-                />
-              </ButtonGroup>
-            </div>
-          </div>
+          {this.renderHeader()}
           <div>
-            {this.renderMetadata()}
+            {this.renderBody()}
           </div>
         </div>
       </Collapse>

diff --git a/superset/assets/javascripts/components/AsyncSelect.jsx b/superset/assets/javascripts/components/AsyncSelect.jsx
@@ -8,6 +8,7 @@ const propTypes = {
   dataEndpoint: PropTypes.string.isRequired,
   onChange: PropTypes.func.isRequired,
   mutator: PropTypes.func.isRequired,
+  onAsyncError: PropTypes.func,
   value: PropTypes.number,
   valueRenderer: PropTypes.func,
   placeholder: PropTypes.string,
@@ -17,6 +18,7 @@ const propTypes = {
 const defaultProps = {
   placeholder: 'Select ...',
   valueRenderer: o => (<div>{o.label}</div>),
+  onAsyncError: () => {},
 };
 
 class AsyncSelect extends React.PureComponent {
@@ -42,6 +44,12 @@ class AsyncSelect extends React.PureComponent {
       if (this.props.autoSelect && this.state.options.length) {
         this.onChange(this.state.options[0]);
       }
+    })
+    .fail(() => {
+      this.props.onAsyncError();
+    })
+    .always(() => {
+      this.setState({ isLoading: false });
     });
   }
   render() {

diff --git a/superset/assets/javascripts/components/Loading.jsx b/superset/assets/javascripts/components/Loading.jsx
@@ -0,0 +1,27 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+const propTypes = {
+  size: PropTypes.number,
+};
+const defaultProps = {
+  size: 25,
+};
+
+export default function Loading(props) {
+  return (
+    <img
+      className="loading"
+      alt="Loading..."
+      src="/static/assets/images/loading.gif"
+      style={{
+        width: props.size,
+        height: props.size,
+        padding: 0,
+        margin: 0,
+      }}
+    />
+  );
+}
+Loading.propTypes = propTypes;
+Loading.defaultProps = defaultProps;