Merge branch 'issue_13499' into 'master'

Allow bulk-assign labels to issues

- [x] Allow bulk-assignment labels.
- [x] Show indeterminate state for labels present on selection.
- [x] Remove existing labels from selected items if label gets unmarked.
- [x] Fix conflicting tests.
- [x] Write tests.

Closes #13499 and #15489

See merge request !3917

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.9.0 (unreleased)
+  - Bulk assign/unassign labels to issues.
   - Allow enabling wiki page events from Webhook management UI
   - Make EmailsOnPushWorker use Sidekiq mailers queue
   - Fix wiki page events' webhook to point to the wiki repository

app/assets/javascripts/dispatcher.js.coffee

@@ -17,6 +17,7 @@ class Dispatcher
     switch page
       when 'projects:issues:index'
         Issuable.init()
+        new IssuableBulkActions()
         shortcut_handler = new ShortcutsNavigation()
       when 'projects:issues:show'
         new Issue()

app/assets/javascripts/flash.js.coffee

 class @Flash
-  constructor: (message, type)->
+  constructor: (message, type = 'alert')->
     @flash = $(".flash-container")
     @flash.html("")
 

app/assets/javascripts/gl_dropdown.js.coffee

@@ -11,6 +11,8 @@ class GitLabDropdownFilter
     $inputContainer = @input.parent()
     $clearButton = $inputContainer.find('.js-dropdown-input-clear')
 
+    @indeterminateIds = []
+
     # Clear click
     $clearButton.on 'click', (e) =>
       e.preventDefault()
@@ -35,20 +37,20 @@ class GitLabDropdownFilter
       if keyCode is 13
         return false
 
-      clearTimeout timeout
-      timeout = setTimeout =>
-        blur_field = @shouldBlur keyCode
-        search_text = @input.val()
+      # Only filter asynchronously only if option remote is set
+      if @options.remote
+        clearTimeout timeout
+        timeout = setTimeout =>
+          blur_field = @shouldBlur keyCode
 
-        if blur_field and @filterInputBlur
-          @input.blur()
+          if blur_field and @filterInputBlur
+            @input.blur()
 
-        if @options.remote
-          @options.query search_text, (data) =>
+          @options.query @input.val(), (data) =>
             @options.callback(data)
-        else
-          @filter search_text
-      , 250
+        , 250
+      else
+        @filter @input.val()
 
   shouldBlur: (keyCode) ->
     return BLUR_KEYCODES.indexOf(keyCode) >= 0
@@ -142,6 +144,7 @@ class GitLabDropdown
   LOADING_CLASS = "is-loading"
   PAGE_TWO_CLASS = "is-page-two"
   ACTIVE_CLASS = "is-active"
+  INDETERMINATE_CLASS = "is-indeterminate"
   currentIndex = -1
 
   FILTER_INPUT = '.dropdown-input .dropdown-input-field'
@@ -182,9 +185,6 @@ class GitLabDropdown
             @fullData = data
 
             @parseData @fullData
-
-            if @options.filterable
-              @filterInput.trigger 'keyup'
         }
 
     # Init filterable
@@ -298,6 +298,13 @@ class GitLabDropdown
   opened: =>
     @addArrowKeyEvent()
 
+    if @options.setIndeterminateIds
+      @options.setIndeterminateIds.call(@)
+
+    # Makes indeterminate items effective
+    if @fullData and @dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update')
+      @parseData @fullData
+
     contentHtml = $('.dropdown-content', @dropdown).html()
     if @remote && contentHtml is ""
       @remote.execute()
@@ -309,12 +316,18 @@ class GitLabDropdown
 
   hidden: (e) =>
     @removeArrayKeyEvent()
+
+    $input = @dropdown.find(".dropdown-input-field")
+
     if @options.filterable
-      @dropdown
-        .find(".dropdown-input-field")
+      $input
         .blur()
         .val("")
-        .trigger("keyup")
+
+    # Triggering 'keyup' will re-render the dropdown which is not always required
+    # specially if we want to keep the state of the dropdown needed for bulk-assignment
+    if not @options.persistWhenHide
+      $input.trigger("keyup")
 
     if @dropdown.find(".dropdown-toggle-page").length
       $('.dropdown-menu', @dropdown).removeClass PAGE_TWO_CLASS
@@ -358,7 +371,7 @@ class GitLabDropdown
 
     if @options.renderRow
       # Call the render function
-      html = @options.renderRow(data)
+      html = @options.renderRow.call(@options, data, @)
     else
       if not selected
         value = if @options.id then @options.id(data) else data.id
@@ -443,6 +456,17 @@ class GitLabDropdown
         $(@el).find(".dropdown-toggle-text").text @options.toggleLabel
       else
         selectedObject
+    else if el.hasClass(INDETERMINATE_CLASS)
+      el.addClass ACTIVE_CLASS
+      el.removeClass INDETERMINATE_CLASS
+
+      if not value?
+        field.remove()
+
+      if not field.length and fieldName
+        @addInput(fieldName, value)
+
+      return selectedObject
     else
       if not @options.multiSelect or el.hasClass('dropdown-clear-active')
         @dropdown.find(".#{ACTIVE_CLASS}").removeClass ACTIVE_CLASS
@@ -459,17 +483,23 @@ class GitLabDropdown
         $(@el).find(".dropdown-toggle-text").text @options.toggleLabel(selectedObject, el)
       if value?
         if !field.length and fieldName
-          # Create hidden input for form
-          input = "<input type='hidden' name='#{fieldName}' value='#{value}' />"
-          if @options.inputId?
-            input = $(input)
-                      .attr('id', @options.inputId)
-          @dropdown.before input
+          @addInput(fieldName, value)
         else
           field.val value
 
       return selectedObject
 
+  addInput: (fieldName, value)->
+    # Create hidden input for form
+    $input = $('<input>').attr('type', 'hidden')
+                         .attr('name', fieldName)
+                        .val(value)
+
+    if @options.inputId?
+      $input.attr('id', @options.inputId)
+
+    @dropdown.before $input
+
   selectRowAtIndex: (e, index) ->
     selector = ".dropdown-content li:not(.divider,.dropdown-header,.separator):eq(#{index}) a"
 

app/assets/javascripts/issues-bulk-assignment.js.coffee

+class @IssuableBulkActions
+  constructor: (opts = {}) ->
+    # Set defaults
+    {
+      @container = $('.content')
+      @form = @getElement('.bulk-update')
+      @issues = @getElement('.issues-list .issue')
+    } = opts
+
+    @bindEvents()
+
+  getElement: (selector) ->
+    @container.find selector
+
+  bindEvents: ->
+    @form.off('submit').on('submit', @onFormSubmit.bind(@))
+
+  onFormSubmit: (e) ->
+    e.preventDefault()
+    @submit()
+
+  submit: ->
+    _this = @
+
+    xhr = $.ajax
+            url: @form.attr 'action'
+            method: @form.attr 'method'
+            dataType: 'JSON',
+            data: @getFormDataAsObject()
+
+    xhr.done (response, status, xhr) ->
+      location.reload()
+
+    xhr.fail ->
+      new Flash("Issue update failed")
+
+    xhr.always @onFormSubmitAlways.bind(@)
+
+  onFormSubmitAlways: ->
+    @form.find('[type="submit"]').enable()
+
+  getSelectedIssues: ->
+    @issues.has('.selected_issue:checked')
+
+  getLabelsFromSelection: ->
+    labels = []
+
+    @getSelectedIssues().map ->
+      _labels = $(@).data('labels')
+      if _labels
+        _labels.map (labelId) ->
+          labels.push(labelId) if labels.indexOf(labelId) is -1
+
+    labels
+
+  ###*
+   * Will return only labels that were marked previously and the user has unmarked
+   * @return {Array} Label IDs
+  ###
+  getUnmarkedIndeterminedLabels: ->
+    result = []
+    labelsToKeep = []
+
+    for el in @getElement('.labels-filter .is-indeterminate')
+      labelsToKeep.push $(el).data('labelId')
+
+    for id in @getLabelsFromSelection()
+      # Only the ones that we are not going to keep
+      result.push(id) if labelsToKeep.indexOf(id) is -1
+
+    result
+
+  ###*
+   * Simple form serialization, it will return just what we need
+   * Returns key/value pairs from form data
+  ###
+  getFormDataAsObject: ->
+    formData =
+      update:
+        state_event       : @form.find('input[name="update[state_event]"]').val()
+        assignee_id       : @form.find('input[name="update[assignee_id]"]').val()
+        milestone_id      : @form.find('input[name="update[milestone_id]"]').val()
+        issues_ids        : @form.find('input[name="update[issues_ids]"]').val()
+        add_label_ids     : []
+        remove_label_ids  : []
+
+    @getLabelsToApply().map (id) ->
+      formData.update.add_label_ids.push id
+
+    @getLabelsToRemove().map (id) ->
+      formData.update.remove_label_ids.push id
+
+    formData
+
+  getLabelsToApply: ->
+    labelIds = []
+    $labels = @form.find('.labels-filter input[name="update[label_ids][]"]')
+
+    $labels.each (k, label) ->
+      labelIds.push $(label).val() if label
+
+    labelIds
+
+  ###*
+   * Just an alias of @getUnmarkedIndeterminedLabels
+   * @return {Array} Array of labels
+  ###
+  getLabelsToRemove: ->
+    @getUnmarkedIndeterminedLabels()

app/assets/javascripts/labels_select.js.coffee

 class @LabelsSelect
   constructor: ->
+    _this = @
+
     $('.js-label-select').each (i, dropdown) ->
       $dropdown = $(dropdown)
       projectId = $dropdown.data('project-id')
@@ -196,10 +198,18 @@ class @LabelsSelect
 
             callback data
 
-        renderRow: (label) ->
-          removesAll = label.id is 0 or not label.id?
+        renderRow: (label, instance) ->
+          $li = $('<li>')
+          $a  = $('<a href="#">')
 
           selectedClass = []
+          removesAll = label.id is 0 or not label.id?
+
+          if $dropdown.hasClass('js-filter-bulk-update')
+            indeterminate = instance.indeterminateIds
+            if indeterminate.indexOf(label.id) isnt -1
+              selectedClass.push 'is-indeterminate'
+
           if $form.find("input[type='hidden']\
             [name='#{$dropdown.data('fieldName')}']\
             [value='#{this.id(label)}']").length
@@ -230,13 +240,17 @@ class @LabelsSelect
           else
             colorEl = ''
 
-          "<li>
-            <a href='#' class='#{selectedClass.join(' ')}'>
-              #{colorEl}
-              #{_.escape(label.title)}
-            </a>
-          </li>"
-        filterable: true
+          # We need to identify which items are actually labels
+          if label.id
+            selectedClass.push('label-item')
+            $a.attr('data-label-id', label.id)
+
+          $a.addClass(selectedClass.join(' '))
+            .html("#{colorEl} #{_.escape(label.title)}")
+
+          # Return generated html
+          $li.html($a).prop('outerHTML')
+        persistWhenHide: $dropdown.data('persistWhenHide')
         search:
           fields: ['title']
         selectable: true
@@ -280,10 +294,19 @@ class @LabelsSelect
             else if $dropdown.hasClass('js-filter-submit')
               $dropdown.closest('form').submit()
             else
-              saveLabelData()
+              if not $dropdown.hasClass 'js-filter-bulk-update'
+                saveLabelData()
+
+          if $dropdown.hasClass('js-filter-bulk-update')
+            # If we are persisting state we need the classes
+            if not @options.persistWhenHide
+              $dropdown.parent().find('.is-active, .is-indeterminate').removeClass()
 
         multiSelect: $dropdown.hasClass 'js-multiselect'
         clicked: (label) ->
+          if $dropdown.hasClass('js-filter-bulk-update')
+            return
+
           page = $('body').data 'page'
           isIssueIndex = page is 'projects:issues:index'
           isMRIndex = page is 'projects:merge_requests:index'
@@ -298,4 +321,31 @@ class @LabelsSelect
               return
             else
               saveLabelData()
+
+        setIndeterminateIds: ->
+          if @dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update')
+            @indeterminateIds = _this.getIndeterminateIds()
       )
+
+    @bindEvents()
+
+  bindEvents: ->
+    $('body').on 'change', '.selected_issue', @onSelectCheckboxIssue
+
+  onSelectCheckboxIssue: ->
+    return if $('.selected_issue:checked').length
+
+    # Remove inputs
+    $('.issues_bulk_update .labels-filter input[type="hidden"]').remove()
+
+    # Also restore button text
+    $('.issues_bulk_update .labels-filter .dropdown-toggle-text').text('Label')
+
+  getIndeterminateIds: ->
+    label_ids = []
+
+    $('.selected_issue:checked').each (i, el) ->
+      issue_id = $(el).data('id')
+      label_ids.push $("#issue_#{issue_id}").data('labels')
+
+    _.flatten(label_ids)

app/assets/stylesheets/framework/dropdowns.scss

@@ -232,9 +232,8 @@
   a {
     padding-left: 25px;
 
-    &.is-active {
+    &.is-indeterminate, &.is-active {
       &::before {
-        content: "\f00c";
         position: absolute;
         left: 5px;
         top: 50%;
@@ -246,6 +245,14 @@
         -moz-osx-font-smoothing: grayscale;
       }
     }
+
+    &.is-indeterminate::before {
+      content: "\f068";
+    }
+
+    &.is-active::before {
+      content: "\f00c";
+    }
   }
 }
 

app/controllers/projects/issues_controller.rb

@@ -156,7 +156,12 @@ class Projects::IssuesController < Projects::ApplicationController
 
   def bulk_update
     result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute
-    redirect_back_or_default(default: { action: 'index' }, options: { notice: "#{result[:count]} issues updated" })
+
+    respond_to do |format|
+      format.json do
+        render json: { notice: "#{result[:count]} issues updated" }
+      end
+    end
   end
 
   protected
@@ -216,7 +221,10 @@ class Projects::IssuesController < Projects::ApplicationController
       :issues_ids,
       :assignee_id,
       :milestone_id,
-      :state_event
+      :state_event,
+      label_ids: [],
+      add_label_ids: [],
+      remove_label_ids: []
     )
   end
 end

app/helpers/issuables_helper.rb

@@ -96,5 +96,4 @@ module IssuablesHelper
       issuable.open? ? :opened : :closed
     end
   end
-
 end

app/services/issuable_base_service.rb

@@ -45,6 +45,8 @@ class IssuableBaseService < BaseService
 
     unless can?(current_user, ability, project)
       params.delete(:milestone_id)
+      params.delete(:add_label_ids)
+      params.delete(:remove_label_ids)
       params.delete(:label_ids)
       params.delete(:assignee_id)
     end
@@ -67,10 +69,34 @@ class IssuableBaseService < BaseService
   end
 
   def filter_labels
-    return if params[:label_ids].to_a.empty?
+    if params[:add_label_ids].present? || params[:remove_label_ids].present?
+      params.delete(:label_ids)
+
+      filter_labels_in_param(:add_label_ids)
+      filter_labels_in_param(:remove_label_ids)
+    else
+      filter_labels_in_param(:label_ids)
+    end
+  end
+
+  def filter_labels_in_param(key)
+    return if params[key].to_a.empty?
 
-    params[:label_ids] =
-      project.labels.where(id: params[:label_ids]).pluck(:id)
+    params[key] = project.labels.where(id: params[key]).pluck(:id)
+  end
+
+  def update_issuable(issuable, attributes)
+    issuable.with_transaction_returning_status do
+      add_label_ids = attributes.delete(:add_label_ids)
+      remove_label_ids = attributes.delete(:remove_label_ids)
+
+      issuable.label_ids |= add_label_ids if add_label_ids
+      issuable.label_ids -= remove_label_ids if remove_label_ids
+
+      issuable.assign_attributes(attributes.merge(updated_by: current_user))
+
+      issuable.save
+    end
   end
 
   def update(issuable)
@@ -78,7 +104,7 @@ class IssuableBaseService < BaseService
     filter_params
     old_labels = issuable.labels.to_a
 
-    if params.present? && issuable.update_attributes(params.merge(updated_by: current_user))
+    if params.present? && update_issuable(issuable, params)
       issuable.reset_events_cache
       handle_common_system_notes(issuable, old_labels: old_labels)
       handle_changes(issuable, old_labels: old_labels)

app/services/issues/bulk_update_service.rb

@@ -4,9 +4,9 @@ module Issues
       issues_ids   = params.delete(:issues_ids).split(",")
       issue_params = params
 
-      issue_params.delete(:state_event)   unless issue_params[:state_event].present?
-      issue_params.delete(:milestone_id)  unless issue_params[:milestone_id].present?
-      issue_params.delete(:assignee_id)   unless issue_params[:assignee_id].present?
+      %i(state_event milestone_id assignee_id add_label_ids remove_label_ids).each do |key|
+        issue_params.delete(key) unless issue_params[key].present?
+      end
 
       issues = Issue.where(id: issues_ids)
       issues.each do |issue|

app/views/projects/issues/_issue.html.haml

-%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue) }
+%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)
     .issue-check
       = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue"

app/views/projects/labels/_label.html.haml

-%li{id: dom_id(label)}
+%li{ id: dom_id(label), data: { id: label.id } }
   = render "shared/label_row", label: label
-
   .pull-info-right
     %span.append-right-20
       = link_to_label(label, type: :merge_request) do

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

@@ -31,7 +31,7 @@
 
     - if controller.controller_name == 'issues'
       .issues_bulk_update.hide
-        = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post  do
+        = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), 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
@@ -44,6 +44,10 @@
               placeholder: "Search authors", data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: @project.id, field_name: "update[assignee_id]" } })
           .filter-item.inline
             = dropdown_tag("Milestone", options: { title: "Assign milestone", toggle_class: 'js-milestone-select js-extra-options js-filter-submit js-filter-bulk-update', filter: true, dropdown_class: "dropdown-menu-selectable dropdown-menu-milestone", placeholder: "Search milestones", data: { show_no: true, field_name: "update[milestone_id]", project_id: @project.id, milestones: namespace_project_milestones_path(@project.namespace, @project, :json), use_id: true } })
+
+          .filter-item.inline.labels-filter
+            = render "shared/issuable/label_dropdown", classes: ['js-filter-bulk-update', 'js-multiselect'], show_create: false, show_footer: false, extra_options: false, filter_submit: false, show_footer: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true }
+
           = hidden_field_tag 'update[issues_ids]', []
           = hidden_field_tag :state_event, params[:state_event]
           .filter-item.inline

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

+- show_create = local_assigns.fetch(:show_create, true)
+- extra_options = local_assigns.fetch(:extra_options, true)
+- filter_submit = local_assigns.fetch(:filter_submit, true)
+- show_footer = local_assigns.fetch(:show_footer, true)
+- data_options = local_assigns.fetch(:data_options, {})
+- classes = local_assigns.fetch(:classes, [])
+- dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}
+- dropdown_data.merge!(data_options)
+- classes << 'js-extra-options' if extra_options
+- classes << 'js-filter-submit' if filter_submit
+
 - if params[:label_name].present?
   - if params[:label_name].respond_to?('any?')
     - params[:label_name].each do |label|
       = hidden_field_tag "label_name[]", label, id: nil
 .dropdown
-  %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}}
+  %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data}
     %span.dropdown-toggle-text
       = h(multi_label_name(params[:label_name], "Label"))
     = icon('chevron-down')
   .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable
-    = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label" }
-    - if can? current_user, :admin_label, @project and @project
+    = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label", show_footer: show_footer, show_create: show_create }
+    - if show_create and @project and can?(current_user, :admin_label, @project)
       = render partial: "shared/issuable/label_page_create"
     = dropdown_loading

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

 - title = local_assigns.fetch(:title, 'Assign labels')
+- show_create = local_assigns.fetch(:show_create, true)
+- show_footer = local_assigns.fetch(:show_footer, true)
 - filter_placeholder = local_assigns.fetch(:filter_placeholder, 'Search labels')
 .dropdown-page-one
   = dropdown_title(title)
   = dropdown_filter(filter_placeholder)
   = dropdown_content
-  - if @project
+  - if @project && show_footer
     = dropdown_footer do
       %ul.dropdown-footer-list
-        - if can? current_user, :admin_label, @project
+        - if can?(current_user, :admin_label, @project)
           %li
             %a.dropdown-toggle-page{href: "#"}
               Create new
         %li
           = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do
-            - if can? current_user, :admin_label, @project
+            - if show_create && @project && can?(current_user, :admin_label, @project)
               Manage labels
             - else
               View labels
-  = dropdown_loading
+  = dropdown_loading
\ No newline at end of file

features/steps/project/issues/filter_labels.rb

@@ -29,7 +29,7 @@ class Spinach::Features::ProjectIssuesFilterLabels < Spinach::FeatureSteps
   end
 
   step 'I click link "bug"' do
-    page.find('.js-label-select').click
+    page.find('.js-label-select', visible: true).click
     sleep 0.5
     execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()")
   end

spec/features/issues/bulk_assigment_labels_spec.rb

+require 'rails_helper'
+
+feature 'Issues > Labels bulk assignment', feature: true do
+  include WaitForAjax
+
+  let(:user)      { create(:user) }
+  let!(:project)  { create(:project) }
+  let!(:issue1)   { create(:issue, project: project, title: "Issue 1") }
+  let!(:issue2)   { create(:issue, project: project, title: "Issue 2") }
+  let!(:bug)      { create(:label, project: project, title: 'bug') }
+  let!(:feature)  { create(:label, project: project, title: 'feature') }
+
+  context 'as a allowed user', js: true do
+    before do
+      project.team << [user, :master]
+
+      login_as user
+    end
+
+    context 'can bulk assign' do
+      before do
+        visit namespace_project_issues_path(project.namespace, project)
+      end
+
+      context 'a label' do
+        context 'to all issues' do
+          before do
+            check 'check_all_issues'
+            open_labels_dropdown ['bug']
+            update_issues
+          end
+
+          it do
+            expect(find("#issue_#{issue1.id}")).to have_content 'bug'
+            expect(find("#issue_#{issue2.id}")).to have_content 'bug'
+          end
+        end
+
+        context 'to a issue' do
+          before do
+            check "selected_issue_#{issue1.id}"
+            open_labels_dropdown ['bug']
+            update_issues
+          end
+
+          it do
+            expect(find("#issue_#{issue1.id}")).to have_content 'bug'
+            expect(find("#issue_#{issue2.id}")).not_to have_content 'bug'
+          end
+        end
+      end
+
+      context 'multiple labels' do
+        context 'to all issues' do
+          before do
+            check 'check_all_issues'
+            open_labels_dropdown ['bug', 'feature']
+            update_issues
+          end
+
+          it do
+            expect(find("#issue_#{issue1.id}")).to have_content 'bug'
+            expect(find("#issue_#{issue1.id}")).to have_content 'feature'
+            expect(find("#issue_#{issue2.id}")).to have_content 'bug'
+            expect(find("#issue_#{issue2.id}")).to have_content 'feature'
+          end
+        end
+
+        context 'to a issue' do
+          before do
+            check "selected_issue_#{issue1.id}"
+            open_labels_dropdown ['bug', 'feature']
+            update_issues
+          end
+
+          it do
+            expect(find("#issue_#{issue1.id}")).to have_content 'bug'
+            expect(find("#issue_#{issue1.id}")).to have_content 'feature'
+            expect(find("#issue_#{issue2.id}")).not_to have_content 'bug'
+            expect(find("#issue_#{issue2.id}")).not_to have_content 'feature'
+          end
+        end
+      end
+    end
+
+    context 'can bulk un-assign' do
+      context 'all labels to all issues' do
+        before do
+          issue1.labels << bug
+          issue1.labels << feature
+          issue2.labels << bug
+          issue2.labels << feature
+
+          visit namespace_project_issues_path(project.namespace, project)
+
+          check 'check_all_issues'
+          unmark_labels_in_dropdown ['bug', 'feature']
+          update_issues
+        end
+
+        it do
+          expect(find("#issue_#{issue1.id}")).not_to have_content 'bug'
+          expect(find("#issue_#{issue1.id}")).not_to have_content 'feature'
+          expect(find("#issue_#{issue2.id}")).not_to have_content 'bug'
+          expect(find("#issue_#{issue2.id}")).not_to have_content 'feature'
+        end
+      end
+
+      context 'a label to a issue' do
+        before do
+          issue1.labels << bug
+          issue2.labels << feature
+
+          visit namespace_project_issues_path(project.namespace, project)
+
+          check_issue issue1
+          unmark_labels_in_dropdown ['bug']
+          update_issues
+        end
+
+        it do
+          expect(find("#issue_#{issue1.id}")).not_to have_content 'bug'
+          expect(find("#issue_#{issue2.id}")).to have_content 'feature'
+        end
+      end
+
+      context 'a label and keep the others label' do
+        before do
+          issue1.labels << bug
+          issue1.labels << feature
+          issue2.labels << bug
+          issue2.labels << feature
+
+          visit namespace_project_issues_path(project.namespace, project)
+
+          check_issue issue1
+          check_issue issue2
+          unmark_labels_in_dropdown ['bug']
+          update_issues
+        end
+
+        it do
+          expect(find("#issue_#{issue1.id}")).not_to have_content 'bug'
+          expect(find("#issue_#{issue1.id}")).to have_content 'feature'
+          expect(find("#issue_#{issue2.id}")).not_to have_content 'bug'
+          expect(find("#issue_#{issue2.id}")).to have_content 'feature'
+        end
+      end
+    end
+  end
+
+  context 'as a guest' do
+    before do
+      login_as user
+
+      visit namespace_project_issues_path(project.namespace, project)
+    end
+
+    context 'cannot bulk assign labels' do
+      it do
+        expect(page).not_to have_css '.check_all_issues'
+        expect(page).not_to have_css '.issue-check'
+      end
+    end
+  end
+
+  def open_labels_dropdown(items = [], unmark = false)
+    page.within('.issues_bulk_update') do
+      click_button 'Label'
+      wait_for_ajax
+      items.map do |item|
+        click_link item
+      end
+      if unmark
+        items.map do |item|
+          click_link item
+        end
+      end
+    end
+  end
+
+  def unmark_labels_in_dropdown(items = [])
+    open_labels_dropdown(items, true)
+  end
+
+  def check_issue(issue)
+    page.within('.issues-list') do
+      check "selected_issue_#{issue.id}"
+    end
+  end
+
+  def update_issues
+    click_button 'Update issues'
+    wait_for_ajax
+  end
+end

spec/features/issues/update_issues_spec.rb

 require 'rails_helper'
 
 feature 'Multiple issue updating from issues#index', feature: true do
+  include WaitForAjax
+
   let!(:project)   { create(:project) }
   let!(:issue)     { create(:issue, project: project) }
   let!(:user)      { create(:user)}
@@ -24,9 +26,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
 
     it 'should be set to open' do
       create_closed
-      visit namespace_project_issues_path(project.namespace, project)
-
-      find('.issues-state-filters a', text: 'Closed').click
+      visit namespace_project_issues_path(project.namespace, project, state: 'closed')
 
       find('#check_all_issues').click
       find('.js-issue-status').click
@@ -42,7 +42,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
       visit namespace_project_issues_path(project.namespace, project)
 
       find('#check_all_issues').click
-      find('.js-update-assignee').click
+      click_update_assignee_button
 
       find('.dropdown-menu-user-link', text: user.username).click
       click_update_issues_button
@@ -57,14 +57,11 @@ feature 'Multiple issue updating from issues#index', feature: true do
       visit namespace_project_issues_path(project.namespace, project)
 
       find('#check_all_issues').click
-      find('.js-update-assignee').click
+      click_update_assignee_button
 
       click_link 'Unassigned'
       click_update_issues_button
-
-      within first('.issue .controls') do
-        expect(page).to have_no_selector('.author_link')
-      end
+      expect(find('.issue:first-child .controls')).not_to have_css('.author_link')
     end
   end
 
@@ -95,7 +92,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
       find('.dropdown-menu-milestone a', text: "No Milestone").click
       click_update_issues_button
 
-      expect(first('.issue')).not_to have_content milestone.title
+      expect(find('.issue:first-child')).not_to have_content milestone.title
     end
   end
 
@@ -111,7 +108,13 @@ feature 'Multiple issue updating from issues#index', feature: true do
     create(:issue, project: project, milestone: milestone)
   end
 
+  def click_update_assignee_button
+    find('.js-update-assignee').click
+    wait_for_ajax
+  end
+
   def click_update_issues_button
     find('.update_selected_issues').click
+    wait_for_ajax
   end
 end

spec/services/issues/bulk_update_service_spec.rb

 require 'spec_helper'
 
 describe Issues::BulkUpdateService, services: true do
-  let(:issue) { create(:issue, project: @project) }
-
-  before do
-    @user = create :user
-    opts = {
-      name: "GitLab",
-      namespace: @user.namespace
-    }
-    @project = Projects::CreateService.new(@user, opts).execute
-  end
+  let(:user) { create(:user) }
+  let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute }
 
-  describe :close_issue do
+  let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute }
 
-    before do
-      @issues = create_list(:issue, 5, project: @project)
-      @params = {
+  describe :close_issue do
+    let(:issues) { create_list(:issue, 5, project: project) }
+    let(:params) do
+      {
         state_event: 'close',
-        issues_ids: @issues.map(&:id).join(",")
+        issues_ids: issues.map(&:id).join(',')
       }
     end
 
-    it do
-      result = Issues::BulkUpdateService.new(@project, @user, @params).execute
+    it 'succeeds and returns the correct number of issues updated' do
       expect(result[:success]).to be_truthy
-      expect(result[:count]).to eq(@issues.count)
-
-      expect(@project.issues.opened).to be_empty
-      expect(@project.issues.closed).not_to be_empty
+      expect(result[:count]).to eq(issues.count)
     end
 
+    it 'closes all the issues passed' do
+      expect(project.issues.opened).to be_empty
+      expect(project.issues.closed).not_to be_empty
+    end
   end
 
   describe :reopen_issues do
-    before do
-      @issues = create_list(:closed_issue, 5, project: @project)
-      @params = {
+    let(:issues) { create_list(:closed_issue, 5, project: project) }
+    let(:params) do
+      {
         state_event: 'reopen',
-        issues_ids: @issues.map(&:id).join(",")
+        issues_ids: issues.map(&:id).join(',')
       }
     end
 
-    it do
-      result = Issues::BulkUpdateService.new(@project, @user, @params).execute
+    it 'succeeds and returns the correct number of issues updated' do
       expect(result[:success]).to be_truthy
-      expect(result[:count]).to eq(@issues.count)
-
-      expect(@project.issues.closed).to be_empty
-      expect(@project.issues.opened).not_to be_empty
+      expect(result[:count]).to eq(issues.count)
     end
 
+    it 'reopens all the issues passed' do
+      expect(project.issues.closed).to be_empty
+      expect(project.issues.opened).not_to be_empty
+    end
   end
 
-  describe :update_assignee do
+  describe 'updating assignee' do
+    let(:issue) do
+      create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) }
+    end
 
-    before do
-      @new_assignee = create :user
-      @params = {
-        issues_ids: issue.id.to_s,
-        assignee_id: @new_assignee.id
+    let(:params) do
+      {
+        assignee_id: assignee_id,
+        issues_ids: issue.id.to_s
       }
     end
 
-    it do
-      result = Issues::BulkUpdateService.new(@project, @user, @params).execute
-      expect(result[:success]).to be_truthy
-      expect(result[:count]).to eq(1)
+    context 'when the new assignee ID is a valid user' do
+      let(:new_assignee) { create(:user) }
+      let(:assignee_id) { new_assignee.id }
 
-      expect(@project.issues.first.assignee).to eq(@new_assignee)
-    end
+      it 'succeeds' do
+        expect(result[:success]).to be_truthy
+        expect(result[:count]).to eq(1)
+      end
 
-    it 'allows mass-unassigning' do
-      @project.issues.first.update_attribute(:assignee, @new_assignee)
-      expect(@project.issues.first.assignee).not_to be_nil
+      it 'updates the assignee to the use ID passed' do
+        expect(issue.reload.assignee).to eq(new_assignee)
+      end
+    end
 
-      @params[:assignee_id] = -1
+    context 'when the new assignee ID is -1' do
+      let(:assignee_id) { -1 }
 
-      Issues::BulkUpdateService.new(@project, @user, @params).execute
-      expect(@project.issues.first.assignee).to be_nil
+      it 'unassigns the issues' do
+        expect(issue.reload.assignee).to be_nil
+      end
     end
 
-    it 'does not unassign when assignee_id is not present' do
-      @project.issues.first.update_attribute(:assignee, @new_assignee)
-      expect(@project.issues.first.assignee).not_to be_nil
-
-      @params[:assignee_id] = ''
+    context 'when the new assignee ID is not present' do
+      let(:assignee_id) { nil }
 
-      Issues::BulkUpdateService.new(@project, @user, @params).execute
-      expect(@project.issues.first.assignee).not_to be_nil
+      it 'does not unassign' do
+        expect(issue.reload.assignee).to eq(user)
+      end
     end
   end
 
-  describe :update_milestone do
+  describe 'updating milestones' do
+    let(:issue) { create(:issue, project: project) }
+    let(:milestone) { create(:milestone, project: project) }
 
-    before do
-      @milestone = create(:milestone, project: @project)
-      @params = {
+    let(:params) do
+      {
         issues_ids: issue.id.to_s,
-        milestone_id: @milestone.id
+        milestone_id: milestone.id
       }
     end
 
-    it do
-      result = Issues::BulkUpdateService.new(@project, @user, @params).execute
+    it 'succeeds' do
       expect(result[:success]).to be_truthy
       expect(result[:count]).to eq(1)
+    end
 
-      expect(@project.issues.first.milestone).to eq(@milestone)
+    it 'updates the issue milestone' do
+      expect(project.issues.first.milestone).to eq(milestone)
     end
   end
 
+  describe 'updating labels' do
+    def create_issue_with_labels(labels)
+      create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) }
+    end
+
+    let(:bug) { create(:label, project: project) }
+    let(:regression) { create(:label, project: project) }
+    let(:merge_requests) { create(:label, project: project) }
+
+    let(:issue_all_labels) { create_issue_with_labels([bug, regression, merge_requests]) }
+    let(:issue_bug_and_regression) { create_issue_with_labels([bug, regression]) }
+    let(:issue_bug_and_merge_requests) { create_issue_with_labels([bug, merge_requests]) }
+    let(:issue_no_labels) { create(:issue, project: project) }
+    let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] }
+
+    let(:labels) { [] }
+    let(:add_labels) { [] }
+    let(:remove_labels) { [] }
+
+    let(:params) do
+      {
+        label_ids: labels.map(&:id),
+        add_label_ids: add_labels.map(&:id),
+        remove_label_ids: remove_labels.map(&:id),
+        issues_ids: issues.map(&:id).join(',')
+      }
+    end
+
+    context 'when label_ids are passed' do
+      let(:issues) { [issue_all_labels, issue_no_labels] }
+      let(:labels) { [bug, regression] }
+
+      it 'updates the labels of all issues passed to the labels passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(eq(labels.map(&:id)))
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
+      end
+
+      context 'when those label IDs are empty' do
+        let(:labels) { [] }
+
+        it 'updates the issues passed to have no labels' do
+          expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty)
+        end
+      end
+    end
+
+    context 'when add_label_ids are passed' do
+      let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] }
+      let(:add_labels) { [bug, regression, merge_requests] }
+
+      it 'adds those label IDs to all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(include(*add_labels.map(&:id)))
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
+      end
+    end
+
+    context 'when remove_label_ids are passed' do
+      let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] }
+      let(:remove_labels) { [bug, regression, merge_requests] }
+
+      it 'removes those label IDs from all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty)
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
+      end
+    end
+
+    context 'when add_label_ids and remove_label_ids are passed' do
+      let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] }
+      let(:add_labels) { [bug] }
+      let(:remove_labels) { [merge_requests] }
+
+      it 'adds the label IDs to all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
+      end
+
+      it 'removes the label IDs from all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id)
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
+      end
+    end
+
+    context 'when add_label_ids and label_ids are passed' do
+      let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests] }
+      let(:labels) { [merge_requests] }
+      let(:add_labels) { [regression] }
+
+      it 'adds the label IDs to all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(include(regression.id))
+      end
+
+      it 'ignores the label IDs parameter' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_no_labels.label_ids).to be_empty
+      end
+    end
+
+    context 'when remove_label_ids and label_ids are passed' do
+      let(:issues) { [issue_no_labels, issue_bug_and_regression] }
+      let(:labels) { [merge_requests] }
+      let(:remove_labels) { [regression] }
+
+      it 'remove the label IDs from all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id)
+      end
+
+      it 'ignores the label IDs parameter' do
+        expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id)
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_all_labels.label_ids).to contain_exactly(bug.id, regression.id, merge_requests.id)
+      end
+    end
+
+    context 'when add_label_ids, remove_label_ids, and label_ids are passed' do
+      let(:issues) { [issue_bug_and_merge_requests, issue_no_labels] }
+      let(:labels) { [regression] }
+      let(:add_labels) { [bug] }
+      let(:remove_labels) { [merge_requests] }
+
+      it 'adds the label IDs to all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
+      end
+
+      it 'removes the label IDs from all issues passed' do
+        expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id)
+      end
+
+      it 'ignores the label IDs parameter' do
+        expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id)
+      end
+
+      it 'does not update issues not passed in' do
+        expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
+      end
+    end
+  end
 end

spec/services/issues/update_service_spec.rb

+# coding: utf-8
 require 'spec_helper'
 
 describe Issues::UpdateService, services: true do
@@ -273,5 +274,50 @@ describe Issues::UpdateService, services: true do
         end
       end
     end
+
+    context 'updating labels' do
+      let(:label3) { create(:label, project: project) }
+      let(:result) { Issues::UpdateService.new(project, user, params).execute(issue).reload }
+
+      context 'when add_label_ids and label_ids are passed' do
+        let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } }
+
+        it 'ignores the label_ids parameter' do
+          expect(result.label_ids).not_to include(label.id)
+        end
+
+        it 'adds the passed labels' do
+          expect(result.label_ids).to include(label3.id)
+        end
+      end
+
+      context 'when remove_label_ids and label_ids are passed' do
+        let(:params) { { label_ids: [], remove_label_ids: [label.id] } }
+
+        before { issue.update_attributes(labels: [label, label3]) }
+
+        it 'ignores the label_ids parameter' do
+          expect(result.label_ids).not_to be_empty
+        end
+
+        it 'removes the passed labels' do
+          expect(result.label_ids).not_to include(label.id)
+        end
+      end
+
+      context 'when add_label_ids and remove_label_ids are passed' do
+        let(:params) { { add_label_ids: [label3.id], remove_label_ids: [label.id] } }
+
+        before { issue.update_attributes(labels: [label]) }
+
+        it 'adds the passed labels' do
+          expect(result.label_ids).to include(label3.id)
+        end
+
+        it 'removes the passed labels' do
+          expect(result.label_ids).not_to include(label.id)
+        end
+      end
+    end
   end
 end