condition -> type, add action.types, update tests

Author: ceorourke

src/sentry/workflow_engine/migration_helpers/alert_rule.py

@@ -7,6 +7,7 @@
     AlertRuleTrigger,
     AlertRuleTriggerAction,
 )
+from sentry.integrations.services.integration import integration_service
 from sentry.snuba.models import QuerySubscription, SnubaQuery
 from sentry.users.services.user import RpcUser
 from sentry.workflow_engine.models import (
@@ -24,12 +25,28 @@
     Workflow,
     WorkflowDataConditionGroup,
 )
-from sentry.workflow_engine.models.data_condition import Condition, ConditionType
+from sentry.workflow_engine.models.data_condition import Condition
 from sentry.workflow_engine.types import DetectorPriorityLevel
 
 logger = logging.getLogger(__name__)
 
 
+def get_action_type(alert_rule_trigger_action: AlertRuleTriggerAction) -> str | None:
+    if alert_rule_trigger_action.sentry_app_id:
+        return Action.Type.SENTRY_APP
+
+    elif alert_rule_trigger_action.integration_id:
+        integration = integration_service.get_integration(
+            integration_id=alert_rule_trigger_action.integration_id
+        )
+        for type in Action.Type.choices:
+            if type[0] == integration.provider:
+                return type[1]
+        return None
+    else:
+        return Action.Type.EMAIL
+
+
 def migrate_metric_action(
     alert_rule_trigger_action: AlertRuleTriggerAction,
 ) -> tuple[Action, DataConditionGroupAction] | None:
@@ -44,19 +61,28 @@ def migrate_metric_action(
         )
         return None
 
+    action_type = get_action_type(alert_rule_trigger_action)
+    if not action_type:
+        logger.warning(
+            "Could not find a matching Action.Type for the trigger action",
+            extra={"alert_rule_trigger_action_id": alert_rule_trigger_action.id},
+        )
+        return None
+
     data = {
         "type": alert_rule_trigger_action.type,
         "sentry_app_id": alert_rule_trigger_action.sentry_app_id,
         "sentry_app_config": alert_rule_trigger_action.sentry_app_config,
     }
     action = Action.objects.create(
         required=False,
-        type=Action.Type.NOTIFICATION,  # TODO: this is going to change to be the delivery method
+        type=action_type,
         data=data,
         integration_id=alert_rule_trigger_action.integration_id,
         target_display=alert_rule_trigger_action.target_display,
         target_identifier=alert_rule_trigger_action.target_identifier,
         target_type=alert_rule_trigger_action.target_type,
+        legacy_notification_type=Action.LegacyNotificationType.METRIC_ALERT,
     )
     data_condition_group_action = DataConditionGroupAction.objects.create(
         condition_group_id=alert_rule_trigger_data_condition.data_condition.condition_group.id,
@@ -109,10 +135,9 @@ def migrate_metric_data_condition(
         return None
 
     data_condition = DataCondition.objects.create(
-        condition=threshold_to_condition.get(threshold_type, AlertRuleThresholdType.ABOVE.value),
         comparison=alert_rule_trigger.alert_threshold,
         condition_result=condition_result,
-        type=ConditionType.METRIC_CONDITION,
+        type=threshold_to_condition.get(threshold_type, AlertRuleThresholdType.ABOVE.value),
         condition_group=data_condition_group,
     )
     alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.create(

src/sentry/workflow_engine/models/action.py

@@ -32,7 +32,11 @@ class Action(DefaultFieldsModel):
     class Type(models.TextChoices):
         EMAIL = "email"
         SLACK = "slack"
+        MSTEAMS = "msteams"
         PAGERDUTY = "pagerduty"
+        OPSGENIE = "opsgenie"
+        DISCORD = "discord"
+        SENTRY_APP = "sentry_app"
         WEBHOOK = "webhook"
 
     class LegacyNotificationType(models.TextChoices):

tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py

@@ -1,8 +1,11 @@
 from unittest import mock
 
 from sentry.incidents.grouptype import MetricAlertFire
+from sentry.incidents.models.alert_rule import AlertRuleTriggerAction
+from sentry.integrations.models.integration import Integration, OrganizationIntegration
 from sentry.snuba.models import QuerySubscription
 from sentry.testutils.cases import APITestCase
+from sentry.testutils.silo import assume_test_silo_mode_of
 from sentry.users.services.user.service import user_service
 from sentry.workflow_engine.migration_helpers.alert_rule import (
     create_data_condition_group,
@@ -34,12 +37,58 @@
 
 class AlertRuleMigrationHelpersTest(APITestCase):
     def setUp(self):
+        METADATA = {
+            "api_key": "1234-ABCD",
+            "base_url": "https://api.opsgenie.com/",
+            "domain_name": "test-app.app.opsgenie.com",
+        }
+        self.rpc_user = user_service.get_user(user_id=self.user.id)
+        self.og_team = {"id": "123-id", "team": "cool-team", "integration_key": "1234-5678"}
+        self.integration = self.create_provider_integration(
+            provider="opsgenie", name="hello-world", external_id="hello-world", metadata=METADATA
+        )
+        self.sentry_app = self.create_sentry_app(
+            name="foo",
+            organization=self.organization,
+            is_alertable=True,
+            verify_install=False,
+        )
+        self.create_sentry_app_installation(
+            slug=self.sentry_app.slug, organization=self.organization, user=self.rpc_user
+        )
+        with assume_test_silo_mode_of(Integration, OrganizationIntegration):
+            self.integration.add_organization(self.organization, self.user)
+            self.org_integration = OrganizationIntegration.objects.get(
+                organization_id=self.organization.id, integration_id=self.integration.id
+            )
+            self.org_integration.config = {"team_table": [self.og_team]}
+            self.org_integration.save()
+
         self.metric_alert = self.create_alert_rule()
-        self.alert_rule_trigger = self.create_alert_rule_trigger(alert_rule=self.metric_alert)
-        self.alert_rule_trigger_action = self.create_alert_rule_trigger_action(
-            alert_rule_trigger=self.alert_rule_trigger
+        self.alert_rule_trigger_warning = self.create_alert_rule_trigger(
+            alert_rule=self.metric_alert, label="warning"
+        )
+        self.alert_rule_trigger_critical = self.create_alert_rule_trigger(
+            alert_rule=self.metric_alert, label="critical"
+        )
+        self.alert_rule_trigger_action_email = self.create_alert_rule_trigger_action(
+            alert_rule_trigger=self.alert_rule_trigger_warning
+        )
+        self.alert_rule_trigger_action_integration = self.create_alert_rule_trigger_action(
+            target_identifier=self.og_team["id"],
+            type=AlertRuleTriggerAction.Type.OPSGENIE,
+            target_type=AlertRuleTriggerAction.TargetType.SPECIFIC,
+            integration=self.integration,
+            alert_rule_trigger=self.alert_rule_trigger_critical,
+        )
+
+        self.alert_rule_trigger_action_sentry_app = self.create_alert_rule_trigger_action(
+            target_identifier=self.sentry_app.id,
+            type=AlertRuleTriggerAction.Type.SENTRY_APP,
+            target_type=AlertRuleTriggerAction.TargetType.SENTRY_APP,
+            sentry_app=self.sentry_app,
+            alert_rule_trigger=self.alert_rule_trigger_critical,
         )
-        self.rpc_user = user_service.get_user(user_id=self.user.id)
 
     def test_create_metric_alert(self):
         """
@@ -92,25 +141,37 @@ def test_create_metric_alert(self):
 
     def test_create_metric_alert_trigger(self):
         """
-        Test that when we call the helper methods we create all the ACI models correctly for an alert rule trigger
+        Test that when we call the helper methods we create all the ACI models correctly for alert rule triggers
         """
         migrate_alert_rule(self.metric_alert, self.rpc_user)
-        migrate_metric_data_condition(self.alert_rule_trigger)
-        alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.get(
-            alert_rule_trigger=self.alert_rule_trigger
-        )
-        data_condition_group_id = (
-            alert_rule_trigger_data_condition.data_condition.condition_group.id
+        migrate_metric_data_condition(self.alert_rule_trigger_warning)
+        migrate_metric_data_condition(self.alert_rule_trigger_critical)
+
+        alert_rule_trigger_data_conditions = AlertRuleTriggerDataCondition.objects.filter(
+            alert_rule_trigger__in=[
+                self.alert_rule_trigger_critical,
+                self.alert_rule_trigger_warning,
+            ]
         )
-        data_condition = DataCondition.objects.get(condition_group=data_condition_group_id)
+        assert len(alert_rule_trigger_data_conditions) == 2
+        data_condition_group_id = alert_rule_trigger_data_conditions[
+            0
+        ].data_condition.condition_group.id
+        data_conditions = DataCondition.objects.filter(condition_group=data_condition_group_id)
+        assert len(data_conditions) == 2
         alert_rule_workflow = AlertRuleWorkflow.objects.get(alert_rule=self.metric_alert)
         workflow = Workflow.objects.get(id=alert_rule_workflow.workflow.id)
         data_condition_group = workflow.when_condition_group
 
-        assert data_condition.condition == Condition.GREATER
-        assert data_condition.comparison == self.alert_rule_trigger.alert_threshold
-        assert data_condition.condition_result == DetectorPriorityLevel.HIGH
-        assert data_condition.condition_group == data_condition_group
+        assert data_conditions[0].type == Condition.GREATER
+        assert data_conditions[0].comparison == self.alert_rule_trigger_warning.alert_threshold
+        assert data_conditions[0].condition_result == DetectorPriorityLevel.MEDIUM
+        assert data_conditions[0].condition_group == data_condition_group
+
+        assert data_conditions[1].type == Condition.GREATER
+        assert data_conditions[1].comparison == self.alert_rule_trigger_critical.alert_threshold
+        assert data_conditions[1].condition_result == DetectorPriorityLevel.HIGH
+        assert data_conditions[1].condition_group == data_condition_group
 
     @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger")
     def test_create_metric_alert_trigger_no_alert_rule_detector(self, mock_logger):
@@ -121,45 +182,85 @@ def test_create_metric_alert_trigger_no_alert_rule_detector(self, mock_logger):
         )
         create_detector(self.metric_alert, self.project.id, data_condition_group, self.rpc_user)
         # skip creating lookup tables
-        migrate_metric_data_condition(self.alert_rule_trigger)
+        migrate_metric_data_condition(self.alert_rule_trigger_critical)
         mock_logger.exception.assert_called_with(
             "AlertRuleDetector does not exist",
             extra={
-                "alert_rule_id": self.alert_rule_trigger.alert_rule.id,
+                "alert_rule_id": self.alert_rule_trigger_critical.alert_rule.id,
             },
         )
 
     def test_create_metric_alert_trigger_action(self):
         """
-        Test that when we call the helper methods we create all the ACI models correctly for an alert rule trigger action
+        Test that when we call the helper methods we create all the ACI models correctly for alert rule trigger actions
         """
         migrate_alert_rule(self.metric_alert, self.rpc_user)
-        migrate_metric_data_condition(self.alert_rule_trigger)
-        migrate_metric_action(self.alert_rule_trigger_action)
-        alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.get(
-            alert_rule_trigger=self.alert_rule_trigger
-        )
-        data_condition_group_id = (
-            alert_rule_trigger_data_condition.data_condition.condition_group.id
+
+        migrate_metric_data_condition(self.alert_rule_trigger_warning)
+        migrate_metric_data_condition(self.alert_rule_trigger_critical)
+
+        migrate_metric_action(self.alert_rule_trigger_action_email)
+        migrate_metric_action(self.alert_rule_trigger_action_integration)
+        migrate_metric_action(self.alert_rule_trigger_action_sentry_app)
+
+        alert_rule_trigger_data_condition = AlertRuleTriggerDataCondition.objects.filter(
+            alert_rule_trigger__in=[
+                self.alert_rule_trigger_warning,
+                self.alert_rule_trigger_critical,
+            ]
         )
-        data_condition_group_action = DataConditionGroupAction.objects.get(
+        data_condition_group_id = alert_rule_trigger_data_condition[
+            0
+        ].data_condition.condition_group.id
+        data_condition_group_actions = DataConditionGroupAction.objects.filter(
             condition_group_id=data_condition_group_id
         )
-        assert Action.objects.filter(id=data_condition_group_action.action.id).exists()
+        action = Action.objects.filter(
+            id__in=[item.action.id for item in data_condition_group_actions]
+        )
+        assert len(action) == 3
+
+        assert action[0].type.lower() == Action.Type.EMAIL
+        assert action[1].type.lower() == Action.Type.OPSGENIE
+        assert action[2].type.lower() == Action.Type.SENTRY_APP
 
     @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger")
     def test_create_metric_alert_trigger_action_no_alert_rule_trigger_data_condition(
         self, mock_logger
     ):
+        """
+        Test that if the AlertRuleTriggerDataCondition doesn't exist we return None and log.
+        """
         other_metric_alert = self.create_alert_rule()
         other_alert_rule_trigger = self.create_alert_rule_trigger(alert_rule=other_metric_alert)
 
         migrate_alert_rule(other_metric_alert, self.rpc_user)
         migrate_metric_data_condition(other_alert_rule_trigger)
-        migrate_metric_action(self.alert_rule_trigger_action)
+        migrated_action = migrate_metric_action(self.alert_rule_trigger_action_email)
+        assert migrated_action is None
         mock_logger.exception.assert_called_with(
             "AlertRuleTriggerDataCondition does not exist",
             extra={
-                "alert_rule_trigger_id": self.alert_rule_trigger.id,
+                "alert_rule_trigger_id": self.alert_rule_trigger_warning.id,
+            },
+        )
+
+    @mock.patch("sentry.workflow_engine.migration_helpers.alert_rule.logger")
+    def test_create_metric_alert_trigger_action_no_type(self, mock_logger):
+        """
+        Test that if for some reason we don't find a match for Action.Type for the integratio provider we return None and log.
+        """
+        with assume_test_silo_mode_of(Integration, OrganizationIntegration):
+            self.integration.update(provider="hellboy")
+            self.integration.save()
+
+        migrate_alert_rule(self.metric_alert, self.rpc_user)
+        migrate_metric_data_condition(self.alert_rule_trigger_critical)
+        migrated = migrate_metric_action(self.alert_rule_trigger_action_integration)
+        assert migrated is None
+        mock_logger.warning.assert_called_with(
+            "Could not find a matching Action.Type for the trigger action",
+            extra={
+                "alert_rule_trigger_action_id": self.alert_rule_trigger_action_integration.id,
             },
         )