Merge branch '31448-jira-urls' into 'master'
Add API URL to JIRA settings Closes #31448 See merge request !11707
This commit is contained in:
commit
2eb3a40567
|
@ -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
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Add API URL to JIRA settings
|
||||
merge_request:
|
||||
author:
|
|
@ -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). |
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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)
|
||||
|
||||
jira_service.test_settings
|
||||
end
|
||||
|
||||
it 'tries to get JIRA project' do
|
||||
jira_service.test_settings
|
||||
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"
|
||||
@jira_service.save
|
||||
expect(@jira_service.password).to eq("password")
|
||||
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 new url is set together with password, even if it's the same password" do
|
||||
it 'does not reset password if username changed' do
|
||||
@jira_service.username = 'some_name'
|
||||
@jira_service.save
|
||||
|
||||
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
|
||||
@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
|
||||
|
|
Loading…
Reference in New Issue