Merge branch '322638-jira-issue-detail-with-comments' into 'master'

Jira Issue Detail page: Add comments

See merge request gitlab-org/gitlab!55221

ee/app/serializers/integrations/jira/issue_detail_entity.rb

@@ -15,6 +15,16 @@ module Integrations
       expose :due_date do |jira_issue|
         jira_issue.duedate&.to_datetime&.utc
       end
+
+      expose :comments do |jira_issue|
+        jira_issue.renderedFields['comment']['comments'].map do |comment|
+          jira_user(comment['author']).merge(
+            note: Banzai::Pipeline::JiraGfmPipeline.call(comment['body'], project: project)[:output].to_html,
+            created_at: comment['created'].to_datetime.utc,
+            updated_at: comment['updated'].to_datetime.utc
+          )
+        end
+      end
     end
   end
 end

ee/app/serializers/integrations/jira/issue_entity.rb

@@ -41,13 +41,13 @@ module Integrations
       end
 
       expose :author do |jira_issue|
-        jira_user(jira_issue, :reporter)
+        jira_user(jira_issue.fields['reporter'])
       end
 
       expose :assignees do |jira_issue|
-        if jira_issue.assignee.present?
+        if jira_issue.fields['assignee']
           [
-            jira_user(jira_issue, :assignee)
+            jira_user(jira_issue.fields['assignee'])
           ]
         else
           []
@@ -61,8 +61,6 @@ module Integrations
       expose :gitlab_web_url do |jira_issue|
         if ::Feature.enabled?(:jira_issues_show_integration, project, default_enabled: :yaml)
           project_integrations_jira_issue_path(project, jira_issue.key)
-        else
-          nil
         end
       end
 
@@ -78,26 +76,24 @@ module Integrations
 
       private
 
-      # rubocop:disable GitlabSecurity/PublicSend
-      def jira_user(jira_issue, user_type)
+      def jira_user(user)
         {
-          name: jira_issue.public_send(user_type).displayName,
-          web_url: jira_web_url(jira_issue, user_type),
-          avatar_url: jira_issue.public_send(user_type).avatarUrls['48x48']
+          name: user['displayName'],
+          web_url: jira_web_url(user),
+          avatar_url: user['avatarUrls']['48x48']
         }
       end
 
-      def jira_web_url(jira_issue, user_type)
+      def jira_web_url(user)
         # There are differences between Jira Cloud and Jira Server URLs and responses.
         # accountId is only available on Jira Cloud.
         # https://community.atlassian.com/t5/Jira-Questions/How-to-find-account-id-on-jira-on-premise/qaq-p/1168652
-        if jira_issue.public_send(user_type).try(:accountId)
-          "#{base_web_url}/people/#{jira_issue.public_send(user_type).accountId}"
+        if user['accountId'].present?
+          "#{base_web_url}/people/#{user['accountId']}"
         else
-          "#{base_web_url}/secure/ViewProfile.jspa?name=#{jira_issue.public_send(user_type).name}"
+          "#{base_web_url}/secure/ViewProfile.jspa?name=#{user['name']}"
         end
       end
-      # rubocop:enable GitlabSecurity/PublicSend
 
       def base_web_url
         @base_web_url ||= project.jira_service.url

ee/spec/serializers/integrations/jira/issue_detail_entity_spec.rb

@@ -9,32 +9,54 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
   let_it_be(:jira_service) { create(:jira_service, project: project, url: 'http://jira.com', api_url: 'http://api.jira.com') }
 
   let(:reporter) do
-    double(
+    {
       'displayName' => 'reporter',
       'avatarUrls' => { '48x48' => 'http://reporter.avatar' },
       'name' => double
-    )
+    }
   end
 
   let(:assignee) do
-    double(
+    {
       'displayName' => 'assignee',
       'avatarUrls' => { '48x48' => 'http://assignee.avatar' },
       'name' => double
-    )
+    }
+  end
+
+  let(:comment_author) do
+    {
+      'displayName' => 'comment_author',
+      'avatarUrls' => { '48x48' => 'http://comment_author.avatar' },
+      'name' => double
+    }
   end
 
   let(:jira_issue) do
     double(
       summary: 'Title',
-      renderedFields: { 'description' => '<p>Description</p>' },
+      renderedFields: {
+        'description' => '<p>Description</p>',
+        'comment' => {
+          'comments' => [
+            {
+              'author' => comment_author,
+              'body' => '<p>Comment</p>',
+              'created' => '2020-06-25T15:50:00.000+0000',
+              'updated' => '2020-06-25T15:51:00.000+0000'
+            }
+          ]
+        }
+      },
       created: '2020-06-25T15:39:30.000+0000',
       updated: '2020-06-26T15:38:32.000+0000',
       duedate: '2020-06-27T15:40:30.000+0000',
       resolutiondate: '2020-06-27T13:23:51.000+0000',
       labels: ['backend'],
-      reporter: reporter,
-      assignee: assignee,
+      fields: {
+        'reporter' => reporter,
+        'assignee' => assignee
+      },
       project: double(key: 'GL'),
       key: 'GL-5',
       status: double(name: 'To Do')
@@ -74,19 +96,30 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
       ],
       web_url: 'http://jira.com/browse/GL-5',
       references: { relative: 'GL-5' },
-      external_tracker: 'jira'
+      external_tracker: 'jira',
+      comments: [
+        hash_including(
+          name: 'comment_author',
+          avatar_url: 'http://comment_author.avatar',
+          note: "<p dir=\"auto\">Comment</p>",
+          created_at: '2020-06-25T15:50:00.000+0000'.to_datetime.utc,
+          updated_at: '2020-06-25T15:51:00.000+0000'.to_datetime.utc
+        )
+      ]
     )
   end
 
   context 'with Jira Server configuration' do
     before do
-      allow(reporter).to receive(:name).and_return('reporter@reporter.com')
-      allow(assignee).to receive(:name).and_return('assignee@assignee.com')
+      reporter['name'] = 'reporter@reporter.com'
+      assignee['name'] = 'assignee@assignee.com'
+      comment_author['name'] = 'comment@author.com'
     end
 
     it 'returns the Jira Server profile URL' do
       expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
       expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com')
+      expect(subject[:comments].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=comment@author.com')
     end
 
     context 'with only url' do
@@ -104,20 +137,20 @@ RSpec.describe Integrations::Jira::IssueDetailEntity do
 
   context 'with Jira Cloud configuration' do
     before do
-      allow(reporter).to receive(:accountId).and_return('12345')
-      allow(assignee).to receive(:accountId).and_return('67890')
+      reporter['accountId'] = '12345'
+      assignee['accountId'] = '67890'
+      comment_author['accountId'] = '54321'
     end
 
     it 'returns the Jira Cloud profile URL' do
       expect(subject[:author]).to include(web_url: 'http://jira.com/people/12345')
       expect(subject[:assignees].first).to include(web_url: 'http://jira.com/people/67890')
+      expect(subject[:comments].first).to include(web_url: 'http://jira.com/people/54321')
     end
   end
 
   context 'without assignee' do
-    before do
-      allow(jira_issue).to receive(:assignee).and_return(nil)
-    end
+    let(:assignee) { nil }
 
     it 'returns an empty array' do
       expect(subject).to include(assignees: [])

ee/spec/serializers/integrations/jira/issue_entity_spec.rb

@@ -9,19 +9,19 @@ RSpec.describe Integrations::Jira::IssueEntity do
   let_it_be(:jira_service) { create(:jira_service, project: project, url: 'http://jira.com', api_url: 'http://api.jira.com') }
 
   let(:reporter) do
-    double(
+    {
       'displayName' => 'reporter',
       'avatarUrls' => { '48x48' => 'http://reporter.avatar' },
       'name' => double
-    )
+    }
   end
 
   let(:assignee) do
-    double(
+    {
       'displayName' => 'assignee',
       'avatarUrls' => { '48x48' => 'http://assignee.avatar' },
       'name' => double
-    )
+    }
   end
 
   let(:jira_issue) do
@@ -31,8 +31,10 @@ RSpec.describe Integrations::Jira::IssueEntity do
       updated: '2020-06-26T15:38:32.000+0000',
       resolutiondate: '2020-06-27T13:23:51.000+0000',
       labels: ['backend'],
-      reporter: reporter,
-      assignee: assignee,
+      fields: {
+        'reporter' => reporter,
+        'assignee' => assignee
+      },
       project: double(key: 'GL'),
       key: 'GL-5',
       status: double(name: 'To Do')
@@ -76,8 +78,8 @@ RSpec.describe Integrations::Jira::IssueEntity do
 
   context 'with Jira Server configuration' do
     before do
-      allow(reporter).to receive(:name).and_return('reporter@reporter.com')
-      allow(assignee).to receive(:name).and_return('assignee@assignee.com')
+      reporter['name'] = 'reporter@reporter.com'
+      assignee['name'] = 'assignee@assignee.com'
     end
 
     it 'returns the Jira Server profile URL' do
@@ -100,8 +102,8 @@ RSpec.describe Integrations::Jira::IssueEntity do
 
   context 'with Jira Cloud configuration' do
     before do
-      allow(reporter).to receive(:accountId).and_return('12345')
-      allow(assignee).to receive(:accountId).and_return('67890')
+      reporter['accountId'] = '12345'
+      assignee['accountId'] = '67890'
     end
 
     it 'returns the Jira Cloud profile URL' do
@@ -111,9 +113,7 @@ RSpec.describe Integrations::Jira::IssueEntity do
   end
 
   context 'without assignee' do
-    before do
-      allow(jira_issue).to receive(:assignee).and_return(nil)
-    end
+    let(:assignee) { nil }
 
     it 'returns an empty array' do
       expect(subject).to include(assignees: [])