Browse Source

feat(page-filters): Phase 1 rollout (#32787)

Rolling out page filters to GA for user feedback, alerts, and monitors
David Wang 3 years ago
parent
commit
f05d2d0044

+ 2 - 2
static/app/components/organizations/pageFilters/utils.tsx

@@ -79,8 +79,8 @@ export function isSelectionEqual(selection: PageFilters, other: PageFilters): bo
 export function doesPathHaveNewFilters(pathname: string, organization: Organization) {
   const newFilterPaths = (
     organization.features.includes('selection-filters-v2')
-      ? ['issues', 'user-feedback', 'alerts', 'monitors']
-      : []
+      ? ['user-feedback', 'alerts', 'monitors', 'issues']
+      : ['user-feedback', 'alerts', 'monitors']
   ).map(route => `/organizations/${organization.slug}/${route}/`);
 
   return newFilterPaths.some(pageFilterPath => pathname.includes(pageFilterPath));

+ 2 - 7
static/app/views/alerts/filterBar.tsx

@@ -7,7 +7,6 @@ import ProjectPageFilter from 'sentry/components/projectPageFilter';
 import SearchBar from 'sentry/components/searchBar';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {Organization} from 'sentry/types';
 
 import TeamFilter from './rules/teamFilter';
 import {getQueryStatus, getTeamParams} from './utils';
@@ -16,7 +15,6 @@ type Props = {
   location: Location<any>;
   onChangeFilter: (sectionId: string, activeFilters: Set<string>) => void;
   onChangeSearch: (query: string) => void;
-  organization: Organization;
   hasEnvironmentFilter?: boolean;
   hasStatusFilters?: boolean;
 };
@@ -25,14 +23,11 @@ function FilterBar({
   location,
   onChangeSearch,
   onChangeFilter,
-  organization,
   hasEnvironmentFilter,
   hasStatusFilters,
 }: Props) {
   const selectedTeams = new Set(getTeamParams(location.query.team));
 
-  const hasPageFilters = organization.features.includes('selection-filters-v2');
-
   const selectedStatus = hasStatusFilters
     ? new Set(getQueryStatus(location.query.status))
     : undefined;
@@ -46,8 +41,8 @@ function FilterBar({
           selectedStatus={selectedStatus}
           handleChangeFilter={onChangeFilter}
         />
-        {hasPageFilters && <ProjectPageFilter />}
-        {hasPageFilters && hasEnvironmentFilter && <EnvironmentPageFilter />}
+        <ProjectPageFilter />
+        {hasEnvironmentFilter && <EnvironmentPageFilter />}
       </FilterButtons>
       <SearchBar
         placeholder={t('Search by name')}

+ 1 - 2
static/app/views/alerts/list/index.tsx

@@ -262,7 +262,7 @@ class IncidentsList extends AsyncComponent<Props, State & AsyncComponent['state'
         <PageFiltersContainer
           organization={organization}
           showDateSelector={false}
-          hideGlobalHeader={organization.features.includes('selection-filters-v2')}
+          hideGlobalHeader
         >
           <AlertHeader organization={organization} router={router} activeTab="stream" />
           <StyledLayoutBody>
@@ -273,7 +273,6 @@ class IncidentsList extends AsyncComponent<Props, State & AsyncComponent['state'
                     {t('This page only shows metric alerts.')}
                   </StyledAlert>
                   <FilterBar
-                    organization={organization}
                     location={location}
                     onChangeFilter={this.handleChangeFilter}
                     onChangeSearch={this.handleChangeSearch}

+ 1 - 2
static/app/views/alerts/rules/index.tsx

@@ -141,7 +141,6 @@ class AlertRulesList extends AsyncComponent<Props, State & AsyncComponent['state
       <StyledLayoutBody>
         <Layout.Main fullWidth>
           <FilterBar
-            organization={organization}
             location={location}
             onChangeFilter={this.handleChangeFilter}
             onChangeSearch={this.handleChangeSearch}
@@ -277,7 +276,7 @@ class AlertRulesList extends AsyncComponent<Props, State & AsyncComponent['state
           organization={organization}
           showDateSelector={false}
           showEnvironmentSelector={false}
-          hideGlobalHeader={organization.features.includes('selection-filters-v2')}
+          hideGlobalHeader
         >
           <AlertHeader organization={organization} router={router} activeTab="rules" />
           {this.renderList()}

+ 1 - 4
static/app/views/monitors/index.tsx

@@ -4,7 +4,6 @@ import styled from '@emotion/styled';
 import Feature from 'sentry/components/acl/feature';
 import PageFiltersContainer from 'sentry/components/organizations/pageFilters/container';
 import {PageContent} from 'sentry/styles/organization';
-import useOrganization from 'sentry/utils/useOrganization';
 import withPageFilters from 'sentry/utils/withPageFilters';
 
 const Body = styled('div')`
@@ -14,15 +13,13 @@ const Body = styled('div')`
 `;
 
 const MonitorsContainer: React.FC = ({children}) => {
-  const organization = useOrganization();
-
   return (
     <Feature features={['monitors']} renderDisabled>
       <PageFiltersContainer
         showEnvironmentSelector={false}
         showDateSelector={false}
         resetParamsOnChange={['cursor']}
-        hideGlobalHeader={organization.features.includes('selection-filters-v2')}
+        hideGlobalHeader
       >
         <PageContent>
           <Body>{children}</Body>

+ 4 - 5
static/app/views/monitors/monitors.tsx

@@ -65,7 +65,6 @@ class Monitors extends AsyncView<Props, State> {
   renderBody() {
     const {monitorList, monitorListPageLinks} = this.state;
     const {organization} = this.props;
-    const hasPageFilters = organization.features.includes('selection-filters-v2');
 
     return (
       <Fragment>
@@ -82,13 +81,13 @@ class Monitors extends AsyncView<Props, State> {
             </Button>
           </HeaderTitle>
         </PageHeader>
-        <Filters hasPageFilters={hasPageFilters}>
+        <Filters>
           <SearchBar
             query={decodeScalar(qs.parse(location.search)?.query, '')}
             placeholder={t('Search for monitors.')}
             onSearch={this.handleSearch}
           />
-          {hasPageFilters && <ProjectPageFilter />}
+          <ProjectPageFilter />
         </Filters>
         <Panel>
           <PanelBody>
@@ -140,9 +139,9 @@ const StyledTimeSince = styled(TimeSince)`
   font-variant-numeric: tabular-nums;
 `;
 
-const Filters = styled('div')<{hasPageFilters?: boolean}>`
+const Filters = styled('div')`
   display: grid;
-  grid-template-columns: ${p => (p.hasPageFilters ? '1fr minmax(auto, 300px)' : '1fr')};
+  grid-template-columns: 1fr minmax(auto, 300px);
   gap: ${space(1.5)};
   margin-bottom: ${space(2)};
 `;

+ 16 - 34
static/app/views/userFeedback/index.tsx

@@ -3,7 +3,6 @@ import styled from '@emotion/styled';
 import {withProfiler} from '@sentry/react';
 import omit from 'lodash/omit';
 
-import Feature from 'sentry/components/acl/feature';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import DatePageFilter from 'sentry/components/datePageFilter';
@@ -121,46 +120,29 @@ class OrganizationUserFeedback extends AsyncView<Props, State> {
     const unresolvedQuery = omit(query, 'status');
     const allIssuesQuery = {...query, status: ''};
 
-    const hasNewPageFilters = organization.features.includes('selection-filters-v2');
-
     return (
-      <PageFiltersContainer hideGlobalHeader={hasNewPageFilters}>
+      <PageFiltersContainer hideGlobalHeader>
         <PageContent>
           <NoProjectMessage organization={organization}>
             <div data-test-id="user-feedback">
               <Header>
                 <PageHeading>{t('User Feedback')}</PageHeading>
-                {!hasNewPageFilters && (
-                  <ButtonBar active={!Array.isArray(status) ? status || '' : ''} merged>
-                    <Button barId="unresolved" to={{pathname, query: unresolvedQuery}}>
-                      {t('Unresolved')}
-                    </Button>
-                    <Button barId="" to={{pathname, query: allIssuesQuery}}>
-                      {t('All Issues')}
-                    </Button>
-                  </ButtonBar>
-                )}
               </Header>
-              <Feature
-                organization={organization}
-                features={['organizations:selection-filters-v2']}
-              >
-                <Filters>
-                  <ButtonBar active={!Array.isArray(status) ? status || '' : ''} merged>
-                    <Button barId="unresolved" to={{pathname, query: unresolvedQuery}}>
-                      {t('Unresolved')}
-                    </Button>
-                    <Button barId="" to={{pathname, query: allIssuesQuery}}>
-                      {t('All Issues')}
-                    </Button>
-                  </ButtonBar>
-                  <PageFilterBar>
-                    <ProjectPageFilter />
-                    <EnvironmentPageFilter />
-                    <DatePageFilter alignDropdown="right" />
-                  </PageFilterBar>
-                </Filters>
-              </Feature>
+              <Filters>
+                <ButtonBar active={!Array.isArray(status) ? status || '' : ''} merged>
+                  <Button barId="unresolved" to={{pathname, query: unresolvedQuery}}>
+                    {t('Unresolved')}
+                  </Button>
+                  <Button barId="" to={{pathname, query: allIssuesQuery}}>
+                    {t('All Issues')}
+                  </Button>
+                </ButtonBar>
+                <PageFilterBar>
+                  <ProjectPageFilter />
+                  <EnvironmentPageFilter />
+                  <DatePageFilter alignDropdown="right" />
+                </PageFilterBar>
+              </Filters>
               {this.renderStreamBody()}
               <Pagination pageLinks={reportListPageLinks} />
             </div>

+ 21 - 20
tests/js/spec/components/organizations/globalSelectionHeader.spec.jsx

@@ -20,6 +20,7 @@ const changeQuery = (routerContext, query) => ({
     router: {
       ...routerContext.context.router,
       location: {
+        ...routerContext.context.router.location,
         query,
       },
     },
@@ -49,7 +50,7 @@ describe('GlobalSelectionHeader', function () {
       },
     ],
     router: {
-      location: {query: {}},
+      location: {pathname: '/test', query: {}},
       params: {orgId: 'org-slug'},
     },
   });
@@ -274,7 +275,7 @@ describe('GlobalSelectionHeader', function () {
         {id: 2, slug: 'prod-project', environments: ['prod']},
       ],
       router: {
-        location: {query: {project: ['1']}},
+        location: {pathname: '/test', query: {project: ['1']}},
         params: {orgId: 'org-slug'},
       },
     });
@@ -478,7 +479,7 @@ describe('GlobalSelectionHeader', function () {
         // we need this to be set to make sure org in context is same as
         // current org in URL
         params: {orgId: 'org-slug'},
-        location: {query: {project: ['1', '2']}},
+        location: {pathname: '/test', query: {project: ['1', '2']}},
       },
     });
 
@@ -504,7 +505,7 @@ describe('GlobalSelectionHeader', function () {
         // we need this to be set to make sure org in context is same as
         // current org in URL
         params: {orgId: 'org-slug'},
-        location: {query: {project: ['1', '2']}},
+        location: {pathname: '/test', query: {project: ['1', '2']}},
       },
     });
 
@@ -530,7 +531,7 @@ describe('GlobalSelectionHeader', function () {
         // we need this to be set to make sure org in context is same as
         // current org in URL
         params: {orgId: 'org-slug'},
-        location: {query: {}},
+        location: {pathname: '/test', query: {}},
       },
     });
 
@@ -630,7 +631,7 @@ describe('GlobalSelectionHeader', function () {
           // we need this to be set to make sure org in context is same as
           // current org in URL
           params: {orgId: 'org-slug'},
-          location: {query: {project: ['1']}},
+          location: {pathname: '/test', query: {project: ['1']}},
         },
       });
 
@@ -669,7 +670,7 @@ describe('GlobalSelectionHeader', function () {
         location: {query: {}},
         router: {
           ...initialData.router,
-          location: {query: {}},
+          location: {pathname: '/test', query: {}},
         },
       });
       wrapper.setProps({organization: updatedOrganization});
@@ -689,7 +690,7 @@ describe('GlobalSelectionHeader', function () {
           // we need this to be set to make sure org in context is same as
           // current org in URL
           params: {orgId: 'org-slug'},
-          location: {query: {project: ['1', '2']}},
+          location: {pathname: '/test', query: {project: ['1', '2']}},
         },
       });
 
@@ -715,7 +716,7 @@ describe('GlobalSelectionHeader', function () {
         organization: org,
         router: {
           params: {orgId: 'org-slug'},
-          location: {query: {}},
+          location: {pathname: '/test', query: {}},
         },
       });
 
@@ -740,7 +741,7 @@ describe('GlobalSelectionHeader', function () {
         {id: 2, slug: 'prod-project', environments: ['prod']},
       ],
       router: {
-        location: {query: {}},
+        location: {pathname: '/test', query: {}},
       },
     });
 
@@ -794,7 +795,7 @@ describe('GlobalSelectionHeader', function () {
       organization,
       projects: [{id: 1, slug: 'staging-project', environments: ['staging']}],
       router: {
-        location: {query: {}},
+        location: {pathname: '/test', query: {}},
       },
     });
 
@@ -870,7 +871,7 @@ describe('GlobalSelectionHeader', function () {
           {id: 2, slug: 'prod-project', environments: ['prod']},
         ],
         router: {
-          location: {query: {}},
+          location: {pathname: '/test', query: {}},
           params: {orgId: 'org-slug'},
         },
       });
@@ -900,7 +901,7 @@ describe('GlobalSelectionHeader', function () {
         // Projects are returned in sorted slug order, so `prod-project` would
         // be the first project
         expect(initialData.router.replace).toHaveBeenLastCalledWith({
-          pathname: undefined,
+          pathname: '/test',
           query: {cursor: undefined, environment: [], project: ['2']},
         });
       });
@@ -921,7 +922,7 @@ describe('GlobalSelectionHeader', function () {
         wrapper.update();
 
         expect(initialData.router.replace).toHaveBeenLastCalledWith({
-          pathname: undefined,
+          pathname: '/test',
           query: {environment: [], project: ['1']},
         });
 
@@ -962,7 +963,7 @@ describe('GlobalSelectionHeader', function () {
         wrapper.update();
 
         expect(initialData.router.replace).toHaveBeenLastCalledWith({
-          pathname: undefined,
+          pathname: '/test',
           query: {environment: [], project: ['1']},
         });
       });
@@ -976,7 +977,7 @@ describe('GlobalSelectionHeader', function () {
           {id: 2, slug: 'prod-project', environments: ['prod']},
         ],
         router: {
-          location: {query: {statsPeriod: '90d'}},
+          location: {pathname: '/test', query: {statsPeriod: '90d'}},
           params: {orgId: 'org-slug'},
         },
       });
@@ -1009,7 +1010,7 @@ describe('GlobalSelectionHeader', function () {
         wrapper.update();
 
         expect(initialData.router.replace).toHaveBeenLastCalledWith({
-          pathname: undefined,
+          pathname: '/test',
           query: {environment: [], project: ['1'], statsPeriod: '90d'},
         });
       });
@@ -1026,7 +1027,7 @@ describe('GlobalSelectionHeader', function () {
           {id: 2, slug: 'prod-project', environments: ['prod']},
         ],
         router: {
-          location: {query: {}},
+          location: {pathname: '/test', query: {}},
           params: {orgId: 'org-slug'},
         },
       });
@@ -1091,7 +1092,7 @@ describe('GlobalSelectionHeader', function () {
         act(() => ProjectsStore.loadInitialData(initialData.projects));
 
         expect(initialData.router.replace).toHaveBeenLastCalledWith({
-          pathname: undefined,
+          pathname: '/test',
           query: {environment: [], project: ['1']},
         });
 
@@ -1127,7 +1128,7 @@ describe('GlobalSelectionHeader', function () {
       initialData = initializeOrg({
         projects: [memberProject, nonMemberProject],
         router: {
-          location: {query: {}},
+          location: {pathname: '/test', query: {}},
           params: {
             orgId: 'org-slug',
           },

+ 10 - 9
tests/js/spec/views/alerts/list/index.spec.jsx

@@ -4,6 +4,7 @@ import {act, render, screen, userEvent, within} from 'sentry-test/reactTestingLi
 import ProjectsStore from 'sentry/stores/projectsStore';
 import TeamStore from 'sentry/stores/teamStore';
 import IncidentsList from 'sentry/views/alerts/list';
+import {OrganizationContext} from 'sentry/views/organizationContext';
 
 describe('IncidentsList', function () {
   let routerContext;
@@ -16,12 +17,14 @@ describe('IncidentsList', function () {
 
   const createWrapper = (props = {}) => {
     return render(
-      <IncidentsList
-        params={{orgId: organization.slug}}
-        location={{query: {}, search: ''}}
-        router={router}
-        {...props}
-      />,
+      <OrganizationContext.Provider value={organization}>
+        <IncidentsList
+          params={{orgId: organization.slug}}
+          location={{query: {}, search: ''}}
+          router={router}
+          {...props}
+        />
+      </OrganizationContext.Provider>,
       {context: routerContext}
     );
   };
@@ -93,9 +96,7 @@ describe('IncidentsList', function () {
     expect(within(items[0]).getByText('First incident')).toBeInTheDocument();
     expect(within(items[1]).getByText('Second incident')).toBeInTheDocument();
 
-    // PageFiltersContainer loads projects + the Projects render-prop component
-    // to load projects for all rows.
-    expect(projectMock).toHaveBeenCalledTimes(2);
+    expect(projectMock).toHaveBeenCalledTimes(1);
 
     expect(projectMock).toHaveBeenLastCalledWith(
       expect.anything(),

+ 10 - 7
tests/js/spec/views/alerts/rules/index.spec.jsx

@@ -7,6 +7,7 @@ import TeamStore from 'sentry/stores/teamStore';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 import AlertRulesList from 'sentry/views/alerts/rules';
 import {IncidentStatus} from 'sentry/views/alerts/types';
+import {OrganizationContext} from 'sentry/views/organizationContext';
 
 jest.mock('sentry/utils/analytics/trackAdvancedAnalyticsEvent');
 
@@ -20,13 +21,15 @@ describe('OrganizationRuleList', () => {
     '<https://sentry.io/api/0/organizations/org-slug/combined-rules/?cursor=0:100:0>; rel="next"; results="true"; cursor="0:100:0"';
 
   const getComponent = props => (
-    <AlertRulesList
-      organization={organization}
-      params={{orgId: organization.slug}}
-      location={{query: {}, search: ''}}
-      router={router}
-      {...props}
-    />
+    <OrganizationContext.Provider value={organization}>
+      <AlertRulesList
+        organization={organization}
+        params={{orgId: organization.slug}}
+        location={{query: {}, search: ''}}
+        router={router}
+        {...props}
+      />
+    </OrganizationContext.Provider>
   );
 
   const createWrapper = props => render(getComponent(props), {context: routerContext});

Some files were not shown because too many files changed in this diff