Merge branch 'issue_44821' into 'master'

Retrieve merge request closing issues from database cache

Closes #44821

See merge request gitlab-org/gitlab-ce!20911

4 files changed, 21 insertions, 5 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6de44751f1b..acad8b91e9f 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -60,6 +60,8 @@ class MergeRequest < ActiveRecord::Base
     class_name: 'MergeRequestsClosingIssues',
     dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
+  has_many :cached_closes_issues, through: :merge_requests_closing_issues, source: :issue
+
   belongs_to :assignee, class_name: "User"
 
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
@@ -763,8 +765,9 @@ class MergeRequest < ActiveRecord::Base
   # Calculating this information for a number of merge requests requires
   # running `ReferenceExtractor` on each of them separately.
   # This optimization does not apply to issues from external sources.
-  def cache_merge_request_closes_issues!(current_user)
+  def cache_merge_request_closes_issues!(current_user = self.author)
     return unless project.issues_enabled?
+    return if closed? || merged?
 
     transaction do
       self.merge_requests_closing_issues.delete_all
@@ -777,6 +780,18 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
+  def visible_closing_issues_for(current_user = self.author)
+    strong_memoize(:visible_closing_issues_for) do
+      if self.target_project.has_external_issue_tracker?
+        closes_issues(current_user)
+      else
+        cached_closes_issues.select do |issue|
+          Ability.allowed?(current_user, :read_issue, issue)
+        end
+      end
+    end
+  end
+
   # Return the set of issues that will be closed if this merge request is accepted.
   def closes_issues(current_user = self.author)
     if target_branch == project.default_branch
@@ -796,7 +811,7 @@ class MergeRequest < ActiveRecord::Base
     ext = Gitlab::ReferenceExtractor.new(project, current_user)
     ext.analyze("#{title}\n#{description}")
 
-    ext.issues - closes_issues(current_user)
+    ext.issues - visible_closing_issues_for(current_user)
   end
 
   def target_project_path
@@ -844,7 +859,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_commit_message(include_description: false)
-    closes_issues_references = closes_issues.map do |issue|
+    closes_issues_references = visible_closing_issues_for.map do |issue|
       issue.to_reference(target_project)
     end
 
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index ffa238a63d5..8c4eac3c31d 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -206,7 +206,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def closing_issues
-    @closing_issues ||= closes_issues(current_user)
+    @closing_issues ||= visible_closing_issues_for(current_user)
   end
 
   def pipeline
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index 3d2aea4e9b6..f26e3bee06f 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -25,7 +25,7 @@ module MergeRequests
     def close_issues(merge_request)
       return unless merge_request.target_branch == project.default_branch
 
-      closed_issues = merge_request.closes_issues(current_user)
+      closed_issues = merge_request.visible_closing_issues_for(current_user)
 
       closed_issues.each do |issue|
         if can?(current_user, :update_issue, issue)
diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb
index f2fc13ad028..f6cbe769ef4 100644
--- a/app/services/merge_requests/reopen_service.rb
+++ b/app/services/merge_requests/reopen_service.rb
@@ -14,6 +14,7 @@ module MergeRequests
         merge_request.mark_as_unchecked
         invalidate_cache_counts(merge_request, users: merge_request.assignees)
         merge_request.update_project_counter_caches
+        merge_request.cache_merge_request_closes_issues!(current_user)
       end
 
       merge_request