diff options
author | Douwe Maan <douwe@selenight.nl> | 2016-06-20 20:15:44 +0300 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2016-07-07 01:50:59 +0300 |
commit | a27462a5c6da0182f6b3a55c9417e6405f2c0415 (patch) | |
tree | 6bdf936568c0f9274cbf24db7f8b6517b2200980 /app | |
parent | 375193455aa5cb752f1035a6cc69160170a58477 (diff) |
Extract parts of LegacyDiffNote into DiffOnNote concern and move part of responsibility to other classes
Diffstat (limited to 'app')
-rw-r--r-- | app/helpers/diff_helper.rb | 4 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 7 | ||||
-rw-r--r-- | app/models/concerns/note_on_diff.rb | 53 | ||||
-rw-r--r-- | app/models/legacy_diff_note.rb | 69 | ||||
-rw-r--r-- | app/models/note.rb | 4 | ||||
-rw-r--r-- | app/models/sent_notification.rb | 16 | ||||
-rw-r--r-- | app/views/projects/diffs/_line.html.haml | 1 | ||||
-rw-r--r-- | app/views/projects/notes/discussions/_diff_with_notes.html.haml | 4 |
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" ? " ".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 |