Просмотр исходного кода

feat(query-builder): Numeric filter improvements (#72345)

- Adds suggestions for numeric filters (based on the number you type in)
- Allows changing the operator to other things like `<` and `>` instead
of just `is`
- When creating a numeric filter, prefills a value (I chose an arbitrary
`100`) and doesn't allow you to change it to an invalid one. This was
necessary because the parser will not see `timesSeen:` or
`timesSeen:abc` as a numeric filter because it does not have a valid
value. That resulted in some unwanted behavior. We can change the
grammar in the future, but this was a good solution for now.
Malachi Willey 9 месяцев назад
Родитель
Сommit
0464fca1bf

+ 54 - 4
static/app/components/searchQueryBuilder/index.spec.tsx

@@ -22,7 +22,7 @@ const FITLER_KEY_SECTIONS: FilterKeySection[] = [
     value: FieldKind.FIELD,
     label: 'Category 1',
     children: [
-      {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD, predefined: true},
+      {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD},
       {
         key: FieldKey.ASSIGNED,
         name: 'Assigned To',
@@ -56,6 +56,11 @@ const FITLER_KEY_SECTIONS: FilterKeySection[] = [
         alias: 'status',
         predefined: true,
       },
+      {
+        key: FieldKey.TIMES_SEEN,
+        name: 'timesSeen',
+        kind: FieldKind.FIELD,
+      },
     ],
   },
   {
@@ -313,12 +318,12 @@ describe('SearchQueryBuilder', function () {
       await userEvent.click(screen.getByRole('combobox', {name: 'Edit filter value'}));
 
       // Clicking the "+14d" option should update the value
-      await userEvent.click(screen.getByRole('option', {name: '+14d'}));
-      expect(screen.getByRole('row', {name: 'age:+14d'})).toBeInTheDocument();
+      await userEvent.click(screen.getByRole('option', {name: '-14d'}));
+      expect(screen.getByRole('row', {name: 'age:-14d'})).toBeInTheDocument();
       expect(
         within(
           screen.getByRole('button', {name: 'Edit value for filter: age'})
-        ).getByText('+14d')
+        ).getByText('-14d')
       ).toBeInTheDocument();
     });
 
@@ -716,4 +721,49 @@ describe('SearchQueryBuilder', function () {
       ).toBeInTheDocument();
     });
   });
+
+  describe('filter types', function () {
+    describe('numeric', function () {
+      it('new numeric filters start with a value', async function () {
+        render(<SearchQueryBuilder {...defaultProps} />);
+        await userEvent.click(screen.getByRole('grid'));
+        await userEvent.keyboard('time{ArrowDown}{Enter}');
+
+        // Should start with the > operator and a value of 100
+        expect(
+          await screen.findByRole('row', {name: 'timesSeen:>100'})
+        ).toBeInTheDocument();
+      });
+
+      it('does not allow invalid values', async function () {
+        render(<SearchQueryBuilder {...defaultProps} initialQuery="timesSeen:>100" />);
+        await userEvent.click(
+          screen.getByRole('button', {name: 'Edit value for filter: timesSeen'})
+        );
+        await userEvent.keyboard('a{Enter}');
+
+        // Should have the same value because "a" is not a numeric value
+        expect(screen.getByRole('row', {name: 'timesSeen:>100'})).toBeInTheDocument();
+
+        await userEvent.keyboard('{Backspace}7k{Enter}');
+
+        // Should accept "7k" as a valid value
+        expect(
+          await screen.findByRole('row', {name: 'timesSeen:>7k'})
+        ).toBeInTheDocument();
+      });
+
+      it('can change the operator', async function () {
+        render(<SearchQueryBuilder {...defaultProps} initialQuery="timesSeen:>100k" />);
+        await userEvent.click(
+          screen.getByRole('button', {name: 'Edit operator for filter: timesSeen'})
+        );
+        await userEvent.click(screen.getByRole('menuitemradio', {name: '<='}));
+
+        expect(
+          await screen.findByRole('row', {name: 'timesSeen:<=100k'})
+        ).toBeInTheDocument();
+      });
+    });
+  });
 });

+ 1 - 1
static/app/components/searchQueryBuilder/index.stories.tsx

@@ -13,7 +13,7 @@ const FITLER_KEY_SECTIONS: FilterKeySection[] = [
     value: FieldKind.FIELD,
     label: 'Category 1',
     children: [
-      {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD, predefined: true},
+      {key: FieldKey.AGE, name: 'Age', kind: FieldKind.FIELD},
       {
         key: FieldKey.ASSIGNED,
         name: 'Assigned To',

+ 16 - 10
static/app/components/searchQueryBuilder/index.tsx

@@ -72,7 +72,20 @@ export function SearchQueryBuilder({
 }: SearchQueryBuilderProps) {
   const {state, dispatch} = useQueryBuilderState({initialQuery});
 
-  const parsedQuery = useMemo(() => parseQueryBuilderValue(state.query), [state.query]);
+  const keys = useMemo(
+    () =>
+      filterKeySections.reduce((acc, section) => {
+        for (const tag of section.children) {
+          acc[tag.key] = tag;
+        }
+        return acc;
+      }, {}),
+    [filterKeySections]
+  );
+  const parsedQuery = useMemo(
+    () => parseQueryBuilderValue(state.query, {keys}),
+    [keys, state.query]
+  );
 
   useEffectAfterFirstRender(() => {
     dispatch({type: 'UPDATE_QUERY', query: initialQuery});
@@ -83,23 +96,16 @@ export function SearchQueryBuilder({
   }, [onChange, state.query]);
 
   const contextValue = useMemo(() => {
-    const allKeys = filterKeySections.reduce((acc, section) => {
-      for (const tag of section.children) {
-        acc[tag.key] = tag;
-      }
-      return acc;
-    }, {});
-
     return {
       ...state,
       parsedQuery,
       filterKeySections,
-      keys: allKeys,
+      keys,
       getTagValues,
       dispatch,
       onSearch,
     };
-  }, [state, parsedQuery, filterKeySections, getTagValues, dispatch, onSearch]);
+  }, [state, parsedQuery, filterKeySections, keys, getTagValues, dispatch, onSearch]);
 
   return (
     <SearchQueryBuilerContext.Provider value={contextValue}>

+ 27 - 3
static/app/components/searchQueryBuilder/input.tsx

@@ -23,7 +23,7 @@ import {
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Tag} from 'sentry/types/group';
-import {getFieldDefinition} from 'sentry/utils/fields';
+import {FieldValueType, getFieldDefinition} from 'sentry/utils/fields';
 import {toTitleCase} from 'sentry/utils/string/toTitleCase';
 
 type SearchQueryBuilderInputProps = {
@@ -53,6 +53,27 @@ function getWordAtCursorPosition(value: string, cursorPosition: number) {
   return value;
 }
 
+function getInitialFilterText(key: string) {
+  const fieldDef = getFieldDefinition(key);
+
+  if (!fieldDef) {
+    return `${key}:`;
+  }
+
+  switch (fieldDef.valueType) {
+    case FieldValueType.BOOLEAN:
+      return `${key}:true`;
+    case FieldValueType.INTEGER:
+    case FieldValueType.NUMBER:
+      return `${key}:>100`;
+    case FieldValueType.DATE:
+      return `${key}:-24h`;
+    case FieldValueType.STRING:
+    default:
+      return `${key}:`;
+  }
+}
+
 /**
  * Replaces the focused word (at cursorPosition) with the selected filter key.
  *
@@ -72,7 +93,7 @@ function replaceFocusedWordWithFilter(
     if (characterCount >= cursorPosition) {
       return (
         value.slice(0, characterCount - word.length - 1).trim() +
-        ` ${key}: ` +
+        ` ${getInitialFilterText(key)} ` +
         value.slice(characterCount).trim()
       ).trim();
     }
@@ -92,7 +113,10 @@ function replaceAliasedFilterKeys(value: string, aliasToKeyMap: Record<string, s
   const matchedKey = key?.[1];
   if (matchedKey && aliasToKeyMap[matchedKey]) {
     const actualKey = aliasToKeyMap[matchedKey];
-    const replacedValue = value.replace(`${matchedKey}:`, `${actualKey}:`);
+    const replacedValue = value.replace(
+      `${matchedKey}:`,
+      getInitialFilterText(actualKey)
+    );
     return replacedValue;
   }
 

+ 49 - 3
static/app/components/searchQueryBuilder/utils.tsx

@@ -9,17 +9,63 @@ import {
   type ParseResult,
   type ParseResultToken,
   parseSearch,
+  type SearchConfig,
   type TermOperator,
   Token,
   type TokenResult,
 } from 'sentry/components/searchSyntax/parser';
-import type {Tag} from 'sentry/types';
+import type {Tag, TagCollection} from 'sentry/types';
 import {escapeDoubleQuotes} from 'sentry/utils';
+import {FieldValueType, getFieldDefinition} from 'sentry/utils/fields';
 
 export const INTERFACE_TYPE_LOCALSTORAGE_KEY = 'search-query-builder-interface';
 
-export function parseQueryBuilderValue(value: string): ParseResult | null {
-  return collapseTextTokens(parseSearch(value || ' ', {flattenParenGroups: true}));
+function getSearchConfigFromKeys(keys: TagCollection): Partial<SearchConfig> {
+  const config = {
+    booleanKeys: new Set<string>(),
+    numericKeys: new Set<string>(),
+    dateKeys: new Set<string>(),
+    durationKeys: new Set<string>(),
+  } satisfies Partial<SearchConfig>;
+
+  for (const key in keys) {
+    const fieldDef = getFieldDefinition(key);
+    if (!fieldDef) {
+      continue;
+    }
+
+    switch (fieldDef.valueType) {
+      case FieldValueType.BOOLEAN:
+        config.booleanKeys.add(key);
+        break;
+      case FieldValueType.NUMBER:
+      case FieldValueType.INTEGER:
+        config.numericKeys.add(key);
+        break;
+      case FieldValueType.DATE:
+        config.dateKeys.add(key);
+        break;
+      case FieldValueType.DURATION:
+        config.durationKeys.add(key);
+        break;
+      default:
+        break;
+    }
+  }
+
+  return config;
+}
+
+export function parseQueryBuilderValue(
+  value: string,
+  options?: {keys: TagCollection}
+): ParseResult | null {
+  return collapseTextTokens(
+    parseSearch(value || ' ', {
+      flattenParenGroups: true,
+      ...getSearchConfigFromKeys(options?.keys ?? {}),
+    })
+  );
 }
 
 /**

+ 80 - 9
static/app/components/searchQueryBuilder/valueCombobox.tsx

@@ -13,7 +13,12 @@ import {
   formatFilterValue,
   unescapeTagValue,
 } from 'sentry/components/searchQueryBuilder/utils';
-import {FilterType, Token, type TokenResult} from 'sentry/components/searchSyntax/parser';
+import {
+  FilterType,
+  TermOperator,
+  Token,
+  type TokenResult,
+} from 'sentry/components/searchSyntax/parser';
 import type {SearchGroup} from 'sentry/components/smartSearchBar/types';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
@@ -37,22 +42,56 @@ type SuggestionSectionItem = {
   sectionText: string;
 };
 
+const NUMERIC_REGEX = /^-?\d+(\.\d+)?$/;
+const FILTER_VALUE_NUMERIC = /^-?\d+(\.\d+)?[kmb]?$/i;
+const FILTER_VALUE_INT = /^-?\d+[kmb]?$/i;
+
+function isNumeric(value: string) {
+  return NUMERIC_REGEX.test(value);
+}
+
 function isStringFilterValues(
   tagValues: string[] | SearchGroup[]
 ): tagValues is string[] {
   return typeof tagValues[0] === 'string';
 }
 
-function getPredefinedValues({key}: {key?: Tag}): SuggestionSection[] {
+function getPredefinedValues({
+  key,
+  inputValue,
+}: {
+  inputValue: string;
+  key?: Tag;
+}): SuggestionSection[] {
   if (!key) {
     return [];
   }
 
   const fieldDef = getFieldDefinition(key.key);
 
-  if (!key.values) {
+  if (!key.values?.length) {
     switch (fieldDef?.valueType) {
       // TODO(malwilley): Better duration suggestions
+      case FieldValueType.NUMBER:
+        if (!inputValue) {
+          return [{sectionText: '', suggestions: ['100', '100k', '100m', '100b']}];
+        }
+        if (isNumeric(inputValue)) {
+          return [
+            {
+              sectionText: '',
+              suggestions: [
+                inputValue,
+                `${inputValue}k`,
+                `${inputValue}m`,
+                `${inputValue}b`,
+              ],
+            },
+          ];
+        }
+
+        // TODO(malwilley): signal that the value is invalid
+        return [];
       case FieldValueType.DURATION:
         return [{sectionText: '', suggestions: ['-1d', '-7d', '+14d']}];
       case FieldValueType.BOOLEAN:
@@ -96,6 +135,10 @@ function tokenSupportsMultipleValues(
         FieldValueType.INTEGER,
       ].includes(fieldDef?.valueType ?? FieldValueType.STRING);
     case FilterType.NUMERIC:
+      if (token.operator === TermOperator.DEFAULT) {
+        return true;
+      }
+      return false;
     case FilterType.TEXT_IN:
     case FilterType.NUMERIC_IN:
       return true;
@@ -111,6 +154,8 @@ function getOtherSelectedValues(token: TokenResult<Token.FILTER>): string[] {
         return [];
       }
       return [unescapeTagValue(token.value.value)];
+    case Token.VALUE_NUMBER:
+      return token.value.text ? [token.value.text] : [];
     case Token.VALUE_NUMBER_LIST:
       return token.value.items.map(item => item.value?.text ?? '');
     case Token.VALUE_TEXT_LIST:
@@ -120,6 +165,28 @@ function getOtherSelectedValues(token: TokenResult<Token.FILTER>): string[] {
   }
 }
 
+function cleanFilterValue(key: string, value: string): string {
+  const fieldDef = getFieldDefinition(key);
+  if (!fieldDef) {
+    return value;
+  }
+
+  switch (fieldDef.valueType) {
+    case FieldValueType.NUMBER:
+      if (FILTER_VALUE_NUMERIC.test(value)) {
+        return value;
+      }
+      return '';
+    case FieldValueType.INTEGER:
+      if (FILTER_VALUE_INT.test(value)) {
+        return value;
+      }
+      return '';
+    default:
+      return escapeTagValue(value);
+  }
+}
+
 function useFilterSuggestions({
   token,
   inputValue,
@@ -131,7 +198,8 @@ function useFilterSuggestions({
 }) {
   const {getTagValues, keys} = useSearchQueryBuilder();
   const key = keys[token.key.text];
-  const shouldFetchValues = key && !key.predefined;
+  const predefinedValues = getPredefinedValues({key, inputValue});
+  const shouldFetchValues = key && !key.predefined && !predefinedValues.length;
   const canSelectMultipleValues = tokenSupportsMultipleValues(token, keys);
 
   // TODO(malwilley): Display error states
@@ -173,8 +241,8 @@ function useFilterSuggestions({
   const suggestionGroups: SuggestionSection[] = useMemo(() => {
     return shouldFetchValues
       ? [{sectionText: '', suggestions: data ?? []}]
-      : getPredefinedValues({key});
-  }, [data, key, shouldFetchValues]);
+      : predefinedValues;
+  }, [data, predefinedValues, shouldFetchValues]);
 
   // Grouped sections for rendering purposes
   const suggestionSectionItems = useMemo<SuggestionSectionItem[]>(() => {
@@ -276,7 +344,10 @@ export function SearchQueryBuilderValueCombobox({
 
   const handleSelectValue = useCallback(
     (value: string) => {
-      if (!value) {
+      const cleanedValue = cleanFilterValue(token.key.text, value);
+
+      // TODO(malwilley): Add visual feedback for invalid values
+      if (!cleanedValue) {
         return;
       }
 
@@ -284,7 +355,7 @@ export function SearchQueryBuilderValueCombobox({
         dispatch({
           type: 'TOGGLE_FILTER_VALUE',
           token: token,
-          value: escapeTagValue(value),
+          value: cleanedValue,
         });
 
         // If toggling off a value, keep focus inside the value
@@ -295,7 +366,7 @@ export function SearchQueryBuilderValueCombobox({
         dispatch({
           type: 'UPDATE_TOKEN_VALUE',
           token: token.value,
-          value: escapeTagValue(value),
+          value: cleanedValue,
         });
         onCommit();
       }

+ 2 - 2
static/app/components/searchSyntax/utils.tsx

@@ -275,7 +275,7 @@ export function stringifyToken(token: TokenResult<Token>) {
       return `[${textListItems.join(',')}]`;
     case Token.VALUE_NUMBER_LIST:
       const numberListItems = token.items
-        .map(item => (item.value ? item.value.value + item.value.unit : ''))
+        .map(item => (item.value ? item.value.value + (item.value.unit ?? '') : ''))
         .filter(str => str.length > 0);
       return `[${numberListItems.join(',')}]`;
     case Token.KEY_SIMPLE:
@@ -297,7 +297,7 @@ export function stringifyToken(token: TokenResult<Token>) {
     case Token.VALUE_RELATIVE_DATE:
     case Token.VALUE_SIZE:
     case Token.VALUE_NUMBER:
-      return token.value;
+      return token.text;
     default:
       return '';
   }

+ 1 - 1
static/app/utils/fields/index.ts

@@ -520,7 +520,7 @@ const EVENT_FIELD_DEFINITIONS: Record<AllEventFieldKeys, FieldDefinition> = {
   [FieldKey.AGE]: {
     desc: t('The age of the issue in relative time'),
     kind: FieldKind.FIELD,
-    valueType: FieldValueType.DURATION,
+    valueType: FieldValueType.DATE,
   },
   [FieldKey.ASSIGNED]: {
     desc: t('Assignee of the issue as a user ID'),