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 <maximebeauche...@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
maximebeauche...@apache.org.
