Skip to content

Commit

Permalink
[Controls] Add numeric options list (#172106)
Browse files Browse the repository at this point in the history
Closes #143587
Closes #126795

## Summary

This PR adds support for numeric options lists - i.e. options list
controls that are created with a numeric field. In order to make this
possible, I had to add support for fields to have multiple, overlapping
compatible control types (however, it is currently only number fields
where this applies). When selecting a field that is compatible with
multiple control types, the user is given the option to select which
control type they want, like so:

<p align="center"><img alt="GIF of multiple control types being
available for a single field"
src="https://github.com/elastic/kibana/assets/8698078/9ebb6795-1206-47c4-aaef-32437d27cf59"/></p>

> [!NOTE]
> This system currently defaults to options list controls, since these
are the most common - this is backed up by our telemetry, which shows
that (in the last 30 days), clusters with at least one options list
control occured **ten times more often** than clusters with at least one
range slider control. However, if we decide to introduce more control
types (such as, for example, a date picker control), this assumption may
no longer be the case - at that point, we would need to reevaluate
whether the default should **always** be options list.

### Video


https://github.com/elastic/kibana/assets/8698078/4a876c94-a041-4228-aab8-7c2c1c871071

### Exact Match Searching

Since numeric fields do not have a "prefix" query equivalent, the only
possibility for searching these fields is either (1) a range query or
(2) an exact match / equality query. In this PR, I added support for
equality search - i.e. in order to find a value in a numeric options
list, you must enter the *full*, exact term in order for it to be found;
in the future, we could extend this to include a range search. This is
the exact match searching in action:

<p align="center"><img alt="GIF of exact match searching example on
number field"
src="https://github.com/elastic/kibana/assets/8698078/491feff3-031f-4367-8018-67aab9be55e3"/></p>


Since exact match searching is a generic search query that works for
**all** field types, I added this as an option for **all** field types
that support searching - i.e. keyword fields, number fields, IP fields,
etc. For example, here is the supported search techniques for a keyword
field:


<p align="center"><img alt="GIF of all available search techniques for
keyword field"
src="https://github.com/elastic/kibana/assets/8698078/dbc12cef-d43e-4d9f-a779-a5fe9e75325c"/></p>

> [!TIP]
> Exact match / equality / term searching on float values can lead to
slightly unexpected results - refer to the [documentation on precision
loss](https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html#_which_type_should_i_use)
for a description of why. Be mindful when using an options list control
for float values, and consider whether a range slider might be a better
solution!

Eventually, we should be able to add exact-match searching to date
fields, as well; however, due to [discrepancies that exist in the
unified search bar](#172097)
with respect to how searching date fields is handled, we should ideally
wait until this is resolved so that we can be consistent across all
three search experiences (query bar, filter pills, and controls).


Depending on how the author expects a given control to be used, this
search technique will return results **faster** than some of the other
search types since "search as you type" will, more often than not,
return zero results in this setting - that is why I chose to add it for
keyword fields, as well.


### Searching when `allowExpensiveQueries` setting is `false`
Previously, we had **two separate versions** of our options list search
queries - one for when `allowExpensiveQueries` was `true`, and the other
for when it was `false`. This was a significant amount of tech debt that
was time consuming to maintain, which made it difficult to justify
keeping this around considering **how much** of Kibana relies on this
setting to be `true` (searching for existing Dashboards by title on the
listing page, saving a brand new dashboard, etc.).

Therefore, while we still have **some** functionality when
`allowExpensiveQueries` is `false`, I have refactored this code
significantly to simplify the logic.

> [!IMPORTANT]
> Specifically, options list controls now only support **exact match
searching** when `allowExpensiveQueries` is `false`.

Since this query is the same regardless of the type of field or the
value of `allowExpensiveQueries`, this means we no longer have to
maintain two slightly different versions ("cheap" and "expensive") of
our search queries. This cleans up our tech debt significantly.

### Bundle Size Changes @elastic/kibana-operations 

Changes to bundle size are primarily due to the changes I made to the
`OptionsListEditorOptions` component - since this component is directly
added to the options list factory (which is not async imported), this
impacts the bundle size.

### Checklist

- [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] [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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
Heenawter authored Dec 18, 2023
1 parent ed33322 commit 7d024a7
Show file tree
Hide file tree
Showing 41 changed files with 1,852 additions and 1,741 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pageLoadAssetSize:
cloudSecurityPosture: 19109
console: 46091
contentManagement: 16254
controls: 40000
controls: 55082
core: 435325
crossClusterReplication: 65408
customIntegrations: 22034
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/controls/common/options_list/ip_search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ interface IpSegments {
type: 'ipv4' | 'ipv6' | 'unknown';
}

export const getIsValidFullIp = (searchString: string) => {
return ipaddr.IPv4.isValidFourPartDecimal(searchString) || ipaddr.IPv6.isValid(searchString);
};

export const getIpSegments = (searchString: string): IpSegments => {
if (searchString.indexOf('.') !== -1) {
// ipv4 takes priority - so if search string contains both `.` and `:` then it will just be an invalid ipv4 search
Expand Down
118 changes: 118 additions & 0 deletions src/plugins/controls/common/options_list/is_valid_search.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { isValidSearch } from './is_valid_search';

describe('test validity of search strings', () => {
describe('number field', () => {
it('valid search - basic integer', () => {
expect(isValidSearch({ searchString: '123', fieldType: 'number' })).toBe(true);
});

it('valid search - floating point number', () => {
expect(isValidSearch({ searchString: '12.34', fieldType: 'number' })).toBe(true);
});

it('valid search - negative number', () => {
expect(isValidSearch({ searchString: '-42', fieldType: 'number' })).toBe(true);
});

it('invalid search - invalid character search string', () => {
expect(isValidSearch({ searchString: '1!a23', fieldType: 'number' })).toBe(false);
});
});

// we do not currently support searching date fields, so they will always be invalid
describe('date field', () => {
it('invalid search - formatted date', () => {
expect(isValidSearch({ searchString: 'December 12, 2023', fieldType: 'date' })).toBe(false);
});

it('invalid search - invalid character search string', () => {
expect(isValidSearch({ searchString: '!!12/12/23?', fieldType: 'date' })).toBe(false);
});
});

// only testing exact match validity here - the remainder of testing is covered by ./ip_search.test.ts
describe('ip field', () => {
it('valid search - ipv4', () => {
expect(
isValidSearch({
searchString: '1.2.3.4',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(true);
});

it('valid search - full ipv6', () => {
expect(
isValidSearch({
searchString: 'fbbe:a363:9e14:987c:49cf:d4d0:d8c8:bc42',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(true);
});

it('valid search - partial ipv6', () => {
expect(
isValidSearch({
searchString: 'fbbe:a363::',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(true);
});

it('invalid search - invalid character search string', () => {
expect(
isValidSearch({
searchString: '!!123.abc?',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(false);
});

it('invalid search - ipv4', () => {
expect(
isValidSearch({
searchString: '1.2.3.256',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(false);
});

it('invalid search - ipv6', () => {
expect(
isValidSearch({
searchString: '::fbbe:a363::',
fieldType: 'ip',
searchTechnique: 'exact',
})
).toBe(false);
});
});

// string field searches can never be invalid
describe('string field', () => {
it('valid search - basic search string', () => {
expect(isValidSearch({ searchString: 'abc', fieldType: 'string' })).toBe(true);
});

it('valid search - numeric search string', () => {
expect(isValidSearch({ searchString: '123', fieldType: 'string' })).toBe(true);
});

it('valid search - complex search string', () => {
expect(isValidSearch({ searchString: '!+@abc*&[]', fieldType: 'string' })).toBe(true);
});
});
});
52 changes: 52 additions & 0 deletions src/plugins/controls/common/options_list/is_valid_search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { getIpRangeQuery, getIsValidFullIp } from './ip_search';
import { OptionsListSearchTechnique } from './suggestions_searching';

/**
* ipaddr is a fairly large library - therefore, this function needs to be separate from
* the `suggestions_searching` file (which is used in the OptionsListEditorOptions component,
* which is in the factory and not async imported)
*/

export const isValidSearch = ({
searchString,
fieldType,
searchTechnique,
}: {
searchString?: string;
fieldType?: string;
searchTechnique?: OptionsListSearchTechnique;
}): boolean => {
if (!searchString || searchString.length === 0) return true;

switch (fieldType) {
case 'number': {
return !isNaN(Number(searchString));
}
case 'date': {
/** searching is not currently supported for date fields */
return false;
}
case 'ip': {
if (searchTechnique === 'exact') {
/**
* exact match searching will throw an error if the search string isn't a **full** IP,
* so we need a slightly different validity check here than for other search techniques
*/
return getIsValidFullIp(searchString);
}
return getIpRangeQuery(searchString).validSearch;
}
default: {
/** string searches are always considered to be valid */
return true;
}
}
};
31 changes: 31 additions & 0 deletions src/plugins/controls/common/options_list/suggestions_searching.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export type OptionsListSearchTechnique = 'prefix' | 'wildcard' | 'exact';

export const getDefaultSearchTechnique = (type: string): OptionsListSearchTechnique | undefined => {
const compatibleSearchTechniques = getCompatibleSearchTechniques(type);
return compatibleSearchTechniques.length > 0 ? compatibleSearchTechniques[0] : undefined;
};

export const getCompatibleSearchTechniques = (type?: string): OptionsListSearchTechnique[] => {
switch (type) {
case 'string': {
return ['prefix', 'wildcard', 'exact'];
}
case 'ip': {
return ['prefix', 'exact'];
}
case 'number': {
return ['exact'];
}
default: {
return [];
}
}
};
10 changes: 4 additions & 6 deletions src/plugins/controls/common/options_list/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@
* Side Public License, v 1.
*/

import { FieldSpec, DataView, RuntimeFieldSpec } from '@kbn/data-views-plugin/common';
import type { Filter, Query, BoolQuery, TimeRange } from '@kbn/es-query';
import { DataView, FieldSpec, RuntimeFieldSpec } from '@kbn/data-views-plugin/common';
import type { BoolQuery, Filter, Query, TimeRange } from '@kbn/es-query';

import type { OptionsListSortingType } from './suggestions_sorting';
import type { DataControlInput } from '../types';
import { OptionsListSearchTechnique } from './suggestions_searching';
import type { OptionsListSortingType } from './suggestions_sorting';

export const OPTIONS_LIST_CONTROL = 'optionsListControl';

export type OptionsListSearchTechnique = 'prefix' | 'wildcard';
export const OPTIONS_LIST_DEFAULT_SEARCH_TECHNIQUE: OptionsListSearchTechnique = 'prefix';

export interface OptionsListEmbeddableInput extends DataControlInput {
searchTechnique?: OptionsListSearchTechnique;
sort?: OptionsListSortingType;
Expand Down
46 changes: 45 additions & 1 deletion src/plugins/controls/common/range_slider/mocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,56 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { RangeSliderEmbeddableInput } from '..';
import {
ControlFactory,
ControlOutput,
RangeSliderEmbeddable,
RangeSliderEmbeddableFactory,
} from '../../public';
import * as rangeSliderStateModule from '../../public/range_slider/range_slider_reducers';
import { RangeSliderComponentState } from '../../public/range_slider/types';

export const mockRangeSliderEmbeddableInput = {
id: 'sample options list',
fieldName: 'sample field',
dataViewId: 'sample id',
value: ['0', '10'],
} as RangeSliderEmbeddableInput;

const mockRangeSliderComponentState = {
field: { name: 'bytes', type: 'number', aggregatable: true },
min: undefined,
max: undefined,
error: undefined,
isInvalid: false,
} as RangeSliderComponentState;

const mockRangeSliderOutput = {
loading: false,
} as ControlOutput;

export const mockRangeSliderEmbeddable = async (partialState?: {
explicitInput?: Partial<RangeSliderEmbeddableInput>;
componentState?: Partial<RangeSliderEmbeddableInput>;
}) => {
const rangeSliderFactoryStub = new RangeSliderEmbeddableFactory();
const rangeSliderControlFactory = rangeSliderFactoryStub as unknown as ControlFactory;
rangeSliderControlFactory.getDefaultInput = () => ({});

// initial component state can be provided by overriding the defaults.
const initialComponentState = {
...mockRangeSliderComponentState,
...partialState?.componentState,
};
jest
.spyOn(rangeSliderStateModule, 'getDefaultComponentState')
.mockImplementation(() => initialComponentState);

const mockEmbeddable = (await rangeSliderControlFactory.create({
...mockRangeSliderEmbeddableInput,
...partialState?.explicitInput,
})) as RangeSliderEmbeddable;
mockEmbeddable.getOutput = jest.fn().mockReturnValue(mockRangeSliderOutput);
return mockEmbeddable;
};
46 changes: 42 additions & 4 deletions src/plugins/controls/public/control_group/control_group_strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { i18n } from '@kbn/i18n';
import { RANGE_SLIDER_CONTROL } from '../range_slider';

export const ControlGroupStrings = {
manageControl: {
Expand Down Expand Up @@ -35,10 +36,6 @@ export const ControlGroupStrings = {
i18n.translate('controls.controlGroup.manageControl.dataSource.dataViewTitle', {
defaultMessage: 'Data view',
}),
noControlTypeMessage: () =>
i18n.translate('controls.controlGroup.manageControl.dataSource.noControlTypeMessage', {
defaultMessage: 'No field selected yet',
}),
getFieldTitle: () =>
i18n.translate('controls.controlGroup.manageControl.dataSource.fieldTitle', {
defaultMessage: 'Field',
Expand All @@ -47,6 +44,46 @@ export const ControlGroupStrings = {
i18n.translate('controls.controlGroup.manageControl.dataSource.controlTypesTitle', {
defaultMessage: 'Control type',
}),
getControlTypeErrorMessage: ({
fieldSelected,
controlType,
}: {
fieldSelected?: boolean;
controlType?: string;
}) => {
if (!fieldSelected) {
return i18n.translate(
'controls.controlGroup.manageControl.dataSource.controlTypErrorMessage.noField',
{
defaultMessage: 'Select a field first.',
}
);
}

switch (controlType) {
/**
* Note that options list controls are currently compatible with every field type; so, there is no
* need to have a special error message for these.
*/
case RANGE_SLIDER_CONTROL: {
return i18n.translate(
'controls.controlGroup.manageControl.dataSource.controlTypeErrorMessage.rangeSlider',
{
defaultMessage: 'Range sliders are only compatible with number fields.',
}
);
}
default: {
/** This shouldn't ever happen - but, adding just in case as a fallback. */
return i18n.translate(
'controls.controlGroup.manageControl.dataSource.controlTypeErrorMessage.default',
{
defaultMessage: 'Select a compatible control type.',
}
);
}
}
},
},
displaySettings: {
getFormGroupTitle: () =>
Expand Down Expand Up @@ -231,6 +268,7 @@ export const ControlGroupStrings = {
'Selections in one control narrow down available options in the next. Controls are chained from left to right.',
}),
},
/** TODO: These translations aren't used but they will be once https://github.com/elastic/kibana/issues/162985 is resolved */
querySync: {
getQuerySettingsTitle: () =>
i18n.translate('controls.controlGroup.management.query.searchSettingsTitle', {
Expand Down
Loading

0 comments on commit 7d024a7

Please sign in to comment.