Skip to content

Commit

Permalink
[Lens] Relax counter field checks for saved visualizations with unsup…
Browse files Browse the repository at this point in the history
…ported 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 <efstratia.kalafateli@elastic.co>
(cherry picked from commit 4c812e3)
  • Loading branch information
dej611 committed Aug 14, 2023
1 parent 7955df4 commit 5ffbc26
Show file tree
Hide file tree
Showing 4 changed files with 326 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import {
isColumnInvalid,
cloneLayer,
getNotifiableFeatures,
getUnsupportedOperationsWarningMessage,
} from './utils';
import { getUniqueLabelGenerator, isDraggedDataViewField } from '../../utils';
import { hasField, normalizeOperationDataType } from './pure_utils';
Expand Down Expand Up @@ -891,6 +892,7 @@ export function getFormBasedDatasource({
core.docLinks,
setState
),
...getUnsupportedOperationsWarningMessage(state, frameDatasourceAPI, core.docLinks),
];

const infoMessages = getNotifiableFeatures(state, frameDatasourceAPI, visualizationInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ function buildMetricOperation<T extends MetricColumn<string>>({
newField &&
supportedTypes.includes(newField.type) &&
newField.aggregatable &&
isTimeSeriesCompatible(type, newField.timeSeriesMetric) &&
(!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
);
},
Expand Down
200 changes: 198 additions & 2 deletions x-pack/plugins/lens/public/datasources/form_based/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,20 @@
import React from 'react';
import { shallow } from 'enzyme';
import { createDatatableUtilitiesMock } from '@kbn/data-plugin/common/mocks';
import { getPrecisionErrorWarningMessages, cloneLayer } from './utils';
import {
getPrecisionErrorWarningMessages,
cloneLayer,
getUnsupportedOperationsWarningMessage,
} from './utils';
import type { FormBasedPrivateState, GenericIndexPatternColumn } from './types';
import type { FramePublicAPI } from '../../types';
import type { FramePublicAPI, IndexPattern } from '../../types';
import type { DocLinksStart } from '@kbn/core/public';
import { EuiLink } from '@elastic/eui';
import { TermsIndexPatternColumn } from './operations';
import { mountWithIntl } from '@kbn/test-jest-helpers';
import { FormattedMessage } from '@kbn/i18n-react';
import { FormBasedLayer } from './types';
import { createMockedIndexPatternWithAdditionalFields } from './mocks';

describe('indexpattern_datasource utils', () => {
describe('getPrecisionErrorWarningMessages', () => {
Expand Down Expand Up @@ -240,4 +245,195 @@ describe('indexpattern_datasource utils', () => {
).toMatchSnapshot();
});
});

describe('getUnsupportedOperationsWarningMessage', () => {
let docLinks: DocLinksStart;
const affectedOperations = [
'sum',
'average',
'percentile',
'percentile_rank',
'count',
'unique_count',
'standard_deviation',
];

function createColumnsForField(field: string, colOffset: number = 0) {
return Object.fromEntries(
affectedOperations.map((operationType, i) => [
`col_${i + colOffset}`,
{ operationType, sourceField: field, label: `${operationType} of ${field}` },
])
);
}

function createState(fields: string[]) {
return {
layers: {
id: {
indexPatternId: '0',
columns: Object.assign(
{},
...fields.map((field, i) =>
createColumnsForField(field, i * affectedOperations.length)
)
),
},
},
} as unknown as FormBasedPrivateState;
}

function createFramePublic(indexPattern: IndexPattern): FramePublicAPI {
return {
dataViews: {
indexPatterns: Object.fromEntries([indexPattern].map((dataView, i) => [i, dataView])),
},
} as unknown as FramePublicAPI;
}

function createFormulaColumns(formulaParts: string[], field: string, colOffset: number = 0) {
const fullFormula = formulaParts.map((part) => `${part}(${field})`).join(' + ');
// just assume it's a sum of all the parts for testing
const rootId = `col-formula${colOffset}`;
return Object.fromEntries([
[
rootId,
{
operationType: 'formula',
label: `Formula: ${fullFormula}`,
params: { formula: fullFormula },
},
],
...formulaParts.map((part, i) => [
`${rootId}X${i}`,
{ operationType: part, sourceField: field, label: 'Part of formula' },
]),
[
`${rootId}X${formulaParts.length}`,
{ operationType: 'math', references: formulaParts.map((_, i) => `${rootId}X${i}`) },
],
]);
}

beforeEach(() => {
docLinks = {
links: {
fleet: {
datastreamsTSDSMetrics: 'http://tsdb_metric_doc',
},
},
} as DocLinksStart;
});

it.each([['bytes'], ['bytes_gauge']])(
'should return no warning for non-counter fields: %s',
(fieldName: string) => {
const warnings = getUnsupportedOperationsWarningMessage(
createState([fieldName]),
createFramePublic(
createMockedIndexPatternWithAdditionalFields([
{
name: 'bytes_gauge',
displayName: 'bytes_gauge',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'gauge',
},
])
),
docLinks
);
expect(warnings).toHaveLength(0);
}
);

it('should return a warning for a counter field grouped by field', () => {
const warnings = getUnsupportedOperationsWarningMessage(
createState(['bytes_counter']),
createFramePublic(
createMockedIndexPatternWithAdditionalFields([
{
name: 'bytes_counter',
displayName: 'bytes_counter',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'counter',
},
])
),
docLinks
);
expect(warnings).toHaveLength(1);
});

it('should group multiple warnings by field', () => {
const warnings = getUnsupportedOperationsWarningMessage(
createState(['bytes_counter', 'bytes_counter2']),
createFramePublic(
createMockedIndexPatternWithAdditionalFields([
{
name: 'bytes_counter',
displayName: 'bytes_counter',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'counter',
},
{
name: 'bytes_counter2',
displayName: 'bytes_counter2',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'counter',
},
])
),
docLinks
);
expect(warnings).toHaveLength(2);
});

it('should handle formula reporting only the top visible dimension', () => {
const warnings = getUnsupportedOperationsWarningMessage(
{
layers: {
id: {
indexPatternId: '0',
columns: Object.assign(
{},
...['bytes_counter', 'bytes_counter2'].map((field, i) =>
createFormulaColumns(affectedOperations, field, i * affectedOperations.length)
)
),
},
},
} as unknown as FormBasedPrivateState,
createFramePublic(
createMockedIndexPatternWithAdditionalFields([
{
name: 'bytes_counter',
displayName: 'bytes_counter',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'counter',
},
{
name: 'bytes_counter2',
displayName: 'bytes_counter2',
type: 'number',
aggregatable: true,
searchable: true,
timeSeriesMetric: 'counter',
},
])
),
docLinks
);
expect(warnings).toHaveLength(2);
});
});
});
Loading

0 comments on commit 5ffbc26

Please sign in to comment.