Fixed bugs and added tests

6 files changed, 33 insertions, 15 deletions

diff --git a/app/assets/javascripts/droplab/drop_down.js b/app/assets/javascripts/droplab/drop_down.js
index f522859c457..9588921ebcd 100644
--- a/app/assets/javascripts/droplab/drop_down.js
+++ b/app/assets/javascripts/droplab/drop_down.js
@@ -37,7 +37,7 @@ Object.assign(DropDown.prototype, {
   clickEvent: function(e) {
     if (e.target.tagName === 'UL') return;
 
-    var selected = utils.closest(e.target, 'LI', '');
+    var selected = utils.closest(e.target, 'LI');
     if (!selected) return;
 
     this.addSelectedClass(selected);
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 66e89b7607a..9dd516d93fd 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -143,7 +143,7 @@ require('./task_list');
         form.querySelector('.js-comment-type-dropdown .dropdown-menu'),
         form.querySelector('#note_type'),
         form.querySelector('.js-comment-type-dropdown .js-comment-submit-button'),
-        form.querySelector('.js-note-target-close'),
+        form.querySelector('.js-note-target-close:not(.hidden)') || form.querySelector('.js-note-target-reopen'),
       );
 
       this.commentTypeToggle.initDroplab();
diff --git a/app/views/projects/notes/_comment_button.html.haml b/app/views/projects/notes/_comment_button.html.haml
index 008363263b3..2e65a3bf294 100644
--- a/app/views/projects/notes/_comment_button.html.haml
+++ b/app/views/projects/notes/_comment_button.html.haml
@@ -1,4 +1,5 @@
 - noteable_name = @note.noteable.human_class_name
+- noteable_state_action = noteable_name =~ /(merge request|issue)/ && @note.noteable['state'] == 'closed' ? 'reopen' : 'close'
 
 .pull-left.btn-group.append-right-10.comment-type-dropdown.js-comment-type-dropdown
   %input.btn.btn-nr.btn-create.comment-btn.js-comment-button.js-comment-submit-button{ type: 'submit', value: 'Comment' }
@@ -8,7 +9,7 @@
       = icon('caret-down')
 
     %ul#resolvable-comment-menu.dropdown-menu{ data: { dropdown: true } }
-      %li#comment.droplab-item-selected{ data: { value: '', 'button-text' => 'Comment', 'secondary-button-text' => "Comment & close #{noteable_name}" } }
+      %li#comment.droplab-item-selected{ data: { value: '', 'button-text' => 'Comment', 'secondary-button-text' => "Comment & #{noteable_state_action} #{noteable_name}" } }
         = icon('check')
         .description
           %strong Comment
@@ -17,7 +18,7 @@
 
       %li.divider
 
-      %li#discussion{ data: { value: 'DiscussionNote', 'button-text' => 'Start discussion', 'secondary-button-text' => "Start discussion & close #{noteable_name}" } }
+      %li#discussion{ data: { value: 'DiscussionNote', 'button-text' => 'Start discussion', 'secondary-button-text' => "Start discussion & #{noteable_state_action} #{noteable_name}" } }
         = icon('check')
         .description
           %strong Start discussion
diff --git a/spec/features/discussion_comments_spec.rb b/spec/features/discussion_comments_spec.rb
index 9e99fe064e8..ae778118c5c 100644
--- a/spec/features/discussion_comments_spec.rb
+++ b/spec/features/discussion_comments_spec.rb
@@ -93,9 +93,9 @@ shared_examples 'discussion comments' do |resource_name|
     end
 
     it 'clicking the ul padding should not change the text' do
-      find(menu_selector).click
+      find(menu_selector).trigger 'click'
 
-      expect(find(submit_selector)).to have_content 'Comment'
+      expect(find(dropdown_selector)).to have_content 'Comment'
     end
 
     describe 'when selecting "Start discussion"' do
@@ -109,7 +109,7 @@ shared_examples 'discussion comments' do |resource_name|
       end
 
       it 'updates the submit button text' do
-        expect(find(dropdown_selector)).to have_content "Start discussion"
+        expect(find(dropdown_selector)).to have_content 'Start discussion'
       end
 
       if resource_name =~ /(issue|merge request)/
@@ -181,7 +181,7 @@ shared_examples 'discussion comments' do |resource_name|
           end
 
           it 'updates the submit button text' do
-            expect(find(dropdown_selector)).to have_content "Comment"
+            expect(find(dropdown_selector)).to have_content 'Comment'
           end
 
           if resource_name =~ /(issue|merge request)/
@@ -222,20 +222,22 @@ shared_examples 'discussion comments' do |resource_name|
   if resource_name =~ /(issue|merge request)/
     describe "on a closed #{resource_name}" do
       before do
-        find("#{form_selector} .close-mr-link").click
+        find("#{form_selector} .js-note-target-close").click
+
+        find("#{form_selector} .note-textarea").send_keys('a')
       end
 
-      it 'should show a "Comment & reopen #{resource_name}" button' do
-        expect(find(close_selector)).to have_content "Comment & reopen #{resource_name}"
+      it "should show a 'Comment & reopen #{resource_name}' button" do
+        expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Comment & reopen #{resource_name}"
       end
 
-      it 'should show a "Start discussion & reopen #{resource_name}" button when "Start discussion" is selected' do
+      it "should show a 'Start discussion & reopen #{resource_name}' button when 'Start discussion' is selected" do
         find(toggle_selector).click
 
         find("#{menu_selector} li", match: :first)
         all("#{menu_selector} li").last.click
 
-        expect(find(close_selector)).to have_content "Start discussion & reopen #{resource_name}"
+        expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Start discussion & reopen #{resource_name}"
       end
     end
   end
diff --git a/spec/javascripts/droplab/drop_down_spec.js b/spec/javascripts/droplab/drop_down_spec.js
index bbf953658c8..802e2435672 100644
--- a/spec/javascripts/droplab/drop_down_spec.js
+++ b/spec/javascripts/droplab/drop_down_spec.js
@@ -130,7 +130,7 @@ describe('DropDown', function () {
     beforeEach(function () {
       this.list = { dispatchEvent: () => {} };
       this.dropdown = { hide: () => {}, list: this.list, addSelectedClass: () => {} };
-      this.event = { preventDefault: () => {}, target: 'target' };
+      this.event = { preventDefault: () => {}, target: {} };
       this.customEvent = {};
       this.closestElement = {};
 
@@ -168,6 +168,21 @@ describe('DropDown', function () {
       expect(this.list.dispatchEvent).toHaveBeenCalledWith(this.customEvent);
     });
 
+    describe('if the target is a UL element', function () {
+      beforeEach(function () {
+        this.event = { preventDefault: () => {}, target: { tagName: 'UL' } };
+
+        spyOn(this.event, 'preventDefault');
+        utils.closest.calls.reset();
+
+        DropDown.prototype.clickEvent.call(this.dropdown, this.event);
+      });
+
+      it('should return immediately', function () {
+        expect(utils.closest).not.toHaveBeenCalled();
+      });
+    });
+
     describe('if no selected element exists', function () {
       beforeEach(function () {
         this.event.preventDefault.calls.reset();
diff --git a/spec/javascripts/droplab/plugins/input_setter_spec.js b/spec/javascripts/droplab/plugins/input_setter_spec.js
index 412d1054385..bd625f4ae80 100644
--- a/spec/javascripts/droplab/plugins/input_setter_spec.js
+++ b/spec/javascripts/droplab/plugins/input_setter_spec.js
@@ -2,7 +2,7 @@
 
 import InputSetter from '~/droplab/plugins/input_setter';
 
-fdescribe('InputSetter', function () {
+describe('InputSetter', function () {
   describe('init', function () {
     beforeEach(function () {
       this.config = { InputSetter: {} };