fix(metrics): Wrap in quotes when adding filter from summary (#71210)

#### Problem

When adding filters from the summary table (e.g. from a query that is
grouped by transaction) the filters are not wrapped with quotes,
potentially leading to parsing errors on the BE side.

#### Solution

Ensure filters are wrapped with quotes after adding them from the
summary table.

static/app/views/metrics/metricSearchBar.spec.tsx

@@ -1,28 +0,0 @@
-import {ensureQuotedTextFilters} from 'sentry/views/metrics/metricSearchBar';
-
-describe('ensureQuotedTextFilters', () => {
-  it('returns a query with all text filters quoted', () => {
-    expect(ensureQuotedTextFilters('transaction:/{organization_slug}/')).toEqual(
-      'transaction:"/{organization_slug}/"'
-    );
-
-    // transaction.duration defaults to a number filter
-    expect(ensureQuotedTextFilters('transaction.duration:100')).toEqual(
-      'transaction.duration:100'
-    );
-  });
-
-  it('applies config overrides', () => {
-    expect(
-      ensureQuotedTextFilters('transaction:100', {
-        numericKeys: new Set(['transaction']),
-      })
-    ).toEqual('transaction:100');
-
-    expect(
-      ensureQuotedTextFilters('transaction.duration:100', {
-        numericKeys: new Set([]),
-      })
-    ).toEqual('transaction.duration:"100"');
-  });
-});

static/app/views/metrics/metricSearchBar.tsx

@@ -3,23 +3,17 @@ import styled from '@emotion/styled';
 import {useId} from '@react-aria/utils';
 import memoize from 'lodash/memoize';
 
-import type {SearchConfig} from 'sentry/components/searchSyntax/parser';
-import {
-  FilterType,
-  joinQuery,
-  parseSearch,
-  Token,
-} from 'sentry/components/searchSyntax/parser';
 import type {SmartSearchBarProps} from 'sentry/components/smartSearchBar';
 import SmartSearchBar from 'sentry/components/smartSearchBar';
 import {t} from 'sentry/locale';
-import type {MRI, TagCollection} from 'sentry/types';
-import {SavedSearchType} from 'sentry/types';
+import {SavedSearchType, type TagCollection} from 'sentry/types/group';
+import type {MRI} from 'sentry/types/metrics';
 import {getUseCaseFromMRI} from 'sentry/utils/metrics/mri';
 import {useMetricsTags} from 'sentry/utils/metrics/useMetricsTags';
 import useApi from 'sentry/utils/useApi';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
+import {ensureQuotedTextFilters} from 'sentry/views/metrics/utils';
 import {useSelectedProjects} from 'sentry/views/metrics/utils/useSelectedProjects';
 
 interface MetricSearchBarProps extends Partial<SmartSearchBarProps> {
@@ -34,42 +28,6 @@ interface MetricSearchBarProps extends Partial<SmartSearchBarProps> {
 const EMPTY_ARRAY = [];
 const EMPTY_SET = new Set<never>();
 
-export function ensureQuotedTextFilters(
-  query: string,
-  configOverrides?: Partial<SearchConfig>
-) {
-  const parsedSearch = parseSearch(query, configOverrides);
-
-  if (!parsedSearch) {
-    return query;
-  }
-
-  for (let i = 0; i < parsedSearch.length; i++) {
-    const token = parsedSearch[i];
-    if (token.type === Token.FILTER && token.filter === FilterType.TEXT) {
-      // joinQuery() does not access nested tokens, so we need to manipulate the text of the filter instead of it's value
-      if (!token.value.quoted) {
-        token.text = `${token.key.text}:"${token.value.text}"`;
-      }
-
-      const spaceToken = parsedSearch[i + 1];
-      const afterSpaceToken = parsedSearch[i + 2];
-      if (
-        spaceToken &&
-        afterSpaceToken &&
-        spaceToken.type === Token.SPACES &&
-        spaceToken.text === '' &&
-        afterSpaceToken.type === Token.FILTER
-      ) {
-        // Ensure there is a space between two filters
-        spaceToken.text = ' ';
-      }
-    }
-  }
-
-  return joinQuery(parsedSearch);
-}
-
 export function MetricSearchBar({
   mri,
   blockedTags,

static/app/views/metrics/utils/index.spec.tsx

@@ -1,7 +1,7 @@
 import type {MRI} from 'sentry/types/metrics';
 import {MetricSeriesFilterUpdateType} from 'sentry/utils/metrics/types';
 
-import {updateQueryWithSeriesFilter} from './index';
+import {ensureQuotedTextFilters, updateQueryWithSeriesFilter} from './index';
 
 describe('updateQueryWithSeriesFilter', () => {
   it('should add a filter an empty query stirng', () => {
@@ -18,7 +18,7 @@ describe('updateQueryWithSeriesFilter', () => {
       MetricSeriesFilterUpdateType.ADD
     );
 
-    expect(updatedQuery.query).toEqual('project:1');
+    expect(updatedQuery.query).toEqual('project:"1"');
     expect(updatedQuery.groupBy).toEqual([]);
   });
 
@@ -27,7 +27,7 @@ describe('updateQueryWithSeriesFilter', () => {
       mri: 'd:transactions/duration@milisecond' as MRI,
       op: 'count',
       groupBy: [],
-      query: 'release:latest AND (environment:production OR environment:staging)',
+      query: 'release:"latest" AND (environment:production OR environment:staging)',
     };
 
     const updatedQuery = updateQueryWithSeriesFilter(
@@ -37,7 +37,7 @@ describe('updateQueryWithSeriesFilter', () => {
     );
 
     expect(updatedQuery.query).toEqual(
-      '( release:latest AND ( environment:production OR environment:staging ) ) project:1'
+      '( release:latest AND ( environment:production OR environment:staging ) ) project:"1"'
     );
     expect(updatedQuery.groupBy).toEqual([]);
   });
@@ -56,7 +56,7 @@ describe('updateQueryWithSeriesFilter', () => {
       MetricSeriesFilterUpdateType.EXCLUDE
     );
 
-    expect(updatedQuery.query).toEqual('!project:1');
+    expect(updatedQuery.query).toEqual('!project:"1"');
     expect(updatedQuery.groupBy).toEqual([]);
   });
 
@@ -75,7 +75,7 @@ describe('updateQueryWithSeriesFilter', () => {
     );
 
     expect(updatedQuery.query).toEqual(
-      '( environment:prod1 OR environment:prod2 ) !project:2'
+      '( environment:prod1 OR environment:prod2 ) !project:"2"'
     );
     expect(updatedQuery.groupBy).toEqual([]);
   });
@@ -94,7 +94,45 @@ describe('updateQueryWithSeriesFilter', () => {
       MetricSeriesFilterUpdateType.ADD
     );
 
-    expect(updatedQuery.query).toEqual('project:1 release:latest');
+    expect(updatedQuery.query).toEqual('project:"1" release:"latest"');
     expect(updatedQuery.groupBy).toEqual(['environment']);
   });
 });
+
+describe('ensureQuotedTextFilters', () => {
+  it('returns a query with all text filters quoted', () => {
+    expect(ensureQuotedTextFilters('transaction:/{organization_slug}/')).toEqual(
+      'transaction:"/{organization_slug}/"'
+    );
+
+    // transaction.duration defaults to a number filter
+    expect(ensureQuotedTextFilters('transaction.duration:100')).toEqual(
+      'transaction.duration:100'
+    );
+  });
+
+  it('handles negated filters', () => {
+    expect(ensureQuotedTextFilters('!transaction:/{organization_slug}/')).toEqual(
+      '!transaction:"/{organization_slug}/"'
+    );
+
+    // transaction.duration defaults to a number filter
+    expect(ensureQuotedTextFilters('!transaction.duration:100')).toEqual(
+      '!transaction.duration:100'
+    );
+  });
+
+  it('applies config overrides', () => {
+    expect(
+      ensureQuotedTextFilters('transaction:100', {
+        numericKeys: new Set(['transaction']),
+      })
+    ).toEqual('transaction:100');
+
+    expect(
+      ensureQuotedTextFilters('transaction.duration:100', {
+        numericKeys: new Set([]),
+      })
+    ).toEqual('transaction.duration:"100"');
+  });
+});

static/app/views/metrics/utils/index.tsx

@@ -1,3 +1,10 @@
+import {
+  FilterType,
+  joinQuery,
+  parseSearch,
+  type SearchConfig,
+  Token,
+} from 'sentry/components/searchSyntax/parser';
 import {
   MetricSeriesFilterUpdateType,
   type MetricsQuery,
@@ -30,6 +37,45 @@ export function extendQueryWithGroupBys(
   return mutableSearch.formatString();
 }
 
+/**
+ * Wraps text filters of a search string in quotes if they are not already.
+ */
+export function ensureQuotedTextFilters(
+  query: string,
+  configOverrides?: Partial<SearchConfig>
+) {
+  const parsedSearch = parseSearch(query, configOverrides);
+
+  if (!parsedSearch) {
+    return query;
+  }
+
+  for (let i = 0; i < parsedSearch.length; i++) {
+    const token = parsedSearch[i];
+    if (token.type === Token.FILTER && token.filter === FilterType.TEXT) {
+      // joinQuery() does not access nested tokens, so we need to manipulate the text of the filter instead of it's value
+      if (!token.value.quoted) {
+        token.text = `${token.negated ? '!' : ''}${token.key.text}:"${token.value.text}"`;
+      }
+
+      const spaceToken = parsedSearch[i + 1];
+      const afterSpaceToken = parsedSearch[i + 2];
+      if (
+        spaceToken &&
+        afterSpaceToken &&
+        spaceToken.type === Token.SPACES &&
+        spaceToken.text === '' &&
+        afterSpaceToken.type === Token.FILTER
+      ) {
+        // Ensure there is a space between two filters
+        spaceToken.text = ' ';
+      }
+    }
+  }
+
+  return joinQuery(parsedSearch);
+}
+
 /**
  * Used when a user clicks on filter button in the series summary table. Applies
  * tag values to the filter string of the query. Removes the tags from query groupyBy
@@ -60,7 +106,7 @@ export function updateQueryWithSeriesFilter(
 
   return {
     ...query,
-    query: extendedQuery,
+    query: ensureQuotedTextFilters(extendedQuery),
     groupBy: newGroupBy,
   };
 }