Refactor code for bulk update merge requests featuredz-bulk-edit-mr

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

5 files changed, 34 insertions, 29 deletions

diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 02d6d2082f9..63845e6b9ba 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -409,7 +409,7 @@
   li {
     .issue-check {
       float: left;
-      padding-right: 16px;
+      padding-right: $gl-padding;
       margin-bottom: 10px;
       min-width: 15px;
 
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 76f7f179008..77b4efffd7f 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -24,13 +24,13 @@ module IssuableActions
   private
 
   def authorize_destroy_issuable!
-    unless current_user.can?(:"destroy_#{issuable.to_ability_name}", issuable)
+    unless can?(current_user, :"destroy_#{issuable.to_ability_name}", issuable)
       return access_denied!
     end
   end
 
   def authorize_admin_issuable!
-    unless current_user.can?(:"admin_#{resource_name}", @project)
+    unless can?(current_user, :"admin_#{resource_name}", @project)
       return access_denied!
     end
   end
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 3f5af880646..5c04bba323f 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -111,12 +111,4 @@ module IssuablesHelper
       issuable.open? ? :opened : :closed
     end
   end
-
-  def issuable_bulk_update_path(type)
-    if type == :merge_requests
-      bulk_update_namespace_project_merge_requests_path(@project.namespace, @project)
-    else
-      bulk_update_namespace_project_issues_path(@project.namespace, @project)
-    end
-  end
 end
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index 45b8ec72fa4..93c4d5c3d30 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -49,7 +49,7 @@
 
     - if @bulk_edit
       .issues_bulk_update.hide
-        = form_tag issuable_bulk_update_path(type), method: :post, class: 'bulk-update'  do
+        = form_tag [:bulk_update, @project.namespace.becomes(Namespace), @project, type], method: :post, class: 'bulk-update'  do
           .filter-item.inline
             = dropdown_tag("Status", options: { toggle_class: "js-issue-status", title: "Change status", dropdown_class: "dropdown-menu-status dropdown-menu-selectable", data: { field_name: "update[state_event]" } } ) do
               %ul
diff --git a/spec/features/merge_requests/update_merge_requests_spec.rb b/spec/features/merge_requests/update_merge_requests_spec.rb
index 43b31dce9b3..b56fdfe5611 100644
--- a/spec/features/merge_requests/update_merge_requests_spec.rb
+++ b/spec/features/merge_requests/update_merge_requests_spec.rb
@@ -13,29 +13,39 @@ feature 'Multiple merge requests updating from merge_requests#index', feature: t
   end
 
   context 'status', js: true do
-    it 'sets to closed' do
-      visit namespace_project_merge_requests_path(project.namespace, project)
+    describe 'close merge request' do
+      before do
+        visit namespace_project_merge_requests_path(project.namespace, project)
+      end
 
-      change_status('Closed')
-      expect(page).to have_selector('.merge-request', count: 0)
+      it 'closes merge request' do
+        change_status('Closed')
+
+        expect(page).to have_selector('.merge-request', count: 0)
+      end
     end
 
-    it 'sets to open' do
-      merge_request.close
-      visit namespace_project_merge_requests_path(project.namespace, project, state: 'closed')
+    describe 'reopen merge request' do
+      before do
+        merge_request.close
+        visit namespace_project_merge_requests_path(project.namespace, project, state: 'closed')
+      end
+
+      it 'reopens merge request' do
+        change_status('Open')
 
-      change_status('Open')
-      expect(page).to have_selector('.merge-request', count: 0)
+        expect(page).to have_selector('.merge-request', count: 0)
+      end
     end
   end
 
   context 'assignee', js: true do
-    context 'set assignee' do
+    describe 'set assignee' do
       before do
         visit namespace_project_merge_requests_path(project.namespace, project)
       end
 
-      it "should update merge request with assignee" do
+      it "updates merge request with assignee" do
         change_assignee(user.name)
 
         page.within('.merge-request .controls') do
@@ -44,15 +54,16 @@ feature 'Multiple merge requests updating from merge_requests#index', feature: t
       end
     end
 
-    context 'remove assignee' do
+    describe 'remove assignee' do
       before do
         merge_request.assignee = user
         merge_request.save
         visit namespace_project_merge_requests_path(project.namespace, project)
       end
 
-      it "should remove assignee from the merge request" do
+      it "removes assignee from the merge request" do
         change_assignee('Unassigned')
+
         expect(find('.merge-request .controls')).not_to have_css('.author_link')
       end
     end
@@ -61,26 +72,28 @@ feature 'Multiple merge requests updating from merge_requests#index', feature: t
   context 'milestone', js: true do
     let(:milestone)  { create(:milestone, project: project) }
 
-    context 'set milestone' do
+    describe 'set milestone' do
       before do
         visit namespace_project_merge_requests_path(project.namespace, project)
       end
 
-      it "should update merge request with milestone" do
+      it "updates merge request with milestone" do
         change_milestone(milestone.title)
+
         expect(find('.merge-request')).to have_content milestone.title
       end
     end
 
-    context 'unset milestone' do
+    describe 'unset milestone' do
       before do
         merge_request.milestone = milestone
         merge_request.save
         visit namespace_project_merge_requests_path(project.namespace, project)
       end
 
-      it "should remove milestone from the merge request" do
+      it "removes milestone from the merge request" do
         change_milestone("No Milestone")
+
         expect(find('.merge-request')).not_to have_content milestone.title
       end
     end