From 79214be727aaa0704a1be5b50aa6dd3011629bc2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 20 Jul 2016 16:18:18 -0600 Subject: [PATCH] Add Discussion model to represent MR/diff discussion --- app/assets/javascripts/notes.js.coffee | 6 +- app/controllers/projects/commit_controller.rb | 4 +- .../projects/compare_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 8 +- app/controllers/projects/notes_controller.rb | 65 +- app/helpers/diff_helper.rb | 16 +- app/helpers/notes_helper.rb | 37 +- app/models/concerns/note_on_diff.rb | 25 - app/models/discussion.rb | 91 ++ app/models/note.rb | 7 +- .../discussions/_diff_discussion.html.haml | 6 + .../discussions/_diff_with_notes.html.haml | 14 + app/views/discussions/_discussion.html.haml | 45 + app/views/discussions/_notes.html.haml | 5 + .../_parallel_diff_discussion.html.haml | 22 + .../diffs/_match_line_parallel.html.haml | 4 - .../projects/diffs/_parallel_view.html.haml | 53 +- app/views/projects/diffs/_text_file.html.haml | 6 +- .../notes/_diff_notes_with_reply.html.haml | 7 - .../_diff_notes_with_reply_parallel.html.haml | 25 - .../projects/notes/_discussion.html.haml | 46 - app/views/projects/notes/_notes.html.haml | 12 +- .../discussions/_diff_with_notes.html.haml | 17 - .../notes/discussions/_notes.html.haml | 6 - lib/gitlab/diff/parallel_diff.rb | 63 +- spec/fixtures/parallel_diff_result.yml | 800 ------------------ spec/lib/gitlab/diff/parallel_diff_spec.rb | 46 +- 27 files changed, 341 insertions(+), 1097 deletions(-) create mode 100644 app/models/discussion.rb create mode 100644 app/views/discussions/_diff_discussion.html.haml create mode 100644 app/views/discussions/_diff_with_notes.html.haml create mode 100644 app/views/discussions/_discussion.html.haml create mode 100644 app/views/discussions/_notes.html.haml create mode 100644 app/views/discussions/_parallel_diff_discussion.html.haml delete mode 100644 app/views/projects/diffs/_match_line_parallel.html.haml delete mode 100644 app/views/projects/notes/_diff_notes_with_reply.html.haml delete mode 100644 app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml delete mode 100644 app/views/projects/notes/_discussion.html.haml delete mode 100644 app/views/projects/notes/discussions/_diff_with_notes.html.haml delete mode 100644 app/views/projects/notes/discussions/_notes.html.haml delete mode 100644 spec/fixtures/parallel_diff_result.yml diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index 0ea54faae1a..d4de712f88c 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -162,7 +162,7 @@ class @Notes @last_fetched_at = data.last_fetched_at @setPollingInterval(data.notes.length) $.each notes, (i, note) => - if note.discussion_with_diff_html? + if note.discussion_html? @renderDiscussionNote(note) else @renderNote(note) @@ -251,7 +251,7 @@ class @Notes discussionContainer = $(".notes[data-discussion-id='" + note.original_discussion_id + "']") if discussionContainer.length is 0 # insert the note and the reply button after the temp row - row.after note.discussion_html + row.after note.diff_discussion_html # remove the note (will be added again below) row.next().find(".note").remove() @@ -265,7 +265,7 @@ class @Notes # Init discussion on 'Discussion' page if it is merge request page if $('body').attr('data-page').indexOf('projects:merge_request') is 0 $('ul.main-notes-list') - .append(note.discussion_with_diff_html) + .append(note.discussion_html) .syntaxHighlight() else # append new note to all matching discussions diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 727e84b40a1..7ae034f9398 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -115,11 +115,11 @@ class Projects::CommitController < Projects::ApplicationController end def define_note_vars - @grouped_diff_notes = commit.notes.grouped_diff_notes + @grouped_diff_discussions = commit.notes.grouped_diff_discussions @notes = commit.notes.non_diff_notes.fresh Banzai::NoteRenderer.render( - @grouped_diff_notes.values.flatten + @notes, + @grouped_diff_discussions.values.flat_map(&:notes) + @notes, @project, current_user, ) diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 10749d0fef8..8c004724f02 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -54,7 +54,7 @@ class Projects::CompareController < Projects::ApplicationController ) @diff_notes_disabled = true - @grouped_diff_notes = {} + @grouped_diff_discussions = {} end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7beeb7d97d0..594a61464b9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -97,7 +97,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController else build_merge_request @diff_notes_disabled = true - @grouped_diff_notes = {} + @grouped_diff_discussions = {} end define_commit_vars @@ -378,7 +378,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController # This is not executed lazily @notes = Banzai::NoteRenderer.render( - @discussions.flatten, + @discussions.flat_map(&:notes), @project, current_user, @path, @@ -404,10 +404,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController } @use_legacy_diff_notes = !@merge_request.support_new_diff_notes? - @grouped_diff_notes = @merge_request.notes.grouped_diff_notes + @grouped_diff_discussions = @merge_request.notes.grouped_diff_discussions Banzai::NoteRenderer.render( - @grouped_diff_notes.values.flatten, + @grouped_diff_discussions.values.flat_map(&:notes), @project, current_user, @path, diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 3eacdbbd067..766b7e9cf22 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -73,7 +73,7 @@ class Projects::NotesController < Projects::ApplicationController end alias_method :awardable, :note - def note_to_html(note) + def note_html(note) render_to_string( "projects/notes/_note", layout: false, @@ -82,20 +82,20 @@ class Projects::NotesController < Projects::ApplicationController ) end - def note_to_discussion_html(note) - return unless note.diff_note? + def diff_discussion_html(discussion) + return unless discussion.diff_discussion? if params[:view] == 'parallel' - template = "projects/notes/_diff_notes_with_reply_parallel" + template = "discussions/_parallel_diff_discussion" locals = if params[:line_type] == 'old' - { notes_left: [note], notes_right: [] } + { discussion_left: discussion, discussion_right: nil } else - { notes_left: [], notes_right: [note] } + { discussion_left: nil, discussion_right: discussion } end else - template = "projects/notes/_diff_notes_with_reply" - locals = { notes: [note] } + template = "discussions/_diff_discussion" + locals = { discussion: discussion } end render_to_string( @@ -106,14 +106,14 @@ class Projects::NotesController < Projects::ApplicationController ) end - def note_to_discussion_with_diff_html(note) - return unless note.diff_note? + def discussion_html(discussion) + return unless discussion.diff_discussion? render_to_string( - "projects/notes/_discussion", + "discussions/_discussion", layout: false, formats: [:html], - locals: { discussion_notes: [note] } + locals: { discussion: discussion } ) end @@ -132,26 +132,33 @@ class Projects::NotesController < Projects::ApplicationController valid: true, id: note.id, discussion_id: note.discussion_id, - html: note_to_html(note), + html: note_html(note), award: false, - note: note.note, - discussion_html: note_to_discussion_html(note), - discussion_with_diff_html: note_to_discussion_with_diff_html(note) + note: note.note } - # The discussion_id is used to add the comment to the correct discussion - # element on the merge request page. Among other things, the discussion_id - # contains the sha of head commit of the merge request. - # When new commits are pushed into the merge request after the initial - # load of the merge request page, the discussion elements will still have - # the old discussion_ids, with the old head commit sha. The new comment, - # however, will have the new discussion_id with the new commit sha. - # To ensure that these new comments will still end up in the correct - # discussion element, we also send the original discussion_id, with the - # old commit sha, along, and fall back on this value when no discussion - # element with the new discussion_id could be found. - if note.new_diff_note? && note.position != note.original_position - attrs[:original_discussion_id] = note.original_discussion_id + if note.diff_note? + discussion = Discussion.new([note]) + + attrs.merge!( + diff_discussion_html: diff_discussion_html(discussion), + discussion_html: discussion_html(discussion) + ) + + # The discussion_id is used to add the comment to the correct discussion + # element on the merge request page. Among other things, the discussion_id + # contains the sha of head commit of the merge request. + # When new commits are pushed into the merge request after the initial + # load of the merge request page, the discussion elements will still have + # the old discussion_ids, with the old head commit sha. The new comment, + # however, will have the new discussion_id with the new commit sha. + # To ensure that these new comments will still end up in the correct + # discussion element, we also send the original discussion_id, with the + # old commit sha, along, and fall back on this value when no discussion + # element with the new discussion_id could be found. + if note.new_diff_note? && note.position != note.original_position + attrs[:original_discussion_id] = note.original_discussion_id + end end attrs diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 75b029365f9..4c031942793 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -54,18 +54,20 @@ module DiffHelper end end - def organize_comments(left, right) - notes_left = notes_right = nil + def parallel_diff_discussions(left, right, diff_file) + discussion_left = discussion_right = nil - unless left[:type].nil? && right[:type] == 'new' - notes_left = @grouped_diff_notes[left[:line_code]] + if left && (left.unchanged? || left.removed?) + line_code = diff_file.line_code(left) + discussion_left = @grouped_diff_discussions[line_code] end - unless left[:type].nil? && right[:type].nil? - notes_right = @grouped_diff_notes[right[:line_code]] + if right && right.added? + line_code = diff_file.line_code(right) + discussion_right = @grouped_diff_discussions[line_code] end - [notes_left, notes_right] + [discussion_left, discussion_right] end def inline_diff_btn diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 98143dcee9b..0f60dd828ab 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -1,9 +1,4 @@ module NotesHelper - # Helps to distinguish e.g. commit notes in mr notes list - def note_for_main_target?(note) - @noteable.class.name == note.noteable_type && !note.diff_note? - end - def note_target_fields(note) if note.noteable hidden_field_tag(:target_type, note.noteable.class.name.underscore) + @@ -44,8 +39,8 @@ module NotesHelper # If we didn't, diff notes that would show for the same line on the changes # tab, would show in different discussions on the discussion tab. use_legacy_diff_note ||= begin - line_diff_notes = @grouped_diff_notes[line_code] - line_diff_notes && line_diff_notes.any?(&:legacy_diff_note?) + discussion = @grouped_diff_discussions[line_code] + discussion && discussion.legacy_diff_discussion? end data = { @@ -81,22 +76,10 @@ module NotesHelper data end - def link_to_reply_discussion(note, line_type = nil) + def link_to_reply_discussion(discussion, line_type = nil) return unless current_user - data = { - noteable_type: note.noteable_type, - noteable_id: note.noteable_id, - commit_id: note.commit_id, - discussion_id: note.discussion_id, - line_type: line_type - } - - if note.diff_note? - data[:note_type] = note.type - - data.merge!(note.diff_attributes) - end + data = discussion.reply_attributes.merge(line_type: line_type) content_tag(:div, class: "discussion-reply-holder") do button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', @@ -114,13 +97,13 @@ module NotesHelper @max_access_by_user_id[full_key] end - def diff_note_path(note) - return unless note.diff_note? + def discussion_diff_path(discussion) + return unless discussion.diff_discussion? - if note.for_merge_request? && note.active? - diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) - elsif note.for_commit? - namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) + if discussion.for_merge_request? && discussion.active? + diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) + elsif discussion.for_commit? + namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) end end end diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb index 2785fbb21c9..4be6a2f621b 100644 --- a/app/models/concerns/note_on_diff.rb +++ b/app/models/concerns/note_on_diff.rb @@ -1,12 +1,6 @@ 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 @@ -30,23 +24,4 @@ module NoteOnDiff def can_be_award_emoji? false end - - # Returns an array of at most 16 highlighted lines above a diff note - def truncated_diff_lines - prev_lines = [] - - highlighted_diff_lines.each do |line| - if line.meta? - prev_lines.clear - 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/discussion.rb b/app/models/discussion.rb new file mode 100644 index 00000000000..74facfd1c9c --- /dev/null +++ b/app/models/discussion.rb @@ -0,0 +1,91 @@ +class Discussion + NUMBER_OF_TRUNCATED_DIFF_LINES = 16 + + attr_reader :first_note, :notes + + delegate :created_at, + :project, + :author, + + :noteable, + :for_commit?, + :for_merge_request?, + + :line_code, + :diff_file, + :for_line?, + :active?, + + to: :first_note + + delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true + + def self.for_notes(notes) + notes.group_by(&:discussion_id).values.map { |notes| new(notes) } + end + + def self.for_diff_notes(notes) + notes.group_by(&:line_code).values.map { |notes| new(notes) } + end + + def initialize(notes) + @first_note = notes.first + @notes = notes + end + + def id + first_note.discussion_id + end + + def diff_discussion? + first_note.diff_note? + end + + def legacy_diff_discussion? + notes.any?(&:legacy_diff_note?) + end + + def for_target?(target) + self.noteable == target && !diff_discussion? + end + + def expanded? + !diff_discussion? || active? + end + + def reply_attributes + data = { + noteable_type: first_note.noteable_type, + noteable_id: first_note.noteable_id, + commit_id: first_note.commit_id, + discussion_id: self.id, + } + + if diff_discussion? + data[:note_type] = first_note.type + + data.merge!(first_note.diff_attributes) + end + + data + end + + # Returns an array of at most 16 highlighted lines above a diff note + def truncated_diff_lines + prev_lines = [] + + highlighted_diff_lines.each do |line| + if line.meta? + prev_lines.clear + 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/note.rb b/app/models/note.rb index 0ce10c77de9..9b0a7211b4e 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -82,11 +82,12 @@ class Note < ActiveRecord::Base end def discussions - all.group_by(&:discussion_id).values + Discussion.for_notes(all) end - def grouped_diff_notes - diff_notes.select(&:active?).sort_by(&:created_at).group_by(&:line_code) + def grouped_diff_discussions + notes = diff_notes.fresh.select(&:active?) + Discussion.for_diff_notes(notes).map { |d| [d.line_code, d] }.to_h end # Searches for notes matching the given query. diff --git a/app/views/discussions/_diff_discussion.html.haml b/app/views/discussions/_diff_discussion.html.haml new file mode 100644 index 00000000000..fa1ad9efa73 --- /dev/null +++ b/app/views/discussions/_diff_discussion.html.haml @@ -0,0 +1,6 @@ +%tr.notes_holder + %td.notes_line{ colspan: 2 } + %td.notes_content + %ul.notes{ data: { discussion_id: discussion.id } } + = render partial: "projects/notes/note", collection: discussion.notes, as: :note + = link_to_reply_discussion(discussion) diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml new file mode 100644 index 00000000000..02b159ffd45 --- /dev/null +++ b/app/views/discussions/_diff_with_notes.html.haml @@ -0,0 +1,14 @@ +- diff_file = discussion.diff_file +- blob = discussion.blob + +.diff-file.file-holder + .file-title + = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: discussion.project, url: discussion_diff_path(discussion) + + .diff-content.code.js-syntax-highlight + %table + - discussion.truncated_diff_lines.each do |line| + = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true + + - if discussion.for_line?(line) + = render "discussions/diff_discussion", discussion: discussion diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml new file mode 100644 index 00000000000..49702e048aa --- /dev/null +++ b/app/views/discussions/_discussion.html.haml @@ -0,0 +1,45 @@ +- expanded = discussion.expanded? +%li.note.note-discussion.timeline-entry + .timeline-entry-inner + .timeline-icon + = link_to user_path(discussion.author) do + = image_tag avatar_icon(discussion.author), class: "avatar s40" + .timeline-content + .discussion.js-toggle-container{ class: discussion.id } + .discussion-header + = link_to_member(@project, discussion.author, avatar: false) + + .inline.discussion-headline-light + = discussion.author.to_reference + started a discussion on + + - if discussion.for_commit? + - commit = discussion.noteable + - if commit + commit + = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code), class: 'monospace' + - else + a deleted commit + - else + - if discussion.active? + = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) do + the diff + - else + an outdated diff + + = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago") + + .discussion-actions + = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do + - if expanded + = icon("chevron-up") + - else + = icon("chevron-down") + + Toggle discussion + + .discussion-body.js-toggle-content{ class: ("hide" unless expanded) } + - if discussion.diff_discussion? && discussion.diff_file + = render "discussions/diff_with_notes", discussion: discussion + - else + = render "discussions/notes", discussion: discussion diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml new file mode 100644 index 00000000000..a2642b839f6 --- /dev/null +++ b/app/views/discussions/_notes.html.haml @@ -0,0 +1,5 @@ +.panel.panel-default + .notes{ data: { discussion_id: discussion.id } } + %ul.notes.timeline + = render partial: "projects/notes/note", collection: discussion.notes, as: :note + = link_to_reply_discussion(discussion) diff --git a/app/views/discussions/_parallel_diff_discussion.html.haml b/app/views/discussions/_parallel_diff_discussion.html.haml new file mode 100644 index 00000000000..a798c438ea0 --- /dev/null +++ b/app/views/discussions/_parallel_diff_discussion.html.haml @@ -0,0 +1,22 @@ +%tr.notes_holder + - if discussion_left + %td.notes_line.old + %td.notes_content.parallel.old + %ul.notes{ data: { discussion_id: discussion_left.id } } + = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note + + = link_to_reply_discussion(discussion_left, 'old') + - else + %td.notes_line.old= "" + %td.notes_content.parallel.old= "" + + - if discussion_right + %td.notes_line.new + %td.notes_content.parallel.new + %ul.notes{ data: { discussion_id: discussion_right.id } } + = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note + + = link_to_reply_discussion(discussion_right, 'new') + - else + %td.notes_line.new= "" + %td.notes_content.parallel.new= "" diff --git a/app/views/projects/diffs/_match_line_parallel.html.haml b/app/views/projects/diffs/_match_line_parallel.html.haml deleted file mode 100644 index b9c0d9dcdfd..00000000000 --- a/app/views/projects/diffs/_match_line_parallel.html.haml +++ /dev/null @@ -1,4 +0,0 @@ -%td.old_line.diff-line-num.empty-cell -%td.line_content.parallel.match= line -%td.new_line.diff-line-num.empty-cell -%td.line_content.parallel.match= line diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml index d208fcee10b..7f30faa20d8 100644 --- a/app/views/projects/diffs/_parallel_view.html.haml +++ b/app/views/projects/diffs/_parallel_view.html.haml @@ -5,32 +5,35 @@ - left = line[:left] - right = line[:right] %tr.line_holder.parallel - - if left[:type] == 'match' - = render "projects/diffs/match_line_parallel", { line: left[:text] } - - elsif left[:type] == 'nonewline' - %td.old_line.diff-line-num.empty-cell - %td.line_content.parallel.match= left[:text] - %td.new_line.diff-line-num.empty-cell - %td.line_content.parallel.match= left[:text] - - else - %td.old_line.diff-line-num{id: left[:line_code], class: [left[:type], ('empty-cell' unless left[:number])], data: { linenumber: left[:number] }} - %a{href: "##{left[:line_code]}" }= raw(left[:number]) - %td.line_content.parallel.noteable_line{class: [left[:type], ('empty-cell' if left[:text].empty?)], data: diff_view_line_data(left[:line_code], left[:position], 'old')}= diff_line_content(left[:text]) - - - if right[:type] == 'new' - - new_line_type = 'new' - - new_line_code = right[:line_code] - - new_position = right[:position] + - if left + - if left.meta? + %td.old_line.diff-line-num.empty-cell + %td.line_content.parallel.match= left.text - else - - new_line_type = nil - - new_line_code = left[:line_code] - - new_position = left[:position] + - left_line_code = diff_file.line_code(left) + - left_position = diff_file.position(left) + %td.old_line.diff-line-num{id: left_line_code, class: left.type, data: { linenumber: left.old_pos }} + %a{href: "##{left_line_code}" }= raw(left.old_pos) + %td.line_content.parallel.noteable_line{class: left.type, data: diff_view_line_data(left_line_code, left_position, 'old')}= diff_line_content(left.text) + - else + %td.old_line.diff-line-num.empty-cell + %td.line_content.parallel - %td.new_line.diff-line-num{id: new_line_code, class: [new_line_type, ('empty-cell' unless right[:number])], data: { linenumber: right[:number] }} - %a{href: "##{new_line_code}" }= raw(right[:number]) - %td.line_content.parallel.noteable_line{class: [new_line_type, ('empty-cell' if right[:text].empty?)], data: diff_view_line_data(new_line_code, new_position, 'new')}= diff_line_content(right[:text]) + - if right + - if right.meta? + %td.old_line.diff-line-num.empty-cell + %td.line_content.parallel.match= left.text + - else + - right_line_code = diff_file.line_code(right) + - right_position = diff_file.position(right) + %td.new_line.diff-line-num{id: right_line_code, class: right.type, data: { linenumber: right.new_pos }} + %a{href: "##{right_line_code}" }= raw(right.new_pos) + %td.line_content.parallel.noteable_line{class: right.type, data: diff_view_line_data(right_line_code, right_position, 'new')}= diff_line_content(right.text) + - else + %td.old_line.diff-line-num.empty-cell + %td.line_content.parallel - unless @diff_notes_disabled - - notes_left, notes_right = organize_comments(left, right) - - if notes_left.present? || notes_right.present? - = render "projects/notes/diff_notes_with_reply_parallel", notes_left: notes_left, notes_right: notes_right + - discussion_left, discussion_right = parallel_diff_discussions(left, right, diff_file) + - if discussion_left || discussion_right + = render "discussions/parallel_diff_discussion", discussion_left: discussion_left, discussion_right: discussion_right diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml index 196f8122db3..5970b9abf2b 100644 --- a/app/views/projects/diffs/_text_file.html.haml +++ b/app/views/projects/diffs/_text_file.html.haml @@ -11,9 +11,9 @@ - unless @diff_notes_disabled - line_code = diff_file.line_code(line) - - diff_notes = @grouped_diff_notes[line_code] if line_code - - if diff_notes - = render "projects/notes/diff_notes_with_reply", notes: diff_notes + - discussion = @grouped_diff_discussions[line_code] if line_code + - if discussion + = render "discussions/diff_discussion", discussion: discussion - if last_line > 0 = render "projects/diffs/match_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 deleted file mode 100644 index ec6c4938efc..00000000000 --- a/app/views/projects/notes/_diff_notes_with_reply.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -- note = notes.first -%tr.notes_holder - %td.notes_line{ colspan: 2 } - %td.notes_content - %ul.notes{ data: { discussion_id: note.discussion_id } } - = render partial: "projects/notes/note", collection: notes, as: :note - = link_to_reply_discussion(note) 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 deleted file mode 100644 index e50a4f86d03..00000000000 --- a/app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml +++ /dev/null @@ -1,25 +0,0 @@ -- note_left = notes_left.present? ? notes_left.first : nil -- note_right = notes_right.present? ? notes_right.first : nil - -%tr.notes_holder - - if note_left - %td.notes_line.old - %td.notes_content.parallel.old - %ul.notes{ data: { discussion_id: note_left.discussion_id } } - = render partial: "projects/notes/note", collection: notes_left, as: :note - - = link_to_reply_discussion(note_left, 'old') - - else - %td.notes_line.old= "" - %td.notes_content.parallel.old= "" - - - if note_right - %td.notes_line.new - %td.notes_content.parallel.new - %ul.notes{ data: { discussion_id: note_right.discussion_id } } - = render partial: "projects/notes/note", collection: notes_right, as: :note - - = link_to_reply_discussion(note_right, 'new') - - else - %td.notes_line.new= "" - %td.notes_content.parallel.new= "" diff --git a/app/views/projects/notes/_discussion.html.haml b/app/views/projects/notes/_discussion.html.haml deleted file mode 100644 index 7869d6413d8..00000000000 --- a/app/views/projects/notes/_discussion.html.haml +++ /dev/null @@ -1,46 +0,0 @@ -- note = discussion_notes.first -- expanded = !note.diff_note? || note.active? -%li.note.note-discussion.timeline-entry - .timeline-entry-inner - .timeline-icon - = link_to user_path(note.author) do - = image_tag avatar_icon(note.author), class: "avatar s40" - .timeline-content - .discussion.js-toggle-container{ class: note.discussion_id } - .discussion-header - = link_to_member(@project, note.author, avatar: false) - - .inline.discussion-headline-light - = note.author.to_reference - started a discussion on - - - if note.for_commit? - - commit = note.noteable - - if commit - commit - = link_to commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code), class: 'monospace' - - else - a deleted commit - - else - - if note.active? - = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do - the diff - - else - an outdated diff - - = time_ago_with_tooltip(note.created_at, placement: "bottom", html_class: "note-created-ago") - - .discussion-actions - = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do - - if expanded - = icon("chevron-up") - - else - = icon("chevron-down") - - Toggle discussion - - .discussion-body.js-toggle-content{ class: ("hide" unless expanded) } - - if note.diff_note? - = render "projects/notes/discussions/diff_with_notes", discussion_notes: discussion_notes - - else - = render "projects/notes/discussions/notes", discussion_notes: discussion_notes diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml index ebf7e8a9cb3..022578bd6db 100644 --- a/app/views/projects/notes/_notes.html.haml +++ b/app/views/projects/notes/_notes.html.haml @@ -1,10 +1,8 @@ - if @discussions.present? - - @discussions.each do |discussion_notes| - - note = discussion_notes.first - - if note_for_main_target?(note) - = render partial: "projects/notes/note", object: note, as: :note + - @discussions.each do |discussion| + - if discussion.for_target?(@noteable) + = render partial: "projects/notes/note", object: discussion.first_note, as: :note - else - = render 'projects/notes/discussion', discussion_notes: discussion_notes + = render 'discussions/discussion', discussion: discussion - else - - @notes.each do |note| - = render partial: "projects/notes/note", object: note, as: :note + = render partial: "projects/notes/note", collection: @notes, as: :note diff --git a/app/views/projects/notes/discussions/_diff_with_notes.html.haml b/app/views/projects/notes/discussions/_diff_with_notes.html.haml deleted file mode 100644 index 4a69b8f8840..00000000000 --- a/app/views/projects/notes/discussions/_diff_with_notes.html.haml +++ /dev/null @@ -1,17 +0,0 @@ -- note = discussion_notes.first -- diff_file = note.diff_file -- return unless diff_file - -- blob = note.blob - -.diff-file.file-holder - .file-title - = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_file.content_commit, project: note.project, url: diff_note_path(note) - - .diff-content.code.js-syntax-highlight - %table - - note.truncated_diff_lines.each do |line| - = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true - - - if note.for_line?(line) - = render "projects/notes/diff_notes_with_reply", notes: discussion_notes diff --git a/app/views/projects/notes/discussions/_notes.html.haml b/app/views/projects/notes/discussions/_notes.html.haml deleted file mode 100644 index a785149549d..00000000000 --- a/app/views/projects/notes/discussions/_notes.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -- note = discussion_notes.first -.panel.panel-default - .notes{ data: { discussion_id: note.discussion_id } } - %ul.notes.timeline - = render partial: "projects/notes/note", collection: discussion_notes, as: :note - = link_to_reply_discussion(note) diff --git a/lib/gitlab/diff/parallel_diff.rb b/lib/gitlab/diff/parallel_diff.rb index b069afdd28c..481536a380b 100644 --- a/lib/gitlab/diff/parallel_diff.rb +++ b/lib/gitlab/diff/parallel_diff.rb @@ -8,72 +8,35 @@ module Gitlab end def parallelize - i = 0 free_right_index = nil lines = [] highlighted_diff_lines = diff_file.highlighted_diff_lines highlighted_diff_lines.each do |line| - line_code = diff_file.line_code(line) - position = diff_file.position(line) - - case line.type - when 'match', nil + if line.meta? || line.unchanged? # line in the right panel is the same as in the left one lines << { - left: { - type: line.type, - number: line.old_pos, - text: line.text, - line_code: line_code, - position: position - }, - right: { - type: line.type, - number: line.new_pos, - text: line.text, - line_code: line_code, - position: position - } + left: line, + right: line } free_right_index = nil i += 1 - when 'old' + elsif line.removed? lines << { - left: { - type: line.type, - number: line.old_pos, - text: line.text, - line_code: line_code, - position: position - }, - right: { - type: nil, - number: nil, - text: "", - line_code: line_code, - position: position - } + left: line, + right: nil } # Once we come upon a new line it can be put on the right of this old line free_right_index ||= i i += 1 - when 'new' - data = { - type: line.type, - number: line.new_pos, - text: line.text, - line_code: line_code, - position: position - } - + elsif line.added? if free_right_index # If an old line came before this without a line on the right, this # line can be put to the right of it. - lines[free_right_index][:right] = data + lines[free_right_index][:right] = line # If there are any other old lines on the left that don't yet have # a new counterpart on the right, update the free_right_index @@ -81,14 +44,8 @@ module Gitlab free_right_index = next_free_right_index < i ? next_free_right_index : nil else lines << { - left: { - type: nil, - number: nil, - text: "", - line_code: line_code, - position: position - }, - right: data + left: nil, + right: line } free_right_index = nil diff --git a/spec/fixtures/parallel_diff_result.yml b/spec/fixtures/parallel_diff_result.yml deleted file mode 100644 index 37066c8e930..00000000000 --- a/spec/fixtures/parallel_diff_result.yml +++ /dev/null @@ -1,800 +0,0 @@ ---- -- :left: - :type: match - :number: 6 - :text: "@@ -6,12 +6,18 @@ module Popen" - :line_code: - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: match - :number: 6 - :text: "@@ -6,12 +6,18 @@ module Popen" - :line_code: - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 6 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 6 - :new_line: 6 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 6 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_6_6 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 6 - :new_line: 6 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 7 - :text: |2 - def popen(cmd, path=nil) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 7 - :new_line: 7 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 7 - :text: |2 - def popen(cmd, path=nil) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 7 - :new_line: 7 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 8 - :text: |2 - unless cmd.is_a?(Array) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 8 - :new_line: 8 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 8 - :text: |2 - unless cmd.is_a?(Array) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_8_8 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 8 - :new_line: 8 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: old - :number: 9 - :text: | - - raise "System commands must be given as an array of strings" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 9 - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 9 - :text: | - + raise RuntimeError, "System commands must be given as an array of strings" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 9 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 10 - :text: |2 - end - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 10 - :new_line: 10 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 10 - :text: |2 - end - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_10 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 10 - :new_line: 10 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 11 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 11 - :new_line: 11 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 11 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_11_11 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 11 - :new_line: 11 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 12 - :text: |2 - path ||= Dir.pwd - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 12 - :new_line: 12 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 12 - :text: |2 - path ||= Dir.pwd - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_12_12 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 12 - :new_line: 12 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: old - :number: 13 - :text: | - - vars = { "PWD" => path } - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_13_13 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 13 - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 13 - :text: | - + - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_13 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 13 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: old - :number: 14 - :text: | - - options = { chdir: path } - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_13 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 14 - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 14 - :text: | - + vars = { - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 14 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 15 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 15 - :text: | - + "PWD" => path - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 15 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 16 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 16 - :text: | - + } - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_16 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 16 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 17 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 17 - :text: | - + - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_17 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 17 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 18 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 18 - :text: | - + options = { - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_18 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 18 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 19 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 19 - :text: | - + chdir: path - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_19 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 19 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 20 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 20 - :text: | - + } - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_20 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 20 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 15 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 15 - :new_line: 21 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 21 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_21 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 15 - :new_line: 21 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 16 - :text: |2 - unless File.directory?(path) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 16 - :new_line: 22 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 22 - :text: |2 - unless File.directory?(path) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_16_22 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 16 - :new_line: 22 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 17 - :text: |2 - FileUtils.mkdir_p(path) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 17 - :new_line: 23 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 23 - :text: |2 - FileUtils.mkdir_p(path) - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_17_23 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 17 - :new_line: 23 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: match - :number: 19 - :text: "@@ -19,6 +25,7 @@ module Popen" - :line_code: - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: match - :number: 25 - :text: "@@ -19,6 +25,7 @@ module Popen" - :line_code: - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 19 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 19 - :new_line: 25 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 25 - :text: |2 - - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_19_25 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 19 - :new_line: 25 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 20 - :text: |2 - @cmd_output = "" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 20 - :new_line: 26 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 26 - :text: |2 - @cmd_output = "" - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_20_26 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 20 - :new_line: 26 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 21 - :text: |2 - @cmd_status = 0 - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 21 - :new_line: 27 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 27 - :text: |2 - @cmd_status = 0 - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_21_27 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 21 - :new_line: 27 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: - :text: '' - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 28 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: new - :number: 28 - :text: | - + - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_28 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: - :new_line: 28 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 22 - :text: |2 - Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 22 - :new_line: 29 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 29 - :text: |2 - Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_22_29 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 22 - :new_line: 29 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 23 - :text: |2 - @cmd_output << stdout.read - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 23 - :new_line: 30 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 30 - :text: |2 - @cmd_output << stdout.read - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_23_30 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 23 - :new_line: 30 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d -- :left: - :type: - :number: 24 - :text: |2 - @cmd_output << stderr.read - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 24 - :new_line: 31 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d - :right: - :type: - :number: 31 - :text: |2 - @cmd_output << stderr.read - :line_code: 2f6fcd96b88b36ce98c38da085c795a27d92a3dd_24_31 - :position: !ruby/object:Gitlab::Diff::Position - attributes: - :old_path: files/ruby/popen.rb - :new_path: files/ruby/popen.rb - :old_line: 24 - :new_line: 31 - :base_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :start_sha: 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - :head_sha: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d diff --git a/spec/lib/gitlab/diff/parallel_diff_spec.rb b/spec/lib/gitlab/diff/parallel_diff_spec.rb index 5f76b70c6f5..2aa5ae44f54 100644 --- a/spec/lib/gitlab/diff/parallel_diff_spec.rb +++ b/spec/lib/gitlab/diff/parallel_diff_spec.rb @@ -11,11 +11,51 @@ describe Gitlab::Diff::ParallelDiff, lib: true do let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: repository) } subject { described_class.new(diff_file) } - let(:parallel_diff_result_array) { YAML.load_file("#{Rails.root}/spec/fixtures/parallel_diff_result.yml") } - describe '#parallelize' do it 'should return an array of arrays containing the parsed diff' do - expect(subject.parallelize).to match_array(parallel_diff_result_array) + diff_lines = diff_file.highlighted_diff_lines + expected = [ + # Unchanged lines + { left: diff_lines[0], right: diff_lines[0] }, + { left: diff_lines[1], right: diff_lines[1] }, + { left: diff_lines[2], right: diff_lines[2] }, + { left: diff_lines[3], right: diff_lines[3] }, + { left: diff_lines[4], right: diff_lines[5] }, + { left: diff_lines[6], right: diff_lines[6] }, + { left: diff_lines[7], right: diff_lines[7] }, + { left: diff_lines[8], right: diff_lines[8] }, + + # Changed lines + { left: diff_lines[9], right: diff_lines[11] }, + { left: diff_lines[10], right: diff_lines[12] }, + + # Added lines + { left: nil, right: diff_lines[13] }, + { left: nil, right: diff_lines[14] }, + { left: nil, right: diff_lines[15] }, + { left: nil, right: diff_lines[16] }, + { left: nil, right: diff_lines[17] }, + { left: nil, right: diff_lines[18] }, + + # Unchanged lines + { left: diff_lines[19], right: diff_lines[19] }, + { left: diff_lines[20], right: diff_lines[20] }, + { left: diff_lines[21], right: diff_lines[21] }, + { left: diff_lines[22], right: diff_lines[22] }, + { left: diff_lines[23], right: diff_lines[23] }, + { left: diff_lines[24], right: diff_lines[24] }, + { left: diff_lines[25], right: diff_lines[25] }, + + # Added line + { left: nil, right: diff_lines[26] }, + + # Unchanged lines + { left: diff_lines[27], right: diff_lines[27] }, + { left: diff_lines[28], right: diff_lines[28] }, + { left: diff_lines[29], right: diff_lines[29] } + ] + + expect(subject.parallelize).to eq(expected) end end end