Browse Source

feat(escalating): Update alert design for escalating issues (#48762)

Scott Cooper 1 year ago
parent
commit
848d6169b3

+ 12 - 0
src/sentry/models/groupinbox.py

@@ -1,5 +1,6 @@
 import logging
 from enum import Enum
+from typing import Optional
 
 import jsonschema
 from django.db import models
@@ -153,3 +154,14 @@ def get_inbox_details(group_list):
     }
 
     return inbox_stats
+
+
+def get_inbox_reason_text(group_inbox: Optional[GroupInbox]):
+    reason = GroupInboxReason(group_inbox.reason) if group_inbox else None
+    if reason == GroupInboxReason.NEW:
+        return "New issue"
+    elif reason == GroupInboxReason.REGRESSION:
+        return "Regressed issue"
+    elif reason == GroupInboxReason.ONGOING:
+        return "Ongoing issue"
+    return "New Alert"

+ 7 - 0
src/sentry/notifications/notifications/rules.py

@@ -10,6 +10,7 @@ from sentry import features
 from sentry.db.models import Model
 from sentry.issues.grouptype import GROUP_CATEGORIES_CUSTOM_EMAIL, GroupCategory
 from sentry.models import UserOption
+from sentry.models.groupinbox import GroupInbox, get_inbox_reason_text
 from sentry.notifications.notifications.base import ProjectNotification
 from sentry.notifications.types import (
     ActionTargetType,
@@ -117,10 +118,15 @@ class AlertRuleNotification(ProjectNotification):
             fallthrough_choice=self.fallthrough_choice,
         )
         fallback_params: MutableMapping[str, str] = {}
+        group_inbox_reason = GroupInbox.objects.filter(
+            group=self.group, project=self.project
+        ).first()
+        group_header = get_inbox_reason_text(group_inbox_reason)
 
         context = {
             "project_label": self.project.get_full_name(),
             "group": self.group,
+            "group_header": group_header,
             "event": self.event,
             "link": get_group_settings_link(
                 self.group, environment, rule_details, None, **fallback_params
@@ -138,6 +144,7 @@ class AlertRuleNotification(ProjectNotification):
             "has_alert_integration": has_alert_integration(self.project),
             "issue_type": self.group.issue_type.description,
             "subtitle": self.event.title,
+            "has_issue_states": features.has("organizations:issue-states", self.organization),
         }
 
         # if the organization has enabled enhanced privacy controls we don't send

+ 1 - 1
src/sentry/templates/sentry/emails/email-styles.html

@@ -297,7 +297,7 @@
   .inner table th {
     min-width: 60px;
     color: #968ba0;
-    padding: 10px 0 0 0;
+    padding: 2px 0 0 0;
   }
   .interface {
     margin-bottom: 30px;

+ 34 - 1
src/sentry/templates/sentry/emails/issue_alert_base.html

@@ -28,7 +28,7 @@
 {% endblock %}
 
 {% block preheader %}
-  New alert from {{ project_label }}.
+  {{ group_header }} from {{ project_label }}.
 {% endblock %}
 
 {% block header %}
@@ -64,8 +64,12 @@
 <div class="container">
   <div class="inner">
     <h2 {% if notification_reason %}style="margin-bottom: 4px"{% else %}style="margin-bottom: 15px"{% endif %}>
+        {% if has_issue_states and group_header %}
+        {{ group_header }}
+        {% else %}
         New alert from <a href="{{group.project.get_absolute_url}}">{{ project_label }}</a>
         {% if environment %} in {{ environment }}{% endif %}
+        {% endif %}
     </h2>
     {% if notification_reason %}
       <div class="event-notification-reason">
@@ -91,9 +95,11 @@
             <th colspan="2">Issue</th>
         </tr>
         <tr>
+          {% if not has_issue_states %}
           <td class="error-level">
               <span class="level level-{{ group.get_level_display }}">{{ group.get_level_display }}</span>
           </td>
+          {% endif %}
           <td class="event-detail">
             <div class="issue">
               {% with type=event.get_event_type metadata=event.get_event_metadata transaction=event.transaction %}
@@ -168,6 +174,33 @@
         {% endif %}
       </div>
 
+      {% if has_issue_states %}
+      <div class="interface">
+        <table>
+          <colgroup>
+            <col style="width:130px;">
+          </colgroup>
+          <tbody>
+            <tr>
+              <th>project</th>
+              <td><a href="{{group.project.get_absolute_url}}">{{ project_label }}</a></td>
+            </tr>
+            {% if environment %}
+              <tr>
+                <th>environment</th>
+                <td>{{ environment }}</td>
+              </tr>
+            {% endif %}
+            <tr>
+              <th>level</th>
+              <td>{{ group.get_level_display }}</td>
+            </tr>
+          </tbody>
+        </table>
+      </div>
+      {% endif %}
+
+
       {% if commits %}
       <div class="committers">
         <h3 class="title" style="margin-bottom: 10px">Suspect Commits</h3>

+ 14 - 2
src/sentry/web/frontend/debug/mail.py

@@ -41,6 +41,7 @@ from sentry.models import (
     Team,
     User,
 )
+from sentry.models.groupinbox import GroupInbox, GroupInboxReason, get_inbox_reason_text
 from sentry.notifications.notifications.activity import EMAIL_CLASSES_BY_TYPE
 from sentry.notifications.notifications.base import BaseNotification
 from sentry.notifications.notifications.digest import DigestNotification
@@ -261,7 +262,7 @@ def make_generic_event(project):
     return generic_group.get_latest_event()
 
 
-def get_shared_context(rule, org, project, group, event):
+def get_shared_context(rule, org, project, group, event, group_inbox: GroupInbox | None = None):
     rules = get_rules([rule], org, project)
     snooze_alert = len(rules) > 0
     snooze_alert_url = rules[0].status_url + urlencode({"mute": "1"}) if snooze_alert else ""
@@ -269,6 +270,7 @@ def get_shared_context(rule, org, project, group, event):
         "rule": rule,
         "rules": rules,
         "group": group,
+        "group_header": get_inbox_reason_text(group_inbox),
         "event": event,
         "timezone": pytz.timezone("Europe/Vienna"),
         # http://testserver/organizations/example/issues/<issue-id>/?referrer=alert_email
@@ -433,6 +435,9 @@ class ActivityMailDebugView(View):
         )
 
 
+has_issue_states = True
+
+
 @login_required
 def alert(request):
     random = get_random(request)
@@ -449,12 +454,16 @@ def alert(request):
         and f"We notified all members in the {project.get_full_name()} project of this issue"
         or None
     )
+    inbox_reason = random.choice(
+        [GroupInboxReason.NEW, GroupInboxReason.REGRESSION, GroupInboxReason.ONGOING]
+    )
+    group_inbox = GroupInbox(reason=inbox_reason.value, group=group, project=project)
 
     return MailPreview(
         html_template="sentry/emails/error.html",
         text_template="sentry/emails/error.txt",
         context={
-            **get_shared_context(rule, org, project, group, event),
+            **get_shared_context(rule, org, project, group, event, group_inbox),
             "interfaces": get_interface_list(event),
             "project_label": project.slug,
             "commits": json.loads(COMMIT_EXAMPLE),
@@ -463,7 +472,10 @@ def alert(request):
             "notification_settings_link": absolute_uri(
                 "/settings/account/notifications/alerts/?referrer=alert_email"
             ),
+            "culprit": random.choice(["sentry.tasks.culprit.culprit", None]),
+            "subtitle": random.choice(["subtitles are cool", None]),
             "issue_type": group.issue_type.description,
+            "has_issue_states": has_issue_states,
         },
     ).render(request)
 

+ 20 - 0
tests/sentry/mail/test_adapter.py

@@ -43,6 +43,7 @@ from sentry.models import (
     UserOption,
     UserReport,
 )
+from sentry.models.groupinbox import GroupInboxReason, add_group_to_inbox
 from sentry.notifications.notifications.rules import AlertRuleNotification
 from sentry.notifications.types import (
     ActionTargetType,
@@ -1208,6 +1209,25 @@ class MailAdapterNotifyIssueOwnersTest(BaseMailAdapterTest):
                 [],
             )
 
+    @with_feature("organizations:issue-states")
+    def test_has_inbox_reason(self):
+        event = self.store_event(
+            data={"message": "Hello world", "level": "error"}, project_id=self.project.id
+        )
+        add_group_to_inbox(event.group, GroupInboxReason.REGRESSION)
+
+        rule = Rule.objects.create(project=self.project, label="my rule")
+        ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True)
+
+        notification = Notification(event=event, rule=rule)
+
+        with self.options({"system.url-prefix": "http://example.com"}), self.tasks():
+            self.adapter.notify(notification, ActionTargetType.ISSUE_OWNERS)
+
+        msg = mail.outbox[0]
+        assert msg.subject == "[Sentry] BAR-1 - Hello world"
+        assert "Regressed issue" in msg.alternatives[0][0]
+
 
 class MailAdapterGetDigestSubjectTest(BaseMailAdapterTest):
     def test_get_digest_subject(self):