Просмотр исходного кода

feat(jira): Allow multiple Sentry Issues to be rendered on Jira Issue View (#33842)

Fixes SENTRY-TC6

Also addresses a TODO in code and allows for multiple sentry issues to be rendered with details in jira if they are all linked with one jira ticket.
Leander Rodrigues 2 лет назад
Родитель
Сommit
aa38dcdf5c

+ 31 - 23
src/sentry/integrations/jira/views/issue_hook.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 
 import logging
 import logging
 from functools import reduce
 from functools import reduce
-from typing import Any, Mapping
+from typing import Any, Mapping, Sequence
 from urllib.parse import quote
 from urllib.parse import quote
 
 
 from jwt import ExpiredSignatureError
 from jwt import ExpiredSignatureError
@@ -12,7 +12,7 @@ from rest_framework.response import Response
 from sentry.api.serializers import serialize
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.group import StreamGroupSerializer
 from sentry.api.serializers.models.group import StreamGroupSerializer
 from sentry.integrations.utils import AtlassianConnectValidationError, get_integration_from_request
 from sentry.integrations.utils import AtlassianConnectValidationError, get_integration_from_request
-from sentry.models import ExternalIssue, Group, GroupLink
+from sentry.models import ExternalIssue, Group
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.shared_integrations.exceptions import ApiError
 from sentry.utils.http import absolute_uri
 from sentry.utils.http import absolute_uri
 from sentry.utils.sdk import configure_scope
 from sentry.utils.sdk import configure_scope
@@ -86,13 +86,19 @@ def build_context(group: Group) -> Mapping[str, Any]:
 class JiraIssueHookView(JiraBaseHook):
 class JiraIssueHookView(JiraBaseHook):
     html_file = "sentry/integrations/jira-issue.html"
     html_file = "sentry/integrations/jira-issue.html"
 
 
-    def handle_group(self, group: Group) -> Response:
-        context = build_context(group)
+    def handle_groups(self, groups: Sequence[Group]) -> Response:
+        response_context = {"groups": []}
+        for group in groups:
+            context = build_context(group)
+            response_context["groups"].append(context)
+
         logger.info(
         logger.info(
             "issue_hook.response",
             "issue_hook.response",
-            extra={"type": context["type"], "title_url": context["title_url"]},
+            # TODO(PR Review): Backwards compatability with logs?
+            # extra={"type": context["type"], "title_url": context["title_url"]},
         )
         )
-        return self.get_response(context)
+
+        return self.get_response(response_context)
 
 
     def dispatch(self, request: Request, *args, **kwargs) -> Response:
     def dispatch(self, request: Request, *args, **kwargs) -> Response:
         try:
         try:
@@ -119,26 +125,28 @@ class JiraIssueHookView(JiraBaseHook):
                 external_issue = ExternalIssue.objects.get(
                 external_issue = ExternalIssue.objects.get(
                     integration_id=integration.id, key=issue_key
                     integration_id=integration.id, key=issue_key
                 )
                 )
-                # TODO: handle multiple
-                group_link = GroupLink.objects.filter(
-                    linked_type=GroupLink.LinkedType.issue,
-                    linked_id=external_issue.id,
-                    relationship=GroupLink.Relationship.references,
+                organization = integration.organizations.filter(
+                    id=external_issue.organization_id
                 ).first()
                 ).first()
-                if not group_link:
-                    raise GroupLink.DoesNotExist()
-                group = Group.objects.get(id=group_link.group_id)
-            except (ExternalIssue.DoesNotExist, GroupLink.DoesNotExist, Group.DoesNotExist) as e:
+                groups = Group.objects.get_groups_by_external_issue(
+                    integration=integration,
+                    organizations=[organization],
+                    external_issue_key=issue_key,
+                )
+            except (
+                ExternalIssue.DoesNotExist,
+                # Multiple ExternalIssues are returned if organizations share one integration.
+                # Since we cannot identify the organization from the request alone, for now, we just
+                # avoid crashing on the MultipleObjectsReturned error.
+                ExternalIssue.MultipleObjectsReturned,
+            ) as e:
                 scope.set_tag("failure", e.__class__.__name__)
                 scope.set_tag("failure", e.__class__.__name__)
                 set_badge(integration, issue_key, 0)
                 set_badge(integration, issue_key, 0)
                 return self.get_response({"issue_not_linked": True})
                 return self.get_response({"issue_not_linked": True})
 
 
-            scope.set_tag("organization.slug", group.organization.slug)
-            result = self.handle_group(group)
-            scope.set_tag("status_code", result.status_code)
+            scope.set_tag("organization.slug", organization.slug)
+            response = self.handle_groups(groups)
+            scope.set_tag("status_code", response.status_code)
 
 
-            # XXX(CEO): group_link_num is hardcoded as 1 now, but when we handle
-            #  displaying multiple linked issues this should be updated to the
-            #  actual count.
-            set_badge(integration, issue_key, 1)
-            return result
+            set_badge(integration, issue_key, len(groups))
+            return response

+ 38 - 21
src/sentry/templates/sentry/integrations/jira-issue.html

@@ -45,20 +45,29 @@
       </div>
       </div>
       {% else %}
       {% else %}
 
 
+      {% for group in groups %}
       <div>
       <div>
-        <h2>{{ type }}</h2>
-        <p><a href="{{ title_url }}" target="_blank">{{title}}</a></p>
+        <h2>{{ group.type }}</h2>
+        <p><a href="{{ group.title_url }}" target="_blank">{{ group.title }}</a></p>
         <br />
         <br />
+
+        <!-- EVENTS GROUP -->
         <div class="aui-group">
         <div class="aui-group">
           <div class="aui-item">
           <div class="aui-item">
-            <span>{% trans "Events last 24 hours:" %}</span>
-            <aui-badge>{{ stats_24hr }}</aui-badge>
+            <h4>{% trans "Events" %}</h4>
+          </div>
+        </div>
+
+        <div class="aui-group">
+          <div class="aui-item" >
+            <span>{% trans "Last 24 hours:" %}</span>
+            <aui-badge>{{ group.stats_24hr }}</aui-badge>
           </div>
           </div>
         </div>
         </div>
         <div class="aui-group">
         <div class="aui-group">
           <div class="aui-item">
           <div class="aui-item">
-            <span>{% trans "Events last 14 days:" %}</span>
-            <aui-badge>{{ stats_14d }}</aui-badge>
+            <span>{% trans "Last 14 days:" %}</span>
+            <aui-badge>{{ group.stats_14d }}</aui-badge>
           </div>
           </div>
         </div>
         </div>
       </div>
       </div>
@@ -71,23 +80,23 @@
       </div>
       </div>
 
 
       <div class="aui-group">
       <div class="aui-group">
-        <div class="aui-item">
+        <div class="aui-item" style="width:75px;">
           <span class="left-grid">{% trans "Date:" %}</span>
           <span class="left-grid">{% trans "Date:" %}</span>
         </div>
         </div>
         <div class="aui-item">
         <div class="aui-item">
-          <span class="right-grid">{{ first_seen }} UTC</span>
+          <span class="right-grid">{{ group.first_seen }} UTC</span>
         </div>
         </div>
       </div>
       </div>
 
 
-      {% if first_release %}
+      {% if group.first_release %}
       <div class="aui-group">
       <div class="aui-group">
-        <div class="aui-item">
+        <div class="aui-item" style="width:75px;">
           <span class="left-grid">{% trans "Release:" %}</span>
           <span class="left-grid">{% trans "Release:" %}</span>
         </div>
         </div>
         <div class="aui-item">
         <div class="aui-item">
-          <span class="right-grid"
-            ><a href="{{ first_release_url}}" target="_blank">{{first_release}}</a></span
-          >
+          <span class="right-grid">
+            <a href="{{ group.first_release_url }}" target="_blank">{{ group.first_release }}</a>
+          </span>
         </div>
         </div>
       </div>
       </div>
       {% endif %}
       {% endif %}
@@ -100,26 +109,34 @@
       </div>
       </div>
 
 
       <div class="aui-group">
       <div class="aui-group">
-        <div class="aui-item">
+        <div class="aui-item" style="width:75px;">
           <span class="left-grid">{% trans "Date:" %}</span>
           <span class="left-grid">{% trans "Date:" %}</span>
         </div>
         </div>
         <div class="aui-item">
         <div class="aui-item">
-          <span class="right-grid">{{ last_seen }} UTC</span>
+          <span class="right-grid">{{ group.last_seen }} UTC</span>
         </div>
         </div>
       </div>
       </div>
 
 
-      {% if last_release %}
+      {% if group.last_release %}
       <div class="aui-group">
       <div class="aui-group">
-        <div class="aui-item">
+        <div class="aui-item" style="width:75px;">
           <span class="left-grid">{% trans "Release:" %}</span>
           <span class="left-grid">{% trans "Release:" %}</span>
         </div>
         </div>
         <div class="aui-item">
         <div class="aui-item">
-          <span class="right-grid"
-            ><a href="{{ last_release_url}}" target="_blank">{{last_release}}</a></span
-          >
+          <span class="right-grid">
+            <a href="{{ group.last_release_url }}" target="_blank">{{ group.last_release }}</a>
+          </span>
         </div>
         </div>
       </div>
       </div>
-      {% endif %} {% endif %}
+      {% endif %}
+
+      <br/>
+      {% if not forloop.last %}
+      <hr style="opacity:0.5;" />
+      {% endif %}
+
+      {% endfor %}
+      {% endif %}
     </div>
     </div>
   </body>
   </body>
 </html>
 </html>

+ 34 - 2
tests/sentry/integrations/jira/test_issue_hook.py

@@ -49,7 +49,7 @@ class JiraIssueHookTest(APITestCase):
         )
         )
         self.integration.add_organization(self.organization, self.user)
         self.integration.add_organization(self.organization, self.user)
 
 
-        external_issue = ExternalIssue.objects.create(
+        self.external_issue = ExternalIssue.objects.create(
             organization_id=group.organization.id,
             organization_id=group.organization.id,
             integration_id=self.integration.id,
             integration_id=self.integration.id,
             key=self.issue_key,
             key=self.issue_key,
@@ -59,7 +59,7 @@ class JiraIssueHookTest(APITestCase):
             group_id=group.id,
             group_id=group.id,
             project_id=group.project_id,
             project_id=group.project_id,
             linked_type=GroupLink.LinkedType.issue,
             linked_type=GroupLink.LinkedType.issue,
-            linked_id=external_issue.id,
+            linked_id=self.external_issue.id,
             relationship=GroupLink.Relationship.references,
             relationship=GroupLink.Relationship.references,
         )
         )
 
 
@@ -100,11 +100,43 @@ class JiraIssueHookTest(APITestCase):
         assert response.status_code == 200
         assert response.status_code == 200
         resp_content = str(response.content)
         resp_content = str(response.content)
         assert self.group.title in resp_content
         assert self.group.title in resp_content
+        assert self.group.get_absolute_url() in resp_content
         assert self.first_seen.strftime("%b. %d, %Y") in resp_content
         assert self.first_seen.strftime("%b. %d, %Y") in resp_content
         assert self.last_seen.strftime("%b. %d, %Y") in resp_content
         assert self.last_seen.strftime("%b. %d, %Y") in resp_content
         assert self.first_release.version in resp_content
         assert self.first_release.version in resp_content
         assert self.last_release.version in resp_content
         assert self.last_release.version in resp_content
 
 
+    @patch.object(Group, "get_last_release")
+    @patch("sentry.integrations.jira.views.issue_hook.get_integration_from_request")
+    @responses.activate
+    def test_multiple_issues(self, mock_get_integration_from_request, mock_get_last_release):
+        responses.add(
+            responses.PUT, self.properties_url % (self.issue_key, self.properties_key), json={}
+        )
+        mock_get_integration_from_request.return_value = self.integration
+        mock_get_last_release.return_value = self.last_release.version
+
+        new_group = self.create_group()
+
+        GroupLink.objects.create(
+            group_id=new_group.id,
+            project_id=new_group.project_id,
+            linked_type=GroupLink.LinkedType.issue,
+            linked_id=self.external_issue.id,
+            relationship=GroupLink.Relationship.references,
+        )
+
+        response = self.client.get(self.path)
+
+        assert response.status_code == 200
+        resp_content = str(response.content)
+        group_url = self.group.get_absolute_url()
+        new_group_url = new_group.get_absolute_url()
+
+        assert group_url != new_group_url
+        assert group_url in resp_content
+        assert new_group_url in resp_content
+
     @patch("sentry.integrations.jira.views.issue_hook.get_integration_from_request")
     @patch("sentry.integrations.jira.views.issue_hook.get_integration_from_request")
     @responses.activate
     @responses.activate
     def test_simple_not_linked(self, mock_get_integration_from_request):
     def test_simple_not_linked(self, mock_get_integration_from_request):