Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickofthyme merged 1 commit into from
Feb 12, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions x-pack/plugins/lens/common/expressions/datatable/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,47 @@
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
*
* Excludes the following types:
* - `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 (
meta?.type === 'number' &&
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);
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2167,6 +2167,7 @@ describe('IndexPattern Data Source', () => {
scale: undefined,
sortingHint: undefined,
interval: undefined,
hasArraySupport: false,
} as OperationDescriptor);
});

Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/lens/public/datasources/form_based/form_based.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
Expand All @@ -1326,6 +1327,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -1406,6 +1408,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
Expand All @@ -1418,6 +1421,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -2255,6 +2259,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
Expand All @@ -2280,6 +2285,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -2332,6 +2338,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -2345,6 +2352,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -2411,6 +2419,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
Expand All @@ -2424,6 +2433,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -2437,6 +2447,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -2524,6 +2535,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
Expand All @@ -2537,6 +2549,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -2550,6 +2563,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -2660,6 +2674,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
Expand All @@ -2673,6 +2688,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -2686,6 +2702,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -3193,6 +3210,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
{
Expand All @@ -3205,6 +3223,7 @@ describe('IndexPattern Data Source suggestions', () => {
isStaticValue: false,
hasTimeShift: false,
hasReducedTimeRange: false,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -3274,6 +3293,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -3287,6 +3307,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
Expand All @@ -3300,6 +3321,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down Expand Up @@ -3367,6 +3389,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: 'auto',
hasArraySupport: false,
},
},
{
Expand All @@ -3380,6 +3403,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
{
Expand All @@ -3393,6 +3417,7 @@ describe('IndexPattern Data Source suggestions', () => {
hasTimeShift: false,
hasReducedTimeRange: false,
interval: undefined,
hasArraySupport: false,
},
},
],
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lens/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
62 changes: 35 additions & 27 deletions x-pack/plugins/lens/public/visualizations/metric/dimension_editor.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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',
Expand All @@ -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(),
Expand Down Expand Up @@ -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();
});

Expand All @@ -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();
Expand Down Expand Up @@ -571,6 +578,7 @@ describe('dimension editor', () => {
...props.datasource,
getOperationForColumnId: (id: string) => ({
hasReducedTimeRange: id === stateWOTrend.metricAccessor,
dataType: 'number',
}),
},
});
Expand All @@ -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();
});
Expand Down
Loading