Browse Source

fix(stat-detectors): Update event comparison controls (#59050)

Simplify event comparison controls to open event detail and
add pagination buttons. Also remove statistical detector related
code from generic group event carousel
Nar Saynorath 1 year ago
parent
commit
4290fb481e

+ 201 - 0
static/app/components/events/eventStatisticalDetector/eventComparison/eventDisplay.spec.tsx

@@ -0,0 +1,201 @@
+import {Event} from 'sentry-fixture/event';
+import {Project as ProjectMock} from 'sentry-fixture/project';
+
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+
+import {Project} from 'sentry/types';
+
+import {EventDisplay} from './eventDisplay';
+
+describe('eventDisplay', () => {
+  let mockProject: Project;
+
+  beforeEach(() => {
+    mockProject = ProjectMock();
+  });
+
+  it('renders an empty state if no events returned', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events/',
+      method: 'GET',
+      body: {data: []},
+    });
+
+    render(
+      <EventDisplay
+        durationBaseline={0}
+        end={0}
+        eventSelectLabel=""
+        project={mockProject}
+        start={0}
+        transaction=""
+      />
+    );
+
+    expect(await screen.findByText('Unable to find a sample event')).toBeInTheDocument();
+  });
+
+  it('renders an event with tags', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events/',
+      method: 'GET',
+      body: {
+        data: [
+          {
+            timestamp: new Date().toISOString(),
+            id: 'mock-id',
+            'project.name': mockProject.name,
+          },
+        ],
+      },
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/events/${mockProject.slug}:mock-id/`,
+      method: 'GET',
+      body: Event({tags: [{key: 'mock-tag', value: 'mock-value'}]}),
+    });
+
+    render(
+      <EventDisplay
+        durationBaseline={0}
+        end={0}
+        eventSelectLabel=""
+        project={mockProject}
+        start={0}
+        transaction=""
+      />
+    );
+
+    expect(await screen.findByText('mock-tag')).toBeInTheDocument();
+    expect(screen.getByText('mock-value')).toBeInTheDocument();
+  });
+
+  it('renders the event in the compact select with the defined label', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events/',
+      method: 'GET',
+      body: {
+        data: [
+          {
+            timestamp: new Date().toISOString(),
+            id: 'mock-id',
+            'project.name': mockProject.name,
+          },
+        ],
+      },
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/events/${mockProject.slug}:mock-id/`,
+      method: 'GET',
+      body: Event({tags: [{key: 'mock-tag', value: 'mock-value'}]}),
+    });
+
+    render(
+      <EventDisplay
+        durationBaseline={0}
+        end={0}
+        eventSelectLabel="Prefix"
+        project={mockProject}
+        start={0}
+        transaction=""
+      />
+    );
+
+    expect(
+      await screen.findByRole('button', {name: 'Prefix: mock-id'})
+    ).toBeInTheDocument();
+  });
+
+  it('renders a button that links to the event detail page', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events/',
+      method: 'GET',
+      body: {
+        data: [
+          {
+            timestamp: new Date().toISOString(),
+            id: 'mock-id',
+            'project.name': mockProject.name,
+          },
+        ],
+      },
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/events/${mockProject.slug}:mock-id/`,
+      method: 'GET',
+      body: Event({tags: [{key: 'mock-tag', value: 'mock-value'}]}),
+    });
+
+    render(
+      <EventDisplay
+        durationBaseline={0}
+        end={0}
+        eventSelectLabel="Prefix"
+        project={mockProject}
+        start={0}
+        transaction=""
+      />
+    );
+
+    expect(
+      await screen.findByRole('button', {name: 'Full Event Details'})
+    ).toHaveAttribute('href', '/organizations/org-slug/discover/project-slug:1/');
+  });
+
+  it('allows for pagination if there are more events loaded', async () => {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events/',
+      method: 'GET',
+      body: {
+        data: [
+          {
+            timestamp: new Date().toISOString(),
+            id: 'event1',
+            'project.name': mockProject.name,
+          },
+          {
+            timestamp: new Date().toISOString(),
+            id: 'event2',
+            'project.name': mockProject.name,
+          },
+        ],
+      },
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/events/${mockProject.slug}:event1/`,
+      method: 'GET',
+      body: Event({tags: [{key: 'mock-tag', value: 'mock-value-for-event1'}]}),
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/events/${mockProject.slug}:event2/`,
+      method: 'GET',
+      body: Event({tags: [{key: 'mock-tag', value: 'mock-value-for-event2'}]}),
+    });
+
+    render(
+      <EventDisplay
+        durationBaseline={0}
+        end={0}
+        eventSelectLabel=""
+        project={mockProject}
+        start={0}
+        transaction=""
+      />
+    );
+
+    expect(await screen.findByText('event1')).toBeInTheDocument();
+    expect(screen.getByText('mock-value-for-event1')).toBeInTheDocument();
+    expect(screen.getByRole('button', {name: 'Previous Event'})).toBeDisabled();
+
+    await userEvent.click(screen.getByRole('button', {name: 'Next Event'}));
+
+    expect(await screen.findByText('event2')).toBeInTheDocument();
+    expect(screen.getByText('mock-value-for-event2')).toBeInTheDocument();
+    expect(screen.getByRole('button', {name: 'Next Event'})).toBeDisabled();
+  });
+});

+ 132 - 32
static/app/components/events/eventStatisticalDetector/eventComparison/eventDisplay.tsx

@@ -1,6 +1,7 @@
 import {useEffect, useState} from 'react';
 import styled from '@emotion/styled';
 
+import {Button, LinkButton} from 'sentry/components/button';
 import {CompactSelect} from 'sentry/components/compactSelect';
 import DateTime from 'sentry/components/dateTime';
 import EmptyStateWarning from 'sentry/components/emptyStateWarning';
@@ -16,9 +17,11 @@ import OpsBreakdown from 'sentry/components/events/opsBreakdown';
 import Link from 'sentry/components/links/link';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import TextOverflow from 'sentry/components/textOverflow';
+import {Tooltip} from 'sentry/components/tooltip';
+import {IconChevron, IconOpen} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {EventTransaction, Group, Project} from 'sentry/types';
+import {EventTransaction, Project} from 'sentry/types';
 import {defined} from 'sentry/utils';
 import {useDiscoverQuery} from 'sentry/utils/discover/discoverQuery';
 import EventView from 'sentry/utils/discover/eventView';
@@ -28,7 +31,9 @@ import {getShortEventId} from 'sentry/utils/events';
 import {useApiQuery} from 'sentry/utils/queryClient';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
-import {GroupEventActions} from 'sentry/views/issueDetails/groupEventCarousel';
+
+const BUTTON_ICON_SIZE = 'sm';
+const BUTTON_SIZE = 'sm';
 
 export function getSampleEventQuery({
   transaction,
@@ -92,19 +97,41 @@ function useFetchSampleEvents({
     eventView,
     location,
     orgSlug: organization.slug,
-    limit: 5,
+    limit: 20,
   });
 }
 
-type EventDisplayProps = {
+interface NavButtonProps {
+  disabled: boolean;
+  icon: React.ReactNode;
+  onPaginate: () => void;
+  title: string;
+}
+
+function NavButton({title, disabled, icon, onPaginate}: NavButtonProps) {
+  return (
+    <Tooltip title={title} disabled={disabled} skipWrapper>
+      <div>
+        <StyledNavButton
+          size={BUTTON_SIZE}
+          aria-label={title}
+          icon={icon}
+          disabled={disabled}
+          onClick={onPaginate}
+        />
+      </div>
+    </Tooltip>
+  );
+}
+
+interface EventDisplayProps {
   durationBaseline: number;
   end: number;
   eventSelectLabel: string;
-  group: Group;
   project: Project;
   start: number;
   transaction: string;
-};
+}
 
 function EventDisplay({
   eventSelectLabel,
@@ -113,7 +140,6 @@ function EventDisplay({
   end,
   transaction,
   durationBaseline,
-  group,
 }: EventDisplayProps) {
   const location = useLocation();
   const organization = useOrganization();
@@ -140,6 +166,12 @@ function EventDisplay({
     }
   }, [eventIds, selectedEventId]);
 
+  const eventIdIndex =
+    eventIds && eventIds.findIndex(eventId => eventId === selectedEventId);
+  const hasNext =
+    defined(eventIdIndex) && defined(eventIds) && eventIdIndex + 1 < eventIds.length;
+  const hasPrev = defined(eventIdIndex) && eventIdIndex - 1 >= 0;
+
   if (isError) {
     return null;
   }
@@ -162,30 +194,65 @@ function EventDisplay({
   return (
     <EventDisplayContainer>
       <div>
-        <StyledEventSelectorControlBar>
-          <CompactSelect
-            size="sm"
-            disabled={false}
-            options={eventIds.map(id => ({
-              value: id,
-              label: id,
-              details: <DateTime date={data?.data.find(d => d.id === id)?.timestamp} />,
-            }))}
-            value={selectedEventId}
-            onChange={({value}) => setSelectedEventId(value)}
-            triggerLabel={
-              <ButtonLabelWrapper>
-                <TextOverflow>
-                  {eventSelectLabel}:{' '}
-                  <SelectionTextWrapper>
-                    {getShortEventId(selectedEventId)}
-                  </SelectionTextWrapper>
-                </TextOverflow>
-              </ButtonLabelWrapper>
-            }
-          />
-          <GroupEventActions event={eventData} group={group} projectSlug={project.slug} />
-        </StyledEventSelectorControlBar>
+        <StyledControlBar>
+          <StyledEventControls>
+            <CompactSelect
+              size="sm"
+              disabled={false}
+              options={eventIds.map(id => ({
+                value: id,
+                label: id,
+                details: <DateTime date={data?.data.find(d => d.id === id)?.timestamp} />,
+              }))}
+              value={selectedEventId}
+              onChange={({value}) => setSelectedEventId(value)}
+              triggerLabel={
+                <ButtonLabelWrapper>
+                  <TextOverflow>
+                    {eventSelectLabel}:{' '}
+                    <SelectionTextWrapper>
+                      {getShortEventId(selectedEventId)}
+                    </SelectionTextWrapper>
+                  </TextOverflow>
+                </ButtonLabelWrapper>
+              }
+            />
+            <LinkButton
+              title={t('Full Event Details')}
+              size={BUTTON_SIZE}
+              to={eventDetailsRoute({
+                eventSlug: generateEventSlug({project: project.slug, id: eventData.id}),
+                orgSlug: organization.slug,
+              })}
+              aria-label={t('Full Event Details')}
+              icon={<IconOpen />}
+            />
+          </StyledEventControls>
+          <div>
+            <NavButtons>
+              <NavButton
+                title={t('Previous Event')}
+                disabled={!hasPrev}
+                icon={<IconChevron direction="left" size={BUTTON_ICON_SIZE} />}
+                onPaginate={() => {
+                  if (hasPrev) {
+                    setSelectedEventId(eventIds[eventIdIndex - 1]);
+                  }
+                }}
+              />
+              <NavButton
+                title={t('Next Event')}
+                disabled={!hasNext}
+                icon={<IconChevron direction="right" size={BUTTON_ICON_SIZE} />}
+                onPaginate={() => {
+                  if (hasNext) {
+                    setSelectedEventId(eventIds[eventIdIndex + 1]);
+                  }
+                }}
+              />
+            </NavButtons>
+          </div>
+        </StyledControlBar>
         <ComparisonContentWrapper>
           <Link
             to={eventDetailsRoute({
@@ -241,7 +308,12 @@ const ButtonLabelWrapper = styled('span')`
   grid-template-columns: 1fr auto;
 `;
 
-const StyledEventSelectorControlBar = styled('div')`
+const StyledControlBar = styled('div')`
+  display: flex;
+  justify-content: space-between;
+`;
+
+const StyledEventControls = styled('div')`
   display: flex;
   align-items: center;
   gap: 8px;
@@ -282,3 +354,31 @@ const EmptyStateWrapper = styled('div')`
 const SelectionTextWrapper = styled('span')`
   font-weight: normal;
 `;
+
+const StyledNavButton = styled(Button)`
+  border-radius: 0;
+`;
+
+const NavButtons = styled('div')`
+  display: flex;
+
+  > * {
+    &:not(:last-child) {
+      ${StyledNavButton} {
+        border-right: none;
+      }
+    }
+
+    &:first-child {
+      ${StyledNavButton} {
+        border-radius: ${p => p.theme.borderRadius} 0 0 ${p => p.theme.borderRadius};
+      }
+    }
+
+    &:last-child {
+      ${StyledNavButton} {
+        border-radius: 0 ${p => p.theme.borderRadius} ${p => p.theme.borderRadius} 0;
+      }
+    }
+  }
+`;

+ 2 - 5
static/app/components/events/eventStatisticalDetector/eventComparison/index.tsx

@@ -5,7 +5,7 @@ import moment from 'moment';
 import {EventDisplay} from 'sentry/components/events/eventStatisticalDetector/eventComparison/eventDisplay';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Event, Group, Project} from 'sentry/types';
+import {Event, Project} from 'sentry/types';
 
 import {DataSection} from '../../styles';
 
@@ -15,11 +15,10 @@ const COMPARISON_DESCRIPTION = t(
 
 type EventComparisonProps = {
   event: Event;
-  group: Group;
   project: Project;
 };
 
-function EventComparison({event, project, group}: EventComparisonProps) {
+function EventComparison({event, project}: EventComparisonProps) {
   const now = useMemo(() => Date.now(), []);
   const retentionPeriodMs = moment().subtract(90, 'days').valueOf();
   const {aggregateRange1, aggregateRange2, dataStart, breakpoint, transaction} =
@@ -38,7 +37,6 @@ function EventComparison({event, project, group}: EventComparisonProps) {
             end={breakpoint * 1000}
             transaction={transaction}
             durationBaseline={aggregateRange1}
-            group={group}
           />
         </StyledGridItem>
         <StyledGridItem position="right">
@@ -49,7 +47,6 @@ function EventComparison({event, project, group}: EventComparisonProps) {
             end={now}
             transaction={transaction}
             durationBaseline={aggregateRange2}
-            group={group}
           />
         </StyledGridItem>
       </StyledGrid>

+ 3 - 26
static/app/views/issueDetails/groupEventCarousel.tsx

@@ -23,7 +23,7 @@ import {
 } from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Event, Group, IssueType, Organization} from 'sentry/types';
+import {Event, Group, Organization} from 'sentry/types';
 import {defined, formatBytesBase2} from 'sentry/utils';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {eventDetailsRoute, generateEventSlug} from 'sentry/utils/discover/urls';
@@ -249,8 +249,6 @@ export function GroupEventActions({event, group, projectSlug}: GroupEventActions
   const isReplayEnabled =
     organization.features.includes('session-replay') &&
     projectCanLinkToReplay(group.project);
-  const isDurationRegressionIssue =
-    group?.issueType === IssueType.PERFORMANCE_DURATION_REGRESSION;
 
   const downloadJson = () => {
     const jsonUrl = `/api/0/projects/${organization.slug}/${projectSlug}/events/${event.id}/json/`;
@@ -274,18 +272,6 @@ export function GroupEventActions({event, group, projectSlug}: GroupEventActions
       }),
   });
 
-  const {onClick: copyEventDetailLink} = useCopyToClipboard({
-    successMessage: t('Event URL copied to clipboard'),
-    text:
-      window.location.origin +
-      normalizeUrl(
-        eventDetailsRoute({
-          eventSlug: generateEventSlug({project: projectSlug, id: event.id}),
-          orgSlug: organization.slug,
-        })
-      ),
-  });
-
   const {onClick: copyEventId} = useCopyToClipboard({
     successMessage: t('Event ID copied to clipboard'),
     text: event.id,
@@ -311,7 +297,7 @@ export function GroupEventActions({event, group, projectSlug}: GroupEventActions
             key: 'copy-event-url',
             label: t('Copy Event Link'),
             hidden: xlargeViewport,
-            onAction: isDurationRegressionIssue ? copyEventDetailLink : copyLink,
+            onAction: copyLink,
           },
           {
             key: 'json',
@@ -353,7 +339,7 @@ export function GroupEventActions({event, group, projectSlug}: GroupEventActions
           },
         ]}
       />
-      {xlargeViewport && !isDurationRegressionIssue && (
+      {xlargeViewport && (
         <Button
           title={t('Copy link to this issue event')}
           size={BUTTON_SIZE}
@@ -362,15 +348,6 @@ export function GroupEventActions({event, group, projectSlug}: GroupEventActions
           icon={<IconLink />}
         />
       )}
-      {xlargeViewport && isDurationRegressionIssue && (
-        <Button
-          title={t('Copy link to this event')}
-          size={BUTTON_SIZE}
-          onClick={copyEventDetailLink}
-          aria-label={t('Copy Link')}
-          icon={<IconLink />}
-        />
-      )}
       {xlargeViewport && (
         <Button
           title={t('View JSON')}

+ 1 - 1
static/app/views/issueDetails/groupEventDetails/groupEventDetailsContent.tsx

@@ -207,7 +207,7 @@ function PerformanceDurationRegressionIssueDetailsContent({
           <AggregateSpanDiff event={event} projectId={project.id} />
         </ErrorBoundary>
         <ErrorBoundary mini>
-          <EventComparison event={event} group={group} project={project} />
+          <EventComparison event={event} project={project} />
         </ErrorBoundary>
       </Fragment>
     </Feature>