Add latest changes from gitlab-org/gitlab@14-0-stable-eev14.0.0-rc42

5 files changed, 47 insertions, 27 deletions

diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index e94274aff9d..7ebdbf94932 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -24,9 +24,42 @@ module MergeRequests
       merge_request.project.execute_hooks(merge_data, :merge_request_hooks)
       merge_request.project.execute_services(merge_data, :merge_request_hooks)
 
+      execute_external_hooks(merge_request, merge_data)
+
       enqueue_jira_connect_messages_for(merge_request)
     end
 
+    def execute_external_hooks(merge_request, merge_data)
+      # Implemented in EE
+    end
+
+    def handle_changes(merge_request, options)
+      old_associations = options.fetch(:old_associations, {})
+      old_assignees = old_associations.fetch(:assignees, [])
+      old_reviewers = old_associations.fetch(:reviewers, [])
+
+      handle_assignees_change(merge_request, old_assignees) if merge_request.assignees != old_assignees
+      handle_reviewers_change(merge_request, old_reviewers) if merge_request.reviewers != old_reviewers
+    end
+
+    def handle_assignees_change(merge_request, old_assignees)
+      MergeRequests::HandleAssigneesChangeService
+        .new(project: project, current_user: current_user)
+        .async_execute(merge_request, old_assignees)
+    end
+
+    def handle_reviewers_change(merge_request, old_reviewers)
+      affected_reviewers = (old_reviewers + merge_request.reviewers) - (old_reviewers & merge_request.reviewers)
+      create_reviewer_note(merge_request, old_reviewers)
+      notification_service.async.changed_reviewer_of_merge_request(merge_request, current_user, old_reviewers)
+      todo_service.reassigned_reviewable(merge_request, current_user, old_reviewers)
+      invalidate_cache_counts(merge_request, users: affected_reviewers.compact)
+
+      new_reviewers = merge_request.reviewers - old_reviewers
+      merge_request_activity_counter.track_users_review_requested(users: new_reviewers)
+      merge_request_activity_counter.track_reviewers_changed_action(user: current_user)
+    end
+
     def cleanup_environments(merge_request)
       Ci::StopEnvironmentsService.new(merge_request.source_project, current_user)
                                  .execute_for_merge_request(merge_request)
diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb
index 9ac386110f7..87cd6544406 100644
--- a/app/services/merge_requests/handle_assignees_change_service.rb
+++ b/app/services/merge_requests/handle_assignees_change_service.rb
@@ -8,7 +8,7 @@ module MergeRequests
           merge_request.id,
           current_user.id,
           old_assignees.map(&:id),
-          options
+          options.stringify_keys # see: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1090
         )
     end
 
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index d5e2595a9c6..3a4e3ba38fd 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -27,7 +27,6 @@ module MergeRequests
 
       merge_requests_for_source_branch.each do |mr|
         outdate_suggestions(mr)
-        refresh_pipelines_on_merge_requests(mr)
         abort_auto_merges(mr)
         mark_pending_todos_done(mr)
       end
@@ -44,6 +43,8 @@ module MergeRequests
         notify_about_push(mr)
         mark_mr_as_draft_from_commits(mr)
         execute_mr_web_hooks(mr)
+        # Run at the end of the loop to avoid any potential contention on the MR object
+        refresh_pipelines_on_merge_requests(mr)
         merge_request_activity_counter.track_mr_including_ci_config(user: mr.author, merge_request: mr)
       end
 
diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb
index f99db35fd49..d52c1bbbcda 100644
--- a/app/services/merge_requests/update_assignees_service.rb
+++ b/app/services/merge_requests/update_assignees_service.rb
@@ -9,9 +9,11 @@ module MergeRequests
     def execute(merge_request)
       return merge_request unless current_user&.can?(:update_merge_request, merge_request)
 
-      old_assignees = merge_request.assignees
+      old_assignees = merge_request.assignees.to_a
       old_ids = old_assignees.map(&:id)
       new_ids = new_assignee_ids(merge_request)
+
+      return merge_request if merge_request.errors.any?
       return merge_request if new_ids.size != update_attrs[:assignee_ids].size
       return merge_request if old_ids.to_set == new_ids.to_set # no-change
 
@@ -30,8 +32,11 @@ module MergeRequests
 
     def new_assignee_ids(merge_request)
       # prime the cache - prevent N+1 lookup during authorization loop.
-      merge_request.project.team.max_member_access_for_user_ids(update_attrs[:assignee_ids])
-      User.id_in(update_attrs[:assignee_ids]).map do |user|
+      user_ids = update_attrs[:assignee_ids]
+      return [] if user_ids.empty?
+
+      merge_request.project.team.max_member_access_for_user_ids(user_ids)
+      User.id_in(user_ids).map do |user|
         if user.can?(:read_merge_request, merge_request)
           user.id
         else
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index b613d88aee4..af041de5596 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -15,6 +15,7 @@ module MergeRequests
     end
 
     def handle_changes(merge_request, options)
+      super
       old_associations = options.fetch(:old_associations, {})
       old_labels = old_associations.fetch(:labels, [])
       old_mentioned_users = old_associations.fetch(:mentioned_users, [])
@@ -31,8 +32,6 @@ module MergeRequests
       end
 
       handle_target_branch_change(merge_request)
-      handle_assignees_change(merge_request, old_assignees) if merge_request.assignees != old_assignees
-      handle_reviewers_change(merge_request, old_reviewers) if merge_request.reviewers != old_reviewers
       handle_milestone_change(merge_request)
       handle_draft_status_change(merge_request, changed_fields)
 
@@ -50,7 +49,7 @@ module MergeRequests
       #
       if merge_request.previous_changes.include?('target_branch') ||
           merge_request.previous_changes.include?('source_branch')
-        merge_request.mark_as_unchecked
+        merge_request.mark_as_unchecked unless merge_request.unchecked?
       end
     end
 
@@ -220,24 +219,6 @@ module MergeRequests
       end
     end
 
-    def handle_assignees_change(merge_request, old_assignees)
-      MergeRequests::HandleAssigneesChangeService
-        .new(project: project, current_user: current_user)
-        .async_execute(merge_request, old_assignees)
-    end
-
-    def handle_reviewers_change(merge_request, old_reviewers)
-      affected_reviewers = (old_reviewers + merge_request.reviewers) - (old_reviewers & merge_request.reviewers)
-      create_reviewer_note(merge_request, old_reviewers)
-      notification_service.async.changed_reviewer_of_merge_request(merge_request, current_user, old_reviewers)
-      todo_service.reassigned_reviewable(merge_request, current_user, old_reviewers)
-      invalidate_cache_counts(merge_request, users: affected_reviewers.compact)
-
-      new_reviewers = merge_request.reviewers - old_reviewers
-      merge_request_activity_counter.track_users_review_requested(users: new_reviewers)
-      merge_request_activity_counter.track_reviewers_changed_action(user: current_user)
-    end
-
     def create_branch_change_note(issuable, branch_type, event_type, old_branch, new_branch)
       SystemNoteService.change_branch(
         issuable, issuable.project, current_user, branch_type, event_type,
@@ -293,7 +274,7 @@ module MergeRequests
 
     def attempt_specialized_update_services(merge_request, attribute)
       case attribute
-      when :assignee_ids
+      when :assignee_ids, :assignee_id
         assignees_service.execute(merge_request)
       when :spend_time
         add_time_spent_service.execute(merge_request)