[Table view] Handle empty arrays in fd.timeseries_limit_metric (#5715)

* Handle empty arrays

* Remove test code

* Adding unit test

* Fix unit tests

* Remove logging

superset/assets/package.json

@@ -155,7 +155,7 @@
     "less": "^2.6.1",
     "less-loader": "^4.1.0",
     "mini-css-extract-plugin": "^0.4.0",
-    "mocha": "^3.2.0",
+    "mocha": "^3.5.3",
     "npm-check-updates": "^2.14.0",
     "po2json": "^0.4.5",
     "prettier": "^1.12.1",

superset/assets/spec/.eslintrc

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "import/no-extraneous-dependencies": ["error", {"devDependencies": true}]
+  }
+}

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

@@ -0,0 +1,105 @@
+import { describe, it } from 'mocha';
+import { expect } from 'chai';
+import $ from 'jquery';
+
+import '../../helpers/browser';
+import { d3format } from '../../../src/modules/utils';
+
+import tableVis from '../../../src/visualizations/table';
+
+describe('table viz', () => {
+  const div = '<div id="slice-container"><div class="dataTables_wrapper"></div></div>';
+  const baseSlice = {
+    selector: '#slice-container',
+    formData: {
+      metrics: ['count'],
+      timeseries_limit_metric: null,
+    },
+    datasource: {
+      verbose_map: {},
+    },
+    getFilters: () => {},
+    d3format,
+    removeFilter: null,
+    addFilter: null,
+    height: () => 0,
+  };
+  const basePayload = {
+    data: {
+      records: [
+        { gender: 'boy', count: 39245 },
+        { gender: 'girl', count: 36446 },
+      ],
+      columns: ['gender', 'count'],
+    },
+  };
+
+  it('renders into a container', () => {
+    $('body').html(div);
+    const container = $(baseSlice.selector);
+    expect(container.length).to.equal(1);
+  });
+
+  it('renders header and body datatables in container', () => {
+    $('body').html(div);
+    const container = $(baseSlice.selector);
+
+    expect(container.find('.dataTable').length).to.equal(0);
+    tableVis(baseSlice, basePayload);
+    expect(container.find('.dataTable').length).to.equal(2);
+
+    const tableHeader = container.find('.dataTable')[0];
+    expect($(tableHeader).find('thead tr').length).to.equal(1);
+    expect($(tableHeader).find('th').length).to.equal(2);
+
+    const tableBody = container.find('.dataTable')[1];
+    expect($(tableBody).find('tbody tr').length).to.equal(2);
+    expect($(tableBody).find('th').length).to.equal(2);
+  });
+
+  it('hides the sort by column', () => {
+    $('body').html(div);
+    const slice = { ...baseSlice };
+    slice.formData = { ...baseSlice.formData,
+      timeseries_limit_metric: {
+        label: 'SUM(sum_boys)',
+      },
+    };
+    const payload = {
+      data: {
+        records: [
+          { gender: 'boy', count: 39245, 'SUM(sum_boys)': 48133355 },
+          { gender: 'girl', count: 36446, 'SUM(sum_boys)': 0 },
+        ],
+        columns: ['gender', 'count', 'SUM(sum_boys)'],
+      },
+    };
+    tableVis(slice, payload);
+
+    const container = $(slice.selector);
+    const tableHeader = container.find('.dataTable')[0];
+    expect($(tableHeader).find('th').length).to.equal(2);
+  });
+
+  it('works with empty list for sort by', () => {
+    $('body').html(div);
+    const slice = { ...baseSlice };
+    slice.formData = { ...baseSlice.formData,
+      timeseries_limit_metric: [],
+    };
+    const payload = {
+      data: {
+        records: [
+          { gender: 'boy', count: 39245, 'SUM(sum_boys)': 48133355 },
+          { gender: 'girl', count: 36446, 'SUM(sum_boys)': 0 },
+        ],
+        columns: ['gender', 'count', 'SUM(sum_boys)'],
+      },
+    };
+    tableVis(slice, payload);
+
+    const container = $(slice.selector);
+    const tableBody = container.find('.dataTable')[1];
+    expect($(tableBody).find('th').length).to.equal(3);
+  });
+});

superset/assets/src/visualizations/table.js

@@ -185,9 +185,12 @@ function tableVis(slice, payload) {
       container.find('.dataTables_wrapper'), height);
   // Sorting table by main column
   let sortBy;
-  if (fd.timeseries_limit_metric) {
+  const limitMetric = Array.isArray(fd.timeseries_limit_metric)
+    ? fd.timeseries_limit_metric[0]
+    : fd.timeseries_limit_metric;
+  if (limitMetric) {
     // Sort by as specified
-    sortBy = fd.timeseries_limit_metric.label || fd.timeseries_limit_metric;
+    sortBy = limitMetric.label || limitMetric;
   } else if (metrics.length > 0) {
     // If not specified, use the first metric from the list
     sortBy = metrics[0];

superset/assets/yarn.lock

@@ -6890,7 +6890,7 @@ mkdirp@0.5.1, mkdirp@0.5.x, "mkdirp@>=0.5 0", mkdirp@^0.5, mkdirp@^0.5.0, mkdirp
   dependencies:
     minimist "0.0.8"
 
-mocha@^3.2.0:
+mocha@^3.5.3:
   version "3.5.3"
   resolved "https://registry.yarnpkg.com/mocha/-/mocha-3.5.3.tgz#1e0480fe36d2da5858d1eb6acc38418b26eaa20d"
   dependencies: