Parameter feedback - #2 Client errors in query page

getredash · Oct 31, 2019 · bad3f14 · bad3f14
1 parent 5213b52
commit bad3f14
Show file tree

Hide file tree

Showing 7 changed files with 204 additions and 23 deletions.
diff --git a/client/app/components/EditParameterSettingsDialog.jsx b/client/app/components/EditParameterSettingsDialog.jsx
@@ -179,6 +179,7 @@ function EditParameterSettingsDialog(props) {
         {param.type === 'enum' && (
           <Form.Item label="Values" help="Dropdown list values (newline delimited)" {...formItemProps}>
             <Input.TextArea
+              data-test="EnumTextArea"
               rows={3}
               value={param.enumOptions}
               onChange={e => setParam({ ...param, enumOptions: e.target.value })}

diff --git a/client/app/components/Parameters.jsx b/client/app/components/Parameters.jsx
@@ -1,12 +1,14 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { size, filter, forEach, extend } from 'lodash';
+import { size, filter, forEach, extend, get } from 'lodash';
 import { react2angular } from 'react2angular';
 import { SortableContainer, SortableElement, DragHandle } from '@/components/sortable';
 import { $location } from '@/services/ng';
 import { Parameter } from '@/services/parameters';
 import ParameterApplyButton from '@/components/ParameterApplyButton';
 import ParameterValueInput from '@/components/ParameterValueInput';
+import Form from 'antd/lib/form';
+import Tooltip from 'antd/lib/tooltip';
 import EditParameterSettingsDialog from './EditParameterSettingsDialog';
 import { toHuman } from '@/filters';
 
@@ -29,6 +31,9 @@ export class Parameters extends React.Component {
     onValuesChange: PropTypes.func,
     onPendingValuesChange: PropTypes.func,
     onParametersEdit: PropTypes.func,
+    queryResultErrorData: PropTypes.shape({
+      parameters: PropTypes.objectOf(PropTypes.string),
+    }),
   };
 
   static defaultProps = {
@@ -38,25 +43,35 @@ export class Parameters extends React.Component {
     onValuesChange: () => {},
     onPendingValuesChange: () => {},
     onParametersEdit: () => {},
+    queryResultErrorData: {},
   };
 
   constructor(props) {
     super(props);
     const { parameters } = props;
-    this.state = { parameters };
+    this.state = {
+      parameters,
+      touched: {},
+    };
+
     if (!props.disableUrlUpdate) {
       updateUrl(parameters);
     }
   }
 
   componentDidUpdate = (prevProps) => {
-    const { parameters, disableUrlUpdate } = this.props;
+    const { parameters, disableUrlUpdate, queryResultErrorData } = this.props;
     if (prevProps.parameters !== parameters) {
       this.setState({ parameters });
       if (!disableUrlUpdate) {
         updateUrl(parameters);
       }
     }
+
+    // reset touched flags on new error data
+    if (prevProps.queryResultErrorData !== queryResultErrorData) {
+      this.setState({ touched: {} });
+    }
   };
 
   handleKeyDown = (e) => {
@@ -69,14 +84,15 @@ export class Parameters extends React.Component {
 
   setPendingValue = (param, value, isDirty) => {
     const { onPendingValuesChange } = this.props;
-    this.setState(({ parameters }) => {
+    this.setState(({ parameters, touched }) => {
       if (isDirty) {
         param.setPendingValue(value);
+        touched[param.name] = true;
       } else {
         param.clearPendingValue();
       }
       onPendingValuesChange();
-      return { parameters };
+      return { parameters, touched };
     });
   };
 
@@ -109,17 +125,32 @@ export class Parameters extends React.Component {
     EditParameterSettingsDialog
       .showModal({ parameter })
       .result.then((updated) => {
-        this.setState(({ parameters }) => {
+        this.setState(({ parameters, touched }) => {
+          touched[parameter.name] = true;
           const updatedParameter = extend(parameter, updated);
           parameters[index] = Parameter.create(updatedParameter, updatedParameter.parentQueryId);
           onParametersEdit();
-          return { parameters };
+          return { parameters, touched };
         });
       });
   };
 
+  getParameterFeedback = (param) => {
+    // error msg
+    const { queryResultErrorData } = this.props;
+    const error = get(queryResultErrorData, ['parameters', param.name], false);
+    if (error) {
+      return [error, 'error'];
+    }
+
+    return [];
+  };
+
   renderParameter(param, index) {
     const { editable } = this.props;
+    const touched = this.state.touched[param.name];
+    const [feedback, status] = this.getParameterFeedback(param);
+
     return (
       <div
         key={param.name}
@@ -139,14 +170,19 @@ export class Parameters extends React.Component {
             </button>
           )}
         </div>
-        <ParameterValueInput
-          type={param.type}
-          value={param.normalizedValue}
-          parameter={param}
-          enumOptions={param.enumOptions}
-          queryId={param.queryId}
-          onSelect={(value, isDirty) => this.setPendingValue(param, value, isDirty)}
-        />
+        <Form.Item
+          validateStatus={touched ? '' : status}
+          help={feedback ? <Tooltip title={feedback}>{feedback}</Tooltip> : null}
+        >
+          <ParameterValueInput
+            type={param.type}
+            value={param.normalizedValue}
+            parameter={param}
+            enumOptions={param.enumOptions}
+            queryId={param.queryId}
+            onSelect={(value, isDirty) => this.setPendingValue(param, value, isDirty)}
+          />
+        </Form.Item>
       </div>
     );
   }

diff --git a/client/app/components/Parameters.less b/client/app/components/Parameters.less
@@ -3,7 +3,7 @@
 .parameter-block {
   display: inline-block;
   background: white;
-  padding: 0 12px 6px 0;
+  padding: 0 12px 17px 0;
   vertical-align: top;
   z-index: 1;
 
@@ -15,12 +15,31 @@
 
   .parameter-container.sortable-container & {
     margin: 4px 0 0 4px;
-    padding: 3px 6px 6px;
+    padding: 3px 6px 19px;
   }
 
   &.parameter-dragged {
     box-shadow:  0 4px 9px -3px rgba(102, 136, 153, 0.15);
   }
+
+  .ant-form-item {
+    margin-bottom: 0 !important;
+  }
+
+  .ant-form-explain {
+    position: absolute;
+    left: 0;
+    right: 0;
+    bottom: -20px;
+    font-size: 12px;
+    overflow: hidden;
+    text-overflow: ellipsis;
+    white-space: nowrap;
+  }
+
+  .ant-form-item-control {
+    line-height: normal;
+  }
 }
 
 .parameter-heading {
@@ -107,4 +126,4 @@
       box-shadow: 0 0 0 1px white, -1px 1px 0 1px #5d6f7d85;
     }
   }
-}
+}
diff --git a/client/app/pages/queries/query.html b/client/app/pages/queries/query.html
@@ -199,7 +199,7 @@ <h3>
           <section class="flex-fill p-relative t-body query-visualizations-wrapper">
             <div class="d-flex flex-column p-b-15 p-absolute static-position__mobile" style="left: 0; top: 0; right: 0; bottom: 0;">
               <div class="p-t-15 p-b-5" ng-if="query.hasParameters()">
-                <parameters parameters="query.getParametersDefs()" editable="sourceMode && canEdit" disable-url-update="query.isNew()"
+                <parameters parameters="query.getParametersDefs()" query-result-error-data="queryResult.getErrorData()" editable="sourceMode && canEdit" disable-url-update="query.isNew()"
                   on-values-change="executeQuery" on-pending-values-change="applyParametersChanges" on-parameters-edit="onParametersUpdated"></parameters>
               </div>
               <!-- Query Execution Status -->

diff --git a/client/app/services/query-result.js b/client/app/services/query-result.js
@@ -132,7 +132,7 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) {
         this.status = 'processing';
       } else if (this.job.status === 4) {
         this.status = statuses[this.job.status];
-        this.deferred.reject(new QueryResultError(this.job.error));
+        this.deferred.reject(new QueryResultError(this.job.error, this.job.error_data));
       } else {
         this.status = undefined;
       }
@@ -166,6 +166,10 @@ function QueryResultService($resource, $timeout, $q, QueryResultError, Auth) {
       return this.job.error;
     }
 
+    getErrorData() {
+      return this.job.error_data || undefined;
+    }
+
     getLog() {
       if (!this.query_result.data || !this.query_result.data.log || this.query_result.data.log.length === 0) {
         return null;

diff --git a/client/app/services/query.js b/client/app/services/query.js
@@ -140,8 +140,9 @@ class Parameters {
 
 function QueryResultErrorFactory($q) {
   class QueryResultError {
-    constructor(errorMessage) {
+    constructor(errorMessage, errorData = {}) {
       this.errorMessage = errorMessage;
+      this.errorData = errorData;
       this.updatedAt = moment.utc();
     }
 
@@ -153,6 +154,10 @@ function QueryResultErrorFactory($q) {
       return this.errorMessage;
     }
 
+    getErrorData() {
+      return this.errorData || undefined;
+    }
+
     toPromise() {
       return $q.reject(this);
     }