fix(suspect commits): Add commitId to RELEASE_BASED suspect commits

src/sentry/tasks/groupowner.py

@@ -78,13 +78,17 @@ def _process_suspect_commits(
                     project, group_id, event_frames, event_platform, sdk_name=sdk_name
                 )
             owner_scores: dict[str, int] = {}
+            owner_commits: dict[str, int] = {}
             for committer in committers:
                 author = cast(UserSerializerResponse, committer["author"])
                 if author and "id" in author:
                     author_id = author["id"]
-                    for _, score in committer["commits"]:
+                    for commit, score in committer["commits"]:
                         if score >= MIN_COMMIT_SCORE:
-                            owner_scores[author_id] = max(score, owner_scores.get(author_id, 0))
+                            current_score = owner_scores.get(author_id, 0)
+                            if score > current_score:
+                                owner_scores[author_id] = score
+                                owner_commits[author_id] = commit.id
 
             if owner_scores:
                 for owner_id, _ in sorted(
@@ -99,11 +103,13 @@ def _process_suspect_commits(
                                     "user_id": owner_id,
                                     "project_id": project.id,
                                     "organization_id": project.organization_id,
+                                    "context__asjsonb__commitId": owner_commits[owner_id],
                                 },
                                 defaults={
                                     "date_added": timezone.now(),
                                 },
                                 context_defaults={
+                                    "commitId": owner_commits[owner_id],
                                     "suspectCommitStrategy": SuspectCommitStrategy.RELEASE_BASED,
                                 },
                             )

tests/sentry/api/endpoints/test_event_committers.py

@@ -291,3 +291,51 @@ def test_endpoint_with_no_user_groupowner(self) -> None:
         )
         assert "group_owner_id" in response.data["committers"][0]
         assert response.data["committers"][0]["group_owner_id"] == group_owner.id
+
+    def test_release_based_suspect_commit_displayed(self) -> None:
+        """Test that RELEASE_BASED suspect commits are displayed via the endpoint."""
+        self.login_as(user=self.user)
+        project = self.create_project()
+
+        repo = self.create_repo(project=project, name="example/repo")
+        release = self.create_release(project=project, version="v1.0")
+        commit = self.create_commit(project=project, repo=repo)
+        release.set_commits([{"id": commit.key, "repository": repo.name}])
+
+        min_ago = before_now(minutes=1).isoformat()
+        event = self.store_event(
+            data={"fingerprint": ["group1"], "timestamp": min_ago},
+            project_id=project.id,
+            default_event_type=EventType.DEFAULT,
+        )
+        assert event.group is not None
+
+        GroupOwner.objects.create(
+            group_id=event.group.id,
+            project=project,
+            organization_id=project.organization_id,
+            type=GroupOwnerType.SUSPECT_COMMIT.value,
+            user_id=self.user.id,
+            context={
+                "commitId": commit.id,
+                "suspectCommitStrategy": SuspectCommitStrategy.RELEASE_BASED,
+            },
+        )
+
+        url = reverse(
+            "sentry-api-0-event-file-committers",
+            kwargs={
+                "event_id": event.event_id,
+                "project_id_or_slug": event.project.slug,
+                "organization_id_or_slug": event.project.organization.slug,
+            },
+        )
+
+        response = self.client.get(url, format="json")
+        assert response.status_code == 200, response.content
+        assert len(response.data["committers"]) == 1
+
+        commits = response.data["committers"][0]["commits"]
+        assert len(commits) == 1
+        assert commits[0]["id"] == commit.key
+        assert commits[0]["suspectCommitType"] == "via commit in release"

tests/sentry/tasks/test_groupowner.py

@@ -3,6 +3,7 @@
 from django.utils import timezone
 
 from sentry.deletions.tasks.hybrid_cloud import schedule_hybrid_cloud_foreign_key_jobs
+from sentry.models.commit import Commit
 from sentry.models.groupowner import GroupOwner, GroupOwnerType, SuspectCommitStrategy
 from sentry.models.grouprelease import GroupRelease
 from sentry.models.repository import Repository
@@ -69,10 +70,11 @@ def setUp(self) -> None:
         )
 
     def set_release_commits(self, author_email):
+        self.commitSha = "a" * 40
         self.release.set_commits(
             [
                 {
-                    "id": "a" * 40,
+                    "id": self.commitSha,
                     "repository": self.repo.name,
                     "author_email": author_email,
                     "author_name": "Bob",
@@ -270,6 +272,7 @@ def test_delete_old_entries(self) -> None:
     def test_update_existing_entries(self) -> None:
         # As new events come in associated with existing owners, we should update the date_added of that owner.
         self.set_release_commits(self.user.email)
+        suspect_commit = Commit.objects.get(key=self.commitSha)
         event_frames = get_frame_paths(self.event)
         process_suspect_commits(
             event_id=self.event.event_id,
@@ -286,7 +289,9 @@ def test_update_existing_entries(self) -> None:
         )
 
         date_added_before_update = go.date_added
-        assert go.context == {"suspectCommitStrategy": SuspectCommitStrategy.RELEASE_BASED}
+        assert "commitId" in go.context
+        assert go.context["commitId"] == suspect_commit.id
+        assert go.context["suspectCommitStrategy"] == SuspectCommitStrategy.RELEASE_BASED
 
         process_suspect_commits(
             event_id=self.event.event_id,
@@ -297,7 +302,9 @@ def test_update_existing_entries(self) -> None:
         )
         go.refresh_from_db()
         assert go.date_added > date_added_before_update
-        assert go.context == {"suspectCommitStrategy": SuspectCommitStrategy.RELEASE_BASED}
+        assert "commitId" in go.context
+        assert go.context["commitId"] == suspect_commit.id
+        assert go.context["suspectCommitStrategy"] == SuspectCommitStrategy.RELEASE_BASED
         assert GroupOwner.objects.filter(group=self.event.group).count() == 1
         assert GroupOwner.objects.get(
             group=self.event.group,
@@ -341,9 +348,14 @@ def test_no_release_or_commit(self) -> None:
     def test_keep_highest_score(self, patched_committers: MagicMock) -> None:
         self.user2 = self.create_user(email="user2@sentry.io")
         self.user3 = self.create_user(email="user3@sentry.io")
+
+        mock_commit1 = MagicMock(id=1)
+        mock_commit2 = MagicMock(id=2)
+        mock_commit3 = MagicMock(id=3)
+
         patched_committers.return_value = [
             {
-                "commits": [(None, 3)],
+                "commits": [(mock_commit1, 3)],
                 "author": {
                     "username": self.user.email,
                     "lastLogin": None,
@@ -367,7 +379,7 @@ def test_keep_highest_score(self, patched_committers: MagicMock) -> None:
                 },
             },
             {
-                "commits": [(None, 1)],
+                "commits": [(mock_commit2, 1)],
                 "author": {
                     "username": self.user2.email,
                     "lastLogin": None,
@@ -391,7 +403,7 @@ def test_keep_highest_score(self, patched_committers: MagicMock) -> None:
                 },
             },
             {
-                "commits": [(None, 2)],
+                "commits": [(mock_commit3, 2)],
                 "author": {
                     "username": self.user3.email,
                     "lastLogin": None,
@@ -424,17 +436,21 @@ def test_keep_highest_score(self, patched_committers: MagicMock) -> None:
             project_id=self.event.project_id,
         )
         # Doesn't use self.user2 due to low score.
-        assert GroupOwner.objects.get(user_id=self.user.id)
-        assert GroupOwner.objects.get(user_id=self.user3.id)
+        user1_owner = GroupOwner.objects.get(user_id=self.user.id)
+        user3_owner = GroupOwner.objects.get(user_id=self.user3.id)
         assert not GroupOwner.objects.filter(user_id=self.user2.id).exists()
 
+        assert user1_owner.context["commitId"] == 1
+        assert user3_owner.context["commitId"] == 3
+
     @patch("sentry.tasks.groupowner.get_event_file_committers")
     def test_low_suspect_committer_score(self, patched_committers: MagicMock) -> None:
         self.user = self.create_user()
+        mock_commit = MagicMock(id=1)
         patched_committers.return_value = [
             {
                 # score < MIN_COMMIT_SCORE
-                "commits": [(None, 1)],
+                "commits": [(mock_commit, 1)],
                 "author": {
                     "id": self.user.id,
                 },
@@ -472,40 +488,3 @@ def test_owners_count(self) -> None:
         )
 
         assert owners.count() == 1
-
-    def test_external_author_no_user(self) -> None:
-        """
-        Test _process_suspect_commits with external commit author (no Sentry user).
-        TODO(nora): This test should FAIL until I update the code to handle external authors.
-        """
-        # Set up a commit with external author email (not a Sentry user)
-        external_email = "external@company.com"
-        self.set_release_commits(external_email)
-
-        assert not GroupOwner.objects.filter(group=self.event.group).exists()
-        event_frames = get_frame_paths(self.event)
-
-        # Test: process_suspect_commits should handle external authors.
-        # Currently this will NOT create a GroupOwner because the code
-        # only processes authors that have existing Sentry user accounts.
-        process_suspect_commits(
-            event_id=self.event.event_id,
-            event_platform=self.event.platform,
-            event_frames=event_frames,
-            group_id=self.event.group_id,
-            project_id=self.event.project_id,
-        )
-
-        # Once the code is updated, this should pass:
-        # group_owners = GroupOwner.objects.filter(
-        #     group=self.event.group,
-        #     project=self.event.project,
-        #     organization=self.event.project.organization,
-        #     type=GroupOwnerType.SUSPECT_COMMIT.value,
-        # )
-        # assert group_owners.count() == 1
-        # group_owner = group_owners[0]
-        # assert group_owner.user_id is None  # No Sentry user mapping
-        # assert (
-        #     group_owner.context.get("suspectCommitStrategy") == SuspectCommitStrategy.RELEASE_BASED
-        # )