Browse Source

feat(metric_alerts): Remove unique name validation from metric alerts (#31434)

We removed the unique constraint in https://github.com/getsentry/sentry/pull/31300. Following up and
removing validation that prevented duplicate names.
Dan Fuller 3 years ago
parent
commit
716479a7c8

+ 21 - 29
bin/load-mocks

@@ -21,12 +21,7 @@ from pytz import utc
 
 from sentry import buffer, roles, tsdb
 from sentry.event_manager import HashDiscarded
-from sentry.incidents.logic import (
-    AlertRuleNameAlreadyUsedError,
-    create_alert_rule,
-    create_alert_rule_trigger,
-    create_incident,
-)
+from sentry.incidents.logic import create_alert_rule, create_alert_rule_trigger, create_incident
 from sentry.incidents.models import AlertRuleThresholdType, IncidentType
 from sentry.models import (
     TOMBSTONE_FIELDS_FROM_GROUP,
@@ -671,29 +666,26 @@ def main(num_events=1, extra_events=False, load_trends=False, slow=False):
                     comments=make_sentence(),
                 )
 
-            try:
-                # Metric alerts
-                alert_rule = create_alert_rule(
-                    org,
-                    [project],
-                    "My Alert Rule",
-                    "level:error",
-                    "count()",
-                    10,
-                    AlertRuleThresholdType.ABOVE,
-                    1,
-                )
-                create_alert_rule_trigger(alert_rule, "critical", 10)
-                create_incident(
-                    org,
-                    type_=IncidentType.DETECTED,
-                    title="My Incident",
-                    date_started=datetime.utcnow().replace(tzinfo=utc),
-                    alert_rule=alert_rule,
-                    projects=[project],
-                )
-            except AlertRuleNameAlreadyUsedError:
-                pass
+            # Metric alerts
+            alert_rule = create_alert_rule(
+                org,
+                [project],
+                "My Alert Rule",
+                "level:error",
+                "count()",
+                10,
+                AlertRuleThresholdType.ABOVE,
+                1,
+            )
+            create_alert_rule_trigger(alert_rule, "critical", 10)
+            create_incident(
+                org,
+                type_=IncidentType.DETECTED,
+                title="My Incident",
+                date_started=datetime.utcnow().replace(tzinfo=utc),
+                alert_rule=alert_rule,
+                projects=[project],
+            )
 
             print(f"    > Loading time series data")  # NOQA
             if event1:

+ 0 - 9
src/sentry/incidents/logic.py

@@ -494,8 +494,6 @@ def create_alert_rule(
         resolution = DEFAULT_CMP_ALERT_RULE_RESOLUTION
         comparison_delta = int(timedelta(minutes=comparison_delta).total_seconds())
     validate_alert_rule_query(query)
-    if AlertRule.objects.filter(organization=organization, name=name).exists():
-        raise AlertRuleNameAlreadyUsedError()
     with transaction.atomic():
         snuba_query = create_snuba_query(
             dataset,
@@ -628,13 +626,6 @@ def update_alert_rule(
     comparison period. In minutes.
     :return: The updated `AlertRule`
     """
-    if (
-        name
-        and alert_rule.name != name
-        and AlertRule.objects.filter(organization=alert_rule.organization, name=name).exists()
-    ):
-        raise AlertRuleNameAlreadyUsedError()
-
     updated_fields = {"date_modified": timezone.now()}
     updated_query_fields = {}
     if name:

+ 13 - 20
src/sentry/incidents/serializers/alert_rule.py

@@ -16,7 +16,6 @@ from sentry.exceptions import InvalidSearchQuery, UnsupportedQuerySubscription
 from sentry.incidents.logic import (
     CRITICAL_TRIGGER_LABEL,
     WARNING_TRIGGER_LABEL,
-    AlertRuleNameAlreadyUsedError,
     ChannelLookupTimeoutError,
     check_aggregate_column_support,
     create_alert_rule,
@@ -356,30 +355,24 @@ class AlertRuleSerializer(CamelSnakeModelSerializer):
             )
 
     def create(self, validated_data):
-        try:
-            with transaction.atomic():
-                triggers = validated_data.pop("triggers")
-                alert_rule = create_alert_rule(
-                    user=self.context.get("user", None),
-                    organization=self.context["organization"],
-                    **validated_data,
-                )
-                self._handle_triggers(alert_rule, triggers)
-                return alert_rule
-        except AlertRuleNameAlreadyUsedError:
-            raise serializers.ValidationError("This name is already in use for this organization")
+        with transaction.atomic():
+            triggers = validated_data.pop("triggers")
+            alert_rule = create_alert_rule(
+                user=self.context.get("user", None),
+                organization=self.context["organization"],
+                **validated_data,
+            )
+            self._handle_triggers(alert_rule, triggers)
+            return alert_rule
 
     def update(self, instance, validated_data):
         triggers = validated_data.pop("triggers")
         if "id" in validated_data:
             validated_data.pop("id")
-        try:
-            with transaction.atomic():
-                alert_rule = update_alert_rule(instance, **validated_data)
-                self._handle_triggers(alert_rule, triggers)
-                return alert_rule
-        except AlertRuleNameAlreadyUsedError:
-            raise serializers.ValidationError("This name is already in use for this organization")
+        with transaction.atomic():
+            alert_rule = update_alert_rule(instance, **validated_data)
+            self._handle_triggers(alert_rule, triggers)
+            return alert_rule
 
     def _handle_triggers(self, alert_rule, triggers):
         channel_lookup_timeout_error = None

+ 0 - 60
tests/sentry/incidents/test_logic.py

@@ -22,7 +22,6 @@ from sentry.incidents.logic import (
     DEFAULT_CMP_ALERT_RULE_RESOLUTION,
     WARNING_TRIGGER_LABEL,
     WINDOWED_STATS_DATA_POINTS,
-    AlertRuleNameAlreadyUsedError,
     AlertRuleTriggerLabelAlreadyUsedError,
     InvalidTriggerActionError,
     ProjectsNotAssociatedWithAlertRuleError,
@@ -475,59 +474,6 @@ class CreateAlertRuleTest(TestCase, BaseIncidentsTest):
                 1,
             )
 
-    def test_existing_name(self):
-        name = "uh oh"
-        create_alert_rule(
-            self.organization,
-            [self.project],
-            name,
-            "level:error",
-            "count()",
-            1,
-            AlertRuleThresholdType.ABOVE,
-            1,
-        )
-        with self.assertRaises(AlertRuleNameAlreadyUsedError):
-            create_alert_rule(
-                self.organization,
-                [self.project],
-                name,
-                "level:error",
-                "count()",
-                1,
-                AlertRuleThresholdType.ABOVE,
-                1,
-            )
-
-    def test_existing_name_allowed_when_archived(self):
-        name = "allowed"
-        alert_rule_1 = create_alert_rule(
-            self.organization,
-            [self.project],
-            name,
-            "level:error",
-            "count()",
-            1,
-            AlertRuleThresholdType.ABOVE,
-            1,
-        )
-        alert_rule_1.update(status=AlertRuleStatus.SNAPSHOT.value)
-
-        alert_rule_2 = create_alert_rule(
-            self.organization,
-            [self.project],
-            name,
-            "level:error",
-            "count()",
-            1,
-            AlertRuleThresholdType.ABOVE,
-            1,
-        )
-
-        assert alert_rule_1.name == alert_rule_2.name
-        assert alert_rule_1.status == AlertRuleStatus.SNAPSHOT.value
-        assert alert_rule_2.status == AlertRuleStatus.PENDING.value
-
     # This test will fail unless real migrations are run. Refer to migration 0061.
     @pytest.mark.skipif(
         not settings.MIGRATIONS_TEST_MIGRATE, reason="requires custom migration 0061"
@@ -662,12 +608,6 @@ class UpdateAlertRuleTest(TestCase, BaseIncidentsTest):
         alert_rule = update_alert_rule(self.alert_rule, query="")
         assert alert_rule.snuba_query.query == ""
 
-    def test_name_used(self):
-        used_name = "uh oh"
-        self.create_alert_rule(name=used_name)
-        with self.assertRaises(AlertRuleNameAlreadyUsedError):
-            update_alert_rule(self.alert_rule, name=used_name)
-
     def test_invalid_query(self):
         with self.assertRaises(InvalidSearchQuery):
             update_alert_rule(self.alert_rule, query="has:")