fix(signals): return all available reviewers, not just the first 100

products/signals/backend/test/test_signal_report_api.py

@@ -1,10 +1,13 @@
 import json
+import uuid
 from datetime import timedelta
+from types import SimpleNamespace
 from urllib.parse import urlencode
 
 from posthog.test.base import APIBaseTest
 from unittest.mock import patch
 
+from django.core.cache import cache
 from django.utils import timezone
 
 from parameterized import parameterized
@@ -1209,3 +1212,96 @@ def test_restore_preserves_title_and_summary(self):
         assert report.status == SignalReport.Status.READY
         assert report.title == "Original title"
         assert report.summary == "Original summary"
+
+
+class TestAvailableReviewersAPI(APIBaseTest):
+    """GET signals/reports/available_reviewers/: returns every eligible org member (no cap), with server-side search."""
+
+    def setUp(self):
+        super().setUp()
+        # The over-threshold report is throttled via the cache; clear it so each test starts fresh.
+        cache.clear()
+
+    def _url(self, **query) -> str:
+        base = f"/api/projects/{self.team.id}/signals/reports/available_reviewers/"
+        if not query:
+            return base
+        return f"{base}?{urlencode(query)}"
+
+    def _fake_user(self, n: int) -> SimpleNamespace:
+        return SimpleNamespace(
+            uuid=uuid.UUID(int=n),
+            first_name=f"User{n:04d}",
+            last_name="Tester",
+            email=f"user{n:04d}@example.com",
+        )
+
+    def _login_map(self, count: int) -> dict[str, SimpleNamespace]:
+        return {f"gh{n}": self._fake_user(n) for n in range(count)}
+
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_returns_all_members_without_cap(self, mock_map):
+        # 250 > the old hard cap of 100: every member must come back now.
+        mock_map.return_value = self._login_map(250)
+        response = self.client.get(self._url())
+        assert response.status_code == status.HTTP_200_OK
+        assert len(response.json()) == 250
+
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_search_query_filters_server_side(self, mock_map):
+        mock_map.return_value = self._login_map(250)
+        response = self.client.get(self._url(query="User0123"))
+        assert response.status_code == status.HTTP_200_OK
+        body = response.json()
+        assert len(body) == 1
+        assert next(iter(body.values()))["email"] == "user0123@example.com"
+
+    @patch("products.signals.backend.views.capture_exception")
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_no_exception_captured_under_threshold(self, mock_map, mock_capture):
+        mock_map.return_value = self._login_map(50)
+        response = self.client.get(self._url())
+        assert response.status_code == status.HTTP_200_OK
+        mock_capture.assert_not_called()
+
+    @patch("products.signals.backend.views.capture_exception")
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_exception_captured_over_threshold(self, mock_map, mock_capture):
+        mock_map.return_value = self._login_map(1201)
+        response = self.client.get(self._url())
+        assert response.status_code == status.HTTP_200_OK
+        assert len(response.json()) == 1201
+        mock_capture.assert_called_once()
+
+    @patch("products.signals.backend.views.capture_exception")
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_threshold_capture_deduplicated_across_requests(self, mock_map, mock_capture):
+        # Repeated popover opens for the same over-threshold org must report at most once.
+        mock_map.return_value = self._login_map(1201)
+        for _ in range(3):
+            assert self.client.get(self._url()).status_code == status.HTTP_200_OK
+        mock_capture.assert_called_once()
+
+    @patch("products.signals.backend.views.capture_exception")
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_threshold_not_triggered_by_search_requests(self, mock_map, mock_capture):
+        # A search-as-you-type request must not spam the threshold capture.
+        mock_map.return_value = self._login_map(1201)
+        response = self.client.get(self._url(query="User0001"))
+        assert response.status_code == status.HTTP_200_OK
+        mock_capture.assert_not_called()
+
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_empty_org_returns_empty(self, mock_map):
+        mock_map.return_value = {}
+        response = self.client.get(self._url())
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json() == {}
+
+    @patch("products.signals.backend.views.get_org_member_github_login_to_user_map")
+    def test_missing_team_map_returns_empty(self, mock_map):
+        # The helper returns None for an unknown team; the view coalesces it to an empty result.
+        mock_map.return_value = None
+        response = self.client.get(self._url())
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json() == {}

products/signals/backend/views.py

@@ -4,6 +4,7 @@
 from typing import cast
 
 from django.conf import settings
+from django.core.cache import cache
 from django.db import IntegrityError, transaction
 from django.db.models import (
     BooleanField,
@@ -30,6 +31,7 @@
 from django_filters.rest_framework import DjangoFilterBackend
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view
+from opentelemetry import trace
 from rest_framework import exceptions, mixins, serializers, status, viewsets
 from rest_framework.authentication import SessionAuthentication
 from rest_framework.decorators import action
@@ -43,6 +45,7 @@
 
 from posthog.api.routing import TeamAndOrgViewSetMixin
 from posthog.auth import InternalAPIAuthentication, OAuthAccessTokenAuthentication, PersonalAPIKeyAuthentication
+from posthog.exceptions_capture import capture_exception
 from posthog.models import Team, User
 from posthog.models.integration import Integration
 from posthog.models.team.extensions import get_or_create_team_extension
@@ -100,6 +103,14 @@
 from products.warehouse_sources.backend.models.external_data_schema import ExternalDataSchema
 
 logger = structlog.get_logger(__name__)
+tracer = trace.get_tracer(__name__)
+
+# `available_reviewers` returns every eligible org member in a single unpaginated payload.
+# Org membership is tiny in practice, so even the biggest
+# org today serialises to well under 100 KB. If an org ever exceeds this threshold we want a
+# signal that it's time to add real pagination, rather than silently truncating the list (the
+# old behaviour, which capped at 100 and dropped everyone alphabetically after ~"M").
+REVIEWER_PAGINATION_THRESHOLD = 1200
 
 
 class EmitSignalSerializer(serializers.Serializer):
@@ -949,36 +960,65 @@ def list(self, request, *args, **kwargs):
     @extend_schema(exclude=True)
     @action(detail=False, methods=["get"], url_path="available_reviewers", required_scopes=["task:read"])
     def available_reviewers(self, request, **kwargs):
-        login_to_user = get_org_member_github_login_to_user_map(self.team.id) or {}
-        query = (request.query_params.get("query") or "").strip().lower()
-
-        users_by_uuid = {str(user.uuid): user for user in login_to_user.values()}
-
-        filtered_users = [
-            (user_uuid, user)
-            for user_uuid, user in users_by_uuid.items()
-            if not query
-            or query in f"{user.first_name} {user.last_name}".strip().lower()
-            or query in (user.email or "").lower()
-        ]
+        with tracer.start_as_current_span("signals.available_reviewers") as span:
+            login_to_user = get_org_member_github_login_to_user_map(self.team.id) or {}
+            query = (request.query_params.get("query") or "").strip().lower()
+
+            users_by_uuid = {str(user.uuid): user for user in login_to_user.values()}
+
+            candidate_count = len(users_by_uuid)
+            span.set_attribute("signals.available_reviewers.candidate_count", candidate_count)
+
+            # The full candidate list is returned unpaginated. If an org grows past the threshold,
+            # report it (non-blocking) so we know to add pagination before the payload gets large.
+            # `capture_exception` logs an exception and is not deduplicated, so we throttle to at
+            # most one report per org per day via the cache — otherwise a >threshold org would log
+            # on every popover open. (The span's candidate_count attribute is recorded every request
+            # regardless, if a metric-based alert is preferred later.)
+            if (
+                not query
+                and candidate_count > REVIEWER_PAGINATION_THRESHOLD
+                and cache.add(f"signals:available_reviewers_over_threshold:{self.team.id}", True, 60 * 60 * 24)
+            ):
+                capture_exception(
+                    Exception(
+                        f"available_reviewers exceeded pagination threshold: {candidate_count} "
+                        f"candidates > {REVIEWER_PAGINATION_THRESHOLD}; this endpoint should be paginated."
+                    ),
+                    additional_properties={
+                        "team_id": self.team.id,
+                        "candidate_count": candidate_count,
+                        "threshold": REVIEWER_PAGINATION_THRESHOLD,
+                    },
+                )
 
-        reviewers = {
-            user_uuid: {
-                "name": f"{user.first_name} {user.last_name}".strip(),
-                "email": user.email or "",
+            filtered_users = [
+                (user_uuid, user)
+                for user_uuid, user in users_by_uuid.items()
+                if not query
+                or query in f"{user.first_name} {user.last_name}".strip().lower()
+                or query in (user.email or "").lower()
+            ]
+
+            reviewers = {
+                user_uuid: {
+                    "name": f"{user.first_name} {user.last_name}".strip(),
+                    "email": user.email or "",
+                }
+                for user_uuid, user in sorted(
+                    filtered_users,
+                    key=lambda item: (
+                        (item[1].first_name or "").lower(),
+                        (item[1].last_name or "").lower(),
+                        (item[1].email or "").lower(),
+                        item[0],
+                    ),
+                )
             }
-            for user_uuid, user in sorted(
-                filtered_users,
-                key=lambda item: (
-                    (item[1].first_name or "").lower(),
-                    (item[1].last_name or "").lower(),
-                    (item[1].email or "").lower(),
-                    item[0],
-                ),
-            )[:100]
-        }
 
-        return Response(reviewers)
+            span.set_attribute("signals.available_reviewers.result_count", len(reviewers))
+
+            return Response(reviewers)
 
     def destroy(self, request, *args, **kwargs):
         """Soft-delete a report and its signals via the deletion workflow."""