From 08629f3ba6431259e0da1a5c4872d988e39dc468 Mon Sep 17 00:00:00 2001 From: Diana Derevyankina Date: Fri, 24 Sep 2021 19:15:08 +0300 Subject: [PATCH 1/4] [Viz] legend duplicates percentile options when chart has both left & right Y axes --- .../xy/public/utils/accessors.test.ts | 40 ++++++++++++++++++- .../vis_types/xy/public/utils/accessors.tsx | 15 ++++++- .../xy/public/utils/render_all_series.tsx | 16 ++------ 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/plugins/vis_types/xy/public/utils/accessors.test.ts b/src/plugins/vis_types/xy/public/utils/accessors.test.ts index 61d175fa8ff7d..06920ceebe980 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.test.ts +++ b/src/plugins/vis_types/xy/public/utils/accessors.test.ts @@ -6,7 +6,11 @@ * Side Public License, v 1. */ -import { COMPLEX_SPLIT_ACCESSOR, getComplexAccessor } from './accessors'; +import { + COMPLEX_SPLIT_ACCESSOR, + getComplexAccessor, + isPercentileIdEqualToSeriesId, +} from './accessors'; import { BUCKET_TYPES } from '../../../../data/common'; import { AccessorFn, Datum } from '@elastic/charts'; @@ -99,3 +103,37 @@ describe('XY chart datum accessors', () => { expect(accessor).toBeUndefined(); }); }); + +describe('isPercentileIdEqualToSeriesId', () => { + it('should be equal for plain column ids', () => { + const seriesColumnId = 'col-0-1'; + const columnId = `${seriesColumnId}`; + + const isEqual = isPercentileIdEqualToSeriesId(columnId, seriesColumnId); + expect(isEqual).toBeTruthy(); + }); + + it('should be equal for column with percentile', () => { + const seriesColumnId = '1'; + const columnId = `${seriesColumnId}.95`; + + const isEqual = isPercentileIdEqualToSeriesId(columnId, seriesColumnId); + expect(isEqual).toBeTruthy(); + }); + + it('should not be equal for column with percentile equal to seriesColumnId', () => { + const seriesColumnId = '1'; + const columnId = `2.1`; + + const isEqual = isPercentileIdEqualToSeriesId(columnId, seriesColumnId); + expect(isEqual).toBeFalsy(); + }); + + it('should not be equal for column with percentile, where columnId contains seriesColumnId', () => { + const seriesColumnId = '1'; + const columnId = `${seriesColumnId}2.1`; + + const isEqual = isPercentileIdEqualToSeriesId(columnId, seriesColumnId); + expect(isEqual).toBeFalsy(); + }); +}); diff --git a/src/plugins/vis_types/xy/public/utils/accessors.tsx b/src/plugins/vis_types/xy/public/utils/accessors.tsx index 9566f819ba145..4ab483a99c82a 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.tsx +++ b/src/plugins/vis_types/xy/public/utils/accessors.tsx @@ -9,7 +9,12 @@ import { AccessorFn, Accessor } from '@elastic/charts'; import { BUCKET_TYPES } from '../../../../data/public'; import { FakeParams } from '../../../../visualizations/public'; -import { Aspect } from '../types'; +import type { Aspect } from '../types'; + +interface Dimension { + id?: Aspect['aggId']; + accessor?: Aspect['accessor']; +} export const COMPLEX_X_ACCESSOR = '__customXAccessor__'; export const COMPLEX_SPLIT_ACCESSOR = '__complexSplitAccessor__'; @@ -77,3 +82,11 @@ export const getSplitSeriesAccessorFnMap = ( return m; }; + +// For percentile aggregation id is coming in the form `%d.%d`, where first `%d` is `id` and the second - `percents` +export const isPercentileIdEqualToSeriesId = (columnId: number | string, seriesColumnId: string) => + columnId.toString().split('.')[0] === seriesColumnId; + +export const isValidSeriesForDimension = (seriesColumnId: string, { id, accessor }: Dimension) => + (id === seriesColumnId || isPercentileIdEqualToSeriesId(id ?? '', seriesColumnId)) && + accessor !== null; diff --git a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx index f8ca1d059ae4f..e5a9074811539 100644 --- a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx +++ b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx @@ -22,10 +22,10 @@ import { } from '@elastic/charts'; import { DatatableRow } from '../../../../expressions/public'; -import { METRIC_TYPES } from '../../../../data/public'; import { ChartType } from '../../common'; import { SeriesParam, VisConfig } from '../types'; +import { isValidSeriesForDimension } from './accessors'; /** * Matches vislib curve to elastic charts @@ -82,17 +82,9 @@ export const renderAllSeries = ( interpolate, type, }) => { - const yAspects = aspects.y.filter(({ aggId, aggType, accessor }) => { - if ( - aggType === METRIC_TYPES.PERCENTILES || - aggType === METRIC_TYPES.PERCENTILE_RANKS || - aggType === METRIC_TYPES.STD_DEV - ) { - return aggId?.includes(paramId) && accessor !== null; - } else { - return aggId === paramId && accessor !== null; - } - }); + const yAspects = aspects.y.filter(({ aggId, accessor }) => + isValidSeriesForDimension(paramId, { id: aggId, accessor }) + ); if (!show || !yAspects.length) { return null; } From 4d7b0d4015b5fc933e647a06c05dbaa3521fe2b6 Mon Sep 17 00:00:00 2001 From: Diana Derevyankina Date: Mon, 27 Sep 2021 10:34:35 +0300 Subject: [PATCH 2/4] Update comment for isPercentileIdEqualToSeriesId --- src/plugins/vis_types/xy/public/utils/accessors.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/vis_types/xy/public/utils/accessors.tsx b/src/plugins/vis_types/xy/public/utils/accessors.tsx index 4ab483a99c82a..99b6ab32e0c06 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.tsx +++ b/src/plugins/vis_types/xy/public/utils/accessors.tsx @@ -83,7 +83,7 @@ export const getSplitSeriesAccessorFnMap = ( return m; }; -// For percentile aggregation id is coming in the form `%d.%d`, where first `%d` is `id` and the second - `percents` +// 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; From 11251b716911184ae624509a73992d507c5341b7 Mon Sep 17 00:00:00 2001 From: Diana Derevyankina Date: Mon, 27 Sep 2021 16:17:17 +0300 Subject: [PATCH 3/4] Remove Dimension interface --- src/plugins/vis_types/xy/public/utils/accessors.tsx | 12 +++++------- .../vis_types/xy/public/utils/render_all_series.tsx | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/plugins/vis_types/xy/public/utils/accessors.tsx b/src/plugins/vis_types/xy/public/utils/accessors.tsx index 99b6ab32e0c06..89f1091350047 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.tsx +++ b/src/plugins/vis_types/xy/public/utils/accessors.tsx @@ -11,11 +11,6 @@ import { BUCKET_TYPES } from '../../../../data/public'; import { FakeParams } from '../../../../visualizations/public'; import type { Aspect } from '../types'; -interface Dimension { - id?: Aspect['aggId']; - accessor?: Aspect['accessor']; -} - export const COMPLEX_X_ACCESSOR = '__customXAccessor__'; export const COMPLEX_SPLIT_ACCESSOR = '__complexSplitAccessor__'; const SHARD_DELAY = 'shard_delay'; @@ -87,6 +82,9 @@ export const getSplitSeriesAccessorFnMap = ( export const isPercentileIdEqualToSeriesId = (columnId: number | string, seriesColumnId: string) => columnId.toString().split('.')[0] === seriesColumnId; -export const isValidSeriesForDimension = (seriesColumnId: string, { id, accessor }: Dimension) => - (id === seriesColumnId || isPercentileIdEqualToSeriesId(id ?? '', seriesColumnId)) && +export const isValidSeriesForDimension = ( + seriesColumnId: string, + { aggId, accessor }: Partial +) => + (aggId === seriesColumnId || isPercentileIdEqualToSeriesId(aggId ?? '', seriesColumnId)) && accessor !== null; diff --git a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx index e5a9074811539..4ae371ae9d446 100644 --- a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx +++ b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx @@ -83,7 +83,7 @@ export const renderAllSeries = ( type, }) => { const yAspects = aspects.y.filter(({ aggId, accessor }) => - isValidSeriesForDimension(paramId, { id: aggId, accessor }) + isValidSeriesForDimension(paramId, { aggId, accessor }) ); if (!show || !yAspects.length) { return null; From 938d689ddb11348aa076be5fc3bd4e4aa0d904c3 Mon Sep 17 00:00:00 2001 From: Diana Derevyankina Date: Mon, 27 Sep 2021 16:38:01 +0300 Subject: [PATCH 4/4] Replace partial aspect with whole aspect value --- src/plugins/vis_types/xy/public/utils/accessors.tsx | 5 +---- src/plugins/vis_types/xy/public/utils/render_all_series.tsx | 4 +--- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/plugins/vis_types/xy/public/utils/accessors.tsx b/src/plugins/vis_types/xy/public/utils/accessors.tsx index 89f1091350047..2b552c9f3f9cf 100644 --- a/src/plugins/vis_types/xy/public/utils/accessors.tsx +++ b/src/plugins/vis_types/xy/public/utils/accessors.tsx @@ -82,9 +82,6 @@ export const getSplitSeriesAccessorFnMap = ( export const isPercentileIdEqualToSeriesId = (columnId: number | string, seriesColumnId: string) => columnId.toString().split('.')[0] === seriesColumnId; -export const isValidSeriesForDimension = ( - seriesColumnId: string, - { aggId, accessor }: Partial -) => +export const isValidSeriesForDimension = (seriesColumnId: string, { aggId, accessor }: Aspect) => (aggId === seriesColumnId || isPercentileIdEqualToSeriesId(aggId ?? '', seriesColumnId)) && accessor !== null; diff --git a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx index 4ae371ae9d446..c248b3b86e42a 100644 --- a/src/plugins/vis_types/xy/public/utils/render_all_series.tsx +++ b/src/plugins/vis_types/xy/public/utils/render_all_series.tsx @@ -82,9 +82,7 @@ export const renderAllSeries = ( interpolate, type, }) => { - const yAspects = aspects.y.filter(({ aggId, accessor }) => - isValidSeriesForDimension(paramId, { aggId, accessor }) - ); + const yAspects = aspects.y.filter((aspect) => isValidSeriesForDimension(paramId, aspect)); if (!show || !yAspects.length) { return null; }