Add latest changes from gitlab-org/gitlab@master

5 files changed, 144 insertions, 11 deletions

diff --git a/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb
index 8a53d9fbf7c..c6b184bd003 100644
--- a/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb
+++ b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb
@@ -59,7 +59,9 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
             .create!(project: project2, access_level: Gitlab::Access::MAINTAINER)
 
           to_be_removed = [project2.id]
-          to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]]
+          to_be_added = [
+            { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+          ]
 
           expect(service.execute).to eq([to_be_removed, to_be_added])
         end
@@ -70,7 +72,9 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
           end
 
           to_be_removed = [project.id]
-          to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]]
+          to_be_added = [
+            { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+          ]
 
           expect(service.execute).to eq([to_be_removed, to_be_added])
         end
@@ -80,7 +84,9 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
             .create!(project: project, access_level: Gitlab::Access::DEVELOPER)
 
           to_be_removed = [project.id]
-          to_be_added = [[user.id, project.id, Gitlab::Access::MAINTAINER]]
+          to_be_added = [
+            { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+          ]
 
           expect(service.execute).to eq([to_be_removed, to_be_added])
         end
diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb
index cbbd193a411..781be57d709 100644
--- a/spec/services/merge_requests/after_create_service_spec.rb
+++ b/spec/services/merge_requests/after_create_service_spec.rb
@@ -85,13 +85,67 @@ RSpec.describe MergeRequests::AfterCreateService do
 
     context 'when merge request is in preparing state' do
       before do
+        merge_request.mark_as_unchecked! unless merge_request.unchecked?
         merge_request.mark_as_preparing!
-        execute_service
       end
 
       it 'marks the merge request as unchecked' do
+        execute_service
+
         expect(merge_request.reload).to be_unchecked
       end
+
+      context 'when preparing for mergeability fails' do
+        before do
+          # This is only one of the possible cases that can fail. This is to
+          # simulate a failure that happens during the service call.
+          allow(merge_request)
+            .to receive(:update_head_pipeline)
+            .and_raise(StandardError)
+        end
+
+        it 'does not mark the merge request as unchecked' do
+          expect { execute_service }.to raise_error(StandardError)
+          expect(merge_request.reload).to be_preparing
+        end
+
+        context 'when early_prepare_for_mergeability feature flag is disabled' do
+          before do
+            stub_feature_flags(early_prepare_for_mergeability: false)
+          end
+
+          it 'does not mark the merge request as unchecked' do
+            expect { execute_service }.to raise_error(StandardError)
+            expect(merge_request.reload).to be_preparing
+          end
+        end
+      end
+
+      context 'when preparing merge request fails' do
+        before do
+          # This is only one of the possible cases that can fail. This is to
+          # simulate a failure that happens during the service call.
+          allow(merge_request)
+            .to receive_message_chain(:diffs, :write_cache)
+            .and_raise(StandardError)
+        end
+
+        it 'still marks the merge request as unchecked' do
+          expect { execute_service }.to raise_error(StandardError)
+          expect(merge_request.reload).to be_unchecked
+        end
+
+        context 'when early_prepare_for_mergeability feature flag is disabled' do
+          before do
+            stub_feature_flags(early_prepare_for_mergeability: false)
+          end
+
+          it 'does not mark the merge request as unchecked' do
+            expect { execute_service }.to raise_error(StandardError)
+            expect(merge_request.reload).to be_preparing
+          end
+        end
+      end
     end
 
     it 'increments the usage data counter of create event' do
diff --git a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
index a26b1be529e..e2455a71eef 100644
--- a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
+++ b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb
@@ -13,9 +13,12 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
   let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) }
   let(:result) { service.execute }
   let(:todo_service) { spy('todo service') }
+  let(:notification_service) { spy('notification service') }
 
   before do
+    allow(NotificationService).to receive(:new) { notification_service }
     allow(service).to receive(:todo_service).and_return(todo_service)
+    allow(service).to receive(:notification_service).and_return(notification_service)
 
     project.add_developer(current_user)
     project.add_developer(user)
@@ -59,6 +62,12 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do
 
         service.execute
       end
+
+      it 'sends email to reviewer' do
+        expect(notification_service).to receive_message_chain(:async, :attention_requested_of_merge_request).with(merge_request, current_user, user)
+
+        service.execute
+      end
     end
 
     context 'assignee exists' do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index fbf5b183365..24775ce06a4 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2425,6 +2425,45 @@ RSpec.describe NotificationService, :mailer do
         let(:notification_trigger) { notification.review_requested_of_merge_request(merge_request, current_user, reviewer) }
       end
     end
+
+    describe '#attention_requested_of_merge_request' do
+      let_it_be(:current_user) { create(:user) }
+      let_it_be(:reviewer) { create(:user) }
+      let_it_be(:merge_request) { create(:merge_request, source_project: project, reviewers: [reviewer]) }
+
+      it 'sends email to reviewer', :aggregate_failures do
+        notification.attention_requested_of_merge_request(merge_request, current_user, reviewer)
+
+        merge_request.reviewers.each { |reviewer| should_email(reviewer) }
+        should_not_email(merge_request.author)
+        should_not_email(@u_watcher)
+        should_not_email(@u_participant_mentioned)
+        should_not_email(@subscriber)
+        should_not_email(@watcher_and_subscriber)
+        should_not_email(@u_guest_watcher)
+        should_not_email(@u_guest_custom)
+        should_not_email(@u_custom_global)
+        should_not_email(@unsubscriber)
+        should_not_email(@u_participating)
+        should_not_email(@u_disabled)
+        should_not_email(@u_lazy_participant)
+      end
+
+      it 'adds "attention requested" reason' do
+        notification.attention_requested_of_merge_request(merge_request, current_user, [reviewer])
+
+        merge_request.reviewers.each do |reviewer|
+          email = find_email_for(reviewer)
+
+          expect(email).to have_header('X-GitLab-NotificationReason', NotificationReason::ATTENTION_REQUESTED)
+        end
+      end
+
+      it_behaves_like 'project emails are disabled' do
+        let(:notification_target)  { merge_request }
+        let(:notification_trigger) { notification.attention_requested_of_merge_request(merge_request, current_user, reviewer) }
+      end
+    end
   end
 
   describe 'Projects', :deliver_mails_inline do
diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb
index a8ad0d02f60..aa4df93a241 100644
--- a/spec/services/users/refresh_authorized_projects_service_spec.rb
+++ b/spec/services/users/refresh_authorized_projects_service_spec.rb
@@ -67,11 +67,17 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
 
     it 'updates the authorized projects of the user' do
       project2 = create(:project)
-      to_remove = user.project_authorizations
+      project_authorization = user.project_authorizations
         .create!(project: project2, access_level: Gitlab::Access::MAINTAINER)
 
+      to_be_removed = [project_authorization.project_id]
+
+      to_be_added = [
+        { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+      ]
+
       expect(service).to receive(:update_authorizations)
-        .with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MAINTAINER]])
+        .with(to_be_removed, to_be_added)
 
       service.execute_without_lease
     end
@@ -81,9 +87,14 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
         user.project_authorizations.create!(project: project, access_level: access_level)
       end
 
+      to_be_removed = [project.id]
+
+      to_be_added = [
+        { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+      ]
       expect(service).to(
         receive(:update_authorizations)
-          .with([project.id], [[user.id, project.id, Gitlab::Access::MAINTAINER]])
+          .with(to_be_removed, to_be_added)
           .and_call_original)
 
       service.execute_without_lease
@@ -99,11 +110,17 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
     it 'sets the access level of a project to the highest available level' do
       user.project_authorizations.delete_all
 
-      to_remove = user.project_authorizations
+      project_authorization = user.project_authorizations
         .create!(project: project, access_level: Gitlab::Access::DEVELOPER)
 
+      to_be_removed = [project_authorization.project_id]
+
+      to_be_added = [
+        { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+      ]
+
       expect(service).to receive(:update_authorizations)
-        .with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MAINTAINER]])
+        .with(to_be_removed, to_be_added)
 
       service.execute_without_lease
     end
@@ -134,7 +151,11 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
     it 'inserts authorizations that should be added' do
       user.project_authorizations.delete_all
 
-      service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MAINTAINER]])
+      to_be_added = [
+        { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+      ]
+
+      service.update_authorizations([], to_be_added)
 
       authorizations = user.project_authorizations
 
@@ -160,7 +181,11 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
                 'authorized_projects_refresh.rows_added_slice': [[user.id, project.id, Gitlab::Access::MAINTAINER]])
       )
 
-      service.update_authorizations([], [[user.id, project.id, Gitlab::Access::MAINTAINER]])
+      to_be_added = [
+        { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
+      ]
+
+      service.update_authorizations([], to_be_added)
     end
   end
 end