From 1d5a77ae93dd40558b273aa7e37c47b56b79aa7a Mon Sep 17 00:00:00 2001 From: Nick Partridge Date: Tue, 9 Apr 2024 12:53:22 -0700 Subject: [PATCH] [Lens] Fix default formatter for gauge charts (#179473) --- .../components/gauge_component.test.tsx | 172 ++++++++++++++---- .../public/components/gauge_component.tsx | 11 +- .../components/heatmap_component.test.tsx | 9 - .../partition_vis_component.test.tsx | 9 - .../public/utils/get_color_picker.test.tsx | 9 - .../visualizations/common/utils/accessors.ts | 2 +- .../common/components/charts/common.test.tsx | 9 - 7 files changed, 146 insertions(+), 75 deletions(-) diff --git a/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.test.tsx b/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.test.tsx index ca21711d9bfb7..5ed6fdf52f669 100644 --- a/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.test.tsx +++ b/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.test.tsx @@ -11,7 +11,7 @@ import { ColorStop } from '@kbn/coloring'; import { chartPluginMock } from '@kbn/charts-plugin/public/mocks'; import { fieldFormatsServiceMock } from '@kbn/field-formats-plugin/public/mocks'; import type { Datatable } from '@kbn/expressions-plugin/public'; -import { DatatableColumn, DatatableRow } from '@kbn/expressions-plugin/common'; +import { DatatableColumn, DatatableColumnMeta, DatatableRow } from '@kbn/expressions-plugin/common'; import { shallowWithIntl } from '@kbn/test-jest-helpers'; import { GaugeRenderProps, @@ -29,15 +29,7 @@ import { ColorBandSimpleConfig, Color, } from '@elastic/charts'; - -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); +import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common'; const numberColumn = (id = 'metric-accessor'): DatatableColumn => ({ id, @@ -51,17 +43,8 @@ const numberColumn = (id = 'metric-accessor'): DatatableColumn => ({ }, }); -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); - -const chartsThemeService = chartPluginMock.createSetupContract().theme; -const paletteThemeService = chartPluginMock.createSetupContract().palettes; +const { theme: chartsThemeService, palettes: paletteThemeService } = + chartPluginMock.createSetupContract(); const formatService = fieldFormatsServiceMock.createStartContract(); const args: GaugeArguments = { labelMajor: 'Gauge', @@ -113,13 +96,17 @@ describe('GaugeComponent', function () { }; }); + beforeEach(() => { + jest.clearAllMocks(); + }); + it('renders the chart', () => { const component = shallowWithIntl(); expect(component.find(Chart)).toMatchSnapshot(); }); it('returns null when metric is not provided', async () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -134,7 +121,7 @@ describe('GaugeComponent', function () { }); it('shows empty placeholder when minimum accessor equals maximum accessor', async () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -148,7 +135,7 @@ describe('GaugeComponent', function () { expect(component.find('EmptyPlaceholder')).toHaveLength(1); }); it('shows empty placeholder when minimum accessor value is greater maximum accessor value', async () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -162,7 +149,7 @@ describe('GaugeComponent', function () { expect(component.find('EmptyPlaceholder')).toHaveLength(1); }); it('when metric value is bigger than max, it takes maximum value', () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -180,7 +167,7 @@ describe('GaugeComponent', function () { describe('labelMajor and labelMinor settings', () => { it('displays no labelMajor and no labelMinor when no passed', () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -194,7 +181,7 @@ describe('GaugeComponent', function () { expect(datum?.subtitle).toEqual(''); }); it('displays custom labelMajor and labelMinor when passed', () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -209,7 +196,7 @@ describe('GaugeComponent', function () { expect(datum?.subtitle).toEqual('custom labelMinor'); }); it('displays auto labelMajor', () => { - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -237,7 +224,7 @@ describe('GaugeComponent', function () { rangeMax: 4, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -267,7 +254,7 @@ describe('GaugeComponent', function () { rangeMax: 4, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -295,7 +282,7 @@ describe('GaugeComponent', function () { rangeMax: 100, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -323,7 +310,7 @@ describe('GaugeComponent', function () { rangeMax: 10, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -351,7 +338,7 @@ describe('GaugeComponent', function () { rangeMax: 30, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -379,7 +366,7 @@ describe('GaugeComponent', function () { rangeMax: 10, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -409,7 +396,7 @@ describe('GaugeComponent', function () { rangeMax: 100, }, }; - const customProps = { + const customProps: GaugeRenderProps = { ...wrapperProps, args: { ...wrapperProps.args, @@ -442,4 +429,119 @@ describe('GaugeComponent', function () { expect(settingsComponent.prop('ariaUseDefaultSummary')).toEqual(true); }); }); + + describe('formatting', () => { + let baseFormattingProps: GaugeRenderProps; + const metricFormat: ExpressionValueVisDimension['format'] = { + id: 'bytes', + }; + const tableFormatParams = { + id: 'number', + params: { + pattern: '0,00000', + }, + }; + const metricMetaParams: DatatableColumnMeta = { + type: 'number', + params: { + id: 'test', + params: { + pattern: '000,000.00', + }, + }, + }; + const createColumnsWithMetricParams = (params?: DatatableColumnMeta['params']) => + wrapperProps.data.columns.map((c) => + c.id !== 'metric-accessor' + ? c + : { + ...c, + meta: { + ...c.meta, + params, + }, + } + ); + + beforeAll(() => { + baseFormattingProps = { + ...wrapperProps, + args: { + ...wrapperProps.args, + metric: { + ...(wrapperProps.args.metric as ExpressionValueVisDimension), + accessor: { + id: 'metric-accessor', + name: 'metric-accessor', + meta: metricMetaParams, + }, + format: metricFormat, + }, + }, + data: { + ...wrapperProps.data, + columns: createColumnsWithMetricParams(tableFormatParams), + }, + }; + }); + + it('should use custom metric format params, if provided', () => { + shallowWithIntl(); + expect(formatService.deserialize).toBeCalledWith(metricFormat); + }); + + it('should use table metric format params, if provided', () => { + const customProps: GaugeRenderProps = { + ...baseFormattingProps, + args: { + ...baseFormattingProps.args, + metric: 'metric-accessor', + }, + }; + shallowWithIntl(); + expect(formatService.deserialize).toBeCalledWith(tableFormatParams); + }); + + it('should use default metric format params, if no others provided', () => { + const testParams = { + id: 'test', + }; + const customProps: GaugeRenderProps = { + ...baseFormattingProps, + args: { + ...baseFormattingProps.args, + metric: 'metric-accessor', + }, + data: { + ...baseFormattingProps.data, + columns: createColumnsWithMetricParams(testParams), + }, + }; + + shallowWithIntl(); + expect(formatService.deserialize).toBeCalledWith(testParams); + }); + + it('should use fallback if no default metric format and no others provided', () => { + const customProps: GaugeRenderProps = { + ...baseFormattingProps, + args: { + ...baseFormattingProps.args, + metric: 'metric-accessor', + }, + data: { + ...wrapperProps.data, + columns: createColumnsWithMetricParams(), + }, + }; + + shallowWithIntl(); + expect(formatService.deserialize).toBeCalledWith({ + id: 'number', + params: { + pattern: '0,0.0', + }, + }); + }); + }); }); diff --git a/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx b/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx index 714ac6f3e057e..0e1ebe988e602 100644 --- a/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx +++ b/src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx @@ -14,7 +14,11 @@ import { FieldFormat } from '@kbn/field-formats-plugin/common'; import type { CustomPaletteState } from '@kbn/charts-plugin/public'; import { EmptyPlaceholder } from '@kbn/charts-plugin/public'; import { getOverridesFor } from '@kbn/chart-expressions-common'; -import { findAccessor, isVisDimension } from '@kbn/visualizations-plugin/common/utils'; +import { + findAccessor, + getFormatByAccessor, + isVisDimension, +} from '@kbn/visualizations-plugin/common/utils'; import { i18n } from '@kbn/i18n'; import { GaugeRenderProps, @@ -307,10 +311,11 @@ export const GaugeComponent: FC = ({ ? metricColumn?.meta?.params : undefined; - const defaultMetricFormatParams = { + const defaultMetricFormatParams = (args.metric && + getFormatByAccessor(args.metric, data.columns)) || { id: 'number', params: { - pattern: max - min > 5 ? `0,0` : `0,0.0`, + pattern: max - min > 5 ? '0,0' : '0,0.0', }, }; 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 bf9524c186134..86d40144c67bd 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 @@ -30,15 +30,6 @@ import HeatmapComponent from './heatmap_component'; import { LegendSize } from '@kbn/visualizations-plugin/common'; import { FieldFormat } from '@kbn/field-formats-plugin/common'; -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); - const actWithTimeout = (action: Function, timer: number = 1) => act( () => diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.test.tsx b/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.test.tsx index b5b2fb553676f..9d327c1272b7c 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.test.tsx +++ b/src/plugins/chart_expressions/expression_partition_vis/public/components/partition_vis_component.test.tsx @@ -28,15 +28,6 @@ import { ChartTypes } from '../../common/types'; import { LegendSize } from '@kbn/visualizations-plugin/common'; import { cloneDeep } from 'lodash'; -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); - const actWithTimeout = (action: Function, timer: number = 1) => act( () => diff --git a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_color_picker.test.tsx b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_color_picker.test.tsx index 04aa0136dcadd..14164045326c7 100644 --- a/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_color_picker.test.tsx +++ b/src/plugins/chart_expressions/expression_partition_vis/public/utils/get_color_picker.test.tsx @@ -23,15 +23,6 @@ import { createMockBucketColumns, createMockVisData } from '../mocks'; const bucketColumns = createMockBucketColumns(); const visData = createMockVisData(); -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); - describe('LegendColorPickerWrapper', () => { const mockState = new Map(); const uiState = { diff --git a/src/plugins/visualizations/common/utils/accessors.ts b/src/plugins/visualizations/common/utils/accessors.ts index 929d0fae7b4b4..c258e06779f22 100644 --- a/src/plugins/visualizations/common/utils/accessors.ts +++ b/src/plugins/visualizations/common/utils/accessors.ts @@ -103,7 +103,7 @@ export function getFormatByAccessor( defaultColumnFormat?: SerializedFieldFormat ): SerializedFieldFormat | undefined { return typeof dimension === 'string' - ? getColumnByAccessor(dimension, columns)?.meta.params || defaultColumnFormat + ? getColumnByAccessor(dimension, columns)?.meta?.params || defaultColumnFormat : dimension.format || defaultColumnFormat; } diff --git a/x-pack/plugins/security_solution/public/common/components/charts/common.test.tsx b/x-pack/plugins/security_solution/public/common/components/charts/common.test.tsx index 20dfc140c4138..10234da2fac2f 100644 --- a/x-pack/plugins/security_solution/public/common/components/charts/common.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/charts/common.test.tsx @@ -23,15 +23,6 @@ import { LEGACY_LIGHT_THEME, LEGACY_DARK_THEME } from '@elastic/charts'; jest.mock('../../lib/kibana'); -jest.mock('@elastic/charts', () => { - const original = jest.requireActual('@elastic/charts'); - - return { - ...original, - getSpecId: jest.fn(() => {}), - }; -}); - describe('WrappedByAutoSizer', () => { it('should render correct default height', () => { const wrapper = shallow();