From 466547e974d9b7fee0fa928f92b009d962c639ce Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Thu, 1 Sep 2022 19:27:41 +0200 Subject: [PATCH] fix(plugin-chart-echarts): show zero value in tooltip (#21296) Co-authored-by: Ville Brofeldt (cherry picked from commit 1aeb8fd6b78d5b53501d277f54b46a02f7067163) --- .../src/utils/forecast.ts | 5 +- .../test/utils/forecast.test.ts | 223 +++++++++++------- 2 files changed, 137 insertions(+), 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 index 94e4630bf455e..485e9fb8968a6 100644 --- 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 index 819b2b85b137e..f3d6f60267207 100644 --- 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: '', - seriesId: 'abc', - value: [new Date(0), 10], - }, - { - marker: '', - seriesId: 'abc__yhat', - value: [new Date(0), 1], - }, - { - marker: '', - seriesId: 'abc__yhat_lower', - value: [new Date(0), 5], - }, - { - marker: '', - seriesId: 'abc__yhat_upper', - value: [new Date(0), 6], - }, - { - marker: '', - seriesId: 'qwerty', - value: [new Date(0), 2], - }, - ]), - ).toEqual({ - abc: { +test('extractForecastValuesFromTooltipParams should extract the proper data from tooltip params', () => { + expect( + extractForecastValuesFromTooltipParams([ + { marker: '', - observation: 10, - forecastTrend: 1, - forecastLower: 5, - forecastUpper: 6, + seriesId: 'abc', + value: [new Date(0), 10], }, - qwerty: { + { marker: '', - 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: '', - observation: 10.1, - formatter, - }), - ).toEqual('abc: 10'); - expect( - formatForecastTooltipSeries({ - seriesName: 'qwerty', + seriesId: 'abc__yhat_lower', + value: [new Date(0), 5], + }, + { marker: '', - observation: 10.1, - forecastTrend: 20.1, - forecastLower: 5.1, - forecastUpper: 7.1, - formatter, - }), - ).toEqual('qwerty: 10, ŷ = 20 (5, 12)'); - expect( - formatForecastTooltipSeries({ - seriesName: 'qwerty', + seriesId: 'abc__yhat_upper', + value: [new Date(0), 6], + }, + { marker: '', - forecastTrend: 20, - forecastLower: 5, - forecastUpper: 7, - formatter, - }), - ).toEqual('qwerty: ŷ = 20 (5, 12)'); - expect( - formatForecastTooltipSeries({ - seriesName: 'qwerty', + seriesId: 'qwerty', + value: [new Date(0), 2], + }, + ]), + ).toEqual({ + abc: { + marker: '', + observation: 10, + forecastTrend: 1, + forecastLower: 5, + forecastUpper: 6, + }, + qwerty: { + marker: '', + observation: 2, + }, + }); +}); + +test('extractForecastValuesFromTooltipParams should extract valid values', () => { + expect( + extractForecastValuesFromTooltipParams([ + { marker: '', - observation: 10.1, - forecastLower: 6, - forecastUpper: 7, - formatter, - }), - ).toEqual('qwerty: 10 (6, 13)'); - expect( - formatForecastTooltipSeries({ - seriesName: 'qwerty', + seriesId: 'foo', + value: [0, 10], + }, + { marker: '', - forecastLower: 7, - forecastUpper: 8, - formatter, - }), - ).toEqual('qwerty: (7, 15)'); + seriesId: 'bar', + value: [100, 0], + }, + ]), + ).toEqual({ + foo: { + marker: '', + observation: 10, + }, + bar: { + marker: '', + observation: 0, + }, }); }); + +const formatter = getNumberFormatter(NumberFormats.INTEGER); + +test('formatForecastTooltipSeries should apply format to value', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'abc', + marker: '', + observation: 10.1, + formatter, + }), + ).toEqual('abc: 10'); +}); + +test('formatForecastTooltipSeries should show falsy value', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'abc', + marker: '', + observation: 0, + formatter, + }), + ).toEqual('abc: 0'); +}); + +test('formatForecastTooltipSeries should format full forecast', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'qwerty', + marker: '', + observation: 10.1, + forecastTrend: 20.1, + forecastLower: 5.1, + forecastUpper: 7.1, + formatter, + }), + ).toEqual('qwerty: 10, ŷ = 20 (5, 12)'); +}); + +test('formatForecastTooltipSeries should format forecast without observation', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'qwerty', + marker: '', + forecastTrend: 20, + forecastLower: 5, + forecastUpper: 7, + formatter, + }), + ).toEqual('qwerty: ŷ = 20 (5, 12)'); +}); + +test('formatForecastTooltipSeries should format forecast without point estimate', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'qwerty', + marker: '', + observation: 10.1, + forecastLower: 6, + forecastUpper: 7, + formatter, + }), + ).toEqual('qwerty: 10 (6, 13)'); +}); + +test('formatForecastTooltipSeries should format forecast with only confidence band', () => { + expect( + formatForecastTooltipSeries({ + seriesName: 'qwerty', + marker: '', + forecastLower: 7, + forecastUpper: 8, + formatter, + }), + ).toEqual('qwerty: (7, 15)'); +});