Browse Source

fix(active-release): fix release notification to render release link and commit data (#36242)

* fix last_release actually being passed downstream. update email template to use release_commits

* fix conditional assignment

* mypy explicit typing

* query for ReleaseCommit instead of using util method.

* fix circular import
Gilbert Szeto 2 years ago
parent
commit
4608e37d01

+ 68 - 6
src/sentry/notifications/notifications/rules.py

@@ -1,12 +1,12 @@
 from __future__ import annotations
 
 import logging
-from typing import Any, Iterable, Mapping, MutableMapping, Optional
+from typing import Any, Iterable, Mapping, MutableMapping, Optional, Sequence, TypedDict
 
 import pytz
 
 from sentry.db.models import Model
-from sentry.models import Release, Team, User, UserOption
+from sentry.models import Release, ReleaseCommit, Team, User, UserOption
 from sentry.notifications.notifications.base import ProjectNotification
 from sentry.notifications.types import ActionTargetType, NotificationSettingTypes
 from sentry.notifications.utils import (
@@ -20,8 +20,10 @@ from sentry.notifications.utils import (
 )
 from sentry.notifications.utils.participants import get_send_to
 from sentry.plugins.base.structs import Notification
+from sentry.rules.conditions.active_release import ActiveReleaseEventCondition
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import metrics
+from sentry.utils.http import absolute_uri
 
 logger = logging.getLogger(__name__)
 
@@ -159,6 +161,12 @@ class AlertRuleNotification(ProjectNotification):
         }
 
 
+class CommitData(TypedDict):
+    author: User
+    subject: str
+    key: str
+
+
 class ActiveReleaseAlertNotification(AlertRuleNotification):
     message_builder = "ActiveReleaseIssueNotificationMessageBuilder"
     metrics_key = "release_issue_alert"
@@ -173,7 +181,11 @@ class ActiveReleaseAlertNotification(AlertRuleNotification):
         last_release: Optional[Release] = None,
     ) -> None:
         super().__init__(notification, target_type, target_identifier)
-        self.last_release = last_release
+        self.last_release = (
+            last_release
+            if last_release
+            else ActiveReleaseEventCondition.latest_release(notification.event)
+        )
 
     def get_notification_title(self, context: Mapping[str, Any] | None = None) -> str:
         from sentry.integrations.slack.message_builder.issues import build_rule_url
@@ -190,6 +202,56 @@ class ActiveReleaseAlertNotification(AlertRuleNotification):
         return title_str
 
     def get_context(self) -> MutableMapping[str, Any]:
-        ctx = super().get_context()
-        ctx["last_release"] = self.last_release
-        return ctx
+        environment = self.event.get_tag("environment")
+        enhanced_privacy = self.organization.flags.enhanced_privacy
+        rule_details = get_rules(self.rules, self.organization, self.project)
+        context = {
+            "project_label": self.project.get_full_name(),
+            "group": self.group,
+            "event": self.event,
+            "link": get_group_settings_link(self.group, environment, rule_details),
+            "rules": rule_details,
+            "has_integrations": has_integrations(self.organization, self.project),
+            "enhanced_privacy": enhanced_privacy,
+            "last_release": self.last_release,
+            "last_release_link": self.release_url(self.last_release),
+            "commits": self.get_release_commits(self.last_release),
+            "environment": environment,
+            "slack_link": get_integration_link(self.organization, "slack"),
+            "has_alert_integration": has_alert_integration(self.project),
+        }
+
+        # if the organization has enabled enhanced privacy controls we don't send
+        # data which may show PII or source code
+        if not enhanced_privacy:
+            context.update({"tags": self.event.tags, "interfaces": get_interface_list(self.event)})
+
+        return context
+
+    def get_release_commits(self, release: Release) -> Sequence[CommitData]:
+        if not release:
+            return []
+
+        release_commits = (
+            ReleaseCommit.objects.filter(release_id=release.id)
+            .select_related("commit", "commit__author")
+            .order_by("-order")
+        )
+
+        return [
+            {
+                "author": rc.commit.author,
+                "subject": rc.commit.message.split("\n", 1)[0]
+                if rc.commit.message
+                else "no subject",
+                "key": rc.commit.key,
+            }
+            for rc in release_commits
+        ]
+
+    def release_url(self, release: Release) -> str:
+        return str(
+            absolute_uri(
+                f"/organizations/{release.organization.slug}/releases/{release.version}/?project={release.project_id}"
+            )
+        )

+ 19 - 14
src/sentry/rules/conditions/active_release.py

@@ -14,7 +14,24 @@ class ActiveReleaseEventCondition(EventCondition):
     id = "sentry.rules.conditions.active_release.ActiveReleaseEventCondition"
     label = "A new issue is created within an active release (1 hour of deployment)"
 
-    def is_in_active_release(self, event: Event) -> bool:
+    def passes(self, event: Event, state: EventState) -> bool:
+        if self.rule and self.rule.environment_id is None:
+            return (state.is_new or state.is_regression) and self.is_in_active_release(event)
+        else:
+            return (
+                state.is_new_group_environment or state.is_regression
+            ) and self.is_in_active_release(event)
+
+    @staticmethod
+    def latest_release(event: Event) -> Optional[Release]:
+        return Release.objects.filter(
+            organization_id=event.project.organization_id,
+            projects__id=event.project_id,
+            version=event.release,
+        ).first()
+
+    @staticmethod
+    def is_in_active_release(event: Event) -> bool:
         if not event.group:
             return False
 
@@ -24,11 +41,7 @@ class ActiveReleaseEventCondition(EventCondition):
         if not event.group or not event.project:
             return False
 
-        event_release = Release.objects.filter(
-            organization_id=event.project.organization_id,
-            projects__id=event.project_id,
-            version=event.release,
-        ).first()
+        event_release = ActiveReleaseEventCondition.latest_release(event)
 
         if not event_release:
             return False
@@ -70,11 +83,3 @@ class ActiveReleaseEventCondition(EventCondition):
             return bool(now_minus_1_hour <= deploy_time <= now)
 
         return False
-
-    def passes(self, event: Event, state: EventState) -> bool:
-        if self.rule and self.rule.environment_id is None:
-            return (state.is_new or state.is_regression) and self.is_in_active_release(event)
-        else:
-            return (
-                state.is_new_group_environment or state.is_regression
-            ) and self.is_in_active_release(event)

+ 1 - 1
src/sentry/templates/sentry/emails/release_alert.html

@@ -74,7 +74,7 @@
             <tr>
               <td style="padding:0;width:32px;">{% email_avatar commit.author.name commit.author.email 32 %}</td>
               <td>
-                <h5 class="truncate">You made a commit in release {{ release.version }}</h5>
+                <h5 class="truncate">{{commit.author.name}} made a commit in release <a href="{{ last_release_link }}">{{ last_release.version }}</a>.</h5>
                 <div><small>{{ commit.subject }}</small></div>
               </td>
             </tr>

+ 17 - 33
src/sentry/web/frontend/debug/mail.py

@@ -386,47 +386,31 @@ def release_alert(request):
             "group": group,
             "event": event,
             "timezone": pytz.timezone("Europe/Vienna"),
-            # http://testserver/organizations/example/issues/<issue-id>/?referrer=alert_email
-            #       &alert_type=email&alert_timestamp=<ts>&alert_rule_id=1
             "link": get_group_settings_link(group, None, get_rules([rule], org, project), 1337),
             "interfaces": interfaces,
             "tags": event.tags,
             "project": project,
-            "release": {
-                "version": "13.9.1",
+            "last_release": {
+                "version": "13.9.2",
             },
+            "last_release_link": f"http://testserver/organizations/{org.slug}/releases/13.9.2/?project={project.id}",
             "environment": "production",
             "commits": [
                 {
-                    "repository": {
-                        "status": "active",
-                        "name": "Example Repo",
-                        "url": "https://github.com/example/example",
-                        "dateCreated": "2018-02-28T23:39:22.402Z",
-                        "provider": {"id": "github", "name": "GitHub"},
-                        "id": "1",
-                    },
-                    "score": 2,
-                    "subject": "feat: Do something to raven/base.py",
-                    "message": "feat: Do something to raven/base.py\naptent vivamus vehicula tempus volutpat hac tortor",
-                    "id": "1b17483ffc4a10609e7921ee21a8567bfe0ed006",
-                    "shortId": "1b17483",
-                    "author": {
-                        "username": "dcramer@gmail.com",
-                        "isManaged": False,
-                        "lastActive": "2018-03-01T18:25:28.149Z",
-                        "id": "1",
-                        "isActive": True,
-                        "has2fa": False,
-                        "name": "dcramer@gmail.com",
-                        "avatarUrl": "https://secure.gravatar.com/avatar/51567a4f786cd8a2c41c513b592de9f9?s=32&d=mm",
-                        "dateJoined": "2018-02-27T22:04:32.847Z",
-                        "emails": [{"is_verified": False, "id": "1", "email": "dcramer@gmail.com"}],
-                        "avatar": {"avatarUuid": None, "avatarType": "letter_avatar"},
-                        "lastLogin": "2018-02-27T22:04:32.847Z",
-                        "email": "dcramer@gmail.com",
-                    },
-                }
+                    "subject": "fix bug really",
+                    "author": {"name": "committer1", "email": "test@example.com"},
+                    "key": "sha8910",
+                },
+                {
+                    "subject": "fix bug",
+                    "author": {"name": "committer2", "email": "test2@example.com"},
+                    "key": "sha567",
+                },
+                {
+                    "subject": "first commit",
+                    "author": {"name": "committer1", "email": "test@example.com"},
+                    "key": "sha1234",
+                },
             ],
         },
     ).render(request)

+ 40 - 1
tests/sentry/mail/test_adapter.py

@@ -1,5 +1,5 @@
 from collections import Counter
-from datetime import datetime
+from datetime import datetime, timedelta
 from typing import Mapping, Sequence
 from unittest import mock
 
@@ -17,6 +17,7 @@ from sentry.event_manager import EventManager, get_event_type
 from sentry.mail import build_subject_prefix, mail_adapter
 from sentry.models import (
     Activity,
+    Deploy,
     GroupRelease,
     Integration,
     NotificationSetting,
@@ -26,6 +27,7 @@ from sentry.models import (
     Project,
     ProjectOption,
     ProjectOwnership,
+    Release,
     Repository,
     Rule,
     User,
@@ -71,6 +73,43 @@ class BaseMailAdapterTest(TestCase):
         assert sorted(email.to[0] for email in mail.outbox) == sorted(emails_sent_to)
 
 
+class MailAdapterActiveReleaseTest(BaseMailAdapterTest):
+    @mock.patch("sentry.notifications.utils.participants.get_release_committers")
+    def test_simple(self, mock_get_release_committers):
+        mock_get_release_committers.return_value = [self.create_user(email="test@example.com")]
+        event = self.store_event(
+            data={"message": "Hello world", "level": "error"}, project_id=self.project.id
+        )
+        newRelease = Release.objects.create(
+            organization_id=self.organization.id,
+            project_id=self.project.id,
+            version="2",
+            date_added=timezone.now() - timedelta(days=1),
+            date_released=None,
+        )
+        Deploy.objects.create(
+            organization_id=self.organization.id,
+            environment_id=self.environment.id,
+            name="test_release_deployed",
+            notified=True,
+            release_id=newRelease.id,
+            date_started=timezone.now() - timedelta(minutes=37),
+            date_finished=timezone.now() - timedelta(minutes=20),
+        )
+        newRelease.add_project(self.project)
+
+        event.data["tags"] = (("sentry:release", newRelease.version),)
+
+        mail.outbox = []
+        with self.options({"system.url-prefix": "http://example.com"}), self.tasks():
+            self.adapter.notify(Notification(event=event), ActionTargetType.RELEASE_MEMBERS, None)
+
+        assert len(mail.outbox) == 1
+        to_committer = mail.outbox[0]
+
+        assert to_committer
+
+
 class MailAdapterGetSendableUsersTest(BaseMailAdapterTest):
     def test_get_sendable_user_objects(self):
         user = self.create_user(email="foo@example.com", is_active=True)