Skip to content

Commit c011c98

Browse files
dej611stratoula
andcommitted
[Lens] Relax counter field checks for saved visualizations with unsupported operations (elastic#163515)
## Summary Fix elastic#163473 This PR relaxes a bit the checks on the Lens side for old/saved visualizations with unsupported operations for the `counter` field type, while preserving those checks for newer visualizations. Dashboards with "meaningless" operations will now show a warning message: <img width="556" alt="Screenshot 2023-08-09 at 18 31 21" src="https://github.com/elastic/kibana/assets/924948/7c8f3739-4957-4d1d-8aaa-e9457b8a4426"> When in editor the warning is shown at the top-right corner as well: <img width="845" alt="Screenshot 2023-08-09 at 18 30 31" src="https://github.com/elastic/kibana/assets/924948/c52a7823-d4b9-4efd-9c5d-ca654f3f03a0"> New visualizations still prevent the user from using the unsupported operations: <img width="410" alt="Screenshot 2023-08-09 at 18 30 55" src="https://github.com/elastic/kibana/assets/924948/d2364a01-0dc3-409a-9c0b-3e3a77cb2566"> <img width="848" alt="Screenshot 2023-08-09 at 18 31 48" src="https://github.com/elastic/kibana/assets/924948/086a7360-6b1a-40a2-90d9-f4e8c7bf3f3a"> There's theoretically a last case where users in old SOs might create a new metric dimension trying to force to use a unsupported operation for a counter field: in this case the logic for a "new" visualization will kick-in, clean the data in the workspace and show a full error. Cancelling such metric dimension will lead to the previous "relaxed" state. Messages are grouped by field and by top referencing column (i.e. a formula): this means that if a formula uses the same `counter` field with two different dimensions (i.e. `sum(counter_field) + median(counter_field)` as `myFormula`) will show up as a single column (`myFormula`). The wording of the message mimics the same documentation copy provided in the ES documentation. Ref: elastic/elasticsearch#97974 Testing SO: [export.ndjson.txt](https://github.com/elastic/kibana/files/12304924/export.ndjson.txt) ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Stratoula Kalafateli <[email protected]>
1 parent a686ec9 commit c011c98

File tree

4 files changed

+326
-19
lines changed

4 files changed

+326
-19
lines changed

x-pack/plugins/lens/public/datasources/form_based/form_based.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import {
7171
isColumnInvalid,
7272
cloneLayer,
7373
getNotifiableFeatures,
74+
getUnsupportedOperationsWarningMessage,
7475
} from './utils';
7576
import { getUniqueLabelGenerator, isDraggedDataViewField } from '../../utils';
7677
import { hasField, normalizeOperationDataType } from './pure_utils';
@@ -891,6 +892,7 @@ export function getFormBasedDatasource({
891892
core.docLinks,
892893
setState
893894
),
895+
...getUnsupportedOperationsWarningMessage(state, frameDatasourceAPI, core.docLinks),
894896
];
895897

896898
const infoMessages = getNotifiableFeatures(state, frameDatasourceAPI, visualizationInfo);

x-pack/plugins/lens/public/datasources/form_based/operations/definitions/metrics.tsx

-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ function buildMetricOperation<T extends MetricColumn<string>>({
126126
newField &&
127127
supportedTypes.includes(newField.type) &&
128128
newField.aggregatable &&
129-
isTimeSeriesCompatible(type, newField.timeSeriesMetric) &&
130129
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
131130
);
132131
},

x-pack/plugins/lens/public/datasources/form_based/utils.test.tsx

+198-2
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,20 @@
88
import React from 'react';
99
import { shallow } from 'enzyme';
1010
import { createDatatableUtilitiesMock } from '@kbn/data-plugin/common/mocks';
11-
import { getPrecisionErrorWarningMessages, cloneLayer } from './utils';
11+
import {
12+
getPrecisionErrorWarningMessages,
13+
cloneLayer,
14+
getUnsupportedOperationsWarningMessage,
15+
} from './utils';
1216
import type { FormBasedPrivateState, GenericIndexPatternColumn } from './types';
13-
import type { FramePublicAPI } from '../../types';
17+
import type { FramePublicAPI, IndexPattern } from '../../types';
1418
import type { DocLinksStart } from '@kbn/core/public';
1519
import { EuiLink } from '@elastic/eui';
1620
import { TermsIndexPatternColumn } from './operations';
1721
import { mountWithIntl } from '@kbn/test-jest-helpers';
1822
import { FormattedMessage } from '@kbn/i18n-react';
1923
import { FormBasedLayer } from './types';
24+
import { createMockedIndexPatternWithAdditionalFields } from './mocks';
2025

2126
describe('indexpattern_datasource utils', () => {
2227
describe('getPrecisionErrorWarningMessages', () => {
@@ -240,4 +245,195 @@ describe('indexpattern_datasource utils', () => {
240245
).toMatchSnapshot();
241246
});
242247
});
248+
249+
describe('getUnsupportedOperationsWarningMessage', () => {
250+
let docLinks: DocLinksStart;
251+
const affectedOperations = [
252+
'sum',
253+
'average',
254+
'percentile',
255+
'percentile_rank',
256+
'count',
257+
'unique_count',
258+
'standard_deviation',
259+
];
260+
261+
function createColumnsForField(field: string, colOffset: number = 0) {
262+
return Object.fromEntries(
263+
affectedOperations.map((operationType, i) => [
264+
`col_${i + colOffset}`,
265+
{ operationType, sourceField: field, label: `${operationType} of ${field}` },
266+
])
267+
);
268+
}
269+
270+
function createState(fields: string[]) {
271+
return {
272+
layers: {
273+
id: {
274+
indexPatternId: '0',
275+
columns: Object.assign(
276+
{},
277+
...fields.map((field, i) =>
278+
createColumnsForField(field, i * affectedOperations.length)
279+
)
280+
),
281+
},
282+
},
283+
} as unknown as FormBasedPrivateState;
284+
}
285+
286+
function createFramePublic(indexPattern: IndexPattern): FramePublicAPI {
287+
return {
288+
dataViews: {
289+
indexPatterns: Object.fromEntries([indexPattern].map((dataView, i) => [i, dataView])),
290+
},
291+
} as unknown as FramePublicAPI;
292+
}
293+
294+
function createFormulaColumns(formulaParts: string[], field: string, colOffset: number = 0) {
295+
const fullFormula = formulaParts.map((part) => `${part}(${field})`).join(' + ');
296+
// just assume it's a sum of all the parts for testing
297+
const rootId = `col-formula${colOffset}`;
298+
return Object.fromEntries([
299+
[
300+
rootId,
301+
{
302+
operationType: 'formula',
303+
label: `Formula: ${fullFormula}`,
304+
params: { formula: fullFormula },
305+
},
306+
],
307+
...formulaParts.map((part, i) => [
308+
`${rootId}X${i}`,
309+
{ operationType: part, sourceField: field, label: 'Part of formula' },
310+
]),
311+
[
312+
`${rootId}X${formulaParts.length}`,
313+
{ operationType: 'math', references: formulaParts.map((_, i) => `${rootId}X${i}`) },
314+
],
315+
]);
316+
}
317+
318+
beforeEach(() => {
319+
docLinks = {
320+
links: {
321+
fleet: {
322+
datastreamsTSDSMetrics: 'http://tsdb_metric_doc',
323+
},
324+
},
325+
} as DocLinksStart;
326+
});
327+
328+
it.each([['bytes'], ['bytes_gauge']])(
329+
'should return no warning for non-counter fields: %s',
330+
(fieldName: string) => {
331+
const warnings = getUnsupportedOperationsWarningMessage(
332+
createState([fieldName]),
333+
createFramePublic(
334+
createMockedIndexPatternWithAdditionalFields([
335+
{
336+
name: 'bytes_gauge',
337+
displayName: 'bytes_gauge',
338+
type: 'number',
339+
aggregatable: true,
340+
searchable: true,
341+
timeSeriesMetric: 'gauge',
342+
},
343+
])
344+
),
345+
docLinks
346+
);
347+
expect(warnings).toHaveLength(0);
348+
}
349+
);
350+
351+
it('should return a warning for a counter field grouped by field', () => {
352+
const warnings = getUnsupportedOperationsWarningMessage(
353+
createState(['bytes_counter']),
354+
createFramePublic(
355+
createMockedIndexPatternWithAdditionalFields([
356+
{
357+
name: 'bytes_counter',
358+
displayName: 'bytes_counter',
359+
type: 'number',
360+
aggregatable: true,
361+
searchable: true,
362+
timeSeriesMetric: 'counter',
363+
},
364+
])
365+
),
366+
docLinks
367+
);
368+
expect(warnings).toHaveLength(1);
369+
});
370+
371+
it('should group multiple warnings by field', () => {
372+
const warnings = getUnsupportedOperationsWarningMessage(
373+
createState(['bytes_counter', 'bytes_counter2']),
374+
createFramePublic(
375+
createMockedIndexPatternWithAdditionalFields([
376+
{
377+
name: 'bytes_counter',
378+
displayName: 'bytes_counter',
379+
type: 'number',
380+
aggregatable: true,
381+
searchable: true,
382+
timeSeriesMetric: 'counter',
383+
},
384+
{
385+
name: 'bytes_counter2',
386+
displayName: 'bytes_counter2',
387+
type: 'number',
388+
aggregatable: true,
389+
searchable: true,
390+
timeSeriesMetric: 'counter',
391+
},
392+
])
393+
),
394+
docLinks
395+
);
396+
expect(warnings).toHaveLength(2);
397+
});
398+
399+
it('should handle formula reporting only the top visible dimension', () => {
400+
const warnings = getUnsupportedOperationsWarningMessage(
401+
{
402+
layers: {
403+
id: {
404+
indexPatternId: '0',
405+
columns: Object.assign(
406+
{},
407+
...['bytes_counter', 'bytes_counter2'].map((field, i) =>
408+
createFormulaColumns(affectedOperations, field, i * affectedOperations.length)
409+
)
410+
),
411+
},
412+
},
413+
} as unknown as FormBasedPrivateState,
414+
createFramePublic(
415+
createMockedIndexPatternWithAdditionalFields([
416+
{
417+
name: 'bytes_counter',
418+
displayName: 'bytes_counter',
419+
type: 'number',
420+
aggregatable: true,
421+
searchable: true,
422+
timeSeriesMetric: 'counter',
423+
},
424+
{
425+
name: 'bytes_counter2',
426+
displayName: 'bytes_counter2',
427+
type: 'number',
428+
aggregatable: true,
429+
searchable: true,
430+
timeSeriesMetric: 'counter',
431+
},
432+
])
433+
),
434+
docLinks
435+
);
436+
expect(warnings).toHaveLength(2);
437+
});
438+
});
243439
});

0 commit comments

Comments
 (0)