Add latest changes from gitlab-org/gitlab@master

20 files changed, 763 insertions, 132 deletions

diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index 64f90e44bb6..fb8da52930c 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -84,12 +84,12 @@ describe Projects::Environments::PrometheusApiController do
 
           before do
             expected_params[:query] = %{up{pod_name="#{pod_name}"}}
-            expected_params[:variables] = ['pod_name', pod_name]
+            expected_params[:variables] = { 'pod_name' => pod_name }
           end
 
           it 'replaces variables with values' do
             get :proxy, params: environment_params.merge(
-              query: 'up{pod_name="{{pod_name}}"}', variables: ['pod_name', pod_name]
+              query: 'up{pod_name="{{pod_name}}"}', variables: { 'pod_name' => pod_name }
             )
 
             expect(response).to have_gitlab_http_status(:success)
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 01a9647a763..3a6ddfb1783 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -445,4 +445,27 @@ describe RegistrationsController do
       end
     end
   end
+
+  describe '#experience_level' do
+    subject { get :experience_level }
+
+    let_it_be(:user) { create(:user) }
+
+    let(:part_of_onboarding_issues_experiment) { false }
+
+    before do
+      stub_experiment_for_user(onboarding_issues: part_of_onboarding_issues_experiment)
+      sign_in(user)
+    end
+
+    context 'when not part of the onboarding issues experiment' do
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+    end
+
+    context 'when part of the onboarding issues experiment' do
+      let(:part_of_onboarding_issues_experiment) { true }
+
+      it { is_expected.to render_template(:experience_level) }
+    end
+  end
 end
diff --git a/spec/frontend/ide/components/repo_commit_section_spec.js b/spec/frontend/ide/components/repo_commit_section_spec.js
index 237be018807..d4e4e064a52 100644
--- a/spec/frontend/ide/components/repo_commit_section_spec.js
+++ b/spec/frontend/ide/components/repo_commit_section_spec.js
@@ -2,6 +2,7 @@ import { mount } from '@vue/test-utils';
 import { createStore } from '~/ide/stores';
 import router from '~/ide/ide_router';
 import RepoCommitSection from '~/ide/components/repo_commit_section.vue';
+import EmptyState from '~/ide/components/commit_sidebar/empty_state.vue';
 import { stageKeys } from '~/ide/constants';
 import { file } from '../helpers';
 
@@ -63,7 +64,7 @@ describe('RepoCommitSection', () => {
     wrapper.destroy();
   });
 
-  describe('empty Stage', () => {
+  describe('empty state', () => {
     beforeEach(() => {
       store.state.noChangesStateSvgPath = TEST_NO_CHANGES_SVG;
       store.state.committedStateSvgPath = 'svg';
@@ -74,11 +75,16 @@ describe('RepoCommitSection', () => {
     it('renders no changes text', () => {
       expect(
         wrapper
-          .find('.js-empty-state')
+          .find(EmptyState)
           .text()
           .trim(),
       ).toContain('No changes');
-      expect(wrapper.find('.js-empty-state img').attributes('src')).toBe(TEST_NO_CHANGES_SVG);
+      expect(
+        wrapper
+          .find(EmptyState)
+          .find('img')
+          .attributes('src'),
+      ).toBe(TEST_NO_CHANGES_SVG);
     });
   });
 
@@ -109,6 +115,10 @@ describe('RepoCommitSection', () => {
 
       expect(changedFileNames).toEqual(allFiles.map(x => x.path));
     });
+
+    it('does not show empty state', () => {
+      expect(wrapper.find(EmptyState).exists()).toBe(false);
+    });
   });
 
   describe('with unstaged file', () => {
@@ -129,5 +139,9 @@ describe('RepoCommitSection', () => {
         keyPrefix: stageKeys.unstaged,
       });
     });
+
+    it('does not show empty state', () => {
+      expect(wrapper.find(EmptyState).exists()).toBe(false);
+    });
   });
 });
diff --git a/spec/frontend/ide/stores/actions/file_spec.js b/spec/frontend/ide/stores/actions/file_spec.js
index 43cb06f5d92..e50697af5eb 100644
--- a/spec/frontend/ide/stores/actions/file_spec.js
+++ b/spec/frontend/ide/stores/actions/file_spec.js
@@ -587,20 +587,6 @@ describe('IDE store file actions', () => {
         })
         .catch(done.fail);
     });
-
-    it('bursts unused seal', done => {
-      store
-        .dispatch('changeFileContent', {
-          path: tmpFile.path,
-          content: 'content',
-        })
-        .then(() => {
-          expect(store.state.unusedSeal).toBe(false);
-
-          done();
-        })
-        .catch(done.fail);
-    });
   });
 
   describe('with changed file', () => {
diff --git a/spec/frontend/ide/stores/actions_spec.js b/spec/frontend/ide/stores/actions_spec.js
index d52b0435906..666ed8a24aa 100644
--- a/spec/frontend/ide/stores/actions_spec.js
+++ b/spec/frontend/ide/stores/actions_spec.js
@@ -292,21 +292,6 @@ describe('Multi-file store actions', () => {
           })
           .catch(done.fail);
       });
-
-      it('bursts unused seal', done => {
-        store
-          .dispatch('createTempEntry', {
-            name: 'test',
-            branchId: 'mybranch',
-            type: 'blob',
-          })
-          .then(() => {
-            expect(store.state.unusedSeal).toBe(false);
-
-            done();
-          })
-          .catch(done.fail);
-      });
     });
   });
 
@@ -682,19 +667,6 @@ describe('Multi-file store actions', () => {
         });
       });
     });
-
-    it('bursts unused seal', done => {
-      store.state.entries.test = file('test');
-
-      store
-        .dispatch('deleteEntry', 'test')
-        .then(() => {
-          expect(store.state.unusedSeal).toBe(false);
-
-          done();
-        })
-        .catch(done.fail);
-    });
   });
 
   describe('renameEntry', () => {
@@ -839,20 +811,6 @@ describe('Multi-file store actions', () => {
           .then(done)
           .catch(done.fail);
       });
-
-      it('bursts unused seal', done => {
-        store
-          .dispatch('renameEntry', {
-            path: 'orig',
-            name: 'renamed',
-          })
-          .then(() => {
-            expect(store.state.unusedSeal).toBe(false);
-
-            done();
-          })
-          .catch(done.fail);
-      });
     });
 
     describe('folder', () => {
diff --git a/spec/frontend/ide/stores/mutations/file_spec.js b/spec/frontend/ide/stores/mutations/file_spec.js
index 9b96b910fcb..cd308ee9991 100644
--- a/spec/frontend/ide/stores/mutations/file_spec.js
+++ b/spec/frontend/ide/stores/mutations/file_spec.js
@@ -356,14 +356,6 @@ describe('IDE store file mutations', () => {
 
       expect(localState.changedFiles.length).toBe(1);
     });
-
-    it('bursts unused seal', () => {
-      expect(localState.unusedSeal).toBe(true);
-
-      mutations.ADD_FILE_TO_CHANGED(localState, localFile.path);
-
-      expect(localState.unusedSeal).toBe(false);
-    });
   });
 
   describe('REMOVE_FILE_FROM_CHANGED', () => {
@@ -374,14 +366,6 @@ describe('IDE store file mutations', () => {
 
       expect(localState.changedFiles.length).toBe(0);
     });
-
-    it('bursts unused seal', () => {
-      expect(localState.unusedSeal).toBe(true);
-
-      mutations.REMOVE_FILE_FROM_CHANGED(localState, localFile.path);
-
-      expect(localState.unusedSeal).toBe(false);
-    });
   });
 
   describe.each`
@@ -533,19 +517,6 @@ describe('IDE store file mutations', () => {
     },
   );
 
-  describe('STAGE_CHANGE', () => {
-    it('bursts unused seal', () => {
-      expect(localState.unusedSeal).toBe(true);
-
-      mutations.STAGE_CHANGE(localState, {
-        path: localFile.path,
-        diffInfo: localStore.getters.getDiffInfo(localFile.path),
-      });
-
-      expect(localState.unusedSeal).toBe(false);
-    });
-  });
-
   describe('TOGGLE_FILE_CHANGED', () => {
     it('updates file changed status', () => {
       mutations.TOGGLE_FILE_CHANGED(localState, {
diff --git a/spec/frontend/ide/stores/mutations_spec.js b/spec/frontend/ide/stores/mutations_spec.js
index 2eca9acb8d8..55cc6eb66ab 100644
--- a/spec/frontend/ide/stores/mutations_spec.js
+++ b/spec/frontend/ide/stores/mutations_spec.js
@@ -265,16 +265,6 @@ describe('Multi-file store mutations', () => {
       expect(localState.changedFiles).toEqual([]);
       expect(localState.stagedFiles).toEqual([]);
     });
-
-    it('bursts unused seal', () => {
-      localState.entries.test = file('test');
-
-      expect(localState.unusedSeal).toBe(true);
-
-      mutations.DELETE_ENTRY(localState, 'test');
-
-      expect(localState.unusedSeal).toBe(false);
-    });
   });
 
   describe('UPDATE_FILE_AFTER_COMMIT', () => {
diff --git a/spec/frontend/monitoring/store/getters_spec.js b/spec/frontend/monitoring/store/getters_spec.js
index 9f17dda3b9f..c7d0fb119de 100644
--- a/spec/frontend/monitoring/store/getters_spec.js
+++ b/spec/frontend/monitoring/store/getters_spec.js
@@ -329,7 +329,7 @@ describe('Monitoring store Getters', () => {
     });
   });
 
-  describe('getCustomVariablesArray', () => {
+  describe('getCustomVariablesParams', () => {
     let state;
 
     beforeEach(() => {
@@ -340,25 +340,21 @@ describe('Monitoring store Getters', () => {
 
     it('transforms the variables object to an array in the [variable, variable_value] format for all variable types', () => {
       mutations[types.SET_VARIABLES](state, mockTemplatingDataResponses.allVariableTypes);
-      const variablesArray = getters.getCustomVariablesArray(state);
-
-      expect(variablesArray).toEqual([
-        'simpleText',
-        'Simple text',
-        'advText',
-        'default',
-        'simpleCustom',
-        'value1',
-        'advCustomNormal',
-        'value2',
-      ]);
+      const variablesArray = getters.getCustomVariablesParams(state);
+
+      expect(variablesArray).toEqual({
+        'variables[advCustomNormal]': 'value2',
+        'variables[advText]': 'default',
+        'variables[simpleCustom]': 'value1',
+        'variables[simpleText]': 'Simple text',
+      });
     });
 
     it('transforms the variables object to an empty array when no keys are present', () => {
       mutations[types.SET_VARIABLES](state, {});
-      const variablesArray = getters.getCustomVariablesArray(state);
+      const variablesArray = getters.getCustomVariablesParams(state);
 
-      expect(variablesArray).toEqual([]);
+      expect(variablesArray).toEqual({});
     });
   });
 
diff --git a/spec/helpers/milestones_helper_spec.rb b/spec/helpers/timeboxes_helper_spec.rb
index 4ce7143bdf0..6fe738914ce 100644
--- a/spec/helpers/milestones_helper_spec.rb
+++ b/spec/helpers/timeboxes_helper_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe MilestonesHelper do
+describe TimeboxesHelper do
   describe '#milestones_filter_dropdown_path' do
     let(:project) { create(:project) }
     let(:project2) { create(:project) }
@@ -39,23 +39,34 @@ describe MilestonesHelper do
     end
   end
 
-  describe "#milestone_date_range" do
-    def result_for(*args)
-      milestone_date_range(build(:milestone, *args))
-    end
-
+  describe "#timebox_date_range" do
     let(:yesterday) { Date.yesterday }
     let(:tomorrow) { yesterday + 2 }
     let(:format) { '%b %-d, %Y' }
     let(:yesterday_formatted) { yesterday.strftime(format) }
     let(:tomorrow_formatted) { tomorrow.strftime(format) }
 
-    it { expect(result_for(due_date: nil, start_date: nil)).to be_nil }
-    it { expect(result_for(due_date: tomorrow)).to eq("expires on #{tomorrow_formatted}") }
-    it { expect(result_for(due_date: yesterday)).to eq("expired on #{yesterday_formatted}") }
-    it { expect(result_for(start_date: tomorrow)).to eq("starts on #{tomorrow_formatted}") }
-    it { expect(result_for(start_date: yesterday)).to eq("started on #{yesterday_formatted}") }
-    it { expect(result_for(start_date: yesterday, due_date: tomorrow)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}") }
+    context 'milestone' do
+      def result_for(*args)
+        timebox_date_range(build(:milestone, *args))
+      end
+
+      it { expect(result_for(due_date: nil, start_date: nil)).to be_nil }
+      it { expect(result_for(due_date: tomorrow)).to eq("expires on #{tomorrow_formatted}") }
+      it { expect(result_for(due_date: yesterday)).to eq("expired on #{yesterday_formatted}") }
+      it { expect(result_for(start_date: tomorrow)).to eq("starts on #{tomorrow_formatted}") }
+      it { expect(result_for(start_date: yesterday)).to eq("started on #{yesterday_formatted}") }
+      it { expect(result_for(start_date: yesterday, due_date: tomorrow)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}") }
+    end
+
+    context 'iteration' do
+      # Iterations always have start and due dates, so only A-B format is expected
+      it 'formats properly' do
+        iteration = build(:iteration, start_date: yesterday, due_date: tomorrow)
+
+        expect(timebox_date_range(iteration)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}")
+      end
+    end
   end
 
   describe '#milestone_counts' do
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 1a4f1123c73..8b99cc41a53 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -1726,4 +1726,59 @@ describe Notify do
       is_expected.to have_body_text target_url
     end
   end
+
+  describe 'merge request reviews' do
+    let!(:review) { create(:review, project: project, merge_request: merge_request) }
+    let!(:notes) { create_list(:note, 3, review: review, project: project, author: review.author, noteable: merge_request) }
+
+    subject { described_class.new_review_email(recipient.id, review.id) }
+
+    it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+      let(:model) { review.merge_request }
+    end
+
+    it_behaves_like 'it should show Gmail Actions View Merge request link'
+    it_behaves_like 'an unsubscribeable thread'
+
+    it 'is sent to the given recipient as the author' do
+      sender = subject.header[:from].addrs[0]
+
+      aggregate_failures do
+        expect(sender.display_name).to eq(review.author_name)
+        expect(sender.address).to eq(gitlab_sender)
+        expect(subject).to deliver_to(recipient.notification_email)
+      end
+    end
+
+    it 'contains the message from the notes of the review' do
+      review.notes.each do |note|
+        is_expected.to have_body_text note.note
+      end
+    end
+
+    context 'when diff note' do
+      let!(:notes) { create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request) }
+
+      it 'links to notes' do
+        review.notes.each do |note|
+          # Text part
+          expect(subject.text_part.body.raw_source).to include(
+            project_merge_request_url(project, merge_request, anchor: "note_#{note.id}")
+          )
+        end
+      end
+    end
+
+    it 'contains review author name' do
+      is_expected.to have_body_text review.author_name
+    end
+
+    it 'has the correct subject and body' do
+      aggregate_failures do
+        is_expected.to have_subject "Re: #{project.name} | #{merge_request.title} (#{merge_request.to_reference})"
+
+        is_expected.to have_body_text project_merge_request_path(project, merge_request)
+      end
+    end
+  end
 end
diff --git a/spec/migrations/backfill_status_page_published_incidents_spec.rb b/spec/migrations/backfill_status_page_published_incidents_spec.rb
new file mode 100644
index 00000000000..ccdc8be4168
--- /dev/null
+++ b/spec/migrations/backfill_status_page_published_incidents_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200421195234_backfill_status_page_published_incidents.rb')
+
+describe BackfillStatusPagePublishedIncidents, :migration do
+  subject(:migration) { described_class.new }
+
+  describe '#up' do
+    let(:projects) { table(:projects) }
+    let(:status_page_settings) { table(:status_page_settings) }
+    let(:issues) { table(:issues) }
+    let(:incidents) { table(:status_page_published_incidents) }
+
+    let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') }
+    let(:project_without_status_page) { projects.create!(namespace_id: namespace.id) }
+    let(:enabled_project) { projects.create!(namespace_id: namespace.id) }
+    let(:disabled_project) { projects.create!(namespace_id: namespace.id) }
+
+    let!(:enabled_setting) { status_page_settings.create!(enabled: true, project_id: enabled_project.id, **status_page_setting_attrs) }
+    let!(:disabled_setting) { status_page_settings.create!(enabled: false, project_id: disabled_project.id, **status_page_setting_attrs) }
+
+    let!(:published_issue) { issues.create!(confidential: false, project_id: enabled_project.id) }
+    let!(:nonpublished_issue_1) { issues.create!(confidential: true, project_id: enabled_project.id) }
+    let!(:nonpublished_issue_2) { issues.create!(confidential: false, project_id: disabled_project.id) }
+    let!(:nonpublished_issue_3) { issues.create!(confidential: false, project_id: project_without_status_page.id) }
+
+    let(:current_time) { Time.current.change(usec: 0) }
+    let(:status_page_setting_attrs) do
+      {
+        aws_s3_bucket_name: 'bucket',
+        aws_region: 'region',
+        aws_access_key: 'key',
+        encrypted_aws_secret_key: 'abc123',
+        encrypted_aws_secret_key_iv: 'abc123'
+      }
+    end
+
+    it 'creates a StatusPage::PublishedIncident record for each published issue' do
+      Timecop.freeze(current_time) do
+        expect(incidents.all).to be_empty
+
+        migrate!
+
+        incident = incidents.first
+
+        expect(incidents.count).to eq(1)
+        expect(incident.issue_id).to eq(published_issue.id)
+        expect(incident.created_at).to eq(current_time)
+        expect(incident.updated_at).to eq(current_time)
+      end
+    end
+  end
+end
diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb
new file mode 100644
index 00000000000..ca0a257be7a
--- /dev/null
+++ b/spec/models/concerns/limitable_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Limitable do
+  let(:minimal_test_class) do
+    Class.new do
+      include ActiveModel::Model
+
+      def self.name
+        'TestClass'
+      end
+
+      include Limitable
+    end
+  end
+
+  before do
+    stub_const("MinimalTestClass", minimal_test_class)
+  end
+
+  it { expect(MinimalTestClass.limit_name).to eq('test_classes') }
+
+  context 'with scoped limit' do
+    before do
+      MinimalTestClass.limit_scope = :project
+    end
+
+    it { expect(MinimalTestClass.limit_scope).to eq(:project) }
+
+    it 'triggers scoped validations' do
+      instance = MinimalTestClass.new
+
+      expect(instance).to receive(:validate_scoped_plan_limit_not_exceeded)
+
+      instance.valid?(:create)
+    end
+  end
+
+  context 'with global limit' do
+    before do
+      MinimalTestClass.limit_scope = Limitable::GLOBAL_SCOPE
+    end
+
+    it { expect(MinimalTestClass.limit_scope).to eq(Limitable::GLOBAL_SCOPE) }
+
+    it 'triggers scoped validations' do
+      instance = MinimalTestClass.new
+
+      expect(instance).to receive(:validate_global_plan_limit_not_exceeded)
+
+      instance.valid?(:create)
+    end
+  end
+end
diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb
new file mode 100644
index 00000000000..8f244ed386b
--- /dev/null
+++ b/spec/services/draft_notes/create_service_spec.rb
@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::CreateService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def create_draft(params)
+    described_class.new(merge_request, user, params).execute
+  end
+
+  it 'creates a simple draft note' do
+    draft = create_draft(note: 'This is a test')
+
+    expect(draft).to be_an_instance_of(DraftNote)
+    expect(draft.note).to eq('This is a test')
+    expect(draft.author).to eq(user)
+    expect(draft.project).to eq(merge_request.target_project)
+    expect(draft.discussion_id).to be_nil
+  end
+
+  it 'cannot resolve when there is nothing to resolve' do
+    draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
+
+    expect(draft.errors[:base]).to include('User is not allowed to resolve thread')
+    expect(draft).not_to be_persisted
+  end
+
+  context 'in a thread' do
+    it 'creates a draft note with discussion_id' do
+      discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion
+
+      draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id)
+
+      expect(draft.note).to eq('A reply!')
+      expect(draft.discussion_id).to eq(discussion.reply_id)
+      expect(draft.resolve_discussion).to be_falsey
+    end
+
+    it 'creates a draft that resolves the thread' do
+      discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion
+
+      draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id, resolve_discussion: true)
+
+      expect(draft.note).to eq('A reply!')
+      expect(draft.discussion_id).to eq(discussion.reply_id)
+      expect(draft.resolve_discussion).to be true
+    end
+  end
+
+  it 'creates a draft note with a position in a diff' do
+    diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs)
+
+    position = Gitlab::Diff::Position.new(
+      old_path: "files/ruby/popen.rb",
+      new_path: "files/ruby/popen.rb",
+      old_line: nil,
+      new_line: 14,
+      diff_refs: diff_refs
+    )
+
+    draft = create_draft(note: 'Comment on diff', position: position.to_json)
+
+    expect(draft.note).to eq('Comment on diff')
+    expect(draft.original_position.to_json).to eq(position.to_json)
+  end
+
+  context 'diff highlight cache clearing' do
+    context 'when diff file is unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { true }
+        end
+
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+
+    context 'when diff file is not unfolded and it is not a reply' do
+      it 'clears diff highlighting cache' do
+        expect_next_instance_of(DraftNote) do |draft|
+          allow(draft).to receive_message_chain(:diff_file, :unfolded?) { false }
+        end
+
+        expect(merge_request).not_to receive(:diffs)
+
+        create_draft(note: 'This is a test')
+      end
+    end
+  end
+end
diff --git a/spec/services/draft_notes/destroy_service_spec.rb b/spec/services/draft_notes/destroy_service_spec.rb
new file mode 100644
index 00000000000..d0bf88dcdbe
--- /dev/null
+++ b/spec/services/draft_notes/destroy_service_spec.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::DestroyService do
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+
+  def destroy(draft_note = nil)
+    DraftNotes::DestroyService.new(merge_request, user).execute(draft_note)
+  end
+
+  it 'destroys a single draft note' do
+    drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy(drafts.first) }
+      .to change { DraftNote.count }.by(-1)
+
+    expect(DraftNote.count).to eq(1)
+  end
+
+  it 'destroys all draft notes for a user in a merge request' do
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+    expect { destroy }.to change { DraftNote.count }.by(-2)
+    expect(DraftNote.count).to eq(0)
+  end
+
+  context 'diff highlight cache clearing' do
+    context 'when destroying all draft notes of a user' do
+      it 'clears highlighting cache if unfold required for any' do
+        drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy(drafts.first)
+      end
+    end
+
+    context 'when destroying one draft note' do
+      it 'clears highlighting cache if unfold required' do
+        create_list(:draft_note, 2, merge_request: merge_request, author: user)
+
+        allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true }
+        expect(merge_request).to receive_message_chain(:diffs, :clear_cache)
+
+        destroy
+      end
+    end
+  end
+end
diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
new file mode 100644
index 00000000000..4ebae2f9aa2
--- /dev/null
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -0,0 +1,261 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe DraftNotes::PublishService do
+  include RepoHelpers
+
+  let(:merge_request) { create(:merge_request) }
+  let(:project) { merge_request.target_project }
+  let(:user) { merge_request.author }
+  let(:commit) { project.commit(sample_commit.id) }
+
+  let(:position) do
+    Gitlab::Diff::Position.new(
+      old_path: "files/ruby/popen.rb",
+      new_path: "files/ruby/popen.rb",
+      old_line: nil,
+      new_line: 14,
+      diff_refs: commit.diff_refs
+    )
+  end
+
+  def publish(draft: nil)
+    DraftNotes::PublishService.new(merge_request, user).execute(draft)
+  end
+
+  context 'single draft note' do
+    let(:commit_id) { nil }
+    let!(:drafts) { create_list(:draft_note, 2, merge_request: merge_request, author: user, commit_id: commit_id, position: position) }
+
+    it 'publishes' do
+      expect { publish(draft: drafts.first) }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
+      expect(DraftNote.count).to eq(1)
+    end
+
+    it 'does not skip notification', :sidekiq_might_not_need_inline do
+      expect(Notes::CreateService).to receive(:new).with(project, user, drafts.first.publish_params).and_call_original
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive(:new_note)
+      end
+
+      result = publish(draft: drafts.first)
+
+      expect(result[:status]).to eq(:success)
+    end
+
+    context 'commit_id is set' do
+      let(:commit_id) { commit.id }
+
+      it 'creates note from draft with commit_id' do
+        result = publish(draft: drafts.first)
+
+        expect(result[:status]).to eq(:success)
+        expect(merge_request.notes.first.commit_id).to eq(commit_id)
+      end
+    end
+  end
+
+  context 'multiple draft notes' do
+    let(:commit_id) { nil }
+
+    before do
+      create(:draft_note, merge_request: merge_request, author: user, note: 'first note', commit_id: commit_id, position: position)
+      create(:draft_note, merge_request: merge_request, author: user, note: 'second note', commit_id: commit_id, position: position)
+    end
+
+    context 'when review fails to create' do
+      before do
+        expect_next_instance_of(Review) do |review|
+          allow(review).to receive(:save!).and_raise(ActiveRecord::RecordInvalid.new(review))
+        end
+      end
+
+      it 'does not publish any draft note' do
+        expect { publish }.not_to change { DraftNote.count }
+      end
+
+      it 'returns an error' do
+        result = publish
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to match(/Unable to save Review/)
+      end
+    end
+
+    it 'returns success' do
+      result = publish
+
+      expect(result[:status]).to eq(:success)
+    end
+
+    it 'publishes all draft notes for a user in a merge request' do
+      expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2).and change { Review.count }.by(1)
+      expect(DraftNote.count).to eq(0)
+
+      notes = merge_request.notes.order(id: :asc)
+      expect(notes.first.note).to eq('first note')
+      expect(notes.last.note).to eq('second note')
+    end
+
+    it 'sends batch notification' do
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive_message_chain(:async, :new_review).with(kind_of(Review))
+      end
+
+      publish
+    end
+
+    context 'commit_id is set' do
+      let(:commit_id) { commit.id }
+
+      it 'creates note from draft with commit_id' do
+        result = publish
+
+        expect(result[:status]).to eq(:success)
+
+        merge_request.notes.each do |note|
+          expect(note.commit_id).to eq(commit_id)
+        end
+      end
+    end
+  end
+
+  context 'draft notes with suggestions' do
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
+    let(:suggestion_note) do
+      <<-MARKDOWN.strip_heredoc
+        ```suggestion
+          foo
+        ```
+      MARKDOWN
+    end
+
+    let!(:draft) { create(:draft_note_on_text_diff, note: suggestion_note, merge_request: merge_request, author: user) }
+
+    it 'creates a suggestion with correct content' do
+      expect { publish(draft: draft) }.to change { Suggestion.count }.by(1)
+        .and change { DiffNote.count }.from(0).to(1)
+
+      suggestion = Suggestion.last
+
+      expect(suggestion.from_line).to eq(14)
+      expect(suggestion.to_line).to eq(14)
+      expect(suggestion.from_content).to eq("    vars = {\n")
+      expect(suggestion.to_content).to eq("  foo\n")
+    end
+
+    context 'when the diff is changed' do
+      let(:file_path) { 'files/ruby/popen.rb' }
+      let(:branch_name) { project.default_branch }
+      let(:commit) { project.repository.commit }
+
+      def update_file(file_path, new_content)
+        params = {
+          file_path: file_path,
+          commit_message: "Update File",
+          file_content: new_content,
+          start_project: project,
+          start_branch: project.default_branch,
+          branch_name: branch_name
+        }
+
+        Files::UpdateService.new(project, user, params).execute
+      end
+
+      before do
+        project.add_developer(user)
+      end
+
+      it 'creates a suggestion based on the latest diff content and positions' do
+        diff_file = merge_request.diffs(paths: [file_path]).diff_files.first
+        raw_data = diff_file.new_blob.data
+
+        # Add a line break to the beginning of the file
+        result = update_file(file_path, raw_data.prepend("\n"))
+        oldrev = merge_request.diff_head_sha
+        newrev = result[:result]
+
+        expect(newrev).to be_present
+
+        # Generates new MR revision at DB level
+        refresh = MergeRequests::RefreshService.new(project, user)
+        refresh.execute(oldrev, newrev, merge_request.source_branch_ref)
+
+        expect { publish(draft: draft) }.to change { Suggestion.count }.by(1)
+          .and change { DiffNote.count }.from(0).to(1)
+
+        suggestion = Suggestion.last
+
+        expect(suggestion.from_line).to eq(15)
+        expect(suggestion.to_line).to eq(15)
+        expect(suggestion.from_content).to eq("    vars = {\n")
+        expect(suggestion.to_content).to eq("  foo\n")
+      end
+    end
+  end
+
+  it 'only publishes the draft notes belonging to the current user' do
+    other_user = create(:user)
+    project.add_maintainer(other_user)
+
+    create_list(:draft_note, 2, merge_request: merge_request, author: user)
+    create_list(:draft_note, 2, merge_request: merge_request, author: other_user)
+
+    expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2)
+    expect(DraftNote.count).to eq(2)
+  end
+
+  context 'with quick actions' do
+    it 'performs quick actions' do
+      other_user = create(:user)
+      project.add_developer(other_user)
+
+      create(:draft_note, merge_request: merge_request,
+                          author: user,
+                          note: "thanks\n/assign #{other_user.to_reference}")
+
+      expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(2)
+      expect(merge_request.reload.assignees).to match_array([other_user])
+      expect(merge_request.notes.last).to be_system
+    end
+
+    it 'does not create a note if it only contains quick actions' do
+      create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}")
+
+      expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
+      expect(merge_request.reload.assignees).to eq([user])
+      expect(merge_request.notes.last).to be_system
+    end
+  end
+
+  context 'with drafts that resolve threads' do
+    let!(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
+    let!(:draft_note) { create(:draft_note, merge_request: merge_request, author: user, resolve_discussion: true, discussion_id: note.discussion.reply_id) }
+
+    it 'resolves the thread' do
+      publish(draft: draft_note)
+
+      expect(note.discussion.resolved?).to be true
+    end
+
+    it 'sends notifications if all threads are resolved' do
+      expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance|
+        expect(instance).to receive(:execute).with(merge_request)
+      end
+
+      publish
+    end
+  end
+
+  context 'user cannot create notes' do
+    before do
+      allow(Ability).to receive(:allowed?).with(user, :create_note, merge_request).and_return(false)
+    end
+
+    it 'returns an error' do
+      expect(publish[:status]).to eq(:error)
+    end
+  end
+end
diff --git a/spec/services/notification_recipients/build_service_spec.rb b/spec/services/notification_recipients/build_service_spec.rb
index 2e848c2f04d..e203093623d 100644
--- a/spec/services/notification_recipients/build_service_spec.rb
+++ b/spec/services/notification_recipients/build_service_spec.rb
@@ -58,4 +58,56 @@ describe NotificationRecipients::BuildService do
       end
     end
   end
+
+  describe '#build_new_review_recipients' do
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+    let(:review) { create(:review, merge_request: merge_request, project: project, author: merge_request.author) }
+    let(:notes) { create_list(:note_on_merge_request, 3, review: review, noteable: review.merge_request, project: review.project) }
+
+    shared_examples 'no N+1 queries' do
+      it 'avoids N+1 queries', :request_store do
+        create_user
+
+        service.build_new_review_recipients(review)
+
+        control_count = ActiveRecord::QueryRecorder.new do
+          service.build_new_review_recipients(review)
+        end
+
+        create_user
+
+        expect { service.build_new_review_recipients(review) }.not_to exceed_query_limit(control_count)
+      end
+    end
+
+    context 'when there are multiple watchers' do
+      def create_user
+        watcher = create(:user)
+        create(:notification_setting, source: project, user: watcher, level: :watch)
+
+        other_projects.each do |other_project|
+          create(:notification_setting, source: other_project, user: watcher, level: :watch)
+        end
+      end
+
+      include_examples 'no N+1 queries'
+    end
+
+    context 'when there are multiple subscribers' do
+      def create_user
+        subscriber = create(:user)
+        merge_request.subscriptions.create(user: subscriber, project: project, subscribed: true)
+      end
+
+      include_examples 'no N+1 queries'
+
+      context 'when the project is private' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+        end
+
+        include_examples 'no N+1 queries'
+      end
+    end
+  end
 end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 2a7166e3895..d3376ef0a04 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2863,6 +2863,57 @@ describe NotificationService, :mailer do
     end
   end
 
+  describe '#new_review' do
+    let(:project) { create(:project, :repository) }
+    let(:user) { create(:user) }
+    let(:user2) { create(:user) }
+    let(:reviewer) { create(:user) }
+    let(:merge_request) { create(:merge_request, source_project: project, assignees: [user, user2], author: create(:user)) }
+    let(:review) { create(:review, merge_request: merge_request, project: project, author: reviewer) }
+    let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: reviewer, review: review) }
+
+    before do
+      build_team(review.project)
+      add_users(review.project)
+      add_user_subscriptions(merge_request)
+      project.add_maintainer(merge_request.author)
+      project.add_maintainer(reviewer)
+      merge_request.assignees.each { |assignee| project.add_maintainer(assignee) }
+
+      create(:diff_note_on_merge_request,
+             project: project,
+             noteable: merge_request,
+             author: reviewer,
+             review: review,
+             note: "cc @mention")
+    end
+
+    it 'sends emails' do
+      expect(Notify).not_to receive(:new_review_email).with(review.author.id, review.id)
+      expect(Notify).not_to receive(:new_review_email).with(@unsubscriber.id, review.id)
+      merge_request.assignee_ids.each do |assignee_id|
+        expect(Notify).to receive(:new_review_email).with(assignee_id, review.id).and_call_original
+      end
+      expect(Notify).to receive(:new_review_email).with(merge_request.author.id, review.id).and_call_original
+      expect(Notify).to receive(:new_review_email).with(@u_watcher.id, review.id).and_call_original
+      expect(Notify).to receive(:new_review_email).with(@u_mentioned.id, review.id).and_call_original
+      expect(Notify).to receive(:new_review_email).with(@subscriber.id, review.id).and_call_original
+      expect(Notify).to receive(:new_review_email).with(@watcher_and_subscriber.id, review.id).and_call_original
+      expect(Notify).to receive(:new_review_email).with(@subscribed_participant.id, review.id).and_call_original
+
+      subject.new_review(review)
+    end
+
+    it_behaves_like 'project emails are disabled' do
+      let(:notification_target)  { review }
+      let(:notification_trigger) { subject.new_review(review) }
+
+      around do |example|
+        perform_enqueued_jobs { example.run }
+      end
+    end
+  end
+
   def build_team(project)
     @u_watcher               = create_global_setting_for(create(:user), :watch)
     @u_participating         = create_global_setting_for(create(:user), :participating)
diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
index 82ea356d599..5982dcbc404 100644
--- a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
+++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
@@ -64,7 +64,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
       let(:params_keys) do
         {
           query: 'up{pod_name="{{pod_name}}"}',
-          variables: ['pod_name', pod_name]
+          variables: { 'pod_name' => pod_name }
         }
       end
 
@@ -76,7 +76,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
         let(:params_keys) do
           {
             query: 'up{pod_name="{{pod_name}}",env="{{ci_environment_slug}}"}',
-            variables: ['pod_name', pod_name, 'ci_environment_slug', 'custom_value']
+            variables: { 'pod_name' => pod_name, 'ci_environment_slug' => 'custom_value' }
           }
         end
 
@@ -95,8 +95,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
           }
         end
 
-        it_behaves_like 'error', 'Optional parameter "variables" must be an ' \
-          'array of keys and values. Ex: [key1, value1, key2, value2]'
+        it_behaves_like 'error', 'Optional parameter "variables" must be a Hash. Ex: variables[key1]=value1'
       end
 
       context 'with nil variables' do
diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb
index a7d7c16a66f..c2a793b2368 100644
--- a/spec/services/users/migrate_to_ghost_user_service_spec.rb
+++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb
@@ -84,6 +84,15 @@ describe Users::MigrateToGhostUserService do
       end
     end
 
+    context 'reviews' do
+      let!(:user)   { create(:user) }
+      let(:service) { described_class.new(user) }
+
+      include_examples "migrating a deleted user's associated records to the ghost user", Review, [:author] do
+        let(:created_record) { create(:review, author: user) }
+      end
+    end
+
     context "when record migration fails with a rollback exception" do
       before do
         expect_any_instance_of(ActiveRecord::Associations::CollectionProxy)
diff --git a/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb
index 4bcea36fd42..d21823661f8 100644
--- a/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb
+++ b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb
@@ -26,7 +26,7 @@ RSpec.shared_examples 'includes Limitable concern' do
           subject.dup.save
         end
 
-        it 'cannot create new models exceding the plan limits' do
+        it 'cannot create new models exceeding the plan limits' do
           expect { subject.save }.not_to change { described_class.count }
           expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded")
         end