Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(anomaly detection): surface validation error to user on trigger create/update #76643

Merged
merged 3 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
19 changes: 19 additions & 0 deletions tests/sentry/incidents/endpoints/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from sentry.incidents.models.alert_rule import (
AlertRule,
AlertRuleDetectionType,
AlertRuleSeasonality,
AlertRuleSensitivity,
AlertRuleThresholdType,
AlertRuleTriggerAction,
)
Expand All @@ -43,6 +45,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 +467,22 @@ def test_boundary_off_by_one(self):
},
)

@with_feature("organizations:anomaly-detection-alerts")
def test_invalid_alert_threshold(self):
"""
Anomaly detection alerts cannot have a nonzero alert rule threshold
"""
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):
ceorourke marked this conversation as resolved.
Show resolved Hide resolved
serializer.save()

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
Loading