Skip to content

Commit

Permalink
[Lens] Relax break down group validation for percentage charts (elast…
Browse files Browse the repository at this point in the history 
…ic#114803)

* 🐛 Improved percentage checks for breakdown group

* ✅ Add tests

* 👌 Integrate feedback

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@dej611 @kibanamachine
dej611 and kibanamachine authored Oct 21, 2021
1 parent 1b3f2cd commit 6a80d9a
Show file tree
Hide file tree
Showing 2 changed files with 244 additions and 1 deletion.
208 changes: 208 additions & 0 deletions x-pack/plugins/lens/public/xy_visualization/visualization.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,214 @@ describe('xy_visualization', () => {
expect(ops.filter(filterOperations).map((x) => x.dataType)).toEqual(['number']);
});

describe('breakdown group: percentage chart checks', () => {
const baseState = exampleState();

it('should require break down group with one accessor + one split accessor configuration', () => {
const [, , splitGroup] = xyVisualization.getConfiguration({
state: {
...baseState,
layers: [
{ ...baseState.layers[0], accessors: ['a'], seriesType: 'bar_percentage_stacked' },
],
},
frame,
layerId: 'first',
}).groups;
expect(splitGroup.required).toBe(true);
});

test.each([
[
'multiple accessors on the same layer',
[
{
...baseState.layers[0],
splitAccessor: undefined,
seriesType: 'bar_percentage_stacked',
},
],
],
[
'multiple accessors spread on compatible layers',
[
{
...baseState.layers[0],
accessors: ['a'],
splitAccessor: undefined,
seriesType: 'bar_percentage_stacked',
},
{
...baseState.layers[0],
splitAccessor: undefined,
xAccessor: 'd',
accessors: ['e'],
seriesType: 'bar_percentage_stacked',
},
],
],
] as Array<[string, State['layers']]>)(
'should not require break down group for %s',
(_, layers) => {
const [, , splitGroup] = xyVisualization.getConfiguration({
state: { ...baseState, layers },
frame,
layerId: 'first',
}).groups;
expect(splitGroup.required).toBe(false);
}
);

it.each([
[
'one accessor only',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
splitAccessor: undefined,
xAccessor: undefined,
},
],
],
[
'one accessor only with split accessor',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
xAccessor: undefined,
},
],
],
[
'one accessor only with xAccessor',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
splitAccessor: undefined,
},
],
],
[
'multiple accessors spread on incompatible layers (different xAccessor)',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
splitAccessor: undefined,
},
{
...baseState.layers[0],
accessors: ['e'],
seriesType: 'bar_percentage_stacked',
splitAccessor: undefined,
xAccessor: undefined,
},
],
],
[
'multiple accessors spread on incompatible layers (different splitAccessor)',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
},
{
...baseState.layers[0],
accessors: ['e'],
seriesType: 'bar_percentage_stacked',
splitAccessor: undefined,
xAccessor: undefined,
},
],
],
[
'multiple accessors spread on incompatible layers (different seriesType)',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
},
{
...baseState.layers[0],
accessors: ['e'],
seriesType: 'bar',
},
],
],
[
'one data layer with one accessor + one reference layer',
[
{
...baseState.layers[0],
accessors: ['a'],
seriesType: 'bar_percentage_stacked',
},
{
...baseState.layers[0],
accessors: ['e'],
seriesType: 'bar_percentage_stacked',
layerType: layerTypes.REFERENCELINE,
},
],
],

[
'multiple accessors on the same layers with different axis assigned',
[
{
...baseState.layers[0],
splitAccessor: undefined,
seriesType: 'bar_percentage_stacked',
yConfig: [
{ forAccessor: 'a', axisMode: 'left' },
{ forAccessor: 'b', axisMode: 'right' },
],
},
],
],
[
'multiple accessors spread on multiple layers with different axis assigned',
[
{
...baseState.layers[0],
accessors: ['a'],
xAccessor: undefined,
splitAccessor: undefined,
seriesType: 'bar_percentage_stacked',
yConfig: [{ forAccessor: 'a', axisMode: 'left' }],
},
{
...baseState.layers[0],
accessors: ['b'],
xAccessor: undefined,
splitAccessor: undefined,
seriesType: 'bar_percentage_stacked',
yConfig: [{ forAccessor: 'b', axisMode: 'right' }],
},
],
],
] as Array<[string, State['layers']]>)(
'should require break down group for %s',
(_, layers) => {
const [, , splitGroup] = xyVisualization.getConfiguration({
state: { ...baseState, layers },
frame,
layerId: 'first',
}).groups;
expect(splitGroup.required).toBe(true);
}
);
});

describe('reference lines', () => {
beforeEach(() => {
frame.datasourceLayers = {
Expand Down
37 changes: 36 additions & 1 deletion x-pack/plugins/lens/public/xy_visualization/visualization.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
checkXAccessorCompatibility,
getAxisName,
} from './visualization_helpers';
import { groupAxesByType } from './axes_configuration';

const defaultIcon = LensIconChartBarStacked;
const defaultSeriesType = 'bar_stacked';
Expand Down Expand Up @@ -378,6 +379,40 @@ export const getXyVisualization = ({
};
}

const { left, right } = groupAxesByType([layer], frame.activeData);
// Check locally if it has one accessor OR one accessor per axis
const layerHasOnlyOneAccessor = Boolean(
layer.accessors.length < 2 ||
(left.length && left.length < 2) ||
(right.length && right.length < 2)
);
// Check also for multiple layers that can stack for percentage charts
// Make sure that if multiple dimensions are defined for a single layer, they should belong to the same axis
const hasOnlyOneAccessor =
layerHasOnlyOneAccessor &&
getLayersByType(state, layerTypes.DATA).filter(
// check that the other layers are compatible with this one
(dataLayer) => {
if (
dataLayer.seriesType === layer.seriesType &&
Boolean(dataLayer.xAccessor) === Boolean(layer.xAccessor) &&
Boolean(dataLayer.splitAccessor) === Boolean(layer.splitAccessor)
) {
const { left: localLeft, right: localRight } = groupAxesByType(
[dataLayer],
frame.activeData
);
// return true only if matching axis are found
return (
dataLayer.accessors.length &&
(Boolean(localLeft.length) === Boolean(left.length) ||
Boolean(localRight.length) === Boolean(right.length))
);
}
return false;
}
).length < 2;

return {
groups: [
{
Expand Down Expand Up @@ -417,7 +452,7 @@ export const getXyVisualization = ({
filterOperations: isBucketed,
supportsMoreColumns: !layer.splitAccessor,
dataTestSubj: 'lnsXY_splitDimensionPanel',
required: layer.seriesType.includes('percentage'),
required: layer.seriesType.includes('percentage') && hasOnlyOneAccessor,
enableDimensionEditor: true,
},
],
Expand Down

0 comments on commit 6a80d9a

Please sign in to comment.