From 5adf42589f462ffd5915b92840d0640de68ae5a9 Mon Sep 17 00:00:00 2001
From: Dan Fuller <dfuller@sentry.io>
Date: Wed, 27 Oct 2021 15:52:10 -0700
Subject: [PATCH] fix(cmp_alerts): Allow comparison event frequency percent
 alerts to set a value > 100

Event frequency percent alerts are capped at a value between 0-100%, which makes sense for that kind
of alert. When we change these to be comparison alerts this doesn't make sense though, since we're
just taking into account relative change. This pr changes the validation to be conditional.
---
 .../rules/conditions/event_frequency.py       | 13 ++++++-
 .../api/endpoints/test_project_rules.py       | 38 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/sentry/rules/conditions/event_frequency.py b/src/sentry/rules/conditions/event_frequency.py
index 459d8c15d443ea..e8ffb23e1d57cc 100644
--- a/src/sentry/rules/conditions/event_frequency.py
+++ b/src/sentry/rules/conditions/event_frequency.py
@@ -74,6 +74,7 @@ def clean(self):
             msg = forms.ValidationError("comparisonInterval is required when comparing by percent")
             self.add_error("comparisonInterval", msg)
             return
+        return cleaned_data
 
 
 class BaseEventFrequencyCondition(EventCondition):
@@ -217,7 +218,17 @@ class EventFrequencyPercentForm(EventFrequencyForm):
             )
         ]
     )
-    value = forms.FloatField(widget=forms.TextInput(), min_value=0, max_value=100)
+    value = forms.FloatField(widget=forms.TextInput(), min_value=0)
+
+    def clean(self):
+        cleaned_data = super().clean()
+        if cleaned_data["comparisonType"] == COMPARISON_TYPE_COUNT and cleaned_data["value"] > 100:
+            self.add_error(
+                "value", forms.ValidationError("Ensure this value is less than or equal to 100")
+            )
+            return
+
+        return cleaned_data
 
 
 class EventFrequencyPercentCondition(BaseEventFrequencyCondition):
diff --git a/tests/sentry/api/endpoints/test_project_rules.py b/tests/sentry/api/endpoints/test_project_rules.py
index 039b011cbff0e9..cd062d0f840772 100644
--- a/tests/sentry/api/endpoints/test_project_rules.py
+++ b/tests/sentry/api/endpoints/test_project_rules.py
@@ -187,6 +187,44 @@ def test_missing_name(self):
             status_code=400,
         )
 
+    def test_frequency_percent_validation(self):
+        self.login_as(user=self.user)
+        condition = {
+            "id": "sentry.rules.conditions.event_frequency.EventFrequencyPercentCondition",
+            "interval": "1h",
+            "value": 101,
+            "comparisonType": "count",
+        }
+        response = self.get_error_response(
+            self.organization.slug,
+            self.project.slug,
+            name="test",
+            frequency=30,
+            owner=self.user.actor.get_actor_identifier(),
+            actionMatch="any",
+            filterMatch="any",
+            conditions=[condition],
+            status_code=400,
+        )
+        assert (
+            str(response.data["conditions"][0]) == "Ensure this value is less than or equal to 100"
+        )
+
+        # Upper bound shouldn't be enforced when we're doing a comparison alert
+        condition["comparisonType"] = "percent"
+        condition["comparisonInterval"] = "1d"
+        self.get_success_response(
+            self.organization.slug,
+            self.project.slug,
+            name="test",
+            frequency=30,
+            owner=self.user.actor.get_actor_identifier(),
+            actionMatch="any",
+            filterMatch="any",
+            conditions=[condition],
+            status_code=200,
+        )
+
     def test_match_values(self):
         filters = [
             {