Show existing discussion when adding new comment on line with a hidden resolved discussion

app/assets/javascripts/application.js

@@ -224,7 +224,12 @@
     });
     $body.on("click", ".js-toggle-diff-comments", function(e) {
       $(this).toggleClass('active');
-      $(this).closest(".diff-file").find(".notes_holder").toggle();
+      var notesHolders = $(this).closest(".diff-file").find(".notes_holder");
+      if ($(this).hasClass('active')) {
+        notesHolders.show();
+      } else {
+        notesHolders.hide();
+      }
       return e.preventDefault();
     });
     $document.off("click", '.js-confirm-danger');

app/assets/javascripts/notes.js

@@ -637,16 +637,19 @@
       nextRow = row.next();
       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>";
+      notesContentSelector = ".notes_content";
+      rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"><div class=\"content\"></div></td></tr>";
       if (this.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>";
+        notesContentSelector += "." + lineType;
+        rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line old\"></td><td class=\"notes_content parallel old\"><div class=\"content\"></div></td><td class=\"notes_line new\"></td><td class=\"notes_content parallel new\"><div class=\"content\"></div></td></tr>";
       }
+      notesContentSelector += " .content";
       if (hasNotes) {
-        notesContent = nextRow.find(targetContent);
+        nextRow.show();
+        notesContent = nextRow.find(notesContentSelector);
         if (notesContent.length) {
+          notesContent.show();
           replyButton = notesContent.find(".js-discussion-reply-button:visible");
           if (replyButton.length) {
             e.target = replyButton[0];
@@ -660,11 +663,13 @@
         }
       } else {
         row.after(rowCssToAdd);
+        nextRow = row.next();
+        notesContent = nextRow.find(notesContentSelector);
         addForm = true;
       }
       if (addForm) {
         newForm = this.formClone.clone();
-        newForm.appendTo(row.next().find(targetContent));
+        newForm.appendTo(notesContent);
         return this.setupDiscussionNoteForm($link, newForm);
       }
     };

app/models/note.rb

@@ -88,7 +88,6 @@ class Note < ActiveRecord::Base
     def grouped_diff_discussions
       active_notes = diff_notes.fresh.select(&:active?)
       Discussion.for_diff_notes(active_notes).
-        reject(&:resolved?).
         map { |d| [d.line_code, d] }.to_h
     end
 

app/views/discussions/_diff_discussion.html.haml

@@ -1,13 +1,6 @@
-%tr.notes_holder
+- expanded = local_assigns.fetch(:expanded, discussion.expanded?)
+%tr.notes_holder{class: ('hide' unless expanded)}
   %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
-
-    .discussion-reply-holder
-      .discussion-with-resolve-btn{ role: "group" }
-        .btn-group{ role: "group" }
-          = link_to_reply_discussion(discussion)
-        .btn-group{ role: "group" }
-          = render "discussions/resolve_all", discussion: discussion
-        = render "discussions/jump_to_next", discussion: discussion
+    .content
+      = render "discussions/notes", discussion: discussion

app/views/discussions/_diff_with_notes.html.haml

@@ -11,4 +11,4 @@
         = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true
 
         - if discussion.for_line?(line)
-          = render "discussions/diff_discussion", discussion: discussion
+          = render "discussions/diff_discussion", discussion: discussion, expanded: true

app/views/discussions/_discussion.html.haml

@@ -44,4 +44,5 @@
           - if discussion.diff_discussion? && discussion.diff_file
             = render "discussions/diff_with_notes", discussion: discussion
           - else
-            = render "discussions/notes", discussion: discussion
+            .panel.panel-default
+              = render "discussions/notes", discussion: discussion

app/views/discussions/_notes.html.haml

@@ -1,7 +1,15 @@
-.panel.panel-default
-  .notes{ data: { discussion_id: discussion.id } }
-    %ul.notes.timeline
-      = render partial: "projects/notes/note", collection: discussion.notes, as: :note
+%ul.notes{ data: { discussion_id: discussion.id } }
+  = render partial: "projects/notes/note", collection: discussion.notes, as: :note
 
-  .discussion-reply-holder
+.discussion-reply-holder
+  - if discussion.diff_discussion?
+    - line_type = local_assigns.fetch(:line_type, nil)
+
+    .btn-group-justified.discussion-with-resolve-btn{ role: "group" }
+      .btn-group{ role: "group" }
+        = link_to_reply_discussion(discussion, line_type)
+      .btn-group{ role: "group" }
+        = render "discussions/resolve_all", discussion: discussion
+      = render "discussions/jump_to_next", discussion: discussion
+  - else
     = link_to_reply_discussion(discussion)

app/views/discussions/_parallel_diff_discussion.html.haml

@@ -1,34 +1,21 @@
-%tr.notes_holder
+- expanded = discussion_left.try(:expanded?) || discussion_right.try(:expanded?)
+%tr.notes_holder{class: ('hide' unless expanded)}
   - 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
-
-      .discussion-reply-holder
-        .btn-group-justified.discussion-with-resolve-btn{ role: "group" }
-          .btn-group{ role: "group" }
-            = link_to_reply_discussion(discussion_left, 'old')
-          .btn-group{ role: "group" }
-            = render "discussions/resolve_all", discussion: discussion_left
-          = render "discussions/jump_to_next", discussion: discussion_left
+      .content{class: ('hide' unless discussion_left.expanded?)}
+        = render "discussions/notes", discussion: discussion_left, line_type: 'old'
   - else
     %td.notes_line.old= ""
-    %td.notes_content.parallel.old= ""
+    %td.notes_content.parallel.old
+      .content
 
   - 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
-
-      .discussion-reply-holder
-        .btn-group-justified.discussion-with-resolve-btn{ role: "group" }
-          .btn-group{ role: "group" }
-            = link_to_reply_discussion(discussion_right, 'new')
-          .btn-group{ role: "group" }
-            = render "discussions/resolve_all", discussion: discussion_right
-          = render "discussions/jump_to_next", discussion: discussion_right
+      .content{class: ('hide' unless discussion_right.expanded?)}
+        = render "discussions/notes", discussion: discussion_right, line_type: 'new'
   - else
     %td.notes_line.new= ""
-    %td.notes_content.parallel.new= ""
+    %td.notes_content.parallel.new
+      .content