diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index a91a986e195e89e41fcc253f3bd5889280b93b36..fe8696238331378b7ce07ba698c5d9116f075974 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -2,9 +2,10 @@ class JiraService < IssueTrackerService include Gitlab::Routing.url_helpers validates :url, url: true, presence: true, if: :activated? + validates :api_url, url: true, allow_blank: true validates :project_key, presence: true, if: :activated? - prop_accessor :username, :password, :url, :project_key, + prop_accessor :username, :password, :url, :api_url, :project_key, :jira_issue_transition_id, :title, :description before_update :reset_password @@ -25,20 +26,18 @@ class JiraService < IssueTrackerService super do self.properties = { title: issues_tracker['title'], - url: issues_tracker['url'] + url: issues_tracker['url'], + api_url: issues_tracker['api_url'] } end end def reset_password - # don't reset the password if a new one is provided - if url_changed? && !password_touched? - self.password = nil - end + self.password = nil if reset_password? end def options - url = URI.parse(self.url) + url = URI.parse(client_url) { username: self.username, @@ -87,7 +86,8 @@ class JiraService < IssueTrackerService def fields [ - { type: 'text', name: 'url', title: 'URL', placeholder: 'https://jira.example.com' }, + { type: 'text', name: 'url', title: 'Web URL', placeholder: 'https://jira.example.com' }, + { type: 'text', name: 'api_url', title: 'JIRA API URL', placeholder: 'If different from Web URL' }, { type: 'text', name: 'project_key', placeholder: 'Project Key' }, { type: 'text', name: 'username', placeholder: '' }, { type: 'password', name: 'password', placeholder: '' }, @@ -186,7 +186,7 @@ class JiraService < IssueTrackerService end def test_settings - return unless url.present? + return unless client_url.present? # Test settings by getting the project jira_request { jira_project.present? } end @@ -236,13 +236,13 @@ class JiraService < IssueTrackerService end def send_message(issue, message, remote_link_props) - return unless url.present? + return unless client_url.present? jira_request do if issue.comments.build.save!(body: message) remote_link = issue.remotelink.build remote_link.save!(remote_link_props) - result_message = "#{self.class.name} SUCCESS: Successfully posted to #{url}." + result_message = "#{self.class.name} SUCCESS: Successfully posted to #{client_url}." end Rails.logger.info(result_message) @@ -295,7 +295,20 @@ class JiraService < IssueTrackerService yield rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError, OpenSSL::SSL::SSLError => e - Rails.logger.info "#{self.class.name} Send message ERROR: #{url} - #{e.message}" + Rails.logger.info "#{self.class.name} Send message ERROR: #{client_url} - #{e.message}" nil end + + def client_url + api_url.present? ? api_url : url + end + + def reset_password? + # don't reset the password if a new one is provided + return false if password_touched? + return true if api_url_changed? + return false if api_url.present? + + url_changed? + end end diff --git a/changelogs/unreleased/31448-jira-urls.yml b/changelogs/unreleased/31448-jira-urls.yml new file mode 100644 index 0000000000000000000000000000000000000000..d0e39f61b55599191560db697c1443cd02ddf0dc --- /dev/null +++ b/changelogs/unreleased/31448-jira-urls.yml @@ -0,0 +1,4 @@ +--- +title: Add API URL to JIRA settings +merge_request: +author: diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md index f611029afdc24582e4aa3ed6bc2d84401e413ebe..a048260b033ad3d0da7a068234badf3d2f11f199 100644 --- a/doc/user/project/integrations/jira.md +++ b/doc/user/project/integrations/jira.md @@ -97,7 +97,8 @@ in the table below. | Field | Description | | ----- | ----------- | -| `URL` | The base URL to the JIRA project which is being linked to this GitLab project. E.g., `https://jira.example.com`. | +| `Web URL` | The base URL to the JIRA instance web interface which is being linked to this GitLab project. E.g., `https://jira.example.com`. | +| `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`. | | `Project key` | The short identifier for your JIRA project, all uppercase, e.g., `PROJ`. | | `Username` | The user name created in [configuring JIRA step](#configuring-jira). | | `Password` |The password of the user created in [configuring JIRA step](#configuring-jira). | diff --git a/features/steps/project/services.rb b/features/steps/project/services.rb index 3c0d987e403b81a26db59349d8d969097d30d9fa..66368a159ec9cd81ac6ccc0f9de5100b0795e654 100644 --- a/features/steps/project/services.rb +++ b/features/steps/project/services.rb @@ -178,7 +178,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps end step 'I fill jira settings' do - fill_in 'URL', with: 'http://jira.example' + fill_in 'Web URL', with: 'http://jira.example' + fill_in 'JIRA API URL', with: 'http://jira.example/api' fill_in 'Username', with: 'gitlab' fill_in 'Password', with: 'gitlab' fill_in 'Project Key', with: 'GITLAB' @@ -186,7 +187,8 @@ class Spinach::Features::ProjectServices < Spinach::FeatureSteps end step 'I should see jira service settings saved' do - expect(find_field('URL').value).to eq 'http://jira.example' + expect(find_field('Web URL').value).to eq 'http://jira.example' + expect(find_field('JIRA API URL').value).to eq 'http://jira.example/api' expect(find_field('Username').value).to eq 'gitlab' expect(find_field('Project Key').value).to eq 'GITLAB' end diff --git a/lib/api/services.rb b/lib/api/services.rb index cb07df9e24972a8bd87cb7189ae6c78fb3dc9292..47bd9940f77181368bffcff147b23f7fdb96f1da 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -304,7 +304,13 @@ module API required: true, name: :url, type: String, - desc: 'The URL to the JIRA project which is being linked to this GitLab project, e.g., https://jira.example.com' + desc: 'The base URL to the JIRA instance web interface which is being linked to this GitLab project. E.g., https://jira.example.com' + }, + { + required: false, + name: :api_url, + type: String, + desc: 'The base URL to the JIRA instance API. Web URL value will be used if not set. E.g., https://jira-api.example.com' }, { required: true, diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 4bca0229e7aee3eab66dc1cc2e36c760df3e18d3..349067e73ab6ab6062994d5caab0edb16c71b334 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -22,6 +22,42 @@ describe JiraService, models: true do it { is_expected.not_to validate_presence_of(:url) } end + + context 'validating urls' do + let(:service) do + described_class.new( + project: create(:empty_project), + active: true, + username: 'username', + password: 'test', + project_key: 'TEST', + jira_issue_transition_id: 24, + url: 'http://jira.test.com' + ) + end + + it 'is valid when all fields have required values' do + expect(service).to be_valid + end + + it 'is not valid when url is not a valid url' do + service.url = 'not valid' + + expect(service).not_to be_valid + end + + it 'is not valid when api url is not a valid url' do + service.api_url = 'not valid' + + expect(service).not_to be_valid + end + + it 'is valid when api url is a valid url' do + service.api_url = 'http://jira.test.com/api' + + expect(service).to be_valid + end + end end describe '#reference_pattern' do @@ -187,22 +223,29 @@ describe JiraService, models: true do describe '#test_settings' do let(:jira_service) do described_class.new( + project: create(:project), url: 'http://jira.example.com', - username: 'gitlab_jira_username', - password: 'gitlab_jira_password', + username: 'jira_username', + password: 'jira_password', project_key: 'GitLabProject' ) end - let(:project_url) { 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' } - before do + def test_settings(api_url) + project_url = "http://jira_username:jira_password@#{api_url}/rest/api/2/project/GitLabProject" + WebMock.stub_request(:get, project_url) - end - it 'tries to get JIRA project' do jira_service.test_settings + end + + it 'tries to get JIRA project with URL when API URL not set' do + test_settings('jira.example.com') + end - expect(WebMock).to have_requested(:get, project_url) + it 'tries to get JIRA project with API URL if set' do + jira_service.update(api_url: 'http://jira.api.com') + test_settings('jira.api.com') end end @@ -214,34 +257,75 @@ describe JiraService, models: true do @jira_service = JiraService.create!( project: project, properties: { - url: 'http://jira.example.com/rest/api/2', + url: 'http://jira.example.com/web', username: 'mic', password: "password" } ) end - it "reset password if url changed" do - @jira_service.url = 'http://jira_edited.example.com/rest/api/2' - @jira_service.save - expect(@jira_service.password).to be_nil + context 'when only web url present' do + it 'reset password if url changed' do + @jira_service.url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + + it 'reset password if url not changed but api url added' do + @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end end - it "does not reset password if username changed" do - @jira_service.username = "some_name" + context 'when both web and api url present' do + before do + @jira_service.api_url = 'http://jira.example.com/rest/api/2' + @jira_service.password = 'password' + + @jira_service.save + end + it 'reset password if api url changed' do + @jira_service.api_url = 'http://jira_edited.example.com/rest/api/2' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + + it 'does not reset password if url changed' do + @jira_service.url = 'http://jira_edited.example.com/rweb' + @jira_service.save + + expect(@jira_service.password).to eq("password") + end + + it 'reset password if api url set to ""' do + @jira_service.api_url = '' + @jira_service.save + + expect(@jira_service.password).to be_nil + end + end + + it 'does not reset password if username changed' do + @jira_service.username = 'some_name' @jira_service.save - expect(@jira_service.password).to eq("password") + + expect(@jira_service.password).to eq('password') end - it "does not reset password if new url is set together with password, even if it's the same password" do + it 'does not reset password if new url is set together with password, even if it\'s the same password' do @jira_service.url = 'http://jira_edited.example.com/rest/api/2' @jira_service.password = 'password' @jira_service.save - expect(@jira_service.password).to eq("password") - expect(@jira_service.url).to eq("http://jira_edited.example.com/rest/api/2") + + expect(@jira_service.password).to eq('password') + expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2') end - it "resets password if url changed, even if setter called multiple times" do + it 'resets password if url changed, even if setter called multiple times' do @jira_service.url = 'http://jira1.example.com/rest/api/2' @jira_service.url = 'http://jira1.example.com/rest/api/2' @jira_service.save @@ -249,7 +333,7 @@ describe JiraService, models: true do end end - context "when no password was previously set" do + context 'when no password was previously set' do before do @jira_service = JiraService.create( project: project, @@ -260,26 +344,16 @@ describe JiraService, models: true do ) end - it "saves password if new url is set together with password" do + it 'saves password if new url is set together with password' do @jira_service.url = 'http://jira_edited.example.com/rest/api/2' @jira_service.password = 'password' @jira_service.save - expect(@jira_service.password).to eq("password") - expect(@jira_service.url).to eq("http://jira_edited.example.com/rest/api/2") + expect(@jira_service.password).to eq('password') + expect(@jira_service.url).to eq('http://jira_edited.example.com/rest/api/2') end end end - describe "Validations" do - context "active" do - before do - subject.active = true - end - - it { is_expected.to validate_presence_of :url } - end - end - describe 'description and title' do let(:project) { create(:empty_project) } @@ -321,9 +395,10 @@ describe JiraService, models: true do context 'when gitlab.yml was initialized' do before do settings = { - "jira" => { - "title" => "Jira", - "url" => "http://jira.sample/projects/project_a" + 'jira' => { + 'title' => 'Jira', + 'url' => 'http://jira.sample/projects/project_a', + 'api_url' => 'http://jira.sample/api' } } allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) @@ -335,8 +410,9 @@ describe JiraService, models: true do end it 'is prepopulated with the settings' do - expect(@service.properties["title"]).to eq('Jira') - expect(@service.properties["url"]).to eq('http://jira.sample/projects/project_a') + expect(@service.properties['title']).to eq('Jira') + expect(@service.properties['url']).to eq('http://jira.sample/projects/project_a') + expect(@service.properties['api_url']).to eq('http://jira.sample/api') end end end