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

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 : [],

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;
+}

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');
+    });
+  });
 });