github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Lens] Make top values work for custom numeric formatters (#124566)

Browse source 
* 🐛 Make top values work with custom formatter

* 🐛 Make custom formatter work with multi-terms formatter

* 🐛 simplify parentFormat logic and add more tests

* 👌 Integrate feedback

* ✨ Add migration for top values formatting

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Marco Liberati 2022-02-08 14:33:53 +01:00 • committed by GitHub
parent 28d8bc810b
commit 3f702c1fd4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 399 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

30
x-pack/plugins/lens/common/expressions/format_column/format_column.test.ts
View file

@ -159,6 +159,36 @@ describe('format_column', () => {
params: {
pattern: '0,0.00000',
},
pattern: '0,0.00000',
},
});
});
it('should support multi-fields formatters', async () => {
datatable.columns[0].meta.params = {
id: 'previousWrapper',
params: { id: 'myMultiFieldFormatter', paramsPerField: [{ id: 'number' }] },
};
const result = await fn(datatable, {
columnId: 'test',
format: 'number',
decimals: 5,
parentFormat: JSON.stringify({ id: 'wrapper', params: { wrapperParam: 123 } }),
});
expect(result.columns[0].meta.params).toEqual({
id: 'wrapper',
params: {
wrapperParam: 123,
id: 'number',
paramsPerField: [
{
id: 'number',
params: {
pattern: '0,0.00000',
},
pattern: '0,0.00000',
},
],
},
});
});

38
x-pack/plugins/lens/common/expressions/format_column/format_column_fn.ts
View file

@ -43,18 +43,48 @@ export const formatColumnFn: FormatColumnExpressionFunction['fn'] = (
const parentFormatId = parsedParentFormat.id;
const parentFormatParams = parsedParentFormat.params ?? {};
if (!parentFormatId) {
// Be careful here to check for undefined custom format
const isDuplicateParentFormatter = parentFormatId === col.meta.params?.id && format == null;
if (!parentFormatId || isDuplicateParentFormatter) {
return col;
}
if (format && supportedFormats[format]) {
const customParams = {
pattern: supportedFormats[format].decimalsToPattern(decimals),
};
// Some parent formatters are multi-fields and wrap the custom format into a "paramsPerField"
// property. Here the format is passed to this property to make it work properly
if (col.meta.params?.params?.paramsPerField?.length) {
return withParams(col, {
id: parentFormatId,
params: {
...col.meta.params?.params,
id: format,
...parentFormatParams,
// some wrapper formatters require params to be flatten out (i.e. terms) while others
// require them to be in the params property (i.e. ranges)
// so for now duplicate
paramsPerField: col.meta.params?.params?.paramsPerField.map(
(f: { id: string | undefined; params: Record<string, unknown> | undefined }) => ({
...f,
params: { ...f.params, ...customParams },
...customParams,
})
),
},
});
}
return withParams(col, {
id: parentFormatId,
params: {
...col.meta.params?.params,
id: format,
params: {
pattern: supportedFormats[format].decimalsToPattern(decimals),
},
// some wrapper formatters require params to be flatten out (i.e. terms) while others
// require them to be in the params property (i.e. ranges)
// so for now duplicate
...customParams,
params: customParams,
...parentFormatParams,
},
});

3
x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable/droppable.test.ts
View file

@ -2167,6 +2167,7 @@ describe('IndexPatternDimensionEditorPanel', () => {
},
orderDirection: 'desc',
size: 10,
parentFormat: { id: 'terms' },
},
},
col3: testState.layers.first.columns.col3,
@ -2257,6 +2258,7 @@ describe('IndexPatternDimensionEditorPanel', () => {
type: 'alphabetical',
},
orderDirection: 'desc',
parentFormat: { id: 'terms' },
size: 10,
},
},
@ -2267,6 +2269,7 @@ describe('IndexPatternDimensionEditorPanel', () => {
filter: undefined,
operationType: 'unique_count',
sourceField: 'src',
timeShift: undefined,
dataType: 'number',
params: undefined,
scale: 'ratio',

29
x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx
View file

@ -81,6 +81,11 @@ function isScriptedField(fieldName: string | IndexPatternField, indexPattern?: I
return fieldName.scripted;
}
// It is not always possible to know if there's a numeric field, so just ignore it for now
function getParentFormatter(params: Partial<TermsIndexPatternColumn['params']>) {
return { id: params.secondaryFields?.length ? 'multi_terms' : 'terms' };
}
const idPrefix = htmlIdGenerator()();
const DEFAULT_SIZE = 3;
// Elasticsearch limit
@ -124,9 +129,18 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
if (field && !isScriptedField(field)) {
secondaryFields.add(field.name);
}
return {
secondaryFields: [...secondaryFields].filter((f) => targetColumn.sourceField !== f),
// remove the sourceField
secondaryFields.delete(targetColumn.sourceField);
const secondaryFieldsList: string[] = [...secondaryFields];
const ret: Partial<TermsIndexPatternColumn['params']> = {
secondaryFields: secondaryFieldsList,
parentFormat: getParentFormatter({
...targetColumn.params,
secondaryFields: secondaryFieldsList,
}),
};
return ret;
},
canAddNewField: ({ targetColumn, sourceColumn, field, indexPattern }) => {
// first step: collect the fields from the targetColumn
@ -222,6 +236,7 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
orderDirection: existingMetricColumn ? 'desc' : 'asc',
otherBucket: !indexPattern.hasRestrictions,
missingBucket: false,
parentFormat: { id: 'terms' },
},
};
},
@ -289,6 +304,7 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
if ('format' in newParams && field.type !== 'number') {
delete newParams.format;
}
newParams.parentFormat = getParentFormatter(newParams);
return {
...oldColumn,
dataType: field.type as DataType,
@ -348,8 +364,9 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
updateLayer,
} = props;
const onFieldSelectChange = useCallback(
(fields) => {
(fields: string[]) => {
const column = layer.columns[columnId] as TermsIndexPatternColumn;
const secondaryFields = fields.length > 1 ? fields.slice(1) : undefined;
updateLayer({
...layer,
columns: {
@ -364,7 +381,11 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
),
params: {
...column.params,
secondaryFields: fields.length > 1 ? fields.slice(1) : undefined,
secondaryFields,
parentFormat: getParentFormatter({
...column.params,
secondaryFields,
}),
},
},
} as Record<string, TermsIndexPatternColumn>,

176
x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx
View file

@ -8,7 +8,7 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { shallow, mount } from 'enzyme';
import { EuiButtonGroup, EuiFieldNumber, EuiSelect, EuiSwitch } from '@elastic/eui';
import { EuiButtonGroup, EuiComboBox, EuiFieldNumber, EuiSelect, EuiSwitch } from '@elastic/eui';
import type {
IUiSettingsClient,
SavedObjectsClientContract,
@ -39,6 +39,16 @@ jest.mock('@elastic/eui', () => {
};
});
// Need to mock the debounce call to test some FieldInput behaviour
jest.mock('lodash', () => {
const original = jest.requireActual('lodash');
return {
...original,
debounce: (fn: unknown) => fn,
};
});
const uiSettingsMock = {} as IUiSettingsClient;
const defaultProps = {
@ -224,15 +234,18 @@ describe('terms', () => {
},
orderDirection: 'asc',
format: { id: 'number', params: { decimals: 0 } },
parentFormat: { id: 'terms' },
},
};
const indexPattern = createMockedIndexPattern();
const newStringField = indexPattern.fields.find((i) => i.name === 'source')!;
const newStringField = indexPattern.getFieldByName('source')!;
const column = termsOperation.onFieldChange(oldColumn, newStringField);
expect(column).toHaveProperty('dataType', 'string');
expect(column).toHaveProperty('sourceField', 'source');
expect(column.params.format).toBeUndefined();
// Preserve the parentFormat as it will be ignored down the line if not required
expect(column.params.parentFormat).toEqual({ id: 'terms' });
});
it('should remove secondary fields when a new field is passed', () => {
@ -253,7 +266,7 @@ describe('terms', () => {
},
};
const indexPattern = createMockedIndexPattern();
const newStringField = indexPattern.fields.find((i) => i.name === 'source')!;
const newStringField = indexPattern.getFieldByName('source')!;
const column = termsOperation.onFieldChange(oldColumn, newStringField);
expect(column.params.secondaryFields).toBeUndefined();
@ -277,13 +290,59 @@ describe('terms', () => {
},
};
const indexPattern = createMockedIndexPattern();
const sanemStringField = indexPattern.fields.find((i) => i.name === 'bytes')!;
const newNumericField = indexPattern.getFieldByName('bytes')!;
const column = termsOperation.onFieldChange(oldColumn, sanemStringField, {
const column = termsOperation.onFieldChange(oldColumn, newNumericField, {
secondaryFields: ['dest', 'geo.src'],
});
expect(column.params.secondaryFields).toEqual(expect.arrayContaining(['dest', 'geo.src']));
});
it('should reassign the parentFormatter on single field change', () => {
const oldColumn: TermsIndexPatternColumn = {
operationType: 'terms',
sourceField: 'bytes',
label: 'Top values of bytes',
isBucketed: true,
dataType: 'number',
params: {
size: 5,
orderBy: {
type: 'alphabetical',
},
orderDirection: 'asc',
format: { id: 'number', params: { decimals: 0 } },
},
};
const indexPattern = createMockedIndexPattern();
const newNumberField = indexPattern.getFieldByName('memory')!;
const column = termsOperation.onFieldChange(oldColumn, newNumberField);
expect(column.params.parentFormat).toEqual({ id: 'terms' });
});
it('should reassign the parentFormatter on multiple fields change', () => {
const oldColumn: TermsIndexPatternColumn = {
operationType: 'terms',
sourceField: 'bytes',
label: 'Top values of bytes',
isBucketed: true,
dataType: 'number',
params: {
size: 5,
orderBy: {
type: 'alphabetical',
},
orderDirection: 'asc',
format: { id: 'number', params: { decimals: 0 } },
},
};
const indexPattern = createMockedIndexPattern();
const newNumberField = indexPattern.getFieldByName('memory')!;
const column = termsOperation.onFieldChange(oldColumn, newNumberField);
expect(column.params.parentFormat).toEqual({ id: 'terms' });
});
});
describe('getPossibleOperationForField', () => {
@ -516,6 +575,23 @@ describe('terms', () => {
});
expect(termsColumn.params).toEqual(expect.objectContaining({ size: 5 }));
});
it('should set a parentFormat as "terms" if a numeric field is passed', () => {
const termsColumn = termsOperation.buildColumn({
indexPattern: createMockedIndexPattern(),
layer: { columns: {}, columnOrder: [], indexPatternId: '' },
field: {
aggregatable: true,
searchable: true,
type: 'number',
name: 'numericTest',
displayName: 'test',
},
});
expect(termsColumn.params).toEqual(
expect.objectContaining({ parentFormat: { id: 'terms' } })
);
});
});
describe('onOtherColumnChanged', () => {
@ -1364,6 +1440,47 @@ describe('terms', () => {
instance.find('[data-test-subj="indexPattern-terms-add-field"]').first().prop('isDisabled')
).toBeTruthy();
});
it('should update the parentFormatter on transition between single to multi terms', () => {
const updateLayerSpy = jest.fn();
const existingFields = getExistingFields();
const operationSupportMatrix = getDefaultOperationSupportMatrix('col1', existingFields);
let instance = mount(
<InlineFieldInput
{...defaultFieldInputProps}
layer={layer}
updateLayer={updateLayerSpy}
columnId="col1"
existingFields={existingFields}
operationSupportMatrix={operationSupportMatrix}
selectedColumn={layer.columns.col1 as TermsIndexPatternColumn}
/>
);
// add a new field
act(() => {
instance.find('[data-test-subj="indexPattern-terms-add-field"]').first().simulate('click');
});
instance = instance.update();
act(() => {
instance.find(EuiComboBox).last().prop('onChange')!([
{ value: { type: 'field', field: 'bytes' }, label: 'bytes' },
]);
});
expect(updateLayerSpy).toHaveBeenCalledWith(
expect.objectContaining({
columns: expect.objectContaining({
col1: expect.objectContaining({
params: expect.objectContaining({
parentFormat: { id: 'multi_terms' },
}),
}),
}),
})
);
});
});
describe('param editor', () => {
@ -2046,7 +2163,10 @@ describe('terms', () => {
sourceColumn: createMultiTermsColumn(['bytes', 'memory']),
indexPattern: defaultProps.indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['dest', 'bytes', 'memory']) });
).toEqual({
secondaryFields: expect.arrayContaining(['dest', 'bytes', 'memory']),
parentFormat: { id: 'multi_terms' },
});
});
it('should return existing multiterms with only new fields from source column', () => {
@ -2056,7 +2176,10 @@ describe('terms', () => {
sourceColumn: createMultiTermsColumn(['bytes', 'dest']),
indexPattern: defaultProps.indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['dest', 'bytes']) });
).toEqual({
secondaryFields: expect.arrayContaining(['dest', 'bytes']),
parentFormat: { id: 'multi_terms' },
});
});
it('should return existing multiterms with only multiple new fields from source column', () => {
@ -2066,7 +2189,10 @@ describe('terms', () => {
sourceColumn: createMultiTermsColumn(['dest', 'bytes', 'memory']),
indexPattern: defaultProps.indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['dest', 'bytes', 'memory']) });
).toEqual({
secondaryFields: expect.arrayContaining(['dest', 'bytes', 'memory']),
parentFormat: { id: 'multi_terms' },
});
});
it('should append field to multiterms', () => {
@ -2079,7 +2205,10 @@ describe('terms', () => {
field,
indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['bytes']) });
).toEqual({
secondaryFields: expect.arrayContaining(['bytes']),
parentFormat: { id: 'multi_terms' },
});
});
it('should not append scripted field to multiterms', () => {
@ -2092,7 +2221,7 @@ describe('terms', () => {
field,
indexPattern,
})
).toEqual({ secondaryFields: [] });
).toEqual({ secondaryFields: [], parentFormat: { id: 'terms' } });
});
it('should add both sourceColumn and field (as last term) to the targetColumn', () => {
@ -2105,7 +2234,10 @@ describe('terms', () => {
field,
indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['dest', 'memory', 'bytes']) });
).toEqual({
secondaryFields: expect.arrayContaining(['dest', 'memory', 'bytes']),
parentFormat: { id: 'multi_terms' },
});
});
it('should not add sourceColumn field if it has only scripted field', () => {
@ -2118,7 +2250,27 @@ describe('terms', () => {
field,
indexPattern,
})
).toEqual({ secondaryFields: expect.arrayContaining(['dest', 'bytes']) });
).toEqual({
secondaryFields: expect.arrayContaining(['dest', 'bytes']),
parentFormat: { id: 'multi_terms' },
});
});
it('should assign a parent formatter if a custom formatter is present', () => {
const indexPattern = createMockedIndexPattern();
const targetColumn = createMultiTermsColumn(['source', 'dest']);
targetColumn.params.format = { id: 'bytes', params: { decimals: 2 } };
expect(
termsOperation.getParamsForMultipleFields?.({
targetColumn,
sourceColumn: createMultiTermsColumn(['scripted']),
indexPattern,
})
).toEqual({
secondaryFields: expect.arrayContaining(['dest']),
parentFormat: { id: 'multi_terms' },
});
});
});
});

4
x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/types.ts
View file

@ -30,10 +30,6 @@ export interface TermsIndexPatternColumn extends FieldBasedIndexPatternColumn {
};
parentFormat?: {
id: string;
params?: {
id?: string;
template?: string;
};
};
};
}

31
x-pack/plugins/lens/server/migrations/common_migrations.ts
View file

@ -250,3 +250,34 @@ export const getLensFilterMigrations = (
state: { ...lensDoc.attributes.state, filters: migrate(lensDoc.attributes.state.filters) },
},
}));
export const fixLensTopValuesCustomFormatting = (attributes: LensDocShape810): LensDocShape810 => {
const newAttributes = cloneDeep(attributes);
const datasourceLayers = newAttributes.state.datasourceStates.indexpattern.layers || {};
(newAttributes as LensDocShape810).state.datasourceStates.indexpattern.layers =
Object.fromEntries(
Object.entries(datasourceLayers).map(([layerId, layer]) => {
return [
layerId,
{
...layer,
columns: Object.fromEntries(
Object.entries(layer.columns).map(([columnId, column]) => {
if (column.operationType === 'terms') {
return [
columnId,
{
...column,
params: { ...column.params, parentFormat: { id: 'terms' } },
},
];
}
return [columnId, column];
})
),
},
];
})
);
return newAttributes as LensDocShape810;
};

75
x-pack/plugins/lens/server/migrations/saved_object_migrations.test.ts
View file

@ -1704,4 +1704,79 @@ describe('Lens migrations', () => {
},
});
});
describe('8.1.0 add parentFormat to terms operation', () => {
const context = { log: { warning: () => {} } } as unknown as SavedObjectMigrationContext;
const example = {
type: 'lens',
id: 'mocked-saved-object-id',
attributes: {
savedObjectId: '1',
title: 'MyRenamedOps',
description: '',
visualizationType: null,
state: {
datasourceMetaData: {
filterableIndexPatterns: [],
},
datasourceStates: {
indexpattern: {
currentIndexPatternId: 'logstash-*',
layers: {
'2': {
columns: {
'3': {
dataType: 'string',
isBucketed: true,
label: 'Top values of geoip.country_iso_code',
operationType: 'terms',
params: {},
scale: 'ordinal',
sourceField: 'geoip.country_iso_code',
},
'4': {
label: 'Anzahl der Aufnahmen',
dataType: 'number',
operationType: 'count',
sourceField: 'Aufnahmen',
isBucketed: false,
scale: 'ratio',
},
'5': {
label: 'Sum of bytes',
dataType: 'numver',
operationType: 'sum',
sourceField: 'bytes',
isBucketed: false,
scale: 'ratio',
},
},
columnOrder: ['3', '4', '5'],
},
},
},
},
visualization: {},
query: { query: '', language: 'kuery' },
filters: [],
},
},
} as unknown as SavedObjectUnsanitizedDoc<LensDocShape810>;
it('should change field for count operations but not for others, not changing the vis', () => {
const result = migrations['8.1.0'](example, context) as ReturnType<
SavedObjectMigrationFn<LensDocShape, LensDocShape>
>;
expect(
Object.values(
result.attributes.state.datasourceStates.indexpattern.layers['2'].columns
).find(({ operationType }) => operationType === 'terms')
).toEqual(
expect.objectContaining({
params: expect.objectContaining({ parentFormat: { id: 'terms' } }),
})
);
});
});
});

8
x-pack/plugins/lens/server/migrations/saved_object_migrations.ts
View file

@ -39,6 +39,7 @@ import {
getLensFilterMigrations,
getLensCustomVisualizationMigrations,
commonRenameRecordsField,
fixLensTopValuesCustomFormatting,
} from './common_migrations';
interface LensDocShapePre710<VisualizationState = unknown> {
@ -458,6 +459,11 @@ const renameRecordsField: SavedObjectMigrationFn<LensDocShape810, LensDocShape81
return { ...newDoc, attributes: commonRenameRecordsField(newDoc.attributes) };
};
const addParentFormatter: SavedObjectMigrationFn<LensDocShape810, LensDocShape810> = (doc) => {
const newDoc = cloneDeep(doc);
return { ...newDoc, attributes: fixLensTopValuesCustomFormatting(newDoc.attributes) };
};
const lensMigrations: SavedObjectMigrationMap = {
'7.7.0': removeInvalidAccessors,
// The order of these migrations matter, since the timefield migration relies on the aggConfigs
@ -471,7 +477,7 @@ const lensMigrations: SavedObjectMigrationMap = {
'7.14.0': removeTimezoneDateHistogramParam,
'7.15.0': addLayerTypeToVisualization,
'7.16.0': moveDefaultReversedPaletteToCustom,
'8.1.0': flow(renameFilterReferences, renameRecordsField),
'8.1.0': flow(renameFilterReferences, renameRecordsField, addParentFormatter),
};
export const getAllMigrations = (

31
x-pack/plugins/lens/server/migrations/types.ts
View file

@ -200,9 +200,38 @@ export interface LensDocShape715<VisualizationState = unknown> {
export type LensDocShape810<VisualizationState = unknown> = Omit<
LensDocShape715<VisualizationState>,
'filters'
'filters' | 'state'
> & {
filters: Filter[];
state: Omit<LensDocShape715['state'], 'datasourceStates'> & {
datasourceStates: {
indexpattern: Omit<LensDocShape715['state']['datasourceStates']['indexpattern'], 'layers'> & {
layers: Record<
string,
Omit<
LensDocShape715['state']['datasourceStates']['indexpattern']['layers'][string],
'columns'
> & {
columns: Record<
string,
| {
operationType: 'terms';
params: {
secondaryFields?: string[];
};
[key: string]: unknown;
}
| {
operationType: OperationTypePost712;
params: Record<string, unknown>;
[key: string]: unknown;
}
>;
}
>;
};
};
};
};
export type VisState716 =