fix review changes

app/services/auto_assignment/assignment_service.rb

@@ -32,11 +32,11 @@ class AutoAssignment::AssignmentService
   def unassigned_conversations(limit)
     scope = inbox.conversations.unassigned.open
 
-    if assignment_config['conversation_priority'] == 'longest_waiting'
-      scope = scope.order(last_activity_at: :asc, created_at: :asc)
-    else
-      scope.order(created_at: :asc)
-    end
+    scope = if assignment_config['conversation_priority'] == 'longest_waiting'
+              scope.order(last_activity_at: :asc, created_at: :asc)
+            else
+              scope.order(created_at: :asc)
+            end
 
     scope.limit(limit)
   end

enterprise/app/services/enterprise/auto_assignment/assignment_service.rb

@@ -66,7 +66,12 @@ module Enterprise::AutoAssignment::AssignmentService
     scope = apply_exclusion_rules(scope)
 
     # Apply conversation priority from config
-    scope = apply_conversation_priority(scope)
+    scope = if assignment_config['conversation_priority'] == 'longest_waiting'
+              scope.order(last_activity_at: :asc, created_at: :asc)
+            else
+              scope.order(created_at: :asc)
+            end
+
     scope.limit(limit)
   end
 

spec/enterprise/auto_assignment/capacity_service_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 RSpec.describe Enterprise::AutoAssignment::CapacityService, type: :service do
   let(:account) { create(:account) }
-  let(:inbox) { create(:inbox, account: account) }
+  let(:inbox) { create(:inbox, account: account, enable_auto_assignment: true) }
 
   # Assignment policy with rate limiting
   let(:assignment_policy) do
@@ -87,9 +87,11 @@ RSpec.describe Enterprise::AutoAssignment::CapacityService, type: :service do
 
   describe 'assignment with capacity' do
     let(:service) { AutoAssignment::AssignmentService.new(inbox: inbox) }
-    let(:conversation) { create(:conversation, inbox: inbox, assignee: nil, status: :open) }
 
     it 'assigns to agents with available capacity' do
+      # Create conversation before assignment
+      conversation = create(:conversation, inbox: inbox, assignee: nil, status: :open)
+
       # Mock the selector to prefer agent_at_capacity (but should skip due to capacity)
       selector = instance_double(AutoAssignment::RoundRobinSelector)
       allow(AutoAssignment::RoundRobinSelector).to receive(:new).and_return(selector)
@@ -97,7 +99,8 @@ RSpec.describe Enterprise::AutoAssignment::CapacityService, type: :service do
         agents.map(&:user).find { |u| [agent_with_capacity, agent_without_capacity].include?(u) }
       end
 
-      expect(service.perform_for_conversation(conversation)).to be true
+      assigned_count = service.perform_bulk_assignment(limit: 1)
+      expect(assigned_count).to eq(1)
       expect(conversation.reload.assignee).to be_in([agent_with_capacity, agent_without_capacity])
       expect(conversation.reload.assignee).not_to eq(agent_at_capacity)
     end
@@ -108,7 +111,8 @@ RSpec.describe Enterprise::AutoAssignment::CapacityService, type: :service do
 
       # agent_without_capacity has no limit, so should still be available
       conversation2 = create(:conversation, inbox: inbox, assignee: nil, status: :open)
-      expect(service.perform_for_conversation(conversation2)).to be true
+      assigned_count = service.perform_bulk_assignment(limit: 1)
+      expect(assigned_count).to eq(1)
       expect(conversation2.reload.assignee).to eq(agent_without_capacity)
     end
   end

spec/services/auto_assignment/assignment_service_basic_spec.rb

@@ -26,19 +26,19 @@ RSpec.describe AutoAssignment::AssignmentService do
       it 'assigns an available agent to conversation' do
         conversation = create_test_conversation
 
-        result = service.perform_for_conversation(conversation)
+        assigned_count = service.perform_bulk_assignment(limit: 1)
 
-        expect(result).to be true
+        expect(assigned_count).to eq(1)
         expect(conversation.reload.assignee).to eq(agent)
       end
 
-      it 'returns false when no agents are online' do
+      it 'returns 0 when no agents are online' do
         allow(OnlineStatusTracker).to receive(:get_available_users).and_return({})
         conversation = create_test_conversation
 
-        result = service.perform_for_conversation(conversation)
+        assigned_count = service.perform_bulk_assignment(limit: 1)
 
-        expect(result).to be false
+        expect(assigned_count).to eq(0)
         expect(conversation.reload.assignee).to be_nil
       end
     end
@@ -49,9 +49,9 @@ RSpec.describe AutoAssignment::AssignmentService do
       it 'does not assign any agent' do
         conversation = create_test_conversation
 
-        result = service.perform_for_conversation(conversation)
+        assigned_count = service.perform_bulk_assignment(limit: 1)
 
-        expect(result).to be false
+        expect(assigned_count).to eq(0)
         expect(conversation.reload.assignee).to be_nil
       end
     end
@@ -63,18 +63,19 @@ RSpec.describe AutoAssignment::AssignmentService do
     it 'only assigns to open conversations' do
       resolved_conversation = create_test_conversation(status: 'resolved')
 
-      result = service.perform_for_conversation(resolved_conversation)
+      assigned_count = service.perform_bulk_assignment(limit: 1)
 
-      expect(result).to be false
+      expect(assigned_count).to eq(0)
+      expect(resolved_conversation.reload.assignee).to be_nil
     end
 
     it 'does not reassign already assigned conversations' do
       other_agent = create(:user, account: account, role: :agent)
       assigned_conversation = create_test_conversation(assignee: other_agent)
 
-      result = service.perform_for_conversation(assigned_conversation)
+      assigned_count = service.perform_bulk_assignment(limit: 1)
 
-      expect(result).to be false
+      expect(assigned_count).to eq(0)
       expect(assigned_conversation.reload.assignee).to eq(other_agent)
     end
   end

spec/services/auto_assignment/assignment_service_fair_distribution_spec.rb

@@ -52,10 +52,9 @@ RSpec.describe AutoAssignment::AssignmentService do
       # Each agent can handle 2 conversations
       conversations = Array.new(4) { create_test_conversation }
 
-      # First 4 conversations should be assigned (2 per agent)
-      conversations.each do |conv|
-        expect(service.perform_for_conversation(conv)).to be true
-      end
+      # Assign first 4 conversations (2 per agent)
+      assigned_count = service.perform_bulk_assignment(limit: 4)
+      expect(assigned_count).to eq(4)
 
       # Verify distribution
       assigned_to_agent1 = conversations.count { |c| c.reload.assignee == agent1 }
@@ -64,15 +63,16 @@ RSpec.describe AutoAssignment::AssignmentService do
       expect(assigned_to_agent1).to eq(2)
       expect(assigned_to_agent2).to eq(2)
 
-      # Fifth conversation should fail (both agents at limit)
+      # Fifth conversation should not be assigned (both agents at limit)
       fifth_conversation = create_test_conversation
-      expect(service.perform_for_conversation(fifth_conversation)).to be false
+      additional_assigned = service.perform_bulk_assignment(limit: 1)
+      expect(additional_assigned).to eq(0)
       expect(fifth_conversation.reload.assignee).to be_nil
     end
 
     it 'tracks assignments using individual Redis keys' do
       conversation = create_test_conversation
-      service.perform_for_conversation(conversation)
+      service.perform_bulk_assignment(limit: 1)
 
       # Check that assignment key exists
       pattern = "assignment:#{inbox.id}:agent:#{conversation.reload.assignee.id}:*"
@@ -81,13 +81,18 @@ RSpec.describe AutoAssignment::AssignmentService do
     end
 
     it 'allows new assignments after window expires' do
-      # Assign 2 conversations to agent1
-      2.times do
-        conversation = create_test_conversation
-        allow(service).to receive(:round_robin_selector).and_return(
-          instance_double(AutoAssignment::RoundRobinSelector, select_agent: agent1)
-        )
-        service.perform_for_conversation(conversation)
+      # Create 2 conversations and force assignment to agent1
+      conversations = Array.new(2) { create_test_conversation }
+      allow(service).to receive(:round_robin_selector).and_return(
+        instance_double(AutoAssignment::RoundRobinSelector, select_agent: agent1)
+      )
+
+      # Assign both to agent1
+      assigned_count = service.perform_bulk_assignment(limit: 2)
+      expect(assigned_count).to eq(2)
+
+      conversations.each do |c|
+        expect(c.reload.assignee).to eq(agent1)
       end
 
       # Agent1 is now at limit
@@ -105,7 +110,9 @@ RSpec.describe AutoAssignment::AssignmentService do
       allow(service).to receive(:round_robin_selector).and_return(
         instance_double(AutoAssignment::RoundRobinSelector, select_agent: agent1)
       )
-      expect(service.perform_for_conversation(new_conversation)).to be true
+      assigned_count = service.perform_bulk_assignment(limit: 1)
+      expect(assigned_count).to eq(1)
+      expect(new_conversation.reload.assignee).to eq(agent1)
     end
   end
 
@@ -116,9 +123,12 @@ RSpec.describe AutoAssignment::AssignmentService do
 
     it 'assigns without limits' do
       # Create more conversations than would be allowed with limits
-      5.times do
-        conversation = create_test_conversation
-        expect(service.perform_for_conversation(conversation)).to be true
+      conversations = Array.new(5) { create_test_conversation }
+
+      assigned_count = service.perform_bulk_assignment(limit: 5)
+      expect(assigned_count).to eq(5)
+
+      conversations.each do |conversation|
         expect(conversation.reload.assignee).not_to be_nil
       end
     end

spec/services/auto_assignment/assignment_service_spec.rb

@@ -19,21 +19,23 @@ RSpec.describe AutoAssignment::AssignmentService do
     conversation
   end
 
-  describe '#perform_for_conversation' do
-    let(:conversation) { create_test_conversation }
-
+  describe 'assignment behavior' do
     before do
       allow(OnlineStatusTracker).to receive(:get_available_users).and_return({ agent.id.to_s => 'online' })
     end
 
     context 'when auto assignment is enabled' do
       it 'assigns conversation to available agent' do
-        expect(service.perform_for_conversation(conversation)).to be true
+        conversation = create_test_conversation
+
+        assigned_count = service.perform_bulk_assignment(limit: 1)
+
+        expect(assigned_count).to eq(1)
         expect(conversation.reload.assignee).to eq(agent)
       end
 
       it 'dispatches assignee changed event' do
-        # The conversation update triggers its own event through callbacks
+        conversation = create_test_conversation
         allow(Rails.configuration.dispatcher).to receive(:dispatch).and_call_original
 
         expect(Rails.configuration.dispatcher).to receive(:dispatch).with(
@@ -42,28 +44,38 @@ RSpec.describe AutoAssignment::AssignmentService do
           hash_including(conversation: conversation, user: agent)
         )
 
-        service.perform_for_conversation(conversation)
+        service.perform_bulk_assignment(limit: 1)
       end
 
-      it 'returns false when no agents available' do
+      it 'returns 0 when no agents available' do
+        create_test_conversation
         allow(OnlineStatusTracker).to receive(:get_available_users).and_return({})
-        expect(service.perform_for_conversation(conversation)).to be false
+
+        assigned_count = service.perform_bulk_assignment(limit: 1)
+
+        expect(assigned_count).to eq(0)
       end
     end
 
     context 'when conversation already assigned' do
-      let(:conversation) { create_test_conversation(assignee: agent) }
-
       it 'does not reassign' do
-        expect(service.perform_for_conversation(conversation)).to be false
+        conversation = create_test_conversation(assignee: agent)
+
+        assigned_count = service.perform_bulk_assignment(limit: 1)
+
+        expect(assigned_count).to eq(0)
+        expect(conversation.reload.assignee).to eq(agent)
       end
     end
 
     context 'when conversation is not open' do
-      let(:conversation) { create_test_conversation(status: 'resolved') }
-
       it 'does not assign' do
-        expect(service.perform_for_conversation(conversation)).to be false
+        conversation = create_test_conversation(status: 'resolved')
+
+        assigned_count = service.perform_bulk_assignment(limit: 1)
+
+        expect(assigned_count).to eq(0)
+        expect(conversation.reload.assignee).to be_nil
       end
     end
   end
@@ -124,7 +136,9 @@ RSpec.describe AutoAssignment::AssignmentService do
 
       conversation = create_test_conversation
 
-      expect(service.perform_for_conversation(conversation)).to be true
+      assigned_count = service.perform_bulk_assignment(limit: 1)
+
+      expect(assigned_count).to eq(1)
       expect(conversation.reload.assignee).to eq(agent2)
     end
 
@@ -136,7 +150,7 @@ RSpec.describe AutoAssignment::AssignmentService do
 
       expect(rate_limiter).to receive(:track_assignment).with(conversation)
 
-      service.perform_for_conversation(conversation)
+      service.perform_bulk_assignment(limit: 1)
     end
   end