diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-06 13:45:24 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-06 13:45:24 +0000 |
commit | ad570fa6a1d6c981243ff1c64a8be2c8e369f207 (patch) | |
tree | da7b5754959b9ff5c03049e60a5b8a3af1dd9897 | |
parent | 77c75d2b1b4821b1f5fafd107e1925c225fb6a33 (diff) | |
parent | 32b88294d5a3b7bc22682c7942d8b3c4fa1502c6 (diff) | |
download | gitlab-ce-ad570fa6a1d6c981243ff1c64a8be2c8e369f207.tar.gz |
Merge branch 'issue_43602' into 'master'
Allow multiple JIRA transitions id split by comma or semicolon
Closes #43602
See merge request gitlab-org/gitlab-ce!20939
-rw-r--r-- | app/models/project_services/jira_service.rb | 18 | ||||
-rw-r--r-- | changelogs/unreleased/issue_43602.yml | 5 | ||||
-rw-r--r-- | doc/user/project/integrations/jira.md | 2 | ||||
-rw-r--r-- | lib/gitlab/regex.rb | 4 | ||||
-rw-r--r-- | spec/models/project_services/jira_service_spec.rb | 53 |
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 |