Browse Source

feat(sample): Use sample_event tag to show sample alert banner on transaction & error detail page (#32023)

* Use tag for sample transactions

* Use sample_event tag on group details page

* fix a few issues

* fix testsfor group detail

* fix performance detail view test
Zhixing Zhang 3 years ago
parent
commit
96d12e43e2

+ 1 - 1
static/app/utils/analytics/growthAnalyticsEvents.tsx

@@ -67,8 +67,8 @@ export type GrowthEventParameters = {
   'growth.platformpicker_category': PlatformCategory;
   'growth.platformpicker_search': PlatformSearchParam;
   'growth.sample_error_onboarding_link_clicked': {
-    project_id: string;
     platform?: string;
+    project_id?: string;
   };
   'growth.sample_transaction_docs_link_clicked': {
     project_id: string;

+ 5 - 23
static/app/views/issueList/container.tsx

@@ -1,13 +1,9 @@
-import {Fragment, useEffect, useState} from 'react';
-
 import NoProjectMessage from 'sentry/components/noProjectMessage';
 import PageFiltersContainer from 'sentry/components/organizations/pageFilters/container';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
-import GroupStore from 'sentry/stores/groupStore';
 import {Organization, Project} from 'sentry/types';
 import withOrganization from 'sentry/utils/withOrganization';
-import SampleEventAlert from 'sentry/views/organizationGroupDetails/sampleEventAlert';
 
 type Props = {
   children: React.ReactChildren;
@@ -16,27 +12,13 @@ type Props = {
 };
 
 function IssueListContainer({organization, children}: Props) {
-  const [showSampleEventBanner, setShowSampleEventBanner] = useState(false);
-
-  useEffect(() => {
-    const unlistener = GroupStore.listen(
-      () => setShowSampleEventBanner(GroupStore.getAllItemIds().length === 1),
-      undefined
-    );
-
-    return () => unlistener();
-  }, []);
-
   return (
     <SentryDocumentTitle title={t('Issues')} orgSlug={organization.slug}>
-      <Fragment>
-        {showSampleEventBanner && <SampleEventAlert />}
-        <PageFiltersContainer
-          hideGlobalHeader={organization.features.includes('selection-filters-v2')}
-        >
-          <NoProjectMessage organization={organization}>{children}</NoProjectMessage>
-        </PageFiltersContainer>
-      </Fragment>
+      <PageFiltersContainer
+        hideGlobalHeader={organization.features.includes('selection-filters-v2')}
+      >
+        <NoProjectMessage organization={organization}>{children}</NoProjectMessage>
+      </PageFiltersContainer>
     </SentryDocumentTitle>
   );
 }

+ 22 - 14
static/app/views/organizationGroupDetails/groupDetails.tsx

@@ -23,6 +23,7 @@ import withApi from 'sentry/utils/withApi';
 
 import {ERROR_TYPES} from './constants';
 import GroupHeader from './header';
+import SampleEventAlert from './sampleEventAlert';
 import {Tab} from './types';
 import {
   fetchGroupEvent,
@@ -544,22 +545,29 @@ class GroupDetails extends React.Component<Props, State> {
   }
 
   render() {
-    const {project} = this.state;
+    const {project, group} = this.state;
+    const {organization} = this.props;
+    const isSampleError = group?.tags.some(tag => tag.key === 'sample_event');
 
     return (
-      <SentryDocumentTitle noSuffix title={this.getTitle()}>
-        <PageFiltersContainer
-          skipLoadLastUsed
-          forceProject={project}
-          showDateSelector={false}
-          shouldForceProject
-          lockedMessageSubject={t('issue')}
-          showIssueStreamLink
-          showProjectSettingsLink
-        >
-          <PageContent>{this.renderPageContent()}</PageContent>
-        </PageFiltersContainer>
-      </SentryDocumentTitle>
+      <React.Fragment>
+        {isSampleError && project && (
+          <SampleEventAlert project={project} organization={organization} />
+        )}
+        <SentryDocumentTitle noSuffix title={this.getTitle()}>
+          <PageFiltersContainer
+            skipLoadLastUsed
+            forceProject={project}
+            showDateSelector={false}
+            shouldForceProject
+            lockedMessageSubject={t('issue')}
+            showIssueStreamLink
+            showProjectSettingsLink
+          >
+            <PageContent>{this.renderPageContent()}</PageContent>
+          </PageFiltersContainer>
+        </SentryDocumentTitle>
+      </React.Fragment>
     );
   }
 }

+ 5 - 10
static/app/views/organizationGroupDetails/index.tsx

@@ -8,7 +8,6 @@ import withPageFilters from 'sentry/utils/withPageFilters';
 import withProjects from 'sentry/utils/withProjects';
 
 import GroupDetails from './groupDetails';
-import SampleEventAlert from './sampleEventAlert';
 
 type Props = {
   children: React.ReactNode;
@@ -29,15 +28,11 @@ class OrganizationGroupDetails extends React.Component<Props> {
   render() {
     const {selection, ...props} = this.props;
     return (
-      <React.Fragment>
-        <SampleEventAlert />
-
-        <GroupDetails
-          key={`${this.props.params.groupId}-envs:${selection.environments.join(',')}`}
-          environments={selection.environments}
-          {...props}
-        />
-      </React.Fragment>
+      <GroupDetails
+        key={`${this.props.params.groupId}-envs:${selection.environments.join(',')}`}
+        environments={selection.environments}
+        {...props}
+      />
     );
   }
 }

+ 8 - 23
static/app/views/organizationGroupDetails/sampleEventAlert.tsx

@@ -5,31 +5,16 @@ import PageAlertBar from 'sentry/components/pageAlertBar';
 import {IconLightning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {Organization, PageFilters, Project} from 'sentry/types';
+import {AvatarProject, Organization} from 'sentry/types';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
-import withOrganization from 'sentry/utils/withOrganization';
-import withPageFilters from 'sentry/utils/withPageFilters';
-import withProjects from 'sentry/utils/withProjects';
 
 function SampleEventAlert({
-  selection,
   organization,
-  projects,
+  project,
 }: {
   organization: Organization;
-  projects: Project[];
-  selection: PageFilters;
+  project: AvatarProject;
 }) {
-  if (projects.length === 0) {
-    return null;
-  }
-  if (selection.projects.length !== 1) {
-    return null;
-  }
-  const selectedProject = projects.find(p => p.id === selection.projects[0].toString());
-  if (!selectedProject || selectedProject.firstEvent) {
-    return null;
-  }
   return (
     <PageAlertBar>
       <IconLightning />
@@ -41,14 +26,14 @@ function SampleEventAlert({
       <Button
         size="xsmall"
         priority="primary"
-        to={`/${organization.slug}/${selectedProject.slug}/getting-started/${
-          selectedProject.platform || ''
+        to={`/${organization.slug}/${project.slug}/getting-started/${
+          project.platform || ''
         }`}
         onClick={() =>
           trackAdvancedAnalyticsEvent('growth.sample_error_onboarding_link_clicked', {
-            project_id: selectedProject.id,
+            project_id: project.id?.toString(),
             organization,
-            platform: selectedProject.platform,
+            platform: project.platform,
           })
         }
       >
@@ -58,7 +43,7 @@ function SampleEventAlert({
   );
 }
 
-export default withProjects(withOrganization(withPageFilters(SampleEventAlert)));
+export default SampleEventAlert;
 
 const TextWrapper = styled('span')`
   margin: 0 ${space(1)};

+ 13 - 1
static/app/views/performance/transactionDetails/content.tsx

@@ -33,6 +33,7 @@ import Breadcrumb from 'sentry/views/performance/breadcrumb';
 import {transactionSummaryRouteWithQuery} from '../transactionSummary/utils';
 
 import EventMetas from './eventMetas';
+import FinishSetupAlert from './finishSetupAlert';
 
 type Props = Pick<
   RouteComponentProps<{eventSlug: string}, {}>,
@@ -98,12 +99,23 @@ class EventDetailsContent extends AsyncComponent<Props, State> {
 
   renderBody() {
     const {event} = this.state;
+    const {organization} = this.props;
 
     if (!event) {
       return <NotFound />;
     }
+    const isSampleTransaction = event.tags.some(
+      tag => tag.key === 'sample_event' && tag.value === 'yes'
+    );
 
-    return this.renderContent(event);
+    return (
+      <Fragment>
+        {isSampleTransaction && (
+          <FinishSetupAlert organization={organization} projectId={this.projectId} />
+        )}
+        {this.renderContent(event)}
+      </Fragment>
+    );
   }
 
   renderContent(event: Event) {

+ 4 - 4
static/app/views/performance/transactionDetails/finishSetupAlert.tsx

@@ -5,15 +5,15 @@ import PageAlertBar from 'sentry/components/pageAlertBar';
 import {IconLightning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {Organization, Project} from 'sentry/types';
+import {Organization} from 'sentry/types';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 
 export default function FinishSetupAlert({
   organization,
-  project,
+  projectId,
 }: {
   organization: Organization;
-  project: Project;
+  projectId: string;
 }) {
   return (
     <PageAlertBar>
@@ -31,7 +31,7 @@ export default function FinishSetupAlert({
         href="https://docs.sentry.io/performance-monitoring/getting-started/"
         onClick={() =>
           trackAdvancedAnalyticsEvent('growth.sample_transaction_docs_link_clicked', {
-            project_id: project.id,
+            project_id: projectId,
             organization,
           })
         }

+ 1 - 16
static/app/views/performance/transactionDetails/index.tsx

@@ -6,12 +6,10 @@ import NoProjectMessage from 'sentry/components/noProjectMessage';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
 import {PageContent} from 'sentry/styles/organization';
-import {Organization, Project} from 'sentry/types';
-import Projects from 'sentry/utils/projects';
+import {Organization} from 'sentry/types';
 import withOrganization from 'sentry/utils/withOrganization';
 
 import EventDetailsContent from './content';
-import FinishSetupAlert from './finishSetupAlert';
 
 type Props = RouteComponentProps<{eventSlug: string}, {}> & {
   organization: Organization;
@@ -37,19 +35,6 @@ class EventDetails extends Component<Props> {
       >
         <StyledPageContent>
           <NoProjectMessage organization={organization}>
-            <Projects orgId={organization.slug} slugs={[projectSlug]}>
-              {({projects}) => {
-                if (projects.length === 0) {
-                  return null;
-                }
-                const project = projects.find(p => p.slug === projectSlug) as Project;
-                // only render setup alert if the project has no real transactions
-                if (!project || project.firstTransactionEvent) {
-                  return null;
-                }
-                return <FinishSetupAlert organization={organization} project={project} />;
-              }}
-            </Projects>
             <EventDetailsContent
               organization={organization}
               location={location}

+ 10 - 11
tests/js/spec/views/organizationGroupDetails/groupDetails.spec.jsx

@@ -123,6 +123,9 @@ describe('groupDetails', () => {
     act(() => ProjectsStore.loadInitialData(organization.projects));
 
     expect(await screen.findByText(group.title, {exact: false})).toBeInTheDocument();
+
+    // Sample event alert should not show up
+    expect(screen.queryByText(SAMPLE_EVENT_ALERT_TEXT)).not.toBeInTheDocument();
   });
 
   it('renders error when issue is not found', async function () {
@@ -221,19 +224,15 @@ describe('groupDetails', () => {
   });
 
   it('renders alert for sample event', async function () {
-    const aProject = TestStubs.Project({firstEvent: false});
-    act(() => ProjectsStore.reset());
-    act(() => ProjectsStore.loadInitialData([aProject]));
-    createWrapper();
+    const sampleGruop = TestStubs.Group();
+    sampleGruop.tags.push({key: 'sample_event'});
+    MockApiClient.addMockResponse({
+      url: `/issues/${group.id}/`,
+      body: {...sampleGruop},
+    });
 
-    expect(await screen.findByText(SAMPLE_EVENT_ALERT_TEXT)).toBeInTheDocument();
-  });
-  it('does not render alert for non sample events', function () {
-    const aProject = TestStubs.Project({firstEvent: false});
-    act(() => ProjectsStore.reset());
-    act(() => ProjectsStore.loadInitialData([aProject]));
     createWrapper();
 
-    expect(screen.queryByText(SAMPLE_EVENT_ALERT_TEXT)).not.toBeInTheDocument();
+    expect(await screen.findByText(SAMPLE_EVENT_ALERT_TEXT)).toBeInTheDocument();
   });
 });

+ 34 - 12
tests/js/spec/views/performance/transactionDetails/index.spec.jsx

@@ -1,6 +1,7 @@
 import {cleanup, mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
 
 import ProjectsStore from 'sentry/stores/projectsStore';
+import {OrganizationContext} from 'sentry/views/organizationContext';
 import EventDetails from 'sentry/views/performance/transactionDetails';
 
 const alertText =
@@ -9,7 +10,7 @@ const alertText =
 describe('EventDetails', () => {
   afterEach(cleanup);
 
-  it('renders alert for sample transaction', () => {
+  it('renders alert for sample transaction', async () => {
     const project = TestStubs.Project();
     ProjectsStore.loadInitialData([project]);
     const organization = TestStubs.Organization({
@@ -17,20 +18,31 @@ describe('EventDetails', () => {
       projects: [project],
     });
     const event = TestStubs.Event();
+    event.tags.push({key: 'sample_event', value: 'yes'});
     const routerContext = TestStubs.routerContext([]);
 
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/latest/`,
+      statusCode: 200,
+      body: {
+        ...event,
+      },
+    });
+
     mountWithTheme(
-      <EventDetails
-        organization={organization}
-        params={{orgId: organization.slug, eventSlug: `${project.slug}:${event.id}`}}
-        location={routerContext.context.location}
-      />
+      <OrganizationContext.Provider value={organization}>
+        <EventDetails
+          organization={organization}
+          params={{orgId: organization.slug, eventSlug: 'latest'}}
+          location={routerContext.context.location}
+        />
+      </OrganizationContext.Provider>
     );
     expect(screen.getByText(alertText)).toBeInTheDocument();
   });
 
   it('does not reender alert if already received transaction', () => {
-    const project = TestStubs.Project({firstTransactionEvent: true});
+    const project = TestStubs.Project();
     ProjectsStore.loadInitialData([project]);
     const organization = TestStubs.Organization({
       features: ['performance-view'],
@@ -39,12 +51,22 @@ describe('EventDetails', () => {
     const event = TestStubs.Event();
     const routerContext = TestStubs.routerContext([]);
 
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/latest/`,
+      statusCode: 200,
+      body: {
+        ...event,
+      },
+    });
+
     mountWithTheme(
-      <EventDetails
-        organization={organization}
-        params={{orgId: organization.slug, eventSlug: `${project.slug}:${event.id}`}}
-        location={routerContext.context.location}
-      />
+      <OrganizationContext.Provider value={organization}>
+        <EventDetails
+          organization={organization}
+          params={{orgId: organization.slug, eventSlug: 'latest'}}
+          location={routerContext.context.location}
+        />
+      </OrganizationContext.Provider>
     );
     expect(screen.queryByText(alertText)).not.toBeInTheDocument();
   });