-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(attachments): Include all attachment with screenshot in name #91602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…n name Currently, attachments considered as screenshot is very restrictive. This opens it up to any files containing `screenshot` in their name. This will be immediately useful to Native attachments which name the files as `crash_screenshot.png`. Fixes #91430.
assert len(response.data) == 1 | ||
assert response.data[0]["id"] == str(attachment1.id) | ||
|
||
def test_second_screenshot_filter(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this second test anymore.
) | ||
self.create_attachment(file_name="screenshot.png", event_id=group1_event.event_id) | ||
self.create_attachment(file_name="screenshot-1.png", event_id=group1_event.event_id) | ||
self.create_attachment(file_name="foo.png", event_id=group1_event.event_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This attachment will not be included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, how about a comment?
) | ||
self.create_attachment(file_name="screenshot.png", event_id=group1_event.event_id) | ||
self.create_attachment(file_name="screenshot-1.png", event_id=group1_event.event_id) | ||
self.create_attachment(file_name="foo.png", event_id=group1_event.event_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, how about a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update the develop docs accordingly - https://develop.sentry.dev/sdk/expected-features/#screenshots
might also make sense for us to link to the develop docs in src/sentry/models/eventattachment.py
.
Co-authored-by: Bruno Garcia <[email protected]>
See getsentry/sentry#91602 for details.
Does this change mean that the any jpg/png attachment with string "screenshot" in the name also shows up as a screenshot in the issue? Doesn't do that here: https://sentry-sdks.sentry.io/issues/6588764536/events/abe3bb91767749b09d61a0978ad9a21c/ |
Your issue [1] does not seem to have the clipboard and the Screenshots section [2]. In the Attachments section, you can see your screenshots: Another place that the screenshots will show up under is View all attachments [1] -> Screenshots [2] |
Currently, attachments considered as screenshots are very restrictive. This opens it up to any files containing
screenshot
in their name.This will be immediately useful to Native attachments, which name the files as
crash_screenshot.png
.Fixes #91430.