fix(integrations): Fix deploy notification commits order (#35158)

Currently the order in which commits are fetched for a given release are
in the order in which they are stored in a Python set, which is
inherently unordered. This change orders commits explicitly based on the
order field from the releasecommit table.

src/sentry/models/commit.py

@@ -1,10 +1,33 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
 from django.db import models
 from django.utils import timezone
 
-from sentry.db.models import BoundedPositiveIntegerField, FlexibleForeignKey, Model, sane_repr
+from sentry.db.models import (
+    BaseManager,
+    BoundedPositiveIntegerField,
+    FlexibleForeignKey,
+    Model,
+    QuerySet,
+    sane_repr,
+)
 from sentry.utils.cache import memoize
 from sentry.utils.groupreference import find_referenced_groups
 
+if TYPE_CHECKING:
+    from sentry.models import Release
+
+
+class CommitManager(BaseManager):
+    def get_for_release(self, release: Release) -> QuerySet[Commit]:
+        return (
+            self.filter(releasecommit__release=release)
+            .order_by("-releasecommit__order")
+            .select_related("author")
+        )
+
 
 class Commit(Model):
     __include_in_export__ = False
@@ -18,6 +41,8 @@ class Commit(Model):
     author = FlexibleForeignKey("sentry.CommitAuthor", null=True)
     message = models.TextField(null=True)
 
+    objects = CommitManager()
+
     class Meta:
         app_label = "sentry"
         db_table = "sentry_commit"

src/sentry/notifications/notifications/activity/release.py

@@ -6,6 +6,7 @@ from sentry_relay import parse_release
 
 from sentry.models import (
     Activity,
+    Commit,
     CommitFileChange,
     OrganizationMember,
     Project,
@@ -15,7 +16,6 @@ from sentry.models import (
 )
 from sentry.notifications.types import NotificationSettingTypes
 from sentry.notifications.utils import (
-    get_commits_for_release,
     get_deploy,
     get_environment_for_deploy,
     get_group_counts_by_project,
@@ -40,12 +40,12 @@ class ReleaseActivityNotification(ActivityNotification):
         super().__init__(activity)
         self.group = None
         self.user_id_team_lookup: Mapping[int, list[int]] | None = None
-        self.email_list: set[str] = set()
-        self.user_ids: set[int] = set()
         self.deploy = get_deploy(activity)
         self.release = get_release(activity, self.organization)
 
         if not self.release:
+            self.email_list: set[str] = set()
+            self.user_ids: set[int] = set()
             self.repos: Iterable[Mapping[str, Any]] = set()
             self.projects: set[Project] = set()
             self.version = "unknown"
@@ -53,7 +53,7 @@ class ReleaseActivityNotification(ActivityNotification):
             return
 
         self.projects = set(self.release.projects.all())
-        self.commit_list = get_commits_for_release(self.release)
+        self.commit_list = Commit.objects.get_for_release(self.release)
         self.email_list = {c.author.email for c in self.commit_list if c.author}
         users = UserEmail.objects.get_users_by_emails(self.email_list, self.organization)
         self.user_ids = {u.id for u in users.values()}

src/sentry/notifications/utils/__init__.py

@@ -79,29 +79,24 @@ def get_group_counts_by_project(
 
 
 def get_repos(
-    commits: Iterable[Commit], users_by_email: Mapping[str, User], organization: Organization
+    commits: Iterable[Commit],
+    users_by_email: Mapping[str, User],
+    organization: Organization,
 ) -> Iterable[Mapping[str, str | Iterable[tuple[Commit, User | None]]]]:
-    repos = {
-        r_id: {"name": r_name, "commits": []}
-        for r_id, r_name in Repository.objects.filter(
+    repositories_by_id = {
+        repository_id: {"name": repository_name, "commits": []}
+        for repository_id, repository_name in Repository.objects.filter(
             organization_id=organization.id,
             id__in={c.repository_id for c in commits},
         ).values_list("id", "name")
     }
+    # These commits are in order so they should end up in the list of commits still in order.
     for commit in commits:
+        # Get the user object if it exists
         user_option = users_by_email.get(commit.author.email) if commit.author_id else None
-        repos[commit.repository_id]["commits"].append((commit, user_option))
+        repositories_by_id[commit.repository_id]["commits"].append((commit, user_option))
 
-    return list(repos.values())
-
-
-def get_commits_for_release(release: Release) -> set[Commit]:
-    return {
-        rc.commit
-        for rc in ReleaseCommit.objects.filter(release=release).select_related(
-            "commit", "commit__author"
-        )
-    }
+    return list(repositories_by_id.values())
 
 
 def get_environment_for_deploy(deploy: Deploy | None) -> str:

tests/sentry/mail/activity/test_release.py

@@ -41,10 +41,11 @@ class ReleaseTestCase(ActivityTestCase):
 
         repository = Repository.objects.create(organization_id=self.org.id, name=self.project.name)
 
+        # The commits are intentionally out of order to test commit `order`.
+        self.commit4 = self.another_commit(3, "e", self.user5, repository, user5_alt_email)
         self.commit1 = self.another_commit(0, "a", self.user1, repository)
         self.commit2 = self.another_commit(1, "b", self.user2, repository)
         self.commit3 = self.another_commit(2, "c", self.user4, repository)
-        self.commit4 = self.another_commit(3, "e", self.user5, repository, user5_alt_email)
 
         NotificationSetting.objects.update_settings(
             ExternalProviders.EMAIL,
@@ -99,10 +100,10 @@ class ReleaseTestCase(ActivityTestCase):
         context = email.get_context()
         assert context["environment"] == "production"
         assert context["repos"][0]["commits"] == [
-            (self.commit1, self.user1),
-            (self.commit2, self.user2),
-            (self.commit3, self.user4),
             (self.commit4, self.user5),
+            (self.commit3, self.user4),
+            (self.commit2, self.user2),
+            (self.commit1, self.user1),
         ]
 
         user_context = email.get_recipient_context(self.user1, {})