Skip to content

Commit

Permalink
keep metric accessors in datasource order
Browse files Browse the repository at this point in the history
  • Loading branch information
@drewdaemon
drewdaemon committed Feb 23, 2023
1 parent e950ce5 commit a8fac3e
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 106 deletions.
24 changes: 14 additions & 10 deletions x-pack/plugins/lens/public/visualizations/partition/to_expression.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ export const getColumnToLabelMap = (
return columnToLabel;
};

export const getSortedGroups = (
export const getSortedAccessorsForGroup = (
datasource: DatasourcePublicAPI | undefined,
layer: PieLayerState,
accessor: 'primaryGroups' | 'secondaryGroups' = 'primaryGroups'
accessor: 'primaryGroups' | 'secondaryGroups' | 'metrics'
) => {
const originalOrder = datasource
?.getTableSpec()
Expand Down Expand Up @@ -174,15 +174,17 @@ const generateCommonArguments = (
datasourceLayers: DatasourceLayers,
paletteService: PaletteRegistry
) => {
const columnToLabelMap = getColumnToLabelMap(layer.metrics, datasourceLayers[layer.layerId]);
const datasource = datasourceLayers[layer.layerId];
const columnToLabelMap = getColumnToLabelMap(layer.metrics, datasource);
const sortedMetricAccessors = getSortedAccessorsForGroup(datasource, layer, 'metrics');

return {
labels: generateCommonLabelsAstArgs(state, attributes, layer, columnToLabelMap),
buckets: operations
.filter(({ columnId }) => !isCollapsed(columnId, layer))
.map(({ columnId }) => columnId)
.map(prepareDimension),
metrics: (layer.allowMultipleMetrics ? layer.metrics : [layer.metrics[0]]).map(
metrics: (layer.allowMultipleMetrics ? sortedMetricAccessors : [sortedMetricAccessors[0]]).map(
prepareDimension
),
metricsToLabels: JSON.stringify(columnToLabelMap),
Expand Down Expand Up @@ -290,16 +292,18 @@ function expressionHelper(
const layer = state.layers[0];
const datasource = datasourceLayers[layer.layerId];

const groups = Array.from(
const accessors = Array.from(
new Set(
[
getSortedGroups(datasource, layer, 'primaryGroups'),
layer.secondaryGroups ? getSortedGroups(datasource, layer, 'secondaryGroups') : [],
getSortedAccessorsForGroup(datasource, layer, 'primaryGroups'),
layer.secondaryGroups
? getSortedAccessorsForGroup(datasource, layer, 'secondaryGroups')
: [],
].flat()
)
);

const operations = groups
const operations = accessors
.map((columnId) => ({
columnId,
operation: datasource?.getOperationForColumnId(columnId) as Operation | null,
Expand All @@ -323,11 +327,11 @@ function expressionHelper(
type: 'expression',
chain: [
...(datasourceAst ? datasourceAst.chain : []),
...groups
...accessors
.filter((columnId) => layer.collapseFns?.[columnId])
.map((columnId) => {
return buildExpressionFunction<CollapseExpressionFunction>('lens_collapse', {
by: groups.filter((chk) => chk !== columnId),
by: accessors.filter((chk) => chk !== columnId),
metric: layer.metrics,
fn: [layer.collapseFns![columnId]!],
}).toAst();
Expand Down
230 changes: 142 additions & 88 deletions x-pack/plugins/lens/public/visualizations/partition/visualization.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,119 +448,173 @@ describe('pie_visualization', () => {
});
});

it("doesn't count collapsed columns toward the dimension limits", () => {
const colIds = new Array(PartitionChartsMeta.pie.maxBuckets)
.fill(undefined)
.map((_, i) => String(i + 1));

const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));
it('orders metric accessors by datasource column order', () => {
const colIds = ['1', '2', '3', '4'];

const state = getExampleState();
state.layers[0].primaryGroups = colIds;

const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state: _state,
frame,
layerId: state.layers[0].layerId,
});

expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeFalsy();
state.layers[0].metrics = colIds;
state.layers[0].allowMultipleMetrics = true;

const stateWithCollapsed = cloneDeep(state);
stateWithCollapsed.layers[0].collapseFns = { '1': 'sum' };
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
// reverse the column IDs in the datasource
colIds.reverse().map((id) => ({ columnId: id, fields: [] }));

// this is to make sure the accessors get sorted before palette colors are applied
const palette = paletteServiceMock.get('default');
palette.getCategoricalColor
.mockReturnValueOnce('color 1')
.mockReturnValueOnce('color 2')
.mockReturnValueOnce('color 3')
.mockReturnValueOnce('color 4');

const config = pieVisualization.getConfiguration({
state,
frame,
layerId: state.layers[0].layerId,
});

expect(findPrimaryGroup(getConfig(stateWithCollapsed))?.supportsMoreColumns).toBeTruthy();
expect(findMetricGroup(config)?.accessors).toMatchInlineSnapshot(`
Array [
Object {
"color": "color 1",
"columnId": "4",
"triggerIconType": "color",
},
Object {
"color": "color 2",
"columnId": "3",
"triggerIconType": "color",
},
Object {
"color": "color 3",
"columnId": "2",
"triggerIconType": "color",
},
Object {
"color": "color 4",
"columnId": "1",
"triggerIconType": "color",
},
]
`);
});

it('counts multiple metrics toward the dimension limits when not mosaic', () => {
const colIds = new Array(PartitionChartsMeta.pie.maxBuckets - 1)
.fill(undefined)
.map((_, i) => String(i + 1));
describe('dimension limits', () => {
it("doesn't count collapsed columns toward the dimension limits", () => {
const colIds = new Array(PartitionChartsMeta.pie.maxBuckets)
.fill(undefined)
.map((_, i) => String(i + 1));

const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));

const state = getExampleState();
state.layers[0].primaryGroups = colIds;
state.layers[0].allowMultipleMetrics = true;
const state = getExampleState();
state.layers[0].primaryGroups = colIds;

const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state: _state,
frame,
layerId: state.layers[0].layerId,
});
const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state: _state,
frame,
layerId: state.layers[0].layerId,
});

expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeTruthy();
expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeFalsy();

const stateWithMultipleMetrics = cloneDeep(state);
stateWithMultipleMetrics.layers[0].metrics.push('1', '2');
const stateWithCollapsed = cloneDeep(state);
stateWithCollapsed.layers[0].collapseFns = { '1': 'sum' };

expect(
findPrimaryGroup(getConfig(stateWithMultipleMetrics))?.supportsMoreColumns
).toBeFalsy();
});
expect(findPrimaryGroup(getConfig(stateWithCollapsed))?.supportsMoreColumns).toBeTruthy();
});

it('does NOT count multiple metrics toward the dimension limits when mosaic', () => {
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () => [];
it('counts multiple metrics toward the dimension limits when not mosaic', () => {
const colIds = new Array(PartitionChartsMeta.pie.maxBuckets - 1)
.fill(undefined)
.map((_, i) => String(i + 1));

const state = getExampleState();
state.shape = 'mosaic';
state.layers[0].primaryGroups = [];
state.layers[0].allowMultipleMetrics = false; // always true for mosaic
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));

const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state: _state,
frame,
layerId: state.layers[0].layerId,
});
const state = getExampleState();
state.layers[0].primaryGroups = colIds;
state.layers[0].allowMultipleMetrics = true;

expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeTruthy();
const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state: _state,
frame,
layerId: state.layers[0].layerId,
});

const stateWithMultipleMetrics = cloneDeep(state);
stateWithMultipleMetrics.layers[0].metrics.push('1', '2');
expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeTruthy();

expect(
findPrimaryGroup(getConfig(stateWithMultipleMetrics))?.supportsMoreColumns
).toBeTruthy();
});
const stateWithMultipleMetrics = cloneDeep(state);
stateWithMultipleMetrics.layers[0].metrics.push('1', '2');

it('reports too many metric dimensions if multiple not enabled', () => {
const colIds = ['1', '2', '3', '4'];
expect(
findPrimaryGroup(getConfig(stateWithMultipleMetrics))?.supportsMoreColumns
).toBeFalsy();
});

const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));
it('does NOT count multiple metrics toward the dimension limits when mosaic', () => {
const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () => [];

const state = getExampleState();
state.layers[0].metrics = colIds;
state.layers[0].allowMultipleMetrics = false;
expect(
findMetricGroup(
pieVisualization.getConfiguration({
state,
frame,
layerId: state.layers[0].layerId,
})
)?.dimensionsTooMany
).toBe(3);
const state = getExampleState();
state.shape = 'mosaic';
state.layers[0].primaryGroups = [];
state.layers[0].allowMultipleMetrics = false; // always true for mosaic

state.layers[0].allowMultipleMetrics = true;
expect(
findMetricGroup(
const getConfig = (_state: PieVisualizationState) =>
pieVisualization.getConfiguration({
state,
state: _state,
frame,
layerId: state.layers[0].layerId,
})
)?.dimensionsTooMany
).toBe(0);
});

expect(findPrimaryGroup(getConfig(state))?.supportsMoreColumns).toBeTruthy();

const stateWithMultipleMetrics = cloneDeep(state);
stateWithMultipleMetrics.layers[0].metrics.push('1', '2');

expect(
findPrimaryGroup(getConfig(stateWithMultipleMetrics))?.supportsMoreColumns
).toBeTruthy();
});

it('reports too many metric dimensions if multiple not enabled', () => {
const colIds = ['1', '2', '3', '4'];

const frame = mockFrame();
frame.datasourceLayers[LAYER_ID]!.getTableSpec = () =>
colIds.map((id) => ({ columnId: id, fields: [] }));

const state = getExampleState();
state.layers[0].metrics = colIds;
state.layers[0].allowMultipleMetrics = false;
expect(
findMetricGroup(
pieVisualization.getConfiguration({
state,
frame,
layerId: state.layers[0].layerId,
})
)?.dimensionsTooMany
).toBe(3);

state.layers[0].allowMultipleMetrics = true;
expect(
findMetricGroup(
pieVisualization.getConfiguration({
state,
frame,
layerId: state.layers[0].layerId,
})
)?.dimensionsTooMany
).toBe(0);
});
});

it.each(Object.values(PieChartTypes).filter((type) => type !== 'mosaic'))(
Expand Down
Loading

0 comments on commit a8fac3e

Please sign in to comment.