From 348bfb8b33f418d504489cd4a212539d7e04f256 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Tue, 4 Jan 2022 10:22:36 +0100 Subject: [PATCH] [XY] Fix Y Axis visibility for Percentile aggregation (#122162) * :bug: Fix percentile axis bug * :white_check_mark: Add test for fix Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../xy/public/config/get_config.test.ts | 41 +++++++++- .../vis_types/xy/public/config/get_config.ts | 9 ++- src/plugins/vis_types/xy/public/mocks.ts | 80 +++++++++++++++++++ .../vis_types/xy/public/utils/accessors.tsx | 9 ++- 4 files changed, 134 insertions(+), 5 deletions(-) diff --git a/src/plugins/vis_types/xy/public/config/get_config.test.ts b/src/plugins/vis_types/xy/public/config/get_config.test.ts index 52d68b40766a9..7608ef7cda460 100644 --- a/src/plugins/vis_types/xy/public/config/get_config.test.ts +++ b/src/plugins/vis_types/xy/public/config/get_config.test.ts @@ -7,7 +7,8 @@ */ import { getConfig } from './get_config'; -import { visData, visParamsWithTwoYAxes } from '../mocks'; +import { visData, visDataPercentile, visParamsWithTwoYAxes } from '../mocks'; +import { VisParams } from '../types'; // ToDo: add more tests for all the config properties describe('getConfig', () => { @@ -65,4 +66,42 @@ describe('getConfig', () => { const config = getConfig(visData, newVisParams); expect(config.yAxes.length).toBe(1); }); + + it('assigns the correct number of yAxes if the agg is Percentile', () => { + const newVisParams = { + ...visParamsWithTwoYAxes, + seriesParams: [ + { + type: 'line', + data: { + label: 'Percentiles of bytes', + id: '1', + }, + drawLinesBetweenPoints: true, + interpolate: 'linear', + lineWidth: 2, + mode: 'normal', + show: true, + showCircles: true, + circlesRadius: 3, + valueAxis: 'ValueAxis-1', + }, + ], + dimensions: { + ...visParamsWithTwoYAxes.dimensions, + y: ['1st', '5th', '25th', '50th', '75th', '95th', '99th'].map((prefix, accessor) => ({ + label: `${prefix} percentile of bytes`, + aggType: 'percentiles', + params: {}, + accessor, + format: { + id: 'number', + params: {}, + }, + })), + }, + } as VisParams; + const config = getConfig(visDataPercentile, newVisParams); + expect(config.yAxes.length).toBe(1); + }); }); diff --git a/src/plugins/vis_types/xy/public/config/get_config.ts b/src/plugins/vis_types/xy/public/config/get_config.ts index e23fff937ad9b..d7cf22625e10e 100644 --- a/src/plugins/vis_types/xy/public/config/get_config.ts +++ b/src/plugins/vis_types/xy/public/config/get_config.ts @@ -28,6 +28,7 @@ import { getLegend } from './get_legend'; import { getAxis } from './get_axis'; import { getAspects } from './get_aspects'; import { ChartType } from '../index'; +import { getSafeId } from '../utils/accessors'; export function getConfig( table: Datatable, @@ -51,13 +52,17 @@ export function getConfig( const yAxes: Array> = []; + // avoid duplicates based on aggId + const aspectVisited = new Set(); params.dimensions.y.forEach((y) => { const accessor = y.accessor; const aspect = aspects.y.find(({ column }) => column === accessor); - const serie = params.seriesParams.find(({ data: { id } }) => id === aspect?.aggId); + const aggId = getSafeId(aspect?.aggId); + const serie = params.seriesParams.find(({ data: { id } }) => id === aggId); const valueAxis = params.valueAxes.find(({ id }) => id === serie?.valueAxis); - if (aspect && valueAxis) { + if (aspect && valueAxis && !aspectVisited.has(aggId)) { yAxes.push(getAxis(valueAxis, params.grid, aspect, params.seriesParams)); + aspectVisited.add(aggId); } }); diff --git a/src/plugins/vis_types/xy/public/mocks.ts b/src/plugins/vis_types/xy/public/mocks.ts index 6c0de8de1ac36..187850e41b3a8 100644 --- a/src/plugins/vis_types/xy/public/mocks.ts +++ b/src/plugins/vis_types/xy/public/mocks.ts @@ -63,6 +63,86 @@ export const visData = { ], } as Datatable; +export const visDataPercentile = { + type: 'datatable', + columns: [ + { + id: 'col-0-1.1', + name: '1st percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-1-1.5', + name: '5th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-2-1.25', + name: '25th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-3-1.50', + name: '50th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-4-1.75', + name: '75th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-5-1.95', + name: '95th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + { + id: 'col-6-1.99', + name: '99th percentile of bytes', + meta: { + type: 'number', + field: 'bytes', + index: 'kibana_sample_data_logs', + }, + }, + ], + rows: [ + { + 'col-0-1.1': 0, + 'col-1-1.5': 0, + 'col-2-1.25': 1741.5, + 'col-3-1.50': 4677, + 'col-4-1.75': 5681.5, + 'col-5-1.95': 6816, + 'col-6-1.99': 6816, + }, + ], +} as Datatable; + export const visParamsWithTwoYAxes = { type: 'histogram', addLegend: true, diff --git a/src/plugins/vis_types/xy/public/utils/accessors.tsx b/src/plugins/vis_types/xy/public/utils/accessors.tsx index 2b552c9f3f9cf..6eccb36d6fa73 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.tsx +++ b/src/plugins/vis_types/xy/public/utils/accessors.tsx @@ -79,8 +79,13 @@ export const getSplitSeriesAccessorFnMap = ( }; // For percentile, the aggregation id is coming in the form %s.%d, where %s is agg_id and %d - percents -export const isPercentileIdEqualToSeriesId = (columnId: number | string, seriesColumnId: string) => - columnId.toString().split('.')[0] === seriesColumnId; +export const getSafeId = (columnId?: number | string | null) => + (columnId || '').toString().split('.')[0]; + +export const isPercentileIdEqualToSeriesId = ( + columnId: number | string | null | undefined, + seriesColumnId: string +) => getSafeId(columnId) === seriesColumnId; export const isValidSeriesForDimension = (seriesColumnId: string, { aggId, accessor }: Aspect) => (aggId === seriesColumnId || isPercentileIdEqualToSeriesId(aggId ?? '', seriesColumnId)) &&