Browse Source

ref(replays): update feedback breadcrumb (#67382)

- The new feedback breadcrumb is pink & has a megaphone icon in both the
breadcrumbs & timeline.
- The errors count in the top right doesn't include feedback anymore
- Errors tab doesn't include feedback
- Feedback breadcrumb links to the feedback page rather than issues

## New breadcrumb color & icon



<img width="310" alt="SCR-20240320-myzy"
src="https://github.com/getsentry/sentry/assets/56095982/248d24a9-d271-4395-b8df-177312b78173">

<img width="542" alt="SCR-20240320-myze"
src="https://github.com/getsentry/sentry/assets/56095982/2051acf1-e1fa-4eed-8df9-b8d4d875162b">

## No errors in errors tab or the error count in top right:



https://github.com/getsentry/sentry/assets/56095982/46888899-19ae-4f01-b89e-3e1bf7b53708




## Links to the feedback page rather than issues:


https://github.com/getsentry/sentry/assets/56095982/b767c99b-23d0-4097-a92a-934ca4f3af7d


Closes https://github.com/getsentry/team-replay/issues/408
Michelle Zhang 11 months ago
parent
commit
c04111562a

+ 9 - 5
static/app/components/replays/breadcrumbs/breadcrumbItem.tsx

@@ -17,7 +17,7 @@ import {getShortEventId} from 'sentry/utils/events';
 import type {Extraction} from 'sentry/utils/replays/extractDomNodes';
 import getFrameDetails from 'sentry/utils/replays/getFrameDetails';
 import type {ErrorFrame, ReplayFrame} from 'sentry/utils/replays/types';
-import {isErrorFrame} from 'sentry/utils/replays/types';
+import {isErrorFrame, isFeedbackFrame} from 'sentry/utils/replays/types';
 import useOrganization from 'sentry/utils/useOrganization';
 import useProjectFromSlug from 'sentry/utils/useProjectFromSlug';
 import IconWrapper from 'sentry/views/replays/detail/iconWrapper';
@@ -143,7 +143,9 @@ function BreadcrumbItem({
           />
         ))}
 
-        {isErrorFrame(frame) ? <CrumbErrorIssue frame={frame} /> : null}
+        {isErrorFrame(frame) || isFeedbackFrame(frame) ? (
+          <CrumbErrorIssue frame={frame as ErrorFrame} />
+        ) : null}
       </CrumbDetails>
     </CrumbItem>
   );
@@ -187,12 +189,14 @@ function CrumbErrorIssue({frame}: {frame: ErrorFrame}) {
     );
   }
 
+  const url = isFeedbackFrame(frame as ReplayFrame)
+    ? `/organizations/${organization.slug}/feedback/?feedbackSlug=${frame.data.projectSlug}%3A${frame.data.groupId}/`
+    : `/organizations/${organization.slug}/issues/${frame.data.groupId}/`;
+
   return (
     <CrumbIssueWrapper>
       {projectBadge}
-      <Link to={`/organizations/${organization.slug}/issues/${frame.data.groupId}/`}>
-        {frame.data.groupShortId}
-      </Link>
+      <Link to={url}>{frame.data.groupShortId}</Link>
     </CrumbIssueWrapper>
   );
 }

+ 4 - 1
static/app/components/replays/utils.spec.tsx

@@ -88,7 +88,10 @@ describe('countColumns', () => {
 describe('getFramesByColumn', () => {
   const durationMs = 25710; // milliseconds
 
-  const [CRUMB_1, CRUMB_2, CRUMB_3, CRUMB_4, CRUMB_5] = hydrateErrors(
+  const {
+    errorFrames: [CRUMB_1, CRUMB_2, CRUMB_3, CRUMB_4, CRUMB_5],
+    feedbackFrames: [],
+  } = hydrateErrors(
     ReplayRecordFixture({
       started_at: new Date('2022-04-14T14:19:47.326000Z'),
     }),

+ 13 - 0
static/app/utils/replays/getFrameDetails.tsx

@@ -12,6 +12,7 @@ import {
   IconInput,
   IconKeyDown,
   IconLocation,
+  IconMegaphone,
   IconSort,
   IconTerminal,
   IconUser,
@@ -22,6 +23,7 @@ import {TabKey} from 'sentry/utils/replays/hooks/useActiveReplayTab';
 import type {
   BreadcrumbFrame,
   ErrorFrame,
+  FeedbackFrame,
   LargestContentfulPaintFrame,
   MultiClickFrame,
   MutationFrame,
@@ -61,6 +63,13 @@ const MAPPER_FOR_FRAME: Record<string, (frame) => Details> = {
     title: 'Navigation',
     icon: <IconLocation size="xs" />,
   }),
+  feedback: (frame: FeedbackFrame) => ({
+    color: 'pink300',
+    description: frame.data.projectSlug,
+    tabKey: TabKey.BREADCRUMBS,
+    title: defaultTitle(frame),
+    icon: <IconMegaphone size="xs" />,
+  }),
   issue: (frame: ErrorFrame) => ({
     color: 'red300',
     description: frame.message,
@@ -345,6 +354,10 @@ export default function getFrameDetails(frame: ReplayFrame): Details {
 }
 
 function defaultTitle(frame: ReplayFrame) {
+  // Override title for User Feedback frames
+  if ('message' in frame && frame.message === 'User Feedback') {
+    return t('User Feedback');
+  }
   if ('category' in frame) {
     const [type, action] = frame.category.split('.');
     return `${type} ${action || ''}`.trim();

+ 54 - 48
static/app/utils/replays/hydrateErrors.spec.tsx

@@ -15,62 +15,68 @@ describe('hydrateErrors', () => {
       RawReplayErrorFixture({timestamp: new Date('2023/12/25')}),
     ];
 
-    expect(hydrateErrors(replayRecord, errors)).toStrictEqual([
-      {
-        category: 'issue',
-        data: {
-          eventId: 'e123',
-          groupId: 3740335939,
-          groupShortId: 'JS-374',
-          label: '',
-          labels: [],
-          projectSlug: 'javascript',
+    expect(hydrateErrors(replayRecord, errors)).toStrictEqual({
+      errorFrames: [
+        {
+          category: 'issue',
+          data: {
+            eventId: 'e123',
+            groupId: 3740335939,
+            groupShortId: 'JS-374',
+            label: '',
+            labels: [],
+            projectSlug: 'javascript',
+          },
+          message: 'A Redirect with :orgId param on customer domain',
+          offsetMs: 0,
+          timestamp: new Date('2023/12/23'),
+          timestampMs: 1703307600000,
+          type: 'error',
         },
-        message: 'A Redirect with :orgId param on customer domain',
-        offsetMs: 0,
-        timestamp: new Date('2023/12/23'),
-        timestampMs: 1703307600000,
-        type: 'error',
-      },
-      {
-        category: 'issue',
-        data: {
-          eventId: 'e123',
-          groupId: 3740335939,
-          groupShortId: 'JS-374',
-          label: '',
-          labels: [],
-          projectSlug: 'javascript',
+        {
+          category: 'issue',
+          data: {
+            eventId: 'e123',
+            groupId: 3740335939,
+            groupShortId: 'JS-374',
+            label: '',
+            labels: [],
+            projectSlug: 'javascript',
+          },
+          message: 'A Redirect with :orgId param on customer domain',
+          offsetMs: ONE_DAY_MS,
+          timestamp: new Date('2023/12/24'),
+          timestampMs: 1703307600000 + ONE_DAY_MS,
+          type: 'error',
         },
-        message: 'A Redirect with :orgId param on customer domain',
-        offsetMs: ONE_DAY_MS,
-        timestamp: new Date('2023/12/24'),
-        timestampMs: 1703307600000 + ONE_DAY_MS,
-        type: 'error',
-      },
-      {
-        category: 'issue',
-        data: {
-          eventId: 'e123',
-          groupId: 3740335939,
-          groupShortId: 'JS-374',
-          label: '',
-          labels: [],
-          projectSlug: 'javascript',
+        {
+          category: 'issue',
+          data: {
+            eventId: 'e123',
+            groupId: 3740335939,
+            groupShortId: 'JS-374',
+            label: '',
+            labels: [],
+            projectSlug: 'javascript',
+          },
+          message: 'A Redirect with :orgId param on customer domain',
+          offsetMs: ONE_DAY_MS * 2,
+          timestamp: new Date('2023/12/25'),
+          timestampMs: 1703307600000 + ONE_DAY_MS * 2,
+          type: 'error',
         },
-        message: 'A Redirect with :orgId param on customer domain',
-        offsetMs: ONE_DAY_MS * 2,
-        timestamp: new Date('2023/12/25'),
-        timestampMs: 1703307600000 + ONE_DAY_MS * 2,
-        type: 'error',
-      },
-    ]);
+      ],
+      feedbackFrames: [],
+    });
   });
 
   it('should drop errors that cannot be parsed', () => {
     const errors = [{foo: 'bar'}];
 
     // @ts-expect-error: Explicitly test invalid input
-    expect(hydrateErrors(replayRecord, errors)).toStrictEqual([]);
+    expect(hydrateErrors(replayRecord, errors)).toStrictEqual({
+      errorFrames: [],
+      feedbackFrames: [],
+    });
   });
 });

+ 56 - 26
static/app/utils/replays/hydrateErrors.tsx

@@ -1,47 +1,77 @@
 import * as Sentry from '@sentry/react';
 import invariant from 'invariant';
 
+import {defined} from 'sentry/utils';
 import isValidDate from 'sentry/utils/date/isValidDate';
-import type {ErrorFrame, RawReplayError} from 'sentry/utils/replays/types';
-import {isErrorFrame} from 'sentry/utils/replays/types';
+import type {
+  BreadcrumbFrame,
+  ErrorFrame,
+  RawReplayError,
+} from 'sentry/utils/replays/types';
 import toArray from 'sentry/utils/toArray';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 
 export default function hydrateErrors(
   replayRecord: ReplayRecord,
   errors: RawReplayError[]
-): ErrorFrame[] {
+): {errorFrames: ErrorFrame[]; feedbackFrames: BreadcrumbFrame[]} {
   const startTimestampMs = replayRecord.started_at.getTime();
 
-  return errors
-    .map(error => {
-      try {
-        const time = new Date(error.timestamp);
-        invariant(isValidDate(time), 'errorFrame.timestamp is invalid');
+  const errorFrames: ErrorFrame[] = [];
+  const feedbackFrames: BreadcrumbFrame[] = [];
 
-        return {
-          category: 'issue' as const,
+  errors.forEach((e: RawReplayError) => {
+    try {
+      // Feedback frame
+      if (e.title === 'User Feedback') {
+        const time = new Date(e.timestamp);
+        invariant(isValidDate(time), 'feedbackFrame.timestamp is invalid');
+
+        feedbackFrames.push({
+          category: 'feedback',
           data: {
-            eventId: error.id,
-            groupId: error['issue.id'],
-            groupShortId: error.issue,
+            eventId: e.id,
+            groupId: e['issue.id'],
+            groupShortId: e.issue,
             label:
-              (Array.isArray(error['error.type'])
-                ? error['error.type'][0]
-                : error['error.type']) ?? '',
-            labels: toArray(error['error.type']).filter(Boolean),
-            projectSlug: error['project.name'],
+              (Array.isArray(e['error.type']) ? e['error.type'][0] : e['error.type']) ??
+              '',
+            labels: toArray(e['error.type']).filter(Boolean),
+            projectSlug: e['project.name'],
           },
-          message: error.title,
+          message: e.title,
           offsetMs: Math.abs(time.getTime() - startTimestampMs),
           timestamp: time,
           timestampMs: time.getTime(),
-          type: 'error', // For compatibility reasons. See BreadcrumbType
-        };
-      } catch (err) {
-        Sentry.captureException(err);
-        return undefined;
+          type: 'user', // For compatibility reasons. See BreadcrumbType
+        });
+        return;
       }
-    })
-    .filter(isErrorFrame);
+      // Error frame
+      const time = new Date(e.timestamp);
+      invariant(isValidDate(time), 'errorFrame.timestamp is invalid');
+
+      errorFrames.push({
+        category: 'issue' as const,
+        data: {
+          eventId: e.id,
+          groupId: e['issue.id'],
+          groupShortId: e.issue,
+          label:
+            (Array.isArray(e['error.type']) ? e['error.type'][0] : e['error.type']) ?? '',
+          labels: toArray(e['error.type']).filter(defined),
+          projectSlug: e['project.name'],
+        },
+        message: e.title,
+        offsetMs: Math.abs(time.getTime() - startTimestampMs),
+        timestamp: time,
+        timestampMs: time.getTime(),
+        type: 'error', // For compatibility reasons. See BreadcrumbType
+      });
+    } catch (error) {
+      Sentry.captureException(error);
+    }
+  });
+
+  return {errorFrames, feedbackFrames};
 }

+ 11 - 5
static/app/utils/replays/replayReader.tsx

@@ -181,7 +181,8 @@ export default class ReplayReader {
     this._replayRecord = replayRecord;
     // Errors don't need to be sorted here, they will be merged with breadcrumbs
     // and spans in the getter and then sorted together.
-    this._errors = hydrateErrors(replayRecord, errors).sort(sortFrames);
+    const {errorFrames, feedbackFrames} = hydrateErrors(replayRecord, errors);
+    this._errors = errorFrames.sort(sortFrames);
     // RRWeb Events are not sorted here, they are fetched in sorted order.
     this._sortedRRWebEvents = rrwebFrames;
     this._videoEvents = videoFrames;
@@ -192,7 +193,9 @@ export default class ReplayReader {
       replayRecord,
       breadcrumbFrames,
       this._sortedRRWebEvents
-    ).sort(sortFrames);
+    )
+      .concat(feedbackFrames)
+      .sort(sortFrames);
     // Spans must be sorted so components like the Timeline and Network Chart
     // can have an easier time to render.
     this._sortedSpanFrames = hydrateSpans(replayRecord, spanFrames).sort(sortFrames);
@@ -384,9 +387,12 @@ export default class ReplayReader {
       [
         ...this.getPerfFrames(),
         ...this._sortedBreadcrumbFrames.filter(frame =>
-          ['replay.hydrate-error', 'replay.init', 'replay.mutations'].includes(
-            frame.category
-          )
+          [
+            'replay.hydrate-error',
+            'replay.init',
+            'replay.mutations',
+            'feedback',
+          ].includes(frame.category)
         ),
         ...this._errors,
       ].sort(sortFrames),

+ 25 - 1
static/app/utils/replays/types.tsx

@@ -75,6 +75,12 @@ export function isBreadcrumbFrame(
   return Boolean(frame && 'category' in frame && frame.category !== 'issue');
 }
 
+export function isFeedbackFrame(
+  frame: ReplayFrame | undefined
+): frame is BreadcrumbFrame {
+  return Boolean(frame && 'category' in frame && frame.category === 'feedback');
+}
+
 export function isSpanFrame(frame: ReplayFrame | undefined): frame is SpanFrame {
   return Boolean(frame && 'op' in frame);
 }
@@ -182,9 +188,27 @@ type HydratedSpan<Op extends string> = Overwrite<
 
 // Breadcrumbs
 export type BreadcrumbFrame = Overwrite<
-  TRawBreadcrumbFrame | ExtraBreadcrumbTypes,
+  TRawBreadcrumbFrame | ExtraBreadcrumbTypes | FeedbackFrame,
   HydratedTimestamp
 >;
+
+export type FeedbackFrame = {
+  category: 'feedback';
+  data: {
+    eventId: string;
+    groupId: number;
+    groupShortId: string;
+    label: string;
+    labels: string[];
+    projectSlug: string;
+  };
+  message: string;
+  offsetMs: number;
+  timestamp: Date;
+  timestampMs: number;
+  type: string;
+};
+
 export type BlurFrame = HydratedBreadcrumb<'ui.blur'>;
 export type ClickFrame = HydratedBreadcrumb<'ui.click'>;
 export type ConsoleFrame = HydratedBreadcrumb<'console'>;

+ 1 - 0
static/app/views/replays/detail/breadcrumbs/useBreadcrumbFilters.tsx

@@ -69,6 +69,7 @@ const OPORCATEGORY_TO_TYPE: Record<string, keyof typeof TYPE_TO_LABEL> = {
   'ui.click': 'click',
   'ui.keyDown': 'keydown',
   'ui.input': 'input',
+  feedback: 'feedback',
 };
 
 function typeToLabel(val: string): string {

+ 35 - 33
static/app/views/replays/detail/errorList/useErrorFilters.spec.tsx

@@ -18,39 +18,41 @@ jest.mock('sentry/utils/useLocation');
 
 const mockUseLocation = jest.mocked(useLocation);
 
-const [ERROR_1_JS_RANGEERROR, ERROR_2_NEXTJS_TYPEERROR, ERROR_3_JS_UNDEFINED] =
-  hydrateErrors(
-    ReplayRecordFixture({started_at: new Date('2023-06-09T12:00:00+00:00')}),
-    [
-      RawReplayErrorFixture({
-        'error.type': ['RangeError'],
-        timestamp: new Date('2023-06-09T12:00:00+00:00'),
-        id: '415ecb5c85ac43b19f1886bb41ddab96',
-        'issue.id': 11,
-        issue: 'JAVASCRIPT-RANGE',
-        title: 'Invalid time value',
-        'project.name': 'javascript',
-      }),
-      RawReplayErrorFixture({
-        'error.type': ['TypeError'],
-        timestamp: new Date('2023-06-09T12:10:00+00:00'),
-        id: 'ac43b19f1886bb41ddab96415ecb5c85',
-        'issue.id': 22,
-        issue: 'NEXTJS-TYPE',
-        title: `undefined is not an object (evaluating 'e.apply').`,
-        'project.name': 'next-js',
-      }),
-      RawReplayErrorFixture({
-        'error.type': ['TypeError'],
-        timestamp: new Date('2023-06-09T12:20:00+00:00'),
-        id: '9f1886bb41ddab96415ecb5c85ac43b1',
-        'issue.id': 22,
-        issue: 'JAVASCRIPT-UNDEF',
-        title: `Maximum update depth exceeded`,
-        'project.name': 'javascript',
-      }),
-    ]
-  );
+const {
+  errorFrames: [ERROR_1_JS_RANGEERROR, ERROR_2_NEXTJS_TYPEERROR, ERROR_3_JS_UNDEFINED],
+  feedbackFrames: [],
+} = hydrateErrors(
+  ReplayRecordFixture({started_at: new Date('2023-06-09T12:00:00+00:00')}),
+  [
+    RawReplayErrorFixture({
+      'error.type': ['RangeError'],
+      timestamp: new Date('2023-06-09T12:00:00+00:00'),
+      id: '415ecb5c85ac43b19f1886bb41ddab96',
+      'issue.id': 11,
+      issue: 'JAVASCRIPT-RANGE',
+      title: 'Invalid time value',
+      'project.name': 'javascript',
+    }),
+    RawReplayErrorFixture({
+      'error.type': ['TypeError'],
+      timestamp: new Date('2023-06-09T12:10:00+00:00'),
+      id: 'ac43b19f1886bb41ddab96415ecb5c85',
+      'issue.id': 22,
+      issue: 'NEXTJS-TYPE',
+      title: `undefined is not an object (evaluating 'e.apply').`,
+      'project.name': 'next-js',
+    }),
+    RawReplayErrorFixture({
+      'error.type': ['TypeError'],
+      timestamp: new Date('2023-06-09T12:20:00+00:00'),
+      id: '9f1886bb41ddab96415ecb5c85ac43b1',
+      'issue.id': 22,
+      issue: 'JAVASCRIPT-UNDEF',
+      title: `Maximum update depth exceeded`,
+      'project.name': 'javascript',
+    }),
+  ]
+);
 
 describe('useErrorFilters', () => {
   beforeEach(() => {

+ 35 - 33
static/app/views/replays/detail/errorList/useSortErrors.spec.tsx

@@ -23,39 +23,41 @@ jest.mock('sentry/utils/useUrlParams', () => {
   };
 });
 
-const [ERROR_1_JS_RANGEERROR, ERROR_2_NEXTJS_TYPEERROR, ERROR_3_JS_UNDEFINED] =
-  hydrateErrors(
-    ReplayRecordFixture({started_at: new Date('2023-06-09T12:00:00+00:00')}),
-    [
-      RawReplayErrorFixture({
-        'error.type': ['RangeError'],
-        timestamp: new Date('2023-06-09T12:00:00+00:00'),
-        id: '415ecb5c85ac43b19f1886bb41ddab96',
-        'issue.id': 11,
-        issue: 'JAVASCRIPT-RANGE',
-        title: 'Invalid time value',
-        'project.name': 'javascript',
-      }),
-      RawReplayErrorFixture({
-        'error.type': ['TypeError'],
-        timestamp: new Date('2023-06-09T12:10:00+00:00'),
-        id: 'ac43b19f1886bb41ddab96415ecb5c85',
-        'issue.id': 22,
-        issue: 'NEXTJS-TYPE',
-        title: `undefined is not an object (evaluating 'e.apply').`,
-        'project.name': 'next-js',
-      }),
-      RawReplayErrorFixture({
-        'error.type': ['TypeError'],
-        timestamp: new Date('2023-06-09T12:20:00+00:00'),
-        id: '9f1886bb41ddab96415ecb5c85ac43b1',
-        'issue.id': 22,
-        issue: 'JAVASCRIPT-UNDEF',
-        title: `Maximum update depth exceeded`,
-        'project.name': 'javascript',
-      }),
-    ]
-  );
+const {
+  errorFrames: [ERROR_1_JS_RANGEERROR, ERROR_2_NEXTJS_TYPEERROR, ERROR_3_JS_UNDEFINED],
+  feedbackFrames: [],
+} = hydrateErrors(
+  ReplayRecordFixture({started_at: new Date('2023-06-09T12:00:00+00:00')}),
+  [
+    RawReplayErrorFixture({
+      'error.type': ['RangeError'],
+      timestamp: new Date('2023-06-09T12:00:00+00:00'),
+      id: '415ecb5c85ac43b19f1886bb41ddab96',
+      'issue.id': 11,
+      issue: 'JAVASCRIPT-RANGE',
+      title: 'Invalid time value',
+      'project.name': 'javascript',
+    }),
+    RawReplayErrorFixture({
+      'error.type': ['TypeError'],
+      timestamp: new Date('2023-06-09T12:10:00+00:00'),
+      id: 'ac43b19f1886bb41ddab96415ecb5c85',
+      'issue.id': 22,
+      issue: 'NEXTJS-TYPE',
+      title: `undefined is not an object (evaluating 'e.apply').`,
+      'project.name': 'next-js',
+    }),
+    RawReplayErrorFixture({
+      'error.type': ['TypeError'],
+      timestamp: new Date('2023-06-09T12:20:00+00:00'),
+      id: '9f1886bb41ddab96415ecb5c85ac43b1',
+      'issue.id': 22,
+      issue: 'JAVASCRIPT-UNDEF',
+      title: `Maximum update depth exceeded`,
+      'project.name': 'javascript',
+    }),
+  ]
+);
 
 describe('useSortErrors', () => {
   const items = [ERROR_1_JS_RANGEERROR, ERROR_3_JS_UNDEFINED, ERROR_2_NEXTJS_TYPEERROR];

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