From 8ab3ab9e0a21c087e90eda485486e4eff905b486 Mon Sep 17 00:00:00 2001
From: Paco Guzman <pacoguzmanp@gmail.com>
Date: Thu, 7 Jul 2016 10:32:01 +0200
Subject: [PATCH] Don't render discussion notes when requesting diff tab
 through AJAX

---
 CHANGELOG                                     |  1 +
 .../projects/merge_requests_controller.rb     | 64 +++++++++++--------
 .../projects/merge_requests/_show.html.haml   |  4 +-
 3 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 7fbfa5e7377..907e1765aa7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -45,6 +45,7 @@ v 8.10.0 (unreleased)
   - RailsCache metris now includes fetch_hit/fetch_miss and read_hit/read_miss info.
   - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w)
   - Set import_url validation to be more strict
+  - Don't render discussion notes when requesting diff tab through AJAX
   - Add basic system information like memory and disk usage to the admin panel
   - Don't garbage collect commits that have related DB records like comments
   - More descriptive message for git hooks and file locks
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,30 +347,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def define_show_vars
-    # Build a note object for comment form
-    @note = @project.notes.new(noteable: @merge_request)
-
-    @discussions = @merge_request.mr_and_commit_notes.
-      inc_author_project_award_emoji.
-      fresh.
-      discussions
-
-    @notes = Banzai::NoteRenderer.render(
-      @discussions.flatten,
-      @project,
-      current_user,
-      @path,
-      @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
+    @commits_count = @merge_request.commits.count
 
     @pipeline = @merge_request.pipeline
     @statuses = @pipeline.statuses if @pipeline
@@ -372,6 +357,31 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       @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: @noteable)
+
+    @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,
+      current_user,
+      @path,
+      @project_wiki,
+      @ref
+    )
   end
 
   def define_widget_vars
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 2ec96308fd7..873ed9b59ee 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -42,7 +42,7 @@
         = succeed '.' do
           = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal"
 
-    - if @commits.present?
+    - if @commits_count.nonzero?
       %ul.merge-request-tabs.nav-links.no-top.no-bottom
         %li.notes-tab
           = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#notes', action: 'notes', toggle: 'tab'} do
@@ -51,7 +51,7 @@
         %li.commits-tab
           = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: 'div#commits', action: 'commits', toggle: 'tab'} do
             Commits
-            %span.badge= @commits.size
+            %span.badge= @commits_count
         - if @pipeline
           %li.builds-tab
             = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: {target: '#builds', action: 'builds', toggle: 'tab'} do