Merge branch 'discussion-model' into 'master'

Add Discussion model to represent MR/diff discussion

Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/10325.

See merge request !5376

app/assets/javascripts/notes.js.coffee

@@ -162,7 +162,7 @@ class @Notes
         @last_fetched_at = data.last_fetched_at
         @setPollingInterval(data.notes.length)
         $.each notes, (i, note) =>
-          if note.discussion_with_diff_html?
+          if note.discussion_html?
             @renderDiscussionNote(note)
           else
             @renderNote(note)
@@ -251,7 +251,7 @@ class @Notes
       discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']")
     if discussionContainer.length is 0
       # insert the note and the reply button after the temp row
-      row.after note.discussion_html
+      row.after note.diff_discussion_html
 
       # remove the note (will be added again below)
       row.next().find(".note").remove()
@@ -265,7 +265,7 @@ class @Notes
       # Init discussion on 'Discussion' page if it is merge request page
       if $('body').attr('data-page').indexOf('projects:merge_request') is 0
         $('ul.main-notes-list')
-          .append(note.discussion_with_diff_html)
+          .append(note.discussion_html)
           .syntaxHighlight()
     else
       # append new note to all matching discussions

app/controllers/projects/commit_controller.rb

@@ -115,11 +115,11 @@ class Projects::CommitController < Projects::ApplicationController
   end
 
   def define_note_vars
-    @grouped_diff_notes = commit.notes.grouped_diff_notes
+    @grouped_diff_discussions = commit.notes.grouped_diff_discussions
     @notes = commit.notes.non_diff_notes.fresh
 
     Banzai::NoteRenderer.render(
-      @grouped_diff_notes.values.flatten + @notes,
+      @grouped_diff_discussions.values.flat_map(&:notes) + @notes,
       @project,
       current_user,
     )

app/controllers/projects/compare_controller.rb

@@ -54,7 +54,7 @@ class Projects::CompareController < Projects::ApplicationController
       )
 
       @diff_notes_disabled = true
-      @grouped_diff_notes = {}
+      @grouped_diff_discussions = {}
     end
   end
 

app/controllers/projects/merge_requests_controller.rb

@@ -97,7 +97,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     else
       build_merge_request
       @diff_notes_disabled = true
-      @grouped_diff_notes = {}
+      @grouped_diff_discussions = {}
     end
 
     define_commit_vars
@@ -378,7 +378,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     # This is not executed lazily
     @notes = Banzai::NoteRenderer.render(
-      @discussions.flatten,
+      @discussions.flat_map(&:notes),
       @project,
       current_user,
       @path,
@@ -404,10 +404,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     }
 
     @use_legacy_diff_notes = !@merge_request.support_new_diff_notes?
-    @grouped_diff_notes = @merge_request.notes.grouped_diff_notes
+    @grouped_diff_discussions = @merge_request.notes.grouped_diff_discussions
 
     Banzai::NoteRenderer.render(
-      @grouped_diff_notes.values.flatten,
+      @grouped_diff_discussions.values.flat_map(&:notes),
       @project,
       current_user,
       @path,

app/controllers/projects/notes_controller.rb

@@ -73,7 +73,7 @@ class Projects::NotesController < Projects::ApplicationController
   end
   alias_method :awardable, :note
 
-  def note_to_html(note)
+  def note_html(note)
     render_to_string(
       "projects/notes/_note",
       layout: false,
@@ -82,20 +82,20 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def note_to_discussion_html(note)
-    return unless note.diff_note?
+  def diff_discussion_html(discussion)
+    return unless discussion.diff_discussion?
 
     if params[:view] == 'parallel'
-      template = "projects/notes/_diff_notes_with_reply_parallel"
+      template = "discussions/_parallel_diff_discussion"
       locals =
         if params[:line_type] == 'old'
-          { notes_left: [note], notes_right: [] }
+          { discussion_left: discussion, discussion_right: nil }
         else
-          { notes_left: [], notes_right: [note] }
+          { discussion_left: nil, discussion_right: discussion }
         end
     else
-      template = "projects/notes/_diff_notes_with_reply"
-      locals = { notes: [note] }
+      template = "discussions/_diff_discussion"
+      locals = { discussion: discussion }
     end
 
     render_to_string(
@@ -106,14 +106,14 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def note_to_discussion_with_diff_html(note)
-    return unless note.diff_note?
+  def discussion_html(discussion)
+    return unless discussion.diff_discussion?
 
     render_to_string(
-      "projects/notes/_discussion",
+      "discussions/_discussion",
       layout: false,
       formats: [:html],
-      locals: { discussion_notes: [note] }
+      locals: { discussion: discussion }
     )
   end
 
@@ -132,26 +132,33 @@ class Projects::NotesController < Projects::ApplicationController
         valid: true,
         id: note.id,
         discussion_id: note.discussion_id,
-        html: note_to_html(note),
+        html: note_html(note),
         award: false,
-        note: note.note,
-        discussion_html: note_to_discussion_html(note),
-        discussion_with_diff_html: note_to_discussion_with_diff_html(note)
+        note: note.note
       }
 
-      # The discussion_id is used to add the comment to the correct discussion
-      # element on the merge request page. Among other things, the discussion_id
-      # contains the sha of head commit of the merge request.
-      # When new commits are pushed into the merge request after the initial
-      # load of the merge request page, the discussion elements will still have
-      # the old discussion_ids, with the old head commit sha. The new comment,
-      # however, will have the new discussion_id with the new commit sha.
-      # To ensure that these new comments will still end up in the correct
-      # discussion element, we also send the original discussion_id, with the
-      # old commit sha, along, and fall back on this value when no discussion
-      # element with the new discussion_id could be found.
-      if note.new_diff_note? && note.position != note.original_position
-        attrs[:original_discussion_id] = note.original_discussion_id
+      if note.diff_note?
+        discussion = Discussion.new([note])
+
+        attrs.merge!(
+          diff_discussion_html: diff_discussion_html(discussion),
+          discussion_html: discussion_html(discussion)
+        )
+
+        # The discussion_id is used to add the comment to the correct discussion
+        # element on the merge request page. Among other things, the discussion_id
+        # contains the sha of head commit of the merge request.
+        # When new commits are pushed into the merge request after the initial
+        # load of the merge request page, the discussion elements will still have
+        # the old discussion_ids, with the old head commit sha. The new comment,
+        # however, will have the new discussion_id with the new commit sha.
+        # To ensure that these new comments will still end up in the correct
+        # discussion element, we also send the original discussion_id, with the
+        # old commit sha, along, and fall back on this value when no discussion
+        # element with the new discussion_id could be found.
+        if note.new_diff_note? && note.position != note.original_position
+          attrs[:original_discussion_id] = note.original_discussion_id
+        end
       end
 
       attrs

app/helpers/diff_helper.rb

@@ -54,18 +54,20 @@ module DiffHelper
     end
   end
 
-  def organize_comments(left, right)
-    notes_left = notes_right = nil
+  def parallel_diff_discussions(left, right, diff_file)
+    discussion_left = discussion_right = nil
 
-    unless left[:type].nil? && right[:type] == 'new'
-      notes_left = @grouped_diff_notes[left[:line_code]]
+    if left && (left.unchanged? || left.removed?)
+      line_code = diff_file.line_code(left)
+      discussion_left = @grouped_diff_discussions[line_code]
     end
 
-    unless left[:type].nil? && right[:type].nil?
-      notes_right = @grouped_diff_notes[right[:line_code]]
+    if right && right.added?
+      line_code = diff_file.line_code(right)
+      discussion_right = @grouped_diff_discussions[line_code]
     end
 
-    [notes_left, notes_right]
+    [discussion_left, discussion_right]
   end
 
   def inline_diff_btn

app/helpers/notes_helper.rb

 module NotesHelper
-  # Helps to distinguish e.g. commit notes in mr notes list
-  def note_for_main_target?(note)
-    @noteable.class.name == note.noteable_type && !note.diff_note?
-  end
-
   def note_target_fields(note)
     if note.noteable
       hidden_field_tag(:target_type, note.noteable.class.name.underscore) +
@@ -44,8 +39,8 @@ module NotesHelper
     # If we didn't, diff notes that would show for the same line on the changes
     # tab, would show in different discussions on the discussion tab.
     use_legacy_diff_note ||= begin
-      line_diff_notes = @grouped_diff_notes[line_code]
-      line_diff_notes && line_diff_notes.any?(&:legacy_diff_note?)
+      discussion = @grouped_diff_discussions[line_code]
+      discussion && discussion.legacy_diff_discussion?
     end
 
     data = {
@@ -81,22 +76,10 @@ module NotesHelper
     data
   end
 
-  def link_to_reply_discussion(note, line_type = nil)
+  def link_to_reply_discussion(discussion, line_type = nil)
     return unless current_user
 
-    data = {
-      noteable_type: note.noteable_type,
-      noteable_id:   note.noteable_id,
-      commit_id:     note.commit_id,
-      discussion_id: note.discussion_id,
-      line_type:     line_type
-    }
-
-    if note.diff_note?
-      data[:note_type] = note.type
-
-      data.merge!(note.diff_attributes)
-    end
+    data = discussion.reply_attributes.merge(line_type: line_type)
 
     content_tag(:div, class: "discussion-reply-holder") do
       button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
@@ -114,13 +97,13 @@ module NotesHelper
     @max_access_by_user_id[full_key]
   end
 
-  def diff_note_path(note)
-    return unless note.diff_note?
+  def discussion_diff_path(discussion)
+    return unless discussion.diff_discussion?
 
-    if note.for_merge_request? && note.active?
-      diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
-    elsif note.for_commit?
-      namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code)
+    if discussion.for_merge_request? && discussion.active?
+      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+    elsif discussion.for_commit?
+      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
     end
   end
 end

app/models/concerns/note_on_diff.rb

 module NoteOnDiff
   extend ActiveSupport::Concern
 
-  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
-
-  included do
-    delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true
-  end
-
   def diff_note?
     true
   end
@@ -30,23 +24,4 @@ module NoteOnDiff
   def can_be_award_emoji?
     false
   end
-
-  # Returns an array of at most 16 highlighted lines above a diff note
-  def truncated_diff_lines
-    prev_lines = []
-
-    highlighted_diff_lines.each do |line|
-      if line.meta?
-        prev_lines.clear
-      else
-        prev_lines << line
-
-        break if for_line?(line)
-
-        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
-      end
-    end
-
-    prev_lines
-  end
 end

app/models/discussion.rb

+class Discussion
+  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+
+  attr_reader :first_note, :notes
+
+  delegate  :created_at,
+            :project,
+            :author,
+
+            :noteable,
+            :for_commit?,
+            :for_merge_request?,
+
+            :line_code,
+            :diff_file,
+            :for_line?,
+            :active?,
+
+            to: :first_note
+
+  delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true
+
+  def self.for_notes(notes)
+    notes.group_by(&:discussion_id).values.map { |notes| new(notes) }
+  end
+
+  def self.for_diff_notes(notes)
+    notes.group_by(&:line_code).values.map { |notes| new(notes) }
+  end
+
+  def initialize(notes)
+    @first_note = notes.first
+    @notes = notes
+  end
+
+  def id
+    first_note.discussion_id
+  end
+
+  def diff_discussion?
+    first_note.diff_note?
+  end
+
+  def legacy_diff_discussion?
+    notes.any?(&:legacy_diff_note?)
+  end
+
+  def for_target?(target)
+    self.noteable == target && !diff_discussion?
+  end
+
+  def expanded?
+    !diff_discussion? || active?
+  end
+
+  def reply_attributes
+    data = {
+      noteable_type: first_note.noteable_type,
+      noteable_id:   first_note.noteable_id,
+      commit_id:     first_note.commit_id,
+      discussion_id: self.id,
+    }
+
+    if diff_discussion?
+      data[:note_type] = first_note.type
+
+      data.merge!(first_note.diff_attributes)
+    end
+
+    data
+  end
+
+  # Returns an array of at most 16 highlighted lines above a diff note
+  def truncated_diff_lines
+    prev_lines = []
+
+    highlighted_diff_lines.each do |line|
+      if line.meta?
+        prev_lines.clear
+      else
+        prev_lines << line
+
+        break if for_line?(line)
+
+        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
+      end
+    end
+
+    prev_lines
+  end
+end

app/models/note.rb

@@ -82,11 +82,12 @@ class Note < ActiveRecord::Base
     end
 
     def discussions
-      all.group_by(&:discussion_id).values
+      Discussion.for_notes(all)
     end
 
-    def grouped_diff_notes
-      diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code)
+    def grouped_diff_discussions
+      notes = diff_notes.fresh.select(&:active?)
+      Discussion.for_diff_notes(notes).map { |d| [d.line_code, d] }.to_h
     end
 
     # Searches for notes matching the given query.

app/views/discussions/_diff_discussion.html.haml

+%tr.notes_holder
+  %td.notes_line{ colspan: 2 }
+  %td.notes_content
+    %ul.notes{ data: { discussion_id: discussion.id } }
+      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+    = link_to_reply_discussion(discussion)

app/views/projects/notes/discussions/_diff_with_notes.html.haml → app/views/discussions/_diff_with_notes.html.haml

-- note = discussion_notes.first
-- diff_file = note.diff_file
-- return unless diff_file
-
-- blob = note.blob
+- diff_file = discussion.diff_file
+- blob = discussion.blob
 
 .diff-file.file-holder
   .file-title
-    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: note.project, url: diff_note_path(note)
+    = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: discussion.project, url: discussion_diff_path(discussion)
 
   .diff-content.code.js-syntax-highlight
     %table
-      - note.truncated_diff_lines.each do |line|
+      - discussion.truncated_diff_lines.each do |line|
         = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
 
-        - if note.for_line?(line)
-          = render "projects/notes/diff_notes_with_reply", notes: discussion_notes
+        - if discussion.for_line?(line)
+          = render "discussions/diff_discussion", discussion: discussion

app/views/projects/notes/_discussion.html.haml → app/views/discussions/_discussion.html.haml

-- note = discussion_notes.first
-- expanded = !note.diff_note? || note.active?
+- expanded = discussion.expanded?
 %li.note.note-discussion.timeline-entry
   .timeline-entry-inner
     .timeline-icon
-      = link_to user_path(note.author) do
-        = image_tag avatar_icon(note.author), class: "avatar s40"
+      = link_to user_path(discussion.author) do
+        = image_tag avatar_icon(discussion.author), class: "avatar s40"
     .timeline-content
-      .discussion.js-toggle-container{ class: note.discussion_id }
+      .discussion.js-toggle-container{ class: discussion.id }
         .discussion-header
-          = link_to_member(@project, note.author, avatar: false)
+          = link_to_member(@project, discussion.author, avatar: false)
 
           .inline.discussion-headline-light
-            = note.author.to_reference
+            = discussion.author.to_reference
             started a discussion on
 
-            - if note.for_commit?
-              - commit = note.noteable
+            - if discussion.for_commit?
+              - commit = discussion.noteable
               - if commit
                 commit
-                = link_to commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code), class: 'monospace'
+                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code), class: 'monospace'
               - else
                 a deleted commit
             - else
-              - if note.active?
-                = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
+              - if discussion.active?
+                = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) do
                   the diff
               - else
                 an outdated diff
 
-            = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "note-created-ago")
+            = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
 
           .discussion-actions
             = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
@@ -40,7 +39,7 @@
               Toggle discussion
 
         .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
-          - if note.diff_note?
-            = render "projects/notes/discussions/diff_with_notes", discussion_notes: discussion_notes
+          - if discussion.diff_discussion? && discussion.diff_file
+            = render "discussions/diff_with_notes", discussion: discussion
           - else
-            = render "projects/notes/discussions/notes", discussion_notes: discussion_notes
+            = render "discussions/notes", discussion: discussion

app/views/projects/notes/discussions/_notes.html.haml → app/views/discussions/_notes.html.haml

-- note = discussion_notes.first
 .panel.panel-default
-  .notes{ data: { discussion_id: note.discussion_id } }
+  .notes{ data: { discussion_id: discussion.id } }
     %ul.notes.timeline
-      = render partial: "projects/notes/note", collection: discussion_notes, as: :note
-  = link_to_reply_discussion(note)
+      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+  = link_to_reply_discussion(discussion)

app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml → app/views/discussions/_parallel_diff_discussion.html.haml

-- note_left = notes_left.present? ? notes_left.first : nil
-- note_right = notes_right.present? ? notes_right.first : nil
-
 %tr.notes_holder
-  - if note_left
+  - if discussion_left
     %td.notes_line.old
     %td.notes_content.parallel.old
-      %ul.notes{ data: { discussion_id: note_left.discussion_id } }
-        = render partial: "projects/notes/note", collection: notes_left, as: :note
+      %ul.notes{ data: { discussion_id: discussion_left.id } }
+        = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note
 
-      = link_to_reply_discussion(note_left, 'old')
+      = link_to_reply_discussion(discussion_left, 'old')
   - else
     %td.notes_line.old= ""
     %td.notes_content.parallel.old= ""
 
-  - if note_right
+  - if discussion_right
     %td.notes_line.new
     %td.notes_content.parallel.new
-      %ul.notes{ data: { discussion_id: note_right.discussion_id } }
-        = render partial: "projects/notes/note", collection: notes_right, as: :note
+      %ul.notes{ data: { discussion_id: discussion_right.id } }
+        = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note
 
-      = link_to_reply_discussion(note_right, 'new')
+      = link_to_reply_discussion(discussion_right, 'new')
   - else
     %td.notes_line.new= ""
     %td.notes_content.parallel.new= ""

app/views/projects/diffs/_match_line_parallel.html.haml

-%td.old_line.diff-line-num.empty-cell
-%td.line_content.parallel.match= line
-%td.new_line.diff-line-num.empty-cell
-%td.line_content.parallel.match= line

app/views/projects/diffs/_parallel_view.html.haml

@@ -5,32 +5,35 @@
       - left = line[:left]
       - right = line[:right]
       %tr.line_holder.parallel
-        - if left[:type] == 'match'
-          = render "projects/diffs/match_line_parallel", { line: left[:text] }
-        - elsif left[:type] == 'nonewline'
-          %td.old_line.diff-line-num.empty-cell
-          %td.line_content.parallel.match= left[:text]
-          %td.new_line.diff-line-num.empty-cell
-          %td.line_content.parallel.match= left[:text]
+        - if left
+          - if left.meta?
+            %td.old_line.diff-line-num.empty-cell
+            %td.line_content.parallel.match= left.text
+          - else
+            - left_line_code = diff_file.line_code(left)
+            - left_position = diff_file.position(left)
+            %td.old_line.diff-line-num{id: left_line_code, class: left.type, data: { linenumber: left.old_pos }}
+              %a{href: "##{left_line_code}" }= raw(left.old_pos)
+            %td.line_content.parallel.noteable_line{class: left.type, data: diff_view_line_data(left_line_code, left_position, 'old')}= diff_line_content(left.text)
         - else
-          %td.old_line.diff-line-num{id: left[:line_code], class: [left[:type], ('empty-cell' unless left[:number])], data: { linenumber: left[:number] }}
-            %a{href: "##{left[:line_code]}" }= raw(left[:number])
-          %td.line_content.parallel.noteable_line{class: [left[:type], ('empty-cell' if left[:text].empty?)], data: diff_view_line_data(left[:line_code], left[:position], 'old')}= diff_line_content(left[:text])
+          %td.old_line.diff-line-num.empty-cell
+          %td.line_content.parallel
 
-          - if right[:type] == 'new'
-            - new_line_type = 'new'
-            - new_line_code = right[:line_code]
-            - new_position = right[:position]
+        - if right
+          - if right.meta?
+            %td.old_line.diff-line-num.empty-cell
+            %td.line_content.parallel.match= left.text
           - else
-            - new_line_type = nil
-            - new_line_code = left[:line_code]
-            - new_position = left[:position]
-
-          %td.new_line.diff-line-num{id: new_line_code, class: [new_line_type, ('empty-cell' unless right[:number])], data: { linenumber: right[:number] }}
-            %a{href: "##{new_line_code}" }= raw(right[:number])
-          %td.line_content.parallel.noteable_line{class: [new_line_type, ('empty-cell' if right[:text].empty?)], data: diff_view_line_data(new_line_code, new_position, 'new')}= diff_line_content(right[:text])
+            - right_line_code = diff_file.line_code(right)
+            - right_position = diff_file.position(right)
+            %td.new_line.diff-line-num{id: right_line_code, class: right.type, data: { linenumber: right.new_pos }}
+              %a{href: "##{right_line_code}" }= raw(right.new_pos)
+            %td.line_content.parallel.noteable_line{class: right.type, data: diff_view_line_data(right_line_code, right_position, 'new')}= diff_line_content(right.text)
+        - else
+          %td.old_line.diff-line-num.empty-cell
+          %td.line_content.parallel
 
       - unless @diff_notes_disabled
-        - notes_left, notes_right = organize_comments(left, right)
-        - if notes_left.present? || notes_right.present?
-          = render "projects/notes/diff_notes_with_reply_parallel", notes_left: notes_left, notes_right: notes_right
+        - discussion_left, discussion_right = parallel_diff_discussions(left, right, diff_file)
+        - if discussion_left || discussion_right
+          = render "discussions/parallel_diff_discussion", discussion_left: discussion_left, discussion_right: discussion_right

app/views/projects/diffs/_text_file.html.haml

@@ -11,9 +11,9 @@
 
     - unless @diff_notes_disabled
       - line_code = diff_file.line_code(line)
-      - diff_notes = @grouped_diff_notes[line_code] if line_code
-      - if diff_notes
-        = render "projects/notes/diff_notes_with_reply", notes: diff_notes
+      - discussion = @grouped_diff_discussions[line_code] if line_code
+      - if discussion
+        = render "discussions/diff_discussion", discussion: discussion
 
   - if last_line > 0
     = render "projects/diffs/match_line", { line: "",

app/views/projects/notes/_diff_notes_with_reply.html.haml

-- note = notes.first
-%tr.notes_holder
-  %td.notes_line{ colspan: 2 }
-  %td.notes_content
-    %ul.notes{ data: { discussion_id: note.discussion_id } }
-      = render partial: "projects/notes/note", collection: notes, as: :note
-    = link_to_reply_discussion(note)

app/views/projects/notes/_notes.html.haml

 - if @discussions.present?
-  - @discussions.each do |discussion_notes|
-    - note = discussion_notes.first
-    - if note_for_main_target?(note)
-      = render partial: "projects/notes/note", object: note, as: :note
+  - @discussions.each do |discussion|
+    - if discussion.for_target?(@noteable)
+      = render partial: "projects/notes/note", object: discussion.first_note, as: :note
     - else
-      = render 'projects/notes/discussion', discussion_notes: discussion_notes
+      = render 'discussions/discussion', discussion: discussion
 - else
-  - @notes.each do |note|
-    = render partial: "projects/notes/note", object: note, as: :note
+  = render partial: "projects/notes/note", collection: @notes, as: :note

lib/gitlab/diff/parallel_diff.rb

@@ -8,72 +8,35 @@ module Gitlab
       end
 
       def parallelize
-
         i = 0
         free_right_index = nil
 
         lines = []
         highlighted_diff_lines = diff_file.highlighted_diff_lines
         highlighted_diff_lines.each do |line|
-          line_code = diff_file.line_code(line)
-          position = diff_file.position(line)
-
-          case line.type
-          when 'match', nil
+          if line.meta? || line.unchanged?
             # line in the right panel is the same as in the left one
             lines << {
-              left: {
-                type:       line.type,
-                number:     line.old_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              },
-              right: {
-                type:       line.type,
-                number:     line.new_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              }
+              left: line,
+              right: line
             }
 
             free_right_index = nil
             i += 1
-          when 'old'
+          elsif line.removed?
             lines << {
-              left: {
-                type:       line.type,
-                number:     line.old_pos,
-                text:       line.text,
-                line_code:  line_code,
-                position:   position
-              },
-              right: {
-                type:       nil,
-                number:     nil,
-                text:       "",
-                line_code:  line_code,
-                position:   position
-              }
+              left: line,
+              right: nil
             }
 
             # Once we come upon a new line it can be put on the right of this old line
             free_right_index ||= i
             i += 1
-          when 'new'
-            data = {
-              type:       line.type,
-              number:     line.new_pos,
-              text:       line.text,
-              line_code:  line_code,
-              position:   position
-            }
-
+          elsif line.added?
             if free_right_index
               # If an old line came before this without a line on the right, this
               # line can be put to the right of it.
-              lines[free_right_index][:right] = data
+              lines[free_right_index][:right] = line
 
               # If there are any other old lines on the left that don't yet have
               # a new counterpart on the right, update the free_right_index
@@ -81,14 +44,8 @@ module Gitlab
               free_right_index = next_free_right_index < i ? next_free_right_index : nil
             else
               lines << {
-                left: {
-                  type:       nil,
-                  number:     nil,
-                  text:       "",
-                  line_code:  line_code,
-                  position:   position
-                },
-                right: data
+                left: nil,
+                right: line
               }
 
               free_right_index = nil

spec/lib/gitlab/diff/parallel_diff_spec.rb

@@ -11,11 +11,51 @@ describe Gitlab::Diff::ParallelDiff, lib: true do
   let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) }
   subject { described_class.new(diff_file) }
 
-  let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") }
-
   describe '#parallelize' do
     it 'should return an array of arrays containing the parsed diff' do
-      expect(subject.parallelize).to match_array(parallel_diff_result_array)
+      diff_lines = diff_file.highlighted_diff_lines
+      expected = [
+        # Unchanged lines
+        { left: diff_lines[0], right: diff_lines[0] },
+        { left: diff_lines[1], right: diff_lines[1] },
+        { left: diff_lines[2], right: diff_lines[2] },
+        { left: diff_lines[3], right: diff_lines[3] },
+        { left: diff_lines[4], right: diff_lines[5] },
+        { left: diff_lines[6], right: diff_lines[6] },
+        { left: diff_lines[7], right: diff_lines[7] },
+        { left: diff_lines[8], right: diff_lines[8] },
+
+        # Changed lines
+        { left: diff_lines[9], right: diff_lines[11] },
+        { left: diff_lines[10], right: diff_lines[12] },
+
+        # Added lines
+        { left: nil, right: diff_lines[13] },
+        { left: nil, right: diff_lines[14] },
+        { left: nil, right: diff_lines[15] },
+        { left: nil, right: diff_lines[16] },
+        { left: nil, right: diff_lines[17] },
+        { left: nil, right: diff_lines[18] },
+
+        # Unchanged lines
+        { left: diff_lines[19], right: diff_lines[19] },
+        { left: diff_lines[20], right: diff_lines[20] },
+        { left: diff_lines[21], right: diff_lines[21] },
+        { left: diff_lines[22], right: diff_lines[22] },
+        { left: diff_lines[23], right: diff_lines[23] },
+        { left: diff_lines[24], right: diff_lines[24] },
+        { left: diff_lines[25], right: diff_lines[25] },
+
+        # Added line
+        { left: nil, right: diff_lines[26] },
+
+        # Unchanged lines
+        { left: diff_lines[27], right: diff_lines[27] },
+        { left: diff_lines[28], right: diff_lines[28] },
+        { left: diff_lines[29], right: diff_lines[29] }
+      ]
+
+      expect(subject.parallelize).to eq(expected)
     end
   end
 end