From ba60a799729621d9a54bef676988782a358d075d Mon Sep 17 00:00:00 2001 From: Malachi Willey Date: Thu, 18 Jul 2024 15:31:08 -0700 Subject: [PATCH] feat(query-builder): Add config for disabling wildcard tokens --- .../app/components/compactSelect/control.tsx | 7 + .../searchQueryBuilder/index.spec.tsx | 41 +++ .../searchQueryBuilder/index.stories.tsx | 42 ++- .../components/searchQueryBuilder/index.tsx | 14 +- .../searchQueryBuilder/tokenizedQueryGrid.tsx | 1 - .../tokens/deletableToken.tsx | 14 +- .../tokens/filter/filter.tsx | 70 +++-- .../tokens/filter/filterKeyOperator.tsx | 9 +- .../searchQueryBuilder/tokens/freeText.tsx | 267 +++++++++++------- .../tokens/invalidTokenTooltip.tsx | 61 ++++ .../components/searchQueryBuilder/utils.tsx | 20 +- 11 files changed, 405 insertions(+), 141 deletions(-) create mode 100644 static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx diff --git a/static/app/components/compactSelect/control.tsx b/static/app/components/compactSelect/control.tsx index 20e243a0b85fe3..8b63cb81284816 100644 --- a/static/app/components/compactSelect/control.tsx +++ b/static/app/components/compactSelect/control.tsx @@ -171,6 +171,10 @@ export interface ControlProps * true). */ onClear?: () => void; + /** + * Called when the menu is opened or closed. + */ + onOpenChange?: (newOpenState: boolean) => void; /** * Called when the search input's value changes (applicable only when `searchable` * is true). @@ -233,6 +237,7 @@ export function Control({ menuHeaderTrailingItems, menuBody, menuFooter, + onOpenChange, // Select props size = 'md', @@ -327,6 +332,8 @@ export function Control({ preventOverflowOptions, flipOptions, onOpenChange: open => { + onOpenChange?.(open); + nextFrameCallback(() => { if (open) { // Focus on search box if present diff --git a/static/app/components/searchQueryBuilder/index.spec.tsx b/static/app/components/searchQueryBuilder/index.spec.tsx index 6cd1116dc81de5..69cdfbdcbaa00a 100644 --- a/static/app/components/searchQueryBuilder/index.spec.tsx +++ b/static/app/components/searchQueryBuilder/index.spec.tsx @@ -1969,4 +1969,45 @@ describe('SearchQueryBuilder', function () { ).toBeInTheDocument(); }); }); + + describe('disallowWildcard', function () { + it('should mark tokens with wildcards invalid', async function () { + render( + + ); + + expect(screen.getByRole('row', {name: 'browser.name:Firefox*'})).toHaveAttribute( + 'aria-invalid', + 'true' + ); + + // Put focus into token, should show error message + await userEvent.click(getLastInput()); + await userEvent.keyboard('{ArrowLeft}'); + + expect( + await screen.findByText('Wildcards not supported in search') + ).toBeInTheDocument(); + }); + + it('should mark free text with wildcards invalid', async function () { + render( + + ); + + expect(screen.getByRole('row', {name: 'foo*'})).toHaveAttribute( + 'aria-invalid', + 'true' + ); + + await userEvent.click(getLastInput()); + expect( + await screen.findByText('Wildcards not supported in search') + ).toBeInTheDocument(); + }); + }); }); diff --git a/static/app/components/searchQueryBuilder/index.stories.tsx b/static/app/components/searchQueryBuilder/index.stories.tsx index c00e9960198e7b..1eb1e4604afb88 100644 --- a/static/app/components/searchQueryBuilder/index.stories.tsx +++ b/static/app/components/searchQueryBuilder/index.stories.tsx @@ -1,7 +1,8 @@ -import {Fragment} from 'react'; +import {Fragment, useState} from 'react'; import styled from '@emotion/styled'; import Alert from 'sentry/components/alert'; +import MultipleCheckbox from 'sentry/components/forms/controls/multipleCheckbox'; import {SearchQueryBuilder} from 'sentry/components/searchQueryBuilder'; import type {FilterKeySection} from 'sentry/components/searchQueryBuilder/types'; import SizingWindow from 'sentry/components/stories/sizingWindow'; @@ -111,6 +112,45 @@ export default storyBook(SearchQueryBuilder, story => { ); }); + + story('Config Options', () => { + const configs = ['disallowLogicalOperators', 'disallowWildcard']; + + const [enabledConfigs, setEnabledConfigs] = useState([...configs]); + const queryBuilderOptions = enabledConfigs.reduce((acc, config) => { + acc[config] = true; + return acc; + }, {}); + + return ( + +

+ There are some config options which allow you to customize which types of syntax + are considered valid. This should be used when the search backend does not + support certain operators like boolean logic or wildcards. +

+ + {configs.map(config => ( + + {config} + + ))} + + + + ); + }); }); const MinHeightSizingWindow = styled(SizingWindow)` diff --git a/static/app/components/searchQueryBuilder/index.tsx b/static/app/components/searchQueryBuilder/index.tsx index 7083b9acf74126..0779b22011039c 100644 --- a/static/app/components/searchQueryBuilder/index.tsx +++ b/static/app/components/searchQueryBuilder/index.tsx @@ -44,6 +44,10 @@ export interface SearchQueryBuilderProps { * When true, parens and logical operators (AND, OR) will be marked as invalid. */ disallowLogicalOperators?: boolean; + /** + * When true, the wildcard (*) in filter values or free text will be marked as invalid. + */ + disallowWildcard?: boolean; /** * The lookup strategy for field definitions. * Each SearchQueryBuilder instance can support a different list of fields and @@ -92,6 +96,7 @@ function ActionButtons() { export function SearchQueryBuilder({ className, disallowLogicalOperators, + disallowWildcard, label, initialQuery, fieldDefinitionGetter = getFieldDefinition, @@ -113,9 +118,16 @@ export function SearchQueryBuilder({ () => parseQueryBuilderValue(state.query, fieldDefinitionGetter, { disallowLogicalOperators, + disallowWildcard, filterKeys, }), - [disallowLogicalOperators, fieldDefinitionGetter, filterKeys, state.query] + [ + disallowLogicalOperators, + disallowWildcard, + fieldDefinitionGetter, + filterKeys, + state.query, + ] ); useEffectAfterFirstRender(() => { diff --git a/static/app/components/searchQueryBuilder/tokenizedQueryGrid.tsx b/static/app/components/searchQueryBuilder/tokenizedQueryGrid.tsx index e04438f008ba63..c7f4c9393d0913 100644 --- a/static/app/components/searchQueryBuilder/tokenizedQueryGrid.tsx +++ b/static/app/components/searchQueryBuilder/tokenizedQueryGrid.tsx @@ -86,7 +86,6 @@ function Grid(props: GridProps) { /> ); case Token.FREE_TEXT: - case Token.SPACES: return ( {children} - + - + ); } diff --git a/static/app/components/searchQueryBuilder/tokens/filter/filter.tsx b/static/app/components/searchQueryBuilder/tokens/filter/filter.tsx index b1b8e60e46c7de..de2c813eb342fc 100644 --- a/static/app/components/searchQueryBuilder/tokens/filter/filter.tsx +++ b/static/app/components/searchQueryBuilder/tokens/filter/filter.tsx @@ -13,6 +13,7 @@ import {FilterKeyOperator} from 'sentry/components/searchQueryBuilder/tokens/fil import {useFilterButtonProps} from 'sentry/components/searchQueryBuilder/tokens/filter/useFilterButtonProps'; import {formatFilterValue} from 'sentry/components/searchQueryBuilder/tokens/filter/utils'; import {SearchQueryBuilderValueCombobox} from 'sentry/components/searchQueryBuilder/tokens/filter/valueCombobox'; +import {InvalidTokenTooltip} from 'sentry/components/searchQueryBuilder/tokens/invalidTokenTooltip'; import { type ParseResultToken, Token, @@ -31,6 +32,7 @@ interface SearchQueryTokenProps { interface FilterValueProps extends SearchQueryTokenProps { filterRef: React.RefObject; + onActiveChange: (active: boolean) => void; } function FilterValueText({token}: {token: TokenResult}) { @@ -81,7 +83,7 @@ function FilterValueText({token}: {token: TokenResult}) { } } -function FilterValue({token, state, item, filterRef}: FilterValueProps) { +function FilterValue({token, state, item, filterRef, onActiveChange}: FilterValueProps) { const ref = useRef(null); const {dispatch, focusOverride} = useSearchQueryBuilder(); @@ -94,9 +96,10 @@ function FilterValue({token, state, item, filterRef}: FilterValueProps) { focusOverride.part === 'value' ) { setIsEditing(true); + onActiveChange(true); dispatch({type: 'RESET_FOCUS_OVERRIDE'}); } - }, [dispatch, focusOverride, isEditing, item.key]); + }, [dispatch, focusOverride, isEditing, item.key, onActiveChange]); const {focusWithinProps} = useFocusWithin({ onBlurWithin: () => { @@ -116,9 +119,11 @@ function FilterValue({token, state, item, filterRef}: FilterValueProps) { filterRef.current?.focus(); state.selectionManager.setFocusedKey(item.key); setIsEditing(false); + onActiveChange(false); }} onCommit={() => { setIsEditing(false); + onActiveChange(false); if (state.collection.getKeyAfter(item.key)) { state.selectionManager.setFocusedKey( state.collection.getKeyAfter(item.key) @@ -133,7 +138,10 @@ function FilterValue({token, state, item, filterRef}: FilterValueProps) { return ( setIsEditing(true)} + onClick={() => { + setIsEditing(true); + onActiveChange(true); + }} {...filterButtonProps} > @@ -160,6 +168,7 @@ function FilterDelete({token, state, item}: SearchQueryTokenProps) { export function SearchQueryBuilderFilter({item, state, token}: SearchQueryTokenProps) { const ref = useRef(null); + const [filterMenuOpen, setFilterMenuOpen] = useState(false); const isFocused = item.key === state.selectionManager.focusedKey; @@ -185,38 +194,52 @@ export function SearchQueryBuilderFilter({item, state, token}: SearchQueryTokenP onKeyDown, }); - // TODO(malwilley): Add better error messaging const tokenHasError = 'invalid' in token && defined(token.invalid); return ( - - - - - - - - - + + + + + + + + + + + ); } const FilterWrapper = styled('div')` position: relative; - display: grid; - grid-template-columns: auto auto auto auto; - align-items: stretch; border: 1px solid ${p => p.theme.innerBorder}; border-radius: ${p => p.theme.borderRadius}; height: 24px; - /* Ensures that filters do not grow outside of the container */ min-width: 0; @@ -228,6 +251,17 @@ const FilterWrapper = styled('div')` &[aria-selected='true'] { background-color: ${p => p.theme.gray100}; } + + &[aria-invalid='true'] { + border-color: ${p => p.theme.red400}; + } +`; + +const GridInvalidTokenTooltip = styled(InvalidTokenTooltip)` + display: grid; + grid-template-columns: auto auto auto auto; + align-items: stretch; + height: 22px; `; const BaseGridCell = styled('div')` diff --git a/static/app/components/searchQueryBuilder/tokens/filter/filterKeyOperator.tsx b/static/app/components/searchQueryBuilder/tokens/filter/filterKeyOperator.tsx index 00ce337586689c..cd7d284140bbfc 100644 --- a/static/app/components/searchQueryBuilder/tokens/filter/filterKeyOperator.tsx +++ b/static/app/components/searchQueryBuilder/tokens/filter/filterKeyOperator.tsx @@ -24,6 +24,7 @@ import useOrganization from 'sentry/utils/useOrganization'; type FilterOperatorProps = { item: Node; + onOpenChange: (isOpen: boolean) => void; state: ListState; token: TokenResult; }; @@ -185,7 +186,12 @@ function getOperatorInfo(token: TokenResult): { }; } -export function FilterKeyOperator({token, state, item}: FilterOperatorProps) { +export function FilterKeyOperator({ + token, + state, + item, + onOpenChange, +}: FilterOperatorProps) { const organization = useOrganization(); const {dispatch, searchSource, query, savedSearchType} = useSearchQueryBuilder(); const filterButtonProps = useFilterButtonProps({state, item}); @@ -206,6 +212,7 @@ export function FilterKeyOperator({token, state, item}: FilterOperatorProps) { size="sm" options={options} value={operator} + onOpenChange={onOpenChange} onChange={option => { trackAnalytics('search.operator_autocompleted', { organization, diff --git a/static/app/components/searchQueryBuilder/tokens/freeText.tsx b/static/app/components/searchQueryBuilder/tokens/freeText.tsx index 52f72f17f34652..2f5b4f591b63e0 100644 --- a/static/app/components/searchQueryBuilder/tokens/freeText.tsx +++ b/static/app/components/searchQueryBuilder/tokens/freeText.tsx @@ -11,6 +11,7 @@ import {useSearchQueryBuilder} from 'sentry/components/searchQueryBuilder/contex import {useQueryBuilderGridItem} from 'sentry/components/searchQueryBuilder/hooks/useQueryBuilderGridItem'; import {replaceTokensWithPadding} from 'sentry/components/searchQueryBuilder/hooks/useQueryBuilderState'; import {SearchQueryBuilderCombobox} from 'sentry/components/searchQueryBuilder/tokens/combobox'; +import {InvalidTokenTooltip} from 'sentry/components/searchQueryBuilder/tokens/invalidTokenTooltip'; import { getDefaultFilterValue, useShiftFocusToChild, @@ -38,15 +39,14 @@ import useOrganization from 'sentry/utils/useOrganization'; type SearchQueryBuilderInputProps = { item: Node; state: ListState; - token: TokenResult | TokenResult; + token: TokenResult; }; type SearchQueryBuilderInputInternalProps = { item: Node; rowRef: React.RefObject; state: ListState; - tabIndex: number; - token: TokenResult | TokenResult; + token: TokenResult; }; type KeyItem = { @@ -261,10 +261,36 @@ function KeyDescription({tag}: {tag: Tag}) { ); } +function InvalidText({ + token, + state, + item, + inputValue, +}: { + inputValue: string; + item: Node; + state: ListState; + token: TokenResult; +}) { + // Because the text input may be larger than the actual text, we use a div + // with the same text contents to determine where the tooltip should be + // positioned. + return ( + + {inputValue} + + ); +} + function SearchQueryBuilderInputInternal({ item, token, - tabIndex, state, rowRef, }: SearchQueryBuilderInputInternalProps) { @@ -273,6 +299,8 @@ function SearchQueryBuilderInputInternal({ const trimmedTokenValue = token.text.trim(); const [inputValue, setInputValue] = useState(trimmedTokenValue); const [selectionIndex, setSelectionIndex] = useState(0); + const isFocused = + state.selectionManager.isFocused && item.key === state.selectionManager.focusedKey; const updateSelectionIndex = useCallback(() => { setSelectionIndex(inputRef.current?.selectionStart ?? 0); @@ -379,112 +407,116 @@ function SearchQueryBuilderInputInternal({ }, [updateSelectionIndex]); return ( - { - dispatch({ - type: 'UPDATE_FREE_TEXT', - tokens: [token], - text: replaceFocusedWordWithFilter( - inputValue, - selectionIndex, - value, - getFieldDefinition - ), - focusOverride: calculateNextFocusForFilter(state), - }); - resetInputValue(); - const selectedKey = filterKeys[value]; - trackAnalytics('search.key_autocompleted', { - organization, - search_type: savedSearchType === 0 ? 'issues' : 'events', - search_source: searchSource, - item_name: value, - item_kind: selectedKey?.kind ?? FieldKind.FIELD, - item_value_type: getFieldDefinition(value)?.valueType ?? FieldValueType.STRING, - filtered: Boolean(filterValue), - new_experience: true, - }); - }} - onCustomValueBlurred={value => { - dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: value}); - resetInputValue(); - }} - onCustomValueCommitted={value => { - dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: value}); - resetInputValue(); - - // Because the query does not change until a subsequent render, - // we need to do the replacement that is does in the reducer here - handleSearch(replaceTokensWithPadding(query, [token], value)); - }} - onExit={() => { - if (inputValue !== token.value.trim()) { - dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: inputValue}); - resetInputValue(); - } - }} - inputValue={inputValue} - token={token} - inputLabel={t('Add a search term')} - onInputChange={e => { - if (e.target.value.includes('(') || e.target.value.includes(')')) { + + { dispatch({ type: 'UPDATE_FREE_TEXT', tokens: [token], - text: e.target.value, - focusOverride: calculateNextFocusForParen(item), + text: replaceFocusedWordWithFilter( + inputValue, + selectionIndex, + value, + getFieldDefinition + ), + focusOverride: calculateNextFocusForFilter(state), }); resetInputValue(); - return; - } - - if (e.target.value.includes(':')) { - dispatch({ - type: 'UPDATE_FREE_TEXT', - tokens: [token], - text: e.target.value, - focusOverride: calculateNextFocusForFilter(state), + const selectedKey = filterKeys[value]; + trackAnalytics('search.key_autocompleted', { + organization, + search_type: savedSearchType === 0 ? 'issues' : 'events', + search_source: searchSource, + item_name: value, + item_kind: selectedKey?.kind ?? FieldKind.FIELD, + item_value_type: + getFieldDefinition(value)?.valueType ?? FieldValueType.STRING, + filtered: Boolean(filterValue), + new_experience: true, }); + }} + onCustomValueBlurred={value => { + dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: value}); resetInputValue(); - return; - } + }} + onCustomValueCommitted={value => { + dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: value}); + resetInputValue(); + + // Because the query does not change until a subsequent render, + // we need to do the replacement that is does in the reducer here + handleSearch(replaceTokensWithPadding(query, [token], value)); + }} + onExit={() => { + if (inputValue !== token.value.trim()) { + dispatch({type: 'UPDATE_FREE_TEXT', tokens: [token], text: inputValue}); + resetInputValue(); + } + }} + inputValue={inputValue} + token={token} + inputLabel={t('Add a search term')} + onInputChange={e => { + if (e.target.value.includes('(') || e.target.value.includes(')')) { + dispatch({ + type: 'UPDATE_FREE_TEXT', + tokens: [token], + text: e.target.value, + focusOverride: calculateNextFocusForParen(item), + }); + resetInputValue(); + return; + } - setInputValue(e.target.value); - setSelectionIndex(e.target.selectionStart ?? 0); - }} - onKeyDown={onKeyDown} - tabIndex={tabIndex} - maxOptions={50} - onPaste={onPaste} - displayTabbedMenu={inputValue.length === 0 && filterKeySections.length > 0} - shouldFilterResults={false} - shouldCloseOnInteractOutside={el => { - if (rowRef.current?.contains(el)) { - return false; + if (e.target.value.includes(':')) { + dispatch({ + type: 'UPDATE_FREE_TEXT', + tokens: [token], + text: e.target.value, + focusOverride: calculateNextFocusForFilter(state), + }); + resetInputValue(); + return; + } + + setInputValue(e.target.value); + setSelectionIndex(e.target.selectionStart ?? 0); + }} + onKeyDown={onKeyDown} + tabIndex={isFocused ? 0 : -1} + maxOptions={50} + onPaste={onPaste} + displayTabbedMenu={inputValue.length === 0 && filterKeySections.length > 0} + shouldFilterResults={false} + shouldCloseOnInteractOutside={el => { + if (rowRef.current?.contains(el)) { + return false; + } + return true; + }} + onClick={onClick} + > + {keyItem => + isSection(keyItem) ? ( +
+ {keyItem.options.map(child => ( + + {child.label} + + ))} +
+ ) : ( + + {keyItem.label} + + ) } - return true; - }} - onClick={onClick} - > - {keyItem => - isSection(keyItem) ? ( -
- {keyItem.options.map(child => ( - - {child.label} - - ))} -
- ) : ( - - {keyItem.label} - - ) - } - + + + ); } @@ -502,16 +534,20 @@ export function SearchQueryBuilderFreeText({ const {rowProps, gridCellProps} = useQueryBuilderGridItem(item, state, ref); const {shiftFocusProps} = useShiftFocusToChild(item, state); - const isFocused = item.key === state.selectionManager.focusedKey; + const isInvalid = Boolean(token.invalid); return ( - + e.stopPropagation()}> @@ -530,6 +566,12 @@ const Row = styled('div')` flex-grow: 1; } + &[aria-invalid='true'] { + input { + color: ${p => p.theme.red400}; + } + } + &[aria-selected='true'] { &::before { content: ''; @@ -550,6 +592,7 @@ const Row = styled('div')` `; const GridCell = styled('div')` + position: relative; display: flex; align-items: stretch; height: 100%; @@ -585,3 +628,19 @@ const Term = styled('dt')` `; const Details = styled('dd')``; + +const PositionedTooltip = styled(InvalidTokenTooltip)` + position: absolute; + z-index: -1; + top: 0; + left: 0; + height: 100%; +`; + +const InvisibleText = styled('div')` + color: transparent; + visibility: hidden; + padding: 0 ${space(0.5)}; + min-width: 9px; + height: 100%; +`; diff --git a/static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx b/static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx new file mode 100644 index 00000000000000..219a92ef15dba2 --- /dev/null +++ b/static/app/components/searchQueryBuilder/tokens/invalidTokenTooltip.tsx @@ -0,0 +1,61 @@ +import type {ReactNode} from 'react'; +import type {ListState} from '@react-stately/list'; +import type {Node} from '@react-types/shared'; + +import type {ParseResultToken} from 'sentry/components/searchSyntax/parser'; +import {Tooltip, type TooltipProps} from 'sentry/components/tooltip'; +import {t} from 'sentry/locale'; +import {defined} from 'sentry/utils'; + +interface InvalidTokenTooltipProps extends Omit { + children: ReactNode; + item: Node; + state: ListState; + token: ParseResultToken; +} + +function getForceVisible({ + isFocused, + isInvalid, + forceVisible, +}: { + isFocused: boolean; + isInvalid: boolean; + forceVisible?: boolean; +}) { + if (!isInvalid) { + return false; + } + + if (defined(forceVisible)) { + return forceVisible; + } + + return isFocused ? true : undefined; +} + +export function InvalidTokenTooltip({ + children, + token, + state, + item, + forceVisible, + ...tooltipProps +}: InvalidTokenTooltipProps) { + const invalid = 'invalid' in token ? token.invalid : null; + const isInvalid = Boolean(invalid); + const isFocused = + state.selectionManager.isFocused && state.selectionManager.focusedKey === item.key; + + return ( + + {children} + + ); +} diff --git a/static/app/components/searchQueryBuilder/utils.tsx b/static/app/components/searchQueryBuilder/utils.tsx index d3dfaf692da8e3..3314e464d49956 100644 --- a/static/app/components/searchQueryBuilder/utils.tsx +++ b/static/app/components/searchQueryBuilder/utils.tsx @@ -63,11 +63,16 @@ function getSearchConfigFromKeys( export function parseQueryBuilderValue( value: string, getFieldDefinition: FieldDefinitionGetter, - options?: {filterKeys: TagCollection; disallowLogicalOperators?: boolean} + options?: { + filterKeys: TagCollection; + disallowLogicalOperators?: boolean; + disallowWildcard?: boolean; + } ): ParseResult | null { return collapseTextTokens( parseSearch(value || ' ', { flattenParenGroups: true, + disallowWildcard: options?.disallowWildcard, disallowedLogicalOperators: options?.disallowLogicalOperators ? new Set([BooleanOperator.AND, BooleanOperator.OR]) : undefined, @@ -124,9 +129,16 @@ function collapseTextTokens(tokens: ParseResult | null) { const lastToken = acc[acc.length - 1]; if (isSimpleTextToken(token) && isSimpleTextToken(lastToken)) { - lastToken.value += token.value; - lastToken.text += token.text; - lastToken.location.end = token.location.end; + const freeTextToken = lastToken as TokenResult; + freeTextToken.value += token.value; + freeTextToken.text += token.text; + freeTextToken.location.end = token.location.end; + + if (token.type === Token.FREE_TEXT) { + freeTextToken.quoted = freeTextToken.quoted || token.quoted; + freeTextToken.invalid = freeTextToken.invalid ?? token.invalid; + } + return acc; }