diff --git a/src/sentry/api/helpers/group_index/__init__.py b/src/sentry/api/helpers/group_index/__init__.py index cf19a6371caca8..f2b54d6d87c7db 100644 --- a/src/sentry/api/helpers/group_index/__init__.py +++ b/src/sentry/api/helpers/group_index/__init__.py @@ -11,8 +11,6 @@ # complicated right now. BULK_MUTATION_LIMIT = 1000 -ACTIVITIES_COUNT = 100 - # XXX: The 1000 magic number for `max_hits` is an abstraction leak from # `sentry.api.paginator.BasePaginator.get_result`. SEARCH_MAX_HITS = 1000 @@ -20,7 +18,6 @@ SearchFunction = Callable[[Mapping[str, Any]], tuple[CursorResult[Group], Mapping[str, Any]]] __all__ = ( - "ACTIVITIES_COUNT", "BULK_MUTATION_LIMIT", "SEARCH_MAX_HITS", "delete_group_list", diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py index 4de82a14e6dcf5..cf6e77fa515411 100644 --- a/src/sentry/api/helpers/group_index/update.py +++ b/src/sentry/api/helpers/group_index/update.py @@ -60,7 +60,7 @@ from sentry.users.services.user_option import user_option_service from sentry.utils import metrics -from . import ACTIVITIES_COUNT, BULK_MUTATION_LIMIT, SearchFunction, delete_group_list +from . import BULK_MUTATION_LIMIT, SearchFunction, delete_group_list from .validators import GroupValidator, ValidationError logger = logging.getLogger(__name__) @@ -752,9 +752,7 @@ def prepare_response( if len(group_list) == 1: if res_type in (GroupResolution.Type.in_next_release, GroupResolution.Type.in_release): result["activity"] = serialize( - Activity.objects.get_activities_for_group( - group=group_list[0], num=ACTIVITIES_COUNT - ), + Activity.objects.get_activities_for_group(group=group_list[0]), acting_user, ) except UnboundLocalError: diff --git a/src/sentry/issues/endpoints/group_activities.py b/src/sentry/issues/endpoints/group_activities.py index 62ef99131fb39b..01b967b679525a 100644 --- a/src/sentry/issues/endpoints/group_activities.py +++ b/src/sentry/issues/endpoints/group_activities.py @@ -19,9 +19,9 @@ def get(self, request: Request, group: Group) -> Response: """ Retrieve all the Activities for a Group """ - activity = Activity.objects.get_activities_for_group(group, num=100) + activities = Activity.objects.get_activities_for_group(group) return Response( { - "activity": serialize(activity, request.user), + "activity": serialize(activities, request.user), } ) diff --git a/src/sentry/issues/endpoints/group_details.py b/src/sentry/issues/endpoints/group_details.py index 700f226b27b2e6..3b89f9414a9009 100644 --- a/src/sentry/issues/endpoints/group_details.py +++ b/src/sentry/issues/endpoints/group_details.py @@ -163,7 +163,7 @@ def get(self, request: Request, group: Group) -> Response: ) # TODO: these probably should be another endpoint - activity = Activity.objects.get_activities_for_group(group, 100) + activity = Activity.objects.get_activities_for_group(group) seen_by = self._get_seen_by(request, group) if "release" not in collapse: diff --git a/src/sentry/models/activity.py b/src/sentry/models/activity.py index 7c749e60c48da6..d68b3653d96b52 100644 --- a/src/sentry/models/activity.py +++ b/src/sentry/models/activity.py @@ -40,7 +40,13 @@ class ActivityManager(BaseManager["Activity"]): - def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity]: + def get_activities_for_group(self, group: Group) -> Sequence[Activity]: + """ + Returns a list of Activity objects for the given group, ordered by most recent first, + with duplicate consecutive activities (by type, ident, user_id, and data) filtered out, + except for notes which are always included. Also appends a synthetic FIRST_SEEN activity + at the end, using the group's initial priority if available. + """ activities = [] activity_qs = self.filter(group=group).order_by("-datetime") @@ -56,7 +62,7 @@ def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity] prev_sig = None sig = None # we select excess so we can filter dupes - for item in activity_qs[: num * 2]: + for item in activity_qs: prev_sig = sig sig = (item.type, item.ident, item.user_id, item.data) @@ -78,7 +84,7 @@ def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity] ) ) - return activities[:num] + return activities def create_group_activity( self, diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 7bb0c805029dfd..5f8710261e372b 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -1169,6 +1169,9 @@ def create_group(project, create_open_period=True, **kwargs): if "status" not in kwargs: kwargs["status"] = GroupStatus.UNRESOLVED kwargs["substatus"] = GroupSubStatus.NEW + if "status" in kwargs: + if kwargs["status"] == GroupStatus.UNRESOLVED and "substatus" not in kwargs: + kwargs["substatus"] = GroupSubStatus.NEW group = Group.objects.create(project=project, **kwargs) if create_open_period: diff --git a/tests/sentry/issues/endpoints/test_group_activities.py b/tests/sentry/issues/endpoints/test_group_activities.py index b8a45f59bfde9c..77b82d65669982 100644 --- a/tests/sentry/issues/endpoints/test_group_activities.py +++ b/tests/sentry/issues/endpoints/test_group_activities.py @@ -22,7 +22,7 @@ def test_endpoint_with_no_group_activities(self) -> None: def test_endpoint_with_group_activities(self) -> None: group = self.create_group(status=GroupStatus.UNRESOLVED) - for i in range(0, 4): + for _ in range(0, 100): Activity.objects.create( group=group, project=group.project, @@ -33,10 +33,7 @@ def test_endpoint_with_group_activities(self) -> None: self.login_as(user=self.user) url = f"/api/0/issues/{group.id}/activities/" - response = self.client.get( - url, - format="json", - ) + response = self.client.get(url, format="json") assert "activity" in response.data - assert len(response.data["activity"]) == 5 + assert len(response.data["activity"]) == 101 diff --git a/tests/sentry/models/test_activity.py b/tests/sentry/models/test_activity.py index 0ddd63bd7a9d77..d0400c3a73c694 100644 --- a/tests/sentry/models/test_activity.py +++ b/tests/sentry/models/test_activity.py @@ -17,7 +17,7 @@ def test_get_activities_for_group_none(self) -> None: project = self.create_project(name="test_activities_group") group = self.create_group(project) - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 1 assert act_for_group[0].type == ActivityType.FIRST_SEEN.value @@ -47,7 +47,7 @@ def test_get_activities_for_group_priority(self) -> None: ), ] - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 3 assert act_for_group[0] == activities[-1] assert act_for_group[1] == activities[-2] @@ -87,7 +87,7 @@ def test_get_activities_for_group_simple_priority_ff_on_dups(self) -> None: ), ] - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 3 assert act_for_group[0] == activities[-1] @@ -117,7 +117,7 @@ def test_get_activities_for_group_simple(self) -> None: ), ] - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 3 assert act_for_group[0] == activities[-1] assert act_for_group[1] == activities[-2] @@ -210,7 +210,7 @@ def test_get_activities_for_group_collapse_same(self) -> None: ), ] - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 7 assert act_for_group[0] == activities[-1] assert act_for_group[1] == activities[-2] @@ -314,7 +314,7 @@ def test_get_activities_for_group_flip_flop(self) -> None: ), ] - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == len(activities) + 1 assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value diff --git a/tests/snuba/api/endpoints/test_group_details.py b/tests/snuba/api/endpoints/test_group_details.py index 287b84a61fba9c..0311d47ba5d81a 100644 --- a/tests/snuba/api/endpoints/test_group_details.py +++ b/tests/snuba/api/endpoints/test_group_details.py @@ -239,7 +239,7 @@ def test_group_post_priority(self) -> None: assert response.status_code == 200, response.content assert response.data["priority"] == "high" - act_for_group = Activity.objects.get_activities_for_group(group=group, num=100) + act_for_group = Activity.objects.get_activities_for_group(group=group) assert len(act_for_group) == 2 assert act_for_group[0].type == ActivityType.SET_PRIORITY.value assert act_for_group[-1].type == ActivityType.FIRST_SEEN.value