Skip to content

Commit

Permalink
Fixes problem with one chart plotted for multiple y axis when migrati…
Browse files Browse the repository at this point in the history
…ng from an old SO (#112972)

* Fixes problem with one chart plotted for multiple y axis when migrationg from an old SO

* Add unit tests

* Address PR comments

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
stratoula and kibanamachine authored Oct 6, 2021
1 parent 7c27822 commit fa59b52
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React, { useState, useEffect, useCallback, useMemo } from 'react';
import React, { useState, useEffect, useCallback } from 'react';
import { cloneDeep, get } from 'lodash';

import { EuiSpacer } from '@elastic/eui';
Expand All @@ -19,14 +19,14 @@ import { SeriesPanel } from './series_panel';
import { CategoryAxisPanel } from './category_axis_panel';
import { ValueAxesPanel } from './value_axes_panel';
import {
makeSerie,
isAxisHorizontal,
countNextAxisNumber,
getUpdatedAxisName,
mapPositionOpposite,
mapPosition,
mapPositionOpposingOpposite,
} from './utils';
import { getSeriesParams } from '../../../../utils/get_series_params';

export type SetParamByIndex = <P extends keyof ValueAxis, O extends keyof SeriesParam>(
axesName: 'valueAxes' | 'seriesParams',
Expand Down Expand Up @@ -273,40 +273,19 @@ function MetricsAxisOptions(props: ValidationVisOptionsProps<VisParams>) {
);

const schemaName = vis.type.schemas.metrics[0].name;
const metrics = useMemo(() => {
return aggs.bySchemaName(schemaName);
}, [schemaName, aggs]);

const firstValueAxesId = stateParams.valueAxes[0].id;

useEffect(() => {
const updatedSeries = metrics.map((agg) => {
const params = stateParams.seriesParams.find((param) => param.data.id === agg.id);
const label = agg.makeLabel();

// update labels for existing params or create new one
if (params) {
return {
...params,
data: {
...params.data,
label,
},
};
} else {
const series = makeSerie(
agg.id,
label,
firstValueAxesId,
stateParams.seriesParams[stateParams.seriesParams.length - 1]
);
return series;
}
});
const updatedSeries = getSeriesParams(
aggs,
stateParams.seriesParams,
schemaName,
firstValueAxesId
);

setValue('seriesParams', updatedSeries);
if (updatedSeries) setValue('seriesParams', updatedSeries);
updateAxisTitle(updatedSeries);
}, [metrics, firstValueAxesId, setValue, stateParams.seriesParams, updateAxisTitle]);
}, [firstValueAxesId, setValue, stateParams.seriesParams, updateAxisTitle, aggs, schemaName]);

return isTabSelected ? (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,7 @@ import { upperFirst } from 'lodash';

import { Position } from '@elastic/charts';

import { VisParams, ValueAxis, SeriesParam, ChartMode, InterpolationMode } from '../../../../types';
import { ChartType } from '../../../../../common';

export const makeSerie = (
id: string,
label: string,
defaultValueAxis: ValueAxis['id'],
lastSerie?: SeriesParam
): SeriesParam => {
const data = { id, label };
const defaultSerie = {
show: true,
mode: ChartMode.Normal,
type: ChartType.Line,
drawLinesBetweenPoints: true,
showCircles: true,
circlesRadius: 3,
interpolate: InterpolationMode.Linear,
lineWidth: 2,
valueAxis: defaultValueAxis,
data,
};
return lastSerie ? { ...lastSerie, data } : defaultSerie;
};
import { VisParams, ValueAxis } from '../../../../types';

export const isAxisHorizontal = (position: Position) =>
[Position.Top, Position.Bottom].includes(position as any);
Expand Down
22 changes: 22 additions & 0 deletions src/plugins/vis_types/xy/public/sample_vis.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1780,6 +1780,28 @@ export const sampleAreaVis = {
},
aggs: {
typesRegistry: {},
bySchemaName: () => [
{
id: '1',
enabled: true,
type: 'sum',
params: {
field: 'total_quantity',
},
schema: 'metric',
makeLabel: () => 'Total quantity',
toSerializedFieldFormat: () => ({
id: 'number',
params: {
parsedUrl: {
origin: 'http://localhost:5801',
pathname: '/app/visualize',
basePath: '',
},
},
}),
},
],
getResponseAggs: () => [
{
id: '1',
Expand Down
14 changes: 13 additions & 1 deletion src/plugins/vis_types/xy/public/to_ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { visName, VisTypeXyExpressionFunctionDefinition } from './expression_fun
import { XyVisType } from '../common';
import { getEsaggsFn } from './to_ast_esaggs';
import { TimeRangeBounds } from '../../../data/common';
import { getSeriesParams } from './utils/get_series_params';

const prepareLabel = (data: Labels) => {
const label = buildExpressionFunction('label', {
Expand Down Expand Up @@ -145,6 +146,17 @@ export const toExpressionAst: VisToExpressionAst<VisParams> = async (vis, params

const responseAggs = vis.data.aggs?.getResponseAggs().filter(({ enabled }) => enabled) ?? [];

const schemaName = vis.type.schemas?.metrics[0].name;
const firstValueAxesId = vis.params.valueAxes[0].id;
const updatedSeries = getSeriesParams(
vis.data.aggs,
vis.params.seriesParams,
schemaName,
firstValueAxesId
);

const finalSeriesParams = updatedSeries ?? vis.params.seriesParams;

if (dimensions.x) {
const xAgg = responseAggs[dimensions.x.accessor] as any;
if (xAgg.type.name === BUCKET_TYPES.DATE_HISTOGRAM) {
Expand Down Expand Up @@ -202,7 +214,7 @@ export const toExpressionAst: VisToExpressionAst<VisParams> = async (vis, params
orderBucketsBySum: vis.params.orderBucketsBySum,
categoryAxes: vis.params.categoryAxes.map(prepareCategoryAxis),
valueAxes: vis.params.valueAxes.map(prepareValueAxis),
seriesParams: vis.params.seriesParams.map(prepareSeriesParam),
seriesParams: finalSeriesParams.map(prepareSeriesParam),
labels: prepareLabel(vis.params.labels),
thresholdLine: prepareThresholdLine(vis.params.thresholdLine),
gridCategoryLines: vis.params.grid.categoryLines,
Expand Down
64 changes: 64 additions & 0 deletions src/plugins/vis_types/xy/public/utils/get_series_params.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type { AggConfigs } from '../../../../data/public';
import type { SeriesParam } from '../types';
import { getSeriesParams } from './get_series_params';
import { sampleAreaVis } from '../sample_vis.test.mocks';

describe('getSeriesParams', () => {
it('returns correct params', () => {
const seriesParams = getSeriesParams(
sampleAreaVis.data.aggs as unknown as AggConfigs,
sampleAreaVis.params.seriesParams as unknown as SeriesParam[],
'metric',
'ValueAxis-1'
);
expect(seriesParams).toStrictEqual([
{
circlesRadius: 5,
data: {
id: '1',
label: 'Total quantity',
},
drawLinesBetweenPoints: true,
interpolate: 'linear',
mode: 'stacked',
show: 'true',
showCircles: true,
type: 'area',
valueAxis: 'ValueAxis-1',
},
]);
});

it('returns default params if no params provided', () => {
const seriesParams = getSeriesParams(
sampleAreaVis.data.aggs as unknown as AggConfigs,
[],
'metric',
'ValueAxis-1'
);
expect(seriesParams).toStrictEqual([
{
circlesRadius: 3,
data: {
id: '1',
label: 'Total quantity',
},
drawLinesBetweenPoints: true,
interpolate: 'linear',
lineWidth: 2,
mode: 'normal',
show: true,
showCircles: true,
type: 'line',
valueAxis: 'ValueAxis-1',
},
]);
});
});
63 changes: 63 additions & 0 deletions src/plugins/vis_types/xy/public/utils/get_series_params.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { ValueAxis, SeriesParam, ChartMode, InterpolationMode } from '../types';
import { ChartType } from '../../common';
import type { AggConfigs } from '../../../../data/public';

const makeSerie = (
id: string,
label: string,
defaultValueAxis: ValueAxis['id'],
lastSerie?: SeriesParam
): SeriesParam => {
const data = { id, label };
const defaultSerie = {
show: true,
mode: ChartMode.Normal,
type: ChartType.Line,
drawLinesBetweenPoints: true,
showCircles: true,
circlesRadius: 3,
interpolate: InterpolationMode.Linear,
lineWidth: 2,
valueAxis: defaultValueAxis,
};
return { ...defaultSerie, ...lastSerie, data };
};
export const getSeriesParams = (
aggs: AggConfigs | undefined,
seriesParams: SeriesParam[],
schemaName: string,
firstValueAxesId: string
) => {
const metrics = aggs?.bySchemaName(schemaName);

return metrics?.map((agg) => {
const params = seriesParams.find((param) => param.data.id === agg.id);
const label = agg.makeLabel();

// update labels for existing params or create new one
if (params) {
return {
...params,
data: {
...params.data,
label,
},
};
} else {
const series = makeSerie(
agg.id,
label,
firstValueAxesId,
seriesParams[seriesParams.length - 1]
);
return series;
}
});
};
2 changes: 1 addition & 1 deletion src/plugins/vis_types/xy/public/vis_types/area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export const areaVisTypeDefinition = {
truncate: 100,
},
title: {
text: defaultCountLabel,
text: '',
},
style: {},
},
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_types/xy/public/vis_types/histogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export const histogramVisTypeDefinition = {
truncate: 100,
},
title: {
text: defaultCountLabel,
text: '',
},
style: {},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const horizontalBarVisTypeDefinition = {
truncate: 100,
},
title: {
text: defaultCountLabel,
text: '',
},
style: {},
},
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_types/xy/public/vis_types/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export const lineVisTypeDefinition = {
truncate: 100,
},
title: {
text: defaultCountLabel,
text: '',
},
style: {},
},
Expand Down

0 comments on commit fa59b52

Please sign in to comment.