Fixed MR single button submit failure and attempted to tidy up close_reopen...

Fixed MR single button submit failure and attempted to tidy up close_reopen partials using issuable_helpers and setDisable will use true as default bool

app/assets/javascripts/close_reopen_report_toggle.js

@@ -44,7 +44,7 @@ class CloseReopenReportToggle {
     return isClosed ? [this.reopenItem, this.closeItem] : [this.closeItem, this.reopenItem];
   }
 
-  setDisable(shouldDisable) {
+  setDisable(shouldDisable = true) {
     if (shouldDisable) {
       this.button.setAttribute('disabled', 'true');
       this.dropdownTrigger.setAttribute('disabled', 'true');

app/assets/javascripts/issue.js

@@ -57,7 +57,7 @@ class Issue {
         Issue.submitNoteForm($button.closest('form'));
       }
 
-      this.disableCloseReopenButton($button, true);
+      this.disableCloseReopenButton($button);
 
       url = $button.attr('href');
       return $.ajax({

app/assets/javascripts/merge_request.js

@@ -69,7 +69,7 @@ import CloseReopenReportToggle from './close_reopen_report_toggle';
           return;
         }
 
-        _this.disableCloseReopenButton($this, true);
+        if (this.closeReopenReportToggle) this.closeReopenReportToggle.setDisable();
 
         if (shouldSubmit) {
           if ($this.hasClass('btn-comment-and-close') || $this.hasClass('btn-comment-and-reopen')) {
@@ -145,14 +145,6 @@ import CloseReopenReportToggle from './close_reopen_report_toggle';
       this.closeReopenReportToggle.initDroplab();
     };
 
-    MergeRequest.prototype.disableCloseReopenButton = function ($button, shouldDisable) {
-      if (this.closeReopenReportToggle) {
-        this.closeReopenReportToggle.setDisable(shouldDisable);
-      } else {
-        $button.prop('disabled', shouldDisable);
-      }
-    };
-
     return MergeRequest;
   })();
 }).call(window);

app/helpers/issuables_helper.rb

@@ -246,21 +246,11 @@ module IssuablesHelper
   end
 
   def close_issuable_url(issuable)
-    params = {}
-    params[issuable.model_name.to_s.underscore] = {}
-    params[issuable.model_name.to_s.underscore][:state_event] = :close
-    params[:format] = :json if issuable.is_a?(Issue)
-
-    issuable_url(issuable, params)
+    issuable_url(issuable, close_reopen_params(issuable, :close))
   end
 
   def reopen_issuable_url(issuable)
-    params = {}
-    params[issuable.model_name.to_s.underscore] = {}
-    params[issuable.model_name.to_s.underscore][:state_event] = :reopen
-    params[:format] = :json if issuable.is_a?(Issue)
-
-    issuable_url(issuable, params)
+    issuable_url(issuable, close_reopen_params(issuable, :reopen))
   end
 
   def close_reopen_issuable_url(issuable, should_inverse = false)
@@ -278,6 +268,22 @@ module IssuablesHelper
     end
   end
 
+  def issuable_button_visibility(issuable, closed)
+    case issuable
+    when Issue then issue_button_visibility(issuable, closed)
+    when MergeRequest then merge_request_button_visibility(issuable, closed)
+    else raise 'unknown issuable type'
+    end
+  end
+
+  def issuable_close_reopen_button_method(issuable)
+    case issuable
+    when Issue then ''
+    when MergeRequest then 'put'
+    else raise 'unknown issuable type'
+    end
+  end
+
   private
 
   def sidebar_gutter_collapsed?
@@ -334,4 +340,12 @@ module IssuablesHelper
       container: (is_collapsed ? 'body' : nil)
     }
   end
+
+  def close_reopen_params(issuable, action)
+    params = {}
+    params[issuable.model_name.to_s.underscore] = {}
+    params[issuable.model_name.to_s.underscore][:state_event] = action
+    params[:format] = :json if issuable.is_a?(Issue)
+    params
+  end
 end

app/views/shared/issuable/_close_reopen_button.html.haml

 - is_current_user = current_user == issuable.author
 - display_issuable_type = issuable.model_name.human.downcase
-- button_method = issuable.is_a?(MergeRequest) ? :put : nil
+- button_method = issuable_close_reopen_button_method(issuable)
 
 - if can_update && is_current_user
-  = link_to "Close #{display_issuable_type}", close_issuable_url(issuable), method: button_method, class: "hidden-xs hidden-sm btn btn-close btn-grouped #{'hidden' if issuable.closed?}", title: "Close #{display_issuable_type}"
-  = link_to "Reopen #{display_issuable_type}", reopen_issuable_url(issuable), method: button_method, class: "hidden-xs hidden-sm btn btn-reopen btn-grouped #{'hidden' unless issuable.closed?}", title: "Reopen #{display_issuable_type}"
+  = link_to "Close #{display_issuable_type}", close_issuable_url(issuable), method: button_method,
+            class: "btn btn-grouped btn-close #{issuable_button_visibility(issuable, true)}", title: "Close #{display_issuable_type}"
+  = link_to "Reopen #{display_issuable_type}", reopen_issuable_url(issuable), method: button_method,
+            class: "btn btn-grouped btn-reopen #{issuable_button_visibility(issuable, false)}", title: "Reopen #{display_issuable_type}"
 - elsif can_update && !is_current_user
   = render 'shared/issuable/close_reopen_report_toggle', issuable: issuable
 - else
-  = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)), class: 'hidden-xs hidden-sm btn btn-grouped btn-close-color', title: 'Report abuse'
+  = link_to 'Report abuse', new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
+            class: 'hidden-xs hidden-sm btn btn-grouped btn-close-color', title: 'Report abuse'

app/views/shared/issuable/_close_reopen_report_toggle.html.haml

@@ -4,16 +4,20 @@
 - button_responsive_class = 'hidden-xs hidden-sm'
 - button_class = "#{button_responsive_class} btn btn-grouped js-issuable-close-button issuable-close-button"
 - toggle_class = "#{button_responsive_class} btn btn-nr dropdown-toggle js-issuable-close-toggle"
-- button_method = issuable.is_a?(MergeRequest) ? :put : nil
+- button_method = issuable_close_reopen_button_method(issuable)
 
 .pull-left.btn-group.prepend-left-10.issuable-close-dropdown.droplab-dropdown.js-issuable-close-dropdown
-  = link_to "#{display_button_action} #{display_issuable_type}", close_reopen_issuable_url(issuable), method: button_method, class: "#{button_class} btn-#{button_action}", title: "#{display_button_action} #{display_issuable_type}"
+  = link_to "#{display_button_action} #{display_issuable_type}", close_reopen_issuable_url(issuable),
+            method: button_method, class: "#{button_class} btn-#{button_action}", title: "#{display_button_action} #{display_issuable_type}"
 
-  = button_tag type: 'button', class: "#{toggle_class} btn-#{button_action}-color", data: { 'dropdown-trigger' => '#issuable-close-menu' }, 'aria-label' => 'Toggle dropdown' do
+  = button_tag type: 'button', class: "#{toggle_class} btn-#{button_action}-color",
+               data: { 'dropdown-trigger' => '#issuable-close-menu' }, 'aria-label' => 'Toggle dropdown' do
     = icon('caret-down', class: 'toggle-icon icon')
 
   %ul#issuable-close-menu.js-issuable-close-menu.dropdown-menu{ class: button_responsive_class, data: { dropdown: true } }
-    %li.close-item{ class: "#{issuable.closed? ? 'hidden' : 'droplab-item-selected'}", data: { text: "Close #{display_issuable_type}", url: close_issuable_url(issuable), button_class: "#{button_class} btn-close", toggle_class: "#{toggle_class} btn-close-color", method: button_method } }
+    %li.close-item{ class: "#{issuable_button_visibility(issuable, true) || 'droplab-item-selected'}",
+                    data: { text: "Close #{display_issuable_type}", url: close_issuable_url(issuable),
+                    button_class: "#{button_class} btn-close", toggle_class: "#{toggle_class} btn-close-color", method: button_method } }
       %button.btn.btn-transparent
         = icon('check', class: 'icon')
         .description
@@ -22,7 +26,9 @@
             = display_issuable_type
           %p
 
-    %li.reopen-item{ class: "#{issuable.closed? ? 'droplab-item-selected' : 'hidden'}", data: { text: "Reopen #{display_issuable_type}", url: reopen_issuable_url(issuable), button_class: "#{button_class} btn-reopen", toggle_class: "#{toggle_class} btn-reopen-color", method: button_method } }
+    %li.reopen-item{ class: "#{issuable_button_visibility(issuable, false) || 'droplab-item-selected'}",
+                     data: { text: "Reopen #{display_issuable_type}", url: reopen_issuable_url(issuable),
+                     button_class: "#{button_class} btn-reopen", toggle_class: "#{toggle_class} btn-reopen-color", method: button_method } }
       %button.btn.btn-transparent
         = icon('check', class: 'icon')
         .description
@@ -33,7 +39,8 @@
 
       %li.divider.droplab-item-ignore
 
-      %li.report-item{ data: { text: 'Report abuse', url: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)), button_class: "#{button_class} btn-close-color", toggle_class: "#{toggle_class} btn-close-color", method: '' } }
+      %li.report-item{ data: { text: 'Report abuse', url: new_abuse_report_path(user_id: issuable.author.id, ref_url: issuable_url(issuable)),
+                       button_class: "#{button_class} btn-close-color", toggle_class: "#{toggle_class} btn-close-color", method: '' } }
         %button.btn.btn-transparent
           = icon('check', class: 'icon')
           .description

spec/javascripts/close_reopen_report_toggle_spec.js

@@ -198,6 +198,13 @@ describe('CloseReopenReportToggle', () => {
       expect(dropdownTrigger.setAttribute).toHaveBeenCalledWith('disabled', 'true');
     });
 
+    it('disable .button and .dropdownTrigger if shouldDisable is undefined', () => {
+      closeReopenReportToggle.setDisable();
+
+      expect(button.setAttribute).toHaveBeenCalledWith('disabled', 'true');
+      expect(dropdownTrigger.setAttribute).toHaveBeenCalledWith('disabled', 'true');
+    });
+
     it('enable .button and .dropdownTrigger if shouldDisable is false', () => {
       closeReopenReportToggle.setDisable(false);