diff options
| -rw-r--r-- | app/helpers/commits_helper.rb | 58 | ||||
| -rw-r--r-- | app/models/note.rb | 19 | ||||
| -rw-r--r-- | app/views/projects/commits/_text_file.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/notes/_diff_notes_with_reply.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/notes/_discussion.html.haml | 2 | ||||
| -rw-r--r-- | doc/update/5.4-to-6.0.md | 1 | ||||
| -rw-r--r-- | lib/gitlab/diff_parser.rb | 77 | ||||
| -rw-r--r-- | lib/tasks/migrate/migrate_inline_notes.rake | 2 |
8 files changed, 99 insertions, 64 deletions
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index f808ac764d4..ae1021aa52f 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -15,63 +15,9 @@ module CommitsHelper commit_person_link(commit, options.merge(source: :committer)) end - def identification_type(line) - if line[0] == "+" - "new" - elsif line[0] == "-" - "old" - else - nil - end - end - - def build_line_anchor(diff, line_new, line_old) - "#{hexdigest(diff.new_path)}_#{line_old}_#{line_new}" - end - def each_diff_line(diff, index) - diff_arr = diff.diff.lines.to_a - - line_old = 1 - line_new = 1 - type = nil - - lines_arr = ::Gitlab::InlineDiff.processing diff_arr - lines_arr.each do |line| - raw_line = line.dup - - next if line.match(/^\-\-\- \/dev\/null/) - next if line.match(/^\+\+\+ \/dev\/null/) - next if line.match(/^\-\-\- a/) - next if line.match(/^\+\+\+ b/) - - full_line = html_escape(line.gsub(/\n/, '')) - full_line = ::Gitlab::InlineDiff.replace_markers full_line - - if line.match(/^@@ -/) - type = "match" - - line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 - line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 - - next if line_old == 1 && line_new == 1 #top of file - yield(full_line, type, nil, nil, nil) - next - else - type = identification_type(line) - line_code = build_line_anchor(diff, line_new, line_old) - yield(full_line, type, line_code, line_new, line_old, raw_line) - end - - - if line[0] == "+" - line_new += 1 - elsif line[0] == "-" - line_old += 1 - else - line_new += 1 - line_old += 1 - end + Gitlab::DiffParser.new(diff).each do |full_line, type, line_code, line_new, line_old| + yield(full_line, type, line_code, line_new, line_old) end end diff --git a/app/models/note.rb b/app/models/note.rb index 8714db2e10e..697c9d03a93 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -51,7 +51,7 @@ class Note < ActiveRecord::Base scope :inc_author, ->{ includes(:author) } serialize :st_diff - before_create :set_diff, if: ->(n) { n.noteable_type == 'MergeRequest' && n.line_code.present? } + before_create :set_diff, if: ->(n) { n.line_code.present? } def self.create_status_change_note(noteable, author, status) create({ @@ -81,7 +81,7 @@ class Note < ActiveRecord::Base def set_diff # First lets find notes with same diff # before iterating over all mr diffs - diff = self.noteable.notes.where(line_code: self.line_code).last.try(:diff) + diff = Note.where(noteable_id: self.noteable_id, noteable_type: self.noteable_type, line_code: self.line_code).last.try(:diff) diff ||= find_diff self.st_diff = diff.to_hash if diff @@ -91,6 +91,12 @@ class Note < ActiveRecord::Base @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map) end + def active? + # TODO: determine if discussion is outdated + # according to recent MR diff or not + true + end + def diff_file_index line_code.split('_')[0] end @@ -108,10 +114,15 @@ class Note < ActiveRecord::Base end def diff_line + return @diff_line if @diff_line + if diff - @diff_line ||= diff.diff.lines.select { |line| line =~ /\A\+/ }[diff_new_line] || - diff.diff.lines.select { |line| line =~ /\A\-/ }[diff_old_line] + Gitlab::DiffParser.new(diff).each do |full_line, type, line_code, line_new, line_old| + @diff_line = full_line if line_code == self.line_code + end end + + @diff_line end def discussion_id diff --git a/app/views/projects/commits/_text_file.html.haml b/app/views/projects/commits/_text_file.html.haml index 3e9a80325b8..c724213878a 100644 --- a/app/views/projects/commits/_text_file.html.haml +++ b/app/views/projects/commits/_text_file.html.haml @@ -20,4 +20,4 @@ - if @reply_allowed - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at) - unless comments.empty? - = render "projects/notes/diff_notes_with_reply", notes: comments, raw_line: raw_line + = render "projects/notes/diff_notes_with_reply", notes: comments, line: line diff --git a/app/views/projects/notes/_diff_notes_with_reply.html.haml b/app/views/projects/notes/_diff_notes_with_reply.html.haml index 1364ad209b6..9537ab18caa 100644 --- a/app/views/projects/notes/_diff_notes_with_reply.html.haml +++ b/app/views/projects/notes/_diff_notes_with_reply.html.haml @@ -1,6 +1,6 @@ - note = notes.first # example note -# Check if line want not changed since comment was left -- if !defined?(raw_line) || raw_line == note.diff_line +- if !defined?(line) || line == note.diff_line %tr.notes_holder %td.notes_line{ colspan: 2 } %span.btn.disabled diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml index 14d81bbb5ce..5cb1fd0ebf9 100644 --- a/app/views/projects/notes/_discussion.html.haml +++ b/app/views/projects/notes/_discussion.html.haml @@ -36,7 +36,7 @@ ago .discussion-body - if note.for_diff_line? - - if note.diff + - if note.active? .content .file= render "projects/notes/discussion_diff", discussion_notes: discussion_notes, note: note - else diff --git a/doc/update/5.4-to-6.0.md b/doc/update/5.4-to-6.0.md index 37e8983df8f..be6fa98ae7a 100644 --- a/doc/update/5.4-to-6.0.md +++ b/doc/update/5.4-to-6.0.md @@ -61,6 +61,7 @@ sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production sudo -u git -H bundle exec rake migrate_groups RAILS_ENV=production
sudo -u git -H bundle exec rake migrate_global_projects RAILS_ENV=production
sudo -u git -H bundle exec rake migrate_keys RAILS_ENV=production
+sudo -u git -H bundle exec rake migrate_inline_notes RAILS_ENV=production
```
diff --git a/lib/gitlab/diff_parser.rb b/lib/gitlab/diff_parser.rb new file mode 100644 index 00000000000..fb27280c4a4 --- /dev/null +++ b/lib/gitlab/diff_parser.rb @@ -0,0 +1,77 @@ +module Gitlab + class DiffParser + include Enumerable + + attr_reader :lines, :new_path + + def initialize(diff) + @lines = diff.diff.lines.to_a + @new_path = diff.new_path + end + + def each + line_old = 1 + line_new = 1 + type = nil + + lines_arr = ::Gitlab::InlineDiff.processing lines + lines_arr.each do |line| + raw_line = line.dup + + next if line.match(/^\-\-\- \/dev\/null/) + next if line.match(/^\+\+\+ \/dev\/null/) + next if line.match(/^\-\-\- a/) + next if line.match(/^\+\+\+ b/) + + full_line = html_escape(line.gsub(/\n/, '')) + full_line = ::Gitlab::InlineDiff.replace_markers full_line + + if line.match(/^@@ -/) + type = "match" + + line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 + line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 + + next if line_old == 1 && line_new == 1 #top of file + yield(full_line, type, nil, nil, nil) + next + else + type = identification_type(line) + line_code = generate_line_code(new_path, line_new, line_old) + yield(full_line, type, line_code, line_new, line_old, raw_line) + end + + + if line[0] == "+" + line_new += 1 + elsif line[0] == "-" + line_old += 1 + else + line_new += 1 + line_old += 1 + end + end + end + + private + + def identification_type(line) + if line[0] == "+" + "new" + elsif line[0] == "-" + "old" + else + nil + end + end + + def generate_line_code(path, line_new, line_old) + "#{Digest::SHA1.hexdigest(path)}_#{line_old}_#{line_new}" + end + + def html_escape str + replacements = { '&' => '&', '>' => '>', '<' => '<', '"' => '"', "'" => ''' } + str.gsub(/[&"'><]/, replacements) + end + end +end diff --git a/lib/tasks/migrate/migrate_inline_notes.rake b/lib/tasks/migrate/migrate_inline_notes.rake index ec338259abc..e21fa0e8ce8 100644 --- a/lib/tasks/migrate/migrate_inline_notes.rake +++ b/lib/tasks/migrate/migrate_inline_notes.rake @@ -1,6 +1,6 @@ desc "GITLAB | Migrate inline notes" task migrate_inline_notes: :environment do - Note.where(noteable_type: 'MergeRequest').find_each(batch_size: 100) do |note| + Note.where('line_code IS NOT NULL').find_each(batch_size: 100) do |note| begin note.set_diff if note.save |