Skip to content

Commit 89f7480

Browse files
scttcperandrewshie-sentry
scttcper
authored and
andrewshie-sentry
committed
feat(issues): Fall back to image renderer (#93858)
We wanted to open this up the screenshot viewer (in #93211) to anyone uploading a screenshot.png but we've been erroring when the mimetype is not in our short list of allowed image mimetypes and attempting to render undefined. Also fixes a bug where I removed the rrweb viewer in #93359
1 parent 505825e commit 89f7480

File tree

6 files changed

+245
-135
lines changed

6 files changed

+245
-135
lines changed

‎static/app/components/events/attachmentViewers/previewAttachmentTypes.tsx

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import ImageViewer from 'sentry/components/events/attachmentViewers/imageViewer';
22
import JsonViewer from 'sentry/components/events/attachmentViewers/jsonViewer';
33
import LogFileViewer from 'sentry/components/events/attachmentViewers/logFileViewer';
4-
import type RRWebJsonViewer from 'sentry/components/events/attachmentViewers/rrwebJsonViewer';
4+
import RRWebJsonViewer from 'sentry/components/events/attachmentViewers/rrwebJsonViewer';
55
import {WebMViewer} from 'sentry/components/events/attachmentViewers/webmViewer';
66
import type {IssueAttachment} from 'sentry/types/group';
77

@@ -37,26 +37,36 @@ type AttachmentRenderer =
3737
| typeof RRWebJsonViewer
3838
| typeof WebMViewer;
3939

40-
export const getInlineAttachmentRenderer = (
40+
export const getImageAttachmentRenderer = (
4141
attachment: IssueAttachment
4242
): AttachmentRenderer | undefined => {
4343
if (imageMimeTypes.includes(attachment.mimetype)) {
4444
return ImageViewer;
4545
}
46+
if (webmMimeType === attachment.mimetype) {
47+
return WebMViewer;
48+
}
49+
return undefined;
50+
};
51+
52+
export const getInlineAttachmentRenderer = (
53+
attachment: IssueAttachment
54+
): AttachmentRenderer | undefined => {
55+
const imageAttachmentRenderer = getImageAttachmentRenderer(attachment);
56+
if (imageAttachmentRenderer) {
57+
return imageAttachmentRenderer;
58+
}
4659

4760
if (logFileMimeTypes.includes(attachment.mimetype)) {
4861
return LogFileViewer;
4962
}
5063

51-
if (
52-
(jsonMimeTypes.includes(attachment.mimetype) && attachment.name === 'rrweb.json') ||
53-
attachment.name.startsWith('rrweb-')
54-
) {
55-
return JsonViewer;
56-
}
64+
if (jsonMimeTypes.includes(attachment.mimetype)) {
65+
if (attachment.name === 'rrweb.json' || attachment.name.startsWith('rrweb-')) {
66+
return RRWebJsonViewer;
67+
}
5768

58-
if (webmMimeType === attachment.mimetype) {
59-
return WebMViewer;
69+
return JsonViewer;
6070
}
6171

6272
return undefined;

‎static/app/components/events/eventTagsAndScreenshot/screenshot/index.tsx

Lines changed: 98 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type {ReactEventHandler} from 'react';
2-
import {Fragment, useState} from 'react';
2+
import {useState} from 'react';
33
import {css} from '@emotion/react';
44
import styled from '@emotion/styled';
55

@@ -8,8 +8,9 @@ import {openConfirmModal} from 'sentry/components/confirm';
88
import {Button} from 'sentry/components/core/button';
99
import {ButtonBar} from 'sentry/components/core/button/buttonBar';
1010
import {DropdownMenu} from 'sentry/components/dropdownMenu';
11+
import ImageViewer from 'sentry/components/events/attachmentViewers/imageViewer';
1112
import {
12-
getInlineAttachmentRenderer,
13+
getImageAttachmentRenderer,
1314
imageMimeTypes,
1415
webmMimeType,
1516
} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
@@ -70,115 +71,105 @@ function Screenshot({
7071
onDelete(screenshotAttachmentId);
7172
}
7273

73-
function renderContent(screenshotAttachment: EventAttachment) {
74-
const AttachmentComponent = getInlineAttachmentRenderer(screenshotAttachment)!;
75-
76-
const downloadUrl = `/api/0/projects/${organization.slug}/${projectSlug}/events/${eventId}/attachments/${screenshot.id}/`;
77-
78-
return (
79-
<Fragment>
80-
{totalScreenshots > 1 && (
81-
<StyledPanelHeader lightText>
74+
const AttachmentComponent = getImageAttachmentRenderer(screenshot) ?? ImageViewer;
75+
const downloadUrl = `/api/0/projects/${organization.slug}/${projectSlug}/events/${eventId}/attachments/${screenshot.id}/`;
76+
77+
return (
78+
<StyledPanel>
79+
{totalScreenshots > 1 && (
80+
<StyledPanelHeader lightText>
81+
<Button
82+
disabled={screenshotInFocus === 0}
83+
aria-label={t('Previous Screenshot')}
84+
onClick={onPrevious}
85+
icon={<IconChevron direction="left" />}
86+
size="xs"
87+
/>
88+
{tct('[currentScreenshot] of [totalScreenshots]', {
89+
currentScreenshot: screenshotInFocus + 1,
90+
totalScreenshots,
91+
})}
92+
<Button
93+
disabled={screenshotInFocus + 1 === totalScreenshots}
94+
aria-label={t('Next Screenshot')}
95+
onClick={onNext}
96+
icon={<IconChevron direction="right" />}
97+
size="xs"
98+
/>
99+
</StyledPanelHeader>
100+
)}
101+
<StyledPanelBody hasHeader={totalScreenshots > 1}>
102+
{loadingImage && (
103+
<StyledLoadingIndicator>
104+
<LoadingIndicator mini />
105+
</StyledLoadingIndicator>
106+
)}
107+
<AttachmentComponentWrapper
108+
onClick={() => openVisualizationModal(screenshot, `${downloadUrl}?download=1`)}
109+
>
110+
<AttachmentComponent
111+
orgSlug={organization.slug}
112+
projectSlug={projectSlug}
113+
eventId={eventId}
114+
attachment={screenshot}
115+
onLoad={() => setLoadingImage(false)}
116+
onError={() => setLoadingImage(false)}
117+
controls={false}
118+
onCanPlay={() => setLoadingImage(false)}
119+
/>
120+
</AttachmentComponentWrapper>
121+
</StyledPanelBody>
122+
{!onlyRenderScreenshot && (
123+
<StyledPanelFooter>
124+
<ButtonBar gap={1}>
82125
<Button
83-
disabled={screenshotInFocus === 0}
84-
aria-label={t('Previous Screenshot')}
85-
onClick={onPrevious}
86-
icon={<IconChevron direction="left" />}
87126
size="xs"
88-
/>
89-
{tct('[currentScreenshot] of [totalScreenshots]', {
90-
currentScreenshot: screenshotInFocus + 1,
91-
totalScreenshots,
92-
})}
93-
<Button
94-
disabled={screenshotInFocus + 1 === totalScreenshots}
95-
aria-label={t('Next Screenshot')}
96-
onClick={onNext}
97-
icon={<IconChevron direction="right" />}
127+
onClick={() =>
128+
openVisualizationModal(screenshot, `${downloadUrl}?download=1`)
129+
}
130+
>
131+
{t('View screenshot')}
132+
</Button>
133+
<DropdownMenu
134+
position="bottom"
135+
offset={4}
136+
triggerProps={{
137+
showChevron: false,
138+
icon: <IconEllipsis />,
139+
'aria-label': t('More screenshot actions'),
140+
}}
98141
size="xs"
99-
/>
100-
</StyledPanelHeader>
101-
)}
102-
<StyledPanelBody hasHeader={totalScreenshots > 1}>
103-
{loadingImage && (
104-
<StyledLoadingIndicator>
105-
<LoadingIndicator mini />
106-
</StyledLoadingIndicator>
107-
)}
108-
<AttachmentComponentWrapper
109-
onClick={() =>
110-
openVisualizationModal(screenshot, `${downloadUrl}?download=1`)
111-
}
112-
>
113-
<AttachmentComponent
114-
orgSlug={organization.slug}
115-
projectSlug={projectSlug}
116-
eventId={eventId}
117-
attachment={screenshot}
118-
onLoad={() => setLoadingImage(false)}
119-
onError={() => setLoadingImage(false)}
120-
controls={false}
121-
onCanPlay={() => setLoadingImage(false)}
122-
/>
123-
</AttachmentComponentWrapper>
124-
</StyledPanelBody>
125-
{!onlyRenderScreenshot && (
126-
<StyledPanelFooter>
127-
<ButtonBar gap={1}>
128-
<Button
129-
size="xs"
130-
onClick={() =>
131-
openVisualizationModal(
132-
screenshotAttachment,
133-
`${downloadUrl}?download=1`
134-
)
135-
}
136-
>
137-
{t('View screenshot')}
138-
</Button>
139-
<DropdownMenu
140-
position="bottom"
141-
offset={4}
142-
triggerProps={{
143-
showChevron: false,
144-
icon: <IconEllipsis />,
145-
'aria-label': t('More screenshot actions'),
146-
}}
147-
size="xs"
148-
items={[
149-
{
150-
key: 'download',
151-
label: t('Download'),
152-
onAction: () => {
153-
window.location.assign(`${downloadUrl}?download=1`);
154-
trackAnalytics(
155-
'issue_details.issue_tab.screenshot_dropdown_download',
156-
{organization}
157-
);
158-
},
159-
},
160-
{
161-
key: 'delete',
162-
label: t('Delete'),
163-
onAction: () =>
164-
openConfirmModal({
165-
header: t('Delete this image?'),
166-
message: t(
167-
'This image was captured around the time that the event occurred. Are you sure you want to delete this image?'
168-
),
169-
onConfirm: () => handleDelete(screenshotAttachment.id),
170-
}),
142+
items={[
143+
{
144+
key: 'download',
145+
label: t('Download'),
146+
onAction: () => {
147+
window.location.assign(`${downloadUrl}?download=1`);
148+
trackAnalytics(
149+
'issue_details.issue_tab.screenshot_dropdown_download',
150+
{organization}
151+
);
171152
},
172-
]}
173-
/>
174-
</ButtonBar>
175-
</StyledPanelFooter>
176-
)}
177-
</Fragment>
178-
);
179-
}
180-
181-
return <StyledPanel>{renderContent(screenshot)}</StyledPanel>;
153+
},
154+
{
155+
key: 'delete',
156+
label: t('Delete'),
157+
onAction: () =>
158+
openConfirmModal({
159+
header: t('Delete this image?'),
160+
message: t(
161+
'This image was captured around the time that the event occurred. Are you sure you want to delete this image?'
162+
),
163+
onConfirm: () => handleDelete(screenshot.id),
164+
}),
165+
},
166+
]}
167+
/>
168+
</ButtonBar>
169+
</StyledPanelFooter>
170+
)}
171+
</StyledPanel>
172+
);
182173
}
183174

184175
export default Screenshot;

‎static/app/components/events/eventTagsAndScreenshot/screenshot/modal.tsx

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import type {ComponentProps} from 'react';
2-
import {Fragment, useCallback, useEffect, useMemo, useState} from 'react';
2+
import {Fragment, useCallback, useMemo, useState} from 'react';
33
import {css} from '@emotion/react';
44
import styled from '@emotion/styled';
5-
import * as Sentry from '@sentry/react';
65

76
import type {ModalRenderProps} from 'sentry/actionCreators/modal';
87
import Confirm from 'sentry/components/confirm';
@@ -11,7 +10,8 @@ import {ButtonBar} from 'sentry/components/core/button/buttonBar';
1110
import {LinkButton} from 'sentry/components/core/button/linkButton';
1211
import {Flex} from 'sentry/components/core/layout';
1312
import {DateTime} from 'sentry/components/dateTime';
14-
import {getInlineAttachmentRenderer} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
13+
import ImageViewer from 'sentry/components/events/attachmentViewers/imageViewer';
14+
import {getImageAttachmentRenderer} from 'sentry/components/events/attachmentViewers/previewAttachmentTypes';
1515
import {KeyValueData} from 'sentry/components/keyValueData';
1616
import {t, tct} from 'sentry/locale';
1717
import {space} from 'sentry/styles/space';
@@ -106,18 +106,8 @@ export default function ScreenshotModal({
106106
};
107107
}
108108

109-
const AttachmentComponent = getInlineAttachmentRenderer(currentEventAttachment)!;
110-
111-
useEffect(() => {
112-
if (currentEventAttachment && !AttachmentComponent) {
113-
Sentry.withScope(scope => {
114-
scope.setExtra('mimetype', currentEventAttachment.mimetype);
115-
scope.setExtra('attachmentName', currentEventAttachment.name);
116-
scope.setFingerprint(['no-inline-attachment-renderer']);
117-
scope.captureException(new Error('No screenshot attachment renderer found'));
118-
});
119-
}
120-
}, [currentEventAttachment, AttachmentComponent]);
109+
const AttachmentComponent =
110+
getImageAttachmentRenderer(currentEventAttachment) ?? ImageViewer;
121111

122112
return (
123113
<Fragment>

‎static/app/components/events/eventTagsAndScreenshot/screenshot/screenshotDataSection.spec.tsx

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import {EventFixture} from 'sentry-fixture/event';
2+
import {EventAttachmentFixture} from 'sentry-fixture/eventAttachment';
3+
import {OrganizationFixture} from 'sentry-fixture/organization';
4+
import {ProjectFixture} from 'sentry-fixture/project';
5+
6+
import {render, screen} from 'sentry-test/reactTestingLibrary';
7+
8+
import {ScreenshotDataSection} from 'sentry/components/events/eventTagsAndScreenshot/screenshot/screenshotDataSection';
9+
import ProjectsStore from 'sentry/stores/projectsStore';
10+
11+
describe('ScreenshotDataSection', function () {
12+
const organization = OrganizationFixture({
13+
features: ['event-attachments'],
14+
orgRole: 'member',
15+
attachmentsRole: 'member',
16+
});
17+
const project = ProjectFixture();
18+
const event = EventFixture();
19+
20+
beforeEach(() => {
21+
ProjectsStore.loadInitialData([project]);
22+
});
23+
24+
it('renders without error when screenshot has application/json mimetype', async function () {
25+
const attachment = EventAttachmentFixture({
26+
name: 'screenshot.png',
27+
mimetype: 'application/json',
28+
headers: {
29+
'Content-Type': 'application/json',
30+
},
31+
});
32+
33+
MockApiClient.addMockResponse({
34+
url: `/projects/${organization.slug}/${project.slug}/events/${event.id}/attachments/`,
35+
body: [attachment],
36+
});
37+
38+
render(<ScreenshotDataSection event={event} projectSlug={project.slug} />, {
39+
organization,
40+
});
41+
42+
expect(await screen.findByTestId('image-viewer')).toBeInTheDocument();
43+
});
44+
});

‎static/app/components/events/eventTagsAndScreenshot/screenshot/screenshotDataSection.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,17 @@ export function ScreenshotDataSection({
4747
},
4848
{enabled: !isShare}
4949
);
50+
const [screenshotInFocus, setScreenshotInFocus] = useState<number>(0);
5051
const {mutate: deleteAttachment} = useDeleteEventAttachmentOptimistic();
51-
const screenshots = attachments?.filter(({name}) => name.includes('screenshot')) ?? [];
52+
const screenshots = attachments?.filter(attachment =>
53+
attachment.name.includes('screenshot')
54+
);
5255

53-
const [screenshotInFocus, setScreenshotInFocus] = useState<number>(0);
56+
const showScreenshot = !isShare && !!screenshots?.length;
57+
if (!showScreenshot) {
58+
return null;
59+
}
5460

55-
const showScreenshot = !isShare && !!screenshots.length;
5661
const screenshot = screenshots[screenshotInFocus]!;
5762

5863
const handleDeleteScreenshot = (attachmentId: string) => {

0 commit comments

Comments
 (0)