Add latest changes from gitlab-org/gitlab@13-8-stable-eev13.8.0-rc42

11 files changed, 122 insertions, 68 deletions

diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb
index 69bab3b1ea4..9ae310d8cee 100644
--- a/spec/services/merge_requests/after_create_service_spec.rb
+++ b/spec/services/merge_requests/after_create_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe MergeRequests::AfterCreateService do
+  include AfterNextHelpers
+
   let_it_be(:merge_request) { create(:merge_request) }
 
   subject(:after_create_service) do
@@ -27,6 +29,14 @@ RSpec.describe MergeRequests::AfterCreateService do
       execute_service
     end
 
+    it 'calls the merge request activity counter' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_create_mr_action)
+        .with(user: merge_request.author)
+
+      execute_service
+    end
+
     it 'creates a new merge request notification' do
       expect(notification_service)
         .to receive(:new_merge_request).with(merge_request, merge_request.author)
@@ -56,11 +66,15 @@ RSpec.describe MergeRequests::AfterCreateService do
       execute_service
     end
 
-    it 'records a namespace onboarding progress action' do
-      expect(NamespaceOnboardingAction).to receive(:create_action)
-        .with(merge_request.target_project.namespace, :merge_request_created).and_call_original
+    it 'registers an onboarding progress action' do
+      OnboardingProgress.onboard(merge_request.target_project.namespace)
+
+      expect_next(OnboardingProgressService, merge_request.target_project.namespace)
+        .to receive(:execute).with(action: :merge_request_created).and_call_original
+
+      execute_service
 
-      expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1)
+      expect(OnboardingProgress.completed?(merge_request.target_project.namespace, :merge_request_created)).to be(true)
     end
   end
 end
diff --git a/spec/services/merge_requests/cleanup_refs_service_spec.rb b/spec/services/merge_requests/cleanup_refs_service_spec.rb
index 38c0e204e54..a1822a4d5ba 100644
--- a/spec/services/merge_requests/cleanup_refs_service_spec.rb
+++ b/spec/services/merge_requests/cleanup_refs_service_spec.rb
@@ -91,6 +91,26 @@ RSpec.describe MergeRequests::CleanupRefsService do
         it_behaves_like 'service that does not clean up merge request refs'
       end
 
+      context 'when a git error is raised' do
+        context 'Gitlab::Git::Repository::GitError' do
+          before do
+            allow(merge_request.project.repository).to receive(:delete_refs).and_raise(Gitlab::Git::Repository::GitError)
+          end
+
+          it_behaves_like 'service that does not clean up merge request refs'
+        end
+
+        context 'Gitlab::Git::CommandError' do
+          before do
+            allow_next_instance_of(Gitlab::Git::KeepAround) do |keep_around|
+              expect(keep_around).to receive(:kept_around?).and_raise(Gitlab::Git::CommandError)
+            end
+          end
+
+          it_behaves_like 'service that does not clean up merge request refs'
+        end
+      end
+
       context 'when cleanup schedule fails to update' do
         before do
           allow(merge_request.cleanup_schedule).to receive(:update).and_return(false)
diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb
index 67fb4eaade5..48f56b3ec68 100644
--- a/spec/services/merge_requests/close_service_spec.rb
+++ b/spec/services/merge_requests/close_service_spec.rb
@@ -18,6 +18,7 @@ RSpec.describe MergeRequests::CloseService do
 
   describe '#execute' do
     it_behaves_like 'cache counters invalidator'
+    it_behaves_like 'merge request reviewers cache counters invalidator'
 
     context 'valid params' do
       let(:service) { described_class.new(project, user, {}) }
@@ -75,6 +76,14 @@ RSpec.describe MergeRequests::CloseService do
       described_class.new(project, user, {}).execute(merge_request)
     end
 
+    it 'calls the merge request activity counter' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_close_mr_action)
+        .with(user: user)
+
+      described_class.new(project, user, {}).execute(merge_request)
+    end
+
     it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do
       service = described_class.new(project, user, {})
 
diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb
index 86e49fe601c..be8c41bc4a1 100644
--- a/spec/services/merge_requests/create_from_issue_service_spec.rb
+++ b/spec/services/merge_requests/create_from_issue_service_spec.rb
@@ -66,10 +66,11 @@ RSpec.describe MergeRequests::CreateFromIssueService do
         expect { service.execute }.to change(target_project.merge_requests, :count).by(1)
       end
 
-      it 'sets the merge request author to current user', :sidekiq_might_not_need_inline do
+      it 'sets the merge request author to current user and assigns them', :sidekiq_might_not_need_inline do
         result = service.execute
 
         expect(result[:merge_request].author).to eq(user)
+        expect(result[:merge_request].assignees).to eq([user])
       end
 
       it 'sets the merge request source branch to the new issue branch', :sidekiq_might_not_need_inline do
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index d042b318d02..4f47a22b07c 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -155,6 +155,12 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
 
           expect(Todo.where(attributes).count).to eq 1
         end
+
+        it 'invalidates counter cache for reviewers', :use_clean_rails_memory_store_caching do
+          expect { merge_request }
+            .to change { user2.review_requested_open_merge_requests_count }
+            .by(1)
+        end
       end
 
       context 'when head pipelines already exist for merge request source branch', :sidekiq_inline do
diff --git a/spec/services/merge_requests/export_csv_service_spec.rb b/spec/services/merge_requests/export_csv_service_spec.rb
index ecb17b3fe77..4ce032c396e 100644
--- a/spec/services/merge_requests/export_csv_service_spec.rb
+++ b/spec/services/merge_requests/export_csv_service_spec.rb
@@ -27,11 +27,11 @@ RSpec.describe MergeRequests::ExportCsvService do
         let_it_be(:merge_request) { create(:merge_request, assignees: create_list(:user, 2)) }
 
         it 'contains the names of assignees' do
-          expect(csv['Assignees']).to eq(merge_request.assignees.map(&:name).join(', '))
+          expect(csv['Assignees'].split(', ')).to match_array(merge_request.assignees.map(&:name))
         end
 
         it 'contains the usernames of assignees' do
-          expect(csv['Assignee Usernames']).to eq(merge_request.assignees.map(&:username).join(', '))
+          expect(csv['Assignee Usernames'].split(', ')).to match_array(merge_request.assignees.map(&:username))
         end
       end
 
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index d0e3102f157..dd37d87e3f5 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -37,6 +37,10 @@ RSpec.describe MergeRequests::MergeService do
         expect(merge_request.in_progress_merge_commit_sha).to be_nil
       end
 
+      it 'does not update squash_commit_sha if it is not a squash' do
+        expect(merge_request.squash_commit_sha).to be_nil
+      end
+
       it 'sends email to user2 about merge of new merge_request' do
         email = ActionMailer::Base.deliveries.last
         expect(email.to.first).to eq(user2.email)
@@ -76,6 +80,12 @@ RSpec.describe MergeRequests::MergeService do
           expect(merge_commit.message).to eq('Merge commit message')
           expect(squash_commit.message).to eq("Squash commit message\n")
         end
+
+        it 'persists squash_commit_sha' do
+          squash_commit = merge_request.merge_commit.parents.last
+
+          expect(merge_request.squash_commit_sha).to eq(squash_commit.id)
+        end
       end
     end
 
@@ -361,6 +371,7 @@ RSpec.describe MergeRequests::MergeService do
           expect(merge_request).to be_open
 
           expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.squash_commit_sha).to be_nil
           expect(merge_request.merge_error).to include(error_message)
           expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
         end
@@ -381,6 +392,7 @@ RSpec.describe MergeRequests::MergeService do
 
           expect(merge_request).to be_open
           expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.squash_commit_sha).to be_nil
           expect(merge_request.merge_error).to include(error_message)
           expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
         end
@@ -395,10 +407,27 @@ RSpec.describe MergeRequests::MergeService do
 
           expect(merge_request).to be_open
           expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.squash_commit_sha).to be_nil
           expect(merge_request.merge_error).to include(error_message)
           expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
         end
 
+        it 'logs and saves error if there is an PreReceiveError exception' do
+          error_message = 'error message'
+
+          allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
+          allow(service).to receive(:execute_hooks)
+          merge_request.update!(squash: true)
+
+          service.execute(merge_request)
+
+          expect(merge_request).to be_open
+          expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.squash_commit_sha).to be_nil
+          expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook')
+          expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
+        end
+
         context 'when fast-forward merge is not allowed' do
           before do
             allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
@@ -415,6 +444,7 @@ RSpec.describe MergeRequests::MergeService do
 
               expect(merge_request).to be_open
               expect(merge_request.merge_commit_sha).to be_nil
+              expect(merge_request.squash_commit_sha).to be_nil
               expect(merge_request.merge_error).to include(error_message)
               expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
             end
diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb
index 725fc16fa7c..17bfa9d7368 100644
--- a/spec/services/merge_requests/mergeability_check_service_spec.rb
+++ b/spec/services/merge_requests/mergeability_check_service_spec.rb
@@ -124,14 +124,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
 
     it_behaves_like 'mergeable merge request'
 
-    context 'when lock is disabled' do
-      before do
-        stub_feature_flags(merge_ref_auto_sync_lock: false)
-      end
-
-      it_behaves_like 'mergeable merge request'
-    end
-
     context 'when concurrent calls' do
       it 'waits first lock and returns "cached" result in subsequent calls' do
         threads = execute_within_threads(amount: 3)
@@ -167,25 +159,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
       end
     end
 
-    context 'disabled merge ref sync feature flag' do
-      before do
-        stub_feature_flags(merge_ref_auto_sync: false)
-      end
-
-      it 'returns error and no payload' do
-        result = subject
-
-        expect(result).to be_a(ServiceResponse)
-        expect(result.error?).to be(true)
-        expect(result.message).to eq('Merge ref is outdated due to disabled feature')
-        expect(result.payload).to be_empty
-      end
-
-      it 'ignores merge-ref and updates merge status' do
-        expect { subject }.to change(merge_request, :merge_status).from('unchecked').to('can_be_merged')
-      end
-    end
-
     context 'when broken' do
       before do
         expect(merge_request).to receive(:broken?) { true }
@@ -305,28 +278,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
     context 'recheck enforced' do
       subject { described_class.new(merge_request).execute(recheck: true) }
 
-      context 'when MR is mergeable and merge-ref auto-sync is disabled' do
-        before do
-          stub_feature_flags(merge_ref_auto_sync: false)
-          merge_request.mark_as_mergeable!
-        end
-
-        it 'returns ServiceResponse.error' do
-          result = subject
-
-          expect(result).to be_a(ServiceResponse)
-          expect(result.error?).to be(true)
-          expect(result.message).to eq('Merge ref is outdated due to disabled feature')
-          expect(result.payload).to be_empty
-        end
-
-        it 'merge status is not changed' do
-          subject
-
-          expect(merge_request.merge_status).to eq('can_be_merged')
-        end
-      end
-
       context 'when MR is marked as mergeable, but repo is not mergeable and MR is not opened' do
         before do
           # Making sure that we don't touch the merge-status after
diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb
index 402f753c0af..6523b5a158c 100644
--- a/spec/services/merge_requests/post_merge_service_spec.rb
+++ b/spec/services/merge_requests/post_merge_service_spec.rb
@@ -15,6 +15,7 @@ RSpec.describe MergeRequests::PostMergeService do
 
   describe '#execute' do
     it_behaves_like 'cache counters invalidator'
+    it_behaves_like 'merge request reviewers cache counters invalidator'
 
     it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do
       # Cache the counter before the MR changed state.
@@ -37,6 +38,14 @@ RSpec.describe MergeRequests::PostMergeService do
       subject
     end
 
+    it 'calls the merge request activity counter' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_merge_mr_action)
+        .with(user: user)
+
+      subject
+    end
+
     it 'deletes non-latest diffs' do
       diff_removal_service = instance_double(MergeRequests::DeleteNonLatestDiffsService, execute: nil)
 
diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb
index 2bd83dc36a8..8541d597581 100644
--- a/spec/services/merge_requests/reopen_service_spec.rb
+++ b/spec/services/merge_requests/reopen_service_spec.rb
@@ -17,6 +17,7 @@ RSpec.describe MergeRequests::ReopenService do
 
   describe '#execute' do
     it_behaves_like 'cache counters invalidator'
+    it_behaves_like 'merge request reviewers cache counters invalidator'
 
     context 'valid params' do
       let(:service) { described_class.new(project, user, {}) }
@@ -80,6 +81,14 @@ RSpec.describe MergeRequests::ReopenService do
       described_class.new(project, user, {}).execute(merge_request)
     end
 
+    it 'calls the merge request activity counter' do
+      expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+        .to receive(:track_reopen_mr_action)
+        .with(user: user)
+
+      described_class.new(project, user, {}).execute(merge_request)
+    end
+
     it 'refreshes the number of open merge requests for a valid MR' do
       service = described_class.new(project, user, {})
 
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index ed8872b71f7..9eb82dcd0ad 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -431,14 +431,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
 
     describe 'merge' do
       it_behaves_like 'correct merge behavior'
-
-      context 'when merge_orchestration_service feature flag is disabled' do
-        before do
-          stub_feature_flags(merge_orchestration_service: false)
-        end
-
-        it_behaves_like 'correct merge behavior'
-      end
     end
 
     context 'todos' do
@@ -529,6 +521,19 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           should_email(user2)
           should_email(user3)
         end
+
+        it 'updates open merge request counter for reviewers', :use_clean_rails_memory_store_caching do
+          merge_request.reviewers = [user3]
+
+          # Cache them to ensure the cache gets invalidated on update
+          expect(user2.review_requested_open_merge_requests_count).to eq(0)
+          expect(user3.review_requested_open_merge_requests_count).to eq(1)
+
+          update_merge_request(reviewer_ids: [user2.id])
+
+          expect(user2.review_requested_open_merge_requests_count).to eq(1)
+          expect(user3.review_requested_open_merge_requests_count).to eq(0)
+        end
       end
 
       context 'when the milestone is removed' do
@@ -837,20 +842,20 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
       it 'does not allow a maintainer of the target project to set `allow_collaboration`' do
         target_project.add_developer(user)
 
-        update_merge_request(allow_collaboration: true, title: 'Updated title')
+        update_merge_request(allow_collaboration: false, title: 'Updated title')
 
         expect(merge_request.title).to eq('Updated title')
-        expect(merge_request.allow_collaboration).to be_falsy
+        expect(merge_request.allow_collaboration).to be_truthy
       end
 
       it 'is allowed by a user that can push to the source and can update the merge request' do
         merge_request.update!(assignees: [user])
         source_project.add_developer(user)
 
-        update_merge_request(allow_collaboration: true, title: 'Updated title')
+        update_merge_request(allow_collaboration: false, title: 'Updated title')
 
         expect(merge_request.title).to eq('Updated title')
-        expect(merge_request.allow_collaboration).to be_truthy
+        expect(merge_request.allow_collaboration).to be_falsy
       end
     end