ref(js): Remove `queryToObj` and `objToQuery` use `parseSearch` and add `joinQuery` respectively (#36459)

* ref(js): Remove  and  use  and add  respectively

* ref(js): Remove old query parsing use the new grammer parser

* fix test

* remove old obj

* no need to reinvent join

* rename back to parseSearch

* name parseSearch correctly

static/app/components/searchSyntax/parser.tsx

@@ -818,3 +818,26 @@ export function parseSearch(query: string): ParseResult | null {
 
   return null;
 }
+
+/**
+ * Join a parsed query array into a string.
+ * Should handle null cases to chain easily with parseSearch.
+ * Option to add a leading space when applicable (e.g. to combine with other strings).
+ * Option to add a space between elements (e.g. for when no Token.Spaces present).
+ */
+export function joinQuery(
+  parsedTerms: ParseResult | null | undefined,
+  leadingSpace?: boolean,
+  additionalSpaceBetween?: boolean
+): string {
+  if (!parsedTerms || !parsedTerms.length) {
+    return '';
+  }
+
+  return (
+    (leadingSpace ? ' ' : '') +
+    (parsedTerms.length === 1
+      ? parsedTerms[0].text
+      : parsedTerms.map(p => p.text).join(additionalSpaceBetween ? ' ' : ''))
+  );
+}

static/app/components/stream/group.tsx

@@ -16,6 +16,7 @@ import {getRelativeSummary} from 'sentry/components/organizations/timeRangeSelec
 import {PanelItem} from 'sentry/components/panels';
 import Placeholder from 'sentry/components/placeholder';
 import ProgressBar from 'sentry/components/progressBar';
+import {joinQuery, parseSearch, Token} from 'sentry/components/searchSyntax/parser';
 import GroupChart from 'sentry/components/stream/groupChart';
 import GroupCheckBox from 'sentry/components/stream/groupCheckBox';
 import TimeSince from 'sentry/components/timeSince';
@@ -37,7 +38,6 @@ import {defined, percent, valueIsEqual} from 'sentry/utils';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 import {callIfFunction} from 'sentry/utils/callIfFunction';
 import EventView from 'sentry/utils/discover/eventView';
-import {queryToObj} from 'sentry/utils/stream';
 import withOrganization from 'sentry/utils/withOrganization';
 import withPageFilters from 'sentry/utils/withPageFilters';
 import {TimePeriodType} from 'sentry/views/alerts/rules/metric/details/constants';
@@ -225,28 +225,16 @@ class StreamGroup extends Component<Props, State> {
     // when there is no discover feature open events page
     const hasDiscoverQuery = organization.features.includes('discover-basic');
 
-    const queryTerms: string[] = [];
-
-    if (isFiltered && typeof query === 'string') {
-      const queryObj = queryToObj(query);
-      for (const queryTag in queryObj) {
-        if (!DISCOVER_EXCLUSION_FIELDS.includes(queryTag)) {
-          const queryVal = queryObj[queryTag].includes(' ')
-            ? `"${queryObj[queryTag]}"`
-            : queryObj[queryTag];
-          queryTerms.push(`${queryTag}:${queryVal}`);
-        }
-      }
-
-      if (queryObj.__text) {
-        queryTerms.push(queryObj.__text);
-      }
-    }
+    const parsedResult = parseSearch(
+      isFiltered && typeof query === 'string' ? query : ''
+    );
+    const filteredTerms = parsedResult?.filter(
+      p => !(p.type === Token.Filter && DISCOVER_EXCLUSION_FIELDS.includes(p.key.text))
+    );
+    const filteredQuery = joinQuery(filteredTerms, true);
 
     const commonQuery = {projects: [Number(data.project.id)]};
 
-    const searchQuery = (queryTerms.length ? ' ' : '') + queryTerms.join(' ');
-
     if (hasDiscoverQuery) {
       const {period, start, end, utc} = customStatsPeriod ?? (selection.datetime || {});
 
@@ -256,7 +244,7 @@ class StreamGroup extends Component<Props, State> {
         name: data.title || data.type,
         fields: ['title', 'release', 'environment', 'user', 'timestamp'],
         orderby: '-timestamp',
-        query: `issue.id:${data.id}${searchQuery}`,
+        query: `issue.id:${data.id}${filteredQuery}`,
         version: 2,
       };
 
@@ -278,7 +266,7 @@ class StreamGroup extends Component<Props, State> {
       pathname: `/organizations/${organization.slug}/issues/${data.id}/events/`,
       query: {
         ...commonQuery,
-        query: searchQuery,
+        query: filteredQuery,
       },
     };
   }

static/app/utils/stream.tsx

@@ -1,66 +0,0 @@
-/**
- * Converts a stream query to an object representation, with
- * keys representing tag names, and the magic __text key
- * representing the text component of the search.
- *
- * Example:
- *
- * "python is:unresolved assigned:foo@bar.com"
- * => {
- *      __text: "python",
- *      is: "unresolved",
- *      assigned: "foo@bar.com"
- *    }
- */
-
-export type QueryObj = Record<string, string>;
-
-export function queryToObj(queryStr = ''): QueryObj {
-  const text: string[] = [];
-
-  const queryItems = queryStr.match(/\S+:"[^"]*"?|\S+/g);
-  const queryObj: QueryObj = (queryItems || []).reduce((obj, item) => {
-    const index = item.indexOf(':');
-    if (index === -1) {
-      text.push(item);
-    } else {
-      const tagKey = item.slice(0, index);
-      const value = item.slice(index + 1).replace(/^"|"$/g, '');
-      obj[tagKey] = value;
-    }
-    return obj;
-  }, {});
-
-  queryObj.__text = '';
-  if (text.length) {
-    queryObj.__text = text.join(' ');
-  }
-
-  return queryObj;
-}
-
-/**
- * Converts an object representation of a stream query to a string
- * (consumable by the Sentry stream HTTP API).
- */
-export function objToQuery(queryObj: QueryObj): string {
-  const {__text, ...tags} = queryObj;
-
-  const parts = Object.entries(tags).map(([tagKey, value]) => {
-    if (
-      value.indexOf(' ') > -1 &&
-      value.indexOf('[') === -1 &&
-      value.indexOf(']') === -1
-    ) {
-      value = `"${value}"`;
-    }
-
-    return `${tagKey}:${value}`;
-  });
-
-  if (queryObj.__text) {
-    parts.push(queryObj.__text);
-  }
-
-  return parts.join(' ');
-}

static/app/views/dashboardsV2/datasetConfig/issues.tsx

@@ -1,11 +1,11 @@
 import {Client} from 'sentry/api';
+import {joinQuery, parseSearch, Token} from 'sentry/components/searchSyntax/parser';
 import {t} from 'sentry/locale';
 import GroupStore from 'sentry/stores/groupStore';
 import {Group, Organization, PageFilters} from 'sentry/types';
 import {getIssueFieldRenderer} from 'sentry/utils/dashboards/issueFieldRenderers';
 import {getUtcDateString} from 'sentry/utils/dates';
 import {TableData, TableDataRow} from 'sentry/utils/discover/discoverQuery';
-import {queryToObj} from 'sentry/utils/stream';
 import {
   DISCOVER_EXCLUSION_FIELDS,
   getSortLabel,
@@ -136,24 +136,12 @@ export function transformIssuesResponseToTable(
 
       // Discover Url properties
       const query = widgetQuery.conditions;
-      const queryTerms: string[] = [];
-      if (typeof query === 'string') {
-        const queryObj = queryToObj(query);
-        for (const queryTag in queryObj) {
-          if (!DISCOVER_EXCLUSION_FIELDS.includes(queryTag)) {
-            const queryVal = queryObj[queryTag].includes(' ')
-              ? `"${queryObj[queryTag]}"`
-              : queryObj[queryTag];
-            queryTerms.push(`${queryTag}:${queryVal}`);
-          }
-        }
-
-        if (queryObj.__text) {
-          queryTerms.push(queryObj.__text);
-        }
-      }
-      transformedTableResult.discoverSearchQuery =
-        (queryTerms.length ? ' ' : '') + queryTerms.join(' ');
+      const parsedResult = parseSearch(query);
+      const filteredTerms = parsedResult?.filter(
+        p => !(p.type === Token.Filter && DISCOVER_EXCLUSION_FIELDS.includes(p.key.text))
+      );
+
+      transformedTableResult.discoverSearchQuery = joinQuery(filteredTerms, true);
       transformedTableResult.projectId = project.id;
 
       const {period, start, end} = pageFilters.datetime || {};

static/app/views/issueList/overview.tsx

@@ -28,6 +28,7 @@ import {extractSelectionParameters} from 'sentry/components/organizations/pageFi
 import Pagination, {CursorHandler} from 'sentry/components/pagination';
 import {Panel, PanelBody} from 'sentry/components/panels';
 import QueryCount from 'sentry/components/queryCount';
+import {parseSearch} from 'sentry/components/searchSyntax/parser';
 import StreamGroup from 'sentry/components/stream/group';
 import ProcessingIssueList from 'sentry/components/stream/processingIssueList';
 import {DEFAULT_QUERY, DEFAULT_STATS_PERIOD} from 'sentry/constants';
@@ -1300,6 +1301,7 @@ class IssueListOverview extends Component<Props, State> {
                   loading={tagsLoading}
                   tags={tags}
                   query={query}
+                  parsedQuery={parseSearch(query) || []}
                   onQueryChange={this.onIssueListSidebarSearch}
                   tagValueLoader={this.tagValueLoader}
                 />

static/app/views/issueList/sidebar.tsx

@@ -6,6 +6,7 @@ import map from 'lodash/map';
 import Input from 'sentry/components/forms/controls/input';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {
+  joinQuery,
   ParseResult,
   parseSearch,
   Token,
@@ -16,7 +17,6 @@ import {IconClose} from 'sentry/icons/iconClose';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {Tag, TagCollection} from 'sentry/types';
-import {objToQuery} from 'sentry/utils/stream';
 
 import IssueListTagFilter from './tagFilter';
 import {TagValueLoader} from './types';
@@ -28,12 +28,13 @@ type DefaultProps = {
 };
 
 type Props = DefaultProps & {
+  parsedQuery: ParseResult;
   tagValueLoader: TagValueLoader;
   loading?: boolean;
 };
 
 type State = {
-  queryObj: Record<string, string>;
+  filters: Record<string, TokenResult<Token.Filter>>;
   textFilter: string;
 };
 
@@ -44,48 +45,44 @@ class IssueListSidebar extends Component<Props, State> {
     onQueryChange: function () {},
   };
 
-  state: State = this.parseQueryToState(this.props.query);
+  state: State = this.parsedQueryToState(this.props.parsedQuery);
 
   componentWillReceiveProps(nextProps: Props) {
-    // If query was updated by another source (e.g. SearchBar),
-    // clobber state of sidebar with new query.
-    const query = objToQuery(this.state.queryObj);
-
-    if (!isEqual(nextProps.query, query)) {
-      this.setState(this.parseQueryToState(nextProps.query));
+    if (!isEqual(nextProps.query, this.props.query)) {
+      this.setState(this.parsedQueryToState(nextProps.parsedQuery));
     }
   }
 
-  parseQueryToState(query: string): State {
-    const parsedResult: ParseResult = parseSearch(query) ?? [];
-    const textFilter = parsedResult
-      ?.filter(p => p.type === Token.FreeText)
-      .map(p => p.text)
-      .join(' ');
-    const parsedFilers = parsedResult?.filter(
+  parsedQueryToState(parsedQuery: ParseResult): State {
+    const parsedFilters = parsedQuery.filter(
       (p): p is TokenResult<Token.Filter> => p.type === Token.Filter
     );
-    const queryObj = Object.fromEntries(
-      parsedFilers.map((p: TokenResult<Token.Filter>) => [p.key.text, p.value.text])
-    );
 
     return {
-      queryObj,
-      textFilter,
+      filters: Object.fromEntries(parsedFilters.map(p => [p.key.text, p])),
+      textFilter: joinQuery(parsedQuery.filter(p => p.type === Token.FreeText)),
     };
   }
 
   onSelectTag = (tag: Tag, value: string | null) => {
-    const newQuery = {...this.state.queryObj};
+    const parsedResult: TokenResult<Token.Filter>[] = (
+      parseSearch(`${tag.key}:${value}`) ?? []
+    ).filter((p): p is TokenResult<Token.Filter> => p.type === Token.Filter);
+    if (parsedResult.length !== 1 || parsedResult[0].type !== Token.Filter) {
+      return;
+    }
+    const newEntry = parsedResult[0] as TokenResult<Token.Filter>;
+    const newFilters = {...this.state.filters};
+
     if (value) {
-      newQuery[tag.key] = value;
+      newFilters[tag.key] = newEntry;
     } else {
-      delete newQuery[tag.key];
+      delete newFilters[tag.key];
     }
 
     this.setState(
       {
-        queryObj: newQuery,
+        filters: newFilters,
       },
       this.onQueryChange
     );
@@ -95,25 +92,15 @@ class IssueListSidebar extends Component<Props, State> {
     this.setState({textFilter: evt.target.value});
   };
 
-  onTextFilterSubmit = (evt?: React.FormEvent<HTMLFormElement>) => {
-    evt && evt.preventDefault();
-
-    const newQueryObj = {
-      ...this.state.queryObj,
-      __text: this.state.textFilter,
-    };
-
-    this.setState(
-      {
-        queryObj: newQueryObj,
-      },
-      this.onQueryChange
-    );
-  };
-
   onQueryChange = () => {
-    const query = objToQuery(this.state.queryObj);
-    this.props.onQueryChange && this.props.onQueryChange(query);
+    const newQuery = [
+      joinQuery(Object.values(this.state.filters), false, true),
+      this.state.textFilter,
+    ]
+      .filter(f => f) // filter out empty strings
+      .join(' ');
+
+    this.props.onQueryChange && this.props.onQueryChange(newQuery);
   };
 
   onClearSearch = () => {
@@ -121,12 +108,13 @@ class IssueListSidebar extends Component<Props, State> {
       {
         textFilter: '',
       },
-      this.onTextFilterSubmit
+      this.onQueryChange
     );
   };
 
   render() {
     const {loading, tagValueLoader, tags} = this.props;
+    // TODO: @taylangocmen: 1. We need to render negated tags better, 2. We need an option to add negated tags to query
     return (
       <StreamSidebar>
         {loading ? (
@@ -134,7 +122,7 @@ class IssueListSidebar extends Component<Props, State> {
         ) : (
           <Fragment>
             <SidebarSection title={t('Text')}>
-              <form onSubmit={this.onTextFilterSubmit}>
+              <form onSubmit={this.onQueryChange}>
                 <Input
                   placeholder={t('Search title and culprit text body')}
                   onChange={this.onTextChange}
@@ -149,7 +137,7 @@ class IssueListSidebar extends Component<Props, State> {
 
             {map(tags, tag => (
               <IssueListTagFilter
-                value={this.state.queryObj[tag.key]}
+                value={this.state.filters[tag.key]?.value.text || undefined}
                 key={tag.key}
                 tag={tag}
                 onSelect={this.onSelectTag}

tests/js/spec/utils/stream.spec.tsx

@@ -1,74 +0,0 @@
-import {objToQuery, queryToObj} from 'sentry/utils/stream';
-
-describe('utils/stream', function () {
-  describe('queryToObj()', function () {
-    it('should convert a basic query string to a query object', function () {
-      expect(queryToObj('is:unresolved')).toEqual({
-        __text: '',
-        is: 'unresolved',
-      });
-
-      expect(queryToObj('is:unresolved browser:"Chrome 36"')).toEqual({
-        __text: '',
-        is: 'unresolved',
-        browser: 'Chrome 36',
-      });
-
-      expect(queryToObj('python is:unresolved browser:"Chrome 36"')).toEqual({
-        __text: 'python',
-        is: 'unresolved',
-        browser: 'Chrome 36',
-      });
-    });
-
-    it('should convert separate query tokens into a single __text property', function () {
-      expect(queryToObj('python    exception')).toEqual({
-        __text: 'python exception',
-      });
-
-      // NOTE: "python exception" is extracted despite being broken up by "is:unresolved"
-      expect(queryToObj('python  is:unresolved exception')).toEqual({
-        __text: 'python exception',
-        is: 'unresolved',
-      });
-    });
-
-    it('should use empty string as __text and not fail if query is undefined', function () {
-      expect(queryToObj()).toEqual({
-        __text: '',
-      });
-    });
-  });
-
-  describe('objToQuery()', function () {
-    it('should convert a query object to a query string', function () {
-      expect(
-        objToQuery({
-          is: 'unresolved',
-        })
-      ).toEqual('is:unresolved');
-
-      expect(
-        objToQuery({
-          is: 'unresolved',
-          assigned: 'foo@bar.com',
-        })
-      ).toEqual('is:unresolved assigned:foo@bar.com');
-
-      expect(
-        objToQuery({
-          is: 'unresolved',
-          __text: 'python exception',
-        })
-      ).toEqual('is:unresolved python exception');
-    });
-
-    it('should quote query values that contain spaces', function () {
-      expect(
-        objToQuery({
-          browser: 'Chrome 36',
-        })
-      ).toEqual('browser:"Chrome 36"');
-    });
-  });
-});