ref(deletions): Remove unnecessary Seer calls in endpoint (#93541)

During the deletion API call we were calling seer & a couple more
changes which the Group deletion model already does, thus, we don't need
to do it during the API call.

Prior to this change, if the Seer call failed, the deletion task would
not get scheduled, thus, many groups stay in a pending deletion state
and never deleted.

Fixes #93509 
Fixes
[ID-716](https://linear.app/getsentry/issue/ID-716/move-endpoint-code-to-the-deletion-task)

Author: armenzg

src/sentry/api/helpers/group_index/delete.py

@@ -13,13 +13,10 @@
 from sentry import audit_log, eventstream
 from sentry.api.base import audit_logger
 from sentry.deletions.tasks.groups import delete_groups as delete_groups_task
-from sentry.issues.grouptype import GroupCategory
 from sentry.models.group import Group, GroupStatus
-from sentry.models.grouphash import GroupHash
 from sentry.models.groupinbox import GroupInbox
 from sentry.models.project import Project
 from sentry.signals import issue_deleted
-from sentry.tasks.delete_seer_grouping_records import may_schedule_task_to_delete_hashes_from_seer
 from sentry.utils.audit import create_audit_entry
 
 from . import BULK_MUTATION_LIMIT, SearchFunction
@@ -47,12 +44,7 @@ def delete_group_list(
     # deterministic sort for sanity, and for very large deletions we'll
     # delete the "smaller" groups first
     group_list.sort(key=lambda g: (g.times_seen, g.id))
-    group_ids = []
-    error_ids = []
-    for g in group_list:
-        group_ids.append(g.id)
-        if g.issue_category == GroupCategory.ERROR:
-            error_ids.append(g.id)
+    group_ids = [g.id for g in group_list]
 
     transaction_id = uuid4().hex
     delete_logger.info(
@@ -84,13 +76,6 @@ def delete_group_list(
     # fails, we will still have a record of who requested the deletion.
     create_audit_entries(request, project, group_list, delete_type, transaction_id)
 
-    # Tell seer to delete grouping records for these groups
-    may_schedule_task_to_delete_hashes_from_seer(error_ids)
-
-    # Removing GroupHash rows prevents new events from associating to the groups
-    # we just deleted.
-    GroupHash.objects.filter(project_id=project.id, group__id__in=group_ids).delete()
-
     # We remove `GroupInbox` rows here so that they don't end up influencing queries for
     # `Group` instances that are pending deletion
     GroupInbox.objects.filter(project_id=project.id, group__id__in=group_ids).delete()

src/sentry/deletions/defaults/group.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 import os
 from collections import defaultdict
 from collections.abc import Mapping, Sequence
@@ -21,6 +22,8 @@
 from ..base import BaseDeletionTask, BaseRelation, ModelDeletionTask, ModelRelation
 from ..manager import DeletionTaskManager
 
+logger = logging.getLogger(__name__)
+
 # Group models that relate only to groups and not to events. We assume those to
 # be safe to delete/mutate within a single transaction for user-triggered
 # actions (delete/reprocess/merge/unmerge)
@@ -261,20 +264,6 @@ def delete_bulk(self, instance_list: Sequence[Group]) -> bool:
             return True
 
         self.mark_deletion_in_progress(instance_list)
-
-        error_group_ids = []
-        # XXX: If a group type has been removed, we shouldn't error here.
-        # Ideally, we should refactor `issue_category` to return None if the type is
-        # unregistered.
-        for group in instance_list:
-            try:
-                if group.issue_category == GroupCategory.ERROR:
-                    error_group_ids.append(group.id)
-            except InvalidGroupTypeError:
-                pass
-        # Tell seer to delete grouping records with these group hashes
-        may_schedule_task_to_delete_hashes_from_seer(error_group_ids)
-
         self._delete_children(instance_list)
 
         # Remove group objects with children removed.
@@ -291,6 +280,9 @@ def _delete_children(self, instance_list: Sequence[Group]) -> None:
 
         error_groups, issue_platform_groups = separate_by_group_category(instance_list)
 
+        # Tell seer to delete grouping records with these group hashes
+        may_schedule_task_to_delete_hashes_from_seer([group.id for group in error_groups])
+
         # If this isn't a retention cleanup also remove event data.
         if not os.environ.get("_SENTRY_CLEANUP"):
             if error_groups:

tests/sentry/api/helpers/test_group_index.py

@@ -1174,7 +1174,8 @@ def test_delete_groups_simple(self, send_robust: Mock):
             GroupHash.objects.create(project=self.project, group=group, hash=hashes[i])
             add_group_to_inbox(group, GroupInboxReason.NEW)
 
-        delete_groups(request, [self.project], self.organization.id)
+        with self.tasks():
+            delete_groups(request, [self.project], self.organization.id)
 
         assert (
             len(GroupHash.objects.filter(project_id=self.project.id, group_id__in=group_ids).all())
@@ -1205,7 +1206,8 @@ def test_delete_groups_deletes_seer_records_by_hash(
             GroupHash.objects.create(project=self.project, group=group, hash=hashes[i])
             add_group_to_inbox(group, GroupInboxReason.NEW)
 
-        delete_groups(request, [self.project], self.organization.id)
+        with self.tasks():
+            delete_groups(request, [self.project], self.organization.id)
 
         assert (
             len(GroupHash.objects.filter(project_id=self.project.id, group_id__in=group_ids).all())

tests/sentry/issues/endpoints/test_group_details.py

@@ -878,10 +878,7 @@ def test_delete_deferred(self) -> None:
 
         # Deletion was deferred, so it should still exist
         assert Group.objects.get(id=group.id).status == GroupStatus.PENDING_DELETION
-        # BUT the hash should be gone
-        assert not GroupHash.objects.filter(group_id=group.id).exists()
-
-        Group.objects.filter(id=group.id).update(status=GroupStatus.UNRESOLVED)
+        assert GroupHash.objects.filter(group_id=group.id).exists()
 
     def test_delete_and_tasks_run(self) -> None:
         self.login_as(user=self.user)

tests/sentry/issues/endpoints/test_organization_group_index.py

@@ -5602,7 +5602,7 @@ def create_n_groups(self, n: int, type: int | None = None) -> list[Group]:
     def assert_pending_deletion_groups(self, groups: Sequence[Group]) -> None:
         for group in groups:
             assert Group.objects.get(id=group.id).status == GroupStatus.PENDING_DELETION
-            assert not GroupHash.objects.filter(group_id=group.id).exists()
+            assert GroupHash.objects.filter(group_id=group.id).exists()
 
     def assert_deleted_groups(self, groups: Sequence[Group]) -> None:
         for group in groups:
@@ -5639,12 +5639,7 @@ def test_delete_by_id(self, mock_eventstream: MagicMock) -> None:
         )
 
         assert response.status_code == 204
-
-        assert Group.objects.get(id=group1.id).status == GroupStatus.PENDING_DELETION
-        assert not GroupHash.objects.filter(group_id=group1.id).exists()
-
-        assert Group.objects.get(id=group2.id).status == GroupStatus.PENDING_DELETION
-        assert not GroupHash.objects.filter(group_id=group2.id).exists()
+        self.assert_pending_deletion_groups([group1, group2])
 
         assert Group.objects.get(id=group3.id).status != GroupStatus.PENDING_DELETION
         assert GroupHash.objects.filter(group_id=group3.id).exists()

tests/snuba/api/endpoints/test_project_group_index.py

@@ -1505,7 +1505,6 @@ def create_groups(
     def assert_groups_being_deleted(self, groups: Sequence[Group]) -> None:
         for g in groups:
             assert Group.objects.get(id=g.id).status == GroupStatus.PENDING_DELETION
-            assert not GroupHash.objects.filter(group_id=g.id).exists()
 
         # This is necessary before calling the delete task
         Group.objects.filter(id__in=[g.id for g in groups]).update(status=GroupStatus.UNRESOLVED)
@@ -1635,25 +1634,3 @@ def test_bulk_delete_performance_issues(self):
             response = self.client.delete(url, format="json")
             assert response.status_code == 204
             self.assert_groups_are_gone(groups)
-
-    @patch("sentry.api.helpers.group_index.delete.may_schedule_task_to_delete_hashes_from_seer")
-    @patch("sentry.utils.audit.log_service.record_audit_log")
-    def test_audit_log_even_if_exception_raised(
-        self, mock_record_audit_log: Mock, mock_seer_delete: Mock
-    ):
-        """
-        Test that audit log is created even if an exception is raised after the audit log is created.
-        """
-        # Calling seer happens after creating the audit log entry
-        mock_seer_delete.side_effect = Exception("Seer error!")
-        group1 = self.create_group()
-        self.login_as(user=self.user)
-        url = f"{self.path}?id={group1.id}"
-        with self.tasks():
-            response = self.client.delete(url, format="json")
-            assert response.status_code == 500
-
-        self.assert_audit_log_entry([group1], mock_record_audit_log)
-
-        # They have been marked as pending deletion but the exception prevented their complete deletion
-        assert Group.objects.get(id=group1.id).status == GroupStatus.PENDING_DELETION