[8.17] [Lens] Restore dynamic colouring by value for Last value agg (#209110) (#210878)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Lens] Restore dynamic colouring by value for Last value agg
(#209110)](https://github.com/elastic/kibana/pull/209110)

<!--- Backport version: 9.6.4 -->

### 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-02-11T14:55:35Z","message":"[Lens]
Restore dynamic colouring by value for Last value agg (#209110)\n\n##
Summary\n\nFixes #208924\n\nThis PR improves the numeric check for the
Last value agg within the\nMetric chart type avoiding completely to
access the active data and\nrather rely on the datasource
configuration.\nThe new utility function in fact won't rely any more on
active data\nrather on the Lens configuration itself, which is more
robust, faster\nand flexible.\n\n<img width=\"2552\" alt=\"Screenshot
2025-01-31 at 14 30
12\"\nsrc=\"https://github.com/user-attachments/assets/5f8792db-40ff-497b-8e2f-0737c2932f92\"\n/>\n\n\n###
Notes for testing\n\nI've created a testing dashboard with all the
possible combinations of\ncolouring for metric and
tables.\n\n\n[last_value_dashboard.ndjson.txt](https://github.com/user-attachments/files/18618905/last_value_dashboard.ndjson.txt)\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## Release
note\n\nThis fixes an issue where dynamic colouring has been disabled
from Last\nvalue aggregation
types.","sha":"abba6675e29b28b91daea1f46d2ff48363ab78ab","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Visualizations","Feature:Lens","backport
missing","backport:prev-major","v9.1.0"],"title":"[Lens] Restore dynamic
colouring by value for Last value
agg","number":209110,"url":"https://github.com/elastic/kibana/pull/209110","mergeCommit":{"message":"[Lens]
Restore dynamic colouring by value for Last value agg (#209110)\n\n##
Summary\n\nFixes #208924\n\nThis PR improves the numeric check for the
Last value agg within the\nMetric chart type avoiding completely to
access the active data and\nrather rely on the datasource
configuration.\nThe new utility function in fact won't rely any more on
active data\nrather on the Lens configuration itself, which is more
robust, faster\nand flexible.\n\n<img width=\"2552\" alt=\"Screenshot
2025-01-31 at 14 30
12\"\nsrc=\"https://github.com/user-attachments/assets/5f8792db-40ff-497b-8e2f-0737c2932f92\"\n/>\n\n\n###
Notes for testing\n\nI've created a testing dashboard with all the
possible combinations of\ncolouring for metric and
tables.\n\n\n[last_value_dashboard.ndjson.txt](https://github.com/user-attachments/files/18618905/last_value_dashboard.ndjson.txt)\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## Release
note\n\nThis fixes an issue where dynamic colouring has been disabled
from Last\nvalue aggregation
types.","sha":"abba6675e29b28b91daea1f46d2ff48363ab78ab"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209110","number":209110,"mergeCommit":{"message":"[Lens]
Restore dynamic colouring by value for Last value agg (#209110)\n\n##
Summary\n\nFixes #208924\n\nThis PR improves the numeric check for the
Last value agg within the\nMetric chart type avoiding completely to
access the active data and\nrather rely on the datasource
configuration.\nThe new utility function in fact won't rely any more on
active data\nrather on the Lens configuration itself, which is more
robust, faster\nand flexible.\n\n<img width=\"2552\" alt=\"Screenshot
2025-01-31 at 14 30
12\"\nsrc=\"https://github.com/user-attachments/assets/5f8792db-40ff-497b-8e2f-0737c2932f92\"\n/>\n\n\n###
Notes for testing\n\nI've created a testing dashboard with all the
possible combinations of\ncolouring for metric and
tables.\n\n\n[last_value_dashboard.ndjson.txt](https://github.com/user-attachments/files/18618905/last_value_dashboard.ndjson.txt)\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## Release
note\n\nThis fixes an issue where dynamic colouring has been disabled
from Last\nvalue aggregation
types.","sha":"abba6675e29b28b91daea1f46d2ff48363ab78ab"}}]}]
BACKPORT-->

Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
This commit is contained in:
Nick Partridge 2025-02-12 11:12:27 -07:00 committed by GitHub
parent 244d3a654c
commit 6a61db01b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 133 additions and 53 deletions

View file

@ -8,6 +8,20 @@
import { type Datatable, type DatatableColumnMeta } from '@kbn/expressions-plugin/common';
import { getOriginalId } from '@kbn/transpose-utils';
/**
* Make sure to specifically check for "top_hits" when looking for array values
*
* **Note**: use this utility function only at the expression level,
* not before (i.e. to decide if a column in numeric in a configuration panel)
*/
function isLastValueWithoutArraySupport(meta: DatatableColumnMeta): boolean {
return (
meta.sourceParams?.type !== 'filtered_metric' ||
(meta.sourceParams?.params as { customMetric: { type: 'top_hits' | 'top_metrics' } })
?.customMetric?.type !== 'top_hits'
);
}
/**
* Returns true for numerical fields
*
@ -15,7 +29,10 @@ import { getOriginalId } from '@kbn/transpose-utils';
* - `range` - Stringified range
* - `multi_terms` - Multiple values
* - `filters` - Arbitrary label
* - `filtered_metric` - Array of values
* - Last value with array values
*
* **Note**: use this utility function only at the expression level,
* not before (i.e. to decide if a column in numeric in a configuration panel)
*/
export function isNumericField(meta?: DatatableColumnMeta): boolean {
return (
@ -23,12 +40,15 @@ export function isNumericField(meta?: DatatableColumnMeta): boolean {
meta.params?.id !== 'range' &&
meta.params?.id !== 'multi_terms' &&
meta.sourceParams?.type !== 'filters' &&
meta.sourceParams?.type !== 'filtered_metric'
isLastValueWithoutArraySupport(meta)
);
}
/**
* Returns true for numerical fields, excluding ranges
*
* **Note**: use this utility function only at the expression level,
* not before (i.e. to decide if a column in numeric in a configuration panel)
*/
export function isNumericFieldForDatatable(table: Datatable | undefined, accessor: string) {
const meta = getFieldMetaFromDatatable(table, accessor);

View file

@ -2167,6 +2167,7 @@ describe('IndexPattern Data Source', () => {
scale: undefined,
sortingHint: undefined,
interval: undefined,
hasArraySupport: false,
} as OperationDescriptor);
});

View file

@ -177,6 +177,9 @@ export function columnToOperation(
interval: isColumnOfType<DateHistogramIndexPatternColumn>('date_histogram', column)
? column.params.interval
: undefined,
hasArraySupport:
isColumnOfType<LastValueIndexPatternColumn>('last_value', column) &&
column.params.showArrayValues,
};
}

View file

@ -1314,6 +1314,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
@ -1326,6 +1327,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
@ -1406,6 +1408,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
@ -1418,6 +1421,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
@ -2255,6 +2259,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
@ -2280,6 +2285,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
@ -2332,6 +2338,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -2345,6 +2352,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
@ -2411,6 +2419,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
@ -2424,6 +2433,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -2437,6 +2447,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
@ -2524,6 +2535,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
@ -2537,6 +2549,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -2550,6 +2563,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
@ -2660,6 +2674,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
@ -2673,6 +2688,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -2686,6 +2702,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
@ -3193,6 +3210,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
@ -3205,6 +3223,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
@ -3274,6 +3293,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -3287,6 +3307,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
@ -3300,6 +3321,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
@ -3367,6 +3389,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
@ -3380,6 +3403,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
@ -3393,6 +3417,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],

View file

@ -756,6 +756,8 @@ export interface OperationMetadata {
// document and an aggregated metric which might be handy in some cases. Once we
// introduce a raw document datasource, this should be considered here.
isStaticValue?: boolean;
// Extra metadata to infer array support in an operation
hasArraySupport?: boolean;
}
/**

View file

@ -20,7 +20,7 @@ import {
SupportingVisType,
} from './dimension_editor';
import { DatasourcePublicAPI } from '../..';
import { createMockFramePublicAPI, generateActiveData } from '../../mocks';
import { createMockFramePublicAPI } from '../../mocks';
// see https://github.com/facebook/jest/issues/4402#issuecomment-534516219
const expectCalledBefore = (mock1: jest.Mock, mock2: jest.Mock) =>
@ -71,22 +71,29 @@ describe('dimension editor', () => {
trendlineBreakdownByAccessor: 'trendline-breakdown-col-id',
};
const nonNumericMetricFrame = createMockFramePublicAPI({
activeData: generateActiveData([
{
id: 'first',
rows: Array(3).fill({
'metric-col-id': faker.lorem.word(3),
'max-col-id': faker.random.number(),
}),
},
]),
});
let props: VisualizationDimensionEditorProps<MetricVisualizationState> & {
paletteService: PaletteRegistry;
};
const getNonNumericDatasource = () =>
({
hasDefaultTimeField: jest.fn(() => true),
getOperationForColumnId: jest.fn(() => ({
hasReducedTimeRange: false,
dataType: 'keyword',
})),
} as unknown as DatasourcePublicAPI);
const getNumericDatasourceWithArraySupport = () =>
({
hasDefaultTimeField: jest.fn(() => true),
getOperationForColumnId: jest.fn(() => ({
hasReducedTimeRange: false,
dataType: 'number',
hasArraySupport: true,
})),
} as unknown as DatasourcePublicAPI);
beforeEach(() => {
props = {
layerId: 'first',
@ -97,21 +104,12 @@ describe('dimension editor', () => {
hasDefaultTimeField: jest.fn(() => true),
getOperationForColumnId: jest.fn(() => ({
hasReducedTimeRange: false,
dataType: 'number',
})),
} as unknown as DatasourcePublicAPI,
removeLayer: jest.fn(),
addLayer: jest.fn(),
frame: createMockFramePublicAPI({
activeData: generateActiveData([
{
id: 'first',
rows: Array(3).fill({
'metric-col-id': faker.random.number(),
'secondary-metric-col-id': faker.random.number(),
}),
},
]),
}),
frame: createMockFramePublicAPI(),
setState: jest.fn(),
panelRef: {} as React.MutableRefObject<HTMLDivElement | null>,
paletteService: chartPluginMock.createPaletteRegistry(),
@ -177,7 +175,16 @@ describe('dimension editor', () => {
});
it('Color mode switch is not shown when the primary metric is non-numeric', () => {
const { colorModeGroup } = renderPrimaryMetricEditor({ frame: nonNumericMetricFrame });
const { colorModeGroup } = renderPrimaryMetricEditor({
datasource: getNonNumericDatasource(),
});
expect(colorModeGroup).not.toBeInTheDocument();
});
it('Color mode switch is not shown when the primary metric is numeric but with array support', () => {
const { colorModeGroup } = renderPrimaryMetricEditor({
datasource: getNumericDatasourceWithArraySupport(),
});
expect(colorModeGroup).not.toBeInTheDocument();
});
@ -196,7 +203,7 @@ describe('dimension editor', () => {
});
it('is visible when metric is non-numeric even if palette is set', () => {
const { staticColorPicker } = renderPrimaryMetricEditor({
frame: nonNumericMetricFrame,
datasource: getNonNumericDatasource(),
state: { ...metricAccessorState, palette },
});
expect(staticColorPicker).toBeInTheDocument();
@ -571,6 +578,7 @@ describe('dimension editor', () => {
...props.datasource,
getOperationForColumnId: (id: string) => ({
hasReducedTimeRange: id === stateWOTrend.metricAccessor,
dataType: 'number',
}),
},
});
@ -579,7 +587,7 @@ describe('dimension editor', () => {
it('should not show a trendline button group when primary metric dimension is non-numeric', () => {
const { container } = renderAdditionalSectionEditor({
frame: nonNumericMetricFrame,
datasource: getNonNumericDatasource(),
});
expect(container).toBeEmptyDOMElement();
});

View file

@ -30,7 +30,6 @@ import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils';
import { css } from '@emotion/react';
import { DebouncedInput, IconSelect } from '@kbn/visualization-ui-components';
import { useDebouncedValue } from '@kbn/visualization-utils';
import { isNumericFieldForDatatable } from '../../../common/expressions/datatable/utils';
import { applyPaletteParams, PalettePanelContainer } from '../../shared_components';
import type { VisualizationDimensionEditorProps } from '../../types';
import { defaultNumberPaletteParams, defaultPercentagePaletteParams } from './palette_config';
@ -38,6 +37,7 @@ import { DEFAULT_MAX_COLUMNS, getDefaultColor, showingBar } from './visualizatio
import { CollapseSetting } from '../../shared_components/collapse_setting';
import { MetricVisualizationState } from './types';
import { metricIconsSet } from '../../shared_components/icon_set';
import { isMetricNumericType } from './helpers';
export type SupportingVisType = 'none' | 'bar' | 'trendline';
@ -212,23 +212,21 @@ function SecondaryMetricEditor({ accessor, idPrefix, frame, layerId, setState, s
function PrimaryMetricEditor(props: SubProps) {
const { state, setState, frame, accessor, idPrefix, isInlineEditing } = props;
const currentData = frame.activeData?.[state.layerId];
const isMetricNumeric = isNumericFieldForDatatable(currentData, accessor);
if (accessor == null) {
return null;
}
const hasDynamicColoring = Boolean(isMetricNumeric && state?.palette);
const isMetricNumeric = isMetricNumericType(props.datasource, accessor);
const hasDynamicColoring = Boolean(isMetricNumeric && state.palette);
const supportsPercentPalette = Boolean(
state.maxAccessor ||
(state.breakdownByAccessor && !state.collapseFn) ||
state?.palette?.params?.rangeType === 'percent'
state.palette?.params?.rangeType === 'percent'
);
const activePalette = state?.palette || {
const activePalette = state.palette || {
type: 'palette',
name: (supportsPercentPalette ? defaultPercentagePaletteParams : defaultNumberPaletteParams)
.name,
@ -313,7 +311,9 @@ function PrimaryMetricEditor(props: SubProps) {
/>
</EuiFormRow>
)}
{!hasDynamicColoring && <StaticColorControls {...props} />}
{!hasDynamicColoring && (
<StaticColorControls state={state} setState={setState} isMetricNumeric={isMetricNumeric} />
)}
{hasDynamicColoring && (
<EuiFormRow
display="columnCompressed"
@ -367,15 +367,11 @@ function PrimaryMetricEditor(props: SubProps) {
function StaticColorControls({
state,
setState,
frame,
}: Pick<Props, 'state' | 'setState' | 'frame'>) {
isMetricNumeric,
}: Pick<Props, 'state' | 'setState'> & { isMetricNumeric: boolean }) {
const colorLabel = i18n.translate('xpack.lens.metric.color', {
defaultMessage: 'Color',
});
const currentData = frame.activeData?.[state.layerId];
const isMetricNumeric = Boolean(
state.metricAccessor && isNumericFieldForDatatable(currentData, state.metricAccessor)
);
const setColor = useCallback(
(color: string) => {
@ -420,8 +416,8 @@ export function DimensionEditorAdditionalSection({
}: VisualizationDimensionEditorProps<MetricVisualizationState>) {
const { euiTheme } = useEuiTheme();
const currentData = frame.activeData?.[state.layerId];
if (accessor !== state.metricAccessor || !isNumericFieldForDatatable(currentData, accessor)) {
const isMetricNumeric = isMetricNumericType(datasource, accessor);
if (accessor !== state.metricAccessor || !isMetricNumeric) {
return null;
}

View file

@ -0,0 +1,26 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { DatasourcePublicAPI } from '../../types';
/**
* Infer the numeric type of a metric column purely on the configuration
*/
export function isMetricNumericType(
datasource: DatasourcePublicAPI | undefined,
accessor: string | undefined
) {
// No accessor means it's not a numeric type by default
if (!accessor || !datasource) {
return false;
}
const operation = datasource.getOperationForColumnId(accessor);
const isNumericTypeFromOperation = Boolean(
operation?.dataType === 'number' && !operation.hasArraySupport
);
return isNumericTypeFromOperation;
}

View file

@ -21,6 +21,7 @@ import { showingBar } from './metric_visualization';
import { DEFAULT_MAX_COLUMNS, getDefaultColor } from './visualization';
import { MetricVisualizationState } from './types';
import { metricStateDefaults } from './constants';
import { isMetricNumericType } from './helpers';
// TODO - deduplicate with gauges?
function computePaletteParams(params: CustomPaletteParams) {
@ -93,10 +94,7 @@ export const toExpression = (
const datasource = datasourceLayers[state.layerId];
const datasourceExpression = datasourceExpressionsByLayers[state.layerId];
const isMetricNumeric = Boolean(
state.metricAccessor &&
datasource?.getOperationForColumnId(state.metricAccessor)?.dataType === 'number'
);
const isMetricNumeric = isMetricNumericType(datasource, state.metricAccessor);
const maxPossibleTiles =
// if there's a collapse function, no need to calculate since we're dealing with a single tile
state.breakdownByAccessor && !state.collapseFn

View file

@ -33,6 +33,7 @@ import { toExpression } from './to_expression';
import { nonNullable } from '../../utils';
import { METRIC_NUMERIC_MAX } from '../../user_messages_ids';
import { MetricVisualizationState } from './types';
import { isMetricNumericType } from './helpers';
export const DEFAULT_MAX_COLUMNS = 3;
@ -653,9 +654,9 @@ export const getMetricVisualization = ({
const hasStaticColoring = !!state.color;
const hasDynamicColoring = !!state.palette;
const currentData = frame?.activeData?.[state.layerId];
const isMetricNumeric = Boolean(
state.metricAccessor && isNumericFieldForDatatable(currentData, state.metricAccessor)
const isMetricNumeric = isMetricNumericType(
frame?.datasourceLayers[state.layerId],
state.metricAccessor
);
return {