Просмотр исходного кода

feat(actor) Start dual writing for Rule (#69427)

Write to both the `owner` and `owner_user_id` and `owner_team_id`
columns. Part of the work to remove `Actor`.

Refs HC-1169
Mark Story 10 месяцев назад
Родитель
Сommit
0182c0c6e4

+ 5 - 1
src/sentry/api/endpoints/project_rule_details.py

@@ -326,7 +326,11 @@ class ProjectRuleDetailsEndpoint(RuleEndpoint):
             owner = data.get("owner")
             if owner:
                 try:
-                    kwargs["owner"] = owner.resolve_to_actor().id
+                    # TODO(mark) Use owner.resolve() instead when owner is removed.
+                    actor = owner.resolve_to_actor()
+                    kwargs["owner"] = actor.id
+                    kwargs["owner_user_id"] = actor.user_id
+                    kwargs["owner_team_id"] = actor.team_id
                 except (User.DoesNotExist, Team.DoesNotExist):
                     return Response(
                         "Could not resolve owner",

+ 5 - 1
src/sentry/api/endpoints/project_rules.py

@@ -825,7 +825,11 @@ class ProjectRulesEndpoint(ProjectEndpoint):
         owner = data.get("owner")
         if owner:
             try:
-                kwargs["owner"] = owner.resolve_to_actor().id
+                # TODO(mark) Use owner.resolve() intead when owner is removed.
+                actor = owner.resolve_to_actor()
+                kwargs["owner"] = actor.id
+                kwargs["owner_user_id"] = actor.user_id
+                kwargs["owner_team_id"] = actor.team_id
             except (User.DoesNotExist, Team.DoesNotExist):
                 return Response(
                     "Could not resolve owner",

+ 4 - 1
src/sentry/api/serializers/rest_framework/rule.py

@@ -182,7 +182,10 @@ class RuleSerializer(RuleSetSerializer):
         if self.validated_data.get("frequency"):
             rule.data["frequency"] = self.validated_data["frequency"]
         if self.validated_data.get("owner"):
-            rule.owner = self.validated_data["owner"].resolve_to_actor()
+            actor = self.validated_data["owner"].resolve_to_actor()
+            rule.owner_id = actor.id
+            rule.owner_user_id = actor.user_id
+            rule.owner_team_id = actor.team_id
         rule.save()
         return rule
 

+ 6 - 3
src/sentry/mediators/project_rules/creator.py

@@ -3,7 +3,6 @@ from rest_framework.request import Request
 
 from sentry.mediators.mediator import Mediator
 from sentry.mediators.param import Param
-from sentry.models.actor import Actor
 from sentry.models.project import Project
 from sentry.models.rule import Rule
 
@@ -11,7 +10,9 @@ from sentry.models.rule import Rule
 class Creator(Mediator):
     name = Param(str)
     environment = Param(int, required=False)
-    owner = Param(Actor, required=False)
+    owner = Param(int, required=False)
+    owner_team_id = Param(int, required=False)
+    owner_user_id = Param(int, required=False)
     project = Param(Project)
     action_match = Param(str)
     filter_match = Param(str, required=False)
@@ -40,7 +41,9 @@ class Creator(Mediator):
         }
         _kwargs = {
             "label": self.name,
-            "owner": Actor.objects.get(id=self.owner) if self.owner else None,
+            "owner_id": self.owner,
+            "owner_team_id": self.owner_team_id,
+            "owner_user_id": self.owner_user_id,
             "environment_id": self.environment or None,
             "project": self.project,
             "data": data,

+ 5 - 2
src/sentry/mediators/project_rules/updater.py

@@ -3,7 +3,6 @@ from rest_framework.request import Request
 
 from sentry.mediators.mediator import Mediator
 from sentry.mediators.param import Param
-from sentry.models.actor import Actor
 from sentry.models.project import Project
 from sentry.models.rule import Rule
 
@@ -12,6 +11,8 @@ class Updater(Mediator):
     rule = Param(Rule)
     name = Param(str, required=False)
     owner = Param(int, required=False)
+    owner_team_id = Param(int, required=False)
+    owner_user_id = Param(int, required=False)
     environment = Param(int, required=False)
     project = Param(Project)
     action_match = Param(str, required=False)
@@ -40,7 +41,9 @@ class Updater(Mediator):
             self.rule.label = self.name
 
     def _update_owner(self) -> None:
-        self.rule.owner = Actor.objects.get(id=self.owner) if self.owner else None
+        self.rule.owner_id = self.owner
+        self.rule.owner_user_id = self.owner_user_id
+        self.rule.owner_team_id = self.owner_team_id
 
     def _update_environment(self):
         self.rule.environment_id = self.environment

+ 7 - 0
src/sentry/models/rule.py

@@ -57,7 +57,9 @@ class Rule(Model):
         default=RuleSource.ISSUE,
         choices=RuleSource.as_choices(),
     )
+    # Deprecated. Use owner_user_id or owner_team instead.
     owner = FlexibleForeignKey("sentry.Actor", null=True, on_delete=models.SET_NULL)
+
     owner_user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="SET_NULL")
     owner_team = FlexibleForeignKey("sentry.Team", null=True, on_delete=models.SET_NULL)
 
@@ -110,11 +112,16 @@ class Rule(Model):
         return rv
 
     def save(self, *args, **kwargs):
+        self._validate_owner()
         rv = super().save(*args, **kwargs)
         cache_key = f"project:{self.project_id}:rules"
         cache.delete(cache_key)
         return rv
 
+    def _validate_owner(self):
+        if self.owner_id is not None and self.owner_team_id is None and self.owner_user_id is None:
+            raise ValueError("Rule with owner requires either owner_team or owner_user_id")
+
     def get_audit_log_data(self):
         return {
             "label": self.label,

+ 37 - 0
tests/sentry/api/endpoints/test_project_rule_details.py

@@ -534,6 +534,43 @@ class UpdateProjectRuleTest(ProjectRuleDetailsBaseTestCase):
         assert response.data["id"] == str(self.rule.id)
         assert_rule_from_payload(self.rule, payload)
 
+    def test_update_owner_type(self):
+        team = self.create_team(organization=self.organization)
+        actions = [{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}]
+        payload = {
+            "name": "hello world 2",
+            "owner": f"team:{team.id}",
+            "actionMatch": "all",
+            "actions": actions,
+            "conditions": self.first_seen_condition,
+        }
+        response = self.get_success_response(
+            self.organization.slug, self.project.slug, self.rule.id, status_code=200, **payload
+        )
+        assert response.data["id"] == str(self.rule.id)
+        assert response.data["owner"] == f"team:{team.id}"
+        rule = Rule.objects.get(id=response.data["id"])
+        assert rule.owner_id
+        assert rule.owner_team_id == team.id
+        assert rule.owner_user_id is None
+
+        payload = {
+            "name": "hello world 2",
+            "owner": f"user:{self.user.id}",
+            "actionMatch": "all",
+            "actions": actions,
+            "conditions": self.first_seen_condition,
+        }
+        response = self.get_success_response(
+            self.organization.slug, self.project.slug, self.rule.id, status_code=200, **payload
+        )
+        assert response.data["id"] == str(self.rule.id)
+        assert response.data["owner"] == f"user:{self.user.id}"
+        rule = Rule.objects.get(id=response.data["id"])
+        assert rule.owner_id
+        assert rule.owner_team_id is None
+        assert rule.owner_user_id == self.user.id
+
     def test_update_name(self):
         conditions = [
             {

+ 23 - 0
tests/sentry/api/endpoints/test_project_rules.py

@@ -541,6 +541,27 @@ class CreateProjectRuleTest(ProjectRuleBaseTestCase):
         )
         assert str(response.data["owner"][0]) == "Team is not a member of this organization"
 
+    def test_team_owner(self):
+        team = self.create_team(organization=self.organization)
+        response = self.get_success_response(
+            self.organization.slug,
+            self.project.slug,
+            name="test",
+            owner=f"team:{team.id}",
+            actionMatch="any",
+            filterMatch="any",
+            frequency=5,
+            actions=self.notify_event_action,
+            conditions=self.first_seen_condition,
+        )
+        assert response.status_code == 200
+        assert response.data["owner"] == f"team:{team.id}"
+
+        rule = Rule.objects.get(id=response.data["id"])
+        assert rule.owner_id
+        assert rule.owner_team_id == team.id
+        assert rule.owner_user_id is None
+
     def test_frequency_percent_validation(self):
         condition = {
             "id": "sentry.rules.conditions.event_frequency.EventFrequencyPercentCondition",
@@ -747,6 +768,8 @@ class CreateProjectRuleTest(ProjectRuleBaseTestCase):
             "actions": payload.get("actions", []),
             "frequency": payload.get("frequency"),
             "user_id": self.user.id,
+            "owner_user_id": self.user.id,
+            "owner_team_id": None,
             "uuid": "abc123",
         }
         call_args = mock_find_channel_id_for_alert_rule.call_args[1]["kwargs"]

+ 4 - 2
tests/sentry/api/endpoints/test_project_team_details.py

@@ -83,9 +83,11 @@ class ProjectTeamDetailsDeleteTest(ProjectTeamDetailsTest):
 
         # Associate rules with the team that also get deleted:
         # self.create_rule(name="test_rule", owner=f"team:{team.id}")
-        r1 = Rule.objects.create(label="test rule", project=project, owner=team.actor)
+        r1 = Rule.objects.create(
+            label="test rule", project=project, owner=team.actor, owner_team=team
+        )
         r2 = Rule.objects.create(
-            label="another test rule", project=another_project, owner=team.actor
+            label="another test rule", project=another_project, owner=team.actor, owner_team=team
         )
         ar1 = self.create_alert_rule(
             name="test alert rule", owner=team.actor.get_actor_tuple(), projects=[project]

+ 8 - 2
tests/sentry/api/endpoints/test_rule_snooze.py

@@ -15,7 +15,10 @@ from sentry.utils.actor import ActorTuple
 class BaseRuleSnoozeTest(APITestCase):
     def setUp(self):
         self.issue_alert_rule = Rule.objects.create(
-            label="test rule", project=self.project, owner=self.team.actor
+            label="test rule",
+            project=self.project,
+            owner=self.team.actor,
+            owner_team_id=self.team.id,
         )
         self.metric_alert_rule = self.create_alert_rule(
             organization=self.project.organization, projects=[self.project]
@@ -216,7 +219,10 @@ class PostRuleSnoozeTest(BaseRuleSnoozeTest):
         """Test that if a user doesn't belong to the team that can edit an issue alert rule, they can still mute it for just themselves."""
         other_team = self.create_team()
         other_issue_alert_rule = Rule.objects.create(
-            label="test rule", project=self.project, owner=other_team.actor
+            label="test rule",
+            project=self.project,
+            owner=other_team.actor,
+            owner_team_id=other_team.id,
         )
         data = {"target": "me"}
         response = self.get_response(

Некоторые файлы не были показаны из-за большого количества измененных файлов