From 4b97e74efa17f162beaf0ddf222457cde007b00f Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 11:43:26 -0700 Subject: [PATCH 1/7] fix(issues/replay): if fetchError, don't render replayId in highlights --- .../highlights/highlightsDataSection.tsx | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/static/app/components/events/highlights/highlightsDataSection.tsx b/static/app/components/events/highlights/highlightsDataSection.tsx index 0926005f15475..7b3be470e3672 100644 --- a/static/app/components/events/highlights/highlightsDataSection.tsx +++ b/static/app/components/events/highlights/highlightsDataSection.tsx @@ -30,6 +30,7 @@ import {space} from 'sentry/styles/space'; import type {Event} from 'sentry/types/event'; import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; +import useReplayData from 'sentry/utils/replays/hooks/useReplayData'; import theme from 'sentry/utils/theme'; import {useDetailedProject} from 'sentry/utils/useDetailedProject'; import {useFeedbackForm} from 'sentry/utils/useFeedbackForm'; @@ -147,6 +148,39 @@ function HighlightsData({ highlightContext, location, }); + const highlightTagItems = getHighlightTagData({event, highlightTags}); + + // find the replayId from either context or tags, if it exists + const contextReplayItem = highlightContextDataItems.find( + e => e.data[0].key === 'replay_id' + ); + const contextReplayId = contextReplayItem?.value ?? ''; + + const tagReplayItem = highlightTagItems.find(e => e.originalTag.key === 'replayId'); + const tagReplayId = tagReplayItem?.value ?? ''; + + // if the id doesn't exist for either tag or context, it's rendered as '--' + const replayId = + contextReplayId.length > 2 + ? contextReplayId + : tagReplayId.length > 2 + ? tagReplayId + : undefined; + + const {fetchError: replayFetchError} = useReplayData({ + orgSlug: organization.slug, + replayId: replayId, + }); + + // if fetchError, replace the replay id so we don't link to an invalid replay + if (contextReplayItem && replayFetchError) { + contextReplayItem.value = '--'; + } + if (tagReplayItem && replayFetchError) { + tagReplayItem.value = '--'; + tagReplayItem.originalTag.value = '--'; + } + const highlightContextRows = highlightContextDataItems.reduce( (rowList, {alias, data}, i) => { const meta = getContextMeta(event, alias); @@ -165,7 +199,6 @@ function HighlightsData({ [] ); - const highlightTagItems = getHighlightTagData({event, highlightTags}); const highlightTagRows = highlightTagItems.map((content, i) => ( Date: Tue, 27 Aug 2024 11:56:40 -0700 Subject: [PATCH 2/7] :art: clean up --- .../events/highlights/highlightsDataSection.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/static/app/components/events/highlights/highlightsDataSection.tsx b/static/app/components/events/highlights/highlightsDataSection.tsx index 7b3be470e3672..aa0e59dbdede2 100644 --- a/static/app/components/events/highlights/highlightsDataSection.tsx +++ b/static/app/components/events/highlights/highlightsDataSection.tsx @@ -169,16 +169,16 @@ function HighlightsData({ const {fetchError: replayFetchError} = useReplayData({ orgSlug: organization.slug, - replayId: replayId, + replayId, }); - // if fetchError, replace the replay id so we don't link to an invalid replay + // if fetchError, replace the replayId so we don't link to an invalid replay if (contextReplayItem && replayFetchError) { - contextReplayItem.value = '--'; + contextReplayItem.value = EMPTY_HIGHLIGHT_DEFAULT; } if (tagReplayItem && replayFetchError) { - tagReplayItem.value = '--'; - tagReplayItem.originalTag.value = '--'; + tagReplayItem.value = EMPTY_HIGHLIGHT_DEFAULT; + tagReplayItem.originalTag.value = EMPTY_HIGHLIGHT_DEFAULT; } const highlightContextRows = highlightContextDataItems.reduce( From 78b17cc388c25030f4d42b8e1435f86479fcbaa7 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:18:04 -0700 Subject: [PATCH 3/7] :white_check_mark: fix test --- .../groupEventDetails.spec.tsx | 43 +++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx index a4b7324175d6a..cd308b01f66a0 100644 --- a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx +++ b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx @@ -198,6 +198,7 @@ const mockGroupApis = ( project: Project, group: Group, event: Event, + replayId: string | undefined, trace?: QuickTraceEvent ) => { MockApiClient.addMockResponse({ @@ -205,6 +206,11 @@ const mockGroupApis = ( body: group, }); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/replays/${replayId}/`, + body: [], + }); + MockApiClient.addMockResponse({ url: `/projects/${organization.slug}/${project.slug}/issues/`, method: 'PUT', @@ -358,7 +364,7 @@ describe('groupEventDetails', () => { it('redirects on switching to an invalid environment selection for event', async function () { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event); + mockGroupApis(props.organization, props.project, props.group, props.event, undefined); const {rerender} = render(); expect(browserHistory.replace).not.toHaveBeenCalled(); @@ -370,7 +376,7 @@ describe('groupEventDetails', () => { it('does not redirect when switching to a valid environment selection for event', async function () { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event); + mockGroupApis(props.organization, props.project, props.group, props.event, undefined); const {rerender} = render(); @@ -397,7 +403,8 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }) + }), + undefined ); render(); @@ -429,7 +436,8 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }) + }), + undefined ); render(, { @@ -477,7 +485,8 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }) + }), + undefined ); render(, {}); @@ -496,7 +505,7 @@ describe('groupEventDetails', () => { it('renders event tags ui', async () => { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event); + mockGroupApis(props.organization, props.project, props.group, props.event, undefined); render(, {}); expect(await screen.findByText('Event ID:')).toBeInTheDocument(); @@ -543,7 +552,8 @@ describe('EventCause', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }) + }), + undefined ); MockApiClient.addMockResponse({ @@ -603,7 +613,8 @@ describe('Platform Integrations', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }) + }), + undefined ); const component = SentryAppComponentFixture({ @@ -642,6 +653,7 @@ describe('Platform Integrations', () => { props.project, props.group, props.event, + undefined, mockedTrace(props.project) ); @@ -660,10 +672,17 @@ describe('Platform Integrations', () => { it('does not render root issues section if related perf issues do not exist', async () => { const props = makeDefaultMockData(); const trace = mockedTrace(props.project); - mockGroupApis(props.organization, props.project, props.group, props.event, { - ...trace, - performance_issues: [], - }); + mockGroupApis( + props.organization, + props.project, + props.group, + props.event, + undefined, + { + ...trace, + performance_issues: [], + } + ); render(, { organization: props.organization, From 4a31e243584c90168643bbd331a9a629f5d3f801 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:21:31 -0700 Subject: [PATCH 4/7] :white_check_mark: fix more tests --- .../events/highlights/highlightsDataSection.spec.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/static/app/components/events/highlights/highlightsDataSection.spec.tsx b/static/app/components/events/highlights/highlightsDataSection.spec.tsx index aa770705a3354..da9d20b49549f 100644 --- a/static/app/components/events/highlights/highlightsDataSection.spec.tsx +++ b/static/app/components/events/highlights/highlightsDataSection.spec.tsx @@ -43,6 +43,11 @@ describe('HighlightsDataSection', function () { url: `/projects/${organization.slug}/${project.slug}/`, body: {...project, highlightTags: [], highlightContext: {}}, }); + const replayId = undefined; + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/replays/${replayId}/`, + body: {}, + }); render( , { organization, }); From 0ec8f9ff50d592d0d727118f093b855f4bf4d13c Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:21:50 -0700 Subject: [PATCH 5/7] :white_check_mark: fix more tests --- .../issueDetails/groupEventDetails/groupEventDetails.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx index cd308b01f66a0..c533869ef802f 100644 --- a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx +++ b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx @@ -208,7 +208,7 @@ const mockGroupApis = ( MockApiClient.addMockResponse({ url: `/organizations/${organization.slug}/replays/${replayId}/`, - body: [], + body: {}, }); MockApiClient.addMockResponse({ From 052f8068ecea585e8dba465b41e74d146aeb9367 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:08:41 -0700 Subject: [PATCH 6/7] :recycle: code review --- .../events/highlights/highlightsDataSection.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/static/app/components/events/highlights/highlightsDataSection.tsx b/static/app/components/events/highlights/highlightsDataSection.tsx index aa0e59dbdede2..97d89783e3ea8 100644 --- a/static/app/components/events/highlights/highlightsDataSection.tsx +++ b/static/app/components/events/highlights/highlightsDataSection.tsx @@ -154,16 +154,16 @@ function HighlightsData({ const contextReplayItem = highlightContextDataItems.find( e => e.data[0].key === 'replay_id' ); - const contextReplayId = contextReplayItem?.value ?? ''; + const contextReplayId = contextReplayItem?.value ?? EMPTY_HIGHLIGHT_DEFAULT; const tagReplayItem = highlightTagItems.find(e => e.originalTag.key === 'replayId'); - const tagReplayId = tagReplayItem?.value ?? ''; + const tagReplayId = tagReplayItem?.value ?? EMPTY_HIGHLIGHT_DEFAULT; // if the id doesn't exist for either tag or context, it's rendered as '--' const replayId = - contextReplayId.length > 2 + contextReplayId !== EMPTY_HIGHLIGHT_DEFAULT ? contextReplayId - : tagReplayId.length > 2 + : tagReplayId !== EMPTY_HIGHLIGHT_DEFAULT ? tagReplayId : undefined; From 7f119bef38b96a175be64cf27680d4bbe0c4c382 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:49:01 -0700 Subject: [PATCH 7/7] :recycle: fix test --- .../groupEventDetails.spec.tsx | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx index c533869ef802f..f3217aa2d707b 100644 --- a/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx +++ b/static/app/views/issueDetails/groupEventDetails/groupEventDetails.spec.tsx @@ -198,7 +198,7 @@ const mockGroupApis = ( project: Project, group: Group, event: Event, - replayId: string | undefined, + replayId?: string, trace?: QuickTraceEvent ) => { MockApiClient.addMockResponse({ @@ -364,7 +364,7 @@ describe('groupEventDetails', () => { it('redirects on switching to an invalid environment selection for event', async function () { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event, undefined); + mockGroupApis(props.organization, props.project, props.group, props.event); const {rerender} = render(); expect(browserHistory.replace).not.toHaveBeenCalled(); @@ -376,7 +376,7 @@ describe('groupEventDetails', () => { it('does not redirect when switching to a valid environment selection for event', async function () { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event, undefined); + mockGroupApis(props.organization, props.project, props.group, props.event); const {rerender} = render(); @@ -403,8 +403,7 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }), - undefined + }) ); render(); @@ -436,8 +435,7 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }), - undefined + }) ); render(, { @@ -485,8 +483,7 @@ describe('groupEventDetails', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }), - undefined + }) ); render(, {}); @@ -505,7 +502,7 @@ describe('groupEventDetails', () => { it('renders event tags ui', async () => { const props = makeDefaultMockData(); - mockGroupApis(props.organization, props.project, props.group, props.event, undefined); + mockGroupApis(props.organization, props.project, props.group, props.event); render(, {}); expect(await screen.findByText('Event ID:')).toBeInTheDocument(); @@ -552,8 +549,7 @@ describe('EventCause', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }), - undefined + }) ); MockApiClient.addMockResponse({ @@ -613,8 +609,7 @@ describe('Platform Integrations', () => { tags: [{key: 'environment', value: 'dev'}], previousEventID: 'prev-event-id', nextEventID: 'next-event-id', - }), - undefined + }) ); const component = SentryAppComponentFixture({