incorporate pr comments

Author: suejungshin

fixtures/github.py

@@ -3548,6 +3548,7 @@
 # Simplified example of a check_run rerequested action event
 # Note: installation.id must match the external_id used in create_github_integration (default: "12345")
 # Note: repository.id must match a Repository created for the organization (use create_repo in tests)
+# Note: check_run events are linked to PRs via check_run.pull_requests - we use user.id as fallback for billing
 CHECK_RUN_REREQUESTED_ACTION_EVENT_EXAMPLE = b"""{
     "action": "rerequested",
     "installation": {"id": 12345},
@@ -3558,7 +3559,12 @@
     },
     "check_run": {
         "external_id": "4663713",
-        "html_url": "https://github.com/test/repo/runs/4"
+        "html_url": "https://github.com/test/repo/runs/4",
+        "pull_requests": []
+    },
+    "user": {
+        "id": 12345678,
+        "login": "test-user"
     }
 }"""
 
@@ -3569,5 +3575,9 @@
         "id": 35129377,
         "full_name": "getsentry/sentry",
         "html_url": "https://github.com/getsentry/sentry"
+    },
+    "user": {
+        "id": 12345678,
+        "login": "test-user"
     }
 }"""

src/sentry/integrations/github/webhook.py

@@ -86,6 +86,7 @@ def __call__(
         event: Mapping[str, Any],
         organization: Organization,
         repo: Repository,
+        integration: RpcIntegration | None = None,
         **kwargs: Any,
     ) -> None: ...
 

src/sentry/models/repositorysettings.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+from dataclasses import dataclass
 from enum import StrEnum
 
 from django.contrib.postgres.fields.array import ArrayField
@@ -19,6 +20,14 @@ def as_choices(cls) -> tuple[tuple[str, str], ...]:
         return tuple((trigger.value, trigger.value) for trigger in cls)
 
 
+@dataclass
+class CodeReviewSettings:
+    """Settings for code review functionality on a repository."""
+
+    enabled: bool
+    triggers: list[CodeReviewTrigger]
+
+
 @region_silo_model
 class RepositorySettings(Model):
     """
@@ -41,3 +50,8 @@ class Meta:
         db_table = "sentry_repositorysettings"
 
     __repr__ = sane_repr("repository_id", "enabled_code_review")
+
+    def get_code_review_settings(self) -> CodeReviewSettings:
+        """Return code review settings for this repository."""
+        triggers = [CodeReviewTrigger(t) for t in self.code_review_triggers]
+        return CodeReviewSettings(enabled=self.enabled_code_review, triggers=triggers)

src/sentry/overwatch/endpoints/overwatch_rpc.py

@@ -11,21 +11,20 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
-from sentry import features, quotas
+from sentry import features
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.authentication import AuthenticationSiloLimit, StandardAuthentication
 from sentry.api.base import Endpoint, region_silo_endpoint
-from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, DataCategory, ObjectStatus
+from sentry.constants import DEFAULT_CODE_REVIEW_TRIGGERS, ObjectStatus
 from sentry.integrations.services.integration import integration_service
 from sentry.models.organization import Organization
-from sentry.models.organizationcontributors import OrganizationContributors
 from sentry.models.repository import Repository
 from sentry.models.repositorysettings import RepositorySettings
 from sentry.prevent.models import PreventAIConfiguration
 from sentry.prevent.types.config import PREVENT_AI_CONFIG_DEFAULT, PREVENT_AI_CONFIG_DEFAULT_V1
+from sentry.seer.code_review.billing import passes_code_review_billing_check
 from sentry.silo.base import SiloMode
-from sentry.utils import metrics
 from sentry.utils.seer import can_use_prevent_ai_features
 
 logger = logging.getLogger(__name__)
@@ -309,32 +308,11 @@ def _is_eligible_for_code_review(
     if not code_review_enabled:
         return False
 
-    # Check if contributor exists, and if there's either a seat or quota available.
-    # NOTE: We explicitly check billing as the source of truth because if the contributor exists,
-    # then that means that they've opened a PR before, and either have a seat already OR it's their
-    # "Free action."
-    try:
-        contributor = OrganizationContributors.objects.get(
-            organization_id=organization.id,
-            integration_id=integration_id,
-            external_identifier=external_identifier,
-        )
-
-        if not quotas.backend.check_seer_quota(
-            org_id=organization.id,
-            data_category=DataCategory.SEER_USER,
-            seat_object=contributor,
-        ):
-            return False
-
-    except OrganizationContributors.DoesNotExist:
-        metrics.incr(
-            "overwatch.code_review.contributor_not_found",
-            tags={"organization_id": organization.id, "repository_id": repository_id},
-        )
-        return False
-
-    return True
+    return passes_code_review_billing_check(
+        organization_id=organization.id,
+        integration_id=integration_id,
+        external_identifier=external_identifier,
+    )
 
 
 @region_silo_endpoint

src/sentry/seer/code_review/billing.py

@@ -0,0 +1,39 @@
+from __future__ import annotations
+
+from sentry import quotas
+from sentry.constants import DataCategory
+from sentry.models.organizationcontributors import OrganizationContributors
+from sentry.utils import metrics
+
+
+def passes_code_review_billing_check(
+    organization_id: int,
+    integration_id: int,
+    external_identifier: str,
+) -> bool:
+    """
+    Check if contributor exists, and if there's either a seat or quota available.
+    NOTE: We explicitly check billing as the source of truth because if the contributor exists,
+    then that means that they've opened a PR before, and either have a seat already OR it's their
+    "Free action."
+
+    Returns False if the billing check does not pass for code review feature.
+    """
+    try:
+        contributor = OrganizationContributors.objects.get(
+            organization_id=organization_id,
+            integration_id=integration_id,
+            external_identifier=external_identifier,
+        )
+    except OrganizationContributors.DoesNotExist:
+        metrics.incr(
+            "overwatch.code_review.contributor_not_found",
+            tags={"organization_id": organization_id},
+        )
+        return False
+
+    return quotas.backend.check_seer_quota(
+        org_id=organization_id,
+        data_category=DataCategory.SEER_USER,
+        seat_object=contributor,
+    )

src/sentry/seer/code_review/preflight.py

@@ -3,17 +3,13 @@
 from collections.abc import Callable
 from dataclasses import dataclass
 
-from sentry import features, quotas
-from sentry.constants import (
-    ENABLE_PR_REVIEW_TEST_GENERATION_DEFAULT,
-    HIDE_AI_FEATURES_DEFAULT,
-    DataCategory,
-)
+from sentry import features
+from sentry.constants import ENABLE_PR_REVIEW_TEST_GENERATION_DEFAULT, HIDE_AI_FEATURES_DEFAULT
 from sentry.models.organization import Organization
-from sentry.models.organizationcontributors import OrganizationContributors
 from sentry.models.repository import Repository
+from sentry.models.repositorysettings import CodeReviewSettings, RepositorySettings
 
-from .settings import CodeReviewSettings, get_code_review_settings
+from .billing import passes_code_review_billing_check
 
 DenialReason = str | None
 
@@ -31,14 +27,15 @@ def __init__(
         organization: Organization,
         repo: Repository,
         integration_id: int | None = None,
-        external_identifier: str | None = None,
+        pr_author_external_id: str | None = None,
     ):
         self.organization = organization
         self.repo = repo
         self.integration_id = integration_id
-        self.external_identifier = external_identifier
-        self._repo_settings: CodeReviewSettings | None = None
-        self._repo_settings_loaded = False
+        self.pr_author_external_id = pr_author_external_id
+
+        repo_settings = RepositorySettings.objects.filter(repository=repo).first()
+        self._repo_settings = repo_settings.get_code_review_settings() if repo_settings else None
 
     def check(self) -> CodeReviewPreflightResult:
         checks: list[Callable[[], DenialReason]] = [
@@ -53,13 +50,7 @@ def check(self) -> CodeReviewPreflightResult:
             if denial_reason is not None:
                 return CodeReviewPreflightResult(allowed=False, denial_reason=denial_reason)
 
-        return CodeReviewPreflightResult(allowed=True, settings=self._get_repo_settings())
-
-    def _get_repo_settings(self) -> CodeReviewSettings | None:
-        if not self._repo_settings_loaded:
-            self._repo_settings = get_code_review_settings(self.repo)
-            self._repo_settings_loaded = True
-        return self._repo_settings
+        return CodeReviewPreflightResult(allowed=True, settings=self._repo_settings)
 
     # -------------------------------------------------------------------------
     # Checks - each returns denial reason (str) or None if valid
@@ -90,8 +81,7 @@ def _check_org_feature_enablement(self) -> DenialReason:
 
     def _check_repo_feature_enablement(self) -> DenialReason:
         if self._is_seat_based_seer_plan_org():
-            settings = self._get_repo_settings()
-            if settings is None or not settings.enabled:
+            if self._repo_settings is None or not self._repo_settings.enabled:
                 return "repo_code_review_disabled"
             return None
         elif self._is_code_review_beta_org():
@@ -101,28 +91,15 @@ def _check_repo_feature_enablement(self) -> DenialReason:
             return "repo_code_review_disabled"
 
     def _check_billing(self) -> DenialReason:
-        # Check if contributor exists, and if there's either a seat or quota available.
-        # NOTE: We explicitly check billing as the source of truth because if the contributor exists,
-        # then that means that they've opened a PR before, and either have a seat already OR it's their
-        # "Free action."
-        if self.integration_id is None or self.external_identifier is None:
+        if self.integration_id is None or self.pr_author_external_id is None:
             return "billing_missing_contributor_info"
 
-        try:
-            contributor = OrganizationContributors.objects.get(
-                organization_id=self.organization.id,
-                integration_id=self.integration_id,
-                external_identifier=self.external_identifier,
-            )
-        except OrganizationContributors.DoesNotExist:
-            return "billing_contributor_not_found"
-
-        has_quota = quotas.backend.check_seer_quota(
-            org_id=self.organization.id,
-            data_category=DataCategory.SEER_USER,
-            seat_object=contributor,
+        billing_ok = passes_code_review_billing_check(
+            organization_id=self.organization.id,
+            integration_id=self.integration_id,
+            external_identifier=self.pr_author_external_id,
         )
-        if not has_quota:
+        if not billing_ok:
             return "billing_quota_exceeded"
 
         return None

src/sentry/seer/code_review/settings.py

@@ -123,3 +123,23 @@ def transform_webhook_to_codegen_request(
         "request_type": request_type,
         "organization_id": organization_id,
     }
+
+
+def get_pr_author_id(event: Mapping[str, Any]) -> str | None:
+    """
+    Extract the PR author's GitHub user ID from the webhook payload.
+    The user information can be found in different locations depending on the webhook type.
+    """
+    # Check issue.user.id (for issue comments on PRs)
+    if (user_id := event.get("issue", {}).get("user", {}).get("id")) is not None:
+        return str(user_id)
+
+    # Check pull_request.user.id (for pull request events)
+    if (user_id := event.get("pull_request", {}).get("user", {}).get("id")) is not None:
+        return str(user_id)
+
+    # Check user.id (fallback for direct user events)
+    if (user_id := event.get("user", {}).get("id")) is not None:
+        return str(user_id)
+
+    return None

src/sentry/seer/code_review/utils.py

@@ -22,7 +22,6 @@
 from sentry.models.repository import Repository
 from sentry.utils import metrics
 
-from ..preflight import CodeReviewPreflightService
 from ..utils import SeerEndpoint, make_seer_request
 
 logger = logging.getLogger(__name__)
@@ -31,7 +30,6 @@
 class ErrorStatus(enum.StrEnum):
     MISSING_ORGANIZATION = "missing_organization"
     MISSING_ACTION = "missing_action"
-    CODE_REVIEW_NOT_ENABLED = "code_review_not_enabled"
     INVALID_PAYLOAD = "invalid_payload"
 
 
@@ -118,14 +116,6 @@ def handle_check_run_event(
     if action != GitHubCheckRunAction.REREQUESTED:
         return
 
-    preflight = CodeReviewPreflightService(organization, repo).check()
-    if not preflight.allowed:
-        metrics.incr(
-            f"{Metrics.ERROR.value}",
-            tags={**tags, "error_status": ErrorStatus.CODE_REVIEW_NOT_ENABLED.value},
-        )
-        return
-
     try:
         validated_event = _validate_github_check_run_event(event)
     except (ValidationError, ValueError):