Browse Source

Fixing bug with existing 1m rules and 0 session counts (#28243)

Chris Fuller 3 years ago
parent
commit
099660921e

+ 23 - 3
src/sentry/rules/conditions/event_frequency.py

@@ -133,21 +133,29 @@ class EventUniqueUserFrequencyCondition(BaseEventFrequencyCondition):
 
 
 
 
 percent_intervals = {
 percent_intervals = {
+    "1m": ("1 minute", timedelta(minutes=1)),
     "5m": ("5 minutes", timedelta(minutes=5)),
     "5m": ("5 minutes", timedelta(minutes=5)),
     "10m": ("10 minutes", timedelta(minutes=10)),
     "10m": ("10 minutes", timedelta(minutes=10)),
     "30m": ("30 minutes", timedelta(minutes=30)),
     "30m": ("30 minutes", timedelta(minutes=30)),
     "1h": ("1 hour", timedelta(minutes=60)),
     "1h": ("1 hour", timedelta(minutes=60)),
 }
 }
-MIN_SESSIONS_TO_FIRE = 0
+
+percent_intervals_to_display = {
+    "5m": ("5 minutes", timedelta(minutes=5)),
+    "10m": ("10 minutes", timedelta(minutes=10)),
+    "30m": ("30 minutes", timedelta(minutes=30)),
+    "1h": ("1 hour", timedelta(minutes=60)),
+}
+MIN_SESSIONS_TO_FIRE = 1
 
 
 
 
 class EventFrequencyPercentForm(EventFrequencyForm):
 class EventFrequencyPercentForm(EventFrequencyForm):
-    intervals = percent_intervals
+    intervals = percent_intervals_to_display
     interval = forms.ChoiceField(
     interval = forms.ChoiceField(
         choices=[
         choices=[
             (key, label)
             (key, label)
             for key, (label, duration) in sorted(
             for key, (label, duration) in sorted(
-                percent_intervals.items(),
+                percent_intervals_to_display.items(),
                 key=lambda key____label__duration: key____label__duration[1][1],
                 key=lambda key____label__duration: key____label__duration[1][1],
             )
             )
         ]
         ]
@@ -164,6 +172,18 @@ class EventFrequencyPercentCondition(BaseEventFrequencyCondition):
         self.form_cls = EventFrequencyPercentForm
         self.form_cls = EventFrequencyPercentForm
         super().__init__(*args, **kwargs)
         super().__init__(*args, **kwargs)
 
 
+        # Override form fields interval to hide 1 min option from ui, but leave it available to process existing 1m rules
+        self.form_fields["interval"] = {
+            "type": "choice",
+            "choices": [
+                (key, label)
+                for key, (label, duration) in sorted(
+                    percent_intervals_to_display.items(),
+                    key=lambda key____label__duration: key____label__duration[1][1],
+                )
+            ],
+        }
+
     def query_hook(self, event, start, end, environment_id):
     def query_hook(self, event, start, end, environment_id):
         project_id = event.project_id
         project_id = event.project_id
         cache_key = f"r.c.spc:{project_id}-{environment_id}"
         cache_key = f"r.c.spc:{project_id}-{environment_id}"

+ 8 - 8
tests/snuba/rules/conditions/test_event_frequency.py

@@ -232,7 +232,7 @@ class EventFrequencyPercentConditionTestCase(
                 project_id=self.project.id,
                 project_id=self.project.id,
             )
             )
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_five_minutes_with_events(self):
     def test_five_minutes_with_events(self):
         self._make_sessions(60, 5)
         self._make_sessions(60, 5)
         data = {"interval": "5m", "value": 39}
         data = {"interval": "5m", "value": 39}
@@ -240,7 +240,7 @@ class EventFrequencyPercentConditionTestCase(
         data = {"interval": "5m", "value": 41}
         data = {"interval": "5m", "value": 41}
         self._run_test(data=data, minutes=5, passes=False)
         self._run_test(data=data, minutes=5, passes=False)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_ten_minutes_with_events(self):
     def test_ten_minutes_with_events(self):
         self._make_sessions(60, 10)
         self._make_sessions(60, 10)
         data = {"interval": "10m", "value": 49}
         data = {"interval": "10m", "value": 49}
@@ -248,7 +248,7 @@ class EventFrequencyPercentConditionTestCase(
         data = {"interval": "10m", "value": 51}
         data = {"interval": "10m", "value": 51}
         self._run_test(data=data, minutes=10, passes=False)
         self._run_test(data=data, minutes=10, passes=False)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_thirty_minutes_with_events(self):
     def test_thirty_minutes_with_events(self):
         self._make_sessions(60, 30)
         self._make_sessions(60, 30)
         data = {"interval": "30m", "value": 49}
         data = {"interval": "30m", "value": 49}
@@ -256,7 +256,7 @@ class EventFrequencyPercentConditionTestCase(
         data = {"interval": "30m", "value": 51}
         data = {"interval": "30m", "value": 51}
         self._run_test(data=data, minutes=30, passes=False)
         self._run_test(data=data, minutes=30, passes=False)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_one_hour_with_events(self):
     def test_one_hour_with_events(self):
         self._make_sessions(60, 60)
         self._make_sessions(60, 60)
         data = {"interval": "1h", "value": 49}
         data = {"interval": "1h", "value": 49}
@@ -264,25 +264,25 @@ class EventFrequencyPercentConditionTestCase(
         data = {"interval": "1h", "value": 51}
         data = {"interval": "1h", "value": 51}
         self._run_test(data=data, minutes=60, passes=False)
         self._run_test(data=data, minutes=60, passes=False)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_five_minutes_no_events(self):
     def test_five_minutes_no_events(self):
         self._make_sessions(60, 5)
         self._make_sessions(60, 5)
         data = {"interval": "5m", "value": 39}
         data = {"interval": "5m", "value": 39}
         self._run_test(data=data, minutes=5, passes=True, add_events=True)
         self._run_test(data=data, minutes=5, passes=True, add_events=True)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_ten_minutes_no_events(self):
     def test_ten_minutes_no_events(self):
         self._make_sessions(60, 10)
         self._make_sessions(60, 10)
         data = {"interval": "10m", "value": 49}
         data = {"interval": "10m", "value": 49}
         self._run_test(data=data, minutes=10, passes=True, add_events=True)
         self._run_test(data=data, minutes=10, passes=True, add_events=True)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_thirty_minutes_no_events(self):
     def test_thirty_minutes_no_events(self):
         self._make_sessions(60, 30)
         self._make_sessions(60, 30)
         data = {"interval": "30m", "value": 49}
         data = {"interval": "30m", "value": 49}
         self._run_test(data=data, minutes=30, passes=True, add_events=True)
         self._run_test(data=data, minutes=30, passes=True, add_events=True)
 
 
-    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 0)
+    @patch("sentry.rules.conditions.event_frequency.MIN_SESSIONS_TO_FIRE", 1)
     def test_one_hour_no_events(self):
     def test_one_hour_no_events(self):
         self._make_sessions(60, 60)
         self._make_sessions(60, 60)
         data = {"interval": "1h", "value": 49}
         data = {"interval": "1h", "value": 49}