Skip to content

NETOBSERV-1816 allow n/a filter #590

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 11, 2024
Merged

NETOBSERV-1816 allow n/a filter #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6d4955
allow n/a filter
jpinsonneau Sep 6, 2024
14617a0
override empty value by undefined value
jpinsonneau Sep 9, 2024
4664365
allow empty only when equal or different
jpinsonneau Sep 10, 2024
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
1 change: 1 addition & 0 deletions web/locales/en/plugin__netobserv-plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@
"Last 1 week": "Last 1 week",
"Last 2 weeks": "Last 2 weeks",
"Value is empty": "Value is empty",
"Value is malformed": "Value is malformed",
"Not a valid Kubernetes name": "Not a valid Kubernetes name",
"Incomplete resource name, either kind, namespace or name is missing.": "Incomplete resource name, either kind, namespace or name is missing.",
"Kind is empty": "Kind is empty",
Expand Down
10 changes: 8 additions & 2 deletions web/src/components/toolbar/filters-toolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,17 @@ export const FiltersToolbar: React.FC<FiltersToolbarProps> = ({
switch (selectedFilter.component) {
case 'text':
case 'number':
return <TextFilter {...commonProps} regexp={selectedFilter.component === 'number' ? /\D/g : undefined} />;
return (
<TextFilter
{...commonProps}
allowEmpty={selectedCompare !== FilterCompare.moreThanOrEqual}
regexp={selectedFilter.component === 'number' ? /\D/g : undefined}
/>
);
case 'autocomplete':
return <AutocompleteFilter {...commonProps} />;
}
}, [selectedFilter, addFilter, indicator, setIndicator, setMessageWithDelay]);
}, [selectedFilter, addFilter, setMessageWithDelay, indicator, selectedCompare]);

const isForced = !_.isEmpty(forcedFilters);
const filtersOrForced = isForced ? forcedFilters : filters;
Expand Down
9 changes: 8 additions & 1 deletion web/src/components/toolbar/filters/autocomplete-filter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import * as React from 'react';
import { createFilterValue, FilterDefinition, FilterOption, FilterValue } from '../../../model/filters';
import { autoCompleteCache } from '../../../utils/autocomplete-cache';
import { getHTTPErrorDetails } from '../../../utils/errors';
import { undefinedValue } from '../../../utils/filter-definitions';
import { Indicator } from '../../../utils/filters-helper';
import { usePrevious } from '../../../utils/previous-hook';
import './autocomplete-filter.css';
Expand Down Expand Up @@ -128,13 +129,19 @@ export const AutocompleteFilter: React.FC<AutocompleteFilterProps> = ({
);

const onEnter = React.useCallback(() => {
// override empty value by undefined value
let v = currentValue;
if (currentValue.length === 0) {
v = undefinedValue;
}

// Only one choice is present, consider this is what is desired
if (options.length === 1) {
onAutoCompleteOptionSelected(options[0]);
return;
}

const validation = filterDefinition.validate(currentValue);
const validation = filterDefinition.validate(v);
//show tooltip and icon when user try to validate filter
if (!_.isEmpty(validation.err)) {
setMessageWithDelay(validation.err);
Expand Down
19 changes: 16 additions & 3 deletions web/src/components/toolbar/filters/text-filter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { SearchIcon } from '@patternfly/react-icons';
import * as _ from 'lodash';
import * as React from 'react';
import { createFilterValue, FilterDefinition, FilterValue } from '../../../model/filters';
import { doubleQuoteValue, undefinedValue } from '../../../utils/filter-definitions';
import { Indicator } from '../../../utils/filters-helper';

export interface TextFilterProps {
Expand All @@ -11,6 +12,7 @@ export interface TextFilterProps {
setMessageWithDelay: (m: string | undefined) => void;
indicator: Indicator;
setIndicator: (ind: Indicator) => void;
allowEmpty?: boolean;
regexp?: RegExp;
}

Expand All @@ -20,6 +22,7 @@ export const TextFilter: React.FC<TextFilterProps> = ({
setMessageWithDelay,
indicator,
setIndicator,
allowEmpty,
regexp
}) => {
const searchInputRef = React.useRef<HTMLInputElement | null>(null);
Expand All @@ -38,7 +41,7 @@ export const TextFilter: React.FC<TextFilterProps> = ({
const updateValue = React.useCallback(
(v: string) => {
let filteredValue = v;
if (regexp) {
if (![doubleQuoteValue, undefinedValue].includes(filteredValue) && regexp) {
filteredValue = filteredValue.replace(regexp, '');
}
setCurrentValue(filteredValue);
Expand All @@ -53,7 +56,17 @@ export const TextFilter: React.FC<TextFilterProps> = ({
}, [setCurrentValue, setMessageWithDelay, setIndicator]);

const onSelect = React.useCallback(() => {
const validation = filterDefinition.validate(String(currentValue));
// override empty value by undefined value
let v = currentValue;
if (allowEmpty) {
if (currentValue.length === 0) {
v = undefinedValue;
}
} else if (v === undefinedValue) {
v = '';
}

const validation = filterDefinition.validate(String(v));
//show tooltip and icon when user try to validate filter
if (!_.isEmpty(validation.err)) {
setMessageWithDelay(validation.err);
Expand All @@ -66,7 +79,7 @@ export const TextFilter: React.FC<TextFilterProps> = ({
resetFilterValue();
}
});
}, [filterDefinition, currentValue, setMessageWithDelay, setIndicator, addFilter, resetFilterValue]);
}, [currentValue, allowEmpty, filterDefinition, setMessageWithDelay, setIndicator, addFilter, resetFilterValue]);

return (
<>
Expand Down
5 changes: 4 additions & 1 deletion web/src/model/filters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import _ from 'lodash';
import { isEqual } from '../utils/base-compare';
import { undefinedValue } from '../utils/filter-definitions';

export type FiltersEncoder = (values: FilterValue[], matchAny: boolean, not: boolean, moreThan: boolean) => string;

Expand Down Expand Up @@ -94,7 +95,9 @@ export interface FilterOption {
export const createFilterValue = (def: FilterDefinition, value: string): Promise<FilterValue> => {
return def.getOptions(value).then(opts => {
const option = opts.find(opt => opt.name === value || opt.value === value);
return option ? { v: option.value, display: option.name } : { v: value };
return option
? { v: option.value, display: option.name }
: { v: value, display: value === undefinedValue ? 'n/a' : undefined };
});
};

Expand Down
14 changes: 10 additions & 4 deletions web/src/utils/filter-definitions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { validateK8SName, validateStrictK8SName } from './label';

// Convenience string to filter by undefined field values
export const undefinedValue = '""';
// Unique double are allowed while typing but invalid
Copy link
Member

Choose a reason for hiding this comment

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

@jpinsonneau could we use just an empty string as undefined value? I feel it would make more sense, at least for non-string values, than using "". wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

(If we stick to using "" IMO we should mention it in the help tips because it's sounds less intuitive)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that should do the trick @jotak
14617a0

export const doubleQuoteValue = '"';

const matcher = (left: string, right: string[], not: boolean, moreThan: boolean) =>
`${left}${not ? '!=' : moreThan ? '>=' : '='}${right.join(',')}`;
Expand Down Expand Up @@ -136,15 +138,19 @@ export const getFilterDefinitions = (
if (_.isEmpty(value)) {
return invalid(t('Value is empty'));
}
if (value === doubleQuoteValue) {
return invalid(t('Value is malformed'));
}
return valid(value);
};

const k8sNameValidation = (value: string) => {
if (_.isEmpty(value)) {
// Replace with exact match
return valid('""');
return invalid(t('Value is empty'));
}
return value === '""' || validateK8SName(value) ? valid(value) : invalid(t('Not a valid Kubernetes name'));
return value === undefinedValue || validateK8SName(value)
? valid(value)
: invalid(t('Not a valid Kubernetes name'));
};

const k8sResourceValidation = (value: string) => {
Expand Down Expand Up @@ -208,7 +214,7 @@ export const getFilterDefinitions = (
if (_.isEmpty(value)) {
return invalid(t('Value is empty'));
}
return /^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})/.test(value)
return value == undefinedValue || /^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})/.test(value)
? valid(value)
: invalid(t('Not a valid MAC address'));
};
Expand Down
Loading