Merge pull request #9089 from dsander/fix-merge-request-comments

Fix merge request comments on files with multiple commits

5 files changed, 11 insertions, 7 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e8f367c332b..0487cc64745 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -88,6 +88,7 @@ v 7.10.0 (unreleased)
   - Allow user to choose a public email to show on public profile
   - Remove truncation from issue titles on milestone page (Jason Blanchard)
   - Fix stuck Merge Request merging events from old installations (Ben Bodenmiller)
+  - Fix merge request comments on files with multiple commits
 
 v 7.9.3
   - Contains no changes
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index b56f21c7a18..4f42972a4dd 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -101,7 +101,7 @@ module DiffHelper
   end
 
   def line_comments
-    @line_comments ||= @line_notes.group_by(&:line_code)
+    @line_comments ||= @line_notes.select(&:active?).group_by(&:line_code)
   end
 
   def organize_comments(type_left, type_right, line_code_left, line_code_right)
diff --git a/app/models/note.rb b/app/models/note.rb
index fdab4517df3..2cf3fac2def 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -354,7 +354,7 @@ class Note < ActiveRecord::Base
   def set_diff
     # First lets find notes with same diff
     # before iterating over all mr diffs
-    diff = Note.where(noteable_id: self.noteable_id, noteable_type: self.noteable_type, line_code: self.line_code).last.try(:diff)
+    diff = diff_for_line_code unless for_merge_request?
     diff ||= find_diff
 
     self.st_diff = diff.to_hash if diff
@@ -364,6 +364,10 @@ class Note < ActiveRecord::Base
     @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
   end
 
+  def diff_for_line_code
+    Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
+  end
+
   # Check if such line of code exists in merge request diff
   # If exists - its active discussion
   # If not - its outdated diff
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index e691db9c08e..e6dfbfd6511 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -23,7 +23,7 @@
         %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text)
 
     - if @reply_allowed
-      - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
+      - comments = @line_notes.select { |n| n.line_code == line_code && n.active? }.sort_by(&:created_at)
       - unless comments.empty?
         = render "projects/notes/diff_notes_with_reply", notes: comments, line: line.text
 
diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml
index 3561ca49f81..b8068835b3a 100644
--- a/app/views/projects/notes/_discussion.html.haml
+++ b/app/views/projects/notes/_discussion.html.haml
@@ -6,9 +6,8 @@
         = image_tag avatar_icon(note.author_email), class: "avatar s40"
     .timeline-content
       - if note.for_merge_request?
-        - if note.outdated?
-          = render "projects/notes/discussions/outdated", discussion_notes: discussion_notes
-        - else
-          = render "projects/notes/discussions/active", discussion_notes: discussion_notes
+        - (active_notes, outdated_notes) = discussion_notes.partition(&:active?)
+        = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0
+        = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0
       - else
         = render "projects/notes/discussions/commit", discussion_notes: discussion_notes