Browse Source

ref(event-details): Use react-query for fetching and deleting attachments (#43868)

- Adds `useFetchEventAttachments` and
`useDeleteEventAttachmentOptimistic`
- Removes requests made in `EventEntries`, make components that need
attachment data consume the above hooks instead
Malachi Willey 2 years ago
parent
commit
1210aabcbd

+ 114 - 0
static/app/actionCreators/events.tsx

@@ -1,11 +1,15 @@
+import {UseMutationOptions, useQueryClient} from '@tanstack/react-query';
 import {LocationDescriptor} from 'history';
 import pick from 'lodash/pick';
 
+import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {Client, ResponseMeta} from 'sentry/api';
 import {canIncludePreviousPeriod} from 'sentry/components/charts/utils';
+import {t} from 'sentry/locale';
 import {
   DateString,
   EventsStats,
+  IssueAttachment,
   MultiSeriesEventsStats,
   OrganizationSummary,
 } from 'sentry/types';
@@ -13,6 +17,10 @@ import {LocationQuery} from 'sentry/utils/discover/eventView';
 import {getPeriod} from 'sentry/utils/getPeriod';
 import {PERFORMANCE_URL_PARAM} from 'sentry/utils/performance/constants';
 import {QueryBatching} from 'sentry/utils/performance/contexts/genericQueryBatcher';
+import {QueryKey, useMutation, useQuery, UseQueryOptions} from 'sentry/utils/queryClient';
+import RequestError from 'sentry/utils/requestError/requestError';
+import useApi from 'sentry/utils/useApi';
+import useOrganization from 'sentry/utils/useOrganization';
 
 type Options = {
   organization: OrganizationSummary;
@@ -203,3 +211,109 @@ export function fetchTotalCount(
     })
     .then((res: Response) => res.count);
 }
+
+type FetchEventAttachmentParameters = {
+  eventId: string;
+  orgSlug: string;
+  projectSlug: string;
+};
+
+type FetchEventAttachmentResponse = IssueAttachment[];
+
+export const makeFetchEventAttachmentsQueryKey = ({
+  orgSlug,
+  projectSlug,
+  eventId,
+}: FetchEventAttachmentParameters): QueryKey => [
+  `/projects/${orgSlug}/${projectSlug}/events/${eventId}/attachments/`,
+];
+
+export const useFetchEventAttachments = (
+  {orgSlug, projectSlug, eventId}: FetchEventAttachmentParameters,
+  options: Partial<UseQueryOptions<FetchEventAttachmentResponse>> = {}
+) => {
+  const organization = useOrganization();
+  return useQuery<FetchEventAttachmentResponse>(
+    [`/projects/${orgSlug}/${projectSlug}/events/${eventId}/attachments/`],
+    {
+      staleTime: Infinity,
+      ...options,
+      enabled:
+        (organization.features?.includes('event-attachments') ?? false) &&
+        options.enabled !== false,
+    }
+  );
+};
+
+type DeleteEventAttachmentVariables = {
+  attachmentId: string;
+  eventId: string;
+  orgSlug: string;
+  projectSlug: string;
+};
+
+type DeleteEventAttachmentResponse = unknown;
+
+type DeleteEventAttachmentContext = {
+  previous?: IssueAttachment[];
+};
+
+type DeleteEventAttachmentOptions = UseMutationOptions<
+  DeleteEventAttachmentResponse,
+  RequestError,
+  DeleteEventAttachmentVariables,
+  DeleteEventAttachmentContext
+>;
+
+export const useDeleteEventAttachmentOptimistic = (
+  incomingOptions: Partial<DeleteEventAttachmentOptions> = {}
+) => {
+  const api = useApi({persistInFlight: true});
+  const queryClient = useQueryClient();
+
+  const options: DeleteEventAttachmentOptions = {
+    ...incomingOptions,
+    mutationFn: ({orgSlug, projectSlug, eventId, attachmentId}) => {
+      return api.requestPromise(
+        `/projects/${orgSlug}/${projectSlug}/events/${eventId}/attachments/${attachmentId}/`,
+        {method: 'DELETE'}
+      );
+    },
+    onMutate: async variables => {
+      await queryClient.cancelQueries(makeFetchEventAttachmentsQueryKey(variables));
+
+      const previous = queryClient.getQueryData<FetchEventAttachmentResponse>(
+        makeFetchEventAttachmentsQueryKey(variables)
+      );
+
+      queryClient.setQueryData<FetchEventAttachmentResponse>(
+        makeFetchEventAttachmentsQueryKey(variables),
+        oldData => {
+          if (!Array.isArray(oldData)) {
+            return oldData;
+          }
+
+          return oldData.filter(attachment => attachment?.id !== variables.attachmentId);
+        }
+      );
+
+      incomingOptions.onMutate?.(variables);
+
+      return {previous};
+    },
+    onError: (error, variables, context) => {
+      addErrorMessage(t('An error occurred while deleting the attachment'));
+
+      if (context) {
+        queryClient.setQueryData(
+          makeFetchEventAttachmentsQueryKey(variables),
+          context.previous
+        );
+      }
+
+      incomingOptions.onError?.(error, variables, context);
+    },
+  };
+
+  return useMutation(options);
+};

+ 150 - 13
static/app/components/events/eventAttachments.spec.tsx

@@ -1,25 +1,50 @@
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, screen} from 'sentry-test/reactTestingLibrary';
+import {
+  render,
+  renderGlobalModal,
+  screen,
+  userEvent,
+  waitFor,
+  within,
+} from 'sentry-test/reactTestingLibrary';
 
 import {EventAttachments} from 'sentry/components/events/eventAttachments';
 
 describe('EventAttachments', function () {
-  const {routerContext, organization, project} = initializeOrg();
-  const event = TestStubs.Event({metadata: {stripped_crash: true}});
+  const {routerContext, organization, project} = initializeOrg({
+    organization: {
+      features: ['event-attachments'],
+      orgRole: 'member',
+      attachmentsRole: 'member',
+    },
+  } as any);
+  const event = TestStubs.Event({metadata: {stripped_crash: false}});
 
   const props = {
-    orgId: organization.slug,
     projectSlug: project.slug,
-    location: routerContext.context.location,
-    attachments: [],
-    onDeleteAttachment: jest.fn(),
     event,
   };
 
-  it('shows attachments limit reached notice', function () {
-    render(<EventAttachments {...props} />, {context: routerContext});
+  const attachmentsUrl = `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`;
 
-    expect(screen.getByText('Attachments (0)')).toBeInTheDocument();
+  beforeEach(() => {
+    MockApiClient.clearMockResponses();
+  });
+
+  it('shows attachments limit reached notice with stripped_crash: true', async function () {
+    MockApiClient.addMockResponse({
+      url: attachmentsUrl,
+      body: [],
+    });
+    const strippedCrashEvent = {...event, metadata: {stripped_crash: true}};
+    render(<EventAttachments {...props} event={strippedCrashEvent} />, {
+      context: routerContext,
+      organization,
+    });
+
+    expect(await screen.findByText('Attachments (0)')).toBeInTheDocument();
+
+    await tick();
 
     expect(screen.getByRole('link', {name: 'View crashes'})).toHaveAttribute(
       'href',
@@ -28,7 +53,7 @@ describe('EventAttachments', function () {
 
     expect(screen.getByRole('link', {name: 'configure limit'})).toHaveAttribute(
       'href',
-      `/settings/${props.orgId}/projects/${props.projectSlug}/security-and-privacy/`
+      `/settings/org-slug/projects/${props.projectSlug}/security-and-privacy/`
     );
 
     expect(
@@ -39,13 +64,125 @@ describe('EventAttachments', function () {
     ).toBeInTheDocument();
   });
 
-  it('does not render anything if no attachments (nor stripped) are available', function () {
+  it('does not render anything if no attachments (nor stripped) are available', async function () {
+    MockApiClient.addMockResponse({
+      url: attachmentsUrl,
+      body: [],
+    });
     const {container} = render(
       <EventAttachments
         {...props}
         event={{...event, metadata: {stripped_crash: false}}}
-      />
+      />,
+      {context: routerContext, organization}
     );
+
+    // No loading state to wait for
+    await tick();
+
     expect(container).toBeEmptyDOMElement();
   });
+
+  it('displays message when user lacks permission to preview an attachment', async function () {
+    const {routerContext: newRouterContext, organization: orgWithWrongAttachmentRole} =
+      initializeOrg({
+        organization: {
+          features: ['event-attachments'],
+          orgRole: 'member',
+          attachmentsRole: 'admin',
+        },
+      } as any);
+    const attachment = TestStubs.EventAttachment({
+      name: 'some_file.txt',
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+      mimetype: 'text/plain',
+      size: 100,
+    });
+    MockApiClient.addMockResponse({
+      url: attachmentsUrl,
+      body: [attachment],
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/projects/org-slug/events/${event.id}/attachments/?download`,
+      body: 'file contents',
+    });
+
+    render(<EventAttachments {...props} />, {
+      context: newRouterContext,
+      organization: orgWithWrongAttachmentRole,
+    });
+
+    expect(await screen.findByText('Attachments (1)')).toBeInTheDocument();
+
+    expect(screen.getByRole('button', {name: /preview/i})).toBeDisabled();
+    userEvent.hover(screen.getByRole('button', {name: /preview/i}));
+
+    await screen.findByText(/insufficient permissions to preview attachments/i);
+  });
+
+  it('can open attachment previews', async function () {
+    const attachment = TestStubs.EventAttachment({
+      name: 'some_file.txt',
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+      mimetype: 'text/plain',
+      size: 100,
+    });
+    MockApiClient.addMockResponse({
+      url: attachmentsUrl,
+      body: [attachment],
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/projects/org-slug/project-slug/events/1/attachments/1/?download',
+      body: 'file contents',
+    });
+
+    render(<EventAttachments {...props} />, {context: routerContext, organization});
+
+    expect(await screen.findByText('Attachments (1)')).toBeInTheDocument();
+
+    userEvent.click(screen.getByRole('button', {name: /preview/i}));
+
+    expect(screen.getByText('file contents')).toBeInTheDocument();
+  });
+
+  it('can delete attachments', async function () {
+    const attachment1 = TestStubs.EventAttachment({
+      id: '1',
+      name: 'pic_1.png',
+    });
+    const attachment2 = TestStubs.EventAttachment({
+      id: '2',
+      name: 'pic_2.png',
+    });
+    MockApiClient.addMockResponse({
+      url: attachmentsUrl,
+      body: [attachment1, attachment2],
+    });
+    const deleteMock = MockApiClient.addMockResponse({
+      url: '/projects/org-slug/project-slug/events/1/attachments/1/',
+      method: 'DELETE',
+    });
+
+    render(<EventAttachments {...props} />, {context: routerContext, organization});
+    renderGlobalModal();
+
+    expect(await screen.findByText('Attachments (2)')).toBeInTheDocument();
+
+    userEvent.click(screen.getAllByRole('button', {name: 'Delete'})[0]);
+    userEvent.click(
+      within(screen.getByRole('dialog')).getByRole('button', {name: /delete/i})
+    );
+
+    // Should make the delete request and remove the attachment optimistically
+    await waitFor(() => {
+      expect(deleteMock).toHaveBeenCalled();
+      expect(screen.queryByTestId('pic_1.png')).not.toBeInTheDocument();
+    });
+  });
 });

+ 35 - 12
static/app/components/events/eventAttachments.tsx

@@ -1,6 +1,10 @@
 import {Fragment, useState} from 'react';
 import styled from '@emotion/styled';
 
+import {
+  useDeleteEventAttachmentOptimistic,
+  useFetchEventAttachments,
+} from 'sentry/actionCreators/events';
 import AttachmentUrl from 'sentry/components/events/attachmentUrl';
 import ImageViewer from 'sentry/components/events/attachmentViewers/imageViewer';
 import JsonViewer from 'sentry/components/events/attachmentViewers/jsonViewer';
@@ -9,19 +13,17 @@ import RRWebJsonViewer from 'sentry/components/events/attachmentViewers/rrwebJso
 import EventAttachmentActions from 'sentry/components/events/eventAttachmentActions';
 import {EventDataSection} from 'sentry/components/events/eventDataSection';
 import FileSize from 'sentry/components/fileSize';
+import LoadingError from 'sentry/components/loadingError';
 import {PanelTable} from 'sentry/components/panels';
 import {t} from 'sentry/locale';
 import {IssueAttachment} from 'sentry/types';
 import {Event} from 'sentry/types/event';
-import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 
 import EventAttachmentsCrashReportsNotice from './eventAttachmentsCrashReportsNotice';
 
 type EventAttachmentsProps = {
-  attachments: IssueAttachment[];
   event: Event;
-  onDeleteAttachment: (attachmentId: IssueAttachment['id']) => void;
   projectSlug: string;
 };
 
@@ -89,19 +91,34 @@ const InlineEventAttachment = ({
   );
 };
 
-export const EventAttachments = ({
-  attachments,
-  event,
-  onDeleteAttachment,
-  projectSlug,
-}: EventAttachmentsProps) => {
+export const EventAttachments = ({event, projectSlug}: EventAttachmentsProps) => {
   const organization = useOrganization();
-  const location = useLocation();
+  const {
+    data: attachments = [],
+    isError,
+    refetch,
+  } = useFetchEventAttachments({
+    orgSlug: organization.slug,
+    projectSlug,
+    eventId: event.id,
+  });
+  const {mutate: deleteAttachment} = useDeleteEventAttachmentOptimistic();
   const [attachmentPreviews, setAttachmentPreviews] = useState<AttachmentPreviewOpenMap>(
     {}
   );
   const crashFileStripped = event.metadata.stripped_crash;
 
+  if (isError) {
+    return (
+      <EventDataSection type="attachments" title="Attachments">
+        <LoadingError
+          onRetry={refetch}
+          message={t('An error occurred while fetching attachments')}
+        />
+      </EventDataSection>
+    );
+  }
+
   if (!attachments.length && !crashFileStripped) {
     return null;
   }
@@ -126,7 +143,6 @@ export const EventAttachments = ({
           orgSlug={organization.slug}
           projectSlug={projectSlug}
           groupId={event.groupID!}
-          location={location}
         />
       )}
 
@@ -153,7 +169,14 @@ export const EventAttachments = ({
                   <div>
                     <EventAttachmentActions
                       url={url}
-                      onDelete={onDeleteAttachment}
+                      onDelete={(attachmentId: string) =>
+                        deleteAttachment({
+                          orgSlug: organization.slug,
+                          projectSlug,
+                          eventId: event.id,
+                          attachmentId,
+                        })
+                      }
                       onPreview={_attachmentId => togglePreview(attachment)}
                       withPreviewButton
                       previewIsOpen={attachmentPreviewIsOpen(

+ 3 - 9
static/app/components/events/eventAttachmentsCrashReportsNotice.tsx

@@ -1,23 +1,17 @@
-import {Location} from 'history';
-
 import {Alert} from 'sentry/components/alert';
 import Link from 'sentry/components/links/link';
 import {tct} from 'sentry/locale';
+import {useLocation} from 'sentry/utils/useLocation';
 import {crashReportTypes} from 'sentry/views/issueDetails/groupEventAttachments/groupEventAttachmentsFilter';
 
 type Props = {
   groupId: string;
-  location: Location;
   orgSlug: string;
   projectSlug: string;
 };
 
-const EventAttachmentsCrashReportsNotice = ({
-  orgSlug,
-  projectSlug,
-  location,
-  groupId,
-}: Props) => {
+const EventAttachmentsCrashReportsNotice = ({orgSlug, projectSlug, groupId}: Props) => {
+  const location = useLocation();
   const settingsUrl = `/settings/${orgSlug}/projects/${projectSlug}/security-and-privacy/`;
   const attachmentsUrl = {
     pathname: `/organizations/${orgSlug}/issues/${groupId}/attachments/`,

+ 4 - 69
static/app/components/events/eventEntries.tsx

@@ -1,9 +1,7 @@
-import {Fragment, useCallback, useEffect, useState} from 'react';
+import {Fragment} from 'react';
 import styled from '@emotion/styled';
-import * as Sentry from '@sentry/react';
 import {Location} from 'history';
 
-import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {CommitRow} from 'sentry/components/commitRow';
 import ErrorBoundary from 'sentry/components/errorBoundary';
 import {t} from 'sentry/locale';
@@ -13,7 +11,6 @@ import {
   EntryType,
   Event,
   Group,
-  IssueAttachment,
   IssueCategory,
   Organization,
   Project,
@@ -21,7 +18,6 @@ import {
 } from 'sentry/types';
 import {isNotSharedOrganization} from 'sentry/types/utils';
 import {objectIsEmpty} from 'sentry/utils';
-import useApi from 'sentry/utils/useApi';
 
 import {EventContexts} from './contexts';
 import {EventDevice} from './device';
@@ -66,10 +62,6 @@ const EventEntries = ({
   isShare = false,
   showTagSummary = true,
 }: Props) => {
-  const api = useApi();
-
-  const [attachments, setAttachments] = useState<IssueAttachment[]>([]);
-
   const orgSlug = organization.slug;
   const projectSlug = project.slug;
   const orgFeatures = organization?.features ?? [];
@@ -77,49 +69,6 @@ const EventEntries = ({
   const hasEventAttachmentsFeature = orgFeatures.includes('event-attachments');
   const hasReplay = Boolean(event?.tags?.find(({key}) => key === 'replayId')?.value);
 
-  const fetchAttachments = useCallback(async () => {
-    if (!event || isShare || !hasEventAttachmentsFeature) {
-      return;
-    }
-
-    try {
-      const response = await api.requestPromise(
-        `/projects/${orgSlug}/${projectSlug}/events/${event.id}/attachments/`
-      );
-      setAttachments(response);
-    } catch (error) {
-      Sentry.captureException(error);
-      addErrorMessage('An error occurred while fetching attachments');
-    }
-  }, [api, event, hasEventAttachmentsFeature, isShare, orgSlug, projectSlug]);
-
-  const handleDeleteAttachment = useCallback(
-    async (attachmentId: IssueAttachment['id']) => {
-      if (!event) {
-        return;
-      }
-
-      try {
-        await api.requestPromise(
-          `/projects/${orgSlug}/${projectSlug}/events/${event.id}/attachments/${attachmentId}/`,
-          {
-            method: 'DELETE',
-          }
-        );
-
-        setAttachments(attachments.filter(attachment => attachment.id !== attachmentId));
-      } catch (error) {
-        Sentry.captureException(error);
-        addErrorMessage('An error occurred while deleting the attachment');
-      }
-    },
-    [api, attachments, event, orgSlug, projectSlug]
-  );
-
-  useEffect(() => {
-    fetchAttachments();
-  }, [fetchAttachments]);
-
   if (!event) {
     return (
       <LatestEventNotAvailable>
@@ -158,8 +107,6 @@ const EventEntries = ({
           location={location}
           isShare={isShare}
           hasContext={hasContext}
-          attachments={attachments}
-          onDeleteScreenshot={handleDeleteAttachment}
         />
       )}
       <EventEvidence event={event} group={group} />
@@ -176,23 +123,11 @@ const EventEntries = ({
       {event && !objectIsEmpty(event.device) && <EventDevice event={event} />}
       {!isShare &&
         organization.features?.includes('mobile-view-hierarchies') &&
-        hasEventAttachmentsFeature &&
-        !!attachments.filter(attachment => attachment.type === 'event.view_hierarchy')
-          .length && (
-          <EventViewHierarchy
-            projectSlug={projectSlug}
-            viewHierarchies={attachments.filter(
-              attachment => attachment.type === 'event.view_hierarchy'
-            )}
-          />
+        hasEventAttachmentsFeature && (
+          <EventViewHierarchy event={event} projectSlug={projectSlug} />
         )}
       {!isShare && hasEventAttachmentsFeature && (
-        <EventAttachments
-          event={event}
-          projectSlug={projectSlug}
-          attachments={attachments}
-          onDeleteAttachment={handleDeleteAttachment}
-        />
+        <EventAttachments event={event} projectSlug={projectSlug} />
       )}
       {event.sdk && !objectIsEmpty(event.sdk) && (
         <EventSdk sdk={event.sdk} meta={event._meta?.sdk} />

+ 74 - 53
static/app/components/events/eventTagsAndScreenshot/index.spec.tsx

@@ -115,6 +115,7 @@ describe('EventTagsAndScreenshot', function () {
     organization: {
       orgRole: 'member',
       attachmentsRole: 'member',
+      features: ['event-attachments'],
       orgRoleList: [
         {
           id: 'member',
@@ -151,7 +152,32 @@ describe('EventTagsAndScreenshot', function () {
     },
   ];
 
+  beforeEach(() => {
+    MockApiClient.clearMockResponses();
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/repos/',
+      body: [],
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/projects/org-slug/project-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/',
+      body: {},
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/deploys/',
+      body: [],
+    });
+  });
+
   describe('renders tags only', function () {
+    beforeEach(() => {
+      MockApiClient.addMockResponse({
+        url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
+        body: [],
+      });
+    });
+
     it('not shared event - without attachments', function () {
       const {container} = render(
         <EventTagsAndScreenshot
@@ -159,10 +185,9 @@ describe('EventTagsAndScreenshot', function () {
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={[]}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
-        />
+        />,
+        {organization}
       );
 
       // Screenshot Container
@@ -210,11 +235,10 @@ describe('EventTagsAndScreenshot', function () {
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={[]}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
           isShare
-        />
+        />,
+        {organization}
       );
 
       // Screenshot Container
@@ -233,8 +257,6 @@ describe('EventTagsAndScreenshot', function () {
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={attachments}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
           isShare
         />
@@ -251,19 +273,11 @@ describe('EventTagsAndScreenshot', function () {
   });
 
   describe('renders screenshot only', function () {
-    MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/repos/',
-      body: [],
-    });
-
-    MockApiClient.addMockResponse({
-      url: '/projects/org-slug/project-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/',
-      body: {},
-    });
-
-    MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/releases/io.sentry.sample.iOS-Swift%407.2.3%2B390/deploys/',
-      body: [],
+    beforeEach(() => {
+      MockApiClient.addMockResponse({
+        url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
+        body: attachments,
+      });
     });
 
     it('no context and no tags', async function () {
@@ -275,20 +289,19 @@ describe('EventTagsAndScreenshot', function () {
             organization={organization}
             projectSlug={project.slug}
             location={router.location}
-            attachments={attachments}
-            onDeleteScreenshot={() => jest.fn()}
             hasContext={false}
           />
-        </Fragment>
+        </Fragment>,
+        {organization}
       );
 
       // Tags Container
       expect(screen.queryByText('Tags')).not.toBeInTheDocument();
 
       // Screenshot Container
-      expect(screen.getByTestId('screenshot-data-section')?.textContent).toContain(
-        'Screenshot'
-      );
+      expect(
+        (await screen.findByTestId('screenshot-data-section'))?.textContent
+      ).toContain('Screenshot');
       expect(screen.getByText('View screenshot')).toBeInTheDocument();
       expect(screen.getByTestId('image-viewer')).toHaveAttribute(
         'src',
@@ -320,21 +333,27 @@ describe('EventTagsAndScreenshot', function () {
   });
 
   describe('renders screenshot and tags', function () {
-    it('has context, tags and attachments', function () {
+    beforeEach(() => {
+      MockApiClient.addMockResponse({
+        url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
+        body: attachments,
+      });
+    });
+
+    it('has context, tags and attachments', async function () {
       const {container} = render(
         <EventTagsAndScreenshot
           event={{...event, tags, contexts}}
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={attachments}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
-        />
+        />,
+        {organization}
       );
 
       // Screenshot Container
-      expect(screen.getByText('View screenshot')).toBeInTheDocument();
+      expect(await screen.findByText('View screenshot')).toBeInTheDocument();
       expect(screen.getByTestId('image-viewer')).toHaveAttribute(
         'src',
         `/api/0/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/${attachments[1].id}/?download`
@@ -362,7 +381,8 @@ describe('EventTagsAndScreenshot', function () {
       expect(container).toSnapshot();
     });
 
-    it('renders multiple screenshots correctly', function () {
+    it('renders multiple screenshots correctly', async function () {
+      MockApiClient.clearMockResponses();
       const moreAttachments = [
         ...attachments,
         {
@@ -377,21 +397,24 @@ describe('EventTagsAndScreenshot', function () {
           event_id: 'bbf4c61ddaa7d8b2dbbede0f3b482cd9beb9434d',
         },
       ];
+      MockApiClient.addMockResponse({
+        url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
+        body: moreAttachments,
+      });
       render(
         <EventTagsAndScreenshot
           event={{...event, tags, contexts}}
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={moreAttachments}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
-        />
+        />,
+        {organization}
       );
 
-      expect(screen.getByTestId('screenshot-data-section')?.textContent).toContain(
-        '1 of 2'
-      );
+      expect(
+        (await screen.findByTestId('screenshot-data-section'))?.textContent
+      ).toContain('1 of 2');
 
       expect(screen.getByText('View screenshot')).toBeInTheDocument();
       expect(screen.getByTestId('image-viewer')).toHaveAttribute(
@@ -412,23 +435,22 @@ describe('EventTagsAndScreenshot', function () {
       );
     });
 
-    it('has context and attachments only', function () {
+    it('has context and attachments only', async function () {
       const {container} = render(
         <EventTagsAndScreenshot
           event={{...event, contexts}}
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={attachments}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext
-        />
+        />,
+        {organization}
       );
 
       // Screenshot Container
-      expect(screen.getByTestId('screenshot-data-section')?.textContent).toContain(
-        'Screenshot'
-      );
+      expect(
+        (await screen.findByTestId('screenshot-data-section'))?.textContent
+      ).toContain('Screenshot');
       expect(screen.getByText('View screenshot')).toBeInTheDocument();
       expect(screen.getByTestId('image-viewer')).toHaveAttribute(
         'src',
@@ -447,23 +469,22 @@ describe('EventTagsAndScreenshot', function () {
       expect(container).toSnapshot();
     });
 
-    it('has tags and attachments only', function () {
+    it('has tags and attachments only', async function () {
       const {container} = render(
         <EventTagsAndScreenshot
           event={{...event, tags}}
           organization={organization}
           projectSlug={project.slug}
           location={router.location}
-          attachments={attachments}
-          onDeleteScreenshot={() => jest.fn()}
           hasContext={false}
-        />
+        />,
+        {organization}
       );
 
       // Screenshot Container
-      expect(screen.getByTestId('screenshot-data-section')?.textContent).toContain(
-        'Screenshot'
-      );
+      expect(
+        (await screen.findByTestId('screenshot-data-section'))?.textContent
+      ).toContain('Screenshot');
       expect(screen.getByText('View screenshot')).toBeInTheDocument();
       expect(screen.getByTestId('image-viewer')).toHaveAttribute(
         'src',

+ 26 - 10
static/app/components/events/eventTagsAndScreenshot/index.tsx

@@ -1,6 +1,10 @@
 import {useState} from 'react';
 import styled from '@emotion/styled';
 
+import {
+  useDeleteEventAttachmentOptimistic,
+  useFetchEventAttachments,
+} from 'sentry/actionCreators/events';
 import {openModal} from 'sentry/actionCreators/modal';
 import {EventDataSection} from 'sentry/components/events/eventDataSection';
 import {DataSection} from 'sentry/components/events/styles';
@@ -16,8 +20,6 @@ import Modal, {modalCss} from './screenshot/modal';
 import Screenshot from './screenshot';
 import Tags from './tags';
 
-type ScreenshotProps = React.ComponentProps<typeof Screenshot>;
-
 const SCREENSHOT_NAMES = [
   'screenshot.jpg',
   'screenshot.png',
@@ -31,8 +33,6 @@ type Props = Omit<
   React.ComponentProps<typeof Tags>,
   'projectSlug' | 'hasEventContext'
 > & {
-  attachments: ScreenshotProps['screenshot'][];
-  onDeleteScreenshot: ScreenshotProps['onDelete'];
   projectSlug: string;
   hasContext?: boolean;
   isShare?: boolean;
@@ -42,15 +42,22 @@ export function EventTagsAndScreenshot({
   projectSlug,
   location,
   event,
-  attachments,
-  onDeleteScreenshot,
   organization,
   isShare = false,
   hasContext = false,
 }: Props) {
   const {tags = []} = event;
-
-  const screenshots = attachments.filter(({name}) => SCREENSHOT_NAMES.includes(name));
+  const {data: attachments} = useFetchEventAttachments(
+    {
+      orgSlug: organization.slug,
+      projectSlug,
+      eventId: event.id,
+    },
+    {enabled: !isShare}
+  );
+  const {mutate: deleteAttachment} = useDeleteEventAttachmentOptimistic();
+  const screenshots =
+    attachments?.filter(({name}) => SCREENSHOT_NAMES.includes(name)) ?? [];
 
   const [screenshotInFocus, setScreenshotInFocus] = useState<number>(0);
 
@@ -64,6 +71,15 @@ export function EventTagsAndScreenshot({
   const hasEventContext = hasContext && !objectIsEmpty(event.contexts);
   const showTags = !!tags.length || hasContext;
 
+  const handleDeleteScreenshot = (attachmentId: string) => {
+    deleteAttachment({
+      orgSlug: organization.id,
+      projectSlug,
+      eventId: event.id,
+      attachmentId,
+    });
+  };
+
   function handleOpenVisualizationModal(
     eventAttachment: EventAttachment,
     downloadUrl: string
@@ -75,7 +91,7 @@ export function EventTagsAndScreenshot({
       trackAdvancedAnalyticsEvent('issue_details.issue_tab.screenshot_modal_deleted', {
         organization,
       });
-      onDeleteScreenshot(eventAttachment.id);
+      handleDeleteScreenshot(eventAttachment.id);
     }
 
     openModal(
@@ -143,7 +159,7 @@ export function EventTagsAndScreenshot({
                 eventId={event.id}
                 projectSlug={projectSlug}
                 screenshot={screenshot}
-                onDelete={onDeleteScreenshot}
+                onDelete={handleDeleteScreenshot}
                 onNext={() => setScreenshotInFocus(screenshotInFocus + 1)}
                 onPrevious={() => setScreenshotInFocus(screenshotInFocus - 1)}
                 screenshotInFocus={screenshotInFocus}

+ 30 - 8
static/app/components/events/eventViewHierarchy.spec.tsx

@@ -42,26 +42,48 @@ const MOCK_DATA = JSON.stringify({
   ],
 });
 
+const organization = TestStubs.Organization({features: ['event-attachments']});
+const event = TestStubs.Event();
+
 describe('Event View Hierarchy', function () {
   let mockAttachment;
   beforeEach(function () {
-    mockAttachment = TestStubs.EventAttachment();
+    mockAttachment = TestStubs.EventAttachment({type: 'event.view_hierarchy'});
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/mock/events/${event.id}/attachments/`,
+      body: [mockAttachment],
+    });
     MockApiClient.addMockResponse({
-      url: `/projects/org-slug/mock/events/${mockAttachment.event_id}/attachments/${mockAttachment.id}/?download`,
+      url: `/projects/${organization.slug}/mock/events/${mockAttachment.event_id}/attachments/${mockAttachment.id}/?download`,
       body: MOCK_DATA,
     });
   });
 
+  it('renders nothing when no view_hierarchy attachments', async () => {
+    MockApiClient.clearMockResponses();
+    MockApiClient.addMockResponse({
+      url: `/projects/org-slug/mock/events/${event.id}/attachments/`,
+      body: [TestStubs.EventAttachment()],
+    });
+
+    const {container} = render(<EventViewHierarchy projectSlug="mock" event={event} />, {
+      organization,
+    });
+
+    // No loading state so nothing to wait for
+    await tick();
+
+    expect(container).toBeEmptyDOMElement();
+  });
+
   it('does not collapse all nodes when update triggers re-render', async function () {
-    const {rerender} = render(
-      <EventViewHierarchy projectSlug="mock" viewHierarchies={[mockAttachment]} />
-    );
+    const {rerender} = render(<EventViewHierarchy projectSlug="mock" event={event} />, {
+      organization,
+    });
 
     expect(await screen.findByText('Nested Container - nested')).toBeInTheDocument();
 
-    rerender(
-      <EventViewHierarchy projectSlug="mock" viewHierarchies={[mockAttachment]} />
-    );
+    rerender(<EventViewHierarchy projectSlug="mock" event={event} />);
 
     expect(await screen.findByText('Nested Container - nested')).toBeInTheDocument();
   });

+ 31 - 13
static/app/components/events/eventViewHierarchy.tsx

@@ -1,39 +1,53 @@
 import {useMemo} from 'react';
 import * as Sentry from '@sentry/react';
+import isEmpty from 'lodash/isEmpty';
 
+import {useFetchEventAttachments} from 'sentry/actionCreators/events';
 import ErrorBoundary from 'sentry/components/errorBoundary';
 import {getAttachmentUrl} from 'sentry/components/events/attachmentViewers/utils';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {tn} from 'sentry/locale';
-import {EventAttachment} from 'sentry/types';
+import {Event, IssueAttachment} from 'sentry/types';
+import {defined} from 'sentry/utils';
 import {useQuery} from 'sentry/utils/queryClient';
 import useOrganization from 'sentry/utils/useOrganization';
 
 import {EventDataSection} from './eventDataSection';
 import {ViewHierarchy, ViewHierarchyData} from './viewHierarchy';
 
-const FIVE_SECONDS_IN_MS = 5 * 1000;
-
 type Props = {
+  event: Event;
   projectSlug: string;
-  viewHierarchies: EventAttachment[];
 };
 
-function EventViewHierarchy({projectSlug, viewHierarchies}: Props) {
+function EventViewHierarchy({projectSlug, event}: Props) {
   const organization = useOrganization();
 
+  const {data: attachments} = useFetchEventAttachments(
+    {
+      orgSlug: organization.slug,
+      projectSlug,
+      eventId: event.id,
+    },
+    {notifyOnChangeProps: ['data']}
+  );
+  const viewHierarchies =
+    attachments?.filter(attachment => attachment.type === 'event.view_hierarchy') ?? [];
+  const hierarchyMeta: IssueAttachment | undefined = viewHierarchies[0];
+
   // There should be only one view hierarchy
-  const hierarchyMeta = viewHierarchies[0];
   const {isLoading, data} = useQuery<string>(
     [
-      getAttachmentUrl({
-        attachment: hierarchyMeta,
-        eventId: hierarchyMeta.event_id,
-        orgId: organization.slug,
-        projectSlug,
-      }),
+      defined(hierarchyMeta)
+        ? getAttachmentUrl({
+            attachment: hierarchyMeta,
+            eventId: hierarchyMeta.event_id,
+            orgId: organization.slug,
+            projectSlug,
+          })
+        : '',
     ],
-    {staleTime: FIVE_SECONDS_IN_MS, refetchOnWindowFocus: false}
+    {staleTime: Infinity, enabled: defined(hierarchyMeta)}
   );
 
   // Memoize the JSON parsing because downstream hooks depend on
@@ -51,6 +65,10 @@ function EventViewHierarchy({projectSlug, viewHierarchies}: Props) {
     }
   }, [data]);
 
+  if (isEmpty(viewHierarchies)) {
+    return null;
+  }
+
   // TODO(nar): This loading behaviour is subject to change
   if (isLoading || !data) {
     return <LoadingIndicator />;