fix(legacy-plugin-chart-pivot-table): pivot table chart string aggregation empty values

plugins/legacy-plugin-chart-pivot-table/src/PivotTable.js

@@ -20,11 +20,11 @@
 import dt from 'datatables.net-bs';
 import PropTypes from 'prop-types';
 import {
-  formatNumber,
   getTimeFormatter,
   getTimeFormatterForGranularity,
   smartDateFormatter,
 } from '@superset-ui/core';
+import { formatCellValue, formatDateCellValue } from './utils/formatCells';
 import fixTableHeight from './utils/fixTableHeight';
 import 'datatables.net-bs/css/dataTables.bootstrap.css';
 
@@ -77,49 +77,44 @@ function PivotTable(element, props) {
   container.innerHTML = html;
 
   const cols = Array.isArray(columns[0]) ? columns.map(col => col[0]) : columns;
-  // regex to parse dates
   const dateRegex = /^__timestamp:(-?\d*\.?\d*)$/;
 
-  // jQuery hack to set verbose names in headers
-  // eslint-disable-next-line func-name-matching
-  const replaceCell = function replace() {
-    const s = $(this)[0].textContent;
-    const regexMatch = dateRegex.exec(s);
-    let cellValue;
-    if (regexMatch) {
-      const date = new Date(parseFloat(regexMatch[1]));
-      cellValue = dateFormatter(date);
-    } else {
-      cellValue = verboseMap[s] || s;
-    }
+  $container.find('thead tr th').each(function () {
+    const cellValue = formatDateCellValue(
+      $(this)[0].textContent,
+      verboseMap,
+      dateRegex,
+      dateFormatter,
+    );
     $(this)[0].textContent = cellValue;
-  };
-  $container.find('thead tr th').each(replaceCell);
-  $container.find('tbody tr th').each(replaceCell);
+  });
+
+  $container.find('tbody tr th').each(function () {
+    const cellValue = formatDateCellValue(
+      $(this)[0].textContent,
+      verboseMap,
+      dateRegex,
+      dateFormatter,
+    );
+    $(this)[0].textContent = cellValue;
+  });
 
-  // jQuery hack to format number
   $container.find('tbody tr').each(function eachRow() {
     $(this)
       .find('td')
-      .each(function each(i) {
-        const metric = cols[i];
-        const format = columnFormats[metric] || numberFormat || '.3s';
+      .each(function eachTd(index) {
         const tdText = $(this)[0].textContent;
-        const parsedValue = parseFloat(tdText);
-        if (Number.isNaN(parsedValue)) {
-          const regexMatch = dateRegex.exec(tdText);
-          if (regexMatch) {
-            const date = new Date(parseFloat(regexMatch[1]));
-            $(this)[0].textContent = dateFormatter(date);
-            $(this).attr('data-sort', date);
-          } else {
-            $(this)[0].textContent = '';
-            $(this).attr('data-sort', Number.NEGATIVE_INFINITY);
-          }
-        } else {
-          $(this)[0].textContent = formatNumber(format, parsedValue);
-          $(this).attr('data-sort', parsedValue);
-        }
+        const { textContent, attr } = formatCellValue(
+          index,
+          cols,
+          tdText,
+          columnFormats,
+          numberFormat,
+          dateRegex,
+          dateFormatter,
+        );
+        $(this)[0].textContent = textContent;
+        $(this).attr = attr;
       });
   });
 

plugins/legacy-plugin-chart-pivot-table/src/utils/formatCells.ts

@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { formatNumber } from '@superset-ui/core';
+
+function formatCellValue(
+  i: number,
+  cols: string[],
+  tdText: string,
+  columnFormats: any,
+  numberFormat: string,
+  dateRegex: RegExp,
+  dateFormatter: any,
+) {
+  const metric: string = cols[i];
+  const format: string = columnFormats[metric] || numberFormat || '.3s';
+  let textContent: string = tdText;
+  let sortAttributeValue: any = tdText;
+
+  if (parseFloat(tdText)) {
+    const parsedValue = parseFloat(tdText);
+    textContent = formatNumber(format, parsedValue);
+    sortAttributeValue = parsedValue;
+  } else {
+    const regexMatch = dateRegex.exec(tdText);
+    if (regexMatch) {
+      const date = new Date(parseFloat(regexMatch[1]));
+      textContent = dateFormatter(date);
+      sortAttributeValue = date;
+    } else if (tdText === 'null') {
+      textContent = '';
+      sortAttributeValue = Number.NEGATIVE_INFINITY;
+    }
+  }
+  // @ts-ignore
+  const attr = ('data-sort', sortAttributeValue);
+
+  return { textContent, attr };
+}
+
+function formatDateCellValue(text: string, verboseMap: any, dateRegex: RegExp, dateFormatter: any) {
+  const regexMatch = dateRegex.exec(text);
+  let cellValue;
+  if (regexMatch) {
+    const date = new Date(parseFloat(regexMatch[1]));
+    cellValue = dateFormatter(date);
+  } else {
+    cellValue = verboseMap[text] || text;
+  }
+  return cellValue;
+}
+
+export { formatCellValue, formatDateCellValue };

plugins/legacy-plugin-chart-pivot-table/test/PivotTable.test.ts

@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { getTimeFormatterForGranularity } from '@superset-ui/core';
+import { formatCellValue } from '../src/utils/formatCells';
+
+describe('pivot table plugin format cells', () => {
+  const i = 0;
+  const cols = ['SUM'];
+  let tdText = '2222222';
+  const columnFormats = {};
+  const numberFormat = 'SMART_NUMBER';
+  const dateRegex = /^__timestamp:(-?\d*\.?\d*)$/;
+  const dateFormatter = getTimeFormatterForGranularity('P1D');
+
+  it('render number', () => {
+    const { textContent, attr } = formatCellValue(
+      i,
+      cols,
+      tdText,
+      columnFormats,
+      numberFormat,
+      dateRegex,
+      dateFormatter,
+    );
+    expect(textContent).toEqual('2.22M');
+    expect(attr).toEqual(('data-sort', 2222222));
+  });
+
+  it('render date', () => {
+    tdText = '__timestamp:-126230400000.0';
+
+    const { textContent } = formatCellValue(
+      i,
+      cols,
+      tdText,
+      columnFormats,
+      numberFormat,
+      dateRegex,
+      dateFormatter,
+    );
+    expect(textContent).toEqual('1966-01-01');
+  });
+
+  it('render string', () => {
+    tdText = 'some-text';
+
+    const { textContent, attr } = formatCellValue(
+      i,
+      cols,
+      tdText,
+      columnFormats,
+      numberFormat,
+      dateRegex,
+      dateFormatter,
+    );
+    expect(textContent).toEqual(tdText);
+    expect(attr).toEqual(('data-sort', tdText));
+  });
+
+  it('render null', () => {
+    tdText = 'null';
+
+    const { textContent, attr } = formatCellValue(
+      i,
+      cols,
+      tdText,
+      columnFormats,
+      numberFormat,
+      dateRegex,
+      dateFormatter,
+    );
+    expect(textContent).toEqual('');
+    expect(attr).toEqual(('data-sort', Number.NEGATIVE_INFINITY));
+  });
+});