ref(page-filters): Remove feature flag from navigation and sidebar (#34830)

* ref(page-filters): Remove feature flag from navigation, sidebar

* change tests

static/app/actionCreators/pageFilters.tsx

@@ -16,10 +16,7 @@ import {
   setPageFiltersStorage,
 } from 'sentry/components/organizations/pageFilters/persistence';
 import {PageFiltersStringified} from 'sentry/components/organizations/pageFilters/types';
-import {
-  doesPathHaveNewFilters,
-  getDefaultSelection,
-} from 'sentry/components/organizations/pageFilters/utils';
+import {getDefaultSelection} from 'sentry/components/organizations/pageFilters/utils';
 import {DATE_TIME_KEYS, URL_PARAM} from 'sentry/constants/pageFilters';
 import OrganizationStore from 'sentry/stores/organizationStore';
 import PageFiltersStore from 'sentry/stores/pageFiltersStore';
@@ -120,7 +117,6 @@ function mergeDatetime(
 type InitializeUrlStateParams = {
   memberProjects: Project[];
   organization: Organization;
-  pathname: Location['pathname'];
   queryParams: Location['query'];
   router: InjectedRouter;
   shouldEnforceSingleProject: boolean;
@@ -141,7 +137,6 @@ type InitializeUrlStateParams = {
 export function initializeUrlState({
   organization,
   queryParams,
-  pathname,
   router,
   memberProjects,
   skipLoadLastUsed,
@@ -194,21 +189,15 @@ export function initializeUrlState({
   if (storedPageFilters) {
     const {state: storedState, pinnedFilters} = storedPageFilters;
 
-    const pageHasPinning = doesPathHaveNewFilters(pathname, organization);
-
-    const filtersToRestore = pageHasPinning
-      ? pinnedFilters
-      : new Set<PinnedPageFilter>(['projects', 'environments']);
-
-    if (!hasProjectOrEnvironmentInUrl && filtersToRestore.has('projects')) {
+    if (!hasProjectOrEnvironmentInUrl && pinnedFilters.has('projects')) {
       pageFilters.projects = storedState.project ?? [];
     }
 
-    if (!hasProjectOrEnvironmentInUrl && filtersToRestore.has('environments')) {
+    if (!hasProjectOrEnvironmentInUrl && pinnedFilters.has('environments')) {
       pageFilters.environments = storedState.environment ?? [];
     }
 
-    if (!hasDatetimeInUrl && filtersToRestore.has('datetime')) {
+    if (!hasDatetimeInUrl && pinnedFilters.has('datetime')) {
       pageFilters.datetime = getDatetimeFromState(storedState);
       shouldUsePinnedDatetime = true;
     }
@@ -418,7 +407,7 @@ async function updateDesyncedUrlState(router?: Router, shouldForceProject?: bool
     return;
   }
 
-  const {pathname, query} = router.location;
+  const {query} = router.location;
 
   // XXX(epurkhiser): Since this is called immediately after updating the
   // store, wait for a tick since stores are not updated fully synchronously.
@@ -435,11 +424,9 @@ async function updateDesyncedUrlState(router?: Router, shouldForceProject?: bool
   }
 
   const storedPageFilters = getPageFilterStorage(organization.slug);
-  const pageHasPinning = doesPathHaveNewFilters(pathname, organization);
 
-  // If we don't have any stored page filters OR pinning is not enabled for
-  // this page, then we do not check desynced state
-  if (!storedPageFilters || !pageHasPinning) {
+  // If we don't have any stored page filters then we do not check desynced state
+  if (!storedPageFilters) {
     PageFiltersActions.updateDesyncedFilters(new Set<PinnedPageFilter>());
     return;
   }

static/app/components/organizations/pageFilters/container.tsx

@@ -21,7 +21,7 @@ import withOrganization from 'sentry/utils/withOrganization';
 
 import GlobalSelectionHeader from './globalSelectionHeader';
 import {getDatetimeFromState, getStateFromQuery} from './parse';
-import {doesPathHaveNewFilters, extractSelectionParameters} from './utils';
+import {extractSelectionParameters} from './utils';
 
 type GlobalSelectionHeaderProps = Omit<
   React.ComponentPropsWithoutRef<typeof GlobalSelectionHeader>,
@@ -82,7 +82,6 @@ function Container({skipLoadLastUsed, children, ...props}: Props) {
   const {isSuperuser} = ConfigStore.get('user');
   const isOrgAdmin = organization.access.includes('org:admin');
   const enforceSingleProject = !organization.features.includes('global-views');
-  const hasPageFilters = doesPathHaveNewFilters(location.pathname ?? '', organization);
 
   const specifiedProjects = specificProjectSlugs
     ? projects.filter(project => specificProjectSlugs.includes(project.slug))
@@ -106,7 +105,6 @@ function Container({skipLoadLastUsed, children, ...props}: Props) {
     initializeUrlState({
       organization,
       queryParams: location.query,
-      pathname: location.pathname,
       router,
       skipLoadLastUsed,
       memberProjects,
@@ -152,11 +150,7 @@ function Container({skipLoadLastUsed, children, ...props}: Props) {
     // XXX: This re-initalization is only required in new-page-filters
     // land, since we have implicit pinning in the old land which will
     // cause page filters to commonly reset.
-    if (
-      hasPageFilters &&
-      isEmpty(newSelectionQuery) &&
-      !isEqual(oldSelectionQuery, newSelectionQuery)
-    ) {
+    if (isEmpty(newSelectionQuery) && !isEqual(oldSelectionQuery, newSelectionQuery)) {
       doInitialization();
       lastQuery.current = location.query;
       return;

static/app/components/organizations/pageFilters/utils.tsx

@@ -6,7 +6,7 @@ import pickBy from 'lodash/pickBy';
 
 import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
 import {DATE_TIME_KEYS, URL_PARAM} from 'sentry/constants/pageFilters';
-import {Organization, PageFilters} from 'sentry/types';
+import {PageFilters} from 'sentry/types';
 
 /**
  * Make a default page filters object
@@ -69,39 +69,3 @@ export function isSelectionEqual(selection: PageFilters, other: PageFilters): bo
 
   return true;
 }
-
-/**
- * TODO(davidenwang): Temporarily used for when pages with the GSH live alongside new page filters
- * @param pathname
- * @param organization
- * @returns true if the pathname has new page filters
- */
-export function doesPathHaveNewFilters(pathname: string, organization: Organization) {
-  const newFilterPaths = (
-    organization.features.includes('selection-filters-v2')
-      ? [
-          'user-feedback',
-          'alerts',
-          'monitors',
-          'issues',
-          'projects',
-          'dashboard',
-          'releases',
-          'discover',
-          'performance',
-        ]
-      : [
-          'user-feedback',
-          'alerts',
-          'monitors',
-          'issues',
-          'projects',
-          'dashboards',
-          'releases',
-          'discover',
-          'performance',
-        ]
-  ).map(route => `/organizations/${organization.slug}/${route}/`);
-
-  return newFilterPaths.some(pageFilterPath => pathname.includes(pageFilterPath));
-}

static/app/components/sidebar/index.tsx

@@ -1,18 +1,12 @@
 import {Fragment, useEffect} from 'react';
-import {browserHistory} from 'react-router';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 import {Location} from 'history';
-import * as qs from 'query-string';
 
 import {hideSidebar, showSidebar} from 'sentry/actionCreators/preferences';
 import Feature from 'sentry/components/acl/feature';
 import GuideAnchor from 'sentry/components/assistant/guideAnchor';
 import HookOrDefault from 'sentry/components/hookOrDefault';
-import {
-  doesPathHaveNewFilters,
-  extractSelectionParameters,
-} from 'sentry/components/organizations/pageFilters/utils';
 import {
   IconChevron,
   IconDashboard,
@@ -110,50 +104,6 @@ function Sidebar({location, organization}: Props) {
     }
   }, [location?.hash]);
 
-  /**
-   * Navigate to a path, but keep the page filter query strings.
-   */
-  const navigateWithPageFilters = (
-    pathname: string,
-    evt: React.MouseEvent<HTMLAnchorElement>
-  ) => {
-    // XXX(epurkhiser): No need to navigate w/ the page filters in the world
-    // of new page filter selection. You must pin your filters in which case
-    // they will persist anyway.
-    if (organization) {
-      if (doesPathHaveNewFilters(pathname, organization)) {
-        return;
-      }
-    }
-
-    const globalSelectionRoutes = [
-      'alerts',
-      'alerts/rules',
-      'dashboards',
-      'issues',
-      'releases',
-      'user-feedback',
-      'discover',
-      'discover/results', // Team plans do not have query landing page
-      'performance',
-    ].map(route => `/organizations/${organization?.slug}/${route}/`);
-
-    // Only keep the querystring if the current route matches one of the above
-    if (globalSelectionRoutes.includes(pathname)) {
-      const query = extractSelectionParameters(location?.query ?? {});
-
-      // Handle cmd-click (mac) and meta-click (linux)
-      if (evt.metaKey) {
-        const q = qs.stringify(query);
-        evt.currentTarget.href = `${evt.currentTarget.href}?${q}`;
-        return;
-      }
-
-      evt.preventDefault();
-      browserHistory.push({pathname, query});
-    }
-  };
-
   const hasPanel = !!activePanel;
   const hasOrganization = !!organization;
   const orientation: SidebarOrientation = horizontal ? 'top' : 'left';
@@ -179,9 +129,6 @@ function Sidebar({location, organization}: Props) {
   const issues = hasOrganization && (
     <SidebarItem
       {...sidebarItemProps}
-      onClick={(_id, evt) =>
-        navigateWithPageFilters(`/organizations/${organization.slug}/issues/`, evt)
-      }
       icon={<IconIssues size="md" />}
       label={<GuideAnchor target="issues">{t('Issues')}</GuideAnchor>}
       to={`/organizations/${organization.slug}/issues/`}
@@ -197,9 +144,6 @@ function Sidebar({location, organization}: Props) {
     >
       <SidebarItem
         {...sidebarItemProps}
-        onClick={(_id, evt) =>
-          navigateWithPageFilters(getDiscoverLandingUrl(organization), evt)
-        }
         icon={<IconTelescope size="md" />}
         label={<GuideAnchor target="discover">{t('Discover')}</GuideAnchor>}
         to={getDiscoverLandingUrl(organization)}
@@ -218,12 +162,6 @@ function Sidebar({location, organization}: Props) {
         {(overideProps: Partial<React.ComponentProps<typeof SidebarItem>>) => (
           <SidebarItem
             {...sidebarItemProps}
-            onClick={(_id, evt) =>
-              navigateWithPageFilters(
-                `/organizations/${organization.slug}/performance/`,
-                evt
-              )
-            }
             icon={<IconLightning size="md" />}
             label={<GuideAnchor target="performance">{t('Performance')}</GuideAnchor>}
             to={`/organizations/${organization.slug}/performance/`}
@@ -238,9 +176,6 @@ function Sidebar({location, organization}: Props) {
   const releases = hasOrganization && (
     <SidebarItem
       {...sidebarItemProps}
-      onClick={(_id, evt) =>
-        navigateWithPageFilters(`/organizations/${organization.slug}/releases/`, evt)
-      }
       icon={<IconReleases size="md" />}
       label={<GuideAnchor target="releases">{t('Releases')}</GuideAnchor>}
       to={`/organizations/${organization.slug}/releases/`}
@@ -251,9 +186,6 @@ function Sidebar({location, organization}: Props) {
   const userFeedback = hasOrganization && (
     <SidebarItem
       {...sidebarItemProps}
-      onClick={(_id, evt) =>
-        navigateWithPageFilters(`/organizations/${organization.slug}/user-feedback/`, evt)
-      }
       icon={<IconSupport size="md" />}
       label={t('User Feedback')}
       to={`/organizations/${organization.slug}/user-feedback/`}
@@ -264,9 +196,6 @@ function Sidebar({location, organization}: Props) {
   const alerts = hasOrganization && (
     <SidebarItem
       {...sidebarItemProps}
-      onClick={(_id, evt) =>
-        navigateWithPageFilters(`/organizations/${organization.slug}/alerts/rules/`, evt)
-      }
       icon={<IconSiren size="md" />}
       label={t('Alerts')}
       to={`/organizations/${organization.slug}/alerts/rules/`}
@@ -278,9 +207,6 @@ function Sidebar({location, organization}: Props) {
     <Feature features={['monitors']} organization={organization}>
       <SidebarItem
         {...sidebarItemProps}
-        onClick={(_id, evt) =>
-          navigateWithPageFilters(`/organizations/${organization.slug}/monitors/`, evt)
-        }
         icon={<IconLab size="md" />}
         label={t('Monitors')}
         to={`/organizations/${organization.slug}/monitors/`}
@@ -293,9 +219,6 @@ function Sidebar({location, organization}: Props) {
     <Feature features={['session-replay']} organization={organization}>
       <SidebarItem
         {...sidebarItemProps}
-        onClick={(_id, evt) =>
-          navigateWithPageFilters(`/organizations/${organization.slug}/replays/`, evt)
-        }
         icon={<IconPlay size="md" />}
         label={t('Replays')}
         to={`/organizations/${organization.slug}/replays/`}
@@ -314,9 +237,6 @@ function Sidebar({location, organization}: Props) {
       <SidebarItem
         {...sidebarItemProps}
         index
-        onClick={(_id, evt) =>
-          navigateWithPageFilters(`/organizations/${organization.slug}/dashboards/`, evt)
-        }
         icon={<IconDashboard size="md" />}
         label={t('Dashboards')}
         to={`/organizations/${organization.slug}/dashboards/`}
@@ -335,9 +255,6 @@ function Sidebar({location, organization}: Props) {
       <SidebarItem
         {...sidebarItemProps}
         index
-        onClick={(_id, evt) =>
-          navigateWithPageFilters(`/organizations/${organization.slug}/profiling/`, evt)
-        }
         icon={<IconSpan size="md" />}
         label={t('Profiling')}
         to={`/organizations/${organization.slug}/profiling/`}

tests/js/spec/actionCreators/pageFilters.spec.jsx

@@ -13,9 +13,7 @@ jest.mock('sentry/utils/localStorage');
 
 describe('PageFilters ActionCreators', function () {
   const organization = TestStubs.Organization();
-  const pageFiltersOrganization = TestStubs.Organization({
-    features: ['selection-filters-v2'],
-  });
+
   beforeEach(function () {
     localStorage.getItem.mockClear();
     jest.spyOn(PageFiltersActions, 'updateProjects');
@@ -25,12 +23,28 @@ describe('PageFilters ActionCreators', function () {
 
   describe('initializeUrlState', function () {
     let router;
+    const key = `global-selection:${organization.slug}`;
+
     beforeEach(() => {
       router = TestStubs.router();
+      localStorage.setItem(
+        key,
+        JSON.stringify({
+          environments: [],
+          projects: [1],
+        })
+      );
     });
-    it('loads from local storage when no query params', function () {
-      const key = `global-selection:${organization.slug}`;
-      localStorage.setItem(key, JSON.stringify({environments: [], projects: [1]}));
+
+    it('loads from local storage when no query params and filters are pinned', function () {
+      localStorage.setItem(
+        key,
+        JSON.stringify({
+          environments: [],
+          projects: [1],
+          pinnedFilters: ['projects', 'environments'],
+        })
+      );
       initializeUrlState({
         organization,
         queryParams: {},
@@ -46,7 +60,7 @@ describe('PageFilters ActionCreators', function () {
           environments: [],
           projects: [1],
         }),
-        new Set()
+        new Set(['projects', 'environments'])
       );
       expect(router.replace).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -57,6 +71,7 @@ describe('PageFilters ActionCreators', function () {
         })
       );
     });
+
     it('does not load from local storage when no query params and `skipLoadLastUsed` is true', function () {
       jest.spyOn(localStorage, 'getItem');
       initializeUrlState({
@@ -226,7 +241,7 @@ describe('PageFilters ActionCreators', function () {
 
       // Initialize state with a page that shouldn't restore from local storage
       initializeUrlState({
-        organization: pageFiltersOrganization,
+        organization,
         queryParams: {},
         pathname: '/organizations/org-slug/issues/',
         router,
@@ -256,7 +271,7 @@ describe('PageFilters ActionCreators', function () {
 
       // Initialize state with a page that uses pinned filters
       initializeUrlState({
-        organization: pageFiltersOrganization,
+        organization,
         queryParams: {},
         pathname: '/organizations/org-slug/issues/',
         router,

tests/js/spec/components/organizations/globalSelectionHeader.spec.jsx

@@ -421,9 +421,13 @@ describe('GlobalSelectionHeader', function () {
     });
   });
 
-  it('loads from local storage when no URL parameters', async function () {
+  it('loads from local storage when no URL parameters and filters are pinned', async function () {
     getItem.mockImplementation(() =>
-      JSON.stringify({projects: [3], environments: ['staging']})
+      JSON.stringify({
+        projects: [3],
+        environments: ['staging'],
+        pinnedFilters: ['projects', 'environments'],
+      })
     );
     const initializationObj = initializeOrg({
       organization: {