Make MergeRequest respond to assignee_ids & assignee_ids=

To make it simpler to assign users to an Issuable, make MergeRequest
support the attribute `assignee_ids`.

5 files changed, 47 insertions, 32 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0a476efdaa9..1bebd55a089 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -108,11 +108,7 @@ module Issuable
     end
 
     def has_multiple_assignees?
-      supports_multiple_assignees? && assignees.count > 1
-    end
-
-    def supports_multiple_assignees?
-      respond_to?(:assignee_ids)
+      assignees.count > 1
     end
   end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ea22ab53587..77da8413904 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -197,11 +197,19 @@ class MergeRequest < ActiveRecord::Base
     }
   end
 
-  # This method is needed for compatibility with issues to not mess view and other code
+  # These method are needed for compatibility with issues to not mess view and other code
   def assignees
     Array(assignee)
   end
 
+  def assignee_ids
+    Array(assignee_id)
+  end
+
+  def assignee_ids=(ids)
+    write_attribute(:assignee_id, ids.last)
+  end
+
   def assignee_or_author?(user)
     author_id == user.id || assignee_id == user.id
   end
diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb
index 4ceabaf021e..df13976fb3b 100644
--- a/app/services/quick_actions/interpret_service.rb
+++ b/app/services/quick_actions/interpret_service.rb
@@ -107,13 +107,12 @@ module QuickActions
     command :assign do |users|
       next if users.empty?
 
-      if issuable.allows_multiple_assignees?
-        @updates[:assignee_ids] = issuable.assignees.pluck(:id) + users.map(&:id)
-      elsif issuable.supports_multiple_assignees?
-        @updates[:assignee_ids] = [users.last.id]
-      else
-        @updates[:assignee_id] = users.last.id
-      end
+      @updates[:assignee_ids] =
+        if issuable.allows_multiple_assignees?
+          issuable.assignees.pluck(:id) + users.map(&:id)
+        else
+          [users.last.id]
+        end
     end
 
     desc do
@@ -138,16 +137,12 @@ module QuickActions
       # When multiple users are assigned, all will be unassigned if multiple assignees are no longer allowed
       users = extract_users(unassign_param) if issuable.allows_multiple_assignees?
 
-      if issuable.supports_multiple_assignees?
-        @updates[:assignee_ids] =
-          if users&.any?
-            issuable.assignees.pluck(:id) - users.map(&:id)
-          else
-            []
-          end
-      else
-        @updates[:assignee_id] = nil
-      end
+      @updates[:assignee_ids] =
+        if users&.any?
+          issuable.assignees.pluck(:id) - users.map(&:id)
+        else
+          []
+        end
     end
 
     desc do
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index a56bc524a98..945189b922b 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -105,6 +105,22 @@ describe MergeRequest, models: true do
     end
   end
 
+  describe '#assignee_ids' do
+    it 'returns an array of the assigned user id' do
+      subject.assignee_id = 123
+
+      expect(subject.assignee_ids).to eq([123])
+    end
+  end
+
+  describe '#assignee_ids=' do
+    it 'sets assignee_id to the last id in the array' do
+      subject.assignee_ids = [123, 456]
+
+      expect(subject.assignee_id).to eq(456)
+    end
+  end
+
   describe '#assignee_or_author?' do
     let(:user) { create(:user) }
 
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index b1997e64557..35373675894 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -359,7 +359,7 @@ describe QuickActions::InterpretService, services: true do
       let(:content) { "/assign @#{developer.username}" }
 
       context 'Issue' do
-        it 'fetches assignee and populates assignee_id if content contains /assign' do
+        it 'fetches assignee and populates assignee_ids if content contains /assign' do
           _, updates = service.execute(content, issue)
 
           expect(updates[:assignee_ids]).to match_array([developer.id])
@@ -367,10 +367,10 @@ describe QuickActions::InterpretService, services: true do
       end
 
       context 'Merge Request' do
-        it 'fetches assignee and populates assignee_id if content contains /assign' do
+        it 'fetches assignee and populates assignee_ids if content contains /assign' do
           _, updates = service.execute(content, merge_request)
 
-          expect(updates).to eq(assignee_id: developer.id)
+          expect(updates).to eq(assignee_ids: [developer.id])
         end
       end
     end
@@ -383,7 +383,7 @@ describe QuickActions::InterpretService, services: true do
       end
 
       context 'Issue' do
-        it 'fetches assignee and populates assignee_id if content contains /assign' do
+        it 'fetches assignee and populates assignee_ids if content contains /assign' do
           _, updates = service.execute(content, issue)
 
           expect(updates[:assignee_ids]).to match_array([developer.id])
@@ -391,10 +391,10 @@ describe QuickActions::InterpretService, services: true do
       end
 
       context 'Merge Request' do
-        it 'fetches assignee and populates assignee_id if content contains /assign' do
+        it 'fetches assignee and populates assignee_ids if content contains /assign' do
           _, updates = service.execute(content, merge_request)
 
-          expect(updates).to eq(assignee_id: developer.id)
+          expect(updates).to eq(assignee_ids: [developer.id])
         end
       end
     end
@@ -422,11 +422,11 @@ describe QuickActions::InterpretService, services: true do
       end
 
       context 'Merge Request' do
-        it 'populates assignee_id: nil if content contains /unassign' do
-          merge_request.update(assignee_id: developer.id)
+        it 'populates assignee_ids: [] if content contains /unassign' do
+          merge_request.update(assignee_ids: [developer.id])
           _, updates = service.execute(content, merge_request)
 
-          expect(updates).to eq(assignee_id: nil)
+          expect(updates).to eq(assignee_ids: [])
         end
       end
     end