Browse Source

ref(auto-assignment): Remove group check for force-assignment (#65486)

Now that the only caller of force assignment has been updated to send
`force_autoassign` we no longer need the implicit assumption when no
`event` is present (see
https://github.com/getsentry/getsentry/pull/13006). Going forward, to
use this function for forcing auto-assignment, `force_autoassign` must
be set.
Leander Rodrigues 1 year ago
parent
commit
a68fd6e1a1
2 changed files with 13 additions and 34 deletions
  1. 3 10
      src/sentry/models/projectownership.py
  2. 10 24
      tests/sentry/models/test_projectownership.py

+ 3 - 10
src/sentry/models/projectownership.py

@@ -256,13 +256,6 @@ class ProjectOwnership(Model):
         from sentry.models.user import User
         from sentry.services.hybrid_cloud.user import RpcUser
 
-        enable_force_autoassign = False
-        # Force auto-assign will override an existing manual assignment.
-        if (event is None and logging_extra is None) or force_autoassign:
-            # TODO(Leander): Remove this event/logging_details check. This is currently only missing
-            # from getsentry's admin tools, so we can change that caller to set force_autoassign instead
-            enable_force_autoassign = True
-
         if logging_extra is None:
             logging_extra = {}
 
@@ -314,7 +307,7 @@ class ProjectOwnership(Model):
             ).order_by("-datetime")
             if activity:
                 auto_assigned = activity[0].data.get("integration")
-                if not auto_assigned and not enable_force_autoassign:
+                if not auto_assigned and not force_autoassign:
                     logger.info("autoassignment.post_manual_assignment", extra=logging_extra)
                     return
             if (
@@ -327,9 +320,9 @@ class ProjectOwnership(Model):
                 assignment = GroupAssignee.objects.assign(
                     group,
                     owner,
-                    create_only=not enable_force_autoassign,
+                    create_only=not force_autoassign,
                     extra=activity_details,
-                    force_autoassign=enable_force_autoassign,
+                    force_autoassign=force_autoassign,
                 )
 
                 if assignment["new_assignment"] or assignment["updated_assignment"]:

+ 10 - 24
tests/sentry/models/test_projectownership.py

@@ -308,7 +308,7 @@ class ProjectOwnershipTestCase(TestCase):
             context={"rule": str(rule_c)},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.team_id == self.team.id
@@ -357,7 +357,7 @@ class ProjectOwnershipTestCase(TestCase):
             context={"commitId": self.commit.id},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.user_id == self.user2.id
@@ -389,7 +389,7 @@ class ProjectOwnershipTestCase(TestCase):
             context={"rule": str(rule_c)},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.team_id == self.team.id
@@ -399,7 +399,7 @@ class ProjectOwnershipTestCase(TestCase):
         GroupAssignee.objects.assign(self.event.group, self.user)
 
         # ensure the issue was not reassigned
-        ProjectOwnership.handle_auto_assignment(self.project.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.user_id == self.user.id
@@ -434,7 +434,7 @@ class ProjectOwnershipTestCase(TestCase):
         GroupAssignee.objects.create(group=self.event.group, project=self.project, team=self.team)
 
         # ensure we skip calling assign
-        ProjectOwnership.handle_auto_assignment(self.project.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project.id, self.event)
         mock_assign.assert_not_called()
 
     def test_handle_auto_assignment_when_codeowners_and_issueowners_exists(self):
@@ -481,14 +481,14 @@ class ProjectOwnershipTestCase(TestCase):
             context={"rule": str(rule_c)},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event)
         assert len(GroupAssignee.objects.all()) == 0
 
         # Turn on auto assignment
         self.ownership.auto_assignment = True
         self.ownership.suspect_committer_auto_assignment = True
         self.ownership.save()
-        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.team_id == self.team.id
@@ -565,14 +565,14 @@ class ProjectOwnershipTestCase(TestCase):
             context={"rule": str(rule_c)},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event)
         assert len(GroupAssignee.objects.all()) == 0
 
         # Turn on auto assignment
         self.ownership.auto_assignment = True
         self.ownership.suspect_committer_auto_assignment = True
         self.ownership.save()
-        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project2.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.user_id == self.user2.id
@@ -627,7 +627,7 @@ class ProjectOwnershipTestCase(TestCase):
             context={"rule": str(rule_a)},
         )
 
-        ProjectOwnership.handle_auto_assignment(self.project.id, self.event, logging_extra={})
+        ProjectOwnership.handle_auto_assignment(self.project.id, self.event)
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.team_id == self.team.id
@@ -646,26 +646,12 @@ class ProjectOwnershipTestCase(TestCase):
         ProjectOwnership.handle_auto_assignment(
             self.project.id,
             group=self.event.group,
-            logging_extra={},
             force_autoassign=True,
         )
         assert len(GroupAssignee.objects.all()) == 1
         assignee = GroupAssignee.objects.get(group=self.event.group)
         assert assignee.team_id == self.team.id
 
-        # TODO(Leander): Remove after caller in getsentry uses `force_autoassign`
-        # Manually assign the group to someone else (again)
-        GroupAssignee.objects.assign(self.event.group, self.user)
-        assert len(GroupAssignee.objects.all()) == 1
-        assignee = GroupAssignee.objects.get(group=self.event.group)
-        assert assignee.user_id == self.user.id
-
-        # Run force auto-assignment without explicit parameter
-        ProjectOwnership.handle_auto_assignment(self.project.id, group=self.event.group)
-        assert len(GroupAssignee.objects.all()) == 1
-        assignee = GroupAssignee.objects.get(group=self.event.group)
-        assert assignee.team_id == self.team.id
-
     @patch("sentry.models.groupowner.GroupOwner")
     def test_update_modifies_cache(self, mock_group_owner):
         rule_a = Rule(Matcher("path", "*.py"), [Owner("team", self.team.slug)])