feat(attachments): Include all attachment with screenshot in name (#91602)

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.

---------

Co-authored-by: Bruno Garcia <[email protected]>

Author: armenzg

src/sentry/models/eventattachment.py

@@ -34,13 +34,7 @@ def get_crashreport_key(group_id: int) -> str:
 def event_attachment_screenshot_filter(
     queryset: BaseQuerySet[EventAttachment],
 ) -> BaseQuerySet[EventAttachment]:
-    # Intentionally a hardcoded list instead of a regex since current usecases do not have more 3 screenshots
-    return queryset.filter(
-        name__in=[
-            *[f"screenshot{f'-{i}' if i > 0 else ''}.jpg" for i in range(4)],
-            *[f"screenshot{f'-{i}' if i > 0 else ''}.png" for i in range(4)],
-        ]
-    )
+    return queryset.filter(models.Q(name__icontains="screenshot"))
 
 
 @dataclass(frozen=True)

tests/sentry/api/endpoints/test_event_attachments.py

@@ -82,19 +82,26 @@ def test_is_screenshot(self):
             data={"fingerprint": ["group1"], "timestamp": min_ago}, project_id=self.project.id
         )
 
-        attachment1 = EventAttachment.objects.create(
+        file = File.objects.create(name="screenshot.png", type="image/png")
+        EventAttachment.objects.create(
+            event_id=event1.event_id,
+            project_id=event1.project_id,
+            file_id=file.id,
+            name=file.name,
+        )
+        file = File.objects.create(name="crash_screenshot.png")
+        EventAttachment.objects.create(
             event_id=event1.event_id,
             project_id=event1.project_id,
-            file_id=File.objects.create(name="screenshot.png", type="image/png").id,
-            name="screenshot.png",
+            file_id=file.id,
+            name=file.name,
         )
-        file = File.objects.create(name="screenshot-not.png", type="image/png")
+        file = File.objects.create(name="foo.png")
         EventAttachment.objects.create(
             event_id=event1.event_id,
             project_id=event1.project_id,
             file_id=file.id,
-            type=file.type,
-            name="screenshot-not.png",
+            name=file.name,
         )
 
         path = f"/api/0/projects/{event1.project.organization.slug}/{event1.project.slug}/events/{event1.event_id}/attachments/"
@@ -103,7 +110,8 @@ def test_is_screenshot(self):
             response = self.client.get(f"{path}?query=is:screenshot")
 
         assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["id"] == str(attachment1.id)
-        assert response.data[0]["mimetype"] == "image/png"
-        assert response.data[0]["event_id"] == attachment1.event_id
+        assert len(response.data) == 2
+        for attachment in response.data:
+            assert attachment["event_id"] == event1.event_id
+            # foo.png will not be included
+            assert attachment["name"] in ["screenshot.png", "crash_screenshot.png"]

tests/sentry/api/endpoints/test_group_attachments.py

@@ -66,31 +66,35 @@ def test_filter(self):
         assert len(response.data) == 1
         assert response.data[0]["id"] == str(attachment2.id)
 
-    def test_screenshot_filter(self):
+    def test_screenshot_across_groups(self):
         self.login_as(user=self.user)
 
-        attachment1 = self.create_attachment(type="event.attachment", file_name="screenshot.png")
-        self.create_attachment(type="event.attachment", file_name="screenshot-not.png")
-
-        with self.feature("organizations:event-attachments"):
-            response = self.client.get(self.path(screenshot=True))
-
-        assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["id"] == str(attachment1.id)
-
-    def test_second_screenshot_filter(self):
-        self.login_as(user=self.user)
-
-        attachment1 = self.create_attachment(type="event.attachment", file_name="screenshot.png")
-        self.create_attachment(type="event.attachment", file_name="screenshot-not.png")
+        min_ago = before_now(minutes=1).isoformat()
+        group1_event = self.store_event(
+            data={"fingerprint": ["group1"], "timestamp": min_ago}, project_id=self.project.id
+        )
+        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)
+        # This will not be included as name doesn't contain 'screenshot'
+        self.create_attachment(file_name="foo.png", event_id=group1_event.event_id)
+        group2_event = self.store_event(
+            data={"fingerprint": ["group2"], "timestamp": min_ago}, project_id=self.project.id
+        )
+        self.create_attachment(file_name="crash_screenshot.png", event_id=group2_event.event_id)
 
         with self.feature("organizations:event-attachments"):
             response = self.client.get(self.path(screenshot=True))
 
         assert response.status_code == 200, response.content
-        assert len(response.data) == 1
-        assert response.data[0]["id"] == str(attachment1.id)
+        assert len(response.data) == 3
+        for attachment in response.data:
+            # foo.png will not be included
+            assert attachment["name"] in [
+                "screenshot.png",
+                "screenshot-1.png",
+                "crash_screenshot.png",
+            ]
+            assert attachment["event_id"] in [group1_event.event_id, group2_event.event_id]
 
     def test_without_feature(self):
         self.login_as(user=self.user)