Add latest changes from gitlab-org/gitlab@master

29 files changed, 709 insertions, 409 deletions

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index e1f67054d0a..eda8c282341 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -1289,7 +1289,7 @@ describe Projects::MergeRequestsController do
             expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection|
               note_diff_file = commit_diff_note.note_diff_file
 
-              expect(collection).to receive(:load_highlight).with([note_diff_file.id]).and_call_original
+              expect(collection).to receive(:load_highlight).and_call_original
               expect(collection).to receive(:find_by_id).with(note_diff_file.id).and_call_original
             end
 
@@ -1306,7 +1306,7 @@ describe Projects::MergeRequestsController do
             expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection|
               note_diff_file = diff_note.note_diff_file
 
-              expect(collection).to receive(:load_highlight).with([note_diff_file.id]).and_call_original
+              expect(collection).to receive(:load_highlight).and_call_original
               expect(collection).to receive(:find_by_id).with(note_diff_file.id).and_call_original
             end
 
@@ -1319,7 +1319,7 @@ describe Projects::MergeRequestsController do
             expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection|
               note_diff_file = diff_note.note_diff_file
 
-              expect(collection).to receive(:load_highlight).with([]).and_call_original
+              expect(collection).to receive(:load_highlight).and_call_original
               expect(collection).to receive(:find_by_id).with(note_diff_file.id).and_call_original
             end
 
diff --git a/spec/factories/services.rb b/spec/factories/services.rb
index b2d6ada91fa..c4cb3f07fe7 100644
--- a/spec/factories/services.rb
+++ b/spec/factories/services.rb
@@ -9,11 +9,7 @@ FactoryBot.define do
   factory :custom_issue_tracker_service, class: CustomIssueTrackerService do
     project
     active true
-    properties(
-      project_url: 'https://project.url.com',
-      issues_url: 'https://issues.url.com',
-      new_issue_url: 'https://newissue.url.com'
-    )
+    issue_tracker
   end
 
   factory :emails_on_push_service do
@@ -47,12 +43,24 @@ FactoryBot.define do
   factory :jira_service do
     project
     active true
-    properties(
-      url: 'https://jira.example.com',
-      username: 'jira_user',
-      password: 'my-secret-password',
-      project_key: 'jira-key'
-    )
+
+    transient do
+      create_data true
+      url 'https://jira.example.com'
+      api_url nil
+      username 'jira_username'
+      password 'jira_password'
+      jira_issue_transition_id '56-1'
+    end
+
+    after(:build) do |service, evaluator|
+      if evaluator.create_data
+        create(:jira_tracker_data, service: service,
+               url: evaluator.url, api_url: evaluator.api_url, jira_issue_transition_id: evaluator.jira_issue_transition_id,
+               username: evaluator.username, password: evaluator.password
+        )
+      end
+    end
   end
 
   factory :bugzilla_service do
@@ -80,20 +88,26 @@ FactoryBot.define do
   end
 
   trait :issue_tracker do
-    properties(
-      project_url: 'http://issue-tracker.example.com',
-      issues_url: 'http://issue-tracker.example.com/issues/:id',
-      new_issue_url: 'http://issue-tracker.example.com'
-    )
+    transient do
+      create_data true
+      project_url 'http://issuetracker.example.com'
+      issues_url 'http://issues.example.com/issues/:id'
+      new_issue_url 'http://new-issue.example.com'
+    end
+
+    after(:build) do |service, evaluator|
+      if evaluator.create_data
+        create(:issue_tracker_data, service: service,
+               project_url: evaluator.project_url, issues_url: evaluator.issues_url, new_issue_url: evaluator.new_issue_url
+        )
+      end
+    end
   end
 
   trait :jira_cloud_service do
-    properties(
-      url: 'https://mysite.atlassian.net',
-      username: 'jira_user',
-      password: 'my-secret-password',
-      project_key: 'jira-key'
-    )
+    url 'https://mysite.atlassian.net'
+    username 'jira_user'
+    password 'my-secret-password'
   end
 
   factory :hipchat_service do
@@ -102,15 +116,21 @@ FactoryBot.define do
     token 'test_token'
   end
 
+  # this is for testing storing values inside properties, which is deprecated and will be removed in
+  # https://gitlab.com/gitlab-org/gitlab-ce/issues/63084
   trait :without_properties_callback do
+    jira_tracker_data nil
+    issue_tracker_data nil
+    create_data false
+
     after(:build) do |service|
-      allow(service).to receive(:handle_properties)
+      IssueTrackerService.skip_callback(:validation, :before, :handle_properties)
     end
 
-    after(:create) do |service|
-      # we have to remove the stub because the behaviour of
-      # handle_properties method is tested after the creation
-      allow(service).to receive(:handle_properties).and_call_original
+    to_create { |instance| instance.save(validate: false)}
+
+    after(:create) do
+      IssueTrackerService.set_callback(:validation, :before, :handle_properties)
     end
   end
 end
diff --git a/spec/fixtures/api/schemas/public_api/v4/release.json b/spec/fixtures/api/schemas/public_api/v4/release.json
index 078b1c0b982..662e61a9c06 100644
--- a/spec/fixtures/api/schemas/public_api/v4/release.json
+++ b/spec/fixtures/api/schemas/public_api/v4/release.json
@@ -15,7 +15,10 @@
     "author": {
       "oneOf": [{ "type": "null" }, { "$ref": "user/basic.json" }]
     },
-    "milestone": { "type": "string" },
+    "milestones": {
+      "type": "array",
+      "items": { "$ref": "milestone.json" }
+    },
     "assets": {
       "required": ["count", "links", "sources"],
       "properties": {
diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
index 7eb63fea413..47ea273ef3a 100644
--- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
+++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
@@ -34,7 +34,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+        expect(link['href']).to eq 'http://issues.example.com/issues/12'
       end
 
       it 'parses cross-project references to regular issues' do
@@ -63,7 +63,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+        expect(link['href']).to eq 'http://issues.example.com/issues/12'
       end
 
       it 'allows to use long external reference syntax for Redmine' do
@@ -72,7 +72,7 @@ describe Banzai::Pipeline::GfmPipeline do
         result = described_class.call(markdown, project: project)[:output]
         link = result.css('a').first
 
-        expect(link['href']).to eq 'http://issue-tracker.example.com/issues/12'
+        expect(link['href']).to eq 'http://issues.example.com/issues/12'
       end
 
       it 'parses cross-project references to regular issues' do
diff --git a/spec/lib/gitlab/discussions_diff/file_collection_spec.rb b/spec/lib/gitlab/discussions_diff/file_collection_spec.rb
index 0489206458b..6ef1e41450f 100644
--- a/spec/lib/gitlab/discussions_diff/file_collection_spec.rb
+++ b/spec/lib/gitlab/discussions_diff/file_collection_spec.rb
@@ -22,11 +22,13 @@ describe Gitlab::DiscussionsDiff::FileCollection do
                 note_diff_file_b.id => file_b_caching_content })
         .and_call_original
 
-      subject.load_highlight([note_diff_file_a.id, note_diff_file_b.id])
+      subject.load_highlight
     end
 
     it 'does not write cache for already cached file' do
-      subject.load_highlight([note_diff_file_a.id])
+      file_a_caching_content = diff_note_a.diff_file.highlighted_diff_lines.map(&:to_hash)
+      Gitlab::DiscussionsDiff::HighlightCache
+        .write_multiple({ note_diff_file_a.id => file_a_caching_content })
 
       file_b_caching_content = diff_note_b.diff_file.highlighted_diff_lines.map(&:to_hash)
 
@@ -35,27 +37,42 @@ describe Gitlab::DiscussionsDiff::FileCollection do
         .with({ note_diff_file_b.id => file_b_caching_content })
         .and_call_original
 
-      subject.load_highlight([note_diff_file_a.id, note_diff_file_b.id])
+      subject.load_highlight
     end
 
-    it 'does not err when given ID does not exist in @collection' do
-      expect { subject.load_highlight([999]) }.not_to raise_error
+    it 'does not write cache for resolved notes' do
+      diff_note_a.update_column(:resolved_at, Time.now)
+
+      file_b_caching_content = diff_note_b.diff_file.highlighted_diff_lines.map(&:to_hash)
+
+      expect(Gitlab::DiscussionsDiff::HighlightCache)
+        .to receive(:write_multiple)
+        .with({ note_diff_file_b.id => file_b_caching_content })
+        .and_call_original
+
+      subject.load_highlight
     end
 
     it 'loaded diff files have highlighted lines loaded' do
-      subject.load_highlight([note_diff_file_a.id])
+      subject.load_highlight
 
-      diff_file = subject.find_by_id(note_diff_file_a.id)
+      diff_file_a = subject.find_by_id(note_diff_file_a.id)
+      diff_file_b = subject.find_by_id(note_diff_file_b.id)
 
-      expect(diff_file.highlight_loaded?).to be(true)
+      expect(diff_file_a).to be_highlight_loaded
+      expect(diff_file_b).to be_highlight_loaded
     end
 
     it 'not loaded diff files does not have highlighted lines loaded' do
-      subject.load_highlight([note_diff_file_a.id])
+      diff_note_a.update_column(:resolved_at, Time.now)
+
+      subject.load_highlight
 
-      diff_file = subject.find_by_id(note_diff_file_b.id)
+      diff_file_a = subject.find_by_id(note_diff_file_a.id)
+      diff_file_b = subject.find_by_id(note_diff_file_b.id)
 
-      expect(diff_file.highlight_loaded?).to be(false)
+      expect(diff_file_a).not_to be_highlight_loaded
+      expect(diff_file_b).to be_highlight_loaded
     end
   end
 end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 6d573a4f39a..d3be1e86539 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -65,8 +65,8 @@ milestone:
 - participants
 - events
 - boards
-- milestone_release
-- release
+- milestone_releases
+- releases
 snippets:
 - author
 - project
@@ -77,8 +77,8 @@ releases:
 - author
 - project
 - links
-- milestone_release
-- milestone
+- milestone_releases
+- milestones
 links:
 - release
 project_members:
diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb
index 8eb64b97d6a..62787c5abaf 100644
--- a/spec/lib/gitlab/usage_data_spec.rb
+++ b/spec/lib/gitlab/usage_data_spec.rb
@@ -3,14 +3,16 @@
 require 'spec_helper'
 
 describe Gitlab::UsageData do
-  let(:projects) { create_list(:project, 3) }
+  let(:projects) { create_list(:project, 4) }
   let!(:board) { create(:board, project: projects[0]) }
 
   describe '#data' do
     before do
       create(:jira_service, project: projects[0])
-      create(:jira_service, project: projects[1])
+      create(:jira_service, :without_properties_callback, project: projects[1])
       create(:jira_service, :jira_cloud_service, project: projects[2])
+      create(:jira_service, :without_properties_callback, project: projects[3],
+             properties: { url: 'https://mysite.atlassian.net' })
       create(:prometheus_service, project: projects[1])
       create(:service, project: projects[0], type: 'SlackSlashCommandsService', active: true)
       create(:service, project: projects[1], type: 'SlackService', active: true)
@@ -156,7 +158,7 @@ describe Gitlab::UsageData do
       count_data = subject[:counts]
 
       expect(count_data[:boards]).to eq(1)
-      expect(count_data[:projects]).to eq(3)
+      expect(count_data[:projects]).to eq(4)
       expect(count_data.keys).to include(*expected_keys)
       expect(expected_keys - count_data.keys).to be_empty
     end
@@ -164,14 +166,14 @@ describe Gitlab::UsageData do
     it 'gathers projects data correctly' do
       count_data = subject[:counts]
 
-      expect(count_data[:projects]).to eq(3)
+      expect(count_data[:projects]).to eq(4)
       expect(count_data[:projects_prometheus_active]).to eq(1)
-      expect(count_data[:projects_jira_active]).to eq(3)
+      expect(count_data[:projects_jira_active]).to eq(4)
       expect(count_data[:projects_jira_server_active]).to eq(2)
-      expect(count_data[:projects_jira_cloud_active]).to eq(1)
+      expect(count_data[:projects_jira_cloud_active]).to eq(2)
       expect(count_data[:projects_slack_notifications_active]).to eq(2)
       expect(count_data[:projects_slack_slash_active]).to eq(1)
-      expect(count_data[:projects_with_repositories_enabled]).to eq(2)
+      expect(count_data[:projects_with_repositories_enabled]).to eq(3)
       expect(count_data[:projects_with_error_tracking_enabled]).to eq(1)
 
       expect(count_data[:clusters_enabled]).to eq(7)
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index fde1b096c76..65cc1a4bd6b 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -650,9 +650,35 @@ describe MergeRequest do
     end
   end
 
-  describe '#preload_discussions_diff_highlight' do
+  describe '#discussions_diffs' do
     let(:merge_request) { create(:merge_request) }
 
+    shared_examples 'discussions diffs collection' do
+      it 'initializes Gitlab::DiscussionsDiff::FileCollection with correct data' do
+        note_diff_file = diff_note.note_diff_file
+
+        expect(Gitlab::DiscussionsDiff::FileCollection)
+          .to receive(:new)
+          .with([note_diff_file])
+          .and_call_original
+
+        result = merge_request.discussions_diffs
+
+        expect(result).to be_a(Gitlab::DiscussionsDiff::FileCollection)
+      end
+
+      it 'eager loads relations' do
+        result = merge_request.discussions_diffs
+
+        recorder = ActiveRecord::QueryRecorder.new do
+          result.first.diff_note
+          result.first.diff_note.project
+        end
+
+        expect(recorder.count).to be_zero
+      end
+    end
+
     context 'with commit diff note' do
       let(:other_merge_request) { create(:merge_request) }
 
@@ -664,40 +690,15 @@ describe MergeRequest do
         create(:diff_note_on_commit, project: other_merge_request.project)
       end
 
-      it 'preloads diff highlighting' do
-        expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection|
-          note_diff_file = diff_note.note_diff_file
-
-          expect(collection)
-            .to receive(:load_highlight)
-            .with([note_diff_file.id]).and_call_original
-        end
-
-        merge_request.preload_discussions_diff_highlight
-      end
+      it_behaves_like 'discussions diffs collection'
     end
 
     context 'with merge request diff note' do
-      let!(:unresolved_diff_note) do
+      let!(:diff_note) do
         create(:diff_note_on_merge_request, project: merge_request.project, noteable: merge_request)
       end
 
-      let!(:resolved_diff_note) do
-        create(:diff_note_on_merge_request, :resolved, project: merge_request.project, noteable: merge_request)
-      end
-
-      it 'preloads diff highlighting' do
-        expect_next_instance_of(Gitlab::DiscussionsDiff::FileCollection) do |collection|
-          note_diff_file = unresolved_diff_note.note_diff_file
-
-          expect(collection)
-            .to receive(:load_highlight)
-            .with([note_diff_file.id])
-            .and_call_original
-        end
-
-        merge_request.preload_discussions_diff_highlight
-      end
+      it_behaves_like 'discussions diffs collection'
     end
   end
 
diff --git a/spec/models/milestone_release_spec.rb b/spec/models/milestone_release_spec.rb
index d6f73275977..28cec7bbc17 100644
--- a/spec/models/milestone_release_spec.rb
+++ b/spec/models/milestone_release_spec.rb
@@ -14,23 +14,29 @@ describe MilestoneRelease do
     it { is_expected.to belong_to(:release) }
   end
 
+  context 'when trying to create the same record in milestone_releases twice' do
+    it 'is not committing on the second time' do
+      create(:milestone_release, milestone: milestone, release: release)
+
+      expect do
+        subject.save!
+      end.to raise_error(ActiveRecord::RecordNotUnique)
+    end
+  end
+
   describe 'validations' do
-    it { is_expected.to validate_uniqueness_of(:milestone_id).scoped_to(:release_id) }
+    subject(:milestone_release) { build(:milestone_release, milestone: milestone, release: release) }
 
     context 'when milestone and release do not have the same project' do
       it 'is not valid' do
-        other_project = create(:project)
-        release = build(:release, project: other_project)
-        milestone_release = described_class.new(milestone: milestone, release: release)
+        milestone_release.release = build(:release, project: create(:project))
+
         expect(milestone_release).not_to be_valid
       end
     end
 
     context 'when milestone and release have the same project' do
-      it 'is valid' do
-        milestone_release = described_class.new(milestone: milestone, release: release)
-        expect(milestone_release).to be_valid
-      end
+      it { is_expected.to be_valid }
     end
   end
 end
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 64030f5b92a..0c4952eebd7 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -55,20 +55,20 @@ describe Milestone do
       end
     end
 
-    describe 'milestone_release' do
+    describe 'milestone_releases' do
       let(:milestone) { build(:milestone, project: project) }
 
       context 'when it is tied to a release for another project' do
         it 'creates a validation error' do
           other_project = create(:project)
-          milestone.release = build(:release, project: other_project)
+          milestone.releases << build(:release, project: other_project)
           expect(milestone).not_to be_valid
         end
       end
 
       context 'when it is tied to a release for the same project' do
         it 'is valid' do
-          milestone.release = build(:release, project: project)
+          milestone.releases << build(:release, project: project)
           expect(milestone).to be_valid
         end
       end
@@ -78,7 +78,8 @@ describe Milestone do
   describe "Associations" do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to have_many(:issues) }
-    it { is_expected.to have_one(:release) }
+    it { is_expected.to have_many(:releases) }
+    it { is_expected.to have_many(:milestone_releases) }
   end
 
   let(:project) { create(:project, :public) }
diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb
index 74c85a13c88..e25d87f61d6 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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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 5259357a254..8359bc6807a 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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
new file mode 100644
index 00000000000..146db0ae227
--- /dev/null
+++ b/spec/models/project_services/data_fields_spec.rb
@@ -0,0 +1,138 @@
+# 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 0c4fc290a13..4f3736ca65b 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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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 aaab654f874..db617cf0abb 100644
--- a/spec/models/project_services/issue_tracker_data_spec.rb
+++ b/spec/models/project_services/issue_tracker_data_spec.rb
@@ -8,28 +8,4 @@ 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 2fc4d69c2db..f1cdee5c4a3 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) }
+      let(:service) { RedmineService.new(project: project, active: true, issue_tracker_data: build(:issue_tracker_data)) }
 
       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 02060699e9a..a976745023b 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -6,10 +6,18 @@ 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.new(
-        project: build_stubbed(:project),
+      described_class.create(
+        project: create(:project),
         active: true,
         username: 'username',
         password: 'test',
@@ -32,78 +40,6 @@ 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
@@ -118,55 +54,260 @@ describe JiraService do
   describe '#create' do
     let(:params) do
       {
-        project: create(:project), title: 'custom title', description: 'custom description'
+        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
       }
     end
 
     subject { described_class.create(params) }
 
-    it 'does not store title & description into properties' do
-      expect(subject.properties.keys).not_to include('title', 'description')
+    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')
     end
 
-    it 'sets title & description correctly' do
+    it 'sets service data 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, username: 'username', password: 'password' }
+      { 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
     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) { access_params.merge(title: title, description: description) }
-      let(:service) do
+      let(:properties) { data_params.merge(title: title, description: description) }
+      let!(:service) do
         create(:jira_service, :without_properties_callback, properties: properties)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
+      it_behaves_like 'handles jira fields'
     end
 
     context 'when data are stored in separated fields' do
       let(:service) do
-        create(:jira_service, title: title, description: description, properties: access_params)
+        create(:jira_service, data_params.merge(properties: {}, title: title, description: description))
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
+      it_behaves_like 'handles jira fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
-      let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') }
+      let(:properties) { data_params.merge(title: title, description: description) }
       let(:service) do
-        create(:jira_service, :without_properties_callback, title: title, description: description, properties: properties)
+        create(:jira_service, :without_properties_callback, active: false, properties: properties).tap do |service|
+          create(:jira_tracker_data, data_params.merge(service: service))
+        end
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
+      it_behaves_like 'handles jira fields'
     end
 
     context 'when no title & description are set' do
@@ -410,111 +551,6 @@ 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' }
@@ -539,7 +575,7 @@ describe JiraService do
 
     context 'when it is set in properties' do
       it 'values from properties are returned' do
-        service = create(:jira_service, properties: properties)
+        service = create(:jira_service, :without_properties_callback, properties: properties)
 
         expect(service.title).to eq(title)
         expect(service.description).to eq(description)
@@ -602,8 +638,8 @@ describe JiraService do
         project = create(:project)
         service = project.create_jira_service(active: true)
 
-        expect(service.properties['url']).to eq('http://jira.sample/projects/project_a')
-        expect(service.properties['api_url']).to eq('http://jira.sample/api')
+        expect(service.url).to eq('http://jira.sample/projects/project_a')
+        expect(service.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 1b6ece8531b..6cd3eb33d9b 100644
--- a/spec/models/project_services/jira_tracker_data_spec.rb
+++ b/spec/models/project_services/jira_tracker_data_spec.rb
@@ -8,35 +8,4 @@ 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 c1ee6546b12..4ef4064d069 100644
--- a/spec/models/project_services/redmine_service_spec.rb
+++ b/spec/models/project_services/redmine_service_spec.rb
@@ -9,6 +9,15 @@ 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
@@ -17,6 +26,7 @@ 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
@@ -54,7 +64,7 @@ describe RedmineService do
         create(:redmine_service, :without_properties_callback, properties: properties)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -62,7 +72,7 @@ describe RedmineService do
         create(:redmine_service, title: title, description: description, properties: access_params)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -71,7 +81,7 @@ describe RedmineService do
         create(:redmine_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like '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 c48bf487af0..eff9f451b1a 100644
--- a/spec/models/project_services/youtrack_service_spec.rb
+++ b/spec/models/project_services/youtrack_service_spec.rb
@@ -16,6 +16,7 @@ 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
@@ -51,7 +52,7 @@ describe YoutrackService do
         create(:youtrack_service, :without_properties_callback, properties: properties)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
     end
 
     context 'when data are stored in separated fields' do
@@ -59,7 +60,7 @@ describe YoutrackService do
         create(:youtrack_service, title: title, description: description, properties: access_params)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
     end
 
     context 'when data are stored in both properties and separated fields' do
@@ -68,7 +69,7 @@ describe YoutrackService do
         create(:youtrack_service, :without_properties_callback, title: title, description: description, properties: properties)
       end
 
-      include_examples 'issue tracker fields'
+      it_behaves_like 'issue tracker fields'
     end
 
     context 'when no title & description are set' do
diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb
index c690390e24d..8714c67f29d 100644
--- a/spec/models/release_spec.rb
+++ b/spec/models/release_spec.rb
@@ -13,7 +13,8 @@ RSpec.describe Release do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to belong_to(:author).class_name('User') }
     it { is_expected.to have_many(:links).class_name('Releases::Link') }
-    it { is_expected.to have_one(:milestone) }
+    it { is_expected.to have_many(:milestones) }
+    it { is_expected.to have_many(:milestone_releases) }
   end
 
   describe 'validation' do
@@ -38,15 +39,15 @@ RSpec.describe Release do
 
     context 'when a release is tied to a milestone for another project' do
       it 'creates a validation error' do
-        release.milestone = build(:milestone, project: create(:project))
-        expect(release).not_to be_valid
+        milestone = build(:milestone, project: create(:project))
+        expect { release.milestones << milestone }.to raise_error
       end
     end
 
     context 'when a release is tied to a milestone linked to the same project' do
-      it 'is valid' do
-        release.milestone = build(:milestone, project: project)
-        expect(release).to be_valid
+      it 'successfully links this release to this milestone' do
+        milestone = build(:milestone, project: project)
+        expect { release.milestones << milestone }.to change { MilestoneRelease.count }.by(1)
       end
     end
   end
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 0797b9a9d83..d96e1398677 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -257,8 +257,8 @@ describe Service do
       expect(service.title).to eq('random title')
     end
 
-    it 'creates the properties' do
-      expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" })
+    it 'sets data correctly' do
+      expect(service.data_fields.project_url).to eq('http://gitlab.example.com')
     end
   end
 
diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb
index 76a70ab6e9e..7153fcc99d7 100644
--- a/spec/requests/api/services_spec.rb
+++ b/spec/requests/api/services_spec.rb
@@ -100,9 +100,15 @@ describe API::Services do
         expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
       end
 
-      it "returns empty hash if properties are empty" do
+      it "returns empty hash if properties and data fields are empty" do
         # deprecated services are not valid for update
         initialized_service.update_attribute(:properties, {})
+
+        if initialized_service.data_fields_present?
+          initialized_service.data_fields.destroy
+          initialized_service.reload
+        end
+
         get api("/projects/#{project.id}/services/#{dashed_service}", user)
 
         expect(response).to have_gitlab_http_status(200)
diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb
new file mode 100644
index 00000000000..5945561aa7b
--- /dev/null
+++ b/spec/requests/projects/merge_requests_discussions_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'merge requests discussions' do
+  # Further tests can be found at merge_requests_controller_spec.rb
+  describe 'GET /:namespace/:project/merge_requests/:iid/discussions' do
+    let(:project) { create(:project, :repository) }
+    let(:user) { project.owner }
+    let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
+
+    before do
+      project.add_developer(user)
+      login_as(user)
+    end
+
+    def send_request
+      get discussions_namespace_project_merge_request_path(namespace_id: project.namespace, project_id: project, id: merge_request.iid)
+    end
+
+    it 'returns 200' do
+      send_request
+
+      expect(response.status).to eq(200)
+    end
+
+    # https://docs.gitlab.com/ee/development/query_recorder.html#use-request-specs-instead-of-controller-specs
+    it 'avoids N+1 DB queries', :request_store do
+      control = ActiveRecord::QueryRecorder.new { send_request }
+
+      create(:diff_note_on_merge_request, noteable: merge_request,
+             project: merge_request.project)
+
+      expect do
+        send_request
+      end.not_to exceed_query_limit(control)
+    end
+
+    it 'limits Gitaly queries', :request_store do
+      Gitlab::GitalyClient.allow_n_plus_1_calls do
+        create_list(:diff_note_on_merge_request, 7, noteable: merge_request,
+                    project: merge_request.project)
+      end
+
+      # The creations above write into the Gitaly counts
+      Gitlab::GitalyClient.reset_counts
+
+      expect { send_request }
+        .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(4)
+    end
+  end
+end
diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb
index ff1e1256166..4f16421c39f 100644
--- a/spec/services/milestones/destroy_service_spec.rb
+++ b/spec/services/milestones/destroy_service_spec.rb
@@ -72,7 +72,7 @@ describe Milestones::DestroyService do
           :release,
           tag: 'v1.0',
           project: project,
-          milestone: milestone
+          milestones: [milestone]
         )
 
         expect { service.execute(milestone) }.not_to change { Release.count }
diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb
index 5c9d6537df1..b624b9475e3 100644
--- a/spec/services/releases/create_service_spec.rb
+++ b/spec/services/releases/create_service_spec.rb
@@ -75,10 +75,12 @@ describe Releases::CreateService do
 
     context 'when a passed-in milestone does not exist for this project' do
       it 'raises an error saying the milestone is inexistent' do
-        service = described_class.new(project, user, params.merge!({ milestone: 'v111.0' }))
+        inexistent_milestone_tag = 'v111.0'
+        service = described_class.new(project, user, params.merge!({ milestones: [inexistent_milestone_tag] }))
         result = service.execute
+
         expect(result[:status]).to eq(:error)
-        expect(result[:message]).to eq('Milestone does not exist')
+        expect(result[:message]).to eq("Milestone(s) not found: #{inexistent_milestone_tag}")
       end
     end
   end
@@ -93,10 +95,10 @@ describe Releases::CreateService do
     context 'when existing milestone is passed in' do
       let(:title) { 'v1.0' }
       let(:milestone) { create(:milestone, :active, project: project, title: title) }
-      let(:params_with_milestone) { params.merge!({ milestone: title }) }
+      let(:params_with_milestone) { params.merge!({ milestones: [title] }) }
+      let(:service) { described_class.new(milestone.project, user, params_with_milestone) }
 
       it 'creates a release and ties this milestone to it' do
-        service = described_class.new(milestone.project, user, params_with_milestone)
         result = service.execute
 
         expect(project.releases.count).to eq(1)
@@ -104,29 +106,66 @@ describe Releases::CreateService do
 
         release = project.releases.last
 
-        expect(release.milestone).to eq(milestone)
+        expect(release.milestones).to match_array([milestone])
       end
 
       context 'when another release was previously created with that same milestone linked' do
         it 'also creates another release tied to that same milestone' do
-          other_release = create(:release, milestone: milestone, project: project, tag: 'v1.0')
-          service = described_class.new(milestone.project, user, params_with_milestone)
+          other_release = create(:release, milestones: [milestone], project: project, tag: 'v1.0')
           service.execute
           release = project.releases.last
 
-          expect(release.milestone).to eq(milestone)
-          expect(other_release.milestone).to eq(milestone)
+          expect(release.milestones).to match_array([milestone])
+          expect(other_release.milestones).to match_array([milestone])
           expect(release.id).not_to eq(other_release.id)
         end
       end
     end
 
+    context 'when multiple existing milestone titles are passed in' do
+      let(:title_1) { 'v1.0' }
+      let(:title_2) { 'v1.0-rc' }
+      let!(:milestone_1) { create(:milestone, :active, project: project, title: title_1) }
+      let!(:milestone_2) { create(:milestone, :active, project: project, title: title_2) }
+      let!(:params_with_milestones) { params.merge!({ milestones: [title_1, title_2] }) }
+
+      it 'creates a release and ties it to these milestones' do
+        described_class.new(project, user, params_with_milestones).execute
+        release = project.releases.last
+
+        expect(release.milestones.map(&:title)).to include(title_1, title_2)
+      end
+    end
+
+    context 'when multiple miletone titles are passed in but one of them does not exist' do
+      let(:title) { 'v1.0' }
+      let(:inexistent_title) { 'v111.0' }
+      let!(:milestone) { create(:milestone, :active, project: project, title: title) }
+      let!(:params_with_milestones) { params.merge!({ milestones: [title, inexistent_title] }) }
+      let(:service) { described_class.new(milestone.project, user, params_with_milestones) }
+
+      it 'raises an error' do
+        result = service.execute
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq("Milestone(s) not found: #{inexistent_title}")
+      end
+
+      it 'does not create any release' do
+        expect do
+          service.execute
+        end.not_to change(Release, :count)
+      end
+    end
+
     context 'when no milestone is passed in' do
       it 'creates a release without a milestone tied to it' do
-        expect(params.key? :milestone).to be_falsey
+        expect(params.key? :milestones).to be_falsey
+
         service.execute
         release = project.releases.last
-        expect(release.milestone).to be_nil
+
+        expect(release.milestones).to be_empty
       end
 
       it 'does not create any new MilestoneRelease object' do
@@ -136,10 +175,11 @@ describe Releases::CreateService do
 
     context 'when an empty value is passed as a milestone' do
       it 'creates a release without a milestone tied to it' do
-        service = described_class.new(project, user, params.merge!({ milestone: '' }))
+        service = described_class.new(project, user, params.merge!({ milestones: [] }))
         service.execute
         release = project.releases.last
-        expect(release.milestone).to be_nil
+
+        expect(release.milestones).to be_empty
       end
     end
   end
diff --git a/spec/services/releases/destroy_service_spec.rb b/spec/services/releases/destroy_service_spec.rb
index c3172e5edbc..9d027767cd2 100644
--- a/spec/services/releases/destroy_service_spec.rb
+++ b/spec/services/releases/destroy_service_spec.rb
@@ -60,7 +60,7 @@ describe Releases::DestroyService do
 
     context 'when a milestone is tied to the release' do
       let!(:milestone) { create(:milestone, :active, project: project, title: 'v1.0') }
-      let!(:release) { create(:release, milestone: milestone, project: project, tag: tag) }
+      let!(:release) { create(:release, milestones: [milestone], project: project, tag: tag) }
 
       it 'destroys the release but leave the milestone intact' do
         expect { subject }.not_to change { Milestone.count }
diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb
index 944f3d8c9ad..178bac3574f 100644
--- a/spec/services/releases/update_service_spec.rb
+++ b/spec/services/releases/update_service_spec.rb
@@ -50,39 +50,60 @@ describe Releases::UpdateService do
     end
 
     context 'when a milestone is passed in' do
-      let(:old_title) { 'v1.0' }
       let(:new_title) { 'v2.0' }
-      let(:milestone) { create(:milestone, project: project, title: old_title) }
+      let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
       let(:new_milestone) { create(:milestone, project: project, title: new_title) }
-      let(:params_with_milestone) { params.merge!({ milestone: new_title }) }
+      let(:params_with_milestone) { params.merge!({ milestones: [new_title] }) }
+      let(:service) { described_class.new(new_milestone.project, user, params_with_milestone) }
 
       before do
-        release.milestone = milestone
-        release.save!
+        release.milestones << milestone
 
-        described_class.new(new_milestone.project, user, params_with_milestone).execute
+        service.execute
         release.reload
       end
 
       it 'updates the related milestone accordingly' do
-        expect(release.milestone.title).to eq(new_title)
+        expect(release.milestones.first.title).to eq(new_title)
       end
     end
 
     context "when an 'empty' milestone is passed in" do
       let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
-      let(:params_with_empty_milestone) { params.merge!({ milestone: '' }) }
+      let(:params_with_empty_milestone) { params.merge!({ milestones: [] }) }
 
       before do
-        release.milestone = milestone
-        release.save!
+        release.milestones << milestone
 
-        described_class.new(milestone.project, user, params_with_empty_milestone).execute
+        service.params = params_with_empty_milestone
+        service.execute
         release.reload
       end
 
       it 'removes the old milestone and does not associate any new milestone' do
-        expect(release.milestone).to be_nil
+        expect(release.milestones).not_to be_present
+      end
+    end
+
+    context "when multiple new milestones are passed in" do
+      let(:new_title_1) { 'v2.0' }
+      let(:new_title_2) { 'v2.0-rc' }
+      let(:milestone) { create(:milestone, project: project, title: 'v1.0') }
+      let(:params_with_milestones) { params.merge!({ milestones: [new_title_1, new_title_2] }) }
+      let(:service) { described_class.new(project, user, params_with_milestones) }
+
+      before do
+        create(:milestone, project: project, title: new_title_1)
+        create(:milestone, project: project, title: new_title_2)
+        release.milestones << milestone
+
+        service.execute
+        release.reload
+      end
+
+      it 'removes the old milestone and update the release with the new ones' do
+        milestone_titles = release.milestones.map(&:title)
+        expect(milestone_titles).to match_array([new_title_1, new_title_2])
       end
     end
   end
diff --git a/spec/support/helpers/jira_service_helper.rb b/spec/support/helpers/jira_service_helper.rb
index 57c33c81ea3..c23a8d52c84 100644
--- a/spec/support/helpers/jira_service_helper.rb
+++ b/spec/support/helpers/jira_service_helper.rb
@@ -5,16 +5,16 @@ module JiraServiceHelper
   JIRA_API = JIRA_URL + "/rest/api/2"
 
   def jira_service_settings
-    properties = {
-      title: "Jira tracker",
-      url: JIRA_URL,
-      username: 'jira-user',
-      password: 'my-secret-password',
-      project_key: "JIRA",
-      jira_issue_transition_id: '1'
-    }
+    title = "Jira tracker"
+    url = JIRA_URL
+    username = 'jira-user'
+    password = 'my-secret-password'
+    jira_issue_transition_id = '1'
 
-    jira_tracker.update(properties: properties, active: true)
+    jira_tracker.update(
+      title: title, url: url, username: username, password: password,
+      jira_issue_transition_id: jira_issue_transition_id, active: true
+    )
   end
 
   def jira_issue_comments