diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index e7bae6fd35570..1680a90413370 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 756d74ef08c1d..b48af153898c4 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_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index e17b2b9bb2cf9..e002d87d08b4d 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -979,11 +979,14 @@ def test_delete_action(self): serialized_alert_rule["triggers"][1]["actions"].pop() with self.feature("organizations:incidents"): - resp = self.get_success_response( - self.organization.slug, alert_rule.id, **serialized_alert_rule + resp = self.get_error_response( + self.organization.slug, alert_rule.id, status_code=400, **serialized_alert_rule ) - - assert len(resp.data["triggers"][1]["actions"]) == 0 + assert resp.data == { + "nonFieldErrors": [ + "Each trigger must have an associated action for this alert to fire." + ] + } def test_update_trigger_action_type(self): self.create_member( 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 c6b9d675a301d..dfef2e48f6d15 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"): @@ -1328,9 +1327,6 @@ def test_performance_alert(self, record_analytics): with self.feature(["organizations:incidents", "organizations:performance-view"]): resp = self.get_response(self.organization.slug, **valid_alert_rule) assert resp.status_code == 201 - assert self.analytics_called_with_args( - record_analytics, "alert.created", query_type="PERFORMANCE" - ) @freeze_time() diff --git a/tests/sentry/incidents/endpoints/test_serializers.py b/tests/sentry/incidents/endpoints/test_serializers.py index 030b605cdf2c8..aebdf529340f7 100644 --- a/tests/sentry/incidents/endpoints/test_serializers.py +++ b/tests/sentry/incidents/endpoints/test_serializers.py @@ -428,6 +428,14 @@ def test_boundary(self): assert serializer.is_valid(), serializer.errors def test_boundary_off_by_one(self): + actions = [ + { + "type": "slack", + "targetIdentifier": "my-channel", + "targetType": "specific", + "integration": self.integration.id, + } + ] self.run_fail_validation_test( { "thresholdType": AlertRuleThresholdType.ABOVE.value, @@ -436,7 +444,7 @@ def test_boundary_off_by_one(self): { "label": "critical", "alertThreshold": 0, - "actions": [], + "actions": actions, }, ], }, @@ -457,7 +465,7 @@ def test_boundary_off_by_one(self): { "label": "critical", "alertThreshold": 2, - "actions": [], + "actions": actions, }, ], },