Improve JIRA issue closing

CHANGELOG-EE

@@ -4,6 +4,7 @@ v 8.2.0
   - When someone as marked as a required approver for a merge request, an email should be sent
   - Allow configuring the Jira API path (Alex Lossent)
   - Fix "Rebase onto master"
+  - Ensure a comment is properly recorded in JIRA when a merge request is accepted
 
 v 8.1.0 (unreleased)
   - added an issues template (Hannes Rosenögger)

app/models/project_services/jira_service.rb

@@ -130,7 +130,7 @@ class JiraService < IssueTrackerService
 
     case result.code
     when 201, 200
-      Rails.logger.info("#{self.class.name} SUCCESS #{result.code}: Sucessfully connected to #{api_url}.")
+      Rails.logger.info("#{self.class.name} SUCCESS #{result.code}: Successfully connected to #{api_url}.")
       true
     else
       Rails.logger.info("#{self.class.name} ERROR #{result.code}: #{result.parsed_response}")
@@ -161,25 +161,36 @@ class JiraService < IssueTrackerService
     self.jira_issue_transition_id ||= "2"
   end
 
-  def close_issue(commit, issue)
-    url = close_issue_url(issue.iid)
-
-    commit_url = build_entity_url(:commit, commit.id)
+  def close_issue(entity, issue)
+    commit_id = if entity.is_a?(Commit)
+                  entity.id
+                elsif entity.is_a?(MergeRequest)
+                  entity.last_commit.id
+                end
+    commit_url = build_entity_url(:commit, commit_id)
+
+    # Depending on the JIRA project's workflow, a comment during transition
+    # may or may not be allowed. Split the operation in to two calls so the
+    # comment always works.
+    transition_issue(issue)
+    add_issue_solved_comment(issue, commit_id, commit_url)
+  end
 
+  def transition_issue(issue)
     message = {
-      update: {
-        comment: [{
-          add: {
-            body: "Issue solved with [#{commit.id}|#{commit_url}]."
-          }
-        }]
-      },
       transition: {
         id: jira_issue_transition_id
       }
-    }.to_json
+    }
+    send_message(close_issue_url(issue.iid), message.to_json)
+  end
+
+  def add_issue_solved_comment(issue, commit_id, commit_url)
+    comment = {
+      body: "Issue solved with [#{commit_id}|#{commit_url}]."
+    }
 
-    send_message(url, message)
+    send_message(comment_url(issue.iid), comment.to_json)
   end
 
   def add_comment(data, issue_name)
@@ -216,8 +227,8 @@ class JiraService < IssueTrackerService
     )
 
     message = case result.code
-              when 201, 200
-                "#{self.class.name} SUCCESS #{result.code}: Sucessfully posted to #{url}."
+              when 201, 200, 204
+                "#{self.class.name} SUCCESS #{result.code}: Successfully posted to #{url}."
               when 401
                 "#{self.class.name} ERROR 401: Unauthorized. Check the #{self.username} credentials and JIRA access permissions and try again."
               else

spec/models/project_services/jira_service_spec.rb

@@ -31,6 +31,7 @@ describe JiraService do
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
+    let(:merge_request) { create(:merge_request) }
 
     before do
       @jira_service = JiraService.new
@@ -46,20 +47,22 @@ describe JiraService do
       @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
       # https://github.com/bblimke/webmock#request-with-basic-authentication
       @api_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
+      @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
 
       WebMock.stub_request(:post, @api_url)
+      WebMock.stub_request(:post, @comment_url)
     end
 
     it "should call JIRA API" do
-      @jira_service.execute(sample_commit, JiraIssue.new("JIRA-123", project))
-      expect(WebMock).to have_requested(:post, @api_url).with(
+      @jira_service.execute(merge_request, JiraIssue.new("JIRA-123", project))
+      expect(WebMock).to have_requested(:post, @comment_url).with(
         body: /Issue solved with/
       ).once
     end
 
     it "calls the api with jira_issue_transition_id" do
       @jira_service.jira_issue_transition_id = 'this-is-a-custom-id'
-      @jira_service.execute(sample_commit, JiraIssue.new("JIRA-123", project))
+      @jira_service.execute(merge_request, JiraIssue.new("JIRA-123", project))
       expect(WebMock).to have_requested(:post, @api_url).with(
         body: /this-is-a-custom-id/
       ).once

spec/services/git_push_service_spec.rb

@@ -310,14 +310,7 @@ describe GitPushService do
         let(:message) { "this is some work.\n\ncloses JIRA-1" }
 
         it "should initiate one api call to jira server to close the issue" do
-          body = {
-            update: {
-              comment: [{
-                add: {
-                  body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]."
-                }
-              }]
-            },
+          transition_body = {
             transition: {
               id: '2'
             }
@@ -325,7 +318,18 @@ describe GitPushService do
 
           service.execute(project, user, @oldrev, @newrev, @ref)
           expect(WebMock).to have_requested(:post, jira_api_transition_url).with(
-            body: body
+            body: transition_body
+          ).once
+        end
+
+        it "should initiate one api call to jira server to comment on the issue" do
+          comment_body = {
+            body: "Issue solved with [#{closing_commit.id}|http://localhost/#{project.path_with_namespace}/commit/#{closing_commit.id}]."
+          }.to_json
+
+          service.execute(project, user, @oldrev, @newrev, @ref)
+          expect(WebMock).to have_requested(:post, jira_api_comment_url).with(
+            body: comment_body
           ).once
         end
       end

spec/support/jira_service_helper.rb

@@ -13,7 +13,7 @@ module JiraServiceHelper
   end
 
   def jira_status_message
-    "JiraService SUCCESS 200: Sucessfully posted to #{jira_api_comment_url}."
+    "JiraService SUCCESS 200: Successfully posted to #{jira_api_comment_url}."
   end
 
   def jira_issue_comments