Extract parts of LegacyDiffNote into DiffOnNote concern and move part of responsibility to other classes

8 files changed, 82 insertions, 76 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 346b04e40f0..c7c291516fc 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -34,10 +34,6 @@ module DiffHelper
     diffs.decorate! { |diff| Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
   end
 
-  def generate_line_code(file_path, line)
-    Gitlab::Diff::LineCode.generate(file_path, line.new_pos, line.old_pos)
-  end
-
   def unfold_bottom_class(bottom)
     bottom ? 'js-unfold-bottom' : ''
   end
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 1a97f884508..721dfcf265f 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -60,10 +60,9 @@ module NotesHelper
     }
 
     if note.diff_note?
-      data.merge!(
-        line_code: note.line_code,
-        note_type: LegacyDiffNote.name
-      )
+      data[:note_type] = note.type
+
+      data.merge!(note.diff_attributes)
     end
 
     button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb
new file mode 100644
index 00000000000..b511f33b8fa
--- /dev/null
+++ b/app/models/concerns/note_on_diff.rb
@@ -0,0 +1,53 @@
+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
+
+  def diff_file
+    raise NotImplementedError
+  end
+
+  def diff_line
+    raise NotImplementedError
+  end
+
+  def for_line?(line)
+    raise NotImplementedError
+  end
+
+  def diff_attributes
+    raise NotImplementedError
+  end
+
+  def can_be_award_emoji?
+    false
+  end
+
+  def truncated_diff_lines
+    prev_match_line = nil
+    prev_lines = []
+
+    highlighted_diff_lines.each do |line|
+      if line.meta?
+        prev_lines.clear
+        prev_match_line = line
+      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
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 33d2a69ebaf..790dfd4d480 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -1,4 +1,6 @@
 class LegacyDiffNote < Note
+  include NoteOnDiff
+
   serialize :st_diff
 
   validates :line_code, presence: true, line_code: true
@@ -11,12 +13,12 @@ class LegacyDiffNote < Note
     end
   end
 
-  def diff_note?
+  def legacy_diff_note?
     true
   end
 
-  def legacy_diff_note?
-    true
+  def diff_attributes
+    { line_code: line_code }
   end
 
   def discussion_id
@@ -27,61 +29,20 @@ class LegacyDiffNote < Note
     line_code.split('_')[0] if line_code
   end
 
-  def diff_old_line
-    line_code.split('_')[1].to_i if line_code
-  end
-
-  def diff_new_line
-    line_code.split('_')[2].to_i if line_code
-  end
-
   def diff
     @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
   end
 
-  def diff_file_path
-    diff.new_path.presence || diff.old_path
-  end
-
-  def diff_lines
-    @diff_lines ||= Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
+  def diff_file
+    @diff_file ||= Gitlab::Diff::File.new(diff, repository: self.project.repository) if diff
   end
 
   def diff_line
-    @diff_line ||= diff_lines.find { |line| generate_line_code(line) == self.line_code }
+    @diff_line ||= diff_file.line_for_line_code(self.line_code)
   end
 
-  def diff_line_text
-    diff_line.try(:text)
-  end
-
-  def diff_line_type
-    diff_line.try(:type)
-  end
-
-  def highlighted_diff_lines
-    Gitlab::Diff::Highlight.new(diff_lines).highlight
-  end
-
-  def truncated_diff_lines
-    max_number_of_lines = 16
-    prev_match_line = nil
-    prev_lines = []
-
-    highlighted_diff_lines.each do |line|
-      if line.type == "match"
-        prev_lines.clear
-        prev_match_line = line
-      else
-        prev_lines << line
-
-        break if generate_line_code(line) == self.line_code
-
-        prev_lines.shift if prev_lines.length >= max_number_of_lines
-      end
-    end
-
-    prev_lines
+  def for_line?(line)
+    !line.meta? && diff_file.line_code(line) == self.line_code
   end
 
   # Check if this note is part of an "active" discussion
@@ -102,7 +63,7 @@ class LegacyDiffNote < Note
     if noteable_diff
       parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
 
-      @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line_text }
+      @active = parsed_lines.any? { |line_obj| line_obj.text == diff_line.text }
     else
       @active = false
     end
@@ -110,10 +71,6 @@ class LegacyDiffNote < Note
     @active
   end
 
-  def award_emoji_supported?
-    false
-  end
-
   private
 
   def find_diff
@@ -149,10 +106,6 @@ class LegacyDiffNote < Note
     self.class.where(attributes).last.try(:diff)
   end
 
-  def generate_line_code(line)
-    Gitlab::Diff::LineCode.generate(diff_file_path, line.new_pos, line.old_pos)
-  end
-
   # Find the diff on noteable that matches our own
   def find_noteable_diff
     diffs = noteable.diffs(Commit.max_diff_options)
diff --git a/app/models/note.rb b/app/models/note.rb
index 81b5c47b738..0c265064630 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -193,7 +193,7 @@ class Note < ActiveRecord::Base
   end
 
   def award_emoji?
-    award_emoji_supported? && contains_emoji_only?
+    can_be_award_emoji? && contains_emoji_only?
   end
 
   def emoji_awardable?
@@ -204,7 +204,7 @@ class Note < ActiveRecord::Base
     self.line_code = nil if self.line_code.blank?
   end
 
-  def award_emoji_supported?
+  def can_be_award_emoji?
     noteable.is_a?(Awardable)
   end
 
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index a2df899d012..928873fb5c3 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -20,7 +20,7 @@ class SentNotification < ActiveRecord::Base
       find_by(reply_key: reply_key)
     end
 
-    def record(noteable, recipient_id, reply_key, params = {})
+    def record(noteable, recipient_id, reply_key, attrs = {})
       return unless reply_key
 
       noteable_id = nil
@@ -31,7 +31,7 @@ class SentNotification < ActiveRecord::Base
         noteable_id = noteable.id
       end
 
-      params.reverse_merge!(
+      attrs.reverse_merge!(
         project:        noteable.project,
         noteable_type:  noteable.class.name,
         noteable_id:    noteable_id,
@@ -40,13 +40,17 @@ class SentNotification < ActiveRecord::Base
         reply_key:      reply_key
       )
 
-      create(params)
+      create(attrs)
     end
 
-    def record_note(note, recipient_id, reply_key, params = {})
-      params[:line_code] = note.line_code
+    def record_note(note, recipient_id, reply_key, attrs = {})
+      if note.diff_note?
+        attrs[:note_type] = note.type
 
-      record(note.noteable, recipient_id, reply_key, params)
+        attrs.merge!(note.diff_attributes)
+      end
+
+      record(note.noteable, recipient_id, reply_key, attrs)
     end
   end
 
diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml
index f1577e8a47b..dbdbb6eb754 100644
--- a/app/views/projects/diffs/_line.html.haml
+++ b/app/views/projects/diffs/_line.html.haml
@@ -1,3 +1,4 @@
+- line_code = diff_file.line_code(line)
 - type = line.type
 %tr.line_holder{ id: line_code, class: type }
   - case type
diff --git a/app/views/projects/notes/discussions/_diff_with_notes.html.haml b/app/views/projects/notes/discussions/_diff_with_notes.html.haml
index b924ed31b42..3866de0f7fa 100644
--- a/app/views/projects/notes/discussions/_diff_with_notes.html.haml
+++ b/app/views/projects/notes/discussions/_diff_with_notes.html.haml
@@ -12,7 +12,7 @@
     %table
       - note.truncated_diff_lines.each do |line|
         - type = line.type
-        - line_code = generate_line_code(note.diff_file_path, line)
+        - line_code = diff_file.line_code(line)
         %tr.line_holder{ id: line_code, class: "#{type}" }
           - if type == "match"
             %td.old_line.diff-line-num= "..."
@@ -23,5 +23,5 @@
             %td.new_line.diff-line-num{ data: { linenumber: type == "old" ? "&nbsp;".html_safe : line.new_pos } }
             %td.line_content{ class: ['noteable_line', type, line_code], line_code: line_code }= diff_line_content(line.text, type)
 
-            - if line_code == note.line_code
+            - if note.for_line?(line)
               = render "projects/notes/diff_notes_with_reply", notes: discussion_notes