Add latest changes from gitlab-org/gitlab@13-4-stable-ee

8 files changed, 198 insertions, 71 deletions

diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 7e301f311e9..abc3f99797d 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -23,6 +23,8 @@ module MergeRequests
       merge_data = hook_data(merge_request, action, old_rev: old_rev, old_associations: old_associations)
       merge_request.project.execute_hooks(merge_data, :merge_request_hooks)
       merge_request.project.execute_services(merge_data, :merge_request_hooks)
+
+      enqueue_jira_connect_messages_for(merge_request)
     end
 
     def cleanup_environments(merge_request)
@@ -52,6 +54,14 @@ module MergeRequests
 
     private
 
+    def enqueue_jira_connect_messages_for(merge_request)
+      return unless project.jira_subscription_exists?
+
+      if Atlassian::JiraIssueKeyExtractor.has_keys?(merge_request.title, merge_request.description)
+        JiraConnect::SyncMergeRequestWorker.perform_async(merge_request.id)
+      end
+    end
+
     def create(merge_request)
       self.params = assign_allowed_merge_params(merge_request, params)
 
@@ -87,6 +97,28 @@ module MergeRequests
       unless merge_request.can_allow_collaboration?(current_user)
         params.delete(:allow_collaboration)
       end
+
+      filter_reviewer(merge_request)
+    end
+
+    def filter_reviewer(merge_request)
+      return if params[:reviewer_ids].blank?
+
+      unless can_admin_issuable?(merge_request) && merge_request.allows_reviewers?
+        params.delete(:reviewer_ids)
+
+        return
+      end
+
+      reviewer_ids = params[:reviewer_ids].select { |reviewer_id| user_can_read?(merge_request, reviewer_id) }
+
+      if params[:reviewer_ids].map(&:to_s) == [IssuableFinder::Params::NONE]
+        params[:reviewer_ids] = []
+      elsif reviewer_ids.any?
+        params[:reviewer_ids] = reviewer_ids
+      else
+        params.delete(:reviewer_ids)
+      end
     end
 
     def merge_request_metrics_service(merge_request)
diff --git a/app/services/merge_requests/cleanup_refs_service.rb b/app/services/merge_requests/cleanup_refs_service.rb
new file mode 100644
index 00000000000..0f03f5f09b4
--- /dev/null
+++ b/app/services/merge_requests/cleanup_refs_service.rb
@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+module MergeRequests
+  class CleanupRefsService
+    include BaseServiceUtility
+
+    TIME_THRESHOLD = 14.days
+
+    attr_reader :merge_request
+
+    def self.schedule(merge_request)
+      MergeRequestCleanupRefsWorker.perform_in(TIME_THRESHOLD, merge_request.id)
+    end
+
+    def initialize(merge_request)
+      @merge_request = merge_request
+      @repository = merge_request.project.repository
+      @ref_path = merge_request.ref_path
+      @merge_ref_path = merge_request.merge_ref_path
+      @ref_head_sha = @repository.commit(merge_request.ref_path).id
+      @merge_ref_sha = merge_request.merge_ref_head&.id
+    end
+
+    def execute
+      return error("Merge request has not been closed nor merged for #{TIME_THRESHOLD.inspect}.") unless eligible?
+
+      # Ensure that commit shas of refs are kept around so we won't lose them when GC runs.
+      keep_around
+
+      return error('Failed to create keep around refs.') unless kept_around?
+      return error('Failed to cache merge ref sha.') unless cache_merge_ref_sha
+
+      delete_refs
+      success
+    end
+
+    private
+
+    attr_reader :repository, :ref_path, :merge_ref_path, :ref_head_sha, :merge_ref_sha
+
+    def eligible?
+      return met_time_threshold?(merge_request.metrics&.latest_closed_at) if merge_request.closed?
+
+      merge_request.merged? && met_time_threshold?(merge_request.metrics&.merged_at)
+    end
+
+    def met_time_threshold?(attr)
+      attr.nil? || attr.to_i <= TIME_THRESHOLD.ago.to_i
+    end
+
+    def kept_around?
+      service = Gitlab::Git::KeepAround.new(repository)
+
+      [ref_head_sha, merge_ref_sha].compact.all? do |sha|
+        service.kept_around?(sha)
+      end
+    end
+
+    def keep_around
+      repository.keep_around(ref_head_sha, merge_ref_sha)
+    end
+
+    def cache_merge_ref_sha
+      return true if merge_ref_sha.nil?
+
+      # Caching the merge ref sha is needed before we delete the merge ref so
+      # we can still show the merge ref diff (via `MergeRequest#merge_ref_head`)
+      merge_request.update_column(:merge_ref_sha, merge_ref_sha)
+    end
+
+    def delete_refs
+      repository.delete_refs(ref_path, merge_ref_path)
+    end
+  end
+end
diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb
index c2174d2a130..b0a7face594 100644
--- a/app/services/merge_requests/close_service.rb
+++ b/app/services/merge_requests/close_service.rb
@@ -2,6 +2,8 @@
 
 module MergeRequests
   class CloseService < MergeRequests::BaseService
+    include RemovesRefs
+
     def execute(merge_request, commit = nil)
       return merge_request unless can?(current_user, :update_merge_request, merge_request)
 
@@ -19,6 +21,7 @@ module MergeRequests
         merge_request.update_project_counter_caches
         cleanup_environments(merge_request)
         abort_auto_merge(merge_request, 'merge request was closed')
+        cleanup_refs(merge_request)
       end
 
       merge_request
diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb
index f9352f10fea..46c4c102091 100644
--- a/app/services/merge_requests/create_pipeline_service.rb
+++ b/app/services/merge_requests/create_pipeline_service.rb
@@ -48,12 +48,18 @@ module MergeRequests
     end
 
     def can_create_pipeline_in_target_project?(merge_request)
-      if Gitlab::Ci::Features.allow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project)
-        can?(current_user, :create_pipeline, merge_request.target_project)
-      else
+      if Gitlab::Ci::Features.disallow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project)
         merge_request.for_same_project?
+      else
+        can?(current_user, :create_pipeline, merge_request.target_project) &&
+          can_update_source_branch_in_target_project?(merge_request)
       end
     end
+
+    def can_update_source_branch_in_target_project?(merge_request)
+      ::Gitlab::UserAccess.new(current_user, container: merge_request.target_project)
+        .can_update_branch?(merge_request.source_branch_ref)
+    end
   end
 end
 
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 961a7cb1ef6..437e87dadf7 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -10,13 +10,14 @@ module MergeRequests
   class MergeService < MergeRequests::MergeBaseService
     delegate :merge_jid, :state, to: :@merge_request
 
-    def execute(merge_request)
+    def execute(merge_request, options = {})
       if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService)
         FfMergeService.new(project, current_user, params).execute(merge_request)
         return
       end
 
       @merge_request = merge_request
+      @options = options
 
       validate!
 
@@ -55,7 +56,7 @@ module MergeRequests
       error =
         if @merge_request.should_be_rebased?
           'Only fast-forward merge is allowed for your project. Please update your source branch'
-        elsif !@merge_request.mergeable?
+        elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check])
           'Merge request is not mergeable'
         elsif !@merge_request.squash && project.squash_always?
           'This project requires squashing commits when merge requests are accepted.'
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index fdf8f442297..1c78fca3c26 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -7,6 +7,8 @@ module MergeRequests
   # and execute all hooks and notifications
   #
   class PostMergeService < MergeRequests::BaseService
+    include RemovesRefs
+
     def execute(merge_request)
       merge_request.mark_as_merged
       close_issues(merge_request)
@@ -20,6 +22,7 @@ module MergeRequests
       delete_non_latest_diffs(merge_request)
       cancel_review_app_jobs!(merge_request)
       cleanup_environments(merge_request)
+      cleanup_refs(merge_request)
     end
 
     private
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 56a91fa0305..405b8fe9c9e 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -2,6 +2,7 @@
 
 module MergeRequests
   class RefreshService < MergeRequests::BaseService
+    include Gitlab::Utils::StrongMemoize
     attr_reader :push
 
     def execute(oldrev, newrev, ref)
@@ -23,25 +24,37 @@ module MergeRequests
       post_merge_manually_merged
       link_forks_lfs_objects
       reload_merge_requests
-      outdate_suggestions
-      refresh_pipelines_on_merge_requests
-      abort_auto_merges
+
+      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
+
       abort_ff_merge_requests_with_when_pipeline_succeeds
-      mark_pending_todos_done
       cache_merge_requests_closing_issues
 
-      # Leave a system note if a branch was deleted/added
-      if @push.branch_added? || @push.branch_removed?
-        comment_mr_branch_presence_changed
-      end
+      merge_requests_for_source_branch.each do |mr|
+        # Leave a system note if a branch was deleted/added
+        if branch_added_or_removed?
+          comment_mr_branch_presence_changed(mr)
+        end
 
-      notify_about_push
-      mark_mr_as_wip_from_commits
-      execute_mr_web_hooks
+        notify_about_push(mr)
+        mark_mr_as_wip_from_commits(mr)
+        execute_mr_web_hooks(mr)
+      end
 
       true
     end
 
+    def branch_added_or_removed?
+      strong_memoize(:branch_added_or_removed) do
+        @push.branch_added? || @push.branch_removed?
+      end
+    end
+
     def close_upon_missing_source_branch_ref
       # MergeRequest#reload_diff ignores not opened MRs. This means it won't
       # create an `empty` diff for `closed` MRs without a source branch, keeping
@@ -140,25 +153,22 @@ module MergeRequests
         merge_request.source_branch == @push.branch_name
     end
 
-    def outdate_suggestions
-      outdate_service = Suggestions::OutdateService.new
+    def outdate_suggestions(merge_request)
+      outdate_service.execute(merge_request)
+    end
 
-      merge_requests_for_source_branch.each do |merge_request|
-        outdate_service.execute(merge_request)
-      end
+    def outdate_service
+      @outdate_service ||= Suggestions::OutdateService.new
     end
 
-    def refresh_pipelines_on_merge_requests
-      merge_requests_for_source_branch.each do |merge_request|
-        create_pipeline_for(merge_request, current_user)
-        UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
-      end
+    def refresh_pipelines_on_merge_requests(merge_request)
+      create_pipeline_for(merge_request, current_user)
+
+      UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id)
     end
 
-    def abort_auto_merges
-      merge_requests_for_source_branch.each do |merge_request|
-        abort_auto_merge(merge_request, 'source branch was updated')
-      end
+    def abort_auto_merges(merge_request)
+      abort_auto_merge(merge_request, 'source branch was updated')
     end
 
     def abort_ff_merge_requests_with_when_pipeline_succeeds
@@ -187,10 +197,8 @@ module MergeRequests
         .with_auto_merge_enabled
     end
 
-    def mark_pending_todos_done
-      merge_requests_for_source_branch.each do |merge_request|
-        todo_service.merge_request_push(merge_request, @current_user)
-      end
+    def mark_pending_todos_done(merge_request)
+      todo_service.merge_request_push(merge_request, @current_user)
     end
 
     def find_new_commits
@@ -218,62 +226,54 @@ module MergeRequests
     end
 
     # Add comment about branches being deleted or added to merge requests
-    def comment_mr_branch_presence_changed
+    def comment_mr_branch_presence_changed(merge_request)
       presence = @push.branch_added? ? :add : :delete
 
-      merge_requests_for_source_branch.each do |merge_request|
-        SystemNoteService.change_branch_presence(
-          merge_request, merge_request.project, @current_user,
-          :source, @push.branch_name, presence)
-      end
+      SystemNoteService.change_branch_presence(
+        merge_request, merge_request.project, @current_user,
+        :source, @push.branch_name, presence)
     end
 
     # Add comment about pushing new commits to merge requests and send nofitication emails
-    def notify_about_push
+    def notify_about_push(merge_request)
       return unless @commits.present?
 
-      merge_requests_for_source_branch.each do |merge_request|
-        mr_commit_ids = Set.new(merge_request.commit_shas)
+      mr_commit_ids = Set.new(merge_request.commit_shas)
 
-        new_commits, existing_commits = @commits.partition do |commit|
-          mr_commit_ids.include?(commit.id)
-        end
+      new_commits, existing_commits = @commits.partition do |commit|
+        mr_commit_ids.include?(commit.id)
+      end
 
-        SystemNoteService.add_commits(merge_request, merge_request.project,
-                                      @current_user, new_commits,
-                                      existing_commits, @push.oldrev)
+      SystemNoteService.add_commits(merge_request, merge_request.project,
+                                    @current_user, new_commits,
+                                    existing_commits, @push.oldrev)
 
-        notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
-      end
+      notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits)
     end
 
-    def mark_mr_as_wip_from_commits
+    def mark_mr_as_wip_from_commits(merge_request)
       return unless @commits.present?
 
-      merge_requests_for_source_branch.each do |merge_request|
-        commit_shas = merge_request.commit_shas
+      commit_shas = merge_request.commit_shas
 
-        wip_commit = @commits.detect do |commit|
-          commit.work_in_progress? && commit_shas.include?(commit.sha)
-        end
+      wip_commit = @commits.detect do |commit|
+        commit.work_in_progress? && commit_shas.include?(commit.sha)
+      end
 
-        if wip_commit && !merge_request.work_in_progress?
-          merge_request.update(title: merge_request.wip_title)
-          SystemNoteService.add_merge_request_wip_from_commit(
-            merge_request,
-            merge_request.project,
-            @current_user,
-            wip_commit
-          )
-        end
+      if wip_commit && !merge_request.work_in_progress?
+        merge_request.update(title: merge_request.wip_title)
+        SystemNoteService.add_merge_request_wip_from_commit(
+          merge_request,
+          merge_request.project,
+          @current_user,
+          wip_commit
+        )
       end
     end
 
     # Call merge request webhook with update branches
-    def execute_mr_web_hooks
-      merge_requests_for_source_branch.each do |merge_request|
-        execute_hooks(merge_request, 'update', old_rev: @push.oldrev)
-      end
+    def execute_mr_web_hooks(merge_request)
+      execute_hooks(merge_request, 'update', old_rev: @push.oldrev)
     end
 
     # If the merge requests closes any issues, save this information in the
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index cf02158b629..1468bfd6bb6 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -24,8 +24,9 @@ module MergeRequests
       old_labels = old_associations.fetch(:labels, [])
       old_mentioned_users = old_associations.fetch(:mentioned_users, [])
       old_assignees = old_associations.fetch(:assignees, [])
+      old_reviewers = old_associations.fetch(:reviewers, [])
 
-      if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees)
+      if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees, old_reviewers: old_reviewers)
         todo_service.resolve_todos_for_target(merge_request, current_user)
       end
 
@@ -44,6 +45,8 @@ module MergeRequests
 
       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
+
       if merge_request.previous_changes.include?('target_branch') ||
           merge_request.previous_changes.include?('source_branch')
         merge_request.mark_as_unchecked
@@ -108,6 +111,10 @@ module MergeRequests
       todo_service.reassigned_assignable(merge_request, current_user, old_assignees)
     end
 
+    def handle_reviewers_change(merge_request, old_reviewers)
+      todo_service.reassigned_reviewable(merge_request, current_user, old_reviewers)
+    end
+
     def create_branch_change_note(issuable, branch_type, old_branch, new_branch)
       SystemNoteService.change_branch(
         issuable, issuable.project, current_user, branch_type,