fix(api): Less aggressive de-duping for activities by group (#33558)

Use previous record signature instead of global de-dupe.

WOR-1746

src/sentry/models/activity.py

@@ -22,21 +22,24 @@ if TYPE_CHECKING:
 
 class ActivityManager(BaseManager):
     def get_activities_for_group(self, group: "Group", num: int) -> Sequence["Group"]:
-        activity_items = set()
-        activity = []
+        activities = []
         activity_qs = self.filter(group=group).order_by("-datetime").select_related("user")
+
+        prev_sig = None
+        sig = None
         # we select excess so we can filter dupes
         for item in activity_qs[: num * 2]:
+            prev_sig = sig
             sig = (item.type, item.ident, item.user_id)
-            # TODO: we could just generate a signature (hash(text)) for notes
-            # so there's no special casing
+
             if item.type == ActivityType.NOTE.value:
-                activity.append(item)
-            elif sig not in activity_items:
-                activity_items.add(sig)
-                activity.append(item)
+                activities.append(item)
+                continue
+
+            if sig != prev_sig:
+                activities.append(item)
 
-        activity.append(
+        activities.append(
             Activity(
                 id=0,
                 project=group.project,
@@ -46,7 +49,7 @@ class ActivityManager(BaseManager):
             )
         )
 
-        return activity[:num]
+        return activities[:num]
 
     def create_group_activity(
         self,

tests/sentry/models/test_activity.py

@@ -0,0 +1,252 @@
+from typing import Sequence
+
+from sentry.models import Activity
+from sentry.testutils import TestCase
+from sentry.types.activity import ActivityType
+from sentry.utils.iterators import chunked
+
+
+class ActivityTest(TestCase):
+    def test_get_activities_for_group_none(self):
+        project = self.create_project(name="test_activities_group")
+        group = self.create_group(project)
+
+        act_for_group: Sequence[Activity] = Activity.objects.get_activities_for_group(
+            group=group, num=100
+        )
+        assert len(act_for_group) == 1
+        assert act_for_group[0].type == ActivityType.FIRST_SEEN.value
+
+    def test_get_activities_for_group_simple(self):
+        project = self.create_project(name="test_activities_group")
+        group = self.create_group(project)
+        user1 = self.create_user()
+
+        activities = [
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+        ]
+
+        act_for_group: Sequence[Activity] = Activity.objects.get_activities_for_group(
+            group=group, num=100
+        )
+        assert len(act_for_group) == 3
+        assert act_for_group[0] == activities[-1]
+        assert act_for_group[1] == activities[-2]
+        assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value
+
+    def test_get_activities_for_group_collapse_same(self):
+        project = self.create_project(name="test_activities_group")
+        group = self.create_group(project)
+        user1 = self.create_user()
+        user2 = self.create_user()
+        user3 = self.create_user()
+
+        activities = [
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.NOTE,
+                user=user1,
+                data={"text": "text", "mentions": []},
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.NOTE,
+                user=user2,
+                data={"text": "text", "mentions": []},
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.NOTE,
+                user=user3,
+                data={"text": "text", "mentions": []},
+                send_notification=False,
+            ),
+        ]
+
+        act_for_group: Sequence[Activity] = Activity.objects.get_activities_for_group(
+            group=group, num=100
+        )
+        assert len(act_for_group) == 7
+        assert act_for_group[0] == activities[-1]
+        assert act_for_group[1] == activities[-2]
+        assert act_for_group[2] == activities[-3]
+        assert act_for_group[3] == activities[-4]
+        assert act_for_group[4] == activities[1]
+        assert act_for_group[5] == activities[0]
+        assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value
+
+    def test_get_activities_for_group_flip_flop(self):
+        project = self.create_project(name="test_activities_group")
+        group = self.create_group(project)
+        user1 = self.create_user()
+        user2 = self.create_user()
+        user3 = self.create_user()
+
+        activities = [
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user2,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user2,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user3,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user3,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_UNRESOLVED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+            Activity.objects.create_group_activity(
+                group=group,
+                type=ActivityType.SET_IGNORED,
+                user=user1,
+                data=None,
+                send_notification=False,
+            ),
+        ]
+
+        act_for_group: Sequence[Activity] = Activity.objects.get_activities_for_group(
+            group=group, num=100
+        )
+
+        assert len(act_for_group) == len(activities) + 1
+        assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value
+
+        for pair in chunked(act_for_group[:-1], 2):
+            assert pair[0].type == ActivityType.SET_IGNORED.value
+            assert pair[1].type == ActivityType.SET_UNRESOLVED.value