diff --git a/app/assets/stylesheets/sections/notes.scss b/app/assets/stylesheets/sections/notes.scss index 4e13e30bac80aa85cd3adeebdef80e63b9ea2c31..8df25f53762cbf433f893fec0134d3d9629dda24 100644 --- a/app/assets/stylesheets/sections/notes.scss +++ b/app/assets/stylesheets/sections/notes.scss @@ -90,6 +90,9 @@ ul.notes { border-width: 1px 0; padding-top: 0; vertical-align: top; + &.parallel{ + border-width: 1px; + } } } } diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index afe7447d4e26558c3ab8e60655f0ea4a6cded5d1..cb50d89cba8c79815ae1c21a7939021f2534f572 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -36,7 +36,10 @@ module DiffHelper # Building array of lines # - # [left_type, left_line_number, left_line_content, line_code, right_line_type, right_line_number, right_line_content] + # [ + # left_type, left_line_number, left_line_content, left_line_code, + # right_line_type, right_line_number, right_line_content, right_line_code + # ] # diff_file.diff_lines.each do |line| @@ -49,26 +52,25 @@ module DiffHelper next_line = diff_file.next_line(line.index) if next_line + next_line_code = generate_line_code(diff_file.file_path, next_line) next_type = next_line.type next_line = next_line.text end - line = [type, line_old, full_line, line_code, next_type, line_new] if type == 'match' || type.nil? # line in the right panel is the same as in the left one - line = [type, line_old, full_line, line_code, type, line_new, full_line] + line = [type, line_old, full_line, line_code, type, line_new, full_line, line_code] lines.push(line) elsif type == 'old' if next_type == 'new' # Left side has text removed, right side has text added - line.push(next_line) + line = [type, line_old, full_line, line_code, next_type, line_new, next_line, next_line_code] lines.push(line) skip_next = true elsif next_type == 'old' || next_type.nil? # Left side has text removed, right side doesn't have any change - line.pop # remove the newline - line.push(nil) # no line number on the right panel - line.push(" ") # empty line on the right panel + # No next line code, no new line number, no new line text + line = [type, line_old, full_line, line_code, next_type, nil, " ", nil] lines.push(line) end elsif type == 'new' @@ -78,7 +80,7 @@ module DiffHelper next else # Change is only on the right side, left side has no change - line = [nil, nil, " ", line_code, type, line_new, full_line] + line = [nil, nil, " ", line_code, type, line_new, full_line, line_code] lines.push(line) end end @@ -97,4 +99,22 @@ module DiffHelper line end end + + def line_comments + @line_comments ||= @line_notes.group_by(&:line_code) + end + + def organize_comments(type_left, type_right, line_code_left, line_code_right) + comments_left = comments_right = nil + + unless type_left.nil? && type_right == 'new' + comments_left = line_comments[line_code_left] + end + + unless type_left.nil? && type_right.nil? + comments_right = line_comments[line_code_right] + end + + [comments_left, comments_right] + end end diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index 3ec769e0b8345e8de561f18ac5e653d721bc8d8b..75f3a80f0d7f03be10d9a0a3e5fdf0f53aab4ab6 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -5,22 +5,36 @@ - type_left = line[0] - line_number_left = line[1] - line_content_left = line[2] - - line_code = line[3] + - line_code_left = line[3] - type_right = line[4] - line_number_right = line[5] - line_content_right = line[6] + - line_code_right = line[7] - %tr.line_holder.parallel{id: line_code} + %tr.line_holder.parallel - if type_left == 'match' = render "projects/diffs/match_line_parallel", { line: line_content_left, line_old: line_number_left, line_new: line_number_right } - elsif type_left == 'old' || type_left.nil? - %td.old_line{class: "#{type_left}"} - = link_to raw(line_number_left), "##{line_code}", id: line_code - %td.line_content{class: "parallel noteable_line #{type_left} #{line_code}", "line_code" => line_code }= raw line_content_left - %td.new_line{ class: "#{type_right == 'new' ? 'new' : nil}", data: { linenumber: line_number_right }} - = link_to raw(line_number_right), "##{line_code}", id: line_code - %td.line_content.parallel{class: "noteable_line #{type_right == 'new' ? 'new' : nil} #{line_code}", "line_code" => line_code}= raw line_content_right + %td.old_line{id: line_code_left, class: "#{type_left}"} + = link_to raw(line_number_left), "##{line_code_left}", id: line_code_left + %td.line_content{class: "parallel noteable_line #{type_left} #{line_code_left}", "line_code" => line_code_left }= raw line_content_left + + - if type_right == 'new' + - new_line_class = 'new' + - new_line_code = line_code_right + - else + - new_line_class = nil + - new_line_code = line_code_left + + %td.new_line{id: new_line_code, class: "#{new_line_class}", data: { linenumber: line_number_right }} + = link_to raw(line_number_right), "##{new_line_code}", id: new_line_code + %td.line_content.parallel{class: "noteable_line #{new_line_class} #{new_line_code}", "line_code" => new_line_code}= raw line_content_right + + - if @reply_allowed + - comments_left, comments_right = organize_comments(type_left, type_right, line_code_left, line_code_right) + - if comments_left.present? || comments_right.present? + = render "projects/notes/diff_notes_with_reply_parallel", notes1: comments_left, notes2: comments_right - if diff_file.diff.diff.blank? && diff_file.mode_changed? .file-mode-changed diff --git a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml index 8adf903a9a1b5d8ef18ba8d55dd0163542ad316e..506d1fff0084c092e1822987fb3b0cbb4e7a104c 100644 --- a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml +++ b/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml @@ -1,14 +1,13 @@ -- note1 = notes1.first # example note -- note2 = notes2.first # example note --# Check if line want not changed since comment was left -/- if !defined?(line) || line == note.diff_line +- note1 = notes1.present? ? notes1.first : nil +- note2 = notes2.present? ? notes2.first : nil + %tr.notes_holder - if note1 %td.notes_line %span.btn.disabled %i.icon-comment = notes1.count - %td.notes_content + %td.notes_content.parallel %ul.notes{ rel: note1.discussion_id } = render notes1 @@ -23,7 +22,7 @@ %span.btn.disabled %i.icon-comment = notes2.count - %td.notes_content + %td.notes_content.parallel %ul.notes{ rel: note2.discussion_id } = render notes2 diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 8b6c296dfe61ce3c7cf5c7d6e25b9326ff6c2b80..f8dccc15c0eebe79fdadafe75f468485d102208a 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -147,3 +147,13 @@ Feature: Project Merge Requests And I switch to the diff tab And I unfold diff Then I should see additional file lines + + @javascript + Scenario: I show comments on a merge request side-by-side diff with comments in multiple files + Given project "Shop" have "Bug NS-05" open merge request with diffs inside + And I visit merge request page "Bug NS-05" + And I switch to the diff tab + And I leave a comment like "Line is correct" on line 12 of the first file + And I leave a comment like "Line is wrong" on line 39 of the second file + And I click Side-by-side Diff tab + Then I should see comments on the side-by-side diff page diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 05d3e5067c573a5d37d833f7ca6c0939ebd5a6e0..3ffa3622f4b512d5491ba50f80035bf5887d0028 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -250,6 +250,16 @@ class ProjectMergeRequests < Spinach::FeatureSteps expect(first('.text-file')).to have_content('.bundle') end + step 'I click Side-by-side Diff tab' do + click_link 'Side-by-side Diff' + end + + step 'I should see comments on the side-by-side diff page' do + within '.files [id^=diff]:nth-child(1) .note-text' do + page.should have_visible_content "Line is correct" + end + end + def project @project ||= Project.find_by!(name: "Shop") end diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 4ab415b4ef301e591ed9c3ef5819ca952c4feb2c..b07742a6ee2ac9dd4960c76f7694e0f5d2875aab 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -67,32 +67,33 @@ describe DiffHelper do def parallel_diff_result_array [ - ["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen"], - [nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " "], - [nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, " def popen(cmd, path=nil)"], - [nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, " unless cmd.is_a?(Array)"], - ["old", 9, "- raise <span class='idiff'></span>"System commands must be given as an array of strings"", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+ raise <span class='idiff'>RuntimeError, </span>"System commands must be given as an array of strings""], - [nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, " end"], [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " "], - [nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, " path ||= Dir.pwd"], - ["old", 13, "- vars = { "PWD" => path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, " "], - ["old", 14, "- options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+ vars = {"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+ "PWD" => path"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+ }"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+ options = {"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+ chdir: path"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+ }"], - [nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " "], - [nil, 16, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, " unless File.directory?(path)"], - [nil, 17, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, " FileUtils.mkdir_p(path)"], - ["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen"], - [nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " "], [nil, 20, " @cmd_output = """, "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, " @cmd_output = """], - [nil, 21, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, " @cmd_status = 0"], - [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+"], - [nil, 22, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|"], - [nil, 23, " @cmd_output << stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, " @cmd_output << stdout.read"], - [nil, 24, " @cmd_output << stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, " @cmd_output << stderr.read"] + ["match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", "match", 6, "@@ -6,12 +6,18 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"], + [nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6", nil, 6, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6"], [nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7", nil, 7, " def popen(cmd, path=nil)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"], + [nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8", nil, 8, " unless cmd.is_a?(Array)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8"], + ["old", 9, "- raise <span class='idiff'></span>"System commands must be given as an array of strings"", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9", "new", 9, "+ raise <span class='idiff'>RuntimeError, </span>"System commands must be given as an array of strings"", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"], + [nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10", nil, 10, " end", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10"], + [nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11", nil, 11, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11"], + [nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12", nil, 12, " path ||= Dir.pwd", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12"], + ["old", 13, "- vars = { "PWD" => path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13", "old", nil, " ", nil], + ["old", 14, "- options = { chdir: path }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13", "new", 13, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14", "new", 14, "+ vars = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15", "new", 15, "+ "PWD" => path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16", "new", 16, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17", "new", 17, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18", "new", 18, "+ options = {", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19", "new", 19, "+ chdir: path", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20", "new", 20, "+ }", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20"], + [nil, 15, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21", nil, 21, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21"], + [nil, 16, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22", nil, 22, " unless File.directory?(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22"], + [nil, 17, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23", nil, 23, " FileUtils.mkdir_p(path)", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23"], + ["match", 19, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", "match", 25, "@@ -19,6 +25,7 @@ module Popen", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"], + [nil, 19, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25", nil, 25, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25"], + [nil, 20, " @cmd_output = """, "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26", nil, 26, " @cmd_output = """, "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26"], + [nil, 21, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27", nil, 27, " @cmd_status = 0", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27"], + [nil, nil, " ", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28", "new", 28, "+", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28"], + [nil, 22, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29", nil, 29, " Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29"], + [nil, 23, " @cmd_output << stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30", nil, 30, " @cmd_output << stdout.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30"], + [nil, 24, " @cmd_output << stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31", nil, 31, " @cmd_output << stderr.read", "2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31"] ] end end