Address some JS coding-style and HTML semantic issues

- Replace 'rel' attributes with 'data' or 'id' attributes
- Style jQuery methods chaining according to Jacob's guidelines
- Rename a method

5 files changed, 33 insertions, 32 deletions

diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index 3927868cdd4..fd6f89be98d 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -130,11 +130,11 @@ class @Notes
     else if @isNewNote(note)
       @note_ids.push(note.id)
 
-      $('ul.main-notes-list').
-        append(note.html).
-        syntaxHighlight()
+      $('ul.main-notes-list')
+      .append(note.html)
+      .syntaxHighlight()
       @initTaskList()
-      @incrementNotesCount()
+      @updateNotesCount(1)
 
 
   ###
@@ -155,13 +155,13 @@ class @Notes
     return unless @isNewNote(note)
 
     @note_ids.push(note.id)
-    form = $("form[rel='" + note.discussion_id + "']")
+    form = $("#new-discussion-note-form-#{note.discussion_id}")
     row = form.closest("tr")
     note_html = $(note.html)
     note_html.syntaxHighlight()
 
     # is this the first note of discussion?
-    discussionContainer = $(".notes[rel='" + note.discussion_id + "']")
+    discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']")
     if discussionContainer.length is 0
       # insert the note and the reply button after the temp row
       row.after note.discussion_html
@@ -170,21 +170,21 @@ class @Notes
       row.next().find(".note").remove()
 
       # Before that, the container didn't exist
-      discussionContainer = $(".notes[rel='" + note.discussion_id + "']")
+      discussionContainer = $(".notes[data-discussion-id='" + note.discussion_id + "']")
 
       # Add note to 'Changes' page discussions
       discussionContainer.append note_html
 
       # 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).
-          syntaxHighlight()
+        $('ul.main-notes-list')
+        .append(note.discussion_with_diff_html)
+        .syntaxHighlight()
     else
       # append new note to all matching discussions
       discussionContainer.append note_html
 
-    @incrementNotesCount()
+    @updateNotesCount(1)
 
   ###
   Called in response the main target form has been successfully submitted.
@@ -292,7 +292,7 @@ class @Notes
     @renderDiscussionNote(note)
 
     # cleanup after successfully creating a diff/discussion note
-    @removeDiscussionNoteForm($("form[rel='" + note.discussion_id + "']"))
+    @removeDiscussionNoteForm($("#new-discussion-note-form-#{note.discussion_id}"))
 
   ###
   Called in response to the edit note form being submitted
@@ -366,24 +366,30 @@ class @Notes
   Removes the whole discussion if the last note is being removed.
   ###
   removeNote: (e) =>
-    noteId = $(e.currentTarget).closest(".note").attr("id")
-
-    $('.note[id="' + noteId + '"]').each (i, el) =>
-      note = $(el)
+    noteId = $(e.currentTarget)
+             .closest(".note")
+             .attr("id")
+
+    # A same note appears in the "Discussion" and in the "Changes" tab, we have
+    # to remove all. Using $(".note[id='noteId']") ensure we get all the notes,
+    # where $("#noteId") would return only one.
+    $(".note[id='#{noteId}']").each (i, el) =>
+      note  = $(el)
       notes = note.closest(".notes")
 
       # check if this is the last note for this line
       if notes.find(".note").length is 1
 
-        # for discussions
-        notes.closest(".discussion").remove()
+        # "Discussions" tab
+        notes.closest(".timeline-entry").remove()
 
-        # for diff lines
+        # "Changes" tab / commit view
         notes.closest("tr").remove()
 
       note.remove()
 
-    @decrementNotesCount()
+    # Decrement the "Discussions" counter only once
+    @updateNotesCount(-1)
 
   ###
   Called in response to clicking the delete attachment link
@@ -424,7 +430,7 @@ class @Notes
   ###
   setupDiscussionNoteForm: (dataHolder, form) =>
     # setup note target
-    form.attr "rel", dataHolder.data("discussionId")
+    form.attr 'id', "new-discussion-note-form-#{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")
@@ -555,8 +561,5 @@ class @Notes
   updateTaskList: ->
     $('form', this).submit()
 
-  incrementNotesCount: (incrementStep = 1) ->
+  updateNotesCount: (incrementStep) ->
     @notesCountBadge.text parseInt(@notesCountBadge.text()) + incrementStep
-
-  decrementNotesCount: ->
-    @incrementNotesCount(-1)
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 c731baf0a65..11f9859a90f 100644
--- a/app/views/projects/notes/_diff_notes_with_reply.html.haml
+++ b/app/views/projects/notes/_diff_notes_with_reply.html.haml
@@ -7,7 +7,7 @@
         %i.fa.fa-comment
         = notes.count
     %td.notes_content
-      %ul.notes{ rel: note.discussion_id }
+      %ul.notes{ data: { discussion_id: note.discussion_id } }
         = render notes
       .discussion-reply-holder
         = link_to_reply_diff(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
index c6726cbafa3..bb761ed2f94 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
@@ -8,7 +8,7 @@
         %i.fa.fa-comment
         = notes_left.count
     %td.notes_content.parallel.old
-      %ul.notes{ rel: note1.discussion_id }
+      %ul.notes{ data: { discussion_id: note1.discussion_id } }
         = render notes_left
 
       .discussion-reply-holder
@@ -23,7 +23,7 @@
         %i.fa.fa-comment
         = notes_right.count
     %td.notes_content.parallel.new
-      %ul.notes{ rel: note2.discussion_id }
+      %ul.notes{ data: { discussion_id: note2.discussion_id } }
         = render notes_right
 
       .discussion-reply-holder
@@ -31,4 +31,3 @@
   - else
     %td.notes_line.new= ""
     %td.notes_content.parallel.new= ""
-
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 922535e5c4a..e858c412836 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -1,4 +1,4 @@
-%li.timeline-entry{ id: dom_id(note), class: [dom_class(note), "note-row-#{note.id}", ('system-note' if note.system)], data: { discussion: note.discussion_id } }
+%li.timeline-entry{ id: dom_id(note), class: [dom_class(note), "note-row-#{note.id}", ('system-note' if note.system)] }
   .timeline-entry-inner
     .timeline-icon
       %a{href: user_path(note.author)}
diff --git a/app/views/projects/notes/discussions/_commit.html.haml b/app/views/projects/notes/discussions/_commit.html.haml
index 6903fad4a0a..3da2f2060b8 100644
--- a/app/views/projects/notes/discussions/_commit.html.haml
+++ b/app/views/projects/notes/discussions/_commit.html.haml
@@ -20,8 +20,7 @@
       = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note
     - else
       .panel.panel-default
-        .notes{ rel: discussion_notes.first.discussion_id }
+        .notes{ data: { discussion_id: discussion_notes.first.discussion_id } }
           = render discussion_notes
         .discussion-reply-holder
           = link_to_reply_diff(discussion_notes.first)
-