Browse Source

fix(discover): Store recent searches differently for errors/transactions tabs (#82404)

I am ripping out all the feature flags for the query builder, but while
doing so the tests failed:

- shows the search history for the transaction dataset
- shows the search history for the error dataset

It looks like the logic for doing this was only being applied to the old
search bar component (which has been gone for a while).

NOTE: This change will remove recent searches in Discover from before
this change, because those searches were being saved in the `EVENTS`
set. Now it only looks at `ERRORS` and `TRANSACTIONS`.
Malachi Willey 2 months ago
parent
commit
e8fe7c2f02

+ 2 - 2
static/app/views/discover/homepage.spec.tsx

@@ -136,7 +136,7 @@ describe('Discover > Homepage', () => {
     // Only the environment field
     expect(screen.getAllByTestId('grid-head-cell')).toHaveLength(1);
     screen.getByText('Previous Period');
-    screen.getByText('event.type:error');
+    screen.getByRole('row', {name: 'event.type:error'});
     expect(screen.queryByText('Dataset')).not.toBeInTheDocument();
   });
 
@@ -588,7 +588,7 @@ describe('Discover > Homepage', () => {
     await screen.findByText('environment');
 
     expect(screen.getAllByTestId('grid-head-cell')).toHaveLength(1);
-    screen.getByText('event.type:error');
+    screen.getByRole('row', {name: 'event.type:error'});
     expect(screen.getByRole('tab', {name: 'Errors'})).toHaveAttribute(
       'aria-selected',
       'true'

+ 7 - 4
static/app/views/discover/results.spec.tsx

@@ -1835,7 +1835,9 @@ describe('Results', function () {
       await userEvent.click(
         screen.getByPlaceholderText('Search for events, users, tags, and more')
       );
-      expect(screen.getByTestId('filter-token')).toHaveTextContent('event.type:error');
+      expect(
+        await screen.findByRole('option', {name: 'event.type:error'})
+      ).toBeInTheDocument();
     });
 
     it('shows the search history for the transaction dataset', async function () {
@@ -1953,9 +1955,10 @@ describe('Results', function () {
       await userEvent.click(
         screen.getByPlaceholderText('Search for events, users, tags, and more')
       );
-      expect(screen.getByTestId('filter-token')).toHaveTextContent(
-        'transaction.status:ok'
-      );
+
+      expect(
+        await screen.findByRole('option', {name: 'transaction.status:ok'})
+      ).toBeInTheDocument();
     });
   });
 });

+ 12 - 35
static/app/views/discover/results.tsx

@@ -14,7 +14,6 @@ import {Alert} from 'sentry/components/alert';
 import {Button} from 'sentry/components/button';
 import Confirm from 'sentry/components/confirm';
 import DeprecatedAsyncComponent from 'sentry/components/deprecatedAsyncComponent';
-import SearchBar from 'sentry/components/events/searchBar';
 import * as Layout from 'sentry/components/layouts/thirds';
 import ExternalLink from 'sentry/components/links/externalLink';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
@@ -29,7 +28,6 @@ import {
 import {ProjectPageFilter} from 'sentry/components/organizations/projectPageFilter';
 import type {CursorHandler} from 'sentry/components/pagination';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
-import {MAX_QUERY_LENGTH} from 'sentry/constants';
 import {IconClose} from 'sentry/icons/iconClose';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
@@ -723,22 +721,6 @@ export class Results extends Component<Props, State> {
       ? generateAggregateFields(organization, eventView.fields)
       : eventView.fields;
 
-    if (organization.features.includes('search-query-builder-discover')) {
-      return (
-        <Wrapper>
-          <ResultsSearchQueryBuilder
-            projectIds={eventView.project}
-            query={eventView.query}
-            fields={fields}
-            onSearch={this.handleSearch}
-            customMeasurements={customMeasurements}
-            dataset={eventView.dataset}
-            includeTransactions
-          />
-        </Wrapper>
-      );
-    }
-
     let savedSearchType: SavedSearchType | undefined = SavedSearchType.EVENT;
     if (hasDatasetSelector(organization)) {
       savedSearchType =
@@ -748,19 +730,18 @@ export class Results extends Component<Props, State> {
     }
 
     return (
-      <StyledSearchBar
-        searchSource="eventsv2"
-        organization={organization}
-        projectIds={eventView.project}
-        query={eventView.query}
-        fields={fields}
-        onSearch={this.handleSearch}
-        maxQueryLength={MAX_QUERY_LENGTH}
-        customMeasurements={customMeasurements}
-        dataset={eventView.dataset}
-        includeTransactions
-        savedSearchType={savedSearchType}
-      />
+      <Wrapper>
+        <ResultsSearchQueryBuilder
+          projectIds={eventView.project}
+          query={eventView.query}
+          fields={fields}
+          onSearch={this.handleSearch}
+          customMeasurements={customMeasurements}
+          dataset={eventView.dataset}
+          includeTransactions
+          recentSearches={savedSearchType}
+        />
+      </Wrapper>
     );
   }
 
@@ -915,10 +896,6 @@ const Wrapper = styled('div')`
   }
 `;
 
-const StyledSearchBar = styled(SearchBar)`
-  margin-bottom: ${space(2)};
-`;
-
 const Top = styled(Layout.Main)`
   flex-grow: 0;
 `;

+ 2 - 1
static/app/views/discover/resultsSearchQueryBuilder.tsx

@@ -124,6 +124,7 @@ type Props = {
   placeholder?: string;
   projectIds?: number[] | Readonly<number[]>;
   query?: string;
+  recentSearches?: SavedSearchType;
   searchSource?: string;
   supportedTags?: TagCollection | undefined;
 };
@@ -289,7 +290,7 @@ function ResultsSearchQueryBuilder(props: Props) {
       searchSource={props.searchSource || 'eventsv2'}
       filterKeySections={filterKeySections}
       getTagValues={getEventFieldValues}
-      recentSearches={SavedSearchType.EVENT}
+      recentSearches={props.recentSearches ?? SavedSearchType.EVENT}
       showUnsubmittedIndicator
     />
   );