fix for Follow-up from "Backport of Multiple Assignees featurefixes_for_multiple_issue_assignees

12 files changed, 64 insertions, 44 deletions

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index b199f18da1e..4cf645d6341 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -60,17 +60,24 @@ module IssuableActions
   end
 
   def bulk_update_params
-    params.require(:update).permit(
+    permitted_keys = [
       :issuable_ids,
       :assignee_id,
       :milestone_id,
       :state_event,
       :subscription_event,
-      assignee_ids: [],
       label_ids: [],
       add_label_ids: [],
       remove_label_ids: []
-    )
+    ]
+
+    if resource_name == 'issue'
+      permitted_keys << { assignee_ids: [] }
+    else
+      permitted_keys.unshift(:assignee_id)
+    end
+
+    params.require(:update).permit(permitted_keys)
   end
 
   def resource_name
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index fbbce6876c2..f7d0ebcb16f 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -67,9 +67,10 @@ module IssuablesHelper
   end
 
   def users_dropdown_label(selected_users)
-    if selected_users.length == 0
+    case selected_users.length
+    when 0
       "Unassigned"
-    elsif selected_users.length == 1
+    when 1
       selected_users[0].name
     else
       "#{selected_users[0].name} + #{selected_users.length - 1} more"
diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb
index 40ff9b8b867..5d42a89fced 100644
--- a/app/services/issuable/bulk_update_service.rb
+++ b/app/services/issuable/bulk_update_service.rb
@@ -7,7 +7,7 @@ module Issuable
       ids = params.delete(:issuable_ids).split(",")
       items = model_class.where(id: ids)
 
-      %i(state_event milestone_id assignee_id assignee_ids add_label_ids remove_label_ids subscription_event).each do |key|
+      permitted_attrs(type).each do |key|
         params.delete(key) unless params[key].present?
       end
 
@@ -26,5 +26,17 @@ module Issuable
         success:  !items.count.zero?
       }
     end
+
+    private
+
+    def permitted_attrs(type)
+      attrs = %i(state_event milestone_id assignee_id assignee_ids add_label_ids remove_label_ids subscription_event)
+
+      if type == 'issue'
+        attrs.push(:assignee_ids)
+      else
+        attrs.push(:assignee_id)
+      end
+    end
   end
 end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 174e7c6e95b..0766df50ed2 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -79,7 +79,7 @@ module SystemNoteService
 
         text_parts.join(' and ')
       elsif old_assignees.any?
-        "removed all assignees"
+        "removed assignee"
       elsif issue.assignees.any?
         "assigned to #{issue.assignees.map(&:to_reference).to_sentence}"
       end
diff --git a/app/views/shared/issuable/_assignees.html.haml b/app/views/shared/issuable/_assignees.html.haml
index 36bbb1148d4..217af7c9fac 100644
--- a/app/views/shared/issuable/_assignees.html.haml
+++ b/app/views/shared/issuable/_assignees.html.haml
@@ -1,9 +1,8 @@
 - max_render = 3
 - max = [max_render, issue.assignees.length].min
 
-- issue.assignees.each_with_index do |assignee, index|
-  - if index < max
-    = link_to_member(@project, assignee, name: false, title: "Assigned to :name")
+- issue.assignees.take(max).each do |assignee|
+  = link_to_member(@project, assignee, name: false, title: "Assigned to :name")
 
 - if issue.assignees.length > max_render
   - counter = issue.assignees.length - max_render
diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml
index c36a45098a8..e9ce7b7ce9c 100644
--- a/app/views/shared/issuable/_sidebar_assignees.html.haml
+++ b/app/views/shared/issuable/_sidebar_assignees.html.haml
@@ -1,4 +1,4 @@
-- if issuable.instance_of?(Issue)
+- if issuable.is_a?(Issue)
   #js-vue-sidebar-assignees{ data: { field: "#{issuable.to_ability_name}[assignee_ids]" } }
 - else
   .sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body" }, title: (issuable.assignee.name if issuable.assignee) }
@@ -33,17 +33,17 @@
 
   - options = { toggle_class: 'js-user-search js-author-search', title: 'Assign to', filter: true, dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author', placeholder: 'Search users', data: { first_user: (current_user.username if current_user), current_user: true, project_id: (@project.id if @project), author_id: issuable.author_id, field_name: "#{issuable.to_ability_name}[assignee_ids][]", issue_update: issuable_json_path(issuable), ability_name: issuable.to_ability_name, null_user: true } }
 
-  - if issuable.instance_of?(Issue)
-    - if issuable.assignees.length == 0
+  - title = 'Select assignee'
+
+  - if issuable.is_a?(Issue)
+    - unless issuable.assignees.any?
       = hidden_field_tag "#{issuable.to_ability_name}[assignee_ids][]", 0, id: nil
-    - title = 'Select assignee'
     - options[:toggle_class] += ' js-multiselect js-save-user-data'
-    - options[:data][:field_name] = "#{issuable.to_ability_name}[assignee_ids][]"
-    - options[:data][:multi_select] = true
-    - options[:data]['dropdown-title'] = title
-    - options[:data]['dropdown-header'] = 'Assignee'
-    - options[:data]['max-select'] = 1
-  - else
-    - title = 'Select assignee'
+    - data = { field_name: "#{issuable.to_ability_name}[assignee_ids][]" }
+    - data[:multi_select] = true
+    - data['dropdown-title'] = title
+    - data['dropdown-header'] = 'Assignee'
+    - data['max-select'] = 1
+    - options[:data].merge!(data)
 
   = dropdown_tag(title, options: options)
diff --git a/app/views/shared/issuable/form/_issue_assignee.html.haml b/app/views/shared/issuable/form/_issue_assignee.html.haml
index c33474ac3b4..66091d95a91 100644
--- a/app/views/shared/issuable/form/_issue_assignee.html.haml
+++ b/app/views/shared/issuable/form/_issue_assignee.html.haml
@@ -1,8 +1,9 @@
 - issue = issuable
+- assignees = issue.assignees
 .block.assignee
   .sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body" }, title: (issuable.assignee_list) }
-    - if issue.assignees.any?
-      - issue.assignees.each do |assignee|
+    - if assignees.any?
+      - assignees.each do |assignee|
         = link_to_member(@project, assignee, size: 24)
     - else
       = icon('user', 'aria-hidden': 'true')
@@ -12,8 +13,8 @@
     - if can_edit_issuable
       = link_to 'Edit', '#', class: 'edit-link pull-right'
   .value.hide-collapsed
-    - if issue.assignees.any?
-      - issue.assignees.each do |assignee|
+    - if assignees.any?
+      - assignees.each do |assignee|
         = link_to_member(@project, assignee, size: 32, extra_class: 'bold') do
           %span.username
             = assignee.to_reference
diff --git a/doc/api/issues.md b/doc/api/issues.md
index 75794cc8d04..3f949ca5667 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -100,7 +100,7 @@ Example response:
 ]
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## List group issues
 
@@ -192,7 +192,7 @@ Example response:
 ]
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## List project issues
 
@@ -284,7 +284,7 @@ Example response:
 ]
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Single issue
 
@@ -359,7 +359,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## New issue
 
@@ -375,7 +375,7 @@ POST /projects/:id/issues
 | `title`                                   | string  | yes      | The title of an issue |
 | `description`                             | string  | no       | The description of an issue  |
 | `confidential`                            | boolean | no       | Set an issue to be confidential. Default is `false`.  |
-| `assignee_ids`                            | Array[integer] | no       | The ID of a user to assign issue |
+| `assignee_ids`                            | Array[integer] | no       | The ID of the users to assign issue |
 | `milestone_id`                            | integer | no       | The ID of a milestone to assign issue  |
 | `labels`                                  | string  | no       | Comma-separated label names for an issue  |
 | `created_at`                              | string  | no       | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) |
@@ -421,7 +421,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Edit issue
 
@@ -439,7 +439,7 @@ PUT /projects/:id/issues/:issue_iid
 | `title`        | string  | no       | The title of an issue                                                                                      |
 | `description`  | string  | no       | The description of an issue                                                                                |
 | `confidential` | boolean | no       | Updates an issue to be confidential                                                                        |
-| `assignee_ids`  | Array[integer] | no       | The ID of a user to assign the issue to                                                                    |
+| `assignee_ids`  | Array[integer] | no       | The ID of the users to assign the issue to                                                                    |
 | `milestone_id` | integer | no       | The ID of a milestone to assign the issue to                                                               |
 | `labels`       | string  | no       | Comma-separated label names for an issue                                                                   |
 | `state_event`  | string  | no       | The state event of an issue. Set `close` to close the issue and `reopen` to reopen it                      |
@@ -484,7 +484,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Delete an issue
 
@@ -570,7 +570,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Subscribe to an issue
 
@@ -635,7 +635,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Unsubscribe from an issue
 
@@ -757,7 +757,7 @@ Example response:
 }
 ```
 
-**Note**: `assignee` column is deprecated, it shows the first assignee only.
+**Note**: `assignee` column is deprecated, now we show it as a single-sized array `assignees` to conform to the GitLab EE API.
 
 ## Set a time estimate for an issue
 
diff --git a/lib/api/helpers/common_helpers.rb b/lib/api/helpers/common_helpers.rb
index 6236fdd43ca..322624c6092 100644
--- a/lib/api/helpers/common_helpers.rb
+++ b/lib/api/helpers/common_helpers.rb
@@ -2,11 +2,11 @@ module API
   module Helpers
     module CommonHelpers
       def convert_parameters_from_legacy_format(params)
-        if params[:assignee_id].present?
-          params[:assignee_ids] = [params.delete(:assignee_id)]
+        params.tap do |params|
+          if params[:assignee_id].present?
+            params[:assignee_ids] = [params.delete(:assignee_id)]
+          end
         end
-
-        params
       end
     end
   end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index da2b56c040b..79cac721202 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -1124,7 +1124,7 @@ describe API::Issues do
     end
 
     context 'CE restrictions' do
-      it 'updates an issue with several assignee but only one has been applied' do
+      it 'updates an issue with several assignees but only one has been applied' do
         put api("/projects/#{project.id}/issues/#{issue.iid}", user),
           assignee_ids: [user2.id, guest.id]
 
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index 3ddd0badd39..6437d00e451 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -62,7 +62,7 @@ describe Issuable::BulkUpdateService, services: true do
         expect(result[:count]).to eq(1)
       end
 
-      it 'updates the assignee to the use ID passed' do
+      it 'updates the assignee to the user ID passed' do
         assignee = create(:user)
         project.team << [assignee, :developer]
 
@@ -100,7 +100,7 @@ describe Issuable::BulkUpdateService, services: true do
         expect(result[:count]).to eq(1)
       end
 
-      it 'updates the assignee to the use ID passed' do
+      it 'updates the assignee to the user ID passed' do
         assignee = create(:user)
         project.team << [assignee, :developer]
         expect { bulk_update(issue, assignee_ids: [assignee.id]) }
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 516566eddef..7a9cd7553b1 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -178,7 +178,7 @@ describe SystemNoteService, services: true do
     end
 
     it 'builds a correct phrase when assignee removed' do
-      expect(build_note([assignee1], [])).to eq 'removed all assignees'
+      expect(build_note([assignee1], [])).to eq 'removed assignee'
     end
 
     it 'builds a correct phrase when assignees changed' do