From 91bb424ea4414cb715d9c297e4774b7edf104bcd Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 5 Dec 2023 16:02:42 +0100 Subject: [PATCH 01/18] :bug: fix eui warnings --- .../src/buttons/toolbar_button/toolbar_button.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx b/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx index 84039857d0891..1e005d110e6cf 100644 --- a/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx +++ b/packages/shared-ux/button_toolbar/src/buttons/toolbar_button/toolbar_button.tsx @@ -23,7 +23,7 @@ type ButtonRenderStyle = 'standard' | 'iconButton'; interface ToolbarButtonCommonProps extends Pick< EuiButtonPropsForButton, - 'onClick' | 'iconType' | 'size' | 'data-test-subj' | 'isDisabled' + 'onClick' | 'iconType' | 'size' | 'data-test-subj' | 'isDisabled' | 'aria-label' > { /** * Render style of the toolbar button @@ -162,7 +162,7 @@ const ToolbarIconButton = ({ Date: Tue, 5 Dec 2023 16:08:42 +0100 Subject: [PATCH 02/18] :bug: Fix for decimals issue --- .../public/components/heatmap_component.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx index f0794318e6001..c0dbc0493d7b9 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx @@ -515,18 +515,16 @@ export const HeatmapComponent: FC = memo( let overwriteArrayIdx; if (endValue === Number.POSITIVE_INFINITY) { - overwriteArrayIdx = `≥ ${metricFormatter.convert(startValue)}`; + overwriteArrayIdx = `≥ ${valueFormatter(startValue)}`; } else { - overwriteArrayIdx = `${metricFormatter.convert(start)} - ${metricFormatter.convert( - endValue - )}`; + overwriteArrayIdx = `${valueFormatter(start)} - ${valueFormatter(endValue)}`; } const overwriteColor = overwriteColors?.[overwriteArrayIdx]; return { // with the default continuity:above the every range is left-closed - start: startValue, - end: endValue, + start: Number(valueFormatter(startValue)), + end: Number(valueFormatter(endValue)), // the current colors array contains a duplicated color at the beginning that we need to skip color: overwriteColor ?? colors[index + 1], }; From be279fc5afd52ca775ff5c752692a52a5371a986 Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 5 Dec 2023 16:08:54 +0100 Subject: [PATCH 03/18] :white_check_mark: Add test for decimals issue --- .../components/heatmap_component.test.tsx | 132 +++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx index 083e6430614da..f15016f950ab7 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx @@ -28,7 +28,7 @@ import { act } from 'react-dom/test-utils'; import { HeatmapRenderProps, HeatmapArguments } from '../../common'; import HeatmapComponent from './heatmap_component'; import { LegendSize } from '@kbn/visualizations-plugin/common'; -import { FieldFormat } from '@kbn/field-formats-plugin/common'; +import { FieldFormat, FormatFactory } from '@kbn/field-formats-plugin/common'; jest.mock('@elastic/charts', () => { const original = jest.requireActual('@elastic/charts'); @@ -111,6 +111,10 @@ const uiState = { setSilent: jest.fn(), } as any; +function toPrecisionWithoutRounding(v: number, digits: number) { + return Math.trunc(v * Math.pow(10, digits)) / Math.pow(10, digits); +} + describe('HeatmapComponent', function () { let wrapperProps: HeatmapRenderProps; @@ -379,6 +383,132 @@ describe('HeatmapComponent', function () { }); }); + it('should respect the value format with 5 digits when computing ranges', () => { + const newData: Datatable = { + type: 'datatable', + rows: [{ 'col-0-1': 3 }], + columns: [{ id: 'col-0-1', name: 'Count', meta: { type: 'number' } }], + }; + const newProps = { + ...wrapperProps, + formatFactory: ((format: { id: string }) => + format?.id === 'string' + ? ({ convert: (v: string) => String(v) } as unknown as FieldFormat) + : ({ + convert: (v: number) => toPrecisionWithoutRounding(v, 5), + } as unknown as FieldFormat)) as unknown as FormatFactory, + data: newData, + args: { + ...wrapperProps.args, + palette: { + params: { + colors: ['#6092c0', '#a8bfda', '#ebeff5', '#ecb385', '#e7664c'], + stops: [19.98123, 39.88, 60, 80, 100], + range: 'number', + gradient: true, + continuity: 'above', + rangeMin: 0, + rangeMax: null, + }, + }, + }, + } as unknown as HeatmapRenderProps; + const component = mountWithIntl(); + expect(component.find(Heatmap).prop('colorScale')).toEqual({ + bands: [ + { + start: 0, + end: 19.98123, + color: '#6092c0', + }, + { + start: 19.98123, + end: 39.88, + color: '#a8bfda', + }, + { + start: 39.88, + end: 60, + color: '#ebeff5', + }, + { + start: 60, + end: 80, + color: '#ecb385', + }, + { + start: 80, + end: Infinity, + color: '#e7664c', + }, + ], + type: 'bands', + }); + }); + + it('should respect the value format with 0 digits when computing ranges', () => { + const newData: Datatable = { + type: 'datatable', + rows: [{ 'col-0-1': 3 }], + columns: [{ id: 'col-0-1', name: 'Count', meta: { type: 'number' } }], + }; + const newProps = { + ...wrapperProps, + formatFactory: ((format: { id: string }) => + format?.id === 'string' + ? ({ convert: (v: string) => String(v) } as unknown as FieldFormat) + : ({ + convert: (v: number) => toPrecisionWithoutRounding(v, 0), + } as unknown as FieldFormat)) as unknown as FormatFactory, + data: newData, + args: { + ...wrapperProps.args, + palette: { + params: { + colors: ['#6092c0', '#a8bfda', '#ebeff5', '#ecb385', '#e7664c'], + stops: [19.98123, 39.88, 60, 80, 100], + range: 'number', + gradient: true, + continuity: 'above', + rangeMin: 0, + rangeMax: null, + }, + }, + }, + } as unknown as HeatmapRenderProps; + const component = mountWithIntl(); + expect(component.find(Heatmap).prop('colorScale')).toEqual({ + bands: [ + { + start: 0, + end: 19, + color: '#6092c0', + }, + { + start: 19, + end: 39, + color: '#a8bfda', + }, + { + start: 39, + end: 60, + color: '#ebeff5', + }, + { + start: 60, + end: 80, + color: '#ecb385', + }, + { + start: 80, + end: Infinity, + color: '#e7664c', + }, + ], + type: 'bands', + }); + }); + it('renders the axis titles', () => { const component = shallowWithIntl(); expect(component.find(Heatmap).prop('xAxisTitle')).toEqual('Dest'); From 6e2b0ecaa7c61cdd3120dc9b615db6de19ce78ab Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 5 Dec 2023 16:09:23 +0100 Subject: [PATCH 04/18] :bug: add missing bit for the fix --- .../expression_heatmap/public/components/heatmap_component.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx index c0dbc0493d7b9..37df615922d07 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx @@ -101,7 +101,7 @@ function shiftAndNormalizeStops( if (!Number.isFinite(result)) { return 1; } - return Number(result.toFixed(2)); + return result; } ); } From 8763b713f7371d3e05e3b4b02ff38aef8d98c966 Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 5 Dec 2023 16:48:37 +0100 Subject: [PATCH 05/18] :bug: fix open extremities coloring issue --- .../public/components/heatmap_component.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx index 37df615922d07..a9afa011fd519 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx @@ -97,10 +97,6 @@ function shiftAndNormalizeStops( if (params.range === 'percent') { result = min + ((max - min) * value) / 100; } - // a division by zero safeguard - if (!Number.isFinite(result)) { - return 1; - } return result; } ); @@ -523,8 +519,8 @@ export const HeatmapComponent: FC = memo( const overwriteColor = overwriteColors?.[overwriteArrayIdx]; return { // with the default continuity:above the every range is left-closed - start: Number(valueFormatter(startValue)), - end: Number(valueFormatter(endValue)), + start: Number.isFinite(startValue) ? Number(valueFormatter(startValue)) : startValue, + end: Number.isFinite(endValue) ? Number(valueFormatter(endValue)) : endValue, // the current colors array contains a duplicated color at the beginning that we need to skip color: overwriteColor ?? colors[index + 1], }; From 2e569267633146d84be4122dbb483748085ad41c Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 5 Dec 2023 16:51:48 +0100 Subject: [PATCH 06/18] :white_check_mark: Add test --- .../components/heatmap_component.test.tsx | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx index f15016f950ab7..a5e1b691c0fb5 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx @@ -509,6 +509,63 @@ describe('HeatmapComponent', function () { }); }); + it('should keep the minimum open end', () => { + const newData: Datatable = { + type: 'datatable', + rows: [{ 'col-0-1': -3 }], + columns: [{ id: 'col-0-1', name: 'Count', meta: { type: 'number' } }], + }; + const newProps = { + ...wrapperProps, + data: newData, + args: { + ...wrapperProps.args, + palette: { + params: { + colors: ['#6092c0', '#a8bfda', '#ebeff5', '#ecb385', '#e7664c'], + stops: [1, 2, 3, 4, 5], + range: 'number', + gradient: true, + continuity: 'above', + rangeMin: -Infinity, + rangeMax: null, + }, + }, + }, + } as unknown as HeatmapRenderProps; + const component = mountWithIntl(); + expect(component.find(Heatmap).prop('colorScale')).toEqual({ + bands: [ + { + start: -Infinity, + end: 1, + color: '#6092c0', + }, + { + start: 1, + end: 2, + color: '#a8bfda', + }, + { + start: 2, + end: 3, + color: '#ebeff5', + }, + { + start: 3, + end: 4, + color: '#ecb385', + }, + { + start: 4, + end: Infinity, + color: '#e7664c', + }, + ], + type: 'bands', + }); + }); + it('renders the axis titles', () => { const component = shallowWithIntl(); expect(component.find(Heatmap).prop('xAxisTitle')).toEqual('Dest'); From 49a4cd23e930ffd4e7e112e65a5ecb4532c14058 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 10:31:25 +0100 Subject: [PATCH 07/18] :bug: fix issue with higher values formatted --- .../public/components/heatmap_component.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx index a9afa011fd519..8120f10eefd13 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx @@ -26,6 +26,7 @@ import { SeriesIdentifier, TooltipValue, } from '@elastic/charts'; +import numeral from '@elastic/numeral'; import type { CustomPaletteState } from '@kbn/charts-plugin/public'; import { search } from '@kbn/data-plugin/public'; import { LegendToggle, EmptyPlaceholder, useActiveCursor } from '@kbn/charts-plugin/public'; @@ -66,6 +67,11 @@ declare global { } } +function formatAndExtractNumericValue(value: number, valueFormatter: (d: number) => string) { + // @ts-expect-error value method is not declared in the Numeral.js types + return Number.isFinite(value) ? numeral(valueFormatter(value)).value() : value; +} + function getStops( { colors, stops, range }: CustomPaletteState, { min, max }: { min: number; max: number } @@ -519,8 +525,8 @@ export const HeatmapComponent: FC = memo( const overwriteColor = overwriteColors?.[overwriteArrayIdx]; return { // with the default continuity:above the every range is left-closed - start: Number.isFinite(startValue) ? Number(valueFormatter(startValue)) : startValue, - end: Number.isFinite(endValue) ? Number(valueFormatter(endValue)) : endValue, + start: formatAndExtractNumericValue(startValue, valueFormatter), + end: formatAndExtractNumericValue(endValue, valueFormatter), // the current colors array contains a duplicated color at the beginning that we need to skip color: overwriteColor ?? colors[index + 1], }; From c903722abe5989212ea26c3a3085ac29efb5dc9d Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 11:47:18 +0100 Subject: [PATCH 08/18] :white_check_mark: Update with new precision --- .../functional/apps/lens/group4/chart_data.ts | 10 ++--- .../functional/apps/lens/group5/heatmap.ts | 40 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/x-pack/test/functional/apps/lens/group4/chart_data.ts b/x-pack/test/functional/apps/lens/group4/chart_data.ts index 9bcd237306920..379d29040a1ea 100644 --- a/x-pack/test/functional/apps/lens/group4/chart_data.ts +++ b/x-pack/test/functional/apps/lens/group4/chart_data.ts @@ -119,11 +119,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // assert legend expect(debugState.legend!.items).to.eql([ - { color: '#6092c0', key: '5,722.77 - 8,529.22', name: '5,722.77 - 8,529.22' }, - { color: '#a8bfda', key: '8,529.22 - 11,335.66', name: '8,529.22 - 11,335.66' }, - { color: '#ebeff5', key: '11,335.66 - 14,142.11', name: '11,335.66 - 14,142.11' }, - { color: '#ecb385', key: '14,142.11 - 16,948.55', name: '14,142.11 - 16,948.55' }, - { color: '#e7664c', key: '≥ 16,948.55', name: '≥ 16,948.55' }, + { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#6092c0' }, + { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, + { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, + { key: '14,142.11 - 16,948.555', name: '14,142.11 - 16,948.555', color: '#ecb385' }, + { key: '≥ 16,948.555', name: '≥ 16,948.555', color: '#e7664c' }, ]); }); diff --git a/x-pack/test/functional/apps/lens/group5/heatmap.ts b/x-pack/test/functional/apps/lens/group5/heatmap.ts index 3e37ce65cc5bc..9bf63d6a6ab39 100644 --- a/x-pack/test/functional/apps/lens/group5/heatmap.ts +++ b/x-pack/test/functional/apps/lens/group5/heatmap.ts @@ -60,11 +60,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // assert legend expect(debugState.legend!.items).to.eql([ - { key: '5,722.77 - 8,529.22', name: '5,722.77 - 8,529.22', color: '#6092c0' }, - { key: '8,529.22 - 11,335.66', name: '8,529.22 - 11,335.66', color: '#a8bfda' }, - { key: '11,335.66 - 14,142.11', name: '11,335.66 - 14,142.11', color: '#ebeff5' }, - { key: '14,142.11 - 16,948.55', name: '14,142.11 - 16,948.55', color: '#ecb385' }, - { key: '≥ 16,948.55', name: '≥ 16,948.55', color: '#e7664c' }, + { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#6092c0' }, + { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, + { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, + { key: '14,142.11 - 16,948.555', name: '14,142.11 - 16,948.555', color: '#ecb385' }, + { key: '≥ 16,948.555', name: '≥ 16,948.555', color: '#e7664c' }, ]); }); @@ -86,11 +86,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // assert legend has changed expect(debugState.legend!.items).to.eql([ - { key: '7,126 - 8,529.22', name: '7,126 - 8,529.22', color: '#6092c0' }, - { key: '8,529.22 - 11,335.66', name: '8,529.22 - 11,335.66', color: '#a8bfda' }, - { key: '11,335.66 - 14,142.11', name: '11,335.66 - 14,142.11', color: '#ebeff5' }, - { key: '14,142.11 - 16,948.55', name: '14,142.11 - 16,948.55', color: '#ecb385' }, - { key: '≥ 16,948.55', name: '≥ 16,948.55', color: '#e7664c' }, + { key: '7,125.997 - 8,529.22', name: '7,125.997 - 8,529.22', color: '#6092c0' }, + { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, + { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, + { key: '14,142.11 - 16,948.555', name: '14,142.11 - 16,948.555', color: '#ecb385' }, + { key: '≥ 16,948.555', name: '≥ 16,948.555', color: '#e7664c' }, ]); }); @@ -148,11 +148,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // assert legend has changed expect(debugState.legend!.items).to.eql([ - { key: '5,722.77 - 8,529.22', name: '5,722.77 - 8,529.22', color: '#209280' }, - { key: '8,529.22 - 11,335.66', name: '8,529.22 - 11,335.66', color: '#54b399' }, - { key: '11,335.66 - 14,142.11', name: '11,335.66 - 14,142.11', color: '#d6bf57' }, - { key: '14,142.11 - 16,948.55', name: '14,142.11 - 16,948.55', color: '#e7664c' }, - { key: '≥ 16,948.55', name: '≥ 16,948.55', color: '#cc5642' }, + { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#209280' }, + { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#54b399' }, + { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#d6bf57' }, + { key: '14,142.11 - 16,948.555', name: '14,142.11 - 16,948.555', color: '#e7664c' }, + { key: '≥ 16,948.555', name: '≥ 16,948.555', color: '#cc5642' }, ]); }); @@ -167,11 +167,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // assert legend has not changed expect(debugState.legend!.items).to.eql([ - { key: '5,722.77 - 8,529.22', name: '5,722.77 - 8,529.22', color: '#209280' }, - { key: '8,529.22 - 11,335.66', name: '8,529.22 - 11,335.66', color: '#54b399' }, - { key: '11,335.66 - 14,142.11', name: '11,335.66 - 14,142.11', color: '#d6bf57' }, - { key: '14,142.11 - 16,948.55', name: '14,142.11 - 16,948.55', color: '#e7664c' }, - { key: '≥ 16,948.55', name: '≥ 16,948.55', color: '#cc5642' }, + { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#209280' }, + { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#54b399' }, + { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#d6bf57' }, + { key: '14,142.11 - 16,948.555', name: '14,142.11 - 16,948.555', color: '#e7664c' }, + { key: '≥ 16,948.555', name: '≥ 16,948.555', color: '#cc5642' }, ]); }); From c4f20d2cdc6c16790c9dde8e092ae9eb569c6852 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 12:30:49 +0100 Subject: [PATCH 09/18] :white_check_mark: fix values --- .../group2/open_in_lens/agg_based/heatmap.ts | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/x-pack/test_serverless/functional/test_suites/common/visualizations/group2/open_in_lens/agg_based/heatmap.ts b/x-pack/test_serverless/functional/test_suites/common/visualizations/group2/open_in_lens/agg_based/heatmap.ts index f1c362838d77e..6e486d6d34e5d 100644 --- a/x-pack/test_serverless/functional/test_suites/common/visualizations/group2/open_in_lens/agg_based/heatmap.ts +++ b/x-pack/test_serverless/functional/test_suites/common/visualizations/group2/open_in_lens/agg_based/heatmap.ts @@ -102,36 +102,12 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { expect(debugState).to.not.be.eql(null); expect(debugState.legend!.items).to.eql([ - { - color: '#006837', - key: '1,322 - 1,585.67', - name: '1,322 - 1,585.67', - }, - { - color: '#4CB15D', - key: '1,585.67 - 1,849.33', - name: '1,585.67 - 1,849.33', - }, - { - color: '#B7E075', - key: '1,849.33 - 2,113', - name: '1,849.33 - 2,113', - }, - { - color: '#FEFEBD', - key: '2,113 - 2,376.67', - name: '2,113 - 2,376.67', - }, - { - color: '#FDBF6F', - key: '2,376.67 - 2,640.33', - name: '2,376.67 - 2,640.33', - }, - { - color: '#EA5839', - key: '2,640.33 - 2,904', - name: '2,640.33 - 2,904', - }, + { key: '1,322 - 1,585.667', name: '1,322 - 1,585.667', color: '#006837' }, + { key: '1,585.667 - 1,849.333', name: '1,585.667 - 1,849.333', color: '#4CB15D' }, + { key: '1,849.333 - 2,113', name: '1,849.333 - 2,113', color: '#B7E075' }, + { key: '2,113 - 2,376.667', name: '2,113 - 2,376.667', color: '#FEFEBD' }, + { key: '2,376.667 - 2,640.333', name: '2,376.667 - 2,640.333', color: '#FDBF6F' }, + { key: '2,640.333 - 2,904', name: '2,640.333 - 2,904', color: '#EA5839' }, ]); }); From 6acaa4c4e7e9e8eb7f1a492bff56979a149009fc Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 14:34:55 +0100 Subject: [PATCH 10/18] :fire: remove unnecessary format and parsing --- .../public/components/heatmap_component.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx index 8120f10eefd13..e541918801b59 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.tsx @@ -26,7 +26,6 @@ import { SeriesIdentifier, TooltipValue, } from '@elastic/charts'; -import numeral from '@elastic/numeral'; import type { CustomPaletteState } from '@kbn/charts-plugin/public'; import { search } from '@kbn/data-plugin/public'; import { LegendToggle, EmptyPlaceholder, useActiveCursor } from '@kbn/charts-plugin/public'; @@ -67,11 +66,6 @@ declare global { } } -function formatAndExtractNumericValue(value: number, valueFormatter: (d: number) => string) { - // @ts-expect-error value method is not declared in the Numeral.js types - return Number.isFinite(value) ? numeral(valueFormatter(value)).value() : value; -} - function getStops( { colors, stops, range }: CustomPaletteState, { min, max }: { min: number; max: number } @@ -525,8 +519,8 @@ export const HeatmapComponent: FC = memo( const overwriteColor = overwriteColors?.[overwriteArrayIdx]; return { // with the default continuity:above the every range is left-closed - start: formatAndExtractNumericValue(startValue, valueFormatter), - end: formatAndExtractNumericValue(endValue, valueFormatter), + start: startValue, + end: endValue, // the current colors array contains a duplicated color at the beginning that we need to skip color: overwriteColor ?? colors[index + 1], }; From 9120389fcbc0336a1b97ddc3ce1d2fb992ab1712 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 17:18:11 +0100 Subject: [PATCH 11/18] :fire: remove useless tests --- .../components/heatmap_component.test.tsx | 126 ------------------ 1 file changed, 126 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx index a5e1b691c0fb5..0f9556c916614 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx @@ -383,132 +383,6 @@ describe('HeatmapComponent', function () { }); }); - it('should respect the value format with 5 digits when computing ranges', () => { - const newData: Datatable = { - type: 'datatable', - rows: [{ 'col-0-1': 3 }], - columns: [{ id: 'col-0-1', name: 'Count', meta: { type: 'number' } }], - }; - const newProps = { - ...wrapperProps, - formatFactory: ((format: { id: string }) => - format?.id === 'string' - ? ({ convert: (v: string) => String(v) } as unknown as FieldFormat) - : ({ - convert: (v: number) => toPrecisionWithoutRounding(v, 5), - } as unknown as FieldFormat)) as unknown as FormatFactory, - data: newData, - args: { - ...wrapperProps.args, - palette: { - params: { - colors: ['#6092c0', '#a8bfda', '#ebeff5', '#ecb385', '#e7664c'], - stops: [19.98123, 39.88, 60, 80, 100], - range: 'number', - gradient: true, - continuity: 'above', - rangeMin: 0, - rangeMax: null, - }, - }, - }, - } as unknown as HeatmapRenderProps; - const component = mountWithIntl(); - expect(component.find(Heatmap).prop('colorScale')).toEqual({ - bands: [ - { - start: 0, - end: 19.98123, - color: '#6092c0', - }, - { - start: 19.98123, - end: 39.88, - color: '#a8bfda', - }, - { - start: 39.88, - end: 60, - color: '#ebeff5', - }, - { - start: 60, - end: 80, - color: '#ecb385', - }, - { - start: 80, - end: Infinity, - color: '#e7664c', - }, - ], - type: 'bands', - }); - }); - - it('should respect the value format with 0 digits when computing ranges', () => { - const newData: Datatable = { - type: 'datatable', - rows: [{ 'col-0-1': 3 }], - columns: [{ id: 'col-0-1', name: 'Count', meta: { type: 'number' } }], - }; - const newProps = { - ...wrapperProps, - formatFactory: ((format: { id: string }) => - format?.id === 'string' - ? ({ convert: (v: string) => String(v) } as unknown as FieldFormat) - : ({ - convert: (v: number) => toPrecisionWithoutRounding(v, 0), - } as unknown as FieldFormat)) as unknown as FormatFactory, - data: newData, - args: { - ...wrapperProps.args, - palette: { - params: { - colors: ['#6092c0', '#a8bfda', '#ebeff5', '#ecb385', '#e7664c'], - stops: [19.98123, 39.88, 60, 80, 100], - range: 'number', - gradient: true, - continuity: 'above', - rangeMin: 0, - rangeMax: null, - }, - }, - }, - } as unknown as HeatmapRenderProps; - const component = mountWithIntl(); - expect(component.find(Heatmap).prop('colorScale')).toEqual({ - bands: [ - { - start: 0, - end: 19, - color: '#6092c0', - }, - { - start: 19, - end: 39, - color: '#a8bfda', - }, - { - start: 39, - end: 60, - color: '#ebeff5', - }, - { - start: 60, - end: 80, - color: '#ecb385', - }, - { - start: 80, - end: Infinity, - color: '#e7664c', - }, - ], - type: 'bands', - }); - }); - it('should keep the minimum open end', () => { const newData: Datatable = { type: 'datatable', From c2f349c2baa232d24fd63e4967b0dd8c203bc591 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 17:18:28 +0100 Subject: [PATCH 12/18] :white_check_mark: Align values to new formatter logic --- .../lens/open_in_lens/agg_based/heatmap.ts | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts b/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts index ad4b1e123b554..8a8852929abe5 100644 --- a/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts +++ b/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts @@ -123,36 +123,12 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { } expect(debugState.legend!.items).to.eql([ - { - color: '#006837', - key: '1,322 - 1,585.67', - name: '1,322 - 1,585.67', - }, - { - color: '#4CB15D', - key: '1,585.67 - 1,849.33', - name: '1,585.67 - 1,849.33', - }, - { - color: '#B7E075', - key: '1,849.33 - 2,113', - name: '1,849.33 - 2,113', - }, - { - color: '#FEFEBD', - key: '2,113 - 2,376.67', - name: '2,113 - 2,376.67', - }, - { - color: '#FDBF6F', - key: '2,376.67 - 2,640.33', - name: '2,376.67 - 2,640.33', - }, - { - color: '#EA5839', - key: '2,640.33 - 2,904', - name: '2,640.33 - 2,904', - }, + { key: '1,322 - 1,585.667', name: '1,322 - 1,585.667', color: '#006837' }, + { key: '1,585.667 - 1,849.333', name: '1,585.667 - 1,849.333', color: '#4CB15D' }, + { key: '1,849.333 - 2,113', name: '1,849.333 - 2,113', color: '#B7E075' }, + { key: '2,113 - 2,376.667', name: '2,113 - 2,376.667', color: '#FEFEBD' }, + { key: '2,376.667 - 2,640.333', name: '2,376.667 - 2,640.333', color: '#FDBF6F' }, + { key: '2,640.333 - 2,904', name: '2,640.333 - 2,904', color: '#EA5839' }, ]); }); From 0ec6853e39c85d4866ac8ff8a7ff75d8b9ac829e Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 6 Dec 2023 17:37:20 +0100 Subject: [PATCH 13/18] :fire: Removed unused code --- .../public/components/heatmap_component.test.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx index 0f9556c916614..bf9524c186134 100644 --- a/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx +++ b/src/plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx @@ -28,7 +28,7 @@ import { act } from 'react-dom/test-utils'; import { HeatmapRenderProps, HeatmapArguments } from '../../common'; import HeatmapComponent from './heatmap_component'; import { LegendSize } from '@kbn/visualizations-plugin/common'; -import { FieldFormat, FormatFactory } from '@kbn/field-formats-plugin/common'; +import { FieldFormat } from '@kbn/field-formats-plugin/common'; jest.mock('@elastic/charts', () => { const original = jest.requireActual('@elastic/charts'); @@ -111,10 +111,6 @@ const uiState = { setSilent: jest.fn(), } as any; -function toPrecisionWithoutRounding(v: number, digits: number) { - return Math.trunc(v * Math.pow(10, digits)) / Math.pow(10, digits); -} - describe('HeatmapComponent', function () { let wrapperProps: HeatmapRenderProps; From 92c5b23122ba7731dec2ee224c94da2d731e3ba3 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 7 Dec 2023 08:28:33 +0100 Subject: [PATCH 14/18] :white_check_mark: Add new test --- .../functional/apps/lens/group5/heatmap.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/x-pack/test/functional/apps/lens/group5/heatmap.ts b/x-pack/test/functional/apps/lens/group5/heatmap.ts index 9bf63d6a6ab39..147e8ea1c496f 100644 --- a/x-pack/test/functional/apps/lens/group5/heatmap.ts +++ b/x-pack/test/functional/apps/lens/group5/heatmap.ts @@ -137,6 +137,28 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ]); }); + it('should reflect the apply stop value without rounding', async () => { + // target item is 5722.774804505345 + // so set a value slightly lower which can be rounded + await testSubjects.setValue('lnsPalettePanel_dynamicColoring_range_value_0', '5722.7747', { + clearWithKeyboard: true, + }); + const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); + + // assert legend has a rounded value + expect(debugState?.legend!.items).to.eql([ + { key: '5,722.775 - 8,529.2', name: '5,722.775 - 8,529.2', color: '#6092c0' }, + { key: '8,529.2 - 11,335.66', name: '8,529.2 - 11,335.66', color: '#a8bfda' }, + { key: '11,335.66 - 14,142.1', name: '11,335.66 - 14,142.1', color: '#ebeff5' }, + { key: '14,142.1 - 16,948.55', name: '14,142.1 - 16,948.55', color: '#ecb385' }, + { key: '≥ 16,948.55', name: '≥ 16,948.55', color: '#e7664c' }, + ]); + // assert the cell has the correct coloring despite the legend rounding + expect(debugState?.heatmap!.cells[debugState.heatmap!.cells.length - 1].fill).to.be( + 'rgba(96, 146, 192, 1)' // rgba version of #6092c0 + ); + }); + it('should reset stop numbers when changing palette', async () => { await PageObjects.lens.changePaletteTo('status'); From 6e73b5d541399c06e5b97dbf9fd286fd96884c32 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 7 Dec 2023 08:28:46 +0100 Subject: [PATCH 15/18] :recycle: Refactor some cruft --- .../functional/apps/lens/group4/chart_data.ts | 28 +++++------ .../functional/apps/lens/group5/heatmap.ts | 46 ++++--------------- .../lens/open_in_lens/agg_based/heatmap.ts | 34 ++++---------- .../test/functional/page_objects/lens_page.ts | 3 +- 4 files changed, 33 insertions(+), 78 deletions(-) diff --git a/x-pack/test/functional/apps/lens/group4/chart_data.ts b/x-pack/test/functional/apps/lens/group4/chart_data.ts index 379d29040a1ea..9d43abd4ac650 100644 --- a/x-pack/test/functional/apps/lens/group4/chart_data.ts +++ b/x-pack/test/functional/apps/lens/group4/chart_data.ts @@ -52,19 +52,19 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { { name: '__other__', value: 5722.77 }, ]; - function assertMatchesExpectedData(state: DebugState) { + function assertMatchesExpectedData(state: DebugState | undefined) { expect( - state.bars![0].bars.map((bar) => ({ + state?.bars![0].bars.map((bar) => ({ x: bar.x, y: Math.floor(bar.y * 100) / 100, })) ).to.eql(expectedData); } - function assertMatchesExpectedPieData(state: DebugState) { + function assertMatchesExpectedPieData(state: DebugState | undefined) { expect( state - .partition![0].partitions.map((partition) => ({ + ?.partition![0].partitions.map((partition) => ({ name: partition.name, value: Math.floor(partition.value * 100) / 100, })) @@ -74,37 +74,33 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { it('should render xy chart', async () => { const data = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); - assertMatchesExpectedData(data!); + assertMatchesExpectedData(data); }); it('should render pie chart', async () => { await PageObjects.lens.switchToVisualization('pie'); const data = await PageObjects.lens.getCurrentChartDebugState('partitionVisChart'); - assertMatchesExpectedPieData(data!); + assertMatchesExpectedPieData(data); }); it('should render donut chart', async () => { await PageObjects.lens.switchToVisualization('donut'); const data = await PageObjects.lens.getCurrentChartDebugState('partitionVisChart'); - assertMatchesExpectedPieData(data!); + assertMatchesExpectedPieData(data); }); it('should render treemap chart', async () => { await PageObjects.lens.switchToVisualization('treemap', 'treemap'); const data = await PageObjects.lens.getCurrentChartDebugState('partitionVisChart'); - assertMatchesExpectedPieData(data!); + assertMatchesExpectedPieData(data); }); it('should render heatmap chart', async () => { await PageObjects.lens.switchToVisualization('heatmap', 'heat'); const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert axes - expect(debugState.axes!.x[0].labels).to.eql([ + expect(debugState?.axes!.x[0].labels).to.eql([ '97.220.3.248', '169.228.188.120', '78.83.247.30', @@ -112,13 +108,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { '93.28.27.24', 'Other', ]); - expect(debugState.axes!.y[0].labels).to.eql(['']); + expect(debugState?.axes!.y[0].labels).to.eql(['']); // assert cells - expect(debugState.heatmap!.cells.length).to.eql(6); + expect(debugState?.heatmap!.cells.length).to.eql(6); // assert legend - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#6092c0' }, { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, diff --git a/x-pack/test/functional/apps/lens/group5/heatmap.ts b/x-pack/test/functional/apps/lens/group5/heatmap.ts index 147e8ea1c496f..26e77578f4545 100644 --- a/x-pack/test/functional/apps/lens/group5/heatmap.ts +++ b/x-pack/test/functional/apps/lens/group5/heatmap.ts @@ -40,12 +40,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.switchToVisualization('heatmap', 'heat'); const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert axes - expect(debugState.axes!.x[0].labels).to.eql([ + expect(debugState?.axes!.x[0].labels).to.eql([ '97.220.3.248', '169.228.188.120', '78.83.247.30', @@ -53,13 +49,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { '93.28.27.24', 'Other', ]); - expect(debugState.axes!.y[0].labels).to.eql(['']); + expect(debugState?.axes!.y[0].labels).to.eql(['']); // assert cells - expect(debugState.heatmap!.cells.length).to.eql(6); + expect(debugState?.heatmap!.cells.length).to.eql(6); // assert legend - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#6092c0' }, { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, @@ -80,12 +76,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert legend has changed - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '7,125.997 - 8,529.22', name: '7,125.997 - 8,529.22', color: '#6092c0' }, { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#a8bfda' }, { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#ebeff5' }, @@ -98,12 +90,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.click('lnsPalettePanel_dynamicColoring_rangeType_groups_number'); const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert legend has changed - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '7,125.99 - 8,529.2', name: '7,125.99 - 8,529.2', color: '#6092c0' }, { key: '8,529.2 - 11,335.66', name: '8,529.2 - 11,335.66', color: '#a8bfda' }, { key: '11,335.66 - 14,142.1', name: '11,335.66 - 14,142.1', color: '#ebeff5' }, @@ -123,12 +111,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert legend has changed - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '0 - 8,529.2', name: '0 - 8,529.2', color: '#6092c0' }, { key: '8,529.2 - 11,335.66', name: '8,529.2 - 11,335.66', color: '#a8bfda' }, { key: '11,335.66 - 14,142.1', name: '11,335.66 - 14,142.1', color: '#ebeff5' }, @@ -164,12 +148,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert legend has changed - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#209280' }, { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#54b399' }, { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#d6bf57' }, @@ -183,12 +163,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert legend has not changed - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '5,722.775 - 8,529.22', name: '5,722.775 - 8,529.22', color: '#209280' }, { key: '8,529.22 - 11,335.665', name: '8,529.22 - 11,335.665', color: '#54b399' }, { key: '11,335.665 - 14,142.11', name: '11,335.665 - 14,142.11', color: '#d6bf57' }, @@ -205,9 +181,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.selectValue('lnsLeftAxisTitle-select', 'Auto'); const debugState = await PageObjects.lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } + expect(debugState?.axes?.y?.[0].title).to.eql('Average of bytes'); }); }); diff --git a/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts b/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts index 8a8852929abe5..f812b741bd440 100644 --- a/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts +++ b/x-pack/test/functional/apps/lens/open_in_lens/agg_based/heatmap.ts @@ -56,15 +56,11 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await lens.waitForVisualization('heatmapChart'); const debugState = await lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - // assert axes - expect(debugState.axes!.x[0].labels).to.eql(['win 8', 'win xp', 'win 7', 'ios', 'osx']); - expect(debugState.axes!.y[0].labels).to.eql(['']); - expect(debugState.heatmap!.cells.length).to.eql(5); - expect(debugState.legend!.items).to.eql([ + expect(debugState?.axes!.x[0].labels).to.eql(['win 8', 'win xp', 'win 7', 'ios', 'osx']); + expect(debugState?.axes!.y[0].labels).to.eql(['']); + expect(debugState?.heatmap!.cells.length).to.eql(5); + expect(debugState?.legend!.items).to.eql([ { color: '#006837', key: '1,322 - 1,717.5', @@ -94,13 +90,9 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await lens.waitForVisualization('heatmapChart'); const debugState = await lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - - expect(debugState.axes!.x[0].labels).to.eql(['*']); - expect(debugState.axes!.y[0].labels).to.eql(['win 8', 'win xp', 'win 7', 'ios', 'osx']); - expect(debugState.heatmap!.cells.length).to.eql(5); + expect(debugState?.axes!.x[0].labels).to.eql(['*']); + expect(debugState?.axes!.y[0].labels).to.eql(['win 8', 'win xp', 'win 7', 'ios', 'osx']); + expect(debugState?.heatmap!.cells.length).to.eql(5); }); it('should respect heatmap colors number', async () => { @@ -118,11 +110,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await lens.waitForVisualization('heatmapChart'); const debugState = await lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { key: '1,322 - 1,585.667', name: '1,322 - 1,585.667', color: '#006837' }, { key: '1,585.667 - 1,849.333', name: '1,585.667 - 1,849.333', color: '#4CB15D' }, { key: '1,849.333 - 2,113', name: '1,849.333 - 2,113', color: '#B7E075' }, @@ -154,11 +142,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { await lens.waitForVisualization('heatmapChart'); const debugState = await lens.getCurrentChartDebugState('heatmapChart'); - if (!debugState) { - throw new Error('Debug state is not available'); - } - - expect(debugState.legend!.items).to.eql([ + expect(debugState?.legend!.items).to.eql([ { color: '#006837', key: '0 - 100', diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index 927e8710017ee..3d55b964ff4f4 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -8,6 +8,7 @@ import expect from '@kbn/expect'; import { setTimeout as setTimeoutAsync } from 'timers/promises'; import type { FittingFunction, XYCurveType } from '@kbn/lens-plugin/public'; +import { DebugState } from '@elastic/charts'; import { WebElementWrapper } from '../../../../test/functional/services/lib/web_element_wrapper'; import { FtrProviderContext } from '../ftr_provider_context'; import { logWrapper } from './log_wrapper'; @@ -1091,7 +1092,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont ); }, - async getCurrentChartDebugState(visType: string) { + async getCurrentChartDebugState(visType: string): Promise { await this.waitForVisualization(visType); return await elasticChart.getChartDebugData('lnsWorkspace'); }, From eece002e4a8c15a3fcfde00b6fdb93a2810c4bba Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 7 Dec 2023 10:06:27 +0100 Subject: [PATCH 16/18] :recycle: Add more checks --- .../test/functional/apps/lens/group4/tsdb.ts | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/x-pack/test/functional/apps/lens/group4/tsdb.ts b/x-pack/test/functional/apps/lens/group4/tsdb.ts index 745592a02cb1d..9e10762af719e 100644 --- a/x-pack/test/functional/apps/lens/group4/tsdb.ts +++ b/x-pack/test/functional/apps/lens/group4/tsdb.ts @@ -224,13 +224,13 @@ function getDataMapping( return dataStreamMapping; } -function sumFirstNValues(n: number, bars: Array<{ y: number }>): number { +function sumFirstNValues(n: number, bars: Array<{ y: number }> | undefined): number { const indexes = Array(n) .fill(1) .map((_, i) => i); let countSum = 0; for (const index of indexes) { - if (bars[index]) { + if (bars?.[index]) { countSum += bars[index].y; } } @@ -816,29 +816,29 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const data = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); - const counterBars = data.bars![0].bars; - const countBars = data.bars![1].bars; + const counterBars = data?.bars![0].bars; + const countBars = data?.bars![1].bars; log.info('Check counter data before the upgrade'); // check there's some data before the upgrade - expect(counterBars[0].y).to.eql(5000); + expect(counterBars?.[0].y).to.eql(5000); log.info('Check counter data after the upgrade'); // check there's some data after the upgrade - expect(counterBars[counterBars.length - 1].y).to.eql(5000); + expect(counterBars?.[counterBars.length - 1].y).to.eql(5000); // due to the flaky nature of exact check here, we're going to relax it // as long as there's data before and after it is ok log.info('Check count before the upgrade'); - const columnsToCheck = countBars.length / 2; + const columnsToCheck = countBars ? countBars.length / 2 : 0; // Before the upgrade the count is N times the indexes expect(sumFirstNValues(columnsToCheck, countBars)).to.be.greaterThan( indexes.length * TEST_DOC_COUNT - 1 ); log.info('Check count after the upgrade'); // later there are only documents for the upgraded stream - expect(sumFirstNValues(columnsToCheck, [...countBars].reverse())).to.be.greaterThan( - TEST_DOC_COUNT - 1 - ); + expect( + sumFirstNValues(columnsToCheck, [...(countBars ?? [])].reverse()) + ).to.be.greaterThan(TEST_DOC_COUNT - 1); }); }); }); @@ -911,7 +911,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const data = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); - const bars = data.bars![0].bars; + const bars = data?.bars![0].bars; const columnsToCheck = bars.length / 2; // due to the flaky nature of exact check here, we're going to relax it // as long as there's data before and after it is ok @@ -952,7 +952,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const dataBefore = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); - const barsBefore = dataBefore.bars![0].bars; + const barsBefore = dataBefore?.bars![0].bars; expect(barsBefore.some(({ y }) => y)).to.eql(true); // check after the downgrade @@ -969,8 +969,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const dataAfter = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); - const barsAfter = dataAfter.bars![0].bars; - expect(barsAfter.some(({ y }) => y)).to.eql(true); + const barsAfter = dataAfter?.bars![0].bars; + expect(barsAfter?.some(({ y }) => y)).to.eql(true); }); }); }); From b009b963c90d09cb91b8077c33816ed08b2a17ff Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 7 Dec 2023 11:37:39 +0100 Subject: [PATCH 17/18] :recycle: More type fixes --- x-pack/test/functional/apps/lens/group4/tsdb.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/test/functional/apps/lens/group4/tsdb.ts b/x-pack/test/functional/apps/lens/group4/tsdb.ts index 9e10762af719e..730318f33db97 100644 --- a/x-pack/test/functional/apps/lens/group4/tsdb.ts +++ b/x-pack/test/functional/apps/lens/group4/tsdb.ts @@ -912,7 +912,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const data = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); const bars = data?.bars![0].bars; - const columnsToCheck = bars.length / 2; + const columnsToCheck = bars ? bars.length / 2 : 0; // due to the flaky nature of exact check here, we're going to relax it // as long as there's data before and after it is ok log.info('Check count before the downgrade'); @@ -922,7 +922,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { ); log.info('Check count after the downgrade'); // later there are only documents for the upgraded stream - expect(sumFirstNValues(columnsToCheck, [...bars].reverse())).to.be.greaterThan( + expect(sumFirstNValues(columnsToCheck, [...(bars ?? [])].reverse())).to.be.greaterThan( TEST_DOC_COUNT - 1 ); }); @@ -953,7 +953,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.lens.waitForVisualization('xyVisChart'); const dataBefore = await PageObjects.lens.getCurrentChartDebugState('xyVisChart'); const barsBefore = dataBefore?.bars![0].bars; - expect(barsBefore.some(({ y }) => y)).to.eql(true); + expect(barsBefore?.some(({ y }) => y)).to.eql(true); // check after the downgrade await PageObjects.lens.goToTimeRange( From d1d86a239633b2ba413a959d79ca7274f8197760 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 7 Dec 2023 14:20:39 +0100 Subject: [PATCH 18/18] :label: Fix type once for all --- x-pack/test/functional/page_objects/lens_page.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index 3d55b964ff4f4..c9b8adff08d4a 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -1092,9 +1092,9 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont ); }, - async getCurrentChartDebugState(visType: string): Promise { + async getCurrentChartDebugState(visType: string): Promise { await this.waitForVisualization(visType); - return await elasticChart.getChartDebugData('lnsWorkspace'); + return (await elasticChart.getChartDebugData('lnsWorkspace'))!; }, /**