fix(crons): Prioritize lookup by slug over guid (#71300)

In GH-69690 we unintentionally introduced a bug where monitors with
slugs that are valid UUIDs (that differ from the monitor guid (id))
could not be loaded via APIs.

This is because we prioritized fetching by GUID when the passed
slug_or_id when the value was a UUID.

src/sentry/monitors/endpoints/base.py

@@ -19,14 +19,6 @@ from sentry.utils.sdk import bind_organization_context, configure_scope
 DEPRECATED_INGEST_API_MESSAGE = "We have removed this deprecated API. Please migrate to using DSN instead: https://docs.sentry.io/product/crons/legacy-endpoint-migration/#am-i-using-legacy-endpoints"
 
 
-def is_uuid(monitor_id_or_slug: str | int) -> bool:
-    try:
-        uuid_obj = UUID(str(monitor_id_or_slug), version=4)
-        return str(uuid_obj) == str(monitor_id_or_slug)
-    except ValueError:
-        return False
-
-
 class OrganizationMonitorPermission(OrganizationPermission):
     scope_map = {
         "GET": ["org:read", "org:write", "org:admin"],
@@ -57,7 +49,7 @@ class MonitorEndpoint(Endpoint):
         self,
         request: Request,
         organization_id_or_slug: int | str,
-        monitor_id_or_slug: int | str,
+        monitor_id_or_slug: str,
         environment: str | None = None,
         checkin_id: str | None = None,
         *args,
@@ -122,24 +114,35 @@ class ProjectMonitorEndpoint(ProjectEndpoint):
     def convert_args(
         self,
         request: Request,
-        monitor_id_or_slug: int | str,
+        monitor_id_or_slug: str,
         *args,
         **kwargs,
     ):
         args, kwargs = super().convert_args(request, *args, **kwargs)
+
+        # Try lookup by slug
         try:
-            if is_uuid(monitor_id_or_slug):
-                kwargs["monitor"] = Monitor.objects.get(
-                    project_id=kwargs["project"].id, guid=monitor_id_or_slug
-                )
-            else:
-                kwargs["monitor"] = Monitor.objects.get(
-                    project_id=kwargs["project"].id, slug=monitor_id_or_slug
-                )
+            kwargs["monitor"] = Monitor.objects.get(
+                project_id=kwargs["project"].id, slug=monitor_id_or_slug
+            )
+            return args, kwargs
         except Monitor.DoesNotExist:
-            raise ResourceDoesNotExist
+            pass
 
-        return args, kwargs
+        # Try lookup by GUID if the monitor_id_or_slug looks like a UUID
+        try:
+            UUID(monitor_id_or_slug, version=4)
+            kwargs["monitor"] = Monitor.objects.get(
+                project_id=kwargs["project"].id, guid=monitor_id_or_slug
+            )
+            return args, kwargs
+        except ValueError:
+            # monitor_id_or_slug does not look like a GUID
+            pass
+        except Monitor.DoesNotExist:
+            pass
+
+        raise ResourceDoesNotExist
 
 
 class ProjectMonitorCheckinEndpoint(ProjectMonitorEndpoint):
@@ -197,25 +200,32 @@ class ProjectMonitorEnvironmentEndpoint(ProjectMonitorEndpoint):
         return args, kwargs
 
 
-def get_monitor_by_org_id_or_slug(
-    organization: Organization, monitor_id_or_slug: int | str
-) -> Monitor:
+def get_monitor_by_org_id_or_slug(organization: Organization, monitor_id_or_slug: str) -> Monitor:
     # Since we have changed our unique constraints to be on unique on (project, slug) we can
     # end up with multiple monitors here. Since we have no idea which project the user wants,
     # we just get the oldest monitor and use that.
     # This is a temporary measure until we remove these org level endpoints
-    if is_uuid(monitor_id_or_slug):
+
+    # Try lookup by slug
+    monitors = list(
+        Monitor.objects.filter(organization_id=organization.id, slug=monitor_id_or_slug)
+    )
+
+    if monitors:
+        return min(monitors, key=lambda m: m.id)
+
+    # Try lookup by GUID if the monitor_id_or_slug looks like a UUID
+    try:
+        UUID(monitor_id_or_slug, version=4)
         monitors = list(
             Monitor.objects.filter(organization_id=organization.id, guid=monitor_id_or_slug)
         )
-    else:
-        monitors = list(
-            Monitor.objects.filter(organization_id=organization.id, slug=monitor_id_or_slug)
-        )
-    if not monitors:
-        raise Monitor.DoesNotExist
+        if monitors:
+            return min(monitors, key=lambda m: m.id)
+    except ValueError:
+        pass
 
-    return min(monitors, key=lambda m: m.id)
+    raise Monitor.DoesNotExist
 
 
 def try_checkin_lookup(monitor: Monitor, checkin_id: str):

tests/sentry/monitors/endpoints/test_base_monitor_details.py

@@ -56,6 +56,22 @@ class BaseMonitorDetailsTest(MonitorTestCase):
         uuid = UUID("00000000-0000-0000-0000-000000000000")
         self.get_error_response(self.organization.slug, uuid, status_code=404)
 
+    @override_options({"api.id-or-slug-enabled": True})
+    def test_simple_with_uuid_like_slug(self):
+        """
+        When the slug looks like a UUID we still want to make sure we're
+        prioritizing slug lookup
+        """
+        monitor = self._create_monitor(slug="8a65d0f2-b7d9-4b4a-9436-3de9db3c9e2f")
+
+        # We can still find our monitor by it's slug
+        resp = self.get_success_response(self.organization.slug, monitor.slug)
+        assert resp.data["slug"] == monitor.slug
+
+        # We can still find our monitor by it's id
+        resp = self.get_success_response(self.organization.slug, monitor.guid)
+        assert resp.data["slug"] == monitor.slug
+
     def test_mismatched_org_slugs(self):
         monitor = self._create_monitor()
         self.get_error_response("asdf", monitor.slug, status_code=404)