Add latest changes from gitlab-org/gitlab@12-3-auto-deploy-20190916stable-branch-foss-test

10 files changed, 267 insertions, 397 deletions

diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb
index e25d87f61d6..74c85a13c88 100644
--- a/spec/models/project_services/bugzilla_service_spec.rb
+++ b/spec/models/project_services/bugzilla_service_spec.rb
@@ -48,7 +48,7 @@ describe BugzillaService do
         create(:bugzilla_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -56,7 +56,7 @@ describe BugzillaService do
         create(:bugzilla_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -65,7 +65,7 @@ describe BugzillaService do
         create(:bugzilla_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb
index 8359bc6807a..5259357a254 100644
--- a/spec/models/project_services/custom_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb
@@ -62,7 +62,7 @@ describe CustomIssueTrackerService do
         create(:custom_issue_tracker_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -70,7 +70,7 @@ describe CustomIssueTrackerService do
         create(:custom_issue_tracker_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -79,7 +79,7 @@ describe CustomIssueTrackerService do
         create(:custom_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
diff --git a/spec/models/project_services/data_fields_spec.rb b/spec/models/project_services/data_fields_spec.rb
deleted file mode 100644
index 146db0ae227..00000000000
--- a/spec/models/project_services/data_fields_spec.rb
+++ /dev/null
@@ -1,138 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe DataFields do
-  let(:url) { 'http://url.com' }
-  let(:username) { 'username_one' }
-  let(:properties) do
-    { url: url, username: username }
-  end
-
-  shared_examples 'data fields' do
-    describe '#arg' do
-      it 'returns an argument correctly' do
-        expect(service.url).to eq(url)
-      end
-    end
-
-    describe '{arg}_changed?' do
-      it 'returns false when the property has not been assigned a new value' do
-        service.username = 'new_username'
-        service.validate
-        expect(service.url_changed?).to be_falsy
-      end
-
-      it 'returns true when the property has been assigned a different value' do
-        service.url = "http://example.com"
-        service.validate
-        expect(service.url_changed?).to be_truthy
-      end
-
-      it 'returns true when the property has been assigned a different value twice' do
-        service.url = "http://example.com"
-        service.url = "http://example.com"
-        service.validate
-        expect(service.url_changed?).to be_truthy
-      end
-
-      it 'returns false when the property has been re-assigned the same value' do
-        service.url = 'http://url.com'
-        service.validate
-        expect(service.url_changed?).to be_falsy
-      end
-    end
-
-    describe '{arg}_touched?' do
-      it 'returns false when the property has not been assigned a new value' do
-        service.username = 'new_username'
-        service.validate
-        expect(service.url_changed?).to be_falsy
-      end
-
-      it 'returns true when the property has been assigned a different value' do
-        service.url = "http://example.com"
-        service.validate
-        expect(service.url_changed?).to be_truthy
-      end
-
-      it 'returns true when the property has been assigned a different value twice' do
-        service.url = "http://example.com"
-        service.url = "http://example.com"
-        service.validate
-        expect(service.url_changed?).to be_truthy
-      end
-
-      it 'returns true when the property has been re-assigned the same value' do
-        service.url = 'http://url.com'
-        expect(service.url_touched?).to be_truthy
-      end
-
-      it 'returns false when the property has been re-assigned the same value' do
-        service.url = 'http://url.com'
-        service.validate
-        expect(service.url_changed?).to be_falsy
-      end
-    end
-  end
-
-  context 'when data are stored in data_fields' do
-    let(:service) do
-      create(:jira_service, url: url, username: username)
-    end
-
-    it_behaves_like 'data fields'
-
-    describe '{arg}_was?' do
-      it 'returns nil' do
-        service.url = 'http://example.com'
-        service.validate
-        expect(service.url_was).to be_nil
-      end
-    end
-  end
-
-  context 'when data are stored in properties' do
-    let(:service) { create(:jira_service, :without_properties_callback, properties: properties) }
-
-    it_behaves_like 'data fields'
-
-    describe '{arg}_was?' do
-      it 'returns nil when the property has not been assigned a new value' do
-        service.username = 'new_username'
-        service.validate
-        expect(service.url_was).to be_nil
-      end
-
-      it 'returns initial value when the property has been assigned a different value' do
-        service.url = 'http://example.com'
-        service.validate
-        expect(service.url_was).to eq('http://url.com')
-      end
-
-      it 'returns initial value when the property has been re-assigned the same value' do
-        service.url = 'http://url.com'
-        service.validate
-        expect(service.url_was).to eq('http://url.com')
-      end
-    end
-  end
-
-  context 'when data are stored in both properties and data_fields' do
-    let(:service) do
-      create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service|
-        create(:jira_tracker_data, properties.merge(service: service))
-      end
-    end
-
-    it_behaves_like 'data fields'
-
-    describe '{arg}_was?' do
-      it 'returns nil' do
-        service.url = 'http://example.com'
-        service.validate
-        expect(service.url_was).to be_nil
-      end
-    end
-  end
-end
diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
index 4f3736ca65b..0c4fc290a13 100644
--- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
@@ -65,7 +65,7 @@ describe GitlabIssueTrackerService do
         create(:gitlab_issue_tracker_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -73,7 +73,7 @@ describe GitlabIssueTrackerService do
         create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -82,7 +82,7 @@ describe GitlabIssueTrackerService do
         create(:gitlab_issue_tracker_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
diff --git a/spec/models/project_services/issue_tracker_data_spec.rb b/spec/models/project_services/issue_tracker_data_spec.rb
index db617cf0abb..aaab654f874 100644
--- a/spec/models/project_services/issue_tracker_data_spec.rb
+++ b/spec/models/project_services/issue_tracker_data_spec.rb
@@ -8,4 +8,28 @@ describe IssueTrackerData do
   describe 'Associations' do
     it { is_expected.to belong_to :service }
   end
+
+  describe 'Validations' do
+    subject { described_class.new(service: service) }
+
+    context 'url validations' do
+      context 'when service is inactive' do
+        it { is_expected.not_to validate_presence_of(:project_url) }
+        it { is_expected.not_to validate_presence_of(:issues_url) }
+      end
+
+      context 'when service is active' do
+        before do
+          service.update(active: true)
+        end
+
+        it_behaves_like 'issue tracker service URL attribute', :project_url
+        it_behaves_like 'issue tracker service URL attribute', :issues_url
+        it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+
+        it { is_expected.to validate_presence_of(:project_url) }
+        it { is_expected.to validate_presence_of(:issues_url) }
+      end
+    end
+  end
 end
diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/project_services/issue_tracker_service_spec.rb
index f1cdee5c4a3..2fc4d69c2db 100644
--- a/spec/models/project_services/issue_tracker_service_spec.rb
+++ b/spec/models/project_services/issue_tracker_service_spec.rb
@@ -7,7 +7,7 @@ describe IssueTrackerService do
     let(:project) { create :project }
 
     describe 'only one issue tracker per project' do
-      let(:service) { RedmineService.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) }
+      let(:service) { RedmineService.new(project: project, active: true) }
 
       before do
         create(:custom_issue_tracker_service, project: project)
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index a976745023b..02060699e9a 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -6,18 +6,10 @@ describe JiraService do
   include Gitlab::Routing
   include AssetsHelpers
 
-  let(:title) { 'custom title' }
-  let(:description) { 'custom description' }
-  let(:url) { 'http://jira.example.com' }
-  let(:api_url) { 'http://api-jira.example.com' }
-  let(:username) { 'jira-username' }
-  let(:password) { 'jira-password' }
-  let(:transition_id) { 'test27' }
-
   describe '#options' do
     let(:service) do
-      described_class.create(
-        project: create(:project),
+      described_class.new(
+        project: build_stubbed(:project),
         active: true,
         username: 'username',
         password: 'test',
@@ -40,6 +32,78 @@ 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
+    context 'when service is active' do
+      before do
+        subject.active = true
+      end
+
+      it { is_expected.to validate_presence_of(:url) }
+      it_behaves_like 'issue tracker service URL attribute', :url
+    end
+
+    context 'when service is inactive' do
+      before do
+        subject.active = false
+      end
+
+      it { is_expected.not_to validate_presence_of(:url) }
+      it { is_expected.not_to validate_presence_of(:username) }
+      it { is_expected.not_to validate_presence_of(:password) }
+    end
+
+    context 'validating urls' do
+      let(:service) do
+        described_class.new(
+          project: create(:project),
+          active: true,
+          username: 'username',
+          password: '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 not valid when username is missing' do
+        service.username = nil
+
+        expect(service).not_to be_valid
+      end
+
+      it 'is not valid when password is missing' do
+        service.password = nil
+
+        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
@@ -54,260 +118,55 @@ describe JiraService do
   describe '#create' do
     let(:params) do
       {
-        project: create(:project),
-        title: 'custom title', description: 'custom description',
-        url: url, api_url: api_url,
-        username: username, password: password,
-        jira_issue_transition_id: transition_id
+        project: create(:project), title: 'custom title', description: 'custom description'
       }
     end
 
     subject { described_class.create(params) }
 
-    it 'does not store data into properties' do
-      expect(subject.properties).to be_nil
-    end
-
-    it 'sets title  correctly' do
-      service = subject
-
-      expect(service.title).to eq('custom title')
+    it 'does not store title & description into properties' do
+      expect(subject.properties.keys).not_to include('title', 'description')
     end
 
-    it 'sets service data correctly' do
+    it 'sets title & description correctly' do
       service = subject
 
       expect(service.title).to eq('custom title')
       expect(service.description).to eq('custom description')
     end
-
-    it 'stores data in data_fields correcty' do
-      service = subject
-
-      expect(service.jira_tracker_data.url).to eq(url)
-      expect(service.jira_tracker_data.api_url).to eq(api_url)
-      expect(service.jira_tracker_data.username).to eq(username)
-      expect(service.jira_tracker_data.password).to eq(password)
-      expect(service.jira_tracker_data.jira_issue_transition_id).to eq(transition_id)
-    end
   end
 
-  # we need to make sure we are able to read both from properties and jira_tracker_data table
-  # TODO: change this as part of #63084
   context 'overriding properties' do
+    let(:url) { 'http://issue_tracker.example.com' }
     let(:access_params) do
-      { url: url, api_url: api_url, username: username, password: password,
-        jira_issue_transition_id: transition_id }
-    end
-    let(:data_params) do
-      {
-        url: url, api_url: api_url,
-        username: username, password: password,
-        jira_issue_transition_id: transition_id
-      }
-    end
-
-    shared_examples 'handles jira fields' do
-      let(:data_params) do
-        {
-          url: url, api_url: api_url,
-          username: username, password: password,
-          jira_issue_transition_id: transition_id
-        }
-      end
-
-      context 'reading data' do
-        it 'reads data correctly' do
-          expect(service.url).to eq(url)
-          expect(service.api_url).to eq(api_url)
-          expect(service.username).to eq(username)
-          expect(service.password).to eq(password)
-          expect(service.jira_issue_transition_id).to eq(transition_id)
-        end
-      end
-
-      context '#update' do
-        context 'basic update' do
-          let(:new_username) { 'new_username' }
-          let(:new_url) { 'http://jira-new.example.com' }
-
-          before do
-            service.update(username: new_username, url: new_url)
-          end
-
-          it 'leaves properties field emtpy' do
-            # expect(service.reload.properties).to be_empty
-          end
-
-          it 'stores updated data in jira_tracker_data table' do
-            data = service.jira_tracker_data.reload
-
-            expect(data.url).to eq(new_url)
-            expect(data.api_url).to eq(api_url)
-            expect(data.username).to eq(new_username)
-            expect(data.password).to eq(password)
-            expect(data.jira_issue_transition_id).to eq(transition_id)
-          end
-        end
-
-        context 'stored password invalidation' do
-          context 'when a password was previously set' do
-            context 'when only web url present' do
-              let(:data_params) do
-                {
-                  url: url, api_url: nil,
-                  username: username, password: password,
-                  jira_issue_transition_id: transition_id
-                }
-              end
-
-              it 'resets password if url changed' do
-                service
-                service.url = 'http://jira_edited.example.com'
-                service.save
-
-                expect(service.reload.url).to eq('http://jira_edited.example.com')
-                expect(service.password).to be_nil
-              end
-
-              it 'does not reset password if url "changed" to the same url as before' do
-                service.title = 'aaaaaa'
-                service.url = 'http://jira.example.com'
-                service.save
-
-                expect(service.reload.url).to eq('http://jira.example.com')
-                expect(service.password).not_to be_nil
-              end
-
-              it 'resets password if url not changed but api url added' do
-                service.api_url = 'http://jira_edited.example.com/rest/api/2'
-                service.save
-
-                expect(service.reload.api_url).to eq('http://jira_edited.example.com/rest/api/2')
-                expect(service.password).to be_nil
-              end
-
-              it 'does not reset password if new url is set together with password, even if it\'s the same password' do
-                service.url = 'http://jira_edited.example.com'
-                service.password = password
-                service.save
-
-                expect(service.password).to eq(password)
-                expect(service.url).to eq('http://jira_edited.example.com')
-              end
-
-              it 'resets password if url changed, even if setter called multiple times' do
-                service.url = 'http://jira1.example.com/rest/api/2'
-                service.url = 'http://jira1.example.com/rest/api/2'
-                service.save
-
-                expect(service.password).to be_nil
-              end
-
-              it 'does not reset password if username changed' do
-                service.username = 'some_name'
-                service.save
-
-                expect(service.reload.password).to eq(password)
-              end
-
-              it 'does not reset password if password changed' do
-                service.url = 'http://jira_edited.example.com'
-                service.password = 'new_password'
-                service.save
-
-                expect(service.reload.password).to eq('new_password')
-              end
-
-              it 'does not reset password if the password is touched and same as before' do
-                service.url = 'http://jira_edited.example.com'
-                service.password = password
-                service.save
-
-                expect(service.reload.password).to eq(password)
-              end
-            end
-
-            context 'when both web and api url present' do
-              let(:data_params) do
-                {
-                  url: url, api_url: 'http://jira.example.com/rest/api/2',
-                  username: username, password: password,
-                  jira_issue_transition_id: transition_id
-                }
-              end
-
-              it 'resets password if api url changed' do
-                service.api_url = 'http://jira_edited.example.com/rest/api/2'
-                service.save
-
-                expect(service.password).to be_nil
-              end
-
-              it 'does not reset password if url changed' do
-                service.url = 'http://jira_edited.example.com'
-                service.save
-
-                expect(service.password).to eq(password)
-              end
-
-              it 'resets password if api url set to empty' do
-                service.update(api_url: '')
-
-                expect(service.reload.password).to be_nil
-              end
-            end
-          end
-
-          context 'when no password was previously set' do
-            let(:data_params) do
-              {
-                url: url, username: username
-              }
-            end
-
-            it 'saves password if new url is set together with password' do
-              service.url = 'http://jira_edited.example.com/rest/api/2'
-              service.password = 'password'
-              service.save
-              expect(service.reload.password).to eq('password')
-              expect(service.reload.url).to eq('http://jira_edited.example.com/rest/api/2')
-            end
-          end
-        end
-      end
+      { url: url, username: 'username', password: 'password' }
     end
 
     # this  will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
     context 'when data are stored in properties' do
-      let(:properties) { data_params.merge(title: title, description: description) }
-      let!(:service) do
+      let(:properties) { access_params.merge(title: title, description: description) }
+      let(:service) do
         create(:jira_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
-      it_behaves_like 'handles jira fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
       let(:service) do
-        create(:jira_service, data_params.merge(properties: {}, title: title, description: description))
+        create(:jira_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
-      it_behaves_like 'handles jira fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
-      let(:properties) { data_params.merge(title: title, description: description) }
+      let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
       let(:service) do
-        create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service|
-          create(:jira_tracker_data, data_params.merge(service: service))
-        end
+        create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
-      it_behaves_like 'handles jira fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
@@ -551,6 +410,111 @@ describe JiraService do
     end
   end
 
+  describe 'Stored password invalidation' do
+    let(:project) { create(:project) }
+
+    context 'when a password was previously set' do
+      before do
+        @jira_service = described_class.create!(
+          project: project,
+          properties: {
+            url: 'http://jira.example.com/web',
+            username: 'mic',
+            password: 'password'
+          }
+        )
+      end
+
+      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
+
+      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 empty' 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')
+      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')
+      end
+
+      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
+        expect(@jira_service.password).to be_nil
+      end
+    end
+
+    context 'when no password was previously set' do
+      before do
+        @jira_service = described_class.create(
+          project: project,
+          properties: {
+            url: 'http://jira.example.com/rest/api/2',
+            username: 'mic'
+          }
+        )
+      end
+
+      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')
+      end
+    end
+  end
+
   describe 'description and title' do
     let(:title) { 'Jira One' }
     let(:description) { 'Jira One issue tracker' }
@@ -575,7 +539,7 @@ describe JiraService do
 
     context 'when it is set in properties' do
       it 'values from properties are returned' do
-        service = create(:jira_service, :without_properties_callback, properties: properties)
+        service = create(:jira_service, properties: properties)
 
         expect(service.title).to eq(title)
         expect(service.description).to eq(description)
@@ -638,8 +602,8 @@ describe JiraService do
         project = create(:project)
         service = project.create_jira_service(active: true)
 
-        expect(service.url).to eq('http://jira.sample/projects/project_a')
-        expect(service.api_url).to eq('http://jira.sample/api')
+        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
 
diff --git a/spec/models/project_services/jira_tracker_data_spec.rb b/spec/models/project_services/jira_tracker_data_spec.rb
index 6cd3eb33d9b..1b6ece8531b 100644
--- a/spec/models/project_services/jira_tracker_data_spec.rb
+++ b/spec/models/project_services/jira_tracker_data_spec.rb
@@ -8,4 +8,35 @@ describe JiraTrackerData do
   describe 'Associations' do
     it { is_expected.to belong_to(:service) }
   end
+
+  describe 'Validations' do
+    subject { described_class.new(service: service) }
+
+    context 'jira_issue_transition_id' do
+      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
+
+    context 'url validations' do
+      context 'when service is inactive' do
+        it { is_expected.not_to validate_presence_of(:url) }
+        it { is_expected.not_to validate_presence_of(:username) }
+        it { is_expected.not_to validate_presence_of(:password) }
+      end
+
+      context 'when service is active' do
+        before do
+          service.update(active: true)
+        end
+
+        it_behaves_like 'issue tracker service URL attribute', :url
+
+        it { is_expected.to validate_presence_of(:url) }
+        it { is_expected.to validate_presence_of(:username) }
+        it { is_expected.to validate_presence_of(:password) }
+      end
+    end
+  end
 end
diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb
index 4ef4064d069..c1ee6546b12 100644
--- a/spec/models/project_services/redmine_service_spec.rb
+++ b/spec/models/project_services/redmine_service_spec.rb
@@ -9,15 +9,6 @@ describe RedmineService do
   end
 
   describe 'Validations' do
-    # if redmine is set in setting the urls are set to defaults
-    # therefore the validation passes as the values are not nil
-    before do
-      settings = {
-        'redmine' => {}
-      }
-      allow(Gitlab.config).to receive(:issues_tracker).and_return(settings)
-    end
-
     context 'when service is active' do
       before do
         subject.active = true
@@ -26,7 +17,6 @@ describe RedmineService do
       it { is_expected.to validate_presence_of(:project_url) }
       it { is_expected.to validate_presence_of(:issues_url) }
       it { is_expected.to validate_presence_of(:new_issue_url) }
-
       it_behaves_like 'issue tracker service URL attribute', :project_url
       it_behaves_like 'issue tracker service URL attribute', :issues_url
       it_behaves_like 'issue tracker service URL attribute', :new_issue_url
@@ -64,7 +54,7 @@ describe RedmineService do
         create(:redmine_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -72,7 +62,7 @@ describe RedmineService do
         create(:redmine_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -81,7 +71,7 @@ describe RedmineService do
         create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb
index eff9f451b1a..c48bf487af0 100644
--- a/spec/models/project_services/youtrack_service_spec.rb
+++ b/spec/models/project_services/youtrack_service_spec.rb
@@ -16,7 +16,6 @@ describe YoutrackService do
 
       it { is_expected.to validate_presence_of(:project_url) }
       it { is_expected.to validate_presence_of(:issues_url) }
-
       it_behaves_like 'issue tracker service URL attribute', :project_url
       it_behaves_like 'issue tracker service URL attribute', :issues_url
     end
@@ -52,7 +51,7 @@ describe YoutrackService do
         create(:youtrack_service, :without_properties_callback, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -60,7 +59,7 @@ describe YoutrackService do
         create(:youtrack_service, title: title, description: description, properties: access_params)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -69,7 +68,7 @@ describe YoutrackService do
         create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      it_behaves_like 'issue tracker fields'
+      include_examples 'issue tracker fields'
     end
 
     context 'when no title & description are set' do