superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: Use 'count' as the default metric when available (#4606)
Date Tue, 20 Mar 2018 04:51:54 GMT
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new ed9867c  Use 'count' as the default metric when available (#4606)
ed9867c is described below

commit ed9867c0cc210de81950053fd4827a9a4d25f7e2
Author: Maxime Beauchemin <maximebeauchemin@gmail.com>
AuthorDate: Mon Mar 19 21:51:51 2018 -0700

    Use 'count' as the default metric when available (#4606)
    
    * Use 'count' as the default metric when available
    
    Count is a much better default than the current behavior which is to use
    whatever the first metric in the list happens to be.
    
    * Addressing nits
---
 .../assets/javascripts/explore/stores/controls.jsx | 14 ++++++++---
 superset/assets/javascripts/modules/utils.js       | 16 +++++++++++++
 .../assets/spec/javascripts/modules/utils_spec.jsx | 28 ++++++++++++++++++++++
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx
index cc35d0e..617c717 100644
--- a/superset/assets/javascripts/explore/stores/controls.jsx
+++ b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -1,5 +1,9 @@
 import React from 'react';
-import { formatSelectOptionsForRange, formatSelectOptions } from '../../modules/utils';
+import {
+  formatSelectOptionsForRange,
+  formatSelectOptions,
+  mainMetric,
+} from '../../modules/utils';
 import * as v from '../validators';
 import { colorPrimary, ALL_COLOR_SCHEMES, spectrums } from '../../modules/colors';
 import { defaultViewport } from '../../modules/geo';
@@ -82,6 +86,7 @@ const jsFunctionInfo = (
     </a>.
   </div>
 );
+
 function jsFunctionControl(label, description, extraDescr = null, height = 100, defaultText
= '') {
   return {
     type: 'TextAreaControl',
@@ -131,7 +136,10 @@ export const controls = {
     valueKey: 'metric_name',
     optionRenderer: m => <MetricOption metric={m} showType />,
     valueRenderer: m => <MetricOption metric={m} />,
-    default: c => c.options && c.options.length > 0 ? [c.options[0].metric_name]
: null,
+    default: (c) => {
+      const metric = mainMetric(c.options);
+      return metric ? [metric] : null;
+    },
     mapStateToProps: state => ({
       options: (state.datasource) ? state.datasource.metrics : [],
     }),
@@ -218,7 +226,7 @@ export const controls = {
     validators: [v.nonEmpty],
     optionRenderer: m => <MetricOption metric={m} showType />,
     valueRenderer: m => <MetricOption metric={m} />,
-    default: c => c.options && c.options.length > 0 ? c.options[0].metric_name
: null,
+    default: c => mainMetric(c.options),
     valueKey: 'metric_name',
     mapStateToProps: state => ({
       options: (state.datasource) ? state.datasource.metrics : [],
diff --git a/superset/assets/javascripts/modules/utils.js b/superset/assets/javascripts/modules/utils.js
index b5590f0..6180783 100644
--- a/superset/assets/javascripts/modules/utils.js
+++ b/superset/assets/javascripts/modules/utils.js
@@ -259,3 +259,19 @@ export function getParam(name) {
   const results = regex.exec(location.search);
   return results === null ? '' : decodeURIComponent(results[1].replace(/\+/g, ' '));
 }
+
+export function mainMetric(metricOptions) {
+  // Using 'count' as default metric if it exists, otherwise using whatever one shows up
first
+  let metric;
+  if (metricOptions && metricOptions.length > 0) {
+    metricOptions.forEach((m) => {
+      if (m.metric_name === 'count') {
+        metric = 'count';
+      }
+    });
+    if (!metric) {
+      metric = metricOptions[0].metric_name;
+    }
+  }
+  return metric;
+}
diff --git a/superset/assets/spec/javascripts/modules/utils_spec.jsx b/superset/assets/spec/javascripts/modules/utils_spec.jsx
index 174e0e1..309d62f 100644
--- a/superset/assets/spec/javascripts/modules/utils_spec.jsx
+++ b/superset/assets/spec/javascripts/modules/utils_spec.jsx
@@ -3,6 +3,7 @@ import { expect } from 'chai';
 import {
   tryNumify, slugify, formatSelectOptionsForRange, d3format,
   d3FormatPreset, d3TimeFormatPreset, defaultNumberFormatter,
+  mainMetric,
 } from '../../../javascripts/modules/utils';
 
 describe('utils', () => {
@@ -69,4 +70,31 @@ describe('utils', () => {
     expect(defaultNumberFormatter(-111000000)).to.equal('-111M');
     expect(defaultNumberFormatter(-0.23)).to.equal('-230m');
   });
+  describe('mainMetric', () => {
+    it('is null when no options', () => {
+      expect(mainMetric([])).to.equal(undefined);
+      expect(mainMetric(null)).to.equal(undefined);
+    });
+    it('prefers the "count" metric when first', () => {
+      const metrics = [
+        { metric_name: 'count' },
+        { metric_name: 'foo' },
+      ];
+      expect(mainMetric(metrics)).to.equal('count');
+    });
+    it('prefers the "count" metric when not first', () => {
+      const metrics = [
+        { metric_name: 'foo' },
+        { metric_name: 'count' },
+      ];
+      expect(mainMetric(metrics)).to.equal('count');
+    });
+    it('selects the first metric when "count" is not an option', () => {
+      const metrics = [
+        { metric_name: 'foo' },
+        { metric_name: 'not_count' },
+      ];
+      expect(mainMetric(metrics)).to.equal('foo');
+    });
+  });
 });

-- 
To stop receiving notification emails like this one, please contact
maximebeauchemin@apache.org.

Mime
View raw message