feat(aci milestone 3): ACI alert rule dual update helper

src/sentry/workflow_engine/migration_helpers/alert_rule.py

@@ -1,4 +1,5 @@
 import logging
+from typing import Any
 
 from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion
 from sentry.incidents.grouptype import MetricAlertFire
@@ -374,6 +375,99 @@ def migrate_alert_rule(
     )
 
 
+def update_migrated_alert_rule(alert_rule: AlertRule, updated_fields: dict[str, Any]) -> (
+    tuple[
+        DetectorState,
+        Detector,
+    ]
+    | None
+):
+    # TODO: maybe pull this into a helper method?
+    try:
+        alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=alert_rule)
+    except AlertRuleDetector.DoesNotExist:
+        logger.exception(
+            "AlertRuleDetector does not exist",
+            extra={"alert_rule_id": AlertRule.id},
+        )
+        return None
+
+    detector: Detector = alert_rule_detector.detector
+
+    try:
+        detector_state = DetectorState.objects.get(detector=detector)
+    except DetectorState.DoesNotExist:
+        logger.exception(
+            "DetectorState does not exist",
+            extra={"alert_rule_id": AlertRule.id, "detector_id": detector.id},
+        )
+        return None
+
+    updated_detector_fields: dict[str, Any] = {}
+    config = detector.config.copy()
+
+    if "name" in updated_fields:
+        updated_detector_fields["name"] = updated_fields["name"]
+    if "description" in updated_fields:
+        updated_detector_fields["description"] = updated_fields["description"]
+    if "user_id" in updated_fields:
+        updated_detector_fields["owner_user_id"] = updated_fields["user_id"]
+    if "team_id" in updated_fields:
+        updated_detector_fields["owner_team_id"] = updated_fields["team_id"]
+    # update config fields
+    if "threshold_period" in updated_fields:
+        config["threshold_period"] = updated_fields["threshold_period"]
+    if "sensitivity" in updated_fields:
+        config["sensitivity"] = updated_fields["sensitivity"]
+    if "seasonality" in updated_fields:
+        config["seasonality"] = updated_fields["seasonality"]
+    if "comparison_delta" in updated_fields:
+        config["comparison_delta"] = updated_fields["comparison_delta"]
+    updated_detector_fields["config"] = config
+
+    # if the user updated resolve_threshold or threshold_type, then we also need to update the detector triggers
+    if "threshold_type" in updated_fields or "resolve_threshold" in updated_fields:
+        data_condition_group = detector.workflow_condition_group
+        if data_condition_group is None:
+            # this shouldn't be possible due to the way we dual write
+            logger.error(
+                "AlertRuleDetector has no associated DataConditionGroup",
+                extra={"alert_rule_id": AlertRule.id},
+            )
+            return None
+        data_conditions = DataCondition.objects.filter(condition_group=data_condition_group)
+        if "threshold_type" in updated_fields:
+            threshold_type = (
+                Condition.GREATER
+                if updated_fields["threshold_type"] == AlertRuleThresholdType.ABOVE.value
+                else Condition.LESS
+            )
+            resolve_threshold_type = (
+                Condition.LESS_OR_EQUAL
+                if updated_fields["threshold_type"] == AlertRuleThresholdType.ABOVE.value
+                else Condition.GREATER_OR_EQUAL
+            )
+            for dc in data_conditions:
+                if dc.condition_result == DetectorPriorityLevel.OK:
+                    dc.update(type=resolve_threshold_type)
+                else:
+                    dc.update(type=threshold_type)
+            resolve_condition = data_conditions.filter(condition_result=DetectorPriorityLevel.OK)
+
+        if "resolve_threshold" in updated_fields:
+            resolve_condition = data_conditions.filter(condition_result=DetectorPriorityLevel.OK)
+            resolve_condition.update(comparison=updated_fields["resolve_threshold"])
+
+    detector.update(**updated_detector_fields)
+
+    # reset detector status, as the rule was updated
+    detector_state.update(active=False, state=DetectorPriorityLevel.OK)
+
+    # TODO: do we need to create an audit log entry here?
+    # do we need to return the detector's data conditions/DCG?
+    return detector_state, detector
+
+
 def get_data_source(alert_rule: AlertRule) -> DataSource | None:
     snuba_query = alert_rule.snuba_query
     organization = alert_rule.organization

tests/sentry/workflow_engine/migration_helpers/test_migrate_alert_rule.py

@@ -16,6 +16,7 @@
     migrate_metric_action,
     migrate_metric_data_conditions,
     migrate_resolve_threshold_data_conditions,
+    update_migrated_alert_rule,
 )
 from sentry.workflow_engine.models import (
     Action,
@@ -292,7 +293,7 @@ def test_calculate_resolve_threshold_with_warning(self):
         resolve_threshold = get_resolve_threshold(detector_dcg)
         assert resolve_threshold == self.alert_rule_trigger_warning.alert_threshold
 
-    def create_metric_alert_trigger_auto_resolve(self):
+    def test_create_metric_alert_trigger_auto_resolve(self):
         """
         Test that we create the correct resolution DataConditions when an AlertRule has no explicit resolve threshold
         """
@@ -301,6 +302,7 @@ def create_metric_alert_trigger_auto_resolve(self):
 
         migrate_alert_rule(metric_alert, self.rpc_user)
         migrate_metric_data_conditions(critical_trigger)
+        migrate_resolve_threshold_data_conditions(metric_alert)
 
         detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector
 
@@ -322,15 +324,16 @@ def create_metric_alert_trigger_auto_resolve(self):
             condition_group=resolve_data_condition.condition_group
         ).exists()
 
-    def create_metric_alert_trigger_auto_resolve_less_than(self):
+    def test_create_metric_alert_trigger_auto_resolve_less_than(self):
         """
-        Test that we assign the resolve detector trigger the correct type if the threshold type is ABOVE
+        Test that we assign the resolve detector trigger the correct type if the threshold type is BELOW
         """
-        metric_alert = self.create_alert_rule(threshold_type=AlertRuleThresholdType.ABOVE)
+        metric_alert = self.create_alert_rule(threshold_type=AlertRuleThresholdType.BELOW)
         critical_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert, label="critical")
 
         migrate_alert_rule(metric_alert, self.rpc_user)
         migrate_metric_data_conditions(critical_trigger)
+        migrate_resolve_threshold_data_conditions(metric_alert)
 
         detector = AlertRuleDetector.objects.get(alert_rule=metric_alert).detector
 
@@ -417,3 +420,92 @@ def test_create_metric_alert_trigger_action_no_type(self, mock_logger):
                 "alert_rule_trigger_action_id": self.alert_rule_trigger_action_integration.id,
             },
         )
+
+    def test_update_metric_alert(self):
+        migrate_alert_rule(self.metric_alert, self.rpc_user)
+        updated_fields = {
+            "name": "hojicha",
+            "description": "a Japanese green tea roasted over charcoal",
+        }
+
+        alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert)
+        detector = alert_rule_detector.detector
+        detector_state = DetectorState.objects.get(detector=detector)
+        detector_state.update(
+            active=True, state=DetectorPriorityLevel.HIGH
+        )  # so we can confirm that our update actually changes things
+        assert detector_state.active
+
+        update_migrated_alert_rule(self.metric_alert, updated_fields)
+        detector.refresh_from_db()
+        detector_state.refresh_from_db()
+
+        assert detector.name == "hojicha"
+        assert detector.description == "a Japanese green tea roasted over charcoal"
+
+        assert detector_state.state == str(DetectorPriorityLevel.OK.value)
+        assert detector_state.active is False
+
+    def test_update_metric_alert_owner(self):
+        migrate_alert_rule(self.metric_alert, self.rpc_user)
+        updated_fields = {
+            "user_id": self.user.id,
+            "team_id": None,
+        }
+        alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert)
+        detector = alert_rule_detector.detector
+
+        update_migrated_alert_rule(self.metric_alert, updated_fields)
+        detector.refresh_from_db()
+
+        assert detector.owner_user_id == self.user.id
+        assert detector.owner_team_id is None
+
+    def test_update_metric_alert_config(self):
+        migrate_alert_rule(self.metric_alert, self.rpc_user)
+        updated_fields = {
+            "threshold_period": 1,
+            "sensitivity": None,
+            "seasonality": None,
+            "comparison_delta": 3600,
+        }
+        alert_rule_detector = AlertRuleDetector.objects.get(alert_rule=self.metric_alert)
+        detector = alert_rule_detector.detector
+
+        update_migrated_alert_rule(self.metric_alert, updated_fields)
+        detector.refresh_from_db()
+
+        assert detector.config == updated_fields
+
+    def test_update_metric_alert_threshold_type(self):
+        metric_alert = self.create_alert_rule()
+        critical_trigger = self.create_alert_rule_trigger(alert_rule=metric_alert, label="critical")
+
+        migrate_alert_rule(metric_alert, self.rpc_user)
+        migrate_metric_data_conditions(critical_trigger)
+        migrate_resolve_threshold_data_conditions(metric_alert)
+
+        updated_fields = {"threshold_type": AlertRuleThresholdType.BELOW}
+        update_migrated_alert_rule(metric_alert, updated_fields)
+        # because there are only two objects in the DB
+        critical_detector_trigger = DataCondition.objects.get(
+            condition_result=DetectorPriorityLevel.HIGH
+        )
+        resolve_detector_trigger = DataCondition.objects.get(
+            condition_result=DetectorPriorityLevel.OK
+        )
+
+        assert critical_detector_trigger.type == Condition.LESS
+        assert resolve_detector_trigger.type == Condition.GREATER_OR_EQUAL
+
+    def test_update_metric_alert_resolve_threshold(self):
+        migrate_alert_rule(self.metric_alert, self.rpc_user)
+        migrate_metric_data_conditions(self.alert_rule_trigger_critical)
+        migrate_resolve_threshold_data_conditions(self.metric_alert)
+
+        updated_fields = {"resolve_threshold": 10}
+        update_migrated_alert_rule(self.metric_alert, updated_fields)
+        resolve_detector_trigger = DataCondition.objects.get(
+            condition_result=DetectorPriorityLevel.OK
+        )
+        assert resolve_detector_trigger.comparison == 10