From 2826e8bb6e4c383f73439954e11266cc49111410 Mon Sep 17 00:00:00 2001
From: Colleen O'Rourke <colleen@sentry.io>
Date: Mon, 15 Jul 2024 11:38:11 -0700
Subject: [PATCH] Add unit tests, don't reinvent stuff

---
 src/sentry/rules/conditions/event_frequency.py     |  7 ++-----
 src/sentry/rules/processing/delayed_processing.py  | 14 ++++----------
 .../rules/processing/test_delayed_processing.py    | 10 +++++++++-
 .../snuba/rules/conditions/test_event_frequency.py | 12 ++++++++++++
 4 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/sentry/rules/conditions/event_frequency.py b/src/sentry/rules/conditions/event_frequency.py
index a170ff5be6f958..dc142bb29013bb 100644
--- a/src/sentry/rules/conditions/event_frequency.py
+++ b/src/sentry/rules/conditions/event_frequency.py
@@ -375,10 +375,6 @@ def get_chunked_result(
             batch_totals.update(result)
         return batch_totals
 
-    def get_group_category_by_type_id(self, type_id: int) -> GroupCategory:
-        issue_type = get_group_type_by_type_id(type_id)
-        return GroupCategory(issue_type.category)
-
     def get_error_and_generic_group_ids(
         self, groups: list[tuple[int, int, int]]
     ) -> tuple[list[int], list[int]]:
@@ -389,7 +385,8 @@ def get_error_and_generic_group_ids(
         error_issue_ids = []
 
         for group in groups:
-            if self.get_group_category_by_type_id(group[1]) == GroupCategory.ERROR:
+            issue_type = get_group_type_by_type_id(group[1])
+            if GroupCategory(issue_type.category) == GroupCategory.ERROR:
                 error_issue_ids.append(group[0])
             else:
                 generic_issue_ids.append(group[0])
diff --git a/src/sentry/rules/processing/delayed_processing.py b/src/sentry/rules/processing/delayed_processing.py
index fe2e1971b994b8..599f7b14a01416 100644
--- a/src/sentry/rules/processing/delayed_processing.py
+++ b/src/sentry/rules/processing/delayed_processing.py
@@ -1,4 +1,5 @@
 import logging
+import math
 import uuid
 from collections import defaultdict
 from datetime import datetime, timedelta, timezone
@@ -379,16 +380,9 @@ def get_group_to_groupevent(
 
 
 def bucket_num_groups(num_groups: int) -> str:
-    if num_groups > 10000:
-        return ">10000"
-    elif num_groups > 1000:
-        return ">1000"
-    elif num_groups > 100:
-        return ">100"
-    elif num_groups > 10:
-        return ">10"
-    elif num_groups > 1:
-        return ">1"
+    if num_groups > 1:
+        magnitude = 10 ** int(math.log10(num_groups))
+        return f">{magnitude}"
     return "1"
 
 
diff --git a/tests/sentry/rules/processing/test_delayed_processing.py b/tests/sentry/rules/processing/test_delayed_processing.py
index 43e203980abd83..34835be9043245 100644
--- a/tests/sentry/rules/processing/test_delayed_processing.py
+++ b/tests/sentry/rules/processing/test_delayed_processing.py
@@ -15,8 +15,9 @@
     EventFrequencyCondition,
     EventFrequencyConditionData,
 )
-from sentry.rules.processing.delayed_processing import (  # build_group_to_groupevent,; bulk_fetch_events,; get_condition_group_results,; get_group_to_groupevent,; get_rules_to_fire,; ; ; parse_rulegroup_to_event_data,
+from sentry.rules.processing.delayed_processing import (
     apply_delayed,
+    bucket_num_groups,
     get_condition_query_groups,
     get_rules_to_groups,
     get_rules_to_slow_conditions,
@@ -39,6 +40,13 @@
 TEST_RULE_FAST_CONDITION = {"id": "sentry.rules.conditions.every_event.EveryEventCondition"}
 
 
+def test_bucket_num_groups():
+    assert bucket_num_groups(1) == "1"
+    assert bucket_num_groups(50) == ">10"
+    assert bucket_num_groups(101) == ">100"
+    assert bucket_num_groups(1001) == ">1000"
+
+
 def mock_get_condition_group(descending=False):
     """
     Mocks get_condition_groups to run with the passed in alert_rules in
diff --git a/tests/snuba/rules/conditions/test_event_frequency.py b/tests/snuba/rules/conditions/test_event_frequency.py
index 0e1f903db7389c..c02aa03fb778da 100644
--- a/tests/snuba/rules/conditions/test_event_frequency.py
+++ b/tests/snuba/rules/conditions/test_event_frequency.py
@@ -8,6 +8,7 @@
 from django.utils import timezone
 
 from sentry.issues.grouptype import PerformanceNPlusOneGroupType
+from sentry.models.group import Group
 from sentry.models.project import Project
 from sentry.models.rule import Rule
 from sentry.rules.conditions.event_frequency import (
@@ -149,6 +150,17 @@ def test_batch_query(self):
         )
         assert batch_query == {self.event3.group_id: 1}
 
+    def test_get_error_and_generic_group_ids(self):
+        groups = Group.objects.filter(
+            id__in=[self.event.group_id, self.event2.group_id, self.perf_event.group_id]
+        ).values_list("id", "type", "project__organization_id")
+        error_issue_ids, generic_issue_ids = self.condition_inst.get_error_and_generic_group_ids(
+            groups
+        )
+        assert self.event.group_id in error_issue_ids
+        assert self.event2.group_id in error_issue_ids
+        assert self.perf_event.group_id in generic_issue_ids
+
 
 class EventUniqueUserFrequencyQueryTest(EventFrequencyQueryTestBase):
     rule_cls = EventUniqueUserFrequencyCondition