Commit 140a706e authored by Rémy Coutable's avatar Rémy Coutable

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
parents e82c72d1 664e4c12
...@@ -39,6 +39,7 @@ v 8.10.0 (unreleased) ...@@ -39,6 +39,7 @@ v 8.10.0 (unreleased)
- Bump Rinku to 2.0.0 - Bump Rinku to 2.0.0
- Remove unused front-end variable -> default_issues_tracker - Remove unused front-end variable -> default_issues_tracker
- Better caching of git calls on ProjectsController#show. - 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 API endpoint for a group issues !4520 (mahcsig)
- Add Bugzilla integration !4930 (iamtjg) - Add Bugzilla integration !4930 (iamtjg)
- Instrument Rinku usage - Instrument Rinku usage
......
...@@ -9,7 +9,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -9,7 +9,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check, :edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip :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 :validates_merge_request, only: [:show, :diffs, :commits, :builds]
before_action :define_show_vars, 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] before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check]
...@@ -315,10 +314,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -315,10 +314,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
alias_method :issuable, :merge_request alias_method :issuable, :merge_request
alias_method :awardable, :merge_request alias_method :awardable, :merge_request
def closes_issues
@closes_issues ||= @merge_request.closes_issues
end
def authorize_update_merge_request! def authorize_update_merge_request!
return render_404 unless can?(current_user, :update_merge_request, @merge_request) return render_404 unless can?(current_user, :update_merge_request, @merge_request)
end end
...@@ -387,7 +382,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -387,7 +382,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def define_widget_vars def define_widget_vars
@pipeline = @merge_request.pipeline @pipeline = @merge_request.pipeline
@pipelines = [@pipeline].compact @pipelines = [@pipeline].compact
closes_issues
end end
def invalid_mr def invalid_mr
......
...@@ -55,6 +55,10 @@ module MergeRequestsHelper ...@@ -55,6 +55,10 @@ module MergeRequestsHelper
end.sort.to_sentence end.sort.to_sentence
end end
def mr_closes_issues
@mr_closes_issues ||= @merge_request.closes_issues
end
def mr_change_branches_path(merge_request) def mr_change_branches_path(merge_request)
new_namespace_project_merge_request_path( new_namespace_project_merge_request_path(
@project.namespace, @project, @project.namespace, @project,
......
...@@ -22,10 +22,10 @@ ...@@ -22,10 +22,10 @@
- elsif @merge_request.can_be_merged? - elsif @merge_request.can_be_merged?
= render 'projects/merge_requests/widget/open/accept' = render 'projects/merge_requests/widget/open/accept'
- if @closes_issues.present? - if mr_closes_issues.present?
.mr-widget-footer .mr-widget-footer
%span %span
%i.fa.fa-check %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 = 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment