Browse Source

ref(event-title): Add eventTitleTreeLabel component option (#27198)

Priscila Oliveira 3 years ago
parent
commit
95c8e80582

+ 36 - 47
static/app/components/eventOrGroupTitle.tsx

@@ -1,4 +1,4 @@
-import * as React from 'react';
+import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import GuideAnchor from 'app/components/assistant/guideAnchor';
@@ -8,6 +8,7 @@ import {Event} from 'app/types/event';
 import {getTitle} from 'app/utils/events';
 import withOrganization from 'app/utils/withOrganization';
 
+import EventTitleTreeLabel from './eventTitleTreeLabel';
 import StacktracePreview from './stacktracePreview';
 
 type Props = Partial<DefaultProps> & {
@@ -23,55 +24,43 @@ type DefaultProps = {
   guideAnchorName: string;
 };
 
-class EventOrGroupTitle extends React.Component<Props> {
-  static defaultProps: DefaultProps = {
-    guideAnchorName: 'issue_title',
-  };
-  render() {
-    const {hasGuideAnchor, data, organization, withStackTracePreview, guideAnchorName} =
-      this.props;
-    const {title, subtitle} = getTitle(data as Event, organization);
-    const {id, eventID, groupID, projectID} = data as Event;
+function EventOrGroupTitle({
+  guideAnchorName = 'issue_title',
+  organization,
+  data,
+  withStackTracePreview,
+  hasGuideAnchor,
+  style,
+}: Props) {
+  const event = data as Event;
+  const {id, eventID, groupID, projectID} = event;
 
-    const titleWithHoverStacktrace = (
-      <StacktracePreview
-        organization={organization}
-        issueId={groupID ? groupID : id}
-        // we need eventId and projectSlug only when hovering over Event, not Group
-        // (different API call is made to get the stack trace then)
-        eventId={eventID}
-        projectSlug={eventID ? ProjectsStore.getById(projectID)?.slug : undefined}
-        disablePreview={!withStackTracePreview}
-      >
-        {title}
-      </StacktracePreview>
-    );
+  const {title, subtitle, treeLabel} = getTitle(event, organization?.features);
 
-    return subtitle ? (
-      <span style={this.props.style}>
-        <GuideAnchor
-          disabled={!hasGuideAnchor}
-          target={guideAnchorName}
-          position="bottom"
+  return (
+    <span style={style}>
+      <GuideAnchor disabled={!hasGuideAnchor} target={guideAnchorName} position="bottom">
+        <StacktracePreview
+          organization={organization}
+          issueId={groupID ? groupID : id}
+          // we need eventId and projectSlug only when hovering over Event, not Group
+          // (different API call is made to get the stack trace then)
+          eventId={eventID}
+          projectSlug={eventID ? ProjectsStore.getById(projectID)?.slug : undefined}
+          disablePreview={!withStackTracePreview}
         >
-          <span>{titleWithHoverStacktrace}</span>
-        </GuideAnchor>
-        <Spacer />
-        <Subtitle title={subtitle}>{subtitle}</Subtitle>
-        <br />
-      </span>
-    ) : (
-      <span style={this.props.style}>
-        <GuideAnchor
-          disabled={!hasGuideAnchor}
-          target={guideAnchorName}
-          position="bottom"
-        >
-          {titleWithHoverStacktrace}
-        </GuideAnchor>
-      </span>
-    );
-  }
+          {treeLabel ? <EventTitleTreeLabel treeLabel={treeLabel} /> : title}
+        </StacktracePreview>
+      </GuideAnchor>
+      {subtitle && (
+        <Fragment>
+          <Spacer />
+          <Subtitle title={subtitle}>{subtitle}</Subtitle>
+          <br />
+        </Fragment>
+      )}
+    </span>
+  );
 }
 
 export default withOrganization(EventOrGroupTitle);

+ 71 - 0
static/app/components/eventTitleTreeLabel.tsx

@@ -0,0 +1,71 @@
+import {Fragment} from 'react';
+import styled from '@emotion/styled';
+
+import overflowEllipsis from 'app/styles/overflowEllipsis';
+import space from 'app/styles/space';
+
+type Props = {
+  treeLabel: string[];
+};
+
+function EventTitleTreeLabel({treeLabel}: Props) {
+  const firstFourLabels = treeLabel.slice(0, 4);
+  const remainingLabels = treeLabel.slice(firstFourLabels.length);
+
+  return (
+    <Wrapper>
+      <FirstFourLabels>
+        {firstFourLabels.map((label, index) => {
+          if (index !== firstFourLabels.length - 1) {
+            return (
+              <Fragment key={index}>
+                <PriorityLabel>{label}</PriorityLabel>
+                <Divider>{'|'}</Divider>
+              </Fragment>
+            );
+          }
+          return <PriorityLabel key={index}>{label}</PriorityLabel>;
+        })}
+      </FirstFourLabels>
+      {!!remainingLabels.length && (
+        <RemainingLabels>
+          {remainingLabels.map((label, index) => {
+            return (
+              <Fragment key={index}>
+                <Divider>{'|'}</Divider>
+                {label}
+              </Fragment>
+            );
+          })}
+        </RemainingLabels>
+      )}
+    </Wrapper>
+  );
+}
+
+export default EventTitleTreeLabel;
+
+const Wrapper = styled('span')`
+  display: inline-grid;
+  grid-template-columns: auto 1fr;
+`;
+
+const FirstFourLabels = styled('span')`
+  display: grid;
+  grid-auto-flow: column;
+`;
+
+const PriorityLabel = styled('div')`
+  ${overflowEllipsis}
+`;
+
+const RemainingLabels = styled('div')`
+  ${overflowEllipsis}
+  min-width: 50px;
+`;
+
+const Divider = styled('span')`
+  color: ${p => p.theme.gray200};
+  display: inline-block;
+  margin: 0 ${space(1)};
+`;

+ 5 - 3
static/app/components/pagination/index.tsx

@@ -14,6 +14,7 @@ import parseLinkHeader from 'app/utils/parseLinkHeader';
 
 const defaultProps: DefaultProps = {
   size: 'small',
+  disabled: false,
   onCursor: (cursor: string, path: string, query: Query, _direction: number) => {
     browserHistory.push({
       pathname: path,
@@ -25,6 +26,7 @@ const defaultProps: DefaultProps = {
 type DefaultProps = {
   size?: 'zero' | 'xsmall' | 'small';
   onCursor?: (cursor: string, path: string, query: Query, _direction: number) => void;
+  disabled?: boolean;
 };
 
 type Props = DefaultProps & {
@@ -45,7 +47,7 @@ class Pagination extends Component<Props> {
   static defaultProps = defaultProps;
 
   render() {
-    const {className, onCursor, pageLinks, size, caption} = this.props;
+    const {className, onCursor, pageLinks, size, caption, disabled} = this.props;
     if (!pageLinks) {
       return null;
     }
@@ -54,8 +56,8 @@ class Pagination extends Component<Props> {
     const path = this.props.to || location.pathname;
     const query = location.query;
     const links = parseLinkHeader(pageLinks);
-    const previousDisabled = links.previous.results === false;
-    const nextDisabled = links.next.results === false;
+    const previousDisabled = disabled || links.previous.results === false;
+    const nextDisabled = disabled || links.next.results === false;
 
     return (
       <Wrapper className={className}>

+ 10 - 4
static/app/types/event.tsx

@@ -7,6 +7,7 @@ import {StacktraceType} from './stacktrace';
 import {
   EventAttachment,
   EventMetadata,
+  EventOrGroupType,
   ExceptionType,
   Frame,
   PlatformType,
@@ -166,6 +167,12 @@ export type EventUser = {
 
 type EventBase = {
   id: string;
+  type:
+    | EventOrGroupType.CSP
+    | EventOrGroupType.DEFAULT
+    | EventOrGroupType.EXPECTCT
+    | EventOrGroupType.EXPECTSTAPLE
+    | EventOrGroupType.HPKP;
   eventID: string;
   title: string;
   culprit: string;
@@ -212,7 +219,7 @@ export type EventTransaction = Omit<EventBase, 'entries' | 'type'> & {
   entries: (EntrySpans | EntryRequest)[];
   startTimestamp: number;
   endTimestamp: number;
-  type: 'transaction';
+  type: EventOrGroupType.TRANSACTION;
   title?: string;
   contexts?: {
     trace?: TraceContextType;
@@ -221,8 +228,7 @@ export type EventTransaction = Omit<EventBase, 'entries' | 'type'> & {
 
 export type EventError = Omit<EventBase, 'entries' | 'type'> & {
   entries: (EntryException | EntryStacktrace | EntryRequest)[];
-  type: 'error';
+  type: EventOrGroupType.ERROR;
 };
 
-// This type is incomplete
-export type Event = EventError | EventTransaction | ({type: string} & EventBase);
+export type Event = EventError | EventTransaction | EventBase;

+ 9 - 8
static/app/types/index.tsx

@@ -768,14 +768,15 @@ export const DataCategoryName = {
   [DataCategory.ATTACHMENTS]: 'Attachments',
 };
 
-export type EventOrGroupType =
-  | 'error'
-  | 'csp'
-  | 'hpkp'
-  | 'expectct'
-  | 'expectstaple'
-  | 'default'
-  | 'transaction';
+export enum EventOrGroupType {
+  ERROR = 'error',
+  CSP = 'csp',
+  HPKP = 'hpkp',
+  EXPECTCT = 'expectct',
+  EXPECTSTAPLE = 'expectstaple',
+  DEFAULT = 'default',
+  TRANSACTION = 'transaction',
+}
 
 export type InboxReasonDetails = {
   until?: string | null;

+ 83 - 60
static/app/utils/events.tsx

@@ -1,4 +1,4 @@
-import {BaseGroup, EventMetadata, GroupTombstone, Organization} from 'app/types';
+import {BaseGroup, EventMetadata, EventOrGroupType, GroupTombstone} from 'app/types';
 import {Event} from 'app/types/event';
 import {isNativePlatform} from 'app/utils/platform';
 
@@ -15,90 +15,113 @@ export function getMessage(
   if (isTombstone(event)) {
     return event.culprit || '';
   }
+
   const {metadata, type, culprit} = event;
 
   switch (type) {
-    case 'error':
+    case EventOrGroupType.ERROR:
       return metadata.value;
-    case 'csp':
+    case EventOrGroupType.CSP:
       return metadata.message;
-    case 'expectct':
-    case 'expectstaple':
-    case 'hpkp':
+    case EventOrGroupType.EXPECTCT:
+    case EventOrGroupType.EXPECTSTAPLE:
+    case EventOrGroupType.HPKP:
       return '';
     default:
       return culprit || '';
   }
 }
 
-function formatTreeLabel(treeLabel: string[]) {
-  return treeLabel.join(' | ');
-}
-
-function computeTitleWithTreeLabel(title: string | undefined, metadata: EventMetadata) {
-  const treeLabel = metadata.current_tree_label || metadata.finest_tree_label;
-  const formattedTreeLabel = treeLabel ? formatTreeLabel(treeLabel) : null;
-
-  if (!title) {
-    return formattedTreeLabel || metadata.function || '<unknown>';
+/**
+ * Get the location from an event.
+ */
+export function getLocation(event: Event | BaseGroup | GroupTombstone) {
+  if (isTombstone(event)) {
+    return undefined;
   }
 
-  if (formattedTreeLabel) {
-    title += ' | ' + formattedTreeLabel;
+  if (event.type === EventOrGroupType.ERROR && isNativePlatform(event.platform)) {
+    return event.metadata.filename || undefined;
   }
 
-  return title;
+  return undefined;
 }
 
-/**
- * Get the location from an event.
- */
-export function getLocation(event: Event | BaseGroup | GroupTombstone): string | null {
-  if (isTombstone(event)) {
-    return null;
+function computeTitleWithTreeLabel(metadata: EventMetadata) {
+  const {type, current_tree_label, finest_tree_label} = metadata;
+  const treeLabel = current_tree_label || finest_tree_label;
+  const formattedTreeLabel = treeLabel ? treeLabel.join(' | ') : undefined;
+
+  if (!type) {
+    return {
+      title: formattedTreeLabel || metadata.function || '<unknown>',
+      treeLabel,
+    };
   }
-  if (event.type === 'error' && isNativePlatform(event.platform)) {
-    return event.metadata.filename || null;
+
+  if (!formattedTreeLabel) {
+    return {title: type, treeLabel: undefined};
   }
-  return null;
-}
 
-type EventTitle = {
-  title: string;
-  subtitle: string;
-};
+  return {
+    title: `${type} | ${formattedTreeLabel}`,
+    treeLabel: [type, ...(treeLabel ?? [])],
+  };
+}
 
-export function getTitle(
-  event: Event | BaseGroup,
-  organization?: Organization
-): EventTitle {
+export function getTitle(event: Event | BaseGroup, features: string[] = []) {
   const {metadata, type, culprit} = event;
-  const result: EventTitle = {
-    title: event.title,
-    subtitle: '',
-  };
 
-  if (type === 'error') {
-    result.subtitle = culprit;
-    result.title = computeTitleWithTreeLabel(metadata.type, metadata);
-  } else if (type === 'csp') {
-    result.title = metadata.directive || '';
-    result.subtitle = metadata.uri || '';
-  } else if (type === 'expectct' || type === 'expectstaple' || type === 'hpkp') {
-    // Due to a regression some reports did not have message persisted
-    // (https://github.com/getsentry/sentry/pull/19794) so we need to fall
-    // back to the computed title for these.
-    result.title = metadata.message || result.title || '';
-    result.subtitle = metadata.origin || '';
-  } else if (type === 'default') {
-    result.title = metadata.title || '';
-  }
+  const customTitle =
+    features.includes('custom-event-title') && metadata?.title
+      ? metadata.title
+      : undefined;
 
-  if (organization?.features.includes('custom-event-title') && metadata?.title) {
-    result.title = metadata.title;
-  }
+  switch (type) {
+    case EventOrGroupType.ERROR: {
+      if (customTitle) {
+        return {
+          title: customTitle,
+          subtitle: culprit,
+          treeLabel: undefined,
+        };
+      }
 
-  return result;
+      return {
+        subtitle: culprit,
+        ...computeTitleWithTreeLabel(metadata),
+      };
+    }
+    case EventOrGroupType.CSP:
+      return {
+        title: customTitle ?? metadata.directive ?? '',
+        subtitle: metadata.uri ?? '',
+        treeLabel: undefined,
+      };
+    case EventOrGroupType.EXPECTCT:
+    case EventOrGroupType.EXPECTSTAPLE:
+    case EventOrGroupType.HPKP:
+      // Due to a regression some reports did not have message persisted
+      // (https://github.com/getsentry/sentry/pull/19794) so we need to fall
+      // back to the computed title for these.
+      return {
+        title: customTitle ?? (metadata.message || event.title),
+        subtitle: metadata.origin ?? '',
+        treeLabel: undefined,
+      };
+    case EventOrGroupType.DEFAULT:
+      return {
+        title: customTitle ?? metadata.title ?? '',
+        subtitle: '',
+        treeLabel: undefined,
+      };
+    default:
+      return {
+        title: customTitle ?? event.title,
+        subtitle: '',
+        treeLabel: undefined,
+      };
+  }
 }
 
 /**

+ 1 - 0
static/app/views/eventsV2/table/tableView.tsx

@@ -204,6 +204,7 @@ class TableView extends React.Component<TableViewProps> {
     const titleText = isEquationAlias(column.name)
       ? eventView.getEquations()[getEquationAliasIndex(column.name)]
       : column.name;
+
     const title = (
       <StyledTooltip title={titleText}>
         <Truncate value={titleText} maxLength={60} expandable={false} />

+ 1 - 1
static/app/views/eventsV2/utils.tsx

@@ -129,7 +129,7 @@ export function generateTitle({
     titles.push(String(eventViewName).trim());
   }
 
-  const eventTitle = event ? getTitle(event, organization).title : undefined;
+  const eventTitle = event ? getTitle(event, organization?.features).title : undefined;
 
   if (eventTitle) {
     titles.push(eventTitle);

+ 1 - 1
static/app/views/organizationGroupDetails/groupDetails.tsx

@@ -426,7 +426,7 @@ class GroupDetails extends React.Component<Props, State> {
       return defaultTitle;
     }
 
-    const {title} = getTitle(group, organization);
+    const {title} = getTitle(group, organization?.features);
     const message = getMessage(group);
 
     const {project} = group;

+ 22 - 20
static/app/views/organizationGroupDetails/grouping/grouping.tsx

@@ -178,12 +178,12 @@ function Grouping({api, groupId, location, organization, router}: Props) {
 
   return (
     <Wrapper>
-      <Description>
+      <Header>
         {t(
           'This issue is an aggregate of multiple events that sentry determined originate from the same root-cause. Use this page to explore more detailed groupings that exist within this issue.'
         )}
-      </Description>
-      <Content>
+      </Header>
+      <Body>
         <SliderWrapper>
           {t('Fewer issues')}
           <StyledRangeSlider
@@ -195,11 +195,8 @@ function Grouping({api, groupId, location, organization, router}: Props) {
           />
           {t('More issues')}
         </SliderWrapper>
-        <div>
-          <StyledPanelTable
-            isReloading={isGroupingLevelDetailsLoading}
-            headers={['', t('Events')]}
-          >
+        <Content isReloading={isGroupingLevelDetailsLoading}>
+          <StyledPanelTable headers={['', t('Events')]}>
             {activeGroupingLevelDetails.map(
               ({hash, title, metadata, latestEvent, eventCount}) => {
                 // XXX(markus): Ugly hack to make NewIssue show the right things.
@@ -220,6 +217,7 @@ function Grouping({api, groupId, location, organization, router}: Props) {
           </StyledPanelTable>
           <StyledPagination
             pageLinks={pagination}
+            disabled={isGroupingLevelDetailsLoading}
             caption={
               <PaginationCaption
                 caption={tct('Showing [current] of [total] [result]', {
@@ -234,8 +232,8 @@ function Grouping({api, groupId, location, organization, router}: Props) {
               />
             }
           />
-        </div>
-      </Content>
+        </Content>
+      </Body>
     </Wrapper>
   );
 }
@@ -250,26 +248,19 @@ const Wrapper = styled('div')`
   padding: ${space(3)} ${space(4)};
 `;
 
-const Description = styled('p')`
+const Header = styled('p')`
   && {
     margin-bottom: ${space(2)};
   }
 `;
 
-const Content = styled('div')`
+const Body = styled('div')`
   display: grid;
   grid-gap: ${space(3)};
 `;
 
-const StyledPanelTable = styled(PanelTable)<{isReloading: boolean}>`
+const StyledPanelTable = styled(PanelTable)`
   grid-template-columns: 1fr minmax(60px, auto);
-  ${p =>
-    p.isReloading &&
-    `
-      opacity: 0.5;
-      pointer-events: none;
-    `}
-
   > * {
     padding: ${space(1.5)} ${space(2)};
     :nth-child(-n + 2) {
@@ -291,6 +282,17 @@ const StyledPagination = styled(Pagination)`
   margin-top: 0;
 `;
 
+const Content = styled('div')<{isReloading: boolean}>`
+  ${p =>
+    p.isReloading &&
+    `
+      ${StyledPanelTable}, ${StyledPagination} {
+        opacity: 0.5;
+        pointer-events: none;
+      }
+    `}
+`;
+
 const SliderWrapper = styled('div')`
   display: grid;
   grid-gap: ${space(1.5)};