Browse Source

fix(crons): Add option to specify monitors by guid in bulk edit (#72375)

Allows for the client to specify a list of monitor guids which
correspond to the monitors they are trying to edit. Keeping the old
functionality as we transition the frontend to use the new request
payload

Related to: https://github.com/getsentry/team-crons/issues/184
David Wang 8 months ago
parent
commit
31b4f587e8

+ 22 - 5
src/sentry/monitors/endpoints/organization_monitor_index.py

@@ -46,7 +46,11 @@ from sentry.monitors.serializers import (
     MonitorSerializerResponse,
 )
 from sentry.monitors.utils import create_issue_alert_rule, signal_monitor_created
-from sentry.monitors.validators import MonitorBulkEditValidator, MonitorValidator
+from sentry.monitors.validators import (
+    MonitorBulkEditValidator,
+    MonitorBulkEditValidatorLegacy,
+    MonitorValidator,
+)
 from sentry.search.utils import tokenize_query
 from sentry.types.actor import Actor
 from sentry.utils.outcomes import Outcome
@@ -362,7 +366,13 @@ class OrganizationMonitorIndexEndpoint(OrganizationEndpoint):
         """
         Bulk edit the muted and disabled status of a list of monitors determined by slug
         """
-        validator = MonitorBulkEditValidator(
+        validator_cls = MonitorBulkEditValidatorLegacy
+
+        req_has_ids = "ids" in request.data
+        if req_has_ids:
+            validator_cls = MonitorBulkEditValidator
+
+        validator = validator_cls(
             data=request.data,
             partial=True,
             context={
@@ -374,10 +384,17 @@ class OrganizationMonitorIndexEndpoint(OrganizationEndpoint):
             return self.respond(validator.errors, status=400)
 
         result = dict(validator.validated_data)
-        monitor_slugs = result.pop("slugs")
-        status = result.get("status")
 
-        monitors = Monitor.objects.filter(slug__in=monitor_slugs, organization_id=organization.id)
+        if req_has_ids:
+            monitor_guids = result.pop("ids", [])
+            monitors = Monitor.objects.filter(guid__in=monitor_guids)
+        else:
+            monitor_slugs = result.pop("slugs", [])
+            monitors = Monitor.objects.filter(
+                slug__in=monitor_slugs, organization_id=organization.id
+            )
+
+        status = result.get("status")
         # If enabling monitors, ensure we can assign all before moving forward
         if status == ObjectStatus.ACTIVE:
             assign_result = quotas.backend.check_assign_monitor_seats(monitors)

+ 15 - 1
src/sentry/monitors/validators.py

@@ -396,7 +396,7 @@ class MonitorCheckInValidator(serializers.Serializer):
         return attrs
 
 
-class MonitorBulkEditValidator(MonitorValidator):
+class MonitorBulkEditValidatorLegacy(MonitorValidator):
     slugs = serializers.ListField(
         child=SentrySerializerSlugField(
             max_length=MAX_SLUG_LENGTH,
@@ -410,3 +410,17 @@ class MonitorBulkEditValidator(MonitorValidator):
         ).count() != len(value):
             raise ValidationError("Not all slugs are valid for this organization.")
         return value
+
+
+class MonitorBulkEditValidator(MonitorValidator):
+    ids = serializers.ListField(
+        child=serializers.UUIDField(format="hex"),
+        required=True,
+    )
+
+    def validate_ids(self, value):
+        if Monitor.objects.filter(
+            guid__in=value, organization_id=self.context["organization"].id
+        ).count() != len(value):
+            raise ValidationError("Not all ids are valid for this organization.")
+        return value

+ 83 - 3
tests/sentry/monitors/endpoints/test_organization_monitor_index.py

@@ -538,7 +538,7 @@ class BulkEditOrganizationMonitorTest(MonitorTestCase):
             ]
         }
 
-    def test_bulk_mute_unmute(self):
+    def test_bulk_mute_unmute_legacy(self):
         monitor_one = self._create_monitor(slug="monitor_one")
         monitor_two = self._create_monitor(slug="monitor_two")
 
@@ -566,7 +566,7 @@ class BulkEditOrganizationMonitorTest(MonitorTestCase):
         assert not monitor_one.is_muted
         assert not monitor_two.is_muted
 
-    def test_bulk_disable_enable(self):
+    def test_bulk_disable_enable_legacy(self):
         monitor_one = self._create_monitor(slug="monitor_one")
         monitor_two = self._create_monitor(slug="monitor_two")
         data = {
@@ -595,7 +595,7 @@ class BulkEditOrganizationMonitorTest(MonitorTestCase):
         assert monitor_two.status == ObjectStatus.ACTIVE
 
     @patch("sentry.quotas.backend.check_assign_monitor_seats")
-    def test_enable_no_quota(self, check_assign_monitor_seats):
+    def test_enable_no_quota_legacy(self, check_assign_monitor_seats):
         monitor_one = self._create_monitor(slug="monitor_one", status=ObjectStatus.DISABLED)
         monitor_two = self._create_monitor(slug="monitor_two", status=ObjectStatus.DISABLED)
         result = SeatAssignmentResult(
@@ -617,3 +617,83 @@ class BulkEditOrganizationMonitorTest(MonitorTestCase):
         monitor_two.refresh_from_db()
         assert monitor_one.status == ObjectStatus.DISABLED
         assert monitor_two.status == ObjectStatus.DISABLED
+
+    def test_bulk_mute_unmute(self):
+        monitor_one = self._create_monitor(slug="monitor_one")
+        monitor_two = self._create_monitor(slug="monitor_two")
+
+        data = {
+            "ids": [monitor_one.guid, monitor_two.guid],
+            "isMuted": True,
+        }
+        response = self.get_success_response(self.organization.slug, **data)
+        assert response.status_code == 200
+
+        monitor_one.refresh_from_db()
+        monitor_two.refresh_from_db()
+        assert monitor_one.is_muted
+        assert monitor_two.is_muted
+
+        data = {
+            "ids": [monitor_one.guid, monitor_two.guid],
+            "isMuted": False,
+        }
+        response = self.get_success_response(self.organization.slug, **data)
+        assert response.status_code == 200
+
+        monitor_one.refresh_from_db()
+        monitor_two.refresh_from_db()
+        assert not monitor_one.is_muted
+        assert not monitor_two.is_muted
+
+    def test_bulk_disable_enable(self):
+        monitor_one = self._create_monitor(slug="monitor_one")
+        monitor_two = self._create_monitor(slug="monitor_two")
+        data = {
+            "ids": [monitor_one.guid, monitor_two.guid],
+            "status": "disabled",
+        }
+        response = self.get_success_response(self.organization.slug, **data)
+        assert response.status_code == 200
+
+        monitor_one.refresh_from_db()
+        monitor_two.refresh_from_db()
+        assert monitor_one.status == ObjectStatus.DISABLED
+        assert monitor_two.status == ObjectStatus.DISABLED
+
+        data = {
+            "ids": [monitor_one.guid, monitor_two.guid],
+            "status": "active",
+        }
+        response = self.get_success_response(self.organization.slug, **data)
+
+        assert response.status_code == 200
+
+        monitor_one.refresh_from_db()
+        monitor_two.refresh_from_db()
+        assert monitor_one.status == ObjectStatus.ACTIVE
+        assert monitor_two.status == ObjectStatus.ACTIVE
+
+    @patch("sentry.quotas.backend.check_assign_monitor_seats")
+    def test_enable_no_quota(self, check_assign_monitor_seats):
+        monitor_one = self._create_monitor(slug="monitor_one", status=ObjectStatus.DISABLED)
+        monitor_two = self._create_monitor(slug="monitor_two", status=ObjectStatus.DISABLED)
+        result = SeatAssignmentResult(
+            assignable=False,
+            reason="Over quota",
+        )
+        check_assign_monitor_seats.return_value = result
+
+        data = {
+            "ids": [monitor_one.guid, monitor_two.guid],
+            "status": "active",
+        }
+        response = self.get_error_response(self.organization.slug, **data)
+        assert response.status_code == 400
+        assert response.data == "Over quota"
+
+        # Verify monitors are still disabled
+        monitor_one.refresh_from_db()
+        monitor_two.refresh_from_db()
+        assert monitor_one.status == ObjectStatus.DISABLED
+        assert monitor_two.status == ObjectStatus.DISABLED