chore: Use Round Robin service for team assignment (#4237)

Co-authored-by: Pranav Raj S <pranav@chatwoot.com>

.circleci/config.yml

@@ -110,7 +110,7 @@ jobs:
       - run:
           name: Run backend tests
           command: |
-            bundle exec rspec $(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings) --profile=10
+            bundle exec rspec $(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings) --profile=10 --format documentation
             ~/tmp/cc-test-reporter format-coverage -t simplecov -o ~/tmp/codeclimate.backend.json coverage/backend/.resultset.json
       - persist_to_workspace:
           root: ~/tmp

app/models/concerns/assignment_handler.rb

@@ -12,18 +12,19 @@ module AssignmentHandler
   def ensure_assignee_is_from_team
     return unless team_id_changed?
 
-    ensure_current_assignee_team
-    self.assignee_id ||= find_team_assignee_id_for_inbox if team&.allow_auto_assign.present?
+    validate_current_assignee_team
+    self.assignee ||= find_assignee_from_team
   end
 
-  def ensure_current_assignee_team
+  def validate_current_assignee_team
     self.assignee_id = nil if team&.members&.exclude?(assignee)
   end
 
-  def find_team_assignee_id_for_inbox
-    members = inbox.members.ids & team.members.ids
-    # TODO: User round robin to determine the next agent instead of using sample
-    members.sample
+  def find_assignee_from_team
+    return if team&.allow_auto_assign.blank?
+
+    team_members = inbox.members.ids & team.members.ids
+    ::RoundRobin::AssignmentService.new(conversation: self, allowed_member_ids: team_members).find_assignee
   end
 
   def notify_assignment_change

app/services/round_robin/assignment_service.rb

@@ -1,9 +1,13 @@
 class RoundRobin::AssignmentService
-  pattr_initialize [:conversation]
+  pattr_initialize [:conversation, { allowed_member_ids: [] }]
+
+  def find_assignee
+    round_robin_manage_service.available_agent(priority_list: online_agents)
+  end
 
   def perform
     # online agents will get priority
-    new_assignee = round_robin_manage_service.available_agent(priority_list: online_agents)
+    new_assignee = find_assignee
     conversation.update(assignee: new_assignee) if new_assignee
   end
 
@@ -15,7 +19,7 @@ class RoundRobin::AssignmentService
   end
 
   def round_robin_manage_service
-    @round_robin_manage_service ||= RoundRobin::ManageService.new(inbox: conversation.inbox)
+    @round_robin_manage_service ||= RoundRobin::ManageService.new(inbox: conversation.inbox, allowed_member_ids: allowed_member_ids)
   end
 
   def round_robin_key

app/services/round_robin/manage_service.rb

@@ -1,5 +1,7 @@
+# If allowed_member_ids are supplied round robin service will only fetch a member from member id
+# This is used in case of team assignment
 class RoundRobin::ManageService
-  pattr_initialize [:inbox!]
+  pattr_initialize [:inbox!, { allowed_member_ids: [] }]
 
   # called on inbox delete
   def clear_queue
@@ -18,9 +20,9 @@ class RoundRobin::ManageService
 
   def available_agent(priority_list: [])
     reset_queue unless validate_queue?
-    user_id = get_agent_via_priority_list(priority_list)
+    user_id = get_member_via_priority_list(priority_list)
     # incase priority list was empty or inbox members weren't present
-    user_id ||= ::Redis::Alfred.rpoplpush(round_robin_key, round_robin_key)
+    user_id ||= fetch_user_id
     inbox.inbox_members.find_by(user_id: user_id)&.user if user_id.present?
   end
 
@@ -31,17 +33,36 @@ class RoundRobin::ManageService
 
   private
 
-  def get_agent_via_priority_list(priority_list)
+  def fetch_user_id
+    if allowed_member_ids_in_str.present?
+      user_id = queue.intersection(allowed_member_ids_in_str).pop
+      pop_push_to_queue(user_id)
+      user_id
+    else
+      ::Redis::Alfred.rpoplpush(round_robin_key, round_robin_key)
+    end
+  end
+
+  # priority list is usually the members who are online passed from assignmebt service
+  def get_member_via_priority_list(priority_list)
+    return if priority_list.blank?
+
+    # when allowed member ids is passed we will be looking to get members from that list alone
+    priority_list = priority_list.intersection(allowed_member_ids_in_str) if allowed_member_ids_in_str.present?
     return if priority_list.blank?
 
     user_id = queue.intersection(priority_list.map(&:to_s)).pop
-    if user_id.present?
-      remove_agent_from_queue(user_id)
-      add_agent_to_queue(user_id)
-    end
+    pop_push_to_queue(user_id)
     user_id
   end
 
+  def pop_push_to_queue(user_id)
+    return if user_id.blank?
+
+    remove_agent_from_queue(user_id)
+    add_agent_to_queue(user_id)
+  end
+
   def validate_queue?
     return true if inbox.inbox_members.map(&:user_id).sort == queue.map(&:to_i).sort
   end
@@ -53,4 +74,9 @@ class RoundRobin::ManageService
   def round_robin_key
     format(::Redis::Alfred::ROUND_ROBIN_AGENTS, inbox_id: inbox.id)
   end
+
+  def allowed_member_ids_in_str
+    # NOTE: the values which are returned from redis for priority list are string
+    @allowed_member_ids_in_str ||= allowed_member_ids.map(&:to_s)
+  end
 end

spec/controllers/api/v1/accounts/conversations/assignments_controller_spec.rb

@@ -35,6 +35,9 @@ RSpec.describe 'Conversation Assignment API', type: :request do
       end
 
       it 'assigns a team to the conversation' do
+        team_member = create(:user, account: account, role: :agent)
+        create(:inbox_member, inbox: conversation.inbox, user: team_member)
+        create(:team_member, team: team, user: team_member)
         params = { team_id: team.id }
 
         post api_v1_account_conversation_assignments_url(account_id: account.id, conversation_id: conversation.display_id),
@@ -44,6 +47,8 @@ RSpec.describe 'Conversation Assignment API', type: :request do
 
         expect(response).to have_http_status(:success)
         expect(conversation.reload.team).to eq(team)
+        # assignee will be from team
+        expect(conversation.reload.assignee).to eq(team_member)
       end
     end
 

spec/services/round_robin/manage_service_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 describe RoundRobin::ManageService do
-  subject(:round_robin_manage_service) { ::RoundRobin::ManageService.new(inbox: inbox) }
+  subject(:round_robin_manage_service) { described_class.new(inbox: inbox) }
 
   let!(:account) { create(:account) }
   let!(:inbox) { create(:inbox, account: account) }
@@ -18,8 +18,9 @@ describe RoundRobin::ManageService do
     it 'gets intersection of priority list and agent queue. get and move agent to the end of the list' do
       expected_queue = [inbox_members[2].user_id, inbox_members[4].user_id, inbox_members[3].user_id, inbox_members[1].user_id,
                         inbox_members[0].user_id].map(&:to_s)
-      expect(round_robin_manage_service.available_agent(priority_list: [inbox_members[3].user_id,
-                                                                        inbox_members[2].user_id])).to eq inbox_members[2].user
+      # prority list will be ids in string, since thats what redis supplies to us
+      expect(round_robin_manage_service.available_agent(priority_list: [inbox_members[3].user_id.to_s,
+                                                                        inbox_members[2].user_id.to_s])).to eq inbox_members[2].user
       expect(round_robin_manage_service.send(:queue)).to eq(expected_queue)
     end
 
@@ -39,5 +40,32 @@ describe RoundRobin::ManageService do
       # the service have refreshed the redis queue before performing
       expect(round_robin_manage_service.send(:queue).sort.map(&:to_i)).to eq(inbox_members.map(&:user_id).sort)
     end
+
+    context 'when allowed_member_ids is passed' do
+      it 'will get the first allowed member and move it to the end of the queue' do
+        expected_queue = [inbox_members[3].user_id, inbox_members[2].user_id, inbox_members[4].user_id, inbox_members[1].user_id,
+                          inbox_members[0].user_id].map(&:to_s)
+        expect(described_class.new(inbox: inbox,
+                                   allowed_member_ids: [inbox_members[3].user_id,
+                                                        inbox_members[2].user_id]).available_agent).to eq inbox_members[2].user
+        expect(described_class.new(inbox: inbox,
+                                   allowed_member_ids: [inbox_members[3].user_id,
+                                                        inbox_members[2].user_id]).available_agent).to eq inbox_members[3].user
+        expect(round_robin_manage_service.send(:queue)).to eq(expected_queue)
+      end
+
+      it 'will get union of priority list and allowed_member_ids and move it to the end of the queue' do
+        expected_queue = [inbox_members[3].user_id, inbox_members[4].user_id, inbox_members[2].user_id, inbox_members[1].user_id,
+                          inbox_members[0].user_id].map(&:to_s)
+        # prority list will be ids in string, since thats what redis supplies to us
+        expect(described_class.new(inbox: inbox,
+                                   allowed_member_ids: [inbox_members[3].user_id,
+                                                        inbox_members[2].user_id])
+                                    .available_agent(
+                                      priority_list: [inbox_members[3].user_id.to_s]
+                                    )).to eq inbox_members[3].user
+        expect(round_robin_manage_service.send(:queue)).to eq(expected_queue)
+      end
+    end
   end
 end