Browse Source

feat(issues): Add better group tombstone type, remove props (#49713)

Tombstones do have a "type" property ([see
api](https://sentry-test.sentry.io/api/0/projects/sentry-test/app-frontend/tombstones/))
the type was incorrect.

This fixes some type casting by adding a more accurate `GroupTombstone`
type and adds a helper type since they only come from one place.

fixes a bug where tombstones tried to open stacktrace previews
Scott Cooper 1 year ago
parent
commit
4e4c62ca09

+ 29 - 0
static/app/components/eventOrGroupHeader.spec.tsx

@@ -236,4 +236,33 @@ describe('EventOrGroupHeader', function () {
       );
     });
   });
+
+  it('renders group tombstone without link to group', function () {
+    const {container} = render(
+      <EventOrGroupHeader
+        organization={organization}
+        data={{
+          id: '123',
+          level: 'error',
+          message: 'numTabItems is not defined ReferenceError something',
+          culprit:
+            'useOverflowTabs(webpack-internal:///./app/components/tabs/tabList.tsx)',
+          type: EventOrGroupType.ERROR,
+          metadata: {
+            value: 'numTabItems is not defined',
+            type: 'ReferenceError',
+            filename: 'webpack-internal:///./app/components/tabs/tabList.tsx',
+            function: 'useOverflowTabs',
+            display_title_with_tree_label: false,
+          },
+          actor: TestStubs.User(),
+          isTombstone: true,
+        }}
+        {...router}
+      />
+    );
+
+    expect(container).toSnapshot();
+    expect(screen.queryByRole('link')).not.toBeInTheDocument();
+  });
 });

+ 37 - 42
static/app/components/eventOrGroupHeader.tsx

@@ -10,9 +10,9 @@ import {Tooltip} from 'sentry/components/tooltip';
 import {IconMute, IconStar} from 'sentry/icons';
 import {tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import {Group, GroupTombstone, Level, Organization} from 'sentry/types';
+import {Group, GroupTombstoneHelper, Level, Organization} from 'sentry/types';
 import {Event} from 'sentry/types/event';
-import {getLocation, getMessage} from 'sentry/utils/events';
+import {getLocation, getMessage, isTombstone} from 'sentry/utils/events';
 import {useLocation} from 'sentry/utils/useLocation';
 import withOrganization from 'sentry/utils/withOrganization';
 import {TagAndMessageWrapper} from 'sentry/views/issueDetails/unhandledTag';
@@ -21,22 +21,20 @@ import EventTitleError from './eventTitleError';
 
 type Size = 'small' | 'normal';
 
-type Props = {
-  data: Event | Group | GroupTombstone;
+interface EventOrGroupHeaderProps {
+  data: Event | Group | GroupTombstoneHelper;
   organization: Organization;
-  className?: string;
   /* is issue breakdown? */
   grouping?: boolean;
   hideIcons?: boolean;
   hideLevel?: boolean;
-  includeLink?: boolean;
   index?: number;
   /** Group link clicked */
   onClick?: () => void;
   query?: string;
   size?: Size;
   source?: string;
-};
+}
 
 /**
  * Displays an event or group/issue title (i.e. in Stream)
@@ -47,14 +45,12 @@ function EventOrGroupHeader({
   organization,
   query,
   onClick,
-  className,
   hideIcons,
   hideLevel,
-  includeLink = true,
   size = 'normal',
   grouping = false,
   source,
-}: Props) {
+}: EventOrGroupHeaderProps) {
   const location = useLocation();
 
   function getTitleChildren() {
@@ -89,7 +85,6 @@ function EventOrGroupHeader({
             // hasSeen is undefined for GroupTombstone
             hasSeen={hasSeen === undefined ? true : hasSeen}
             withStackTracePreview
-            hasGuideAnchor={index === 0}
             grouping={grouping}
           />
         </ErrorBoundary>
@@ -98,8 +93,6 @@ function EventOrGroupHeader({
   }
 
   function getTitle() {
-    const orgId = organization.slug;
-
     const {id, status} = data as Group;
     const {eventID, groupID} = data as Event;
 
@@ -108,45 +101,47 @@ function EventOrGroupHeader({
       style: status === 'resolved' ? {textDecoration: 'line-through'} : undefined,
     };
 
-    if (includeLink) {
+    if (isTombstone(data)) {
       return (
-        <TitleWithLink
-          {...commonEleProps}
-          to={{
-            pathname: `/organizations/${orgId}/issues/${eventID ? groupID : id}/${
-              eventID ? `events/${eventID}/` : ''
-            }`,
-            query: {
-              referrer: source || 'event-or-group-header',
-              stream_index: index,
-              query,
-              // This adds sort to the query if one was selected from the
-              // issues list page
-              ...(location.query.sort !== undefined ? {sort: location.query.sort} : {}),
-              // This appends _allp to the URL parameters if they have no
-              // project selected ("all" projects included in results). This is
-              // so that when we enter the issue details page and lock them to
-              // a project, we can properly take them back to the issue list
-              // page with no project selected (and not the locked project
-              // selected)
-              ...(location.query.project !== undefined ? {} : {_allp: 1}),
-            },
-          }}
-          onClick={onClick}
-        >
-          {getTitleChildren()}
-        </TitleWithLink>
+        <TitleWithoutLink {...commonEleProps}>{getTitleChildren()}</TitleWithoutLink>
       );
     }
 
-    return <TitleWithoutLink {...commonEleProps}>{getTitleChildren()}</TitleWithoutLink>;
+    return (
+      <TitleWithLink
+        {...commonEleProps}
+        to={{
+          pathname: `/organizations/${organization.slug}/issues/${
+            eventID ? groupID : id
+          }/${eventID ? `events/${eventID}/` : ''}`,
+          query: {
+            referrer: source || 'event-or-group-header',
+            stream_index: index,
+            query,
+            // This adds sort to the query if one was selected from the
+            // issues list page
+            ...(location.query.sort !== undefined ? {sort: location.query.sort} : {}),
+            // This appends _allp to the URL parameters if they have no
+            // project selected ("all" projects included in results). This is
+            // so that when we enter the issue details page and lock them to
+            // a project, we can properly take them back to the issue list
+            // page with no project selected (and not the locked project
+            // selected)
+            ...(location.query.project !== undefined ? {} : {_allp: 1}),
+          },
+        }}
+        onClick={onClick}
+      >
+        {getTitleChildren()}
+      </TitleWithLink>
+    );
   }
 
   const eventLocation = getLocation(data);
   const message = getMessage(data);
 
   return (
-    <div className={className} data-test-id="event-issue-header">
+    <div data-test-id="event-issue-header">
       <Title>{getTitle()}</Title>
       {eventLocation && <Location size={size}>{eventLocation}</Location>}
       {message && (

+ 28 - 0
static/app/components/eventOrGroupTitle.spec.tsx

@@ -106,6 +106,34 @@ describe('EventOrGroupTitle', function () {
     expect(screen.queryByTestId('stacktrace-preview')).not.toBeInTheDocument();
   });
 
+  it('does not render stacktrace preview when data is a tombstone', () => {
+    render(
+      <EventOrGroupTitle
+        data={{
+          id: '123',
+          level: 'error',
+          message: 'numTabItems is not defined ReferenceError something',
+          culprit:
+            'useOverflowTabs(webpack-internal:///./app/components/tabs/tabList.tsx)',
+          type: EventOrGroupType.ERROR,
+          metadata: {
+            value: 'numTabItems is not defined',
+            type: 'ReferenceError',
+            filename: 'webpack-internal:///./app/components/tabs/tabList.tsx',
+            function: 'useOverflowTabs',
+            display_title_with_tree_label: false,
+          },
+          actor: TestStubs.User(),
+          isTombstone: true,
+        }}
+        withStackTracePreview
+      />
+    );
+
+    expect(screen.queryByTestId('stacktrace-preview')).not.toBeInTheDocument();
+    expect(screen.getByText('ReferenceError')).toBeInTheDocument();
+  });
+
   describe('performance issue list', () => {
     const perfData = {
       title: 'Hello',

+ 11 - 16
static/app/components/eventOrGroupTitle.tsx

@@ -1,23 +1,22 @@
 import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
-import {BaseGroup, GroupTombstone, Organization} from 'sentry/types';
+import {BaseGroup, GroupTombstoneHelper, Organization} from 'sentry/types';
 import {Event} from 'sentry/types/event';
-import {getTitle} from 'sentry/utils/events';
+import {getTitle, isTombstone} from 'sentry/utils/events';
 import withOrganization from 'sentry/utils/withOrganization';
 
 import EventTitleTreeLabel from './eventTitleTreeLabel';
 import GroupPreviewTooltip from './groupPreviewTooltip';
 
-type Props = {
-  data: Event | BaseGroup | GroupTombstone;
+interface EventOrGroupTitleProps {
+  data: Event | BaseGroup | GroupTombstoneHelper;
   organization: Organization;
   className?: string;
   /* is issue breakdown? */
   grouping?: boolean;
-  hasGuideAnchor?: boolean;
   withStackTracePreview?: boolean;
-};
+}
 
 function EventOrGroupTitle({
   organization,
@@ -25,14 +24,10 @@ function EventOrGroupTitle({
   withStackTracePreview,
   grouping = false,
   className,
-}: Props) {
-  const event = data as Event;
-  const groupingCurrentLevel = (data as BaseGroup).metadata?.current_level;
-  const groupingIssueCategory = (data as BaseGroup)?.issueCategory;
-
-  const {id, eventID, groupID, projectID} = event;
+}: EventOrGroupTitleProps) {
+  const {id, eventID, groupID, projectID} = data as Event;
 
-  const {title, subtitle, treeLabel} = getTitle(event, organization?.features, grouping);
+  const {title, subtitle, treeLabel} = getTitle(data, organization?.features, grouping);
   const titleLabel = treeLabel ? (
     <EventTitleTreeLabel treeLabel={treeLabel} />
   ) : (
@@ -41,11 +36,11 @@ function EventOrGroupTitle({
 
   return (
     <Wrapper className={className}>
-      {withStackTracePreview ? (
+      {!isTombstone(data) && withStackTracePreview ? (
         <GroupPreviewTooltip
           groupId={groupID ? groupID : id}
-          issueCategory={groupingIssueCategory}
-          groupingCurrentLevel={groupingCurrentLevel}
+          issueCategory={data.issueCategory}
+          groupingCurrentLevel={data.metadata?.current_level}
           eventId={eventID}
           projectId={projectID}
         >

+ 1 - 1
static/app/components/groupPreviewTooltip/index.tsx

@@ -10,11 +10,11 @@ import {StackTracePreview} from './stackTracePreview';
 type GroupPreviewTooltipProps = {
   children: ReactChild;
   groupId: string;
-  issueCategory: IssueCategory;
   // we need eventId only when hovering over Event, not Group
   // (different API call is made to get the stack trace then)
   eventId?: string;
   groupingCurrentLevel?: number;
+  issueCategory?: IssueCategory;
   projectId?: string;
 };
 

+ 0 - 1
static/app/components/stream/group.tsx

@@ -433,7 +433,6 @@ function BaseGroupRow({
         <EventOrGroupHeader
           index={index}
           organization={organization}
-          includeLink
           data={group}
           query={query}
           size="normal"

+ 7 - 3
static/app/types/group.tsx

@@ -587,14 +587,18 @@ export interface GroupCollapseRelease
   extends Omit<Group, keyof GroupRelease>,
     Partial<GroupRelease> {}
 
-export type GroupTombstone = {
+export interface GroupTombstone {
   actor: AvatarUser;
   culprit: string;
   id: string;
   level: Level;
   metadata: EventMetadata;
-  title: string;
-};
+  type: EventOrGroupType;
+  title?: string;
+}
+export interface GroupTombstoneHelper extends GroupTombstone {
+  isTombstone: true;
+}
 
 export type ProcessingIssueItem = {
   checksum: string;

+ 13 - 8
static/app/utils/events.tsx

@@ -10,7 +10,7 @@ import {
   Group,
   GroupActivityAssigned,
   GroupActivityType,
-  GroupTombstone,
+  GroupTombstoneHelper,
   IssueCategory,
   IssueType,
   TreeLabelPart,
@@ -22,15 +22,17 @@ import {getDaysSinceDatePrecise} from 'sentry/utils/getDaysSinceDate';
 import {isMobilePlatform, isNativePlatform} from 'sentry/utils/platform';
 import {getReplayIdFromEvent} from 'sentry/utils/replays/getReplayIdFromEvent';
 
-function isTombstone(maybe: BaseGroup | Event | GroupTombstone): maybe is GroupTombstone {
-  return !maybe.hasOwnProperty('type');
+export function isTombstone(
+  maybe: BaseGroup | Event | GroupTombstoneHelper
+): maybe is GroupTombstoneHelper {
+  return 'isTombstone' in maybe && maybe.isTombstone;
 }
 
 /**
  * Extract the display message from an event.
  */
 export function getMessage(
-  event: Event | BaseGroup | GroupTombstone
+  event: Event | BaseGroup | GroupTombstoneHelper
 ): string | undefined {
   if (isTombstone(event)) {
     return event.culprit || '';
@@ -58,7 +60,7 @@ export function getMessage(
 /**
  * Get the location from an event.
  */
-export function getLocation(event: Event | BaseGroup | GroupTombstone) {
+export function getLocation(event: Event | BaseGroup | GroupTombstoneHelper) {
   if (isTombstone(event)) {
     return undefined;
   }
@@ -115,7 +117,7 @@ function computeTitleWithTreeLabel(metadata: EventMetadata) {
 }
 
 export function getTitle(
-  event: Event | BaseGroup,
+  event: Event | BaseGroup | GroupTombstoneHelper,
   features: string[] = [],
   grouping = false
 ) {
@@ -133,6 +135,7 @@ export function getTitle(
       }
 
       const displayTitleWithTreeLabel =
+        !isTombstone(event) &&
         features.includes('grouping-title-ui') &&
         (grouping ||
           isNativePlatform(event.platform) ||
@@ -175,14 +178,16 @@ export function getTitle(
         treeLabel: undefined,
       };
     case EventOrGroupType.TRANSACTION:
-      const isPerfIssue = event.issueCategory === IssueCategory.PERFORMANCE;
+      const isPerfIssue =
+        !isTombstone(event) && event.issueCategory === IssueCategory.PERFORMANCE;
       return {
         title: isPerfIssue ? metadata.title : customTitle ?? title,
         subtitle: isPerfIssue ? culprit : '',
         treeLabel: undefined,
       };
     case EventOrGroupType.GENERIC:
-      const isProfilingIssue = event.issueCategory === IssueCategory.PROFILE;
+      const isProfilingIssue =
+        !isTombstone(event) && event.issueCategory === IssueCategory.PROFILE;
       return {
         title: isProfilingIssue ? metadata.title : customTitle ?? title,
         subtitle: isProfilingIssue ? culprit : '',

+ 1 - 6
static/app/views/issueDetails/groupSimilarIssues/similarStackTrace/item.tsx

@@ -122,12 +122,7 @@ class Item extends Component<Props, State> {
             onChange={this.handleCheckClick}
           />
           <EventDetails>
-            <EventOrGroupHeader
-              data={issue}
-              includeLink
-              size="normal"
-              source="similar-issues"
-            />
+            <EventOrGroupHeader data={issue} size="normal" source="similar-issues" />
             <EventOrGroupExtraDetails data={{...issue, lastSeen: ''}} showAssignee />
           </EventDetails>
 

+ 1 - 1
static/app/views/issueDetails/header.tsx

@@ -334,7 +334,7 @@ function GroupHeader({
           <TitleWrapper>
             <TitleHeading>
               <h3>
-                <StyledEventOrGroupTitle hasGuideAnchor data={group} />
+                <StyledEventOrGroupTitle data={group} />
               </h3>
               {!hasEscalatingIssuesUi && group.inbox && (
                 <InboxReason inbox={group.inbox} fontSize="md" />

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