Skip to content
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.

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

Improve version type support #123739

Merged
merged 8 commits into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/kbn-react-field/src/field_icon/field_icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export interface FieldIconProps extends Omit<EuiTokenProps, 'iconType'> {
| '_source'
| 'string'
| string
| 'nested';
| 'nested'
| 'version';
label?: string;
scripted?: boolean;
}
Expand Down Expand Up @@ -54,6 +55,7 @@ export const typeToEuiIconMap: Partial<Record<string, EuiTokenProps>> = {
text: { iconType: 'tokenString' },
keyword: { iconType: 'tokenKeyword' },
nested: { iconType: 'tokenNested' },
version: { iconType: 'tokenTag' },
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,16 @@ export const setupValueSuggestionProvider = (
useTimeRange ?? core!.uiSettings.get<boolean>(UI_SETTINGS.AUTOCOMPLETE_USE_TIMERANGE);
const { title } = indexPattern;

const isVersionFieldType = field.type === 'string' && field.esTypes?.includes('version');

if (field.type === 'boolean') {
return [true, false];
} else if (!shouldSuggestValues || !field.aggregatable || field.type !== 'string') {
} else if (
!shouldSuggestValues ||
!field.aggregatable ||
field.type !== 'string' ||
isVersionFieldType // suggestions don't work for version fields
) {
return [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ class FilterEditorUI extends Component<Props, State> {

private renderParamsEditor() {
const indexPattern = this.state.selectedIndexPattern;
if (!indexPattern || !this.state.selectedOperator) {
if (!indexPattern || !this.state.selectedOperator || !this.state.selectedField) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logically it couldn't happen that we had selectedOperator without a selectedField, but I wanted to clarify this here because children now have field as required

return '';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import dateMath from '@elastic/datemath';
import { Filter, FieldFilter } from '@kbn/es-query';
import { ES_FIELD_TYPES } from '@kbn/field-types';
import isSemverValid from 'semver/functions/valid';
import { FILTER_OPERATORS, Operator } from './filter_operators';
import { isFilterable, IIndexPattern, IFieldType, IpAddress } from '../../../../../common';

Expand All @@ -27,12 +29,14 @@ export function getFilterableFields(indexPattern: IIndexPattern) {

export function getOperatorOptions(field: IFieldType) {
return FILTER_OPERATORS.filter((operator) => {
return !operator.fieldTypes || operator.fieldTypes.includes(field.type);
if (operator.field) return operator.field(field);
if (operator.fieldTypes) return operator.fieldTypes.includes(field.type);
return true;
});
}

export function validateParams(params: any, type: string) {
switch (type) {
export function validateParams(params: any, field: IFieldType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we need to check against field.esTypes now, I am passing the whole field object

switch (field.type) {
case 'date':
const moment = typeof params === 'string' ? dateMath.parse(params) : null;
return Boolean(typeof params === 'string' && moment && moment.isValid());
Expand All @@ -42,6 +46,11 @@ export function validateParams(params: any, type: string) {
} catch (e) {
return false;
}
case 'string':
if (field.esTypes?.includes(ES_FIELD_TYPES.VERSION)) {
return isSemverValid(params);
}
return true;
default:
return true;
}
Expand All @@ -58,19 +67,19 @@ export function isFilterValid(
}
switch (operator.type) {
case 'phrase':
return validateParams(params, field.type);
return validateParams(params, field);
case 'phrases':
if (!Array.isArray(params) || !params.length) {
return false;
}
return params.every((phrase) => validateParams(phrase, field.type));
return params.every((phrase) => validateParams(phrase, field));
case 'range':
if (typeof params !== 'object') {
return false;
}
return (
(!params.from || validateParams(params.from, field.type)) &&
(!params.to || validateParams(params.to, field.type))
(!params.from || validateParams(params.from, field)) &&
(!params.to || validateParams(params.to, field))
);
case 'exists':
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,24 @@

import { i18n } from '@kbn/i18n';
import { FILTERS } from '@kbn/es-query';
import { ES_FIELD_TYPES } from '@kbn/field-types';
import { IFieldType } from '../../../../../../data_views/common';

export interface Operator {
message: string;
type: FILTERS;
negate: boolean;

/**
* KbnFieldTypes applicable for operator
*/
fieldTypes?: string[];

/**
* A filter predicate for a field,
* takes precedence over {@link fieldTypes}
*/
field?: (field: IFieldType) => boolean;
}

export const isOperator = {
Expand Down Expand Up @@ -56,7 +68,14 @@ export const isBetweenOperator = {
}),
type: FILTERS.RANGE,
negate: false,
fieldTypes: ['number', 'number_range', 'date', 'date_range', 'ip', 'ip_range'],
field: (field: IFieldType) => {
if (['number', 'number_range', 'date', 'date_range', 'ip', 'ip_range'].includes(field.type))
return true;

if (field.type === 'string' && field.esTypes?.includes(ES_FIELD_TYPES.VERSION)) return true;

return false;
},
};

export const isNotBetweenOperator = {
Expand All @@ -65,7 +84,14 @@ export const isNotBetweenOperator = {
}),
type: FILTERS.RANGE,
negate: true,
fieldTypes: ['number', 'number_range', 'date', 'date_range', 'ip', 'ip_range'],
field: (field: IFieldType) => {
if (['number', 'number_range', 'date', 'date_range', 'ip', 'ip_range'].includes(field.type))
return true;

if (field.type === 'string' && field.esTypes?.includes(ES_FIELD_TYPES.VERSION)) return true;

return false;
},
};

export const existsOperator = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { UI_SETTINGS } from '../../../../common';
export interface PhraseSuggestorProps {
kibana: KibanaReactContextValue<IDataPluginServices>;
indexPattern: IIndexPattern;
field?: IFieldType;
field: IFieldType;
timeRangeForSuggestionsOverride?: boolean;
}

Expand Down Expand Up @@ -54,7 +54,15 @@ export class PhraseSuggestorUI<T extends PhraseSuggestorProps> extends React.Com
UI_SETTINGS.FILTERS_EDITOR_SUGGEST_VALUES
);
const { field } = this.props;
return shouldSuggestValues && field && field.aggregatable && field.type === 'string';
const isVersionFieldType = field?.esTypes?.includes('version');

return (
shouldSuggestValues &&
field &&
field.aggregatable &&
field.type === 'string' &&
!isVersionFieldType // suggestions don't work for version fields
);
}

protected onSearchChange = (value: string | number | boolean) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class PhraseValueInputUI extends PhraseSuggestorUI<Props> {
})}
value={this.props.value}
onChange={this.props.onChange}
type={this.props.field ? this.props.field.type : 'string'}
field={this.props.field}
/>
)}
</EuiFormRow>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ interface RangeParams {
type RangeParamsPartial = Partial<RangeParams>;

interface Props {
field?: IFieldType;
field: IFieldType;
value?: RangeParams;
onChange: (params: RangeParamsPartial) => void;
intl: InjectedIntl;
Expand All @@ -32,7 +32,6 @@ interface Props {

function RangeValueInputUI(props: Props) {
const kibana = useKibana();
const type = props.field ? props.field.type : 'string';
const tzConfig = kibana.services.uiSettings!.get('dateFormat:tz');

const formatDateChange = (value: string | number | boolean) => {
Expand Down Expand Up @@ -69,7 +68,7 @@ function RangeValueInputUI(props: Props) {
startControl={
<ValueInputType
controlOnly
type={type}
field={props.field}
value={props.value ? props.value.from : undefined}
onChange={onFromChange}
onBlur={(value) => {
Expand All @@ -84,7 +83,7 @@ function RangeValueInputUI(props: Props) {
endControl={
<ValueInputType
controlOnly
type={type}
field={props.field}
value={props.value ? props.value.to : undefined}
onChange={onToChange}
onBlur={(value) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import { InjectedIntl, injectI18n } from '@kbn/i18n-react';
import { isEmpty } from 'lodash';
import React, { Component } from 'react';
import { validateParams } from './lib/filter_editor_utils';
import { IFieldType } from '../../../../../data_views/common';

interface Props {
value?: string | number;
type: string;
field: IFieldType;
onChange: (value: string | number | boolean) => void;
onBlur?: (value: string | number | boolean) => void;
placeholder: string;
Expand All @@ -27,15 +28,17 @@ interface Props {
class ValueInputTypeUI extends Component<Props> {
public render() {
const value = this.props.value;
const type = this.props.field.type;
let inputElement: React.ReactNode;
switch (this.props.type) {
switch (type) {
case 'string':
inputElement = (
<EuiFieldText
fullWidth={this.props.fullWidth}
placeholder={this.props.placeholder}
value={value}
onChange={this.onChange}
isInvalid={!validateParams(value, this.props.field)}
controlOnly={this.props.controlOnly}
className={this.props.className}
/>
Expand Down Expand Up @@ -63,7 +66,7 @@ class ValueInputTypeUI extends Component<Props> {
value={value}
onChange={this.onChange}
onBlur={this.onBlur}
isInvalid={!isEmpty(value) && !validateParams(value, this.props.type)}
isInvalid={!isEmpty(value) && !validateParams(value, this.props.field)}
controlOnly={this.props.controlOnly}
className={this.props.className}
/>
Expand All @@ -77,7 +80,7 @@ class ValueInputTypeUI extends Component<Props> {
placeholder={this.props.placeholder}
value={value}
onChange={this.onChange}
isInvalid={!isEmpty(value) && !validateParams(value, this.props.type)}
isInvalid={!isEmpty(value) && !validateParams(value, this.props.field)}
controlOnly={this.props.controlOnly}
className={this.props.className}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export function getFieldTypeName(type: string) {
return i18n.translate('discover.fieldNameIcons.nestedFieldAriaLabel', {
defaultMessage: 'Nested field',
});
case 'version':
return i18n.translate('discover.fieldNameIcons.versionFieldAriaLabel', {
defaultMessage: 'Version field',
});
default:
return i18n.translate('discover.fieldNameIcons.unknownFieldAriaLabel', {
defaultMessage: 'Unknown field',
Expand Down
54 changes: 54 additions & 0 deletions test/functional/apps/discover/_filter_editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,60 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await PageObjects.discover.getHitCount()).to.be('1');
});
});

describe('version fields', async () => {
const es = getService('es');
const indexPatterns = getService('indexPatterns');
const indexTitle = 'version-test';

before(async () => {
if (await es.indices.exists({ index: indexTitle })) {
await es.indices.delete({ index: indexTitle });
}

await es.indices.create({
index: indexTitle,
body: {
mappings: {
properties: {
version: {
type: 'version',
},
},
},
},
});

await es.index({
index: indexTitle,
body: {
version: '1.0.0',
},
refresh: 'wait_for',
});

await es.index({
index: indexTitle,
body: {
version: '2.0.0',
},
refresh: 'wait_for',
});

await indexPatterns.create({ title: indexTitle }, { override: true });

await PageObjects.common.navigateToApp('discover');
await PageObjects.discover.selectIndexPattern(indexTitle);
});

it('should support range filter on version fields', async () => {
await filterBar.addFilter('version', 'is between', '2.0.0', '3.0.0');
expect(await filterBar.hasFilter('version', '2.0.0 to 3.0.0')).to.be(true);
await retry.try(async function () {
expect(await PageObjects.discover.getHitCount()).to.be('1');
});
});
});
});
});
}