summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-07-08 16:13:08 +0000
committerRémy Coutable <remy@rymai.me>2016-07-08 16:13:08 +0000
commit140a706e96220ef1dab9ba220d86531ca5d46058 (patch)
treef2a19cb19e28efb695181870c980c738795922c2
parente82c72d1f16f42072ca113da022fa5ebe4d0cfa6 (diff)
parent664e4c125e4c2e096fcf8fd7cd538462e6eec841 (diff)
downloadgitlab-ce-140a706e96220ef1dab9ba220d86531ca5d46058.tar.gz
Merge branch 'retrieve-mr-closes-issues-just-when-required' into 'master'
Avoid calculation of closes_issues. ## What does this MR do? Avoid unneeded calls to MR closes issues ## Are there points in the code the reviewer needs to double check? I'm not sure if calling this method from a view is a good practice, but I cannot see another simple way of avoiding this problem. In case we want to avoid this in the controller we need to specify the action, format and status of the merge request, because in that case we know that the `_open` partial will render. We could add some lazy evaluation but it not a thing I see in use along the app but feedback is welcome ## What are the relevant issue numbers? #14202 , #19490 ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~ - ~~[ ] API support added~~ - Tests - ~~[ ] Added for this feature/bug~~ - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !5140
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/projects/merge_requests_controller.rb6
-rw-r--r--app/helpers/merge_requests_helper.rb4
-rw-r--r--app/views/projects/merge_requests/widget/_open.html.haml6
4 files changed, 8 insertions, 9 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 12906398a94..9f3bce52527 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -39,6 +39,7 @@ v 8.10.0 (unreleased)
- Bump Rinku to 2.0.0
- Remove unused front-end variable -> default_issues_tracker
- Better caching of git calls on ProjectsController#show.
+ - Avoid to retrieve MR closes_issues as much as possible.
- Add API endpoint for a group issues !4520 (mahcsig)
- Add Bugzilla integration !4930 (iamtjg)
- Instrument Rinku usage
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 8fda5618818..5678a4015b6 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -9,7 +9,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip
]
- before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
before_action :define_show_vars, only: [:show, :diffs, :commits, :builds]
before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check]
@@ -315,10 +314,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
alias_method :issuable, :merge_request
alias_method :awardable, :merge_request
- def closes_issues
- @closes_issues ||= @merge_request.closes_issues
- end
-
def authorize_update_merge_request!
return render_404 unless can?(current_user, :update_merge_request, @merge_request)
end
@@ -387,7 +382,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_widget_vars
@pipeline = @merge_request.pipeline
@pipelines = [@pipeline].compact
- closes_issues
end
def invalid_mr
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index c7dedfe9254..4da1f4865a4 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -55,6 +55,10 @@ module MergeRequestsHelper
end.sort.to_sentence
end
+ def mr_closes_issues
+ @mr_closes_issues ||= @merge_request.closes_issues
+ end
+
def mr_change_branches_path(merge_request)
new_namespace_project_merge_request_path(
@project.namespace, @project,
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 0e0af57d76e..dc18f715f25 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -22,10 +22,10 @@
- elsif @merge_request.can_be_merged?
= render 'projects/merge_requests/widget/open/accept'
- - if @closes_issues.present?
+ - if mr_closes_issues.present?
.mr-widget-footer
%span
%i.fa.fa-check
- Accepting this merge request will close #{"issue".pluralize(@closes_issues.size)}
+ Accepting this merge request will close #{"issue".pluralize(mr_closes_issues.size)}
= succeed '.' do
- != markdown issues_sentence(@closes_issues), pipeline: :gfm, author: @merge_request.author
+ != markdown issues_sentence(mr_closes_issues), pipeline: :gfm, author: @merge_request.author