Add bulk update support for merge requests list

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

16 files changed, 98 insertions, 81 deletions

diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 38cdc7b9fba..179d3bc38a5 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -23,6 +23,7 @@
         case 'projects:boards:show':
           shortcut_handler = new ShortcutsNavigation();
           break;
+        case 'projects:merge_requests:index':
         case 'projects:issues:index':
           Issuable.init();
           new IssuableBulkActions();
@@ -93,10 +94,6 @@
           break;
         case "projects:merge_requests:conflicts":
           window.mcui = new MergeConflictResolver()
-        case 'projects:merge_requests:index':
-          shortcut_handler = new ShortcutsNavigation();
-          Issuable.init();
-          break;
         case 'dashboard:activity':
           new Activities();
           break;
diff --git a/app/assets/javascripts/issuable.js.es6 b/app/assets/javascripts/issuable.js.es6
index 4006ac740b2..82c14ef0157 100644
--- a/app/assets/javascripts/issuable.js.es6
+++ b/app/assets/javascripts/issuable.js.es6
@@ -77,7 +77,7 @@
     },
     checkChanged: function() {
       const $checkedIssues = $('.selected_issue:checked');
-      const $updateIssuesIds = $('#update_issues_ids');
+      const $updateIssuesIds = $('#update_issuable_ids');
       const $issuesOtherFilters = $('.issues-other-filters');
       const $issuesBulkUpdate = $('.issues_bulk_update');
 
diff --git a/app/assets/javascripts/issues-bulk-assignment.js b/app/assets/javascripts/issues-bulk-assignment.js
index 98d3358ba92..8ca90490426 100644
--- a/app/assets/javascripts/issues-bulk-assignment.js
+++ b/app/assets/javascripts/issues-bulk-assignment.js
@@ -5,7 +5,7 @@
       if (opts == null) {
         opts = {};
       }
-      this.container = (ref = opts.container) != null ? ref : $('.content'), this.form = (ref1 = opts.form) != null ? ref1 : this.getElement('.bulk-update'), this.issues = (ref2 = opts.issues) != null ? ref2 : this.getElement('.issues-list .issue');
+      this.container = (ref = opts.container) != null ? ref : $('.content'), this.form = (ref1 = opts.form) != null ? ref1 : this.getElement('.bulk-update'), this.issues = (ref2 = opts.issues) != null ? ref2 : this.getElement('.issuable-list > li');
       this.form.data('bulkActions', this);
       this.willUpdateLabels = false;
       this.bindEvents();
@@ -106,7 +106,7 @@
           state_event: this.form.find('input[name="update[state_event]"]').val(),
           assignee_id: this.form.find('input[name="update[assignee_id]"]').val(),
           milestone_id: this.form.find('input[name="update[milestone_id]"]').val(),
-          issues_ids: this.form.find('input[name="update[issues_ids]"]').val(),
+          issuable_ids: this.form.find('input[name="update[issuable_ids]"]').val(),
           subscription_event: this.form.find('input[name="update[subscription_event]"]').val(),
           add_label_ids: [],
           remove_label_ids: []
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 46c4a11aa2e..02d6d2082f9 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -404,3 +404,18 @@
     margin-bottom: $gl-padding;
   }
 }
+
+.issuable-list {
+  li {
+    .issue-check {
+      float: left;
+      padding-right: 16px;
+      margin-bottom: 10px;
+      min-width: 15px;
+
+      .selected_issue {
+        vertical-align: text-top;
+      }
+    }
+  }
+}
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index d14224ed00f..7a26b7ad497 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -7,17 +7,6 @@
       margin-bottom: 2px;
     }
 
-    .issue-check {
-      float: left;
-      padding-right: 16px;
-      margin-bottom: 10px;
-      min-width: 15px;
-
-      .selected_issue {
-        vertical-align: text-top;
-      }
-    }
-
     .issue-labels {
       display: inline-block;
     }
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index f40b62446e5..76f7f179008 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -3,6 +3,7 @@ module IssuableActions
 
   included do
     before_action :authorize_destroy_issuable!, only: :destroy
+    before_action :authorize_admin_issuable!, only: :bulk_update
   end
 
   def destroy
@@ -13,6 +14,13 @@ module IssuableActions
     redirect_to polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable.class])
   end
 
+  def bulk_update
+    result = Issuable::BulkUpdateService.new(project, current_user, bulk_update_params).execute(resource_name)
+    quantity = result[:count]
+
+    render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" }
+  end
+
   private
 
   def authorize_destroy_issuable!
@@ -20,4 +28,27 @@ module IssuableActions
       return access_denied!
     end
   end
+
+  def authorize_admin_issuable!
+    unless current_user.can?(:"admin_#{resource_name}", @project)
+      return access_denied!
+    end
+  end
+
+  def bulk_update_params
+    params.require(:update).permit(
+      :issuable_ids,
+      :assignee_id,
+      :milestone_id,
+      :state_event,
+      :subscription_event,
+      label_ids: [],
+      add_label_ids: [],
+      remove_label_ids: []
+    )
+  end
+
+  def resource_name
+    @resource_name ||= controller_name.singularize
+  end
 end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 72d2d361878..de02e28e384 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -20,9 +20,6 @@ class Projects::IssuesController < Projects::ApplicationController
   # Allow modify issue
   before_action :authorize_update_issue!, only: [:edit, :update]
 
-  # Allow issues bulk update
-  before_action :authorize_admin_issues!, only: [:bulk_update]
-
   respond_to :html
 
   def index
@@ -168,16 +165,6 @@ class Projects::IssuesController < Projects::ApplicationController
     end
   end
 
-  def bulk_update
-    result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute
-
-    respond_to do |format|
-      format.json do
-        render json: { notice: "#{result[:count]} issues updated" }
-      end
-    end
-  end
-
   protected
 
   def issue
@@ -237,17 +224,4 @@ class Projects::IssuesController < Projects::ApplicationController
       :milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: []
     )
   end
-
-  def bulk_update_params
-    params.require(:update).permit(
-      :issues_ids,
-      :assignee_id,
-      :milestone_id,
-      :state_event,
-      :subscription_event,
-      label_ids: [],
-      add_label_ids: [],
-      remove_label_ids: []
-    )
-  end
 end
diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb
new file mode 100644
index 00000000000..60891cbb255
--- /dev/null
+++ b/app/services/issuable/bulk_update_service.rb
@@ -0,0 +1,26 @@
+module Issuable
+  class BulkUpdateService < IssuableBaseService
+    def execute(type)
+      model_class = type.classify.constantize
+      update_class = type.classify.pluralize.constantize::UpdateService
+
+      ids = params.delete(:issuable_ids).split(",")
+      items = model_class.where(id: ids)
+
+      %i(state_event milestone_id assignee_id add_label_ids remove_label_ids subscription_event).each do |key|
+        params.delete(key) unless params[key].present?
+      end
+
+      items.each do |issuable|
+        next unless can?(current_user, :"update_#{type}", issuable)
+
+        update_class.new(issuable.project, current_user, params).execute(issuable)
+      end
+
+      {
+        count:    items.count,
+        success:  !items.count.zero?
+      }
+    end
+  end
+end
diff --git a/app/services/issues/bulk_update_service.rb b/app/services/issues/bulk_update_service.rb
deleted file mode 100644
index 7e19a73f71a..00000000000
--- a/app/services/issues/bulk_update_service.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-module Issues
-  class BulkUpdateService < BaseService
-    def execute
-      issues_ids   = params.delete(:issues_ids).split(",")
-      issue_params = params
-
-      %i(state_event milestone_id assignee_id add_label_ids remove_label_ids subscription_event).each do |key|
-        issue_params.delete(key) unless issue_params[key].present?
-      end
-
-      issues = Issue.where(id: issues_ids)
-
-      issues.each do |issue|
-        next unless can?(current_user, :update_issue, issue)
-
-        Issues::UpdateService.new(issue.project, current_user, issue_params).execute(issue)
-      end
-
-      {
-        count:    issues.count,
-        success:  !issues.count.zero?
-      }
-    end
-  end
-end
diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index 79b14819865..851d4c06990 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -1,7 +1,7 @@
 %li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: issue.label_ids, id: issue.id } }
-  - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project)
+  - if @bulk_edit
     .issue-check
-      = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue"
+      = check_box_tag dom_id(issue, "selected"), nil, false, 'data-id' => issue.id, class: "selected_issue"
 
   .issue-title.title
     %span.issue-title-text
diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml
index 1a87045aa60..023ea5f17d7 100644
--- a/app/views/projects/issues/index.html.haml
+++ b/app/views/projects/issues/index.html.haml
@@ -1,4 +1,6 @@
 - @no_container = true
+- @bulk_edit = can?(current_user, :admin_issue, @project)
+
 - page_title "Issues"
 - new_issue_email = @project.new_issue_address(current_user)
 = render "projects/issues/head"
@@ -29,7 +31,7 @@
             New Issue
     = render 'shared/issuable/filter', type: :issues
 
-    .issues-holder
+    .issues-holder.issuable-list
       = render 'issues'
       - if new_issue_email
         = render 'issue_by_email', email: new_issue_email
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index 5029b365f93..31f8d0aeb5b 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -1,4 +1,8 @@
 %li{ class: mr_css_classes(merge_request) }
+  - if @bulk_edit
+    .issue-check
+      = check_box_tag dom_id(merge_request, "selected"), nil, false, 'data-id' => merge_request.id, class: "selected_issue"
+
   .merge-request-title.title
     %span.merge-request-title-text
       = link_to merge_request.title, merge_request_path(merge_request)
diff --git a/app/views/projects/merge_requests/_merge_requests.html.haml b/app/views/projects/merge_requests/_merge_requests.html.haml
index 446887774a4..fe82f751f53 100644
--- a/app/views/projects/merge_requests/_merge_requests.html.haml
+++ b/app/views/projects/merge_requests/_merge_requests.html.haml
@@ -1,4 +1,4 @@
-%ul.content-list.mr-list
+%ul.content-list.mr-list.issuable-list
   = render @merge_requests
   - if @merge_requests.blank?
     %li
diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml
index ace275c689b..144b3a9c8c8 100644
--- a/app/views/projects/merge_requests/index.html.haml
+++ b/app/views/projects/merge_requests/index.html.haml
@@ -1,4 +1,6 @@
 - @no_container = true
+- @bulk_edit = can?(current_user, :admin_merge_request, @project)
+
 - page_title "Merge Requests"
 = render "projects/issues/head"
 = render 'projects/last_push'
diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml
index fabf6d74392..9b5d1236e1d 100644
--- a/app/views/shared/issuable/_filter.html.haml
+++ b/app/views/shared/issuable/_filter.html.haml
@@ -1,9 +1,11 @@
+- boards_page = controller.controller_name == 'boards'
+
 .issues-filters
   .issues-details-filters.row-content-block.second-block
     = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :issue_search]), method: :get, class: 'filter-form js-filter-form' do
       - if params[:issue_search].present?
         = hidden_field_tag :issue_search, params[:issue_search]
-      - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project)
+      - if @bulk_edit
         .check-all-holder
           = check_box_tag "check_all_issues", nil, false,
             class: "check_all_issues left"
@@ -30,7 +32,7 @@
           %a{href: page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :issue_search])} Reset filters
 
         .pull-right
-          - if controller.controller_name == 'boards'
+          - if boards_page
             #js-boards-seach.issue-boards-search
               %input.pull-left.form-control{ type: "search", placeholder: "Filter by name...", "v-model" => "filters.search", "debounce" => "250" }
               - if can?(current_user, :admin_list, @project)
@@ -45,7 +47,7 @@
           - else
             = render 'shared/sort_dropdown'
 
-    - if controller.controller_name == 'issues'
+    - if @bulk_edit
       .issues_bulk_update.hide
         = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post, class: 'bulk-update'  do
           .filter-item.inline
@@ -70,10 +72,10 @@
                 %li
                   %a{href: "#", data: {id: "unsubscribe"}} Unsubscribe
 
-          = hidden_field_tag 'update[issues_ids]', []
+          = hidden_field_tag 'update[issuable_ids]', []
           = hidden_field_tag :state_event, params[:state_event]
           .filter-item.inline
-            = button_tag "Update issues", class: "btn update_selected_issues btn-save"
+            = button_tag "Update #{type.to_s.humanize(capitalize: false)}", class: "btn update_selected_issues btn-save"
 
   - if !@labels.nil?
     .row-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) }
diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index ac08aa53b0b..6f7ce8ca992 100644
--- a/spec/services/issues/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -1,14 +1,14 @@
 require 'spec_helper'
 
-describe Issues::BulkUpdateService, services: true do
+describe Issuable::BulkUpdateService, services: true do
   let(:user)    { create(:user) }
   let(:project) { create(:empty_project, namespace: user.namespace) }
 
   def bulk_update(issues, extra_params = {})
     bulk_update_params = extra_params
-      .reverse_merge(issues_ids: Array(issues).map(&:id).join(','))
+      .reverse_merge(issuable_ids: Array(issues).map(&:id).join(','))
 
-    Issues::BulkUpdateService.new(project, user, bulk_update_params).execute
+    Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute('issue')
   end
 
   describe 'close issues' do