feat(saved-searches): Formalize new saved searches button location (#45016)

- Removes `useExperiment()` checks
- Removes code related to the old button location
- Updates tests

static/app/views/issueList/filters.tsx

@@ -5,35 +5,13 @@ import EnvironmentPageFilter from 'sentry/components/environmentPageFilter';
 import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
 import ProjectPageFilter from 'sentry/components/projectPageFilter';
 import {space} from 'sentry/styles/space';
-import {useExperiment} from 'sentry/utils/useExperiment';
-import useOrganization from 'sentry/utils/useOrganization';
 import {IssueSearchWithSavedSearches} from 'sentry/views/issueList/issueSearchWithSavedSearches';
 
-import IssueListSearchBar from './searchBar';
-
 interface Props {
   onSearch: (query: string) => void;
   query: string;
 }
 
-function IssueSearch({query, onSearch}: Props) {
-  const organization = useOrganization();
-  const {experimentAssignment} = useExperiment('SavedIssueSearchesLocationExperiment');
-  if (experimentAssignment === 1) {
-    return <IssueSearchWithSavedSearches {...{query, onSearch}} />;
-  }
-
-  return (
-    <StyledIssueListSearchBar
-      searchSource="main_search"
-      organization={organization}
-      query={query || ''}
-      onSearch={onSearch}
-      excludedTags={['environment']}
-    />
-  );
-}
-
 function IssueListFilters({query, onSearch}: Props) {
   return (
     <SearchContainer>
@@ -42,7 +20,7 @@ function IssueListFilters({query, onSearch}: Props) {
         <EnvironmentPageFilter />
         <DatePageFilter alignDropdown="left" />
       </StyledPageFilterBar>
-      <IssueSearch {...{query, onSearch}} />
+      <IssueSearchWithSavedSearches {...{query, onSearch}} />
     </SearchContainer>
   );
 }
@@ -66,17 +44,4 @@ const StyledPageFilterBar = styled(PageFilterBar)`
   max-width: 30rem;
 `;
 
-const StyledIssueListSearchBar = styled(IssueListSearchBar)`
-  flex: 1;
-  width: 100%;
-
-  @media (min-width: ${p => p.theme.breakpoints.small}) {
-    min-width: 20rem;
-  }
-
-  @media (min-width: ${p => p.theme.breakpoints.large}) {
-    min-width: 30rem;
-  }
-`;
-
 export default IssueListFilters;

static/app/views/issueList/header.tsx

@@ -12,26 +12,16 @@ import QueryCount from 'sentry/components/queryCount';
 import {TabList, Tabs} from 'sentry/components/tabs';
 import {Tooltip} from 'sentry/components/tooltip';
 import {SLOW_TOOLTIP_DELAY} from 'sentry/constants';
-import {IconPause, IconPlay, IconStar} from 'sentry/icons';
+import {IconPause, IconPlay} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Organization, SavedSearch} from 'sentry/types';
+import {Organization} from 'sentry/types';
 import {trackAnalyticsEvent} from 'sentry/utils/analytics';
-import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
-import {useExperiment} from 'sentry/utils/useExperiment';
 import useProjects from 'sentry/utils/useProjects';
-import {useSyncedLocalStorageState} from 'sentry/utils/useSyncedLocalStorageState';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import IssueListSetAsDefault from 'sentry/views/issueList/issueListSetAsDefault';
 
-import {
-  getTabs,
-  IssueSortOptions,
-  Query,
-  QueryCounts,
-  SAVED_SEARCHES_SIDEBAR_OPEN_LOCALSTORAGE_KEY,
-  TAB_MAX_COUNT,
-} from './utils';
+import {getTabs, IssueSortOptions, Query, QueryCounts, TAB_MAX_COUNT} from './utils';
 
 type IssueListHeaderProps = {
   displayReprocessingTab: boolean;
@@ -41,7 +31,6 @@ type IssueListHeaderProps = {
   queryCounts: QueryCounts;
   realtimeActive: boolean;
   router: InjectedRouter;
-  savedSearch: SavedSearch | null;
   selectedProjectIds: number[];
   sort: string;
   queryCount?: number;
@@ -87,42 +76,23 @@ function IssueListHeader({
   organization,
   query,
   sort,
-  queryCount,
   queryCounts,
   realtimeActive,
   onRealtimeChange,
-  savedSearch,
   router,
   displayReprocessingTab,
   selectedProjectIds,
 }: IssueListHeaderProps) {
-  const {experimentAssignment} = useExperiment('SavedIssueSearchesLocationExperiment');
-  const [isSavedSearchesOpen, setIsSavedSearchesOpen] = useSyncedLocalStorageState(
-    SAVED_SEARCHES_SIDEBAR_OPEN_LOCALSTORAGE_KEY,
-    false
-  );
   const {projects} = useProjects();
   const tabs = getTabs(organization);
   const visibleTabs = displayReprocessingTab
     ? tabs
     : tabs.filter(([tab]) => tab !== Query.REPROCESSING);
-  const savedSearchTabActive =
-    // Disable tab for saved searches location experiment
-    experimentAssignment !== 1 && !visibleTabs.some(([tabQuery]) => tabQuery === query);
   // Remove cursor and page when switching tabs
   const {cursor: _, page: __, ...queryParms} = router?.location?.query ?? {};
   const sortParam =
     queryParms.sort === IssueSortOptions.INBOX ? undefined : queryParms.sort;
 
-  function onSavedSearchesToggleClicked() {
-    const newOpenState = !isSavedSearchesOpen;
-    trackAdvancedAnalyticsEvent('search.saved_search_sidebar_toggle_clicked', {
-      organization,
-      open: newOpenState,
-    });
-    setIsSavedSearchesOpen(newOpenState);
-  }
-
   function trackTabClick(tabQuery: string) {
     // Clicking on inbox tab and currently another tab is active
     if (tabQuery === Query.FOR_REVIEW && query !== Query.FOR_REVIEW) {
@@ -143,7 +113,7 @@ function IssueListHeader({
     : t('Enable real-time updates');
 
   return (
-    <Layout.Header>
+    <Layout.Header noActionWrap>
       <Layout.HeaderContent>
         <Layout.Title>
           {t('Issues')}
@@ -158,15 +128,6 @@ function IssueListHeader({
       <Layout.HeaderActions>
         <ButtonBar gap={1}>
           <IssueListSetAsDefault {...{sort, query, organization}} />
-          {experimentAssignment !== 1 && (
-            <Button
-              size="sm"
-              icon={<IconStar size="sm" isSolid={isSavedSearchesOpen} />}
-              onClick={onSavedSearchesToggleClicked}
-            >
-              {isSavedSearchesOpen ? t('Hide Searches') : t('Saved Searches')}
-            </Button>
-          )}
           <Button
             size="sm"
             data-test-id="real-time"
@@ -182,7 +143,7 @@ function IssueListHeader({
         onSelectionChange={key =>
           trackTabClick(key === EXTRA_TAB_KEY ? query : key.toString())
         }
-        selectedKey={savedSearchTabActive ? EXTRA_TAB_KEY : query}
+        selectedKey={query}
       >
         <TabList hideBorder>
           {[
@@ -212,18 +173,6 @@ function IssueListHeader({
                 );
               }
             ),
-            <TabList.Item
-              hidden={!savedSearchTabActive}
-              key={EXTRA_TAB_KEY}
-              to={{query: queryParms, pathname: location.pathname}}
-              textValue={savedSearch?.name ?? t('Custom Search')}
-            >
-              <IssueListHeaderTabContent
-                name={savedSearch?.name ?? t('Custom Search')}
-                count={queryCount}
-                query={query}
-              />
-            </TabList.Item>,
           ]}
         </TabList>
       </StyledTabs>

static/app/views/issueList/overview.spec.jsx

@@ -231,8 +231,7 @@ describe('IssueList', function () {
 
       expect(screen.getByRole('textbox')).toHaveValue('is:unresolved ');
 
-      // Tab shows "saved searches" because there is an is:unresolved tab
-      expect(screen.getByRole('button', {name: 'Saved Searches'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: /custom search/i})).toBeInTheDocument();
     });
 
     it('loads with query in URL and pinned queries', async function () {
@@ -270,7 +269,7 @@ describe('IssueList', function () {
       expect(screen.getByRole('textbox')).toHaveValue('level:foo ');
 
       // Tab shows "custom search"
-      expect(screen.getByRole('tab', {name: 'Custom Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Custom Search'})).toBeInTheDocument();
     });
 
     it('loads with a pinned custom query', async function () {
@@ -305,7 +304,7 @@ describe('IssueList', function () {
       expect(screen.getByRole('textbox')).toHaveValue('is:resolved ');
 
       // Organization saved search selector should have default saved search selected
-      expect(screen.getByRole('tab', {name: 'My Default Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'My Default Search'})).toBeInTheDocument();
     });
 
     it('loads with a saved query', async function () {
@@ -350,7 +349,7 @@ describe('IssueList', function () {
       expect(screen.getByRole('textbox')).toHaveValue('assigned:me ');
 
       // Organization saved search selector should have default saved search selected
-      expect(screen.getByRole('tab', {name: 'Assigned to Me'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Assigned to Me'})).toBeInTheDocument();
     });
 
     it('loads with a query in URL', async function () {
@@ -392,7 +391,7 @@ describe('IssueList', function () {
       expect(screen.getByRole('textbox')).toHaveValue('level:error ');
 
       // Organization saved search selector should have default saved search selected
-      expect(screen.getByRole('tab', {name: 'Custom Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Custom Search'})).toBeInTheDocument();
     });
 
     it('loads with an empty query in URL', async function () {
@@ -430,10 +429,10 @@ describe('IssueList', function () {
       expect(screen.getByRole('textbox')).toHaveValue('is:resolved ');
 
       // Organization saved search selector should have default saved search selected
-      expect(screen.getByRole('tab', {name: 'My Default Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'My Default Search'})).toBeInTheDocument();
     });
 
-    it('selects a saved search', async function () {
+    it('1 search', async function () {
       const localSavedSearch = {...savedSearch, projectId: null};
       savedSearchesRequest = MockApiClient.addMockResponse({
         url: '/organizations/org-slug/searches/',
@@ -446,7 +445,7 @@ describe('IssueList', function () {
 
       await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
 
-      userEvent.click(screen.getByRole('button', {name: /saved searches/i}));
+      userEvent.click(screen.getByRole('button', {name: /custom search/i}));
       userEvent.click(screen.getByRole('button', {name: localSavedSearch.name}));
 
       expect(browserHistory.push).toHaveBeenLastCalledWith(
@@ -538,7 +537,7 @@ describe('IssueList', function () {
         {context: routerContext, router: routerWithQuery}
       );
 
-      expect(screen.getByRole('tab', {name: 'Custom Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'Custom Search'})).toBeInTheDocument();
 
       MockApiClient.clearMockResponses();
       const createPin = MockApiClient.addMockResponse({
@@ -598,7 +597,7 @@ describe('IssueList', function () {
 
       await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
 
-      expect(screen.getByRole('tab', {name: 'My Default Search'})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: 'My Default Search'})).toBeInTheDocument();
 
       userEvent.click(screen.getByLabelText(/Remove Default/i));
 
@@ -652,7 +651,7 @@ describe('IssueList', function () {
 
       await waitForElementToBeRemoved(() => screen.getByTestId('loading-indicator'));
 
-      expect(screen.getByRole('tab', {name: savedSearch.name})).toBeInTheDocument();
+      expect(screen.getByRole('button', {name: savedSearch.name})).toBeInTheDocument();
 
       userEvent.click(screen.getByLabelText(/set as default/i));
 

static/app/views/issueList/overview.tsx

@@ -1136,7 +1136,7 @@ class IssueListOverview extends Component<Props, State> {
       issuesLoading,
       error,
     } = this.state;
-    const {organization, savedSearch, selection, location, router} = this.props;
+    const {organization, selection, location, router} = this.props;
     const links = parseLinkHeader(pageLinks);
     const query = this.getQuery();
     const queryPageInt = parseInt(location.query.page, 10);
@@ -1188,7 +1188,6 @@ class IssueListOverview extends Component<Props, State> {
           onRealtimeChange={this.onRealtimeChange}
           router={router}
           displayReprocessingTab={showReprocessingTab}
-          savedSearch={savedSearch}
           selectedProjectIds={selection.projects}
         />
         <StyledBody>
@@ -1275,17 +1274,9 @@ const StyledBody = styled('div')`
   gap: 0;
   padding: 0;
 
-  grid-template-columns: minmax(0, 1fr);
-  grid-template-rows: auto minmax(max-content, 1fr);
-  grid-template-areas:
-    'saved-searches'
-    'content';
-
-  @media (min-width: ${p => p.theme.breakpoints.small}) {
-    grid-template-rows: 1fr;
-    grid-template-columns: minmax(0, 1fr) auto;
-    grid-template-areas: 'content saved-searches';
-  }
+  grid-template-rows: 1fr;
+  grid-template-columns: minmax(0, 1fr) auto;
+  grid-template-areas: 'content saved-searches';
 `;
 
 const StyledMain = styled('section')`

static/app/views/issueList/savedIssueSearches.tsx

@@ -165,7 +165,7 @@ const SavedIssueSearches = ({
   sort,
 }: SavedIssueSearchesProps) => {
   const theme = useTheme();
-  const [isOpen, setIsOpen] = useSyncedLocalStorageState(
+  const [isOpen] = useSyncedLocalStorageState(
     SAVED_SEARCHES_SIDEBAR_OPEN_LOCALSTORAGE_KEY,
     false
   );
@@ -176,17 +176,9 @@ const SavedIssueSearches = ({
     isError,
     refetch,
   } = useFetchSavedSearchesForOrg({orgSlug: organization.slug});
-  const isAboveContent = useMedia(`(max-width: ${theme.breakpoints.small})`);
-  const onClickSavedSearch = (savedSearch: SavedSearch) => {
-    // On small screens, the sidebar appears above the issue list, so we
-    // will close it automatically for convenience.
-    if (isAboveContent) {
-      setIsOpen(false);
-    }
-    onSavedSearchSelect(savedSearch);
-  };
+  const isMobile = useMedia(`(max-width: ${theme.breakpoints.small})`);
 
-  if (!isOpen) {
+  if (!isOpen || isMobile) {
     return null;
   }
 
@@ -230,7 +222,7 @@ const SavedIssueSearches = ({
             <SavedSearchItem
               key={item.id}
               organization={organization}
-              onSavedSearchSelect={onClickSavedSearch}
+              onSavedSearchSelect={onSavedSearchSelect}
               savedSearch={item}
             />
           ))}
@@ -259,7 +251,7 @@ const SavedIssueSearches = ({
               <SavedSearchItem
                 key={item.id}
                 organization={organization}
-                onSavedSearchSelect={onClickSavedSearch}
+                onSavedSearchSelect={onSavedSearchSelect}
                 savedSearch={item}
               />
             ))}

tests/acceptance/test_issue_saved_searches.py

@@ -45,23 +45,23 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
 
     def test_click_saved_search(self):
         self.page.visit_issue_list(self.org.slug)
-        self.browser.click_when_visible('button[aria-label="Saved Searches"]')
+        self.browser.click_when_visible('button[aria-label="Custom Search"]')
 
         # Navigate to a recommended saved search
         self.browser.click('button[aria-label="Errors Only"]')
         self.page.wait_until_loaded()
 
-        self.browser.snapshot("issue list after navigating to saved search")
+        self.browser.snapshot("issue list after navigating to saved search", desktop_only=True)
 
     def test_create_saved_search(self):
         self.page.visit_issue_list(self.org.slug)
-        self.browser.click_when_visible('button[aria-label="Saved Searches"]')
-        self.browser.snapshot("issue list with no saved searches")
+        self.browser.click_when_visible('button[aria-label="Custom Search"]')
+        self.browser.snapshot("issue list with no saved searches", desktop_only=True)
 
         self.browser.click('[aria-label="Create a new saved search"]')
 
         self.browser.wait_until('[role="dialog"]')
-        self.browser.snapshot("create saved search modal open")
+        self.browser.snapshot("create saved search modal open", desktop_only=True)
 
         self.browser.find_element(by=By.NAME, value="name").send_keys("My Saved Search")
         query_input = self.browser.find_element(
@@ -97,7 +97,7 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
         )
 
         self.page.visit_issue_list(self.org.slug)
-        self.browser.click_when_visible('button[aria-label="Saved Searches"]')
+        self.browser.click_when_visible('button[aria-label="Custom Search"]')
 
         self.browser.move_to('button[aria-label="My Saved Search"]')
         self.browser.wait_until_clickable('button[aria-label="Saved search options"]')
@@ -105,7 +105,7 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
         self.browser.click('[data-test-id="edit"]')
 
         self.browser.wait_until('[role="dialog"]')
-        self.browser.snapshot("edit saved search modal open")
+        self.browser.snapshot("edit saved search modal open", desktop_only=True)
 
         self.browser.find_element(by=By.NAME, value="name").clear()
         self.browser.find_element(by=By.NAME, value="name").send_keys("New Saved Search Name")
@@ -137,7 +137,7 @@ class OrganizationGroupIndexTest(AcceptanceTestCase, SnubaTestCase):
         )
 
         self.page.visit_issue_list(self.org.slug)
-        self.browser.click_when_visible('button[aria-label="Saved Searches"]')
+        self.browser.click_when_visible('button[aria-label="Custom Search"]')
 
         self.browser.move_to('button[aria-label="My Saved Search"]')
         self.browser.wait_until_clickable('button[aria-label="Saved search options"]')