From 2bb3c05223c6de6c3fcad84c438e0ded4d438db4 Mon Sep 17 00:00:00 2001 From: Michelle Fu <83109586+mifu67@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:42:29 -0700 Subject: [PATCH] fix(anomaly detection): surface validation error to user on trigger create/update (#76643) Missed a spot. When creating/updating triggers, catch form validation errors and surface them as serializer validation errors. --- .../serializers/alert_rule_trigger.py | 7 +++++ .../incidents/endpoints/test_serializers.py | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/sentry/incidents/serializers/alert_rule_trigger.py b/src/sentry/incidents/serializers/alert_rule_trigger.py index e6aa7d7fffad18..8e6d97c5ab86ec 100644 --- a/src/sentry/incidents/serializers/alert_rule_trigger.py +++ b/src/sentry/incidents/serializers/alert_rule_trigger.py @@ -1,3 +1,4 @@ +from django import forms from rest_framework import serializers from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer @@ -45,6 +46,9 @@ def create(self, validated_data): self._handle_actions(alert_rule_trigger, actions) return alert_rule_trigger + except forms.ValidationError as e: + # if we fail in create_alert_rule_trigger, then only one message is ever returned + raise serializers.ValidationError(e.error_list[0].message) except AlertRuleTriggerLabelAlreadyUsedError: raise serializers.ValidationError("This label is already in use for this alert rule") @@ -56,6 +60,9 @@ def update(self, instance, validated_data): alert_rule_trigger = update_alert_rule_trigger(instance, **validated_data) self._handle_actions(alert_rule_trigger, actions) return alert_rule_trigger + except forms.ValidationError as e: + # if we fail in update_alert_rule_trigger, then only one message is ever returned + raise serializers.ValidationError(e.error_list[0].message) except AlertRuleTriggerLabelAlreadyUsedError: raise serializers.ValidationError("This label is already in use for this alert rule") diff --git a/tests/sentry/incidents/endpoints/test_serializers.py b/tests/sentry/incidents/endpoints/test_serializers.py index 4e38ea80c1902f..3a2154a14d8315 100644 --- a/tests/sentry/incidents/endpoints/test_serializers.py +++ b/tests/sentry/incidents/endpoints/test_serializers.py @@ -10,6 +10,7 @@ from rest_framework import serializers from rest_framework.exceptions import ErrorDetail from slack_sdk.errors import SlackApiError +from urllib3.response import HTTPResponse from sentry.auth.access import from_user from sentry.incidents.logic import ( @@ -21,6 +22,8 @@ from sentry.incidents.models.alert_rule import ( AlertRule, AlertRuleDetectionType, + AlertRuleSeasonality, + AlertRuleSensitivity, AlertRuleThresholdType, AlertRuleTriggerAction, ) @@ -43,6 +46,7 @@ from sentry.snuba.dataset import Dataset from sentry.snuba.models import SnubaQuery, SnubaQueryEventType from sentry.testutils.cases import TestCase +from sentry.testutils.helpers.features import with_feature from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba from sentry.users.models.user import User @@ -464,6 +468,32 @@ def test_boundary_off_by_one(self): }, ) + @with_feature("organizations:anomaly-detection-alerts") + @patch( + "sentry.seer.anomaly_detection.store_data.seer_anomaly_detection_connection_pool.urlopen" + ) + def test_invalid_alert_threshold(self, mock_seer_request): + """ + Anomaly detection alerts cannot have a nonzero alert rule threshold + """ + mock_seer_request.return_value = HTTPResponse(status=200) + + params = self.valid_params.copy() + params["detection_type"] = AlertRuleDetectionType.DYNAMIC + params["seasonality"] = AlertRuleSeasonality.AUTO + params["sensitivity"] = AlertRuleSensitivity.MEDIUM + params["time_window"] = 15 + serializer = AlertRuleSerializer(context=self.context, data=params) + assert serializer.is_valid() + + with pytest.raises( + serializers.ValidationError, + match="Dynamic alerts cannot have a nonzero alert threshold", + ): + serializer.save() + + assert mock_seer_request.call_count == 1 + def test_invalid_slack_channel(self): # We had an error where an invalid slack channel was spitting out unclear # error for the user, and CREATING THE RULE. So the next save (after fixing slack action)