[8.x] [Lens] Disable Collapse by for metric chart when primary metric is not numeric (#216179) (#216787)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Lens] Disable Collapse by for metric chart when primary metric is
not numeric (#216179)](https://github.com/elastic/kibana/pull/216179)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Marco
Liberati","email":"dej611@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-04-02T09:58:24Z","message":"[Lens]
Disable Collapse by for metric chart when primary metric is not numeric
(#216179)\n\n## Summary\n\nFix part of #214593 \n\nThis PR fixes the
Metric chart side of the `Collapse by` problem.\nIn shorts:\n* The
`Collapse by` control disappears when the primary metric is
not\nnumeric\n * while the UI hides it the state still holds it.\n* the
`toExpression` is responsible to evaluate the current state and\nfind if
`collapseFn` is compatible before adding it\n* The `Collapse by` control
has moved into the Data section of the\npanel, aligning the Metric chart
to the rest\n\n<img width=\"1057\" alt=\"Screenshot 2025-04-01 at 18 57
31\"\nsrc=\"https://github.com/user-attachments/assets/1d441329-3611-4452-a40d-54ea25964166\"\n/>\n<img
width=\"1056\" alt=\"Screenshot 2025-04-01 at 18 57
12\"\nsrc=\"https://github.com/user-attachments/assets/66111c2a-957d-44dc-8361-45300df99662\"\n/>\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"1a555fdc864b40183dab5d83e9cc31a517b1b120","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","Feature:Lens","backport:version","v9.1.0","v8.19.0"],"title":"[Lens]
Disable Collapse by for metric chart when primary metric is not
numeric","number":216179,"url":"https://github.com/elastic/kibana/pull/216179","mergeCommit":{"message":"[Lens]
Disable Collapse by for metric chart when primary metric is not numeric
(#216179)\n\n## Summary\n\nFix part of #214593 \n\nThis PR fixes the
Metric chart side of the `Collapse by` problem.\nIn shorts:\n* The
`Collapse by` control disappears when the primary metric is
not\nnumeric\n * while the UI hides it the state still holds it.\n* the
`toExpression` is responsible to evaluate the current state and\nfind if
`collapseFn` is compatible before adding it\n* The `Collapse by` control
has moved into the Data section of the\npanel, aligning the Metric chart
to the rest\n\n<img width=\"1057\" alt=\"Screenshot 2025-04-01 at 18 57
31\"\nsrc=\"https://github.com/user-attachments/assets/1d441329-3611-4452-a40d-54ea25964166\"\n/>\n<img
width=\"1056\" alt=\"Screenshot 2025-04-01 at 18 57
12\"\nsrc=\"https://github.com/user-attachments/assets/66111c2a-957d-44dc-8361-45300df99662\"\n/>\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"1a555fdc864b40183dab5d83e9cc31a517b1b120"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/216179","number":216179,"mergeCommit":{"message":"[Lens]
Disable Collapse by for metric chart when primary metric is not numeric
(#216179)\n\n## Summary\n\nFix part of #214593 \n\nThis PR fixes the
Metric chart side of the `Collapse by` problem.\nIn shorts:\n* The
`Collapse by` control disappears when the primary metric is
not\nnumeric\n * while the UI hides it the state still holds it.\n* the
`toExpression` is responsible to evaluate the current state and\nfind if
`collapseFn` is compatible before adding it\n* The `Collapse by` control
has moved into the Data section of the\npanel, aligning the Metric chart
to the rest\n\n<img width=\"1057\" alt=\"Screenshot 2025-04-01 at 18 57
31\"\nsrc=\"https://github.com/user-attachments/assets/1d441329-3611-4452-a40d-54ea25964166\"\n/>\n<img
width=\"1056\" alt=\"Screenshot 2025-04-01 at 18 57
12\"\nsrc=\"https://github.com/user-attachments/assets/66111c2a-957d-44dc-8361-45300df99662\"\n/>\n\n###
Checklist\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios\n\n---------\n\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"1a555fdc864b40183dab5d83e9cc31a517b1b120"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
This commit is contained in:
Kibana Machine 2025-04-02 13:49:52 +02:00 committed by GitHub
parent 5814ace20b
commit f24bc22bfd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 267 additions and 30 deletions

View file

@ -21,9 +21,11 @@ const options = [
export function CollapseSetting({
value,
onChange,
display,
}: {
value: string;
onChange: (value: CollapseFunction) => void;
display?: 'rowCompressed' | 'columnCompressed';
}) {
return (
<>
@ -45,7 +47,7 @@ export function CollapseSetting({
</span>
</EuiToolTip>
}
display="rowCompressed"
display={display ?? 'rowCompressed'}
fullWidth
>
<EuiSelect

View file

@ -17,10 +17,12 @@ import { MetricVisualizationState } from './types';
import {
DimensionEditor,
DimensionEditorAdditionalSection,
DimensionEditorDataExtraComponent,
SupportingVisType,
} from './dimension_editor';
import { DatasourcePublicAPI } from '../..';
import { createMockFramePublicAPI } from '../../mocks';
import { GROUP_ID } from './constants';
// see https://github.com/facebook/jest/issues/4402#issuecomment-534516219
const expectCalledBefore = (mock1: jest.Mock, mock2: jest.Mock) =>
@ -401,11 +403,6 @@ describe('dimension editor', () => {
/>
);
const selectCollapseBy = async (collapseFn: string) => {
const collapseBySelect = screen.getByLabelText(/collapse by/i);
await userEvent.selectOptions(collapseBySelect, collapseFn);
};
const setMaxCols = async (maxCols: number) => {
const maxColsInput = screen.getByLabelText(/layout columns/i);
await userEvent.clear(maxColsInput);
@ -414,8 +411,18 @@ describe('dimension editor', () => {
return {
...rtlRender,
selectCollapseBy,
setMaxCols,
rerender: (newOverrides = {}) => {
rtlRender.rerender(
<DimensionEditor
{...props}
state={{ ...fullState, breakdownByAccessor: accessor }}
accessor={accessor}
setState={mockSetState}
{...newOverrides}
/>
);
},
};
}
@ -427,14 +434,6 @@ describe('dimension editor', () => {
expect(screen.queryByTestId(SELECTORS.BREAKDOWN_EDITOR)).toBeInTheDocument();
});
it('supports setting a collapse function', async () => {
const { selectCollapseBy } = renderBreakdownEditor();
const newCollapseFn = 'min';
await selectCollapseBy(newCollapseFn);
expect(mockSetState).toHaveBeenCalledWith({ ...fullState, collapseFn: newCollapseFn });
});
it('sets max columns', async () => {
const { setMaxCols } = renderBreakdownEditor();
await setMaxCols(1);
@ -450,6 +449,71 @@ describe('dimension editor', () => {
expect(mockSetState).toHaveBeenCalledWith(expect.objectContaining({ maxCols: 3 }))
);
});
describe('data section', () => {
function renderBreakdownEditorDataSection(overrides = {}) {
const rtlRender = render(
<DimensionEditorDataExtraComponent
{...props}
groupId={GROUP_ID.BREAKDOWN_BY}
state={{ ...fullState, breakdownByAccessor: accessor }}
accessor={accessor}
setState={mockSetState}
{...overrides}
/>
);
const selectCollapseBy = async (collapseFn: string) => {
const collapseBySelect = screen.getByLabelText(/collapse by/i);
await userEvent.selectOptions(collapseBySelect, collapseFn);
};
return {
...rtlRender,
selectCollapseBy,
rerender: (newOverrides = {}) => {
rtlRender.rerender(
<DimensionEditorDataExtraComponent
{...props}
groupId={GROUP_ID.BREAKDOWN_BY}
state={{ ...fullState, breakdownByAccessor: accessor }}
accessor={accessor}
setState={mockSetState}
{...newOverrides}
/>
);
},
};
}
it('supports setting a collapse function', async () => {
const { selectCollapseBy } = renderBreakdownEditorDataSection();
const newCollapseFn = 'min';
await selectCollapseBy(newCollapseFn);
expect(mockSetState).toHaveBeenCalledWith({ ...fullState, collapseFn: newCollapseFn });
});
it('should not display the collapse function if the primary metric is not numeric', async () => {
const { rerender, container } = renderBreakdownEditorDataSection({
datasource: getNonNumericDatasource(),
});
expect(container).toBeEmptyDOMElement();
// now rerender with a numeric metric
rerender({ datasource: props.datasource });
expect(screen.getByLabelText(/collapse by/i)).toBeInTheDocument();
});
it.each([[GROUP_ID.METRIC], [GROUP_ID.SECONDARY_METRIC], [GROUP_ID.MAX]])(
'should not render for other group types: %s',
async (groupId) => {
const { container } = renderBreakdownEditorDataSection({ groupId });
expect(container).toBeEmptyDOMElement();
}
);
});
});
describe('additional section', () => {

View file

@ -38,9 +38,10 @@ import {
import type { VisualizationDimensionEditorProps } from '../../types';
import { defaultNumberPaletteParams, defaultPercentagePaletteParams } from './palette_config';
import { DEFAULT_MAX_COLUMNS, getDefaultColor, showingBar } from './visualization';
import { CollapseSetting } from '../../shared_components/collapse_setting';
import { MetricVisualizationState } from './types';
import { metricIconsSet } from '../../shared_components/icon_set';
import { CollapseSetting } from '../../shared_components/collapse_setting';
import { GROUP_ID } from './constants';
export type SupportingVisType = 'none' | 'bar' | 'trendline';
@ -116,15 +117,6 @@ function BreakdownByEditor({ setState, state }: SubProps) {
onChange={({ target: { value } }) => handleMaxColsChange(value)}
/>
</EuiFormRow>
<CollapseSetting
value={state.collapseFn || ''}
onChange={(collapseFn) => {
setState({
...state,
collapseFn,
});
}}
/>
</>
);
}
@ -607,3 +599,26 @@ export function DimensionEditorAdditionalSection({
</div>
);
}
export function DimensionEditorDataExtraComponent({
groupId,
datasource,
state,
setState,
}: Omit<Props, 'paletteService'>) {
const { isNumeric: isMetricNumeric } = getAccessorType(datasource, state.metricAccessor);
if (!isMetricNumeric || groupId !== GROUP_ID.BREAKDOWN_BY) {
return null;
}
return (
<CollapseSetting
value={state.collapseFn || ''}
onChange={(collapseFn) => {
setState({
...state,
collapseFn,
});
}}
/>
);
}

View file

@ -128,7 +128,9 @@ export const toExpression = (
};
};
const collapseExpressionFunction = state.collapseFn
const canCollapseBy = state.collapseFn && isMetricNumeric;
const collapseExpressionFunction = canCollapseBy
? buildExpressionFunction<CollapseExpressionFunction>(
'lens_collapse',
getCollapseFnArguments()
@ -143,7 +145,7 @@ export const toExpression = (
secondaryPrefix: state.secondaryPrefix,
max: state.maxAccessor,
breakdownBy:
state.breakdownByAccessor && !state.collapseFn ? state.breakdownByAccessor : undefined,
state.breakdownByAccessor && !canCollapseBy ? state.breakdownByAccessor : undefined,
trendline: trendlineExpression ? [trendlineExpression] : [],
subtitle: state.subtitle ?? undefined,
progressDirection: showingBar(state)

View file

@ -122,6 +122,33 @@ describe('metric visualization', () => {
).toMatchSnapshot();
});
test('generates configuration with no collapseBy', () => {
const groups = visualization.getConfiguration({
state: { ...fullState, collapseFn: undefined },
layerId: fullState.layerId,
frame: mockFrameApi,
}).groups;
const breakdownGroup = groups.find(({ groupId }) => groupId === GROUP_ID.BREAKDOWN_BY);
expect(breakdownGroup?.accessors[0].triggerIconType).toBeUndefined();
});
test('generates configuration with no collapseBy if the primary metric is not numeric', () => {
const groups = visualization.getConfiguration({
state: fullState,
layerId: fullState.layerId,
frame: createMockFramePublicAPI({
activeData: generateActiveData([
{
id: 'first',
rows: Array(3).fill({ 'metric-col-id': 'test', 'max-metric-col-id': 100 }),
},
]),
}),
}).groups;
const breakdownGroup = groups.find(({ groupId }) => groupId === GROUP_ID.BREAKDOWN_BY);
expect(breakdownGroup?.accessors[0].triggerIconType).toBeUndefined();
});
test('color-by-value', () => {
expect(
visualization.getConfiguration({
@ -650,6 +677,7 @@ describe('metric visualization', () => {
datasourceLayers.first as jest.Mocked<DatasourcePublicAPI>
).getOperationForColumnId.mockReturnValue({
isStaticValue: true,
dataType: 'number',
} as OperationDescriptor);
const ast = visualization.toExpression(
@ -684,6 +712,79 @@ describe('metric visualization', () => {
datasourceLayers.first as jest.Mocked<DatasourcePublicAPI>
).getOperationForColumnId.mockClear();
});
it('builds breakdown by metric without collapse function if metric is not numeric', () => {
const mockDatasource = createMockDatasource();
mockDatasource.publicAPIMock.getMaxPossibleNumValues.mockReturnValue(maxPossibleNumValues);
mockDatasource.publicAPIMock.getOperationForColumnId.mockReturnValue({
isStaticValue: false,
dataType: 'string',
} as OperationDescriptor);
const newDatasourceLayers = {
first: mockDatasource.publicAPIMock,
};
const ast = visualization.toExpression(
{
...fullState,
// force a collapse fn
collapseFn: 'sum',
// Turning off an accessor to make sure it gets filtered out from the collapse arguments
secondaryMetricAccessor: undefined,
},
newDatasourceLayers
) as ExpressionAstExpression;
expect(ast.chain).toHaveLength(1);
expect(ast.chain[0]).toMatchInlineSnapshot(`
Object {
"arguments": Object {
"breakdownBy": Array [
"breakdown-col-id",
],
"color": Array [
"static-color",
],
"iconAlign": Array [
"left",
],
"inspectorTableId": Array [
"first",
],
"max": Array [
"max-metric-col-id",
],
"maxCols": Array [
5,
],
"metric": Array [
"metric-col-id",
],
"palette": Array [],
"secondaryPrefix": Array [
"extra-text",
],
"subtitle": Array [
"subtitle",
],
"titlesTextAlign": Array [
"left",
],
"trendline": Array [],
"valueFontSize": Array [
"default",
],
"valuesTextAlign": Array [
"right",
],
},
"function": "metricVis",
"type": "function",
}
`);
expect(ast.chain[0].arguments.breakdownBy).not.toBeUndefined();
});
});
it('incorporates datasource expression if provided', () => {

View file

@ -26,7 +26,11 @@ import {
UserMessage,
} from '../../types';
import { GROUP_ID, LENS_METRIC_ID } from './constants';
import { DimensionEditor, DimensionEditorAdditionalSection } from './dimension_editor';
import {
DimensionEditor,
DimensionEditorAdditionalSection,
DimensionEditorDataExtraComponent,
} from './dimension_editor';
import { Toolbar } from './toolbar';
import { generateId } from '../../id_generator';
import { toExpression } from './to_expression';
@ -88,6 +92,7 @@ const getMetricLayerConfiguration = (
};
const isBucketed = (op: OperationMetadata) => op.isBucketed;
const canCollapseBy = isMetricNumeric && props.state.collapseFn;
return {
groups: [
@ -178,7 +183,7 @@ const getMetricLayerConfiguration = (
? [
{
columnId: props.state.breakdownByAccessor,
triggerIconType: props.state.collapseFn ? ('aggregate' as const) : undefined,
triggerIconType: canCollapseBy ? ('aggregate' as const) : undefined,
},
]
: [],
@ -571,6 +576,10 @@ export const getMetricVisualization = ({
return <Toolbar {...props} />;
},
DimensionEditorDataExtraComponent(props) {
return <DimensionEditorDataExtraComponent {...props} />;
},
DimensionEditorComponent(props) {
return <DimensionEditor {...props} paletteService={paletteService} />;
},

View file

@ -14,6 +14,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const filterBar = getService('filterBar');
const retry = getService('retry');
const inspector = getService('inspector');
const find = getService('find');
const inspectorTrendlineData = [
['2015-09-19 06:00', '-'],
@ -352,7 +353,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await lens.goToTimeRange();
await lens.switchToVisualization('lnsLegacyMetric');
// await lens.clickLegacyMetric();
await lens.configureDimension({
dimension: 'lns-empty-dimension',
operation: 'average',
@ -371,5 +372,48 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const decimals = (value?.split(`.`)[1] || '').match(/(\d)+/)?.[0];
expect(decimals).have.length(3);
});
it('should disable collapse by when the primary metric is not numeric', async () => {
await visualize.navigateToNewVisualization();
await visualize.clickVisType('lens');
const N_TILES = 39;
await lens.switchToVisualization('lnsMetric', 'Metric');
await lens.configureDimension({
dimension: 'lnsMetric_primaryMetricDimensionPanel > lns-empty-dimension',
operation: 'average',
field: 'bytes',
});
await lens.configureDimension({
dimension: 'lnsMetric_breakdownByDimensionPanel > lns-empty-dimension',
operation: 'date_histogram',
field: '@timestamp',
keepOpen: true,
});
// test that there are 39 tiles now
expect(await lens.getMetricTiles()).to.have.length(N_TILES);
await find.clickByCssSelector(
'select[data-test-subj="indexPattern-collapse-by"] > option[value="sum"]'
);
// change the collapse by fn
await lens.closeDimensionEditor();
// check that the collapse by is applied to the chart
expect(await lens.getMetricTiles()).to.have.length(1);
// now change the metric to Last value of a string field
await lens.configureDimension({
dimension: 'lnsMetric_primaryMetricDimensionPanel > lns-dimensionTrigger',
operation: 'last_value',
field: 'ip',
});
// test that there are 39 tiles now
expect(await lens.getMetricTiles()).to.have.length(N_TILES);
});
});
}