Allow multiple JIRA transition ids

5 files changed, 73 insertions, 9 deletions

diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb
index 412d62388f0..82d438d5378 100644
--- a/app/models/project_services/jira_service.rb
+++ b/app/models/project_services/jira_service.rb
@@ -8,6 +8,10 @@ class JiraService < IssueTrackerService
   validates :username, presence: true, if: :activated?
   validates :password, presence: true, if: :activated?
 
+  validates :jira_issue_transition_id,
+            format: { with: Gitlab::Regex.jira_transition_id_regex, message: "transition ids can have only numbers which can be split with , or ;" },
+            allow_blank: true
+
   prop_accessor :username, :password, :url, :api_url, :jira_issue_transition_id, :title, :description
 
   before_update :reset_password
@@ -91,7 +95,7 @@ class JiraService < IssueTrackerService
       { type: 'text', name: 'api_url', title: 'JIRA API URL', placeholder: 'If different from Web URL' },
       { type: 'text', name: 'username', placeholder: '', required: true },
       { type: 'password', name: 'password', placeholder: '', required: true },
-      { type: 'text', name: 'jira_issue_transition_id', title: 'Transition ID', placeholder: '' }
+      { type: 'text', name: 'jira_issue_transition_id', title: 'Transition ID(s)', placeholder: 'Use , or ; to separate multiple transition IDs' }
     ]
   end
 
@@ -191,8 +195,18 @@ class JiraService < IssueTrackerService
     end
   end
 
+  # jira_issue_transition_id can have multiple values split by , or ;
+  # the issue is transitioned at the order given by the user
+  # if any transition fails it will log the error message and stop the transition sequence
   def transition_issue(issue)
-    issue.transitions.build.save(transition: { id: jira_issue_transition_id })
+    jira_issue_transition_id.scan(Gitlab::Regex.jira_transition_id_regex).each do |transition_id|
+      begin
+        issue.transitions.build.save!(transition: { id: transition_id })
+      rescue => error
+        Rails.logger.info "#{self.class.name} Issue Transition failed message ERROR: #{client_url} - #{error.message}"
+        return false
+      end
+    end
   end
 
   def add_issue_solved_comment(issue, commit_id, commit_url)
diff --git a/changelogs/unreleased/issue_43602.yml b/changelogs/unreleased/issue_43602.yml
new file mode 100644
index 00000000000..0482606db0a
--- /dev/null
+++ b/changelogs/unreleased/issue_43602.yml
@@ -0,0 +1,5 @@
+---
+title: Allow multiple JIRA transition ids
+merge_request: 20939
+author:
+type: changed
diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md
index 4d5b2c97291..67c543e00fb 100644
--- a/doc/user/project/integrations/jira.md
+++ b/doc/user/project/integrations/jira.md
@@ -113,7 +113,7 @@ in the table below.
 | `JIRA API URL` | The base URL to the JIRA instance API. Web URL value will be used if not set. E.g., `https://jira-api.example.com`. |
 | `Username` | The user name created in [configuring JIRA step](#configuring-jira). Using the email address will cause `401 unauthorized`. |
 | `Password` |The password of the user created in [configuring JIRA step](#configuring-jira). |
-| `Transition ID` | This is the ID of a transition that moves issues to the desired state.  **Closing JIRA issues via commits or Merge Requests won't work if you don't set the ID correctly.** |
+| `Transition ID` | This is the ID of a transition that moves issues to the desired state. It is possible to insert transition ids separated by `,` or `;` which means the issue will be moved to each state after another using the given order.  **Closing JIRA issues via commits or Merge Requests won't work if you don't set the ID correctly.** |
 
 ### Getting a transition ID
 
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index e1a958c508a..0f26fcfe8cb 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -99,5 +99,9 @@ module Gitlab
           )
       }mx
     end
+
+    def jira_transition_id_regex
+      @jira_transition_id_regex ||= /\d+/
+    end
   end
 end
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 6c637533c6b..ac9ff59b9b5 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -30,6 +30,10 @@ describe JiraService do
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
+    it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) }
+    it { is_expected.to allow_value("1,2,3").for(:jira_issue_transition_id) }
+    it { is_expected.to allow_value("1;2;3").for(:jira_issue_transition_id) }
+    it { is_expected.not_to allow_value("a,b,cd").for(:jira_issue_transition_id) }
   end
 
   describe 'Validations' do
@@ -124,7 +128,7 @@ describe JiraService do
         url: 'http://jira.example.com',
         username: 'gitlab_jira_username',
         password: 'gitlab_jira_password',
-        jira_issue_transition_id: "custom-id"
+        jira_issue_transition_id: "999"
       )
 
       # These stubs are needed to test JiraService#close_issue.
@@ -226,12 +230,49 @@ describe JiraService do
       ).once
     end
 
-    it "calls the api with jira_issue_transition_id" do
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+    context '#close_issue' do
+      it "logs exception when transition id is not valid" do
+        allow(Rails.logger).to receive(:info)
+        WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise("Bad Request")
 
-      expect(WebMock).to have_requested(:post, @transitions_url).with(
-        body: /custom-id/
-      ).once
+        @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+
+        expect(Rails.logger).to have_received(:info).with("JiraService Issue Transition failed message ERROR: http://jira.example.com - Bad Request")
+      end
+
+      it "calls the api with jira_issue_transition_id" do
+        @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+
+        expect(WebMock).to have_requested(:post, @transitions_url).with(
+          body: /999/
+        ).once
+      end
+
+      context "when have multiple transition ids" do
+        it "calls the api with transition ids separated by comma" do
+          allow(@jira_service).to receive_messages(jira_issue_transition_id: "1,2,3")
+
+          @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+
+          1.upto(3) do |transition_id|
+            expect(WebMock).to have_requested(:post, @transitions_url).with(
+              body: /#{transition_id}/
+            ).once
+          end
+        end
+
+        it "calls the api with transition ids separated by semicolon" do
+          allow(@jira_service).to receive_messages(jira_issue_transition_id: "1;2;3")
+
+          @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+
+          1.upto(3) do |transition_id|
+            expect(WebMock).to have_requested(:post, @transitions_url).with(
+              body: /#{transition_id}/
+            ).once
+          end
+        end
+      end
     end
   end