Time shift difference (apache#5177)

* Allow different comparisons when using time shift * Remove test code * Remove lead/trail NaN and improve code * Add headers/subheader * Update yarn * Migration script * Fix migration * Small fixes * Trigger tests * Fix lint * Fix javascript
graceguo-supercat · Jul 16, 2018 · 7b4e6c7 · 7b4e6c7
1 parent f8a6e09
commit 7b4e6c7
Show file tree

Hide file tree

Showing 12 changed files with 285 additions and 22,628 deletions.
diff --git a/superset/assets/package-lock.json b/superset/assets/package-lock.json
diff --git a/superset/assets/package.json b/superset/assets/package.json
@@ -68,6 +68,7 @@
     "geojson-extent": "^0.3.2",
     "geolib": "^2.0.24",
     "immutable": "^3.8.2",
+    "is-react": "^1.1.1",
     "jed": "^1.1.1",
     "jquery": "3.1.1",
     "lodash.throttle": "^4.1.1",

diff --git a/superset/assets/src/SqlLab/actions.js b/superset/assets/src/SqlLab/actions.js
@@ -435,7 +435,9 @@ export function popDatasourceQuery(datasourceKey, sql) {
         };
         dispatch(addQueryEditor(queryEditorProps));
       },
-      error: () => notify.error(t("The datasource couldn't be loaded")),
+      error: () => {
+        dispatch(addDangerToast(t("The datasource couldn't be loaded")));
+      },
     });
   };
 }

diff --git a/superset/assets/src/explore/components/ControlPanelsContainer.jsx b/superset/assets/src/explore/components/ControlPanelsContainer.jsx
@@ -1,5 +1,6 @@
 /* eslint camelcase: 0 */
 import React from 'react';
+import isReact from 'is-react';
 import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
@@ -29,6 +30,10 @@ class ControlPanelsContainer extends React.Component {
     this.renderControlPanelSection = this.renderControlPanelSection.bind(this);
   }
   getControlData(controlName) {
+    if (isReact.element(controlName)) {
+      return controlName;
+    }
+
     const control = this.props.controls[controlName];
     // Identifying mapStateToProps function to apply (logic can't be in store)
     let mapF = controls[controlName].mapStateToProps;
@@ -69,19 +74,24 @@ class ControlPanelsContainer extends React.Component {
           <ControlRow
             key={`controlsetrow-${i}`}
             className="control-row"
-            controls={controlSets.map(controlName => (
-              controlName &&
-              ctrls[controlName] &&
-                <Control
+            controls={controlSets.map((controlName) => {
+              if (!controlName) {
+                return null;
+              } else if (isReact.element(controlName)) {
+                return controlName;
+              } else if (ctrls[controlName]) {
+                return (<Control
                   name={controlName}
                   key={`control-${controlName}`}
                   value={this.props.form_data[controlName]}
                   validationErrors={ctrls[controlName].validationErrors}
                   actions={this.props.actions}
                   formData={ctrls[controlName].provideFormDataToProps ? this.props.form_data : null}
                   {...this.getControlData(controlName)}
-                />
-            ))}
+                />);
+              }
+              return null;
+            })}
           />
         ))}
       </ControlPanelSection>

diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx
@@ -864,7 +864,7 @@ export const controls = {
   resample_rule: {
     type: 'SelectControl',
     freeForm: true,
-    label: t('Resample Rule'),
+    label: t('Rule'),
     default: null,
     choices: formatSelectOptions(['', '1T', '1H', '1D', '7D', '1M', '1AS']),
     description: t('Pandas resample rule'),
@@ -873,7 +873,7 @@ export const controls = {
   resample_how: {
     type: 'SelectControl',
     freeForm: true,
-    label: t('Resample How'),
+    label: t('How'),
     default: null,
     choices: formatSelectOptions(['', 'mean', 'sum', 'median']),
     description: t('Pandas resample how'),
@@ -882,7 +882,7 @@ export const controls = {
   resample_fillmethod: {
     type: 'SelectControl',
     freeForm: true,
-    label: t('Resample Fill Method'),
+    label: t('Fill Method'),
     default: null,
     choices: formatSelectOptions(['', 'ffill', 'bfill']),
     description: t('Pandas resample fill method'),
@@ -1203,11 +1203,12 @@ export const controls = {
     mapStateToProps: (state) => {
       const showWarning = (
           state.controls &&
-          state.controls.num_period_compare &&
-          state.controls.num_period_compare.value !== '');
+          state.controls.comparison_type &&
+          state.controls.comparison_type.value === 'percentage');
       return {
         warning: showWarning ?
-          t('When `Period Ratio` is set, the Y Axis Format is forced to `.1%`') : null,
+          t('When `Calculation type` is set to "Percentage change", the Y ' +
+            'Axis Format is forced to `.1%`') : null,
         disabled: showWarning,
       };
     },
@@ -1548,30 +1549,11 @@ export const controls = {
     description: t('Compute the contribution to the total'),
   },
 
-  num_period_compare: {
-    type: 'TextControl',
-    label: t('Period Ratio'),
-    default: '',
-    isInt: true,
-    description: t('[integer] Number of period to compare against, ' +
-    'this is relative to the granularity selected'),
-  },
-
-  period_ratio_type: {
-    type: 'SelectControl',
-    label: t('Period Ratio Type'),
-    default: 'growth',
-    choices: formatSelectOptions(['factor', 'growth', 'value']),
-    description: t('`factor` means (new/previous), `growth` is ' +
-    '((new/previous) - 1), `value` is (new-previous)'),
-  },
-
   time_compare: {
     type: 'SelectControl',
     multi: true,
     freeForm: true,
     label: t('Time Shift'),
-    default: [],
     choices: formatSelectOptions([
       '1 day',
       '1 week',
@@ -1585,6 +1567,21 @@ export const controls = {
     '56 weeks, 365 days)'),
   },
 
+  comparison_type: {
+    type: 'SelectControl',
+    label: t('Calculation type'),
+    default: 'values',
+    choices: [
+      ['values', 'Actual Values'],
+      ['absolute', 'Absolute difference'],
+      ['percentage', 'Percentage change'],
+      ['ratio', 'Ratio'],
+    ],
+    description: t('How to display time shifts: as individual lines; as the ' +
+    'absolute difference between the main time series and each time shift; ' +
+    'as the percentage change; or as the ratio between series and time shifts.'),
+  },
+
   subheader: {
     type: 'TextControl',
     label: t('Subheader'),

diff --git a/superset/assets/src/explore/main.css b/superset/assets/src/explore/main.css
@@ -225,3 +225,24 @@
   text-align: center;
   margin-top: 60px;
 }
+
+h1.section-header {
+  font-size: 14px;
+  font-weight: bold;
+  margin-bottom: 0;
+  margin-top: 0;
+  padding-bottom: 5px;
+  margin-left: -16px;
+}
+
+h2.section-header {
+  font-size: 13px;
+  font-weight: bold;
+  margin-bottom: 0;
+  margin-top: 0;
+  padding-bottom: 5px;
+}
+
+.Select {
+  margin-bottom: 15px;
+}
diff --git a/superset/assets/src/explore/store.js b/superset/assets/src/explore/store.js
@@ -1,4 +1,5 @@
 /* eslint camelcase: 0 */
+import isReact from 'is-react';
 import controls from './controls';
 import visTypes, { sectionsToRender } from './visTypes';
 
@@ -50,6 +51,10 @@ export function getControlsState(state, form_data) {
   const controlOverrides = viz.controlOverrides || {};
   const controlsState = {};
   controlNames.forEach((k) => {
+    if (isReact.element(k)) {
+      // no state
+      return;
+    }
     const control = Object.assign({}, controls[k], controlOverrides[k]);
     if (control.mapStateToProps) {
       Object.assign(control, control.mapStateToProps(state, control));

diff --git a/superset/assets/src/explore/visTypes.js → superset/assets/src/explore/visTypes.jsx b/superset/assets/src/explore/visTypes.js → superset/assets/src/explore/visTypes.jsx
@@ -2,6 +2,7 @@
  * This file defines how controls (defined in controls.js) are structured into sections
  * and associated with each and every visualization type.
  */
+import React from 'react';
 import { D3_TIME_FORMAT_OPTIONS } from './controls';
 import * as v from './validators';
 import { t } from '../locales';
@@ -65,9 +66,12 @@ export const sections = {
       'that allow for advanced analytical post processing ' +
       'of query results'),
       controlSetRows: [
+        [<h1 className="section-header">Moving Average</h1>],
         ['rolling_type', 'rolling_periods', 'min_periods'],
-        ['time_compare'],
-        ['num_period_compare', 'period_ratio_type'],
+        [<h1 className="section-header">Time Comparison</h1>],
+        ['time_compare', 'comparison_type'],
+        [<h1 className="section-header">Python Functions</h1>],
+        [<h2 className="section-header">pandas.resample</h2>],
         ['resample_how', 'resample_rule', 'resample_fillmethod'],
       ],
     },

diff --git a/superset/assets/src/visualizations/nvd3_vis.js b/superset/assets/src/visualizations/nvd3_vis.js
@@ -402,8 +402,8 @@ export default function nvd3Vis(slice, payload) {
 
     const yAxisFormatter = d3FormatPreset(fd.y_axis_format);
     if (chart.yAxis && chart.yAxis.tickFormat) {
-      if (fd.num_period_compare || fd.contribution) {
-        // When computing a "Period Ratio" or "Contribution" selected, we force a percentage format
+      if (fd.contribution || fd.comparison_type === 'percentage') {
+        // When computing a "Percentage" or "Contribution" selected, we force a percentage format
         const percentageFormat = d3.format('.1%');
         chart.yAxis.tickFormat(percentageFormat);
       } else {

diff --git a/superset/assets/yarn.lock b/superset/assets/yarn.lock
@@ -3598,7 +3598,7 @@ fault@^1.0.2:
   dependencies:
     format "^0.2.2"
 
-fbjs@^0.8.1, fbjs@^0.8.4, fbjs@^0.8.9:
+fbjs@^0.8.1, fbjs@^0.8.16, fbjs@^0.8.4, fbjs@^0.8.9:
   version "0.8.17"
   resolved "https://registry.yarnpkg.com/fbjs/-/fbjs-0.8.17.tgz#c4d598ead6949112653d6588b01a5cdcd9f90fdd"
   dependencies:
@@ -4945,6 +4945,12 @@ is-property@^1.0.0:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/is-property/-/is-property-1.0.2.tgz#57fe1c4e48474edd65b09911f26b1cd4095dda84"
 
+is-react@^1.1.1:
+  version "1.1.1"
+  resolved "https://registry.yarnpkg.com/is-react/-/is-react-1.1.1.tgz#304d5541e191190f2389bd588a33da0756bfa0c7"
+  dependencies:
+    react "^16.0.0"
+
 is-redirect@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/is-redirect/-/is-redirect-1.0.0.tgz#1d03dded53bd8db0f30c26e4f95d36fc7c87dc24"
@@ -8042,6 +8048,15 @@ react@^15.6.2:
     object-assign "^4.1.0"
     prop-types "^15.5.10"
 
+react@^16.0.0:
+  version "16.4.1"
+  resolved "https://registry.yarnpkg.com/react/-/react-16.4.1.tgz#de51ba5764b5dbcd1f9079037b862bd26b82fe32"
+  dependencies:
+    fbjs "^0.8.16"
+    loose-envify "^1.1.0"
+    object-assign "^4.1.1"
+    prop-types "^15.6.0"
+
 reactable@1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/reactable/-/reactable-1.0.2.tgz#67a579fee3af68b991b5f04df921a4a40ece0b72"