diff options
author | Douwe Maan <douwe@selenight.nl> | 2016-08-18 18:27:22 -0500 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2016-08-18 19:08:59 -0500 |
commit | 6a355d451eaca2fc10f5fcf31de13b05fa795b9b (patch) | |
tree | f758173d2911fd6ed34698068b90d59e34de5b8c | |
parent | ac89bb0fad1d5820c4defd51cb6ab0626d1fb6ba (diff) | |
download | gitlab-ce-6a355d451eaca2fc10f5fcf31de13b05fa795b9b.tar.gz |
Improve performance of MR show page
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/appearances_helper.rb | 2 | ||||
-rw-r--r-- | app/models/diff_note.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 2 | ||||
-rw-r--r-- | app/models/note.rb | 5 | ||||
-rw-r--r-- | app/views/discussions/_diff_with_notes.html.haml | 13 | ||||
-rw-r--r-- | app/views/notify/repository_push_email.html.haml | 3 | ||||
-rw-r--r-- | app/views/projects/diffs/_line.html.haml | 9 | ||||
-rw-r--r-- | app/views/projects/diffs/_text_file.html.haml | 17 |
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 |