Support commenting on a diff in side-by-side view

Closes https://github.com/gitlabhq/gitlabhq/issues/9283

8 files changed, 82 insertions, 36 deletions

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 21656f59149..1c05a2b9fe8 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -8,11 +8,12 @@
 class @Notes
   @interval: null
 
-  constructor: (notes_url, note_ids, last_fetched_at) ->
+  constructor: (notes_url, note_ids, last_fetched_at, view) ->
     @notes_url = notes_url
     @notes_url = gon.relative_url_root + @notes_url if gon.relative_url_root?
     @note_ids = note_ids
     @last_fetched_at = last_fetched_at
+    @view = view
     @noteable_url = document.URL
     @initRefresh()
     @setupMainTargetNoteForm()
@@ -131,6 +132,8 @@ class @Notes
   isNewNote: (note) ->
     $.inArray(note.id, @note_ids) == -1
 
+  isParallelView: ->
+    @view == 'parallel'
 
   ###
   Render note in discussion area.
@@ -391,6 +394,7 @@ class @Notes
   setupDiscussionNoteForm: (dataHolder, form) =>
     # setup note target
     form.attr "rel", dataHolder.data("discussionId")
+    form.find("#line_type").val dataHolder.data("lineType")
     form.find("#note_commit_id").val dataHolder.data("commitId")
     form.find("#note_line_code").val dataHolder.data("lineCode")
     form.find("#note_noteable_type").val dataHolder.data("noteableType")
@@ -411,19 +415,40 @@ class @Notes
     form = $(".js-new-note-form")
     row = $(link).closest("tr")
     nextRow = row.next()
-
-    # does it already have notes?
-    if nextRow.is(".notes_holder")
-      replyButton = nextRow.find(".js-discussion-reply-button")
-      if replyButton.length > 0
-        $.proxy(@replyToDiscussionNote, replyButton).call()
+    hasNotes = nextRow.is(".notes_holder")
+    addForm = false
+    targetContent = ".notes_content"
+    rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"></td></tr>"
+
+    # In parallel view, look inside the correct left/right pane
+    if @isParallelView()
+      lineType = $(link).data("lineType")
+      targetContent += "." + lineType
+      rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\"></td><td class=\"notes_content parallel old\"></td><td class=\"notes_line\"></td><td class=\"notes_content parallel new\"></td></tr>"
+
+    if hasNotes
+      notesContent = nextRow.find(targetContent)
+      if notesContent.length
+        replyButton = notesContent.find(".js-discussion-reply-button:visible")
+        if replyButton.length
+          e.target = replyButton[0]
+          $.proxy(@replyToDiscussionNote, replyButton[0], e).call()
+        else
+          # In parallel view, the form may not be present in one of the panes
+          noteForm = notesContent.find(".js-discussion-note-form")
+          if noteForm.length == 0
+            addForm = true
     else
       # add a notes row and insert the form
-      row.after "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"></td></tr>"
-      form.clone().appendTo row.next().find(".notes_content")
+      row.after rowCssToAdd
+      addForm = true
+
+    if addForm
+      newForm = form.clone()
+      newForm.appendTo row.next().find(targetContent)
 
       # show the form
-      @setupDiscussionNoteForm $(link), row.next().find("form")
+      @setupDiscussionNoteForm $(link), newForm
 
   ###
   Called in response to "cancel" on a diff note form.
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 496b85cb46d..f3e521adb69 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -77,11 +77,24 @@ class Projects::NotesController < Projects::ApplicationController
   end
 
   def note_to_discussion_html(note)
+    if params[:view] == 'parallel'
+      template = "projects/notes/_diff_notes_with_reply_parallel"
+      locals =
+        if params[:line_type] == 'old'
+          { notes_left: [note], notes_right: [] }
+        else
+          { notes_left: [], notes_right: [note] }
+       end
+    else
+      template = "projects/notes/_diff_notes_with_reply"
+      locals = { notes: [note] }
+    end
+
     render_to_string(
-      "projects/notes/_diff_notes_with_reply",
+      template,
       layout: false,
       formats: [:html],
-      locals: { notes: [note] }
+      locals: locals
     )
   end
 
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 271b53aa2b6..a7c1fa0b071 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -47,7 +47,7 @@ module NotesHelper
     }.to_json
   end
 
-  def link_to_new_diff_note(line_code)
+  def link_to_new_diff_note(line_code, line_type = nil)
     discussion_id = Note.build_discussion_id(
       @comments_target[:noteable_type],
       @comments_target[:noteable_id] || @comments_target[:commit_id],
@@ -59,7 +59,8 @@ module NotesHelper
       noteable_id:   @comments_target[:noteable_id],
       commit_id:     @comments_target[:commit_id],
       line_code:     line_code,
-      discussion_id: discussion_id
+      discussion_id: discussion_id,
+      line_type:     line_type
     }
 
     button_tag(class: 'btn add-diff-note js-add-diff-note-button',
@@ -69,7 +70,7 @@ module NotesHelper
     end
   end
 
-  def link_to_reply_diff(note)
+  def link_to_reply_diff(note, line_type = nil)
     return unless current_user
 
     data = {
@@ -77,7 +78,8 @@ module NotesHelper
       noteable_id:   note.noteable_id,
       commit_id:     note.commit_id,
       line_code:     note.line_code,
-      discussion_id: note.discussion_id
+      discussion_id: note.discussion_id,
+      line_type:     line_type
     }
 
     button_tag class: 'btn reply-btn js-discussion-reply-button',
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index fc91f71e8d2..60b112e67d4 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -1,4 +1,4 @@
 - page_title "#{@commit.title} (#{@commit.short_id})", "Commits"
 = render "commit_box"
 = render "projects/diffs/diffs", diffs: @diffs, project: @project
-= render "projects/notes/notes_with_form"
+= render "projects/notes/notes_with_form", view: params[:view]
diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml
index 75f3a80f0d7..cb41dd852d3 100644
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@@ -18,6 +18,8 @@
         - elsif type_left ==  'old' || type_left.nil?
           %td.old_line{id: line_code_left, class: "#{type_left}"}
             = link_to raw(line_number_left), "##{line_code_left}", id: line_code_left
+            - if @comments_allowed && can?(current_user, :write_note, @project)
+              = link_to_new_diff_note(line_code_left, 'old')
             %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'
@@ -29,12 +31,14 @@
 
           %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
+            - if @comments_allowed && can?(current_user, :write_note, @project)
+              = link_to_new_diff_note(line_code_right, 'new')
             %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
+          = render "projects/notes/diff_notes_with_reply_parallel", notes_left: comments_left, notes_right: 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 789f3e19fd2..c6726cbafa3 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,34 +1,34 @@
-- note1 = notes1.present? ? notes1.first : nil
-- note2 = notes2.present? ? notes2.first : nil
+- note1 = notes_left.present? ? notes_left.first : nil
+- note2 = notes_right.present? ? notes_right.first : nil
 
 %tr.notes_holder
   - if note1
-    %td.notes_line
+    %td.notes_line.old
       %span.btn.disabled
         %i.fa.fa-comment
-        = notes1.count
-    %td.notes_content.parallel
+        = notes_left.count
+    %td.notes_content.parallel.old
       %ul.notes{ rel: note1.discussion_id }
-        = render notes1
+        = render notes_left
 
       .discussion-reply-holder
-        = link_to_reply_diff(note1)
+        = link_to_reply_diff(note1, 'old')
   - else
-    %td= ""
-    %td= ""
+    %td.notes_line.old= ""
+    %td.notes_content.parallel.old= ""
 
   - if note2
-    %td.notes_line
+    %td.notes_line.new
       %span.btn.disabled
         %i.fa.fa-comment
-        = notes2.count
-    %td.notes_content.parallel
+        = notes_right.count
+    %td.notes_content.parallel.new
       %ul.notes{ rel: note2.discussion_id }
-        = render notes2
+        = render notes_right
 
       .discussion-reply-holder
-        = link_to_reply_diff(note2)
+        = link_to_reply_diff(note2, 'new')
   - else
-    %td= ""
-    %td= ""
+    %td.notes_line.new= ""
+    %td.notes_content.parallel.new= ""
 
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index f28b3e9b508..3fb044d736e 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -1,4 +1,6 @@
 = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new_note js-new-note-form common-note-form gfm-form" }, authenticity_token: true do |f|
+  = hidden_field_tag :view, params[:view]
+  = hidden_field_tag :line_type
   = note_target_fields(@note)
   = f.hidden_field :commit_id
   = f.hidden_field :line_code
diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml
index 813e37276bd..a202e74a892 100644
--- a/app/views/projects/notes/_notes_with_form.html.haml
+++ b/app/views/projects/notes/_notes_with_form.html.haml
@@ -4,7 +4,7 @@
 
 .js-main-target-form
 - if can? current_user, :write_note, @project
-  = render "projects/notes/form"
+  = render "projects/notes/form", view: params[:view]
 
 :javascript
-  new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i})
+  new Notes("#{namespace_project_notes_path(namespace_id: @project.namespace, target_id: @noteable.id, target_type: @noteable.class.name.underscore)}", #{@notes.map(&:id).to_json}, #{Time.now.to_i}, "#{params[:view]}")