fix(workflow): Serializer restructure - fixes actions with slack failing to save, and uses native serializer features better (#17300)

bin/load-mocks

@@ -671,6 +671,7 @@ def main(num_events=1, extra_events=False):
                     type=IncidentType.DETECTED,
                     title="My Incident",
                     query="",
+                    date_started=datetime.utcnow().replace(tzinfo=utc),
                     aggregation=QueryAggregations.TOTAL,
                     alert_rule=alert_rule,
                     projects=[project],

src/sentry/incidents/endpoints/serializers.py

@@ -2,10 +2,8 @@ from __future__ import absolute_import
 
 from datetime import timedelta
 
-import six
 import operator
 
-from enum import Enum
 from rest_framework import serializers
 
 from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
@@ -17,10 +15,11 @@ from sentry.incidents.logic import (
     create_alert_rule,
     create_alert_rule_trigger,
     create_alert_rule_trigger_action,
-    get_excluded_projects_for_alert_rule,
     update_alert_rule,
     update_alert_rule_trigger,
     update_alert_rule_trigger_action,
+    delete_alert_rule_trigger_action,
+    delete_alert_rule_trigger,
 )
 from sentry.incidents.models import (
     AlertRule,
@@ -28,7 +27,6 @@ from sentry.incidents.models import (
     AlertRuleTrigger,
     AlertRuleTriggerAction,
 )
-from sentry.models.project import Project
 from sentry.models.organizationmember import OrganizationMember
 from sentry.models.team import Team
 from sentry.models.user import User
@@ -142,24 +140,9 @@ class AlertRuleTriggerActionSerializer(CamelSnakeModelSerializer):
     def create(self, validated_data):
         return create_alert_rule_trigger_action(trigger=self.context["trigger"], **validated_data)
 
-    def _remove_unchanged_fields(self, instance, validated_data):
-        changed = False
-        if (
-            validated_data.get("type", instance.type) == AlertRuleTriggerAction.Type.SLACK.value
-            and validated_data["target_identifier"] != instance.target_display
-        ):
-            changed = True
-        for field_name, value in list(six.iteritems(validated_data)):
-            # Remove any fields that haven't actually changed
-            if isinstance(value, Enum):
-                value = value.value
-            if getattr(instance, field_name) != value:
-                changed = True
-                break
-        return validated_data if changed else {}
-
     def update(self, instance, validated_data):
-        validated_data = self._remove_unchanged_fields(instance, validated_data)
+        if "id" in validated_data:
+            validated_data.pop("id")
         return update_alert_rule_trigger_action(instance, **validated_data)
 
 
@@ -176,8 +159,7 @@ class AlertRuleTriggerSerializer(CamelSnakeModelSerializer):
     # TODO: These might be slow for many projects, since it will query for each
     # individually. If we find this to be a problem then we can look into batching.
     excluded_projects = serializers.ListField(child=ProjectField(), required=False)
-
-    actions = AlertRuleTriggerActionSerializer(many=True)
+    actions = serializers.ListField(required=True)
 
     class Meta:
         model = AlertRuleTrigger
@@ -203,35 +185,64 @@ class AlertRuleTriggerSerializer(CamelSnakeModelSerializer):
 
     def create(self, validated_data):
         try:
-            return create_alert_rule_trigger(
+            actions = validated_data.pop("actions")
+            alert_rule_trigger = create_alert_rule_trigger(
                 alert_rule=self.context["alert_rule"], **validated_data
             )
+            self._handle_action_updates(alert_rule_trigger, actions)
+
+            return alert_rule_trigger
         except AlertRuleTriggerLabelAlreadyUsedError:
             raise serializers.ValidationError("This label is already in use for this alert rule")
 
-    def _remove_unchanged_fields(self, instance, validated_data):
-        for field_name, value in list(six.iteritems(validated_data)):
-            # Remove any fields that haven't actually changed
-            if field_name == "excluded_projects":
-                excluded_slugs = [
-                    e.query_subscription.project.slug for e in instance.exclusions.all()
-                ]
-                if set(excluded_slugs) == set(project.slug for project in value):
-                    validated_data.pop(field_name)
-                continue
-            if isinstance(value, Enum):
-                value = value.value
-            if getattr(instance, field_name) == value:
-                validated_data.pop(field_name)
-        return validated_data
-
     def update(self, instance, validated_data):
-        validated_data = self._remove_unchanged_fields(instance, validated_data)
+        actions = validated_data.pop("actions")
+        if "id" in validated_data:
+            validated_data.pop("id")
         try:
-            return update_alert_rule_trigger(instance, **validated_data)
+            alert_rule_trigger = update_alert_rule_trigger(instance, **validated_data)
+            self._handle_action_updates(alert_rule_trigger, actions)
+            return alert_rule_trigger
         except AlertRuleTriggerLabelAlreadyUsedError:
             raise serializers.ValidationError("This label is already in use for this alert rule")
 
+    def _handle_action_updates(self, alert_rule_trigger, actions):
+        if actions is not None:
+            # Delete actions we don't have present in the updated data.
+            action_ids = [x["id"] for x in actions if "id" in x]
+            actions_to_delete = AlertRuleTriggerAction.objects.filter(
+                alert_rule_trigger=alert_rule_trigger
+            ).exclude(id__in=action_ids)
+            for action in actions_to_delete:
+                delete_alert_rule_trigger_action(action)
+
+            for action_data in actions:
+                if "integration_id" in action_data:
+                    action_data["integration"] = action_data.pop("integration_id")
+
+                if "id" in action_data:
+                    action_instance = AlertRuleTriggerAction.objects.get(
+                        alert_rule_trigger=alert_rule_trigger, id=action_data["id"]
+                    )
+                else:
+                    action_instance = None
+
+                action_serializer = AlertRuleTriggerActionSerializer(
+                    context={
+                        "alert_rule": alert_rule_trigger.alert_rule,
+                        "trigger": alert_rule_trigger,
+                        "organization": self.context["organization"],
+                        "access": self.context["access"],
+                    },
+                    instance=action_instance,
+                    data=action_data,
+                )
+
+                if action_serializer.is_valid():
+                    action_serializer.save()
+                else:
+                    raise serializers.ValidationError(action_serializer.errors)
+
 
 class AlertRuleSerializer(CamelSnakeModelSerializer):
     """
@@ -245,7 +256,7 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
     # individually. If we find this to be a problem then we can look into batching.
     projects = serializers.ListField(child=ProjectField(), required=False)
     excluded_projects = serializers.ListField(child=ProjectField(), required=False)
-    triggers = AlertRuleTriggerSerializer(many=True, required=True)
+    triggers = serializers.ListField(required=True)
 
     class Meta:
         model = AlertRule
@@ -296,12 +307,12 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
                     raise serializers.ValidationError(
                         'First trigger must be labeled "%s"' % (CRITICAL_TRIGGER_LABEL)
                     )
-                if critical["threshold_type"] == AlertRuleThresholdType.ABOVE:
+                if critical["threshold_type"] == AlertRuleThresholdType.ABOVE.value:
                     alert_op, trigger_error = (
                         operator.lt,
                         "alert threshold must be above resolution threshold",
                     )
-                elif critical["threshold_type"] == AlertRuleThresholdType.BELOW:
+                elif critical["threshold_type"] == AlertRuleThresholdType.BELOW.value:
                     alert_op, trigger_error = (
                         operator.gt,
                         "alert threshold must be below resolution threshold",
@@ -326,14 +337,14 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
                             "Must have matching threshold types (i.e. critical and warning triggers must both be an upper or lower bound)"
                         )
 
-                    if critical["threshold_type"] == AlertRuleThresholdType.ABOVE:
+                    if critical["threshold_type"] == AlertRuleThresholdType.ABOVE.value:
                         alert_op, resolve_op = operator.lt, operator.lt
                         alert_error = (
                             "Critical trigger must have an alert threshold above warning trigger"
                         )
                         resolve_error = "Critical trigger must have a resolution threshold above (or equal to) warning trigger"
                         trigger_error = "alert threshold must be above resolution threshold"
-                    elif critical["threshold_type"] == AlertRuleThresholdType.BELOW:
+                    elif critical["threshold_type"] == AlertRuleThresholdType.BELOW.value:
                         alert_op, resolve_op = operator.gt, operator.gt
                         alert_error = (
                             "Critical trigger must have an alert threshold below warning trigger"
@@ -375,45 +386,57 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
 
         return data
 
-    def _remove_unchanged_fields(self, instance, validated_data):
-        for field_name, value in list(six.iteritems(validated_data)):
-            # Remove any fields that haven't actually changed
-            if field_name == "triggers" or field_name == "environment":
-                continue  # No removal for triggers or environment
-            if field_name == "projects":
-                project_slugs = Project.objects.filter(
-                    querysubscription__alert_rules=instance
-                ).values_list("slug", flat=True)
-                if set(project_slugs) == set([project.slug for project in value]):
-                    validated_data.pop(field_name)
-                continue
-            if field_name == "excluded_projects":
-                excluded_slugs = [
-                    p.project.slug for p in get_excluded_projects_for_alert_rule(instance)
-                ]
-                if set(excluded_slugs) == set(project.slug for project in value):
-                    validated_data.pop(field_name)
-                continue
-            if isinstance(value, Enum):
-                value = value.value
-            if getattr(instance, field_name) == value:
-                validated_data.pop(field_name)
-        return validated_data
-
     def create(self, validated_data):
         try:
-            # TODO: Remove this, just temporary while we're supporting both fields.
-            if "aggregation" not in validated_data:
-                raise serializers.ValidationError("aggregation is required")
-
+            triggers = validated_data.pop("triggers")
             alert_rule = create_alert_rule(
                 organization=self.context["organization"], **validated_data
             )
+            self._handle_trigger_updates(alert_rule, triggers)
             return alert_rule
         except AlertRuleNameAlreadyUsedError:
             raise serializers.ValidationError("This name is already in use for this project")
 
     def update(self, instance, validated_data):
-        validated_data = self._remove_unchanged_fields(instance, validated_data)
-        alert_rule = update_alert_rule(instance, **validated_data)
-        return alert_rule
+        triggers = validated_data.pop("triggers")
+        if "id" in validated_data:
+            validated_data.pop("id")
+        try:
+            alert_rule = update_alert_rule(instance, **validated_data)
+            self._handle_trigger_updates(alert_rule, triggers)
+            return alert_rule
+        except AlertRuleNameAlreadyUsedError:
+            raise serializers.ValidationError("This name is already in use for this project")
+
+    def _handle_trigger_updates(self, alert_rule, triggers):
+        if triggers is not None:
+            # Delete triggers we don't have present in the incoming data
+            trigger_ids = [x["id"] for x in triggers if "id" in x]
+            triggers_to_delete = AlertRuleTrigger.objects.filter(alert_rule=alert_rule).exclude(
+                id__in=trigger_ids
+            )
+            for trigger in triggers_to_delete:
+                delete_alert_rule_trigger(trigger)
+
+            for trigger_data in triggers:
+                if "id" in trigger_data:
+                    trigger_instance = AlertRuleTrigger.objects.get(
+                        alert_rule=alert_rule, id=trigger_data["id"]
+                    )
+                else:
+                    trigger_instance = None
+
+                trigger_serializer = AlertRuleTriggerSerializer(
+                    context={
+                        "alert_rule": alert_rule,
+                        "organization": self.context["organization"],
+                        "access": self.context["access"],
+                    },
+                    instance=trigger_instance,
+                    data=trigger_data,
+                )
+
+                if trigger_serializer.is_valid():
+                    trigger_serializer.save()
+                else:
+                    raise serializers.ValidationError(trigger_serializer.errors)

src/sentry/incidents/logic.py

@@ -2,7 +2,6 @@ from __future__ import absolute_import
 
 from collections import defaultdict
 from datetime import timedelta
-from rest_framework import serializers
 from uuid import uuid4
 
 import six
@@ -452,7 +451,6 @@ def create_alert_rule(
     environment=None,
     include_all_projects=False,
     excluded_projects=None,
-    triggers=None,
 ):
     """
     Creates an alert rule for an organization.
@@ -472,7 +470,6 @@ def create_alert_rule(
     from this organization
     :param excluded_projects: List of projects to exclude if we're using
     `include_all_projects`.
-    :param actions: A list of alert rule triggers for this for this rule
 
     :return: The created `AlertRule`
     """
@@ -509,10 +506,6 @@ def create_alert_rule(
             for e in environment:
                 AlertRuleEnvironment.objects.create(alert_rule=alert_rule, environment=e)
 
-        if triggers:
-            for trigger_data in triggers:
-                create_alert_rule_trigger(alert_rule=alert_rule, **trigger_data)
-
         subscribe_projects_to_alert_rule(alert_rule, projects)
 
     return alert_rule
@@ -529,7 +522,6 @@ def update_alert_rule(
     threshold_period=None,
     include_all_projects=None,
     excluded_projects=None,
-    triggers=None,
 ):
     """
     Updates an alert rule.
@@ -653,28 +645,6 @@ def update_alert_rule(
         else:
             AlertRuleEnvironment.objects.filter(alert_rule=alert_rule).delete()
 
-        if triggers is not None:
-            # Delete triggers we don't have present in the updated data.
-            trigger_ids = [x["id"] for x in triggers if "id" in x]
-            AlertRuleTrigger.objects.filter(alert_rule=alert_rule).exclude(
-                id__in=trigger_ids
-            ).delete()
-
-            for trigger_data in triggers:
-                try:
-                    if "id" in trigger_data:
-                        trigger_instance = AlertRuleTrigger.objects.get(
-                            alert_rule=alert_rule, id=trigger_data["id"]
-                        )
-                        trigger_data.pop("id")
-                        update_alert_rule_trigger(trigger_instance, **trigger_data)
-                    else:
-                        create_alert_rule_trigger(alert_rule=alert_rule, **trigger_data)
-                except AlertRuleTriggerLabelAlreadyUsedError:
-                    raise serializers.ValidationError(
-                        "This trigger label is already in use for this alert rule"
-                    )
-
         if existing_subs and (
             query is not None or aggregation is not None or time_window is not None
         ):
@@ -764,7 +734,6 @@ def create_alert_rule_trigger(
     alert_threshold,
     resolve_threshold=None,
     excluded_projects=None,
-    actions=None,
 ):
     """
     Creates a new AlertRuleTrigger
@@ -777,7 +746,6 @@ def create_alert_rule_trigger(
     resolve the alert
     :param excluded_projects: A list of Projects that should be excluded from this
     trigger. These projects must be associate with the alert rule already
-    :param actions: A list of alert rule trigger actions for this trigger
     :return: The created AlertRuleTrigger
     """
     if AlertRuleTrigger.objects.filter(alert_rule=alert_rule, label=label).exists():
@@ -802,10 +770,6 @@ def create_alert_rule_trigger(
             ]
             AlertRuleTriggerExclusion.objects.bulk_create(new_exclusions)
 
-        if actions:
-            for action_data in actions:
-                create_alert_rule_trigger_action(trigger=trigger, **action_data)
-
     return trigger
 
 
@@ -816,7 +780,6 @@ def update_alert_rule_trigger(
     alert_threshold=None,
     resolve_threshold=None,
     excluded_projects=None,
-    actions=None,
 ):
     """
     :param trigger: The AlertRuleTrigger to update
@@ -828,7 +791,6 @@ def update_alert_rule_trigger(
     resolve the alert
     :param excluded_projects: A list of Projects that should be excluded from this
     trigger. These projects must be associate with the alert rule already
-    :param actions: A list of alert rule trigger actions for this trigger
     :return: The updated AlertRuleTrigger
     """
 
@@ -879,22 +841,6 @@ def update_alert_rule_trigger(
             ]
             AlertRuleTriggerExclusion.objects.bulk_create(new_exclusions)
 
-        if actions is not None:
-            # Delete actions we don't have present in the updated data.
-            action_ids = [x["id"] for x in actions if "id" in x]
-            AlertRuleTriggerAction.objects.filter(alert_rule_trigger=trigger).exclude(
-                id__in=action_ids
-            ).delete()
-
-            for action_data in actions:
-                if "id" in action_data:
-                    action_instance = AlertRuleTriggerAction.objects.get(
-                        alert_rule_trigger=trigger, id=action_data["id"]
-                    )
-                    action_data.pop("id")
-                    update_alert_rule_trigger_action(action_instance, **action_data)
-                else:
-                    create_alert_rule_trigger_action(trigger=trigger, **action_data)
     return trigger
 
 

tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

@@ -268,7 +268,7 @@ class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase, APITestCase):
         # We had/have logic to remove unchanged fields from the serializer before passing them to update
         # It is possible with an error here that sending an update without changing the field
         # could nullify it. So we make sure it's not getting nullified here.
-        # we can an update with all the same fields, and it comes back as 125 instead of None.
+        # we can an update with all the same fields, and it comes back as 75 instead of None.
         with self.feature("organizations:incidents"):
             resp = self.get_valid_response(
                 self.organization.slug, alert_rule.id, **serialized_alert_rule

tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py

@@ -241,7 +241,7 @@ class AlertRuleCreateEndpointTest(APITestCase):
             "triggers": [
                 {
                     "label": "critical",
-                    "alertThreshold": 200,
+                    "alertThreshold": 75,
                     "resolveThreshold": 100,
                     "thresholdType": 1,
                 }
@@ -252,7 +252,7 @@ class AlertRuleCreateEndpointTest(APITestCase):
             resp = self.get_valid_response(
                 self.organization.slug, status_code=400, **rule_one_trigger_only_critical_no_action
             )
-            assert resp.data == {"triggers": [{"actions": [u"This field is required."]}]}
+            assert resp.data == {u"nonFieldErrors": [u'"critical" trigger must have an action.']}
 
     def test_invalid_projects(self):
         self.create_member(

tests/sentry/incidents/endpoints/test_serializers.py

@@ -12,12 +12,7 @@ from sentry.incidents.endpoints.serializers import (
     string_to_action_type,
     string_to_action_target_type,
 )
-from sentry.incidents.logic import (
-    create_alert_rule,
-    create_alert_rule_trigger,
-    create_alert_rule_trigger_action,
-    InvalidTriggerActionError,
-)
+from sentry.incidents.logic import create_alert_rule_trigger, InvalidTriggerActionError
 from sentry.incidents.models import (
     AlertRuleThresholdType,
     AlertRuleTriggerAction,
@@ -220,116 +215,6 @@ class TestAlertRuleSerializer(TestCase):
 
         assert serializer.is_valid(), serializer.errors
 
-    def _run_changed_fields_test(self, alert_rule, params, expected):
-        test_params = self.valid_params.copy()
-        test_params.update(params)
-
-        expected.update({"triggers": self.Any(list)})
-        serializer = AlertRuleSerializer(
-            context=self.context, instance=alert_rule, data=test_params, partial=True
-        )
-
-        assert serializer.is_valid(), serializer.errors
-
-        assert (
-            serializer._remove_unchanged_fields(alert_rule, serializer.validated_data) == expected
-        )
-
-    def test_remove_unchanged_fields(self):
-        a_project = self.create_project()
-        projects = [self.project, a_project]
-
-        test_params = self.valid_params.copy()
-        test_params.update({"projects": [self.project.slug, a_project.slug]})
-
-        name = "hello"
-        query = "level:error"
-        aggregation = QueryAggregations.TOTAL
-        time_window = 10
-        alert_rule = create_alert_rule(
-            self.organization, projects, name, query, aggregation, time_window, 1
-        )
-        self._run_changed_fields_test(
-            alert_rule,
-            {
-                "projects": [p.slug for p in projects],
-                "name": name,
-                "query": query,
-                "aggregation": aggregation.value,
-                "time_window": time_window,
-            },
-            {},
-        )
-
-        temp_params = test_params.copy()
-        temp_params.update({"projects": [p.slug for p in projects]})
-        self._run_changed_fields_test(alert_rule, temp_params, {})
-
-        self._run_changed_fields_test(
-            alert_rule, {"projects": [self.project.slug]}, {"projects": [self.project]}
-        )
-
-        temp_params = test_params.copy()
-        temp_params.update({"name": name})
-        self._run_changed_fields_test(alert_rule, temp_params, {})
-
-        temp_params = test_params.copy()
-        temp_params.update({"name": "a name"})
-        self._run_changed_fields_test(alert_rule, temp_params, {"name": "a name"})
-
-        temp_params = test_params.copy()
-        temp_params.update({"query": query})
-        self._run_changed_fields_test(alert_rule, temp_params, {})
-
-        temp_params = test_params.copy()
-        temp_params.update({"query": "level:warning"})
-        self._run_changed_fields_test(alert_rule, temp_params, {"query": "level:warning"})
-
-        temp_params = test_params.copy()
-        temp_params.update({"aggregation": aggregation.value})
-        self._run_changed_fields_test(alert_rule, temp_params, {})
-
-        temp_params = test_params.copy()
-        temp_params.update({"aggregation": 1})
-        self._run_changed_fields_test(
-            alert_rule, temp_params, {"aggregation": QueryAggregations.UNIQUE_USERS}
-        )
-
-        temp_params = test_params.copy()
-        temp_params.update({"time_window": time_window})
-        self._run_changed_fields_test(alert_rule, temp_params, {})
-
-        temp_params = test_params.copy()
-        temp_params.update({"time_window": 20})
-        self._run_changed_fields_test(alert_rule, temp_params, {"time_window": 20})
-
-    def test_remove_unchanged_fields_include_all(self):
-        projects = [self.project]
-        excluded = [self.create_project()]
-        alert_rule = self.create_alert_rule(
-            name="hello", include_all_projects=True, excluded_projects=excluded
-        )
-
-        self._run_changed_fields_test(
-            alert_rule,
-            {"include_all_projects": True, "excluded_projects": [e.slug for e in excluded]},
-            {},
-        )
-
-        self._run_changed_fields_test(
-            alert_rule, {"excluded_projects": [e.slug for e in excluded]}, {}
-        )
-        self._run_changed_fields_test(
-            alert_rule,
-            {"excluded_projects": [p.slug for p in projects]},
-            {"excluded_projects": projects},
-        )
-
-        self._run_changed_fields_test(alert_rule, {"include_all_projects": True}, {})
-        self._run_changed_fields_test(
-            alert_rule, {"include_all_projects": False}, {"include_all_projects": False}
-        )
-
 
 class TestAlertRuleTriggerSerializer(TestCase):
     @fixture
@@ -391,65 +276,6 @@ class TestAlertRuleTriggerSerializer(TestCase):
         )
         self.run_fail_validation_test({"thresholdType": 50}, {"thresholdType": invalid_values})
 
-    def _run_changed_fields_test(self, trigger, params, expected):
-        serializer = AlertRuleTriggerSerializer(
-            context=self.context, instance=trigger, data=params, partial=True
-        )
-        assert serializer.is_valid(), serializer.errors
-        assert serializer._remove_unchanged_fields(trigger, serializer.validated_data) == expected
-
-    def test_remove_unchanged_fields(self):
-        excluded_projects = [self.project]
-        label = "hello"
-        threshold_type = AlertRuleThresholdType.ABOVE
-        alert_threshold = 1000
-        resolve_threshold = 400
-        trigger = create_alert_rule_trigger(
-            self.alert_rule,
-            label,
-            threshold_type,
-            alert_threshold,
-            resolve_threshold,
-            excluded_projects=excluded_projects,
-        )
-
-        self._run_changed_fields_test(
-            trigger,
-            {
-                "label": label,
-                "threshold_type": threshold_type.value,
-                "alert_threshold": alert_threshold,
-                "resolve_threshold": resolve_threshold,
-                "excludedProjects": [p.slug for p in excluded_projects],
-            },
-            {},
-        )
-
-        self._run_changed_fields_test(trigger, {"label": label}, {})
-        self._run_changed_fields_test(trigger, {"label": "a name"}, {"label": "a name"})
-
-        self._run_changed_fields_test(trigger, {"threshold_type": threshold_type.value}, {})
-        self._run_changed_fields_test(
-            trigger, {"threshold_type": 1}, {"threshold_type": AlertRuleThresholdType.BELOW}
-        )
-
-        self._run_changed_fields_test(trigger, {"alert_threshold": alert_threshold}, {})
-        self._run_changed_fields_test(trigger, {"alert_threshold": 2000}, {"alert_threshold": 2000})
-
-        self._run_changed_fields_test(trigger, {"resolve_threshold": resolve_threshold}, {})
-        self._run_changed_fields_test(
-            trigger, {"resolve_threshold": 200}, {"resolve_threshold": 200}
-        )
-
-        self._run_changed_fields_test(
-            trigger, {"excluded_projects": [p.slug for p in excluded_projects]}, {}
-        )
-        self._run_changed_fields_test(
-            trigger,
-            {"excluded_projects": [self.other_project.slug]},
-            {"excluded_projects": [self.other_project]},
-        )
-
 
 class TestAlertRuleTriggerActionSerializer(TestCase):
     @fixture
@@ -519,66 +345,6 @@ class TestAlertRuleTriggerActionSerializer(TestCase):
         ]
         self.run_fail_validation_test({"targetType": 50}, {"targetType": invalid_values})
 
-    def _run_changed_fields_test(self, trigger, params, expected):
-        serializer = AlertRuleTriggerActionSerializer(
-            context=self.context, instance=trigger, data=params, partial=True
-        )
-        assert serializer.is_valid(), serializer.errors
-        assert serializer._remove_unchanged_fields(trigger, serializer.validated_data) == expected
-
-    def test_remove_unchanged_fields(self):
-        type = AlertRuleTriggerAction.Type.EMAIL
-        target_type = AlertRuleTriggerAction.TargetType.USER
-        identifier = six.text_type(self.user.id)
-        action = create_alert_rule_trigger_action(self.trigger, type, target_type, identifier)
-
-        self._run_changed_fields_test(
-            action,
-            {
-                "type": AlertRuleTriggerAction.get_registered_type(type).slug,
-                "target_type": action_target_type_to_string[target_type],
-                "target_identifier": identifier,
-            },
-            {},
-        )
-
-        integration = Integration.objects.create(external_id="1", provider="slack", metadata={})
-
-        self._run_changed_fields_test(
-            action,
-            {
-                "type": AlertRuleTriggerAction.get_registered_type(
-                    AlertRuleTriggerAction.Type.SLACK
-                ).slug,
-                "targetIdentifier": identifier,
-                "targetType": action_target_type_to_string[
-                    AlertRuleTriggerAction.TargetType.SPECIFIC
-                ],
-                "integration": integration.id,
-            },
-            {
-                "type": AlertRuleTriggerAction.Type.SLACK,
-                "integration": integration,
-                "target_identifier": identifier,
-                "target_type": AlertRuleTriggerAction.TargetType.SPECIFIC,
-            },
-        )
-
-        new_team = self.create_team(self.organization)
-        self._run_changed_fields_test(
-            action,
-            {
-                "type": AlertRuleTriggerAction.get_registered_type(type).slug,
-                "target_type": action_target_type_to_string[AlertRuleTriggerAction.TargetType.TEAM],
-                "target_identifier": six.text_type(new_team.id),
-            },
-            {
-                "type": type,
-                "target_type": AlertRuleTriggerAction.TargetType.TEAM,
-                "target_identifier": six.text_type(new_team.id),
-            },
-        )
-
     def test_user_perms(self):
         self.run_fail_validation_test(
             {