Skip to content

Commit

Permalink
fix(anomaly detection): surface validation error to user on trigger c…
Browse files Browse the repository at this point in the history
…reate/update (#76643)

Missed a spot. When creating/updating triggers, catch form validation
errors and surface them as serializer validation errors.
  • Loading branch information
mifu67 authored Aug 28, 2024
1 parent 66cd51e commit 2bb3c05
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/sentry/incidents/serializers/alert_rule_trigger.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django import forms
from rest_framework import serializers

from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
Expand Down Expand Up @@ -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")

Expand All @@ -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")

Expand Down
30 changes: 30 additions & 0 deletions tests/sentry/incidents/endpoints/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -21,6 +22,8 @@
from sentry.incidents.models.alert_rule import (
AlertRule,
AlertRuleDetectionType,
AlertRuleSeasonality,
AlertRuleSensitivity,
AlertRuleThresholdType,
AlertRuleTriggerAction,
)
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 2bb3c05

Please sign in to comment.