diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-06 18:44:40 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-08-06 18:44:40 +0300 |
commit | b4415c01740430cef58baf9bb0cbda2fb1055edb (patch) | |
tree | 89ddade2adcb40d95b710760d4fc806c52f94ada /app | |
parent | 415b2f943ba80ef3b6746af0a98c6dbe062e803c (diff) | |
parent | fdb5f285f924e653400f7bbe18d13718c469d74f (diff) |
Merge branch 'issue_44821' into 'master'
Retrieve merge request closing issues from database cache
Closes #44821
See merge request gitlab-org/gitlab-ce!20911
Diffstat (limited to 'app')
-rw-r--r-- | app/models/merge_request.rb | 21 | ||||
-rw-r--r-- | app/presenters/merge_request_presenter.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/post_merge_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/reopen_service.rb | 1 |
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 |