Improve performance of MR show page

9 files changed, 33 insertions, 22 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index eb6a3b843e8..d6ebc8c204f 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -471,7 +471,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     }
 
     @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
-    @grouped_diff_discussions = @merge_request.notes.inc_author_project_award_emoji.grouped_diff_discussions
+    @grouped_diff_discussions = @merge_request.notes.inc_relations_for_view.grouped_diff_discussions
 
     Banzai::NoteRenderer.render(
       @grouped_diff_discussions.values.flat_map(&:notes),
diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb
index e12a1052988..de13e7a1fc2 100644
--- a/app/helpers/appearances_helper.rb
+++ b/app/helpers/appearances_helper.rb
@@ -32,6 +32,8 @@ module AppearancesHelper
   end
 
   def custom_icon(icon_name, size: 16)
+    # We can't simply do the below, because there are some .erb SVGs.
+    #  File.read(Rails.root.join("app/views/shared/icons/_#{icon_name}.svg")).html_safe
     render "shared/icons/#{icon_name}.svg", size: size
   end
 end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 1ca37c4a5bf..f56c3d74ae3 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -4,8 +4,6 @@ class DiffNote < Note
   serialize :original_position, Gitlab::Diff::Position
   serialize :position, Gitlab::Diff::Position
 
-  belongs_to :resolved_by, class_name: "User"
-
   validates :original_position, presence: true
   validates :position, presence: true
   validates :diff_line, presence: true
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 39e4d182bb7..5330a07ee35 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -420,7 +420,7 @@ class MergeRequest < ActiveRecord::Base
 
   def discussions
     @discussions ||= self.mr_and_commit_notes.
-      inc_author_project_award_emoji.
+      inc_relations_for_view.
       fresh.
       discussions
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index 79f7a247fba..3bbf5db0b70 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -25,6 +25,9 @@ class Note < ActiveRecord::Base
   belongs_to :author, class_name: "User"
   belongs_to :updated_by, class_name: "User"
 
+  # Only used by DiffNote, but defined here so that it can be used in `Note.includes`
+  belongs_to :resolved_by, class_name: "User"
+
   has_many :todos, dependent: :destroy
   has_many :events, as: :target, dependent: :destroy
 
@@ -59,7 +62,7 @@ class Note < ActiveRecord::Base
   scope :fresh, ->{ order(created_at: :asc, id: :asc) }
   scope :inc_author_project, ->{ includes(:project, :author) }
   scope :inc_author, ->{ includes(:author) }
-  scope :inc_author_project_award_emoji, ->{ includes(:project, :author, :award_emoji) }
+  scope :inc_relations_for_view, ->{ includes(:project, :author, :updated_by, :resolved_by, :award_emoji) }
 
   scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) }
   scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml
index 81f538a9de1..b2e55f7647a 100644
--- a/app/views/discussions/_diff_with_notes.html.haml
+++ b/app/views/discussions/_diff_with_notes.html.haml
@@ -7,8 +7,11 @@
 
   .diff-content.code.js-syntax-highlight
     %table
-      - discussion.truncated_diff_lines.each do |line|
-        = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
-
-        - if discussion.for_line?(line)
-          = render "discussions/diff_discussion", discussion: discussion, expanded: true
+      - discussions = { discussion.line_code => discussion }
+      = render partial: "projects/diffs/line",
+        collection: discussion.truncated_diff_lines,
+        as: :line,
+        locals: { diff_file: diff_file,
+          discussions: discussions,
+          discussion_expanded: true,
+          plain: true }
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 4dc39a72225..c0c07d65daa 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -75,8 +75,7 @@
             - blob = diff_file.blob
             - if blob && blob.respond_to?(:text?) && blob_text_viewable?(blob)
               %table.code.white
-                - diff_file.highlighted_diff_lines.each do |line|
-                  = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true, email: true
+                = render partial: "projects/diffs/line", collection: diff_file.highlighted_diff_lines, as: :line, locals: { diff_file: diff_file, plain: true, email: true }
             - else
               No preview for this file type
           %br
diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml
index 891b2bd9802..4082f7bba20 100644
--- a/app/views/projects/diffs/_line.html.haml
+++ b/app/views/projects/diffs/_line.html.haml
@@ -1,7 +1,7 @@
 - email = local_assigns.fetch(:email, false)
 - plain = local_assigns.fetch(:plain, false)
 - type = line.type
-- line_code = diff_file.line_code(line) unless plain
+- line_code = diff_file.line_code(line)
 %tr.line_holder{ plain ? { class: type} : { class: type, id: line_code } }
   - case type
   - when 'match'
@@ -28,3 +28,10 @@
         %pre= diff_line_content(line.text, type)
       - else
         = diff_line_content(line.text, type)
+
+- discussions = local_assigns.fetch(:discussions, nil)
+- discussion_expanded = local_assigns.fetch(:discussion_expanded, false)
+- if discussions && !line.meta?
+  - discussion = discussions[line_code]
+  - if discussion
+    = render "discussions/diff_discussion", discussion: discussion, expanded: discussion_expanded
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index ab5463ba89d..da8e343a121 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -5,15 +5,14 @@
 
 %table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' }
   - last_line = 0
-  - diff_file.highlighted_diff_lines.each do |line|
-    - last_line = line.new_pos
-    = render "projects/diffs/line", line: line, diff_file: diff_file
-
-    - unless @diff_notes_disabled
-      - line_code = diff_file.line_code(line)
-      - discussion = @grouped_diff_discussions[line_code] if line_code
-      - if discussion
-        = render "discussions/diff_discussion", discussion: discussion
+  - discussions = @grouped_diff_discussions unless @diff_notes_disabled
+  = render partial: "projects/diffs/line",
+    collection: diff_file.highlighted_diff_lines,
+    as: :line,
+    locals: { diff_file: diff_file,
+      discussions: discussions,
+      plain: true }
 
+  - last_line = diff_file.highlighted_diff_lines.last.new_pos
   - if !diff_file.new_file && last_line > 0
     = diff_match_line last_line, last_line, bottom: true