[line] fix verbose names in time shift (#4765)

* [line] fix verbose names in time shift

* Addressing comments

superset/assets/spec/javascripts/visualizations/nvd3_viz_spec.jsx

@@ -0,0 +1,28 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+
+import { formatLabel } from '../../../visualizations/nvd3_vis';
+
+describe('nvd3 viz', () => {
+  const verboseMap = {
+    foo: 'Foo',
+    bar: 'Bar',
+  };
+  describe('formatLabel', () => {
+    it('formats simple labels', () => {
+      expect(formatLabel('foo')).to.equal('foo');
+      expect(formatLabel(['foo'])).to.equal('foo');
+      expect(formatLabel(['foo', 'bar'])).to.equal('foo, bar');
+    });
+    it('formats simple labels with lookups', () => {
+      expect(formatLabel('foo', verboseMap)).to.equal('Foo');
+      expect(formatLabel('baz', verboseMap)).to.equal('baz');
+      expect(formatLabel(['foo'], verboseMap)).to.equal('Foo');
+      expect(formatLabel(['foo', 'bar', 'baz'], verboseMap)).to.equal('Foo, Bar, baz');
+    });
+    it('deals with --- properly', () => {
+      expect(formatLabel(['foo', '---'], verboseMap)).to.equal('Foo ---');
+      expect(formatLabel(['foo', 'bar', 'baz', '---'], verboseMap)).to.equal('Foo, Bar, baz ---');
+    });
+  });
+});

superset/assets/visualizations/main.js

@@ -1,4 +1,5 @@
 /* eslint-disable global-require */
+import nvd3Vis from './nvd3_vis';
 
 // You ***should*** use these to reference viz_types in code
 export const VIZ_TYPES = {
@@ -52,29 +53,29 @@ export const VIZ_TYPES = {
 };
 
 const vizMap = {
-  [VIZ_TYPES.area]: require('./nvd3_vis.js'),
-  [VIZ_TYPES.bar]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.area]: nvd3Vis,
+  [VIZ_TYPES.bar]: nvd3Vis,
   [VIZ_TYPES.big_number]: require('./big_number.js'),
   [VIZ_TYPES.big_number_total]: require('./big_number.js'),
-  [VIZ_TYPES.box_plot]: require('./nvd3_vis.js'),
-  [VIZ_TYPES.bubble]: require('./nvd3_vis.js'),
-  [VIZ_TYPES.bullet]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.box_plot]: nvd3Vis,
+  [VIZ_TYPES.bubble]: nvd3Vis,
+  [VIZ_TYPES.bullet]: nvd3Vis,
   [VIZ_TYPES.cal_heatmap]: require('./cal_heatmap.js'),
-  [VIZ_TYPES.compare]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.compare]: nvd3Vis,
   [VIZ_TYPES.directed_force]: require('./directed_force.js'),
   [VIZ_TYPES.chord]: require('./chord.jsx'),
-  [VIZ_TYPES.dist_bar]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.dist_bar]: nvd3Vis,
   [VIZ_TYPES.filter_box]: require('./filter_box.jsx'),
   [VIZ_TYPES.heatmap]: require('./heatmap.js'),
   [VIZ_TYPES.histogram]: require('./histogram.js'),
   [VIZ_TYPES.horizon]: require('./horizon.js'),
   [VIZ_TYPES.iframe]: require('./iframe.js'),
-  [VIZ_TYPES.line]: require('./nvd3_vis.js'),
-  [VIZ_TYPES.time_pivot]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.line]: nvd3Vis,
+  [VIZ_TYPES.time_pivot]: nvd3Vis,
   [VIZ_TYPES.mapbox]: require('./mapbox.jsx'),
   [VIZ_TYPES.markup]: require('./markup.js'),
   [VIZ_TYPES.para]: require('./parallel_coordinates.js'),
-  [VIZ_TYPES.pie]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.pie]: nvd3Vis,
   [VIZ_TYPES.pivot_table]: require('./pivot_table.js'),
   [VIZ_TYPES.sankey]: require('./sankey.js'),
   [VIZ_TYPES.separator]: require('./markup.js'),
@@ -85,7 +86,7 @@ const vizMap = {
   [VIZ_TYPES.country_map]: require('./country_map.js'),
   [VIZ_TYPES.word_cloud]: require('./word_cloud.js'),
   [VIZ_TYPES.world_map]: require('./world_map.js'),
-  [VIZ_TYPES.dual_line]: require('./nvd3_vis.js'),
+  [VIZ_TYPES.dual_line]: nvd3Vis,
   [VIZ_TYPES.event_flow]: require('./EventFlow.jsx'),
   [VIZ_TYPES.paired_ttest]: require('./paired_ttest.jsx'),
   [VIZ_TYPES.partition]: require('./partition.js'),

superset/assets/visualizations/nvd3_vis.js

@@ -83,23 +83,24 @@ function getMaxLabelSize(container, axisClass) {
   return Math.max(...labelDimensions);
 }
 
-/* eslint-disable camelcase */
-function formatLabel(column, verbose_map) {
+export function formatLabel(input, verboseMap = {}) {
+  // The input for label may be a string or an array of string
+  // When using the time shift feature, the label contains a '---' in the array
+  const verboseLkp = s => verboseMap[s] || s;
   let label;
-  if (Array.isArray(column) && column.length) {
-    label = verbose_map[column[0]] || column[0];
-    if (column.length > 1) {
-      label += ', ';
+  if (Array.isArray(input) && input.length) {
+    const verboseLabels = input.filter(s => s !== '---').map(verboseLkp);
+    label = verboseLabels.join(', ');
+    if (input.length > verboseLabels.length) {
+      label += ' ---';
     }
-    label += column.slice(1).join(', ');
   } else {
-    label = verbose_map[column] || column;
+    label = verboseLkp(input);
   }
   return label;
 }
-/* eslint-enable camelcase */
 
-function nvd3Vis(slice, payload) {
+export default function nvd3Vis(slice, payload) {
   let chart;
   let colorKey = 'key';
   const isExplore = $('#explore-container').length === 1;
@@ -778,5 +779,3 @@ function nvd3Vis(slice, payload) {
 
   nv.addGraph(drawGraph);
 }
-
-module.exports = nvd3Vis;

superset/viz.py

@@ -1061,10 +1061,11 @@ def to_series(self, df, classed='', title_suffix=''):
                     len(self.metrics) == 1):
                 # Removing metric from series name if only one metric
                 series_title = series_title[1:]
-            if isinstance(series_title, string_types):
-                series_title += title_suffix
-            elif title_suffix and isinstance(series_title, (list, tuple)):
-                series_title = text_type(series_title[-1]) + title_suffix
+            if title_suffix:
+                if isinstance(series_title, string_types):
+                    series_title = (series_title, title_suffix)
+                elif isinstance(series_title, (list, tuple)):
+                    series_title = series_title + (title_suffix,)
 
             values = []
             for ds in df.index: