Parameter feedback - #2 Client errors in query page (#4319)

* Parameter feedback - #2 Client errors in query page * Added cypress test * Fixed percy screenshot * Safer touched change * Parameter feedback - #3 Added in Widgets (#4320) * Parameter feedback - #3 Added in Widgets * Added cypress tests * Making sure widget-level param is selected * Parameter feedback - #4 Added in Dashboard params (#4321) * Parameter feedback - #4 Added in Dashboard params * Added cypress test * Moved to service * Parameter feedback - #5 Unsaved indication (#4322) * Parameter feedback - #5 Unsaved indication * Added ANGULAR_REMOVE_ME * Added cypress test * Fixed percy screenshot * Some code improvements * Parameter input feedback - #6 Better value normalization (#4327)
getredash · Jul 21, 2023 · 13e5500 · 13e5500
1 parent 5213b52
commit 13e5500
Show file tree

Hide file tree

Showing 18 changed files with 412 additions and 53 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/ParameterValueInput.jsx b/client/app/components/ParameterValueInput.jsx
@@ -5,7 +5,7 @@ import Input from 'antd/lib/input';
 import InputNumber from 'antd/lib/input-number';
 import DateParameter from '@/components/dynamic-parameters/DateParameter';
 import DateRangeParameter from '@/components/dynamic-parameters/DateRangeParameter';
-import { isEqual } from 'lodash';
+import { isEqual, trim } from 'lodash';
 import { QueryBasedParameterInput } from './QueryBasedParameterInput';
 
 import './ParameterValueInput.less';
@@ -59,7 +59,7 @@ class ParameterValueInput extends React.Component {
   }
 
   onSelect = (value) => {
-    const isDirty = !isEqual(value, this.props.value);
+    const isDirty = !isEqual(trim(value), trim(this.props.value));
     this.setState({ value, isDirty });
     this.props.onSelect(value, isDirty);
   }
@@ -140,13 +140,11 @@ class ParameterValueInput extends React.Component {
     const { className } = this.props;
     const { value } = this.state;
 
-    const normalize = val => (isNaN(val) ? undefined : val);
-
     return (
       <InputNumber
         className={className}
-        value={normalize(value)}
-        onChange={val => this.onSelect(normalize(val))}
+        value={value}
+        onChange={val => this.onSelect(val)}
       />
     );
   }

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, includes } 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,10 @@ export class Parameters extends React.Component {
     onValuesChange: PropTypes.func,
     onPendingValuesChange: PropTypes.func,
     onParametersEdit: PropTypes.func,
+    queryResultErrorData: PropTypes.shape({
+      parameters: PropTypes.objectOf(PropTypes.string),
+    }),
+    unsavedParameters: PropTypes.arrayOf(PropTypes.string),
   };
 
   static defaultProps = {
@@ -38,25 +44,36 @@ export class Parameters extends React.Component {
     onValuesChange: () => {},
     onPendingValuesChange: () => {},
     onParametersEdit: () => {},
+    queryResultErrorData: {},
+    unsavedParameters: null,
   };
 
   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 +86,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 = { ...touched, [param.name]: true };
       } else {
         param.clearPendingValue();
       }
       onPendingValuesChange();
-      return { parameters };
+      return { parameters, touched };
     });
   };
 
@@ -109,17 +127,47 @@ export class Parameters extends React.Component {
     EditParameterSettingsDialog
       .showModal({ parameter })
       .result.then((updated) => {
-        this.setState(({ parameters }) => {
+        this.setState(({ parameters, touched }) => {
+          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) {
+      const feedback = <Tooltip title={error}>{error}</Tooltip>;
+      return [feedback, 'error'];
+    }
+
+    // unsaved
+    const { unsavedParameters } = this.props;
+    if (includes(unsavedParameters, param.name)) {
+      const feedback = (
+        <>
+          Unsaved{' '}
+          <Tooltip title='Click the "Save" button to preserve this parameter.'>
+            <i className="fa fa-question-circle" />
+          </Tooltip>
+        </>
+      );
+      return [feedback, 'warning'];
+    }
+
+    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 +187,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 || 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/components/dashboards/dashboard-widget/VisualizationWidget.jsx b/client/app/components/dashboards/dashboard-widget/VisualizationWidget.jsx
@@ -75,6 +75,8 @@ RefreshIndicator.defaultProps = { refreshStartedAt: null };
 
 function VisualizationWidgetHeader({ widget, refreshStartedAt, parameters, onParametersUpdate }) {
   const canViewQuery = currentUser.hasPermission('view_query');
+  const queryResult = widget.getQueryResult();
+  const errorData = queryResult && queryResult.getErrorData();
 
   return (
     <>
@@ -90,8 +92,8 @@ function VisualizationWidgetHeader({ widget, refreshStartedAt, parameters, onPar
         </div>
       </div>
       {!isEmpty(parameters) && (
-        <div className="m-b-10">
-          <Parameters parameters={parameters} onValuesChange={onParametersUpdate} />
+        <div className="m-b-5">
+          <Parameters parameters={parameters} queryResultErrorData={errorData} onValuesChange={onParametersUpdate} />
         </div>
       )}
     </>

diff --git a/client/app/components/queries/visualization-embed.html b/client/app/components/queries/visualization-embed.html
@@ -13,7 +13,7 @@ <h3>
 
   <div class="col-md-12 query__vis">
     <div class="p-t-15 p-b-10" ng-if="$ctrl.query.hasParameters() && !$ctrl.hideParametersUI">
-      <parameters parameters="$ctrl.query.getParametersDefs()" on-values-change="$ctrl.refreshQueryResults"></parameters>
+      <parameters parameters="$ctrl.query.getParametersDefs()" query-result-error-data="$ctrl.errorData" on-values-change="$ctrl.refreshQueryResults"></parameters>
     </div>
 
     <div ng-if="$ctrl.error">

diff --git a/client/app/components/queries/visualization-embed.js b/client/app/components/queries/visualization-embed.js
@@ -14,6 +14,7 @@ const VisualizationEmbed = {
     this.refreshQueryResults = () => {
       this.loading = true;
       this.error = null;
+      this.errorData = {};
       this.refreshStartedAt = moment();
       this.query
         .getQueryResultPromise()
@@ -24,6 +25,7 @@ const VisualizationEmbed = {
         .catch((error) => {
           this.loading = false;
           this.error = error.getError();
+          this.errorData = error.getErrorData();
         });
     };
 

diff --git a/client/app/pages/dashboards/dashboard.html b/client/app/pages/dashboards/dashboard.html
@@ -93,8 +93,8 @@ <h3>
     </label>
   </div>
 
-  <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0" data-test="DashboardParameters">
-    <parameters parameters="$ctrl.globalParameters" on-values-change="$ctrl.refreshDashboard"></parameters>
+  <div class="m-b-10 p-t-15 p-l-15 p-r-15 p-b-5 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0" data-test="DashboardParameters">
+    <parameters parameters="$ctrl.globalParameters" query-result-error-data="$ctrl.dashboard.getQueryResultsErrorData()" on-values-change="$ctrl.refreshDashboard"></parameters>
   </div>
 
   <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.filters | notEmpty">

diff --git a/client/app/pages/dashboards/public-dashboard-page.html b/client/app/pages/dashboards/public-dashboard-page.html
@@ -1,8 +1,8 @@
 <div class="container p-t-10 p-b-20" ng-if="$ctrl.dashboard">
   <page-header title="$ctrl.dashboard.name"></page-header>
 
-  <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0">
-    <parameters parameters="$ctrl.globalParameters" on-values-change="$ctrl.refreshDashboard"></parameters>
+  <div class="m-b-10 p-t-15 p-l-15 p-r-15 p-b-5 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0">
+    <parameters parameters="$ctrl.globalParameters" query-result-error-data="$ctrl.dashboard.getQueryResultsErrorData()" on-values-change="$ctrl.refreshDashboard"></parameters>
   </div>
 
   <div class="m-b-5">

diff --git a/client/app/pages/queries/query.html b/client/app/pages/queries/query.html
@@ -199,8 +199,8 @@ <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()"
-                  on-values-change="executeQuery" on-pending-values-change="applyParametersChanges" on-parameters-edit="onParametersUpdated"></parameters>
+                <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" unsaved-parameters="getUnsavedParameters()"></parameters>
               </div>
               <!-- Query Execution Status -->
 

diff --git a/client/app/pages/queries/source-view.js b/client/app/pages/queries/source-view.js
@@ -1,4 +1,4 @@
-import { map, debounce } from 'lodash';
+import { map, debounce, isEmpty, isEqual } from 'lodash';
 import template from './query.html';
 import EditParameterSettingsDialog from '@/components/EditParameterSettingsDialog';
 
@@ -109,6 +109,22 @@ function QuerySourceCtrl(
   $scope.$watch('query.query', (newQueryText) => {
     $scope.isDirty = newQueryText !== queryText;
   });
+
+  $scope.unsavedParameters = null;
+  $scope.getUnsavedParameters = () => {
+    if (!$scope.isDirty || !queryText) {
+      return null;
+    }
+    const unsavedParameters = $scope.query.$parameters.getUnsavedParameters(queryText);
+    if (isEmpty(unsavedParameters)) {
+      return null;
+    }
+    // avoiding Angular infdig (ANGULAR_REMOVE_ME)
+    if (!isEqual(unsavedParameters, $scope.unsavedParameters)) {
+      $scope.unsavedParameters = unsavedParameters;
+    }
+    return $scope.unsavedParameters;
+  };
 }
 
 export default function init(ngModule) {

diff --git a/client/app/services/dashboard.js b/client/app/services/dashboard.js
@@ -249,6 +249,39 @@ function DashboardService($resource, $http, $location, currentUser) {
     });
   };
 
+  let currentQueryResultsErrorData; // swap for useMemo ANGULAR_REMOVE_ME
+  resource.prototype.getQueryResultsErrorData = function getQueryResultsErrorData() {
+    const dashboardErrors = _.map(this.widgets, (widget) => {
+      // get result
+      const result = widget.getQueryResult();
+      if (!result) {
+        return null;
+      }
+
+      // get error data
+      const errorData = result.getErrorData();
+      if (_.isEmpty(errorData)) {
+        return null;
+      }
+
+      // dashboard params only
+      const localParamNames = _.map(widget.getLocalParameters(), p => p.name);
+      const filtered = _.omit(errorData.parameters, localParamNames);
+
+      return filtered;
+    });
+
+    const merged = _.assign({}, ...dashboardErrors);
+    const errorData = _.isEmpty(merged) ? null : { parameters: merged };
+
+    // avoiding Angular infdig (ANGULAR_REMOVE_ME)
+    if (!_.isEqual(currentQueryResultsErrorData, errorData)) {
+      currentQueryResultsErrorData = errorData;
+    }
+
+    return currentQueryResultsErrorData;
+  };
+
   return resource;
 }