From e8126eb81adc6835b000ca685b6bb2a2db19bbc5 Mon Sep 17 00:00:00 2001
From: Colleen O'Rourke <colleen@sentry.io>
Date: Fri, 18 Oct 2024 16:00:14 -0700
Subject: [PATCH] ref(alerts): Move action check to serializer and remove
 signals

---
 .../organization_alert_rule_index.py          | 28 -------------------
 .../incidents/serializers/alert_rule.py       |  5 ++++
 .../test_organization_alert_rule_index.py     | 11 ++++----
 3 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py
index e7bae6fd35570a..1680a904133705 100644
--- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py
+++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py
@@ -52,7 +52,6 @@
 from sentry.models.rule import Rule, RuleSource
 from sentry.models.team import Team
 from sentry.sentry_apps.services.app import app_service
-from sentry.signals import alert_rule_created
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.models import SnubaQuery
 from sentry.uptime.models import (
@@ -118,17 +117,9 @@ def create_metric_alert(
             },
             data=data,
         )
-
         if not serializer.is_valid():
             raise ValidationError(serializer.errors)
 
-        # if there are no triggers, then the serializer will raise an error
-        for trigger in data["triggers"]:
-            if not trigger.get("actions", []):
-                raise ValidationError(
-                    "Each trigger must have an associated action for this alert to fire."
-                )
-
         trigger_sentry_app_action_creators_for_incidents(serializer.validated_data)
         if get_slack_actions_with_async_lookups(organization, request.user, request.data):
             # need to kick off an async job for Slack
@@ -143,25 +134,6 @@ def create_metric_alert(
             return Response({"uuid": client.uuid}, status=202)
         else:
             alert_rule = serializer.save()
-            referrer = request.query_params.get("referrer")
-            session_id = request.query_params.get("sessionId")
-            duplicate_rule = request.query_params.get("duplicateRule")
-            wizard_v3 = request.query_params.get("wizardV3")
-            subscriptions = alert_rule.snuba_query.subscriptions.all()
-            for sub in subscriptions:
-                alert_rule_created.send_robust(
-                    user=request.user,
-                    project=sub.project,
-                    rule_id=alert_rule.id,
-                    rule_type="metric",
-                    sender=self,
-                    referrer=referrer,
-                    session_id=session_id,
-                    is_api_token=request.auth is not None,
-                    duplicate_rule=duplicate_rule,
-                    wizard_v3=wizard_v3,
-                    query_type=self.get_query_type_description(data.get("queryType", None)),
-                )
             return Response(serialize(alert_rule, request.user), status=status.HTTP_201_CREATED)
 
     def get_query_type_description(self, value):
diff --git a/src/sentry/incidents/serializers/alert_rule.py b/src/sentry/incidents/serializers/alert_rule.py
index 756d74ef08c1d6..b48af153898c4d 100644
--- a/src/sentry/incidents/serializers/alert_rule.py
+++ b/src/sentry/incidents/serializers/alert_rule.py
@@ -263,6 +263,11 @@ def validate(self, data):
             raise serializers.ValidationError(
                 "Must send 1 or 2 triggers - A critical trigger, and an optional warning trigger"
             )
+        for trigger in triggers:
+            if not trigger.get("actions", []):
+                raise serializers.ValidationError(
+                    "Each trigger must have an associated action for this alert to fire."
+                )
 
         if query_type == SnubaQuery.Type.CRASH_RATE:
             data["event_types"] = []
diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py
index c6b9d675a301dc..718243c2a5bafd 100644
--- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py
+++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py
@@ -924,12 +924,11 @@ def test_critical_trigger_no_action(self):
             resp = self.get_error_response(
                 self.organization.slug, status_code=400, **rule_one_trigger_only_critical_no_action
             )
-        assert resp.data == [
-            ErrorDetail(
-                string="Each trigger must have an associated action for this alert to fire.",
-                code="invalid",
-            )
-        ]
+        assert resp.data == {
+            "nonFieldErrors": [
+                "Each trigger must have an associated action for this alert to fire."
+            ]
+        }
 
     def test_invalid_projects(self):
         with self.feature("organizations:incidents"):