Browse Source

feat(gsh): Hide the GSH when selection-filters-v2 is on (#30608)

Evan Purkhiser 3 years ago
parent
commit
bdd7c593d4

+ 94 - 101
static/app/components/organizations/globalSelectionHeader/globalSelectionHeader.tsx

@@ -20,7 +20,6 @@ import Tooltip from 'sentry/components/tooltip';
 import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
 import {IconArrow} from 'sentry/icons';
 import {t} from 'sentry/locale';
-import {PageContent} from 'sentry/styles/organization';
 import space from 'sentry/styles/space';
 import {GlobalSelection, MinimalProject, Organization, Project} from 'sentry/types';
 import {callIfFunction} from 'sentry/utils/callIfFunction';
@@ -53,7 +52,6 @@ const defaultProps = {
 };
 
 type Props = {
-  children?: React.ReactNode;
   organization: Organization;
 
   memberProjects: Project[];
@@ -280,7 +278,6 @@ class GlobalSelectionHeader extends React.Component<Props, State> {
   render() {
     const {
       className,
-      children,
       shouldForceProject,
       forceProject,
       isGlobalSelectionReady,
@@ -312,106 +309,102 @@ class GlobalSelectionHeader extends React.Component<Props, State> {
       : this.props.selection.projects;
 
     return (
-      <React.Fragment>
-        <Header className={className}>
-          <HeaderItemPosition>
-            {showIssueStreamLink && this.getBackButton()}
-            <Projects
-              orgId={organization.slug}
-              limit={PROJECTS_PER_PAGE}
-              slugs={specificProjectSlugs}
-            >
-              {({projects, hasMore, onSearch, fetching}) => {
-                const paginatedProjectSelectorCallbacks = {
-                  onScroll: ({clientHeight, scrollHeight, scrollTop}) => {
-                    // check if no new projects are being fetched and the user has
-                    // scrolled far enough to fetch a new page of projects
-                    if (
-                      !fetching &&
-                      scrollTop + clientHeight >= scrollHeight - clientHeight &&
-                      hasMore
-                    ) {
-                      this.scrollFetchDispatcher(onSearch, {append: true});
-                    }
-                  },
-                  onFilterChange: event => {
-                    this.searchDispatcher(onSearch, event.target.value, {
-                      append: false,
-                    });
-                  },
-                  searching: fetching,
-                  paginated: true,
-                };
-                return (
-                  <MultipleProjectSelector
-                    organization={organization}
-                    shouldForceProject={shouldForceProject}
-                    forceProject={forceProject}
-                    projects={loadingProjects ? (projects as Project[]) : memberProjects}
-                    isGlobalSelectionReady={isGlobalSelectionReady}
-                    nonMemberProjects={nonMemberProjects}
-                    value={this.state.projects || this.props.selection.projects}
-                    onChange={this.handleChangeProjects}
-                    onUpdate={this.handleUpdateProjects}
-                    disableMultipleProjectSelection={disableMultipleProjectSelection}
-                    {...(loadingProjects ? paginatedProjectSelectorCallbacks : {})}
-                    showIssueStreamLink={showIssueStreamLink}
-                    showProjectSettingsLink={showProjectSettingsLink}
-                    lockedMessageSubject={lockedMessageSubject}
-                    footerMessage={projectsFooterMessage}
-                  />
-                );
-              }}
-            </Projects>
-          </HeaderItemPosition>
-
-          {showEnvironmentSelector && (
-            <React.Fragment>
-              <HeaderSeparator />
-              <HeaderItemPosition>
-                <MultipleEnvironmentSelector
-                  organization={organization}
-                  projects={this.props.projects}
-                  loadingProjects={loadingProjects}
-                  selectedProjects={selectedProjects}
-                  value={this.props.selection.environments}
-                  onChange={this.handleChangeEnvironments}
-                  onUpdate={this.handleUpdateEnvironmments}
-                />
-              </HeaderItemPosition>
-            </React.Fragment>
-          )}
-
-          {showDateSelector && (
-            <React.Fragment>
-              <HeaderSeparator />
-              <HeaderItemPosition>
-                <TimeRangeSelector
-                  key={`period:${period}-start:${start}-end:${end}-utc:${utc}-defaultPeriod:${defaultPeriod}`}
-                  showAbsolute={showAbsolute}
-                  showRelative={showRelative}
-                  relative={period}
-                  start={start}
-                  end={end}
-                  utc={utc}
-                  onChange={this.handleChangeTime}
-                  onUpdate={this.handleUpdateTime}
+      <Header className={className}>
+        <HeaderItemPosition>
+          {showIssueStreamLink && this.getBackButton()}
+          <Projects
+            orgId={organization.slug}
+            limit={PROJECTS_PER_PAGE}
+            slugs={specificProjectSlugs}
+          >
+            {({projects, hasMore, onSearch, fetching}) => {
+              const paginatedProjectSelectorCallbacks = {
+                onScroll: ({clientHeight, scrollHeight, scrollTop}) => {
+                  // check if no new projects are being fetched and the user has
+                  // scrolled far enough to fetch a new page of projects
+                  if (
+                    !fetching &&
+                    scrollTop + clientHeight >= scrollHeight - clientHeight &&
+                    hasMore
+                  ) {
+                    this.scrollFetchDispatcher(onSearch, {append: true});
+                  }
+                },
+                onFilterChange: event => {
+                  this.searchDispatcher(onSearch, event.target.value, {
+                    append: false,
+                  });
+                },
+                searching: fetching,
+                paginated: true,
+              };
+              return (
+                <MultipleProjectSelector
                   organization={organization}
-                  defaultPeriod={defaultPeriod}
-                  hint={timeRangeHint}
-                  relativeOptions={relativeDateOptions}
-                  maxPickableDays={maxPickableDays}
+                  shouldForceProject={shouldForceProject}
+                  forceProject={forceProject}
+                  projects={loadingProjects ? (projects as Project[]) : memberProjects}
+                  isGlobalSelectionReady={isGlobalSelectionReady}
+                  nonMemberProjects={nonMemberProjects}
+                  value={this.state.projects || this.props.selection.projects}
+                  onChange={this.handleChangeProjects}
+                  onUpdate={this.handleUpdateProjects}
+                  disableMultipleProjectSelection={disableMultipleProjectSelection}
+                  {...(loadingProjects ? paginatedProjectSelectorCallbacks : {})}
+                  showIssueStreamLink={showIssueStreamLink}
+                  showProjectSettingsLink={showProjectSettingsLink}
+                  lockedMessageSubject={lockedMessageSubject}
+                  footerMessage={projectsFooterMessage}
                 />
-              </HeaderItemPosition>
-            </React.Fragment>
-          )}
-
-          {!showEnvironmentSelector && <HeaderItemPosition isSpacer />}
-          {!showDateSelector && <HeaderItemPosition isSpacer />}
-        </Header>
-
-        {isGlobalSelectionReady ? children : <PageContent />}
-      </React.Fragment>
+              );
+            }}
+          </Projects>
+        </HeaderItemPosition>
+
+        {showEnvironmentSelector && (
+          <React.Fragment>
+            <HeaderSeparator />
+            <HeaderItemPosition>
+              <MultipleEnvironmentSelector
+                organization={organization}
+                projects={this.props.projects}
+                loadingProjects={loadingProjects}
+                selectedProjects={selectedProjects}
+                value={this.props.selection.environments}
+                onChange={this.handleChangeEnvironments}
+                onUpdate={this.handleUpdateEnvironmments}
+              />
+            </HeaderItemPosition>
+          </React.Fragment>
+        )}
+
+        {showDateSelector && (
+          <React.Fragment>
+            <HeaderSeparator />
+            <HeaderItemPosition>
+              <TimeRangeSelector
+                key={`period:${period}-start:${start}-end:${end}-utc:${utc}-defaultPeriod:${defaultPeriod}`}
+                showAbsolute={showAbsolute}
+                showRelative={showRelative}
+                relative={period}
+                start={start}
+                end={end}
+                utc={utc}
+                onChange={this.handleChangeTime}
+                onUpdate={this.handleUpdateTime}
+                organization={organization}
+                defaultPeriod={defaultPeriod}
+                hint={timeRangeHint}
+                relativeOptions={relativeDateOptions}
+                maxPickableDays={maxPickableDays}
+              />
+            </HeaderItemPosition>
+          </React.Fragment>
+        )}
+
+        {!showEnvironmentSelector && <HeaderItemPosition isSpacer />}
+        {!showDateSelector && <HeaderItemPosition isSpacer />}
+      </Header>
     );
   }
 }

+ 21 - 3
static/app/components/organizations/globalSelectionHeader/index.tsx

@@ -1,4 +1,4 @@
-import {useEffect, useRef} from 'react';
+import {Fragment, useEffect, useRef} from 'react';
 import {withRouter, WithRouterProps} from 'react-router';
 import isEqual from 'lodash/isEqual';
 import partition from 'lodash/partition';
@@ -11,6 +11,9 @@ import {
 } from 'sentry/actionCreators/globalSelection';
 import {DATE_TIME_KEYS} from 'sentry/constants/pageFilters';
 import ConfigStore from 'sentry/stores/configStore';
+import GlobalSelectionStore from 'sentry/stores/globalSelectionStore';
+import {useLegacyStore} from 'sentry/stores/useLegacyStore';
+import {PageContent} from 'sentry/styles/organization';
 import useProjects from 'sentry/utils/useProjects';
 import withOrganization from 'sentry/utils/withOrganization';
 
@@ -43,7 +46,7 @@ type Props = WithRouterProps &
     skipLoadLastUsed?: boolean;
   };
 
-function GlobalSelectionHeaderContainer({skipLoadLastUsed, ...props}: Props) {
+function GlobalSelectionHeaderContainer({skipLoadLastUsed, children, ...props}: Props) {
   const {
     location,
     router,
@@ -55,6 +58,8 @@ function GlobalSelectionHeaderContainer({skipLoadLastUsed, ...props}: Props) {
     specificProjectSlugs,
   } = props;
 
+  const {isReady} = useLegacyStore(GlobalSelectionStore);
+
   const {projects, initiallyLoaded: projectsLoaded} = useProjects();
 
   const {isSuperuser} = ConfigStore.get('user');
@@ -148,7 +153,20 @@ function GlobalSelectionHeaderContainer({skipLoadLastUsed, ...props}: Props) {
     lastQuery.current = location.query;
   }, [location.query]);
 
-  return <GlobalSelectionHeader {...props} {...additionalProps} />;
+  // Wait for global selection to be ready before rendering chilren
+  if (!isReady) {
+    return <PageContent />;
+  }
+
+  // New-style selection filters no longer have a 'global'header
+  const hasGlobalHeader = !organization.features.includes('selection-filters-v2');
+
+  return (
+    <Fragment>
+      {hasGlobalHeader && <GlobalSelectionHeader {...props} {...additionalProps} />}
+      {children}
+    </Fragment>
+  );
 }
 
 export default withOrganization(withRouter(GlobalSelectionHeaderContainer));

+ 10 - 16
tests/js/sentry-test/utils.tsx

@@ -1,7 +1,8 @@
-import {screen} from 'sentry-test/reactTestingLibrary';
-
 // Taken from https://stackoverflow.com/a/56859650/1015027
-function findTextWithMarkup(contentNode: null | Element, textMatch: string | RegExp) {
+export function findTextWithMarkup(
+  contentNode: null | Element,
+  textMatch: string | RegExp
+) {
   const hasText = (node: Element) => node.textContent === textMatch;
   const nodeHasText = hasText(contentNode as Element);
   const childrenDontHaveText = Array.from(contentNode?.children || []).every(
@@ -11,19 +12,12 @@ function findTextWithMarkup(contentNode: null | Element, textMatch: string | Reg
 }
 
 /**
- * Search for a text broken up by multiple html elements
+ * May be used with a *ByText RTL matcher to match text within multiple nodes
+ *
  * e.g.: <div>Hello <span>world</span></div>
  */
-export function getByTextContent(textMatch: string | RegExp) {
-  return screen.getByText((_, contentNode) => findTextWithMarkup(contentNode, textMatch));
-}
-
-/**
- * Search for *all* texts broken up by multiple html elements
- * e.g.: <div><div>Hello <span>world</span></div><div>Hello <span>world</span></div></div>
- */
-export function getAllByTextContent(textMatch: string | RegExp) {
-  return screen.getAllByText((_, contentNode) =>
-    findTextWithMarkup(contentNode, textMatch)
-  );
+export function textWithMarkupMatcher(textMatch: string | RegExp) {
+  return function (_: string, element: Element | null) {
+    return findTextWithMarkup(element, textMatch);
+  };
 }

+ 4 - 2
tests/js/spec/components/eventOrGroupHeader.spec.tsx

@@ -1,6 +1,6 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
-import {getByTextContent} from 'sentry-test/utils';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import EventOrGroupHeader from 'sentry/components/eventOrGroupHeader';
 import {EventOrGroupType} from 'sentry/types';
@@ -115,7 +115,9 @@ describe('EventOrGroupHeader', function () {
         {context: routerContext}
       );
 
-      expect(getByTextContent('in path/to/file.swift')).toBeInTheDocument();
+      expect(
+        screen.getByText(textWithMarkupMatcher('in path/to/file.swift'))
+      ).toBeInTheDocument();
     });
   });
 

+ 4 - 4
tests/js/spec/components/events/interfaces/breadcrumbs/breadcrumbs.spec.tsx

@@ -1,6 +1,6 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
 import {mountWithTheme, screen, userEvent} from 'sentry-test/reactTestingLibrary';
-import {getAllByTextContent} from 'sentry-test/utils';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import Breadcrumbs from 'sentry/components/events/interfaces/breadcrumbs';
 import {BreadcrumbLevelType, BreadcrumbType} from 'sentry/types/breadcrumbs';
@@ -84,7 +84,7 @@ describe('Breadcrumbs', () => {
         screen.queryByText('Sorry, no breadcrumbs match your search query')
       ).not.toBeInTheDocument();
 
-      expect(getAllByTextContent('sup')).toHaveLength(3);
+      expect(screen.getAllByText(textWithMarkupMatcher('sup'))).toHaveLength(3);
     });
 
     it('should filter crumbs based on crumb level', function () {
@@ -94,7 +94,7 @@ describe('Breadcrumbs', () => {
 
       // breadcrumbs + filter item
       // TODO(Priscila): Filter should not render in the dom if not open
-      expect(getAllByTextContent('Warning')).toHaveLength(6);
+      expect(screen.getAllByText(textWithMarkupMatcher('Warning'))).toHaveLength(6);
     });
 
     it('should filter crumbs based on crumb category', function () {
@@ -102,7 +102,7 @@ describe('Breadcrumbs', () => {
 
       userEvent.type(screen.getByPlaceholderText('Search breadcrumbs'), 'error');
 
-      expect(getAllByTextContent('error')).toHaveLength(2);
+      expect(screen.getAllByText(textWithMarkupMatcher('error'))).toHaveLength(2);
     });
   });
 

+ 16 - 6
tests/js/spec/components/organizations/globalSelectionHeader.spec.jsx

@@ -628,7 +628,7 @@ describe('GlobalSelectionHeader', function () {
   });
 
   describe('forceProject selection mode', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       MockApiClient.addMockResponse({
         url: '/organizations/org-slug/projects/',
         body: [],
@@ -655,6 +655,9 @@ describe('GlobalSelectionHeader', function () {
         />,
         initialData.routerContext
       );
+
+      await tick();
+      wrapper.update();
     });
 
     it('renders a back button to the forced project', function () {
@@ -662,9 +665,9 @@ describe('GlobalSelectionHeader', function () {
       expect(back).toHaveLength(1);
     });
 
-    it('renders only environments from the forced project', async function () {
-      await wrapper.find('MultipleEnvironmentSelector HeaderItem').simulate('click');
-      await wrapper.update();
+    it('renders only environments from the forced project', function () {
+      wrapper.find('MultipleEnvironmentSelector HeaderItem').simulate('click');
+      wrapper.update();
 
       const items = wrapper.find('MultipleEnvironmentSelector EnvironmentSelectorItem');
       expect(items.length).toEqual(1);
@@ -931,7 +934,8 @@ describe('GlobalSelectionHeader', function () {
 
   describe('projects list', function () {
     let memberProject, nonMemberProject, initialData;
-    beforeEach(function () {
+
+    beforeEach(async function () {
       memberProject = TestStubs.Project({id: '3', isMember: true});
       nonMemberProject = TestStubs.Project({id: '4', isMember: false});
       initialData = initializeOrg({
@@ -950,6 +954,9 @@ describe('GlobalSelectionHeader', function () {
         <GlobalSelectionHeader organization={initialData.organization} />,
         initialData.routerContext
       );
+
+      await tick();
+      wrapper.update();
     });
 
     it('gets member projects', function () {
@@ -958,7 +965,7 @@ describe('GlobalSelectionHeader', function () {
       ]);
     });
 
-    it('gets all projects if superuser', function () {
+    it('gets all projects if superuser', async function () {
       ConfigStore.config = {
         user: {
           isSuperuser: true,
@@ -970,6 +977,9 @@ describe('GlobalSelectionHeader', function () {
         initialData.routerContext
       );
 
+      await tick();
+      wrapper.update();
+
       expect(wrapper.find('MultipleProjectSelector').prop('projects')).toEqual([
         memberProject,
       ]);

+ 6 - 12
tests/js/spec/views/projectDetail/index.spec.tsx

@@ -1,10 +1,6 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {
-  mountWithTheme,
-  screen,
-  waitForElementToBeRemoved,
-} from 'sentry-test/reactTestingLibrary';
-import {getByTextContent} from 'sentry-test/utils';
+import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import GlobalSelectionStore from 'sentry/stores/globalSelectionStore';
 import ProjectsStore from 'sentry/stores/projectsStore';
@@ -56,8 +52,6 @@ describe('ProjectDetail', function () {
         {context: routerContext}
       );
 
-      await waitForElementToBeRemoved(() => screen.getByText('Loading\u2026'));
-
       expect(
         screen.queryByText(
           'Event Processing for this project is currently degraded. Events may appear with larger delays than usual or get dropped.',
@@ -93,11 +87,11 @@ describe('ProjectDetail', function () {
         {context: routerContext}
       );
 
-      await waitForElementToBeRemoved(() => screen.getByText('Loading\u2026'));
-
       expect(
-        getByTextContent(
-          'Event Processing for this project is currently degraded. Events may appear with larger delays than usual or get dropped. Please check the Status page for a potential outage.'
+        await screen.findByText(
+          textWithMarkupMatcher(
+            'Event Processing for this project is currently degraded. Events may appear with larger delays than usual or get dropped. Please check the Status page for a potential outage.'
+          )
         )
       ).toBeInTheDocument();
     });

+ 11 - 7
tests/js/spec/views/settings/projectFiltersAndSampling/index.spec.tsx

@@ -1,5 +1,5 @@
 import {screen} from 'sentry-test/reactTestingLibrary';
-import {getByTextContent} from 'sentry-test/utils';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import {DYNAMIC_SAMPLING_DOC_LINK} from 'sentry/views/settings/project/filtersAndSampling/utils';
 
@@ -21,10 +21,12 @@ describe('Filters and Sampling', function () {
 
       // Error rules container
       expect(
-        getByTextContent(
-          'Manage the inbound data you want to store. To change the sampling rate or rate limits, update your SDK configuration. The rules added below will apply on top of your SDK configuration. Any new rule may take a few minutes to propagate.'
+        screen.getByText(
+          textWithMarkupMatcher(
+            'Manage the inbound data you want to store. To change the sampling rate or rate limits, update your SDK configuration. The rules added below will apply on top of your SDK configuration. Any new rule may take a few minutes to propagate.'
+          )
         )
-      ).toBeTruthy();
+      ).toBeInTheDocument();
 
       expect(
         screen.getByRole('link', {
@@ -137,10 +139,12 @@ describe('Filters and Sampling', function () {
 
       // Error rules container
       expect(
-        getByTextContent(
-          'Manage the inbound data you want to store. To change the sampling rate or rate limits, update your SDK configuration. The rules added below will apply on top of your SDK configuration. Any new rule may take a few minutes to propagate.'
+        screen.getByText(
+          textWithMarkupMatcher(
+            'Manage the inbound data you want to store. To change the sampling rate or rate limits, update your SDK configuration. The rules added below will apply on top of your SDK configuration. Any new rule may take a few minutes to propagate.'
+          )
         )
-      ).toBeTruthy();
+      ).toBeInTheDocument();
 
       expect(
         screen.getByRole('link', {

+ 6 - 4
tests/js/spec/views/settings/projectFiltersAndSampling/transactions.spec.tsx

@@ -3,7 +3,7 @@ import {
   userEvent,
   waitForElementToBeRemoved,
 } from 'sentry-test/reactTestingLibrary';
-import {getByTextContent} from 'sentry-test/utils';
+import {textWithMarkupMatcher} from 'sentry-test/utils';
 
 import {
   DYNAMIC_SAMPLING_DOC_LINK,
@@ -234,10 +234,12 @@ describe('Filters and Sampling - Transaction rule', function () {
         expect(screen.getByText('Tracing')).toBeInTheDocument();
         expect(screen.getByRole('checkbox')).toBeChecked();
         expect(
-          getByTextContent(
-            'Include all related transactions by trace ID. This can span across multiple projects. All related errors will remain. Learn more about tracing.'
+          screen.getByText(
+            textWithMarkupMatcher(
+              'Include all related transactions by trace ID. This can span across multiple projects. All related errors will remain. Learn more about tracing.'
+            )
           )
-        ).toBeTruthy();
+        ).toBeInTheDocument();
         expect(
           screen.getByRole('link', {
             name: 'Learn more about tracing',