Fix: '/unassign' command does not work as expected with multiple assignees[ci skip]

app/services/slash_commands/interpret_service.rb

@@ -99,13 +99,7 @@ module SlashCommands
       current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     parse_params do |assignee_param|
-      users = extract_references(assignee_param, :user)
-
-      if users.empty?
-        users = User.where(username: assignee_param.split(' ').map(&:strip))
-      end
-
-      users
+      extract_users(assignee_param)
     end
     command :assign do |users|
       next if users.empty?
@@ -117,18 +111,31 @@ module SlashCommands
       end
     end
 
-    desc 'Remove assignee'
+    desc 'Remove all or specific assignee(s)'
     explanation do
-      "Removes assignee #{issuable.assignees.first.to_reference}."
+      'Removes assignee(s)'
+    end
+    params do
+      if issuable.is_a?(Issue)
+        ['@user1 @user2']
+      else
+        []
+      end
     end
     condition do
       issuable.persisted? &&
         issuable.assignees.any? &&
         current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
-    command :unassign do
+    command :unassign do |unassign_param = nil|
+      users = extract_users(unassign_param)
+
       if issuable.is_a?(Issue)
+        if users.any?
+          @updates[:assignee_ids] = issuable.assignees.pluck(:id) - users.map(&:id)
+        else
           @updates[:assignee_ids] = []
+        end
       else
         @updates[:assignee_id] = nil
       end
@@ -487,6 +494,18 @@ module SlashCommands
       end
     end
 
+    def extract_users(params)
+      return [] if params.nil?
+
+      users = extract_references(params, :user)
+
+      if users.empty?
+        users = User.where(username: params.split(' ').map(&:strip))
+      end
+
+      users
+    end
+
     def find_labels(labels_param)
       extract_references(labels_param, :label) |
         LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute

doc/user/project/slash_commands.md

@@ -17,7 +17,7 @@ do.
 | `/merge`                   | Merge (when pipeline succeeds) |
 | `/title <New title>`       | Change title |
 | `/assign @username`        | Assign |
-| `/unassign`                | Remove assignee |
+| `/unassign @user1 @user2`  | Remove all the assignees or specified ones |
 | `/milestone %milestone`    | Set milestone |
 | `/remove_milestone`        | Remove milestone |
 | `/label ~foo ~"bar baz"`   | Add label(s) |

lib/gitlab/slash_commands/command_definition.rb

@@ -48,17 +48,23 @@ module Gitlab
       end
 
       def to_h(opts)
+        context = OpenStruct.new(opts)
+
         desc = description
         if desc.respond_to?(:call)
-          context = OpenStruct.new(opts)
           desc = context.instance_exec(&desc) rescue ''
         end
 
+        prms = params
+        if prms.respond_to?(:call)
+          prms = context.instance_exec(&prms) rescue ''
+        end
+
         {
           name: name,
           aliases: aliases,
           description: desc,
-          params: params
+          params: prms
         }
       end
 

lib/gitlab/slash_commands/dsl.rb

@@ -40,8 +40,8 @@ module Gitlab
         #   command :command_key do |arguments|
         #     # Awesome code block
         #   end
-        def params(*params)
-          @params = params
+        def params(*params, &block)
+          @params = block_given? ? block : params
         end
 
         # Allows to give an explanation of what the command will do when

spec/services/issuable/bulk_update_service_spec.rb

@@ -72,7 +72,7 @@ describe Issuable::BulkUpdateService, services: true do
     end
 
     context "when the new assignee ID is #{IssuableFinder::NONE}" do
-      it "unassigns the issues" do
+      it 'unassigns the issues' do
         expect { bulk_update(merge_request, assignee_id: IssuableFinder::NONE) }
           .to change { merge_request.reload.assignee }.to(nil)
       end

spec/services/notes/slash_commands_service_spec.rb

@@ -221,7 +221,7 @@ describe Notes::SlashCommandsService, services: true do
     end
   end
 
-  context 'CE restriction for issue assignees' do
+  context 'Issue assignees' do
     describe '/assign' do
       let(:project) { create(:empty_project) }
       let(:master) { create(:user).tap { |u| project.team << [u, :master] } }

spec/services/slash_commands/interpret_service_spec.rb

@@ -428,11 +428,30 @@ describe SlashCommands::InterpretService, services: true do
       let(:content) { '/unassign' }
 
       context 'Issue' do
-        it 'populates assignee_ids: [] if content contains /unassign' do
-          issue.update(assignee_ids: [developer.id])
-          _, updates = service.execute(content, issue)
+        it 'unassigns user if content contains /unassign @user' do
+          issue.update(assignee_ids: [developer.id, developer2.id])
+
+          _, updates = service.execute("/unassign @#{developer2.username}", issue)
+
+          expect(updates).to eq(assignee_ids: [developer.id])
+        end
+
+        it 'unassigns both users if content contains /unassign @user @user1' do
+          user = create(:user)
+
+          issue.update(assignee_ids: [developer.id, developer2.id, user.id])
+
+          _, updates = service.execute("/unassign @#{developer2.username} @#{developer.username}", issue)
+
+          expect(updates).to eq(assignee_ids: [user.id])
+        end
+
+        it 'unassigns all the users if content contains /unassign' do
+          issue.update(assignee_ids: [developer.id, developer2.id])
+
+          _, updates = service.execute('/unassign', issue)
 
-          expect(updates).to eq(assignee_ids: [])
+          expect(updates[:assignee_ids]).to be_empty
         end
       end
 
@@ -906,7 +925,7 @@ describe SlashCommands::InterpretService, services: true do
       it 'includes current assignee reference' do
         _, explanations = service.explain(content, issue)
 
-        expect(explanations).to eq(["Removes assignee @#{developer.username}."])
+        expect(explanations).to eq(['Removes assignee(s)'])
       end
     end