fix: Enforce team boundaries to prevent cross-team assignments (#13353)
## Description Fixes a critical bug where conversations assigned to a team could be auto-assigned to agents outside that team when all team members were at capacity. ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core assignment selection for both legacy and v2 flows; misconfiguration of `allow_auto_assign` or team membership could cause conversations to remain unassigned. > > **Overview** > Prevents auto-assignment from crossing team boundaries by filtering eligible agents to the conversation’s `team` members (and requiring `team.allow_auto_assign`) in both the legacy `AutoAssignmentHandler` path and the v2 `AutoAssignment::AssignmentService` (including the Enterprise override). > > Adds test coverage to ensure team-scoped conversations only assign to team members, and are skipped when team auto-assign is disabled or no team members are available; also updates the conversations controller spec setup to include team membership. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 67ed2bda0cd8ffd56c7e0253b86369dead2e6155. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This commit is contained in:
committed by
GitHub
parent
fd5ac2a8a3
commit
f4538ae2c5
@@ -19,10 +19,18 @@ module AutoAssignmentHandler
|
||||
AutoAssignment::AssignmentJob.perform_later(inbox_id: inbox.id)
|
||||
else
|
||||
# Use legacy assignment system
|
||||
AutoAssignment::AgentAssignmentService.new(conversation: self, allowed_agent_ids: inbox.member_ids_with_assignment_capacity).perform
|
||||
# If conversation has a team, only consider team members for assignment
|
||||
allowed_agent_ids = team_id.present? ? team_member_ids_with_capacity : inbox.member_ids_with_assignment_capacity
|
||||
AutoAssignment::AgentAssignmentService.new(conversation: self, allowed_agent_ids: allowed_agent_ids).perform
|
||||
end
|
||||
end
|
||||
|
||||
def team_member_ids_with_capacity
|
||||
return [] if team.blank? || team.allow_auto_assign.blank?
|
||||
|
||||
inbox.member_ids_with_assignment_capacity & team.members.ids
|
||||
end
|
||||
|
||||
def should_run_auto_assignment?
|
||||
return false unless inbox.enable_auto_assignment?
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ class AutoAssignment::AssignmentService
|
||||
def perform_for_conversation(conversation)
|
||||
return false unless assignable?(conversation)
|
||||
|
||||
agent = find_available_agent
|
||||
agent = find_available_agent(conversation)
|
||||
return false unless agent
|
||||
|
||||
assign_conversation(conversation, agent)
|
||||
@@ -44,13 +44,26 @@ class AutoAssignment::AssignmentService
|
||||
scope.limit(limit)
|
||||
end
|
||||
|
||||
def find_available_agent
|
||||
agents = filter_agents_by_rate_limit(inbox.available_agents)
|
||||
def find_available_agent(conversation = nil)
|
||||
agents = filter_agents_by_team(inbox.available_agents, conversation)
|
||||
return nil if agents.nil?
|
||||
|
||||
agents = filter_agents_by_rate_limit(agents)
|
||||
return nil if agents.empty?
|
||||
|
||||
round_robin_selector.select_agent(agents)
|
||||
end
|
||||
|
||||
def filter_agents_by_team(agents, conversation)
|
||||
return agents if conversation&.team_id.blank?
|
||||
|
||||
team = conversation.team
|
||||
return nil if team.blank? || team.allow_auto_assign.blank?
|
||||
|
||||
team_member_ids = team.members.ids
|
||||
agents.where(user_id: team_member_ids)
|
||||
end
|
||||
|
||||
def filter_agents_by_rate_limit(agents)
|
||||
agents.select do |agent_member|
|
||||
rate_limiter = build_rate_limiter(agent_member.user)
|
||||
|
||||
@@ -14,8 +14,11 @@ module Enterprise::AutoAssignment::AssignmentService
|
||||
end
|
||||
|
||||
# Extend agent finding to add capacity checks
|
||||
def find_available_agent
|
||||
agents = filter_agents_by_rate_limit(inbox.available_agents)
|
||||
def find_available_agent(conversation = nil)
|
||||
agents = filter_agents_by_team(inbox.available_agents, conversation)
|
||||
return nil if agents.nil?
|
||||
|
||||
agents = filter_agents_by_rate_limit(agents)
|
||||
agents = filter_agents_by_capacity(agents) if capacity_filtering_enabled?
|
||||
return nil if agents.empty?
|
||||
|
||||
|
||||
@@ -330,6 +330,7 @@ RSpec.describe 'Conversations API', type: :request do
|
||||
context 'when it is an authenticated user who has access to the inbox' do
|
||||
before do
|
||||
create(:inbox_member, user: agent, inbox: inbox)
|
||||
create(:team_member, user: agent, team: team)
|
||||
end
|
||||
|
||||
it 'creates a new conversation' do
|
||||
|
||||
@@ -307,5 +307,52 @@ RSpec.describe AutoAssignment::AssignmentService do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with team assignments' do
|
||||
let(:team) { create(:team, account: account, allow_auto_assign: true) }
|
||||
let(:team_member) { create(:user, account: account, role: :agent, availability: :online) }
|
||||
let(:rate_limiter) { instance_double(AutoAssignment::RateLimiter) }
|
||||
|
||||
before do
|
||||
create(:team_member, team: team, user: team_member)
|
||||
create(:inbox_member, inbox: inbox, user: team_member)
|
||||
|
||||
allow(OnlineStatusTracker).to receive(:get_available_users).and_return({ team_member.id.to_s => 'online' })
|
||||
|
||||
allow(AutoAssignment::RateLimiter).to receive(:new).and_return(rate_limiter)
|
||||
allow(rate_limiter).to receive(:within_limit?).and_return(true)
|
||||
allow(rate_limiter).to receive(:track_assignment)
|
||||
|
||||
round_robin_selector = instance_double(AutoAssignment::RoundRobinSelector)
|
||||
allow(AutoAssignment::RoundRobinSelector).to receive(:new).and_return(round_robin_selector)
|
||||
allow(round_robin_selector).to receive(:select_agent).and_return(team_member)
|
||||
end
|
||||
|
||||
it 'assigns conversation with team to team member' do
|
||||
conversation_with_team = create(:conversation, inbox: inbox, team: team, assignee: nil)
|
||||
|
||||
service.perform_bulk_assignment(limit: 1)
|
||||
|
||||
expect(conversation_with_team.reload.assignee).to eq(team_member)
|
||||
end
|
||||
|
||||
it 'skips assignment when team has allow_auto_assign false' do
|
||||
team.update!(allow_auto_assign: false)
|
||||
conversation_with_team = create(:conversation, inbox: inbox, team: team, assignee: nil)
|
||||
|
||||
service.perform_bulk_assignment(limit: 1)
|
||||
|
||||
expect(conversation_with_team.reload.assignee).to be_nil
|
||||
end
|
||||
|
||||
it 'skips assignment when no team members are available' do
|
||||
allow(OnlineStatusTracker).to receive(:get_available_users).and_return({})
|
||||
conversation_with_team = create(:conversation, inbox: inbox, team: team, assignee: nil)
|
||||
|
||||
service.perform_bulk_assignment(limit: 1)
|
||||
|
||||
expect(conversation_with_team.reload.assignee).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user