fix(plugin-chart-echarts): show zero value in tooltip (apache#21296)

Co-authored-by: Ville Brofeldt <ville.brofeldt@apple.com>
Fahrenheit35 · Sep 1, 2022 · 1aeb8fd · 1aeb8fd
1 parent c3a00d4
commit 1aeb8fd
Show file tree

Hide file tree

Showing 2 changed files with 137 additions and 91 deletions.
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
@@ -16,6 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { isNumber } from 'lodash';
 import { DataRecord, DTTM_ALIAS, NumberFormatter } from '@superset-ui/core';
 import { OptionName } from 'echarts/types/src/util/types';
 import { TooltipMarker } from 'echarts/types/src/util/format';
@@ -60,7 +61,7 @@ export const extractForecastValuesFromTooltipParams = (
     const { marker, seriesId, value } = param;
     const context = extractForecastSeriesContext(seriesId);
     const numericValue = isHorizontal ? value[0] : value[1];
-    if (numericValue) {
+    if (isNumber(numericValue)) {
       if (!(context.name in values))
         values[context.name] = {
           marker: marker || '',
@@ -94,7 +95,7 @@ export const formatForecastTooltipSeries = ({
 }): string => {
   let row = `${marker}${sanitizeHtml(seriesName)}: `;
   let isObservation = false;
-  if (observation) {
+  if (isNumber(observation)) {
     isObservation = true;
     row += `${formatter(observation)}`;
   }

diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
@@ -154,103 +154,148 @@ describe('rebaseForecastDatum', () => {
   });
 });
 
-describe('extractForecastValuesFromTooltipParams', () => {
-  it('should extract the proper data from tooltip params', () => {
-    expect(
-      extractForecastValuesFromTooltipParams([
-        {
-          marker: '<img>',
-          seriesId: 'abc',
-          value: [new Date(0), 10],
-        },
-        {
-          marker: '<img>',
-          seriesId: 'abc__yhat',
-          value: [new Date(0), 1],
-        },
-        {
-          marker: '<img>',
-          seriesId: 'abc__yhat_lower',
-          value: [new Date(0), 5],
-        },
-        {
-          marker: '<img>',
-          seriesId: 'abc__yhat_upper',
-          value: [new Date(0), 6],
-        },
-        {
-          marker: '<img>',
-          seriesId: 'qwerty',
-          value: [new Date(0), 2],
-        },
-      ]),
-    ).toEqual({
-      abc: {
+test('extractForecastValuesFromTooltipParams should extract the proper data from tooltip params', () => {
+  expect(
+    extractForecastValuesFromTooltipParams([
+      {
         marker: '<img>',
-        observation: 10,
-        forecastTrend: 1,
-        forecastLower: 5,
-        forecastUpper: 6,
+        seriesId: 'abc',
+        value: [new Date(0), 10],
       },
-      qwerty: {
+      {
         marker: '<img>',
-        observation: 2,
+        seriesId: 'abc__yhat',
+        value: [new Date(0), 1],
       },
-    });
-  });
-});
-
-const formatter = getNumberFormatter(NumberFormats.INTEGER);
-
-describe('formatForecastTooltipSeries', () => {
-  it('should generate a proper series tooltip', () => {
-    expect(
-      formatForecastTooltipSeries({
-        seriesName: 'abc',
+      {
         marker: '<img>',
-        observation: 10.1,
-        formatter,
-      }),
-    ).toEqual('<img>abc: 10');
-    expect(
-      formatForecastTooltipSeries({
-        seriesName: 'qwerty',
+        seriesId: 'abc__yhat_lower',
+        value: [new Date(0), 5],
+      },
+      {
         marker: '<img>',
-        observation: 10.1,
-        forecastTrend: 20.1,
-        forecastLower: 5.1,
-        forecastUpper: 7.1,
-        formatter,
-      }),
-    ).toEqual('<img>qwerty: 10, ŷ = 20 (5, 12)');
-    expect(
-      formatForecastTooltipSeries({
-        seriesName: 'qwerty',
+        seriesId: 'abc__yhat_upper',
+        value: [new Date(0), 6],
+      },
+      {
         marker: '<img>',
-        forecastTrend: 20,
-        forecastLower: 5,
-        forecastUpper: 7,
-        formatter,
-      }),
-    ).toEqual('<img>qwerty: ŷ = 20 (5, 12)');
-    expect(
-      formatForecastTooltipSeries({
-        seriesName: 'qwerty',
+        seriesId: 'qwerty',
+        value: [new Date(0), 2],
+      },
+    ]),
+  ).toEqual({
+    abc: {
+      marker: '<img>',
+      observation: 10,
+      forecastTrend: 1,
+      forecastLower: 5,
+      forecastUpper: 6,
+    },
+    qwerty: {
+      marker: '<img>',
+      observation: 2,
+    },
+  });
+});
+
+test('extractForecastValuesFromTooltipParams should extract valid values', () => {
+  expect(
+    extractForecastValuesFromTooltipParams([
+      {
         marker: '<img>',
-        observation: 10.1,
-        forecastLower: 6,
-        forecastUpper: 7,
-        formatter,
-      }),
-    ).toEqual('<img>qwerty: 10 (6, 13)');
-    expect(
-      formatForecastTooltipSeries({
-        seriesName: 'qwerty',
+        seriesId: 'foo',
+        value: [0, 10],
+      },
+      {
         marker: '<img>',
-        forecastLower: 7,
-        forecastUpper: 8,
-        formatter,
-      }),
-    ).toEqual('<img>qwerty: (7, 15)');
+        seriesId: 'bar',
+        value: [100, 0],
+      },
+    ]),
+  ).toEqual({
+    foo: {
+      marker: '<img>',
+      observation: 10,
+    },
+    bar: {
+      marker: '<img>',
+      observation: 0,
+    },
   });
 });
+
+const formatter = getNumberFormatter(NumberFormats.INTEGER);
+
+test('formatForecastTooltipSeries should apply format to value', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'abc',
+      marker: '<img>',
+      observation: 10.1,
+      formatter,
+    }),
+  ).toEqual('<img>abc: 10');
+});
+
+test('formatForecastTooltipSeries should show falsy value', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'abc',
+      marker: '<img>',
+      observation: 0,
+      formatter,
+    }),
+  ).toEqual('<img>abc: 0');
+});
+
+test('formatForecastTooltipSeries should format full forecast', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'qwerty',
+      marker: '<img>',
+      observation: 10.1,
+      forecastTrend: 20.1,
+      forecastLower: 5.1,
+      forecastUpper: 7.1,
+      formatter,
+    }),
+  ).toEqual('<img>qwerty: 10, ŷ = 20 (5, 12)');
+});
+
+test('formatForecastTooltipSeries should format forecast without observation', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'qwerty',
+      marker: '<img>',
+      forecastTrend: 20,
+      forecastLower: 5,
+      forecastUpper: 7,
+      formatter,
+    }),
+  ).toEqual('<img>qwerty: ŷ = 20 (5, 12)');
+});
+
+test('formatForecastTooltipSeries should format forecast without point estimate', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'qwerty',
+      marker: '<img>',
+      observation: 10.1,
+      forecastLower: 6,
+      forecastUpper: 7,
+      formatter,
+    }),
+  ).toEqual('<img>qwerty: 10 (6, 13)');
+});
+
+test('formatForecastTooltipSeries should format forecast with only confidence band', () => {
+  expect(
+    formatForecastTooltipSeries({
+      seriesName: 'qwerty',
+      marker: '<img>',
+      forecastLower: 7,
+      forecastUpper: 8,
+      formatter,
+    }),
+  ).toEqual('<img>qwerty: (7, 15)');
+});