Addresses bug where advanced search by groups wasn't working on host list

This commit is contained in:
mabashian 2021-06-09 13:19:14 -04:00
parent 3cb3819be9
commit 8b20d770a2
10 changed files with 403 additions and 202 deletions

View File

@ -40,6 +40,7 @@ function DataListToolbar({
qsConfig,
pagination,
enableNegativeFiltering,
enableRelatedFuzzyFiltering,
}) {
const showExpandCollapse = onCompact && onExpand;
const [isKebabOpen, setIsKebabOpen] = useState(false);
@ -92,6 +93,7 @@ function DataListToolbar({
onShowAdvancedSearch={onShowAdvancedSearch}
onRemove={onRemove}
enableNegativeFiltering={enableNegativeFiltering}
enableRelatedFuzzyFiltering={enableRelatedFuzzyFiltering}
/>
</ToolbarItem>
{sortColumns && (
@ -173,6 +175,7 @@ DataListToolbar.propTypes = {
onSort: PropTypes.func,
additionalControls: PropTypes.arrayOf(PropTypes.node),
enableNegativeFiltering: PropTypes.bool,
enableRelatedFuzzyFiltering: PropTypes.bool,
};
DataListToolbar.defaultProps = {
@ -192,6 +195,7 @@ DataListToolbar.defaultProps = {
onSort: null,
additionalControls: [],
enableNegativeFiltering: true,
enableRelatedFuzzyFiltering: true,
};
export default DataListToolbar;

View File

@ -118,6 +118,7 @@ function HostFilterLookup({
organizationId,
value,
enableNegativeFiltering,
enableRelatedFuzzyFiltering,
}) {
const history = useHistory();
const location = useLocation();
@ -347,6 +348,7 @@ function HostFilterLookup({
{...props}
fillWidth
enableNegativeFiltering={enableNegativeFiltering}
enableRelatedFuzzyFiltering={enableRelatedFuzzyFiltering}
/>
)}
toolbarSearchColumns={searchColumns}
@ -381,6 +383,7 @@ HostFilterLookup.propTypes = {
organizationId: number,
value: string,
enableNegativeFiltering: bool,
enableRelatedFuzzyFiltering: bool,
};
HostFilterLookup.defaultProps = {
isValid: true,
@ -389,6 +392,7 @@ HostFilterLookup.defaultProps = {
organizationId: null,
value: '',
enableNegativeFiltering: true,
enableRelatedFuzzyFiltering: true,
};
export default withRouter(HostFilterLookup);

View File

@ -7,9 +7,18 @@ export function toSearchParams(string = '') {
if (string === '') {
return {};
}
return string
.replace(/^\?/, '')
.replace(/&/g, ' and ')
const readableParamsStr = string.replace(/^\?/, '').replace(/&/g, ' and ');
const orArr = readableParamsStr.split(/ or /);
if (orArr.length > 1) {
orArr.forEach((str, index) => {
orArr[index] = `or__${str}`;
});
}
return orArr
.join(' and ')
.split(/ and | or /)
.map(s => s.split('='))
.reduce((searchParams, [k, v]) => {

View File

@ -33,6 +33,20 @@ describe('toSearchParams', () => {
};
expect(toSearchParams(string)).toEqual(paramsObject);
});
test('should take a host filter string separated by or and return search params object with or', () => {
string = 'foo=bar or foo=baz';
paramsObject = {
or__foo: ['bar', 'baz'],
};
expect(toSearchParams(string)).toEqual(paramsObject);
});
test('should take a host filter string with or and return search params object with or', () => {
string = 'or__foo=1&or__foo=2';
paramsObject = {
or__foo: ['1', '2'],
};
expect(toSearchParams(string)).toEqual(paramsObject);
});
});
describe('toQueryString', () => {
@ -108,6 +122,13 @@ describe('toHostFilter', () => {
'name=foo or name__contains=bar or name__iexact=foo'
);
});
test('should return a host filter with or conditional when value is array', () => {
const object = {
or__groups__id: ['1', '2'],
};
expect(toHostFilter(object)).toEqual('groups__id=1 or groups__id=2');
});
});
describe('removeNamespacedKeys', () => {

View File

@ -1,7 +1,6 @@
import 'styled-components/macro';
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import { t } from '@lingui/macro';
import {
Button,
@ -33,6 +32,7 @@ function AdvancedSearch({
relatedSearchableKeys,
maxSelectHeight,
enableNegativeFiltering,
enableRelatedFuzzyFiltering,
}) {
// TODO: blocked by pf bug, eventually separate these into two groups in the select
// for now, I'm spreading set to get rid of duplicate keys...when they are grouped
@ -48,30 +48,40 @@ function AdvancedSearch({
const [lookupSelection, setLookupSelection] = useState(null);
const [keySelection, setKeySelection] = useState(null);
const [searchValue, setSearchValue] = useState('');
const [relatedSearchKeySelected, setRelatedSearchKeySelected] = useState(
false
);
const config = useConfig();
useEffect(() => {
if (
keySelection &&
relatedSearchableKeys.indexOf(keySelection) > -1 &&
searchableKeys.indexOf(keySelection) === -1
) {
setLookupSelection('name__icontains');
setRelatedSearchKeySelected(true);
} else {
setLookupSelection(null);
setRelatedSearchKeySelected(false);
}
}, [keySelection]); // eslint-disable-line react-hooks/exhaustive-deps
useEffect(() => {
if (lookupSelection === 'search') {
setPrefixSelection(null);
}
}, [lookupSelection]); // eslint-disable-line react-hooks/exhaustive-deps
const handleAdvancedSearch = e => {
// keeps page from fully reloading
e.preventDefault();
if (searchValue) {
const actualPrefix = prefixSelection === 'and' ? null : prefixSelection;
let actualSearchKey;
// TODO: once we are able to group options for the key typeahead, we will
// probably want to be able to which group a key was clicked in for duplicates,
// rather than checking to make sure it's not in both for this appending
// __search logic
if (
relatedSearchableKeys.indexOf(keySelection) > -1 &&
searchableKeys.indexOf(keySelection) === -1 &&
keySelection.indexOf('__') === -1
) {
actualSearchKey = `${keySelection}__search`;
} else {
actualSearchKey = [actualPrefix, keySelection, lookupSelection]
.filter(val => !!val)
.join('__');
}
const actualSearchKey = [actualPrefix, keySelection, lookupSelection]
.filter(val => !!val)
.join('__');
onSearch(actualSearchKey, searchValue);
setSearchValue('');
}
@ -83,44 +93,211 @@ function AdvancedSearch({
}
};
const renderSetType = () => (
<Select
ouiaId="set-type-typeahead"
aria-label={t`Set type select`}
className="setTypeSelect"
variant={SelectVariant.typeahead}
typeAheadAriaLabel={t`Set type typeahead`}
onToggle={setIsPrefixDropdownOpen}
onSelect={(event, selection) => setPrefixSelection(selection)}
onClear={() => setPrefixSelection(null)}
selections={prefixSelection}
isOpen={isPrefixDropdownOpen}
placeholderText={t`Set type`}
maxHeight={maxSelectHeight}
noResultsFoundText={t`No results found`}
isDisabled={lookupSelection === 'search'}
>
<SelectOption
id="and-option-select"
key="and"
value="and"
description={t`Returns results that satisfy this one as well as other filters. This is the default set type if nothing is selected.`}
/>
<SelectOption
id="or-option-select"
key="or"
value="or"
description={t`Returns results that satisfy this one or any other filters.`}
/>
{enableNegativeFiltering && (
<SelectOption
id="not-option-select"
key="not"
value="not"
description={t`Returns results that have values other than this one as well as other filters.`}
/>
)}
</Select>
);
const renderRelatedLookupType = () => (
<Select
ouiaId="set-lookup-typeahead"
aria-label={t`Related search type`}
className="lookupSelect"
variant={SelectVariant.typeahead}
typeAheadAriaLabel={t`Related search type typeahead`}
onToggle={setIsLookupDropdownOpen}
onSelect={(event, selection) => setLookupSelection(selection)}
selections={lookupSelection}
isOpen={isLookupDropdownOpen}
placeholderText={t`Related search type`}
maxHeight={maxSelectHeight}
noResultsFoundText={t`No results found`}
>
<SelectOption
id="name-option-select"
key="name__icontains"
value="name__icontains"
description={t`Fuzzy search on name field.`}
/>
<SelectOption
id="id-option-select"
key="id"
value="id"
description={t`Exact search on id field.`}
/>
{enableRelatedFuzzyFiltering && (
<SelectOption
id="search-option-select"
key="search"
value="search"
description={t`Fuzzy search on id, name or description fields.`}
/>
)}
</Select>
);
const renderLookupType = () => (
<Select
ouiaId="set-lookup-typeahead"
aria-label={t`Lookup select`}
className="lookupSelect"
variant={SelectVariant.typeahead}
typeAheadAriaLabel={t`Lookup typeahead`}
onToggle={setIsLookupDropdownOpen}
onSelect={(event, selection) => setLookupSelection(selection)}
onClear={() => setLookupSelection(null)}
selections={lookupSelection}
isOpen={isLookupDropdownOpen}
placeholderText={t`Lookup type`}
maxHeight={maxSelectHeight}
noResultsFoundText={t`No results found`}
>
<SelectOption
id="exact-option-select"
key="exact"
value="exact"
description={t`Exact match (default lookup if not specified).`}
/>
<SelectOption
id="iexact-option-select"
key="iexact"
value="iexact"
description={t`Case-insensitive version of exact.`}
/>
<SelectOption
id="contains-option-select"
key="contains"
value="contains"
description={t`Field contains value.`}
/>
<SelectOption
id="icontains-option-select"
key="icontains"
value="icontains"
description={t`Case-insensitive version of contains`}
/>
<SelectOption
id="startswith-option-select"
key="startswith"
value="startswith"
description={t`Field starts with value.`}
/>
<SelectOption
id="istartswith-option-select"
key="istartswith"
value="istartswith"
description={t`Case-insensitive version of startswith.`}
/>
<SelectOption
id="endswith-option-select"
key="endswith"
value="endswith"
description={t`Field ends with value.`}
/>
<SelectOption
id="iendswith-option-select"
key="iendswith"
value="iendswith"
description={t`Case-insensitive version of endswith.`}
/>
<SelectOption
id="regex-option-select"
key="regex"
value="regex"
description={t`Field matches the given regular expression.`}
/>
<SelectOption
id="iregex-option-select"
key="iregex"
value="iregex"
description={t`Case-insensitive version of regex.`}
/>
<SelectOption
id="gt-option-select"
key="gt"
value="gt"
description={t`Greater than comparison.`}
/>
<SelectOption
id="gte-option-select"
key="gte"
value="gte"
description={t`Greater than or equal to comparison.`}
/>
<SelectOption
id="lt-option-select"
key="lt"
value="lt"
description={t`Less than comparison.`}
/>
<SelectOption
id="lte-option-select"
key="lte"
value="lte"
description={t`Less than or equal to comparison.`}
/>
<SelectOption
id="isnull-option-select"
key="isnull"
value="isnull"
description={t`Check whether the given field or related object is null; expects a boolean value.`}
/>
<SelectOption
id="in-option-select"
key="in"
value="in"
description={t`Check whether the given field's value is present in the list provided; expects a comma-separated list of items.`}
/>
</Select>
);
return (
<AdvancedGroup>
<Select
ouiaId="set-type-typeahead"
aria-label={t`Set type select`}
className="setTypeSelect"
variant={SelectVariant.typeahead}
typeAheadAriaLabel={t`Set type typeahead`}
onToggle={setIsPrefixDropdownOpen}
onSelect={(event, selection) => setPrefixSelection(selection)}
onClear={() => setPrefixSelection(null)}
selections={prefixSelection}
isOpen={isPrefixDropdownOpen}
placeholderText={t`Set type`}
maxHeight={maxSelectHeight}
noResultsFoundText={t`No results found`}
>
<SelectOption
id="and-option-select"
key="and"
value="and"
description={t`Returns results that satisfy this one as well as other filters. This is the default set type if nothing is selected.`}
/>
<SelectOption
id="or-option-select"
key="or"
value="or"
description={t`Returns results that satisfy this one or any other filters.`}
/>
{enableNegativeFiltering && (
<SelectOption
id="not-option-select"
key="not"
value="not"
description={t`Returns results that have values other than this one as well as other filters.`}
/>
)}
</Select>
{lookupSelection === 'search' ? (
<Tooltip
content={t`Set type disabled for related search field fuzzy searches`}
>
{renderSetType()}
</Tooltip>
) : (
renderSetType()
)}
<Select
ouiaId="set-key-typeahead"
aria-label={t`Key select`}
@ -148,118 +325,9 @@ function AdvancedSearch({
</SelectOption>
))}
</Select>
<Select
ouiaId="set-lookup-typeahead"
aria-label={t`Lookup select`}
className="lookupSelect"
variant={SelectVariant.typeahead}
typeAheadAriaLabel={t`Lookup typeahead`}
onToggle={setIsLookupDropdownOpen}
onSelect={(event, selection) => setLookupSelection(selection)}
onClear={() => setLookupSelection(null)}
selections={lookupSelection}
isOpen={isLookupDropdownOpen}
placeholderText={t`Lookup type`}
maxHeight={maxSelectHeight}
noResultsFoundText={t`No results found`}
>
<SelectOption
id="exact-option-select"
key="exact"
value="exact"
description={t`Exact match (default lookup if not specified).`}
/>
<SelectOption
id="iexact-option-select"
key="iexact"
value="iexact"
description={t`Case-insensitive version of exact.`}
/>
<SelectOption
id="contains-option-select"
key="contains"
value="contains"
description={t`Field contains value.`}
/>
<SelectOption
id="icontains-option-select"
key="icontains"
value="icontains"
description={t`Case-insensitive version of contains`}
/>
<SelectOption
id="startswith-option-select"
key="startswith"
value="startswith"
description={t`Field starts with value.`}
/>
<SelectOption
id="istartswith-option-select"
key="istartswith"
value="istartswith"
description={t`Case-insensitive version of startswith.`}
/>
<SelectOption
id="endswith-option-select"
key="endswith"
value="endswith"
description={t`Field ends with value.`}
/>
<SelectOption
id="iendswith-option-select"
key="iendswith"
value="iendswith"
description={t`Case-insensitive version of endswith.`}
/>
<SelectOption
id="regex-option-select"
key="regex"
value="regex"
description={t`Field matches the given regular expression.`}
/>
<SelectOption
id="iregex-option-select"
key="iregex"
value="iregex"
description={t`Case-insensitive version of regex.`}
/>
<SelectOption
id="gt-option-select"
key="gt"
value="gt"
description={t`Greater than comparison.`}
/>
<SelectOption
id="gte-option-select"
key="gte"
value="gte"
description={t`Greater than or equal to comparison.`}
/>
<SelectOption
id="lt-option-select"
key="lt"
value="lt"
description={t`Less than comparison.`}
/>
<SelectOption
id="lte-option-select"
key="lte"
value="lte"
description={t`Less than or equal to comparison.`}
/>
<SelectOption
id="isnull-option-select"
key="isnull"
value="isnull"
description={t`Check whether the given field or related object is null; expects a boolean value.`}
/>
<SelectOption
id="in-option-select"
key="in"
value="in"
description={t`Check whether the given field's value is present in the list provided; expects a comma-separated list of items.`}
/>
</Select>
{relatedSearchKeySelected
? renderRelatedLookupType()
: renderLookupType()}
<InputGroup>
<TextInput
data-cy="advanced-search-text-input"
@ -303,6 +371,7 @@ AdvancedSearch.propTypes = {
relatedSearchableKeys: PropTypes.arrayOf(PropTypes.string),
maxSelectHeight: PropTypes.string,
enableNegativeFiltering: PropTypes.bool,
enableRelatedFuzzyFiltering: PropTypes.bool,
};
AdvancedSearch.defaultProps = {
@ -310,6 +379,7 @@ AdvancedSearch.defaultProps = {
relatedSearchableKeys: [],
maxSelectHeight: '300px',
enableNegativeFiltering: true,
enableRelatedFuzzyFiltering: true,
};
export default AdvancedSearch;

View File

@ -209,6 +209,29 @@ describe('<AdvancedSearch />', () => {
.prop('onKeyDown')({ key: 'Enter', preventDefault: jest.fn });
});
wrapper.update();
expect(advancedSearchMock).toBeCalledWith('baz__name__icontains', 'bar');
jest.clearAllMocks();
act(() => {
wrapper.find('Select[aria-label="Key select"]').invoke('onCreateOption')(
'baz'
);
});
wrapper.update();
act(() => {
wrapper
.find('Select[aria-label="Related search type"]')
.invoke('onSelect')({}, 'search');
wrapper
.find('TextInputBase[aria-label="Advanced search value input"]')
.invoke('onChange')('bar');
});
wrapper.update();
act(() => {
wrapper
.find('TextInputBase[aria-label="Advanced search value input"]')
.prop('onKeyDown')({ key: 'Enter', preventDefault: jest.fn });
});
wrapper.update();
expect(advancedSearchMock).toBeCalledWith('baz__search', 'bar');
});
@ -217,7 +240,7 @@ describe('<AdvancedSearch />', () => {
wrapper = mountWithContexts(
<AdvancedSearch
onSearch={advancedSearchMock}
searchableKeys={[]}
searchableKeys={['foo']}
relatedSearchableKeys={[]}
/>
);
@ -230,6 +253,9 @@ describe('<AdvancedSearch />', () => {
{},
'foo'
);
});
wrapper.update();
act(() => {
wrapper.find('Select[aria-label="Lookup select"]').invoke('onSelect')(
{},
'exact'
@ -253,7 +279,7 @@ describe('<AdvancedSearch />', () => {
wrapper = mountWithContexts(
<AdvancedSearch
onSearch={advancedSearchMock}
searchableKeys={[]}
searchableKeys={['foo']}
relatedSearchableKeys={[]}
/>
);
@ -265,6 +291,9 @@ describe('<AdvancedSearch />', () => {
wrapper.find('Select[aria-label="Key select"]').invoke('onCreateOption')(
'foo'
);
});
wrapper.update();
act(() => {
wrapper.find('Select[aria-label="Lookup select"]').invoke('onSelect')(
{},
'exact'
@ -305,6 +334,9 @@ describe('<AdvancedSearch />', () => {
wrapper.find('Select[aria-label="Key select"]').invoke('onCreateOption')(
'foo'
);
});
wrapper.update();
act(() => {
wrapper.find('Select[aria-label="Lookup select"]').invoke('onSelect')(
{},
'exact'
@ -363,4 +395,34 @@ describe('<AdvancedSearch />', () => {
selectOptions.find('SelectOption[id="and-option-select"]').prop('value')
).toBe('and');
});
test('Remove search option from related search type', () => {
wrapper = mountWithContexts(
<AdvancedSearch
onSearch={jest.fn}
searchableKeys={['foo', 'bar']}
relatedSearchableKeys={['bar', 'baz']}
enableRelatedFuzzyFiltering={false}
/>
);
act(() => {
wrapper.find('Select[aria-label="Key select"]').invoke('onCreateOption')(
'baz'
);
});
wrapper.update();
wrapper
.find('Select[aria-label="Related search type"] SelectToggle')
.simulate('click');
const selectOptions = wrapper.find(
'Select[aria-label="Related search type"] SelectOption'
);
expect(selectOptions).toHaveLength(2);
expect(
selectOptions.find('SelectOption[id="name-option-select"]').prop('value')
).toBe('name__icontains');
expect(
selectOptions.find('SelectOption[id="id-option-select"]').prop('value')
).toBe('id');
});
});

View File

@ -43,6 +43,7 @@ function Search({
isDisabled,
maxSelectHeight,
enableNegativeFiltering,
enableRelatedFuzzyFiltering,
}) {
const [isSearchDropdownOpen, setIsSearchDropdownOpen] = useState(false);
const [searchKey, setSearchKey] = useState(
@ -207,6 +208,7 @@ function Search({
relatedSearchableKeys={relatedSearchableKeys}
maxSelectHeight={maxSelectHeight}
enableNegativeFiltering={enableNegativeFiltering}
enableRelatedFuzzyFiltering={enableRelatedFuzzyFiltering}
/>
)) ||
(options && (
@ -327,6 +329,7 @@ Search.propTypes = {
isDisabled: PropTypes.bool,
maxSelectHeight: PropTypes.string,
enableNegativeFiltering: PropTypes.bool,
enableRelatedFuzzyFiltering: PropTypes.bool,
};
Search.defaultProps = {
@ -335,6 +338,7 @@ Search.defaultProps = {
isDisabled: false,
maxSelectHeight: '300px',
enableNegativeFiltering: true,
enableRelatedFuzzyFiltering: true,
};
export default withRouter(Search);

View File

@ -1,9 +1,7 @@
import React, { useEffect, useCallback } from 'react';
import { useHistory, useLocation, useRouteMatch } from 'react-router-dom';
import { t } from '@lingui/macro';
import { Card, PageSection } from '@patternfly/react-core';
import { HostsAPI } from '../../../api';
import AlertModal from '../../../components/AlertModal';
import DataListToolbar from '../../../components/DataListToolbar';
@ -46,8 +44,15 @@ function HostList() {
}
});
const hasNonDefaultSearchParams =
Object.keys(nonDefaultSearchParams).length > 0;
const hasInvalidHostFilterKeys = () => {
const nonDefaultSearchKeys = Object.keys(nonDefaultSearchParams);
return (
nonDefaultSearchKeys.filter(searchKey => searchKey.startsWith('not__'))
.length > 0 ||
nonDefaultSearchKeys.filter(searchKey => searchKey.endsWith('__search'))
.length > 0
);
};
const {
result: { hosts, count, actions, relatedSearchableKeys, searchableKeys },
@ -185,7 +190,11 @@ function HostList() {
...(canAdd
? [
<SmartInventoryButton
isDisabled={!hasNonDefaultSearchParams}
hasInvalidKeys={hasInvalidHostFilterKeys()}
isDisabled={
Object.keys(nonDefaultSearchParams).length === 0 ||
hasInvalidHostFilterKeys()
}
onClick={() => handleSmartInventoryClick()}
/>,
]

View File

@ -1,52 +1,69 @@
import React from 'react';
import { func } from 'prop-types';
import { bool, func } from 'prop-types';
import { Button, DropdownItem, Tooltip } from '@patternfly/react-core';
import { t } from '@lingui/macro';
import { useKebabifiedMenu } from '../../../contexts/Kebabified';
function SmartInventoryButton({ onClick, isDisabled }) {
function SmartInventoryButton({ onClick, isDisabled, hasInvalidKeys }) {
const { isKebabified } = useKebabifiedMenu();
if (isKebabified) {
const renderTooltipContent = () => {
if (hasInvalidKeys) {
return t`Some search modifiers like not__ and __search are not supported in Smart Inventory host filters. Remove these to create a new Smart Inventory with this filter.`;
}
if (isDisabled) {
return t`Enter at least one search filter to create a new Smart Inventory`;
}
return t`Create a new Smart Inventory with the applied filter`;
};
const renderContent = () => {
if (isKebabified) {
return (
<DropdownItem
key="add"
isDisabled={isDisabled}
component="button"
onClick={onClick}
>
{t`Smart Inventory`}
</DropdownItem>
);
}
return (
<DropdownItem
key="add"
isDisabled={isDisabled}
component="button"
<Button
ouiaId="smart-inventory-button"
onClick={onClick}
aria-label={t`Smart Inventory`}
variant="secondary"
isDisabled={isDisabled}
>
{t`Smart Inventory`}
</DropdownItem>
</Button>
);
}
};
return (
<Tooltip
key="smartInventory"
content={
!isDisabled
? t`Create a new Smart Inventory with the applied filter`
: t`Enter at least one search filter to create a new Smart Inventory`
}
content={renderTooltipContent()}
position="top"
>
<div>
<Button
ouiaId="smart-inventory-button"
onClick={onClick}
aria-label={t`Smart Inventory`}
variant="secondary"
isDisabled={isDisabled}
>
{t`Smart Inventory`}
</Button>
</div>
<div>{renderContent()}</div>
</Tooltip>
);
}
SmartInventoryButton.propTypes = {
hasInvalidKeys: bool,
isDisabled: bool,
onClick: func.isRequired,
};
SmartInventoryButton.defaultProps = {
hasInvalidKeys: false,
isDisabled: false,
};
export default SmartInventoryButton;

View File

@ -82,6 +82,7 @@ const SmartInventoryFormFields = ({ inventory }) => {
isValid={!hostFilterMeta.touched || !hostFilterMeta.error}
isDisabled={!organizationField.value}
enableNegativeFiltering={false}
enableRelatedFuzzyFiltering={false}
/>
<InstanceGroupsLookup
value={instanceGroupsField.value}