From 664e4c125e4c2e096fcf8fd7cd538462e6eec841 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Thu, 7 Jul 2016 13:38:41 +0200 Subject: Avoid calculation of closes_issues. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We just need to get merge requests closes issues when we’re going to show them --- app/controllers/projects/merge_requests_controller.rb | 6 ------ 1 file changed, 6 deletions(-) (limited to 'app/controllers/projects/merge_requests_controller.rb') diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index dd86b940a08..c80d38a7889 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] @@ -308,10 +307,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 @@ -377,7 +372,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_widget_vars @pipeline = @merge_request.pipeline @pipelines = [@pipeline].compact - closes_issues end def invalid_mr -- cgit v1.2.1 From 8ab3ab9e0a21c087e90eda485486e4eff905b486 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Thu, 7 Jul 2016 10:32:01 +0200 Subject: Don't render discussion notes when requesting diff tab through AJAX --- .../projects/merge_requests_controller.rb | 54 +++++++++++++--------- 1 file changed, 32 insertions(+), 22 deletions(-) (limited to 'app/controllers/projects/merge_requests_controller.rb') diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index df1943dd9bb..8fda5618818 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -53,9 +53,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def show - @note_counts = Note.where(commit_id: @merge_request.commits.map(&:id)). - group(:commit_id).count - respond_to do |format| format.html @@ -80,6 +77,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diffs apply_diff_view_cookie! + @merge_request_diff = @merge_request.merge_request_diff + @commit = @merge_request.diff_head_commit @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit @@ -109,7 +108,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController def commits respond_to do |format| format.html { render 'show' } - format.json { render json: { html: view_to_html_string('projects/merge_requests/show/_commits') } } + format.json do + # Get commits from repository + # or from cache if already merged + @commits = @merge_request.commits + @note_counts = Note.where(commit_id: @commits.map(&:id)). + group(:commit_id).count + + render json: { html: view_to_html_string('projects/merge_requests/show/_commits') } + end end end @@ -340,14 +347,33 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def define_show_vars + @noteable = @merge_request + @commits_count = @merge_request.commits.count + + @pipeline = @merge_request.pipeline + @statuses = @pipeline.statuses if @pipeline + + if @merge_request.locked_long_ago? + @merge_request.unlock_mr + @merge_request.close + end + + if request.format == :html || action_name == 'show' + define_show_html_vars + end + end + + # Discussion tab data is only required on html requests + def define_show_html_vars # Build a note object for comment form - @note = @project.notes.new(noteable: @merge_request) + @note = @project.notes.new(noteable: @noteable) - @discussions = @merge_request.mr_and_commit_notes. + @discussions = @noteable.mr_and_commit_notes. inc_author_project_award_emoji. fresh. discussions + # This is not executed lazily @notes = Banzai::NoteRenderer.render( @discussions.flatten, @project, @@ -356,22 +382,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController @project_wiki, @ref ) - - @noteable = @merge_request - - # Get commits from repository - # or from cache if already merged - @commits = @merge_request.commits - - @merge_request_diff = @merge_request.merge_request_diff - - @pipeline = @merge_request.pipeline - @statuses = @pipeline.statuses if @pipeline - - if @merge_request.locked_long_ago? - @merge_request.unlock_mr - @merge_request.close - end end def define_widget_vars -- cgit v1.2.1 From b6b26692ea44cfeab7e8fd64b7df60852850fce2 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 28 Jun 2016 17:25:32 +0100 Subject: Collapse large diffs by default When rendering a list of diff files, skip those where the diff is over 10 KB and provide an endpoint to render individually instead. --- .../projects/merge_requests_controller.rb | 76 +++++++++++++++------- 1 file changed, 53 insertions(+), 23 deletions(-) (limited to 'app/controllers/projects/merge_requests_controller.rb') diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index df1943dd9bb..3fc5a319c9b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -13,6 +13,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController 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] + before_action :define_commit_vars, only: [:diffs] + before_action :define_diff_comment_vars, only: [:diffs] before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds] # Allow read any merge_request @@ -58,7 +60,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController respond_to do |format| format.html - + format.json do render json: @merge_request end @@ -80,32 +82,32 @@ class Projects::MergeRequestsController < Projects::ApplicationController def diffs apply_diff_view_cookie! - @commit = @merge_request.diff_head_commit - @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit - - @comments_target = { - noteable_type: 'MergeRequest', - noteable_id: @merge_request.id - } - - @use_legacy_diff_notes = !@merge_request.support_new_diff_notes? - @grouped_diff_notes = @merge_request.notes.grouped_diff_notes - - Banzai::NoteRenderer.render( - @grouped_diff_notes.values.flatten, - @project, - current_user, - @path, - @project_wiki, - @ref - ) - respond_to do |format| format.html format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } end end + # With an ID param, loads the MR at that ID. Otherwise, accepts the same params as #new + # and uses that (unsaved) MR. + # + def diff_for_path + if params[:id] + merge_request + define_diff_comment_vars + else + build_merge_request + @diff_notes_disabled = true + @grouped_diff_notes = {} + end + + define_commit_vars + diffs = @merge_request.diffs(diff_options.merge(paths: [params[:path]])) + diff_refs = @merge_request.diff_refs + + render_diff_for_path(diffs, diff_refs, @merge_request.project) + end + def commits respond_to do |format| format.html { render 'show' } @@ -121,8 +123,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def new - params[:merge_request] ||= ActionController::Parameters.new(source_project: @project) - @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute + build_merge_request @noteable = @merge_request @target_branches = if @merge_request.target_project @@ -380,6 +381,30 @@ class Projects::MergeRequestsController < Projects::ApplicationController closes_issues end + def define_commit_vars + @commit = @merge_request.diff_head_commit + @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit + end + + def define_diff_comment_vars + @comments_target = { + noteable_type: 'MergeRequest', + noteable_id: @merge_request.id + } + + @use_legacy_diff_notes = !@merge_request.support_new_diff_notes? + @grouped_diff_notes = @merge_request.notes.grouped_diff_notes + + Banzai::NoteRenderer.render( + @grouped_diff_notes.values.flatten, + @project, + current_user, + @path, + @project_wiki, + @ref + ) + end + def invalid_mr # Render special view for MR with removed source or target branch render 'invalid' @@ -408,4 +433,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController params[:merge_when_build_succeeds].present? && @merge_request.pipeline && @merge_request.pipeline.active? end + + def build_merge_request + params[:merge_request] ||= ActionController::Parameters.new(source_project: @project) + @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params).execute + end end -- cgit v1.2.1 From ff55398aafa2feccaba4ed470becabc526b4df48 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 6 Jul 2016 18:15:27 +0100 Subject: DRY up diff_for_path actions 1. Move render method to a concern, not a helper. 2. Let DiffHelper#diff_options automatically add the path option. 3. Move more instance var definitions to before filters. --- app/controllers/projects/merge_requests_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/controllers/projects/merge_requests_controller.rb') diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3fc5a319c9b..31d7c324e55 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -1,5 +1,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController include ToggleSubscriptionAction + include DiffForPath include DiffHelper include IssuableActions include ToggleAwardEmoji @@ -102,10 +103,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController end define_commit_vars - diffs = @merge_request.diffs(diff_options.merge(paths: [params[:path]])) - diff_refs = @merge_request.diff_refs + diffs = @merge_request.diffs(diff_options) - render_diff_for_path(diffs, diff_refs, @merge_request.project) + render_diff_for_path(diffs, @merge_request.diff_refs, @merge_request.project) end def commits -- cgit v1.2.1