fix: assign outbound voice call conversation to the calling agent (#14906)
Outbound voice calls were being auto-assigned to the wrong agent. When an agent placed an outbound call, the conversation was created without an assignee, so inboxes with auto-assignment enabled would round-robin it to a different agent instead of keeping it with the person who actually made the call. This made it hard to tell which agent was on an active call. ## What changed - Set the calling agent as the conversation's assignee when creating an outbound voice call conversation. - This prevents the generic auto-assignment handler from treating the conversation as unassigned and reassigning it. - Added specs covering the assignment, including a regression case with inbox auto-assignment enabled. **Note:** this applies to newly placed calls; it does not retroactively fix conversations that were already mis-assigned. --------- Co-authored-by: Tanmay Deep Sharma <tanmaydeepsharma21@gmail.com> Co-authored-by: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com>
This commit is contained in:
co-authored by
Tanmay Deep Sharma
Tanmay Deep Sharma
parent
34d8741b9b
commit
6c9efc4e92
@@ -106,9 +106,14 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro
|
||||
|
||||
def create_outbound_call
|
||||
contact_phone = @conversation.contact.phone_number.delete('+')
|
||||
# Claim for the caller only if unassigned at trigger time (before the round-trip); wins over auto-assignment.
|
||||
claim_for_caller = @conversation.assignee_id.nil?
|
||||
|
||||
result = provider_service.initiate_call(contact_phone, params[:sdp_offer])
|
||||
provider_call_id = result.dig('calls', 0, 'id') || result['call_id']
|
||||
|
||||
@conversation.with_lock { @conversation.update!(assignee: Current.user) } if claim_for_caller
|
||||
|
||||
Current.account.calls.create!(
|
||||
provider: :whatsapp, inbox: @conversation.inbox, conversation: @conversation, contact: @conversation.contact,
|
||||
provider_call_id: provider_call_id, direction: :outgoing, status: 'ringing',
|
||||
|
||||
@@ -17,10 +17,19 @@ class Voice::OutboundCallBuilder
|
||||
raise ArgumentError, 'Contact phone number required' if contact.phone_number.blank?
|
||||
raise ArgumentError, 'Agent required' if user.blank?
|
||||
|
||||
# Claim for the caller if a reused conversation is unassigned at trigger time; wins over auto-assignment.
|
||||
# New conversations set the assignee at creation instead (see create_conversation!).
|
||||
claim_for_caller = @existing_conversation && @existing_conversation.assignee_id.nil?
|
||||
|
||||
ActiveRecord::Base.transaction do
|
||||
contact_inbox = ensure_contact_inbox!
|
||||
conversation = @existing_conversation || create_conversation!(contact_inbox)
|
||||
# Dial before locking so the Twilio round-trip doesn't hold the conversation row lock.
|
||||
call_sid = initiate_call!
|
||||
if claim_for_caller
|
||||
@existing_conversation.lock!
|
||||
@existing_conversation.update!(assignee: user)
|
||||
end
|
||||
call = create_call!(conversation, call_sid)
|
||||
message = Voice::CallMessageBuilder.new(call).perform!
|
||||
call.update!(message_id: message.id)
|
||||
@@ -44,6 +53,7 @@ class Voice::OutboundCallBuilder
|
||||
contact_inbox_id: contact_inbox.id,
|
||||
inbox_id: inbox.id,
|
||||
contact_id: contact.id,
|
||||
assignee_id: user.id,
|
||||
status: :open
|
||||
)
|
||||
end
|
||||
|
||||
@@ -113,6 +113,31 @@ RSpec.describe 'WhatsApp Calls API', type: :request do
|
||||
expect(Call.find_by(provider_call_id: 'wacid_outbound')).to have_attributes(direction: 'outgoing', status: 'ringing')
|
||||
end
|
||||
|
||||
it 'assigns the conversation to the agent placing the call when it is unassigned' do
|
||||
allow(provider_service).to receive(:initiate_call).and_return({ 'calls' => [{ 'id' => 'wacid_outbound' }] })
|
||||
|
||||
post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate",
|
||||
params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' },
|
||||
headers: agent.create_new_auth_token
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(initiate_conversation.reload.assignee_id).to eq(agent.id)
|
||||
end
|
||||
|
||||
it 'keeps the existing assignee when the conversation is already assigned' do
|
||||
other_agent = create(:user, account: account, role: :agent)
|
||||
create(:inbox_member, user: other_agent, inbox: inbox)
|
||||
initiate_conversation.update!(assignee: other_agent)
|
||||
allow(provider_service).to receive(:initiate_call).and_return({ 'calls' => [{ 'id' => 'wacid_outbound' }] })
|
||||
|
||||
post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate",
|
||||
params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' },
|
||||
headers: agent.create_new_auth_token
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(initiate_conversation.reload.assignee_id).to eq(other_agent.id)
|
||||
end
|
||||
|
||||
it 'sends a permission request and records the wamid when Meta returns NoCallPermission' do
|
||||
allow(provider_service).to receive(:initiate_call).and_raise(Voice::CallErrors::NoCallPermission)
|
||||
allow(provider_service).to receive(:send_call_permission_request).and_return({ 'messages' => [{ 'id' => 'wamid.req_xyz' }] })
|
||||
|
||||
@@ -44,6 +44,54 @@ RSpec.describe Voice::OutboundCallBuilder do
|
||||
end
|
||||
end
|
||||
|
||||
it 'assigns the conversation to the agent placing the call' do
|
||||
call = described_class.perform!(
|
||||
account: account,
|
||||
inbox: inbox,
|
||||
user: user,
|
||||
contact: contact
|
||||
)
|
||||
|
||||
expect(call.conversation.assignee_id).to eq(user.id)
|
||||
end
|
||||
|
||||
it 'keeps the calling agent assigned even when auto-assignment would pick an online agent' do
|
||||
other_agent = create(:user, account: account)
|
||||
create(:inbox_member, inbox: inbox, user: other_agent)
|
||||
create(:inbox_member, inbox: inbox, user: user)
|
||||
inbox.update!(enable_auto_assignment: true)
|
||||
# Only other_agent is online, so round-robin would claim the conversation unless the caller wins at creation.
|
||||
OnlineStatusTracker.update_presence(account.id, 'User', other_agent.id)
|
||||
OnlineStatusTracker.set_status(account.id, other_agent.id, 'online')
|
||||
|
||||
call = described_class.perform!(
|
||||
account: account,
|
||||
inbox: inbox,
|
||||
user: user,
|
||||
contact: contact
|
||||
)
|
||||
|
||||
expect(call.conversation.assignee_id).to eq(user.id)
|
||||
end
|
||||
|
||||
it 'claims a reused conversation for the caller when it is unassigned' do
|
||||
# Reload so the builder gets a DB-fresh record, mirroring the controller's find_by load.
|
||||
conversation = create(:conversation, account: account, inbox: inbox, contact: contact).reload
|
||||
|
||||
described_class.perform!(account: account, inbox: inbox, user: user, contact: contact, conversation: conversation)
|
||||
|
||||
expect(conversation.reload.assignee_id).to eq(user.id)
|
||||
end
|
||||
|
||||
it 'keeps the existing assignee when a reused conversation is already assigned' do
|
||||
other_agent = create(:user, account: account)
|
||||
conversation = create(:conversation, account: account, inbox: inbox, contact: contact, assignee: other_agent).reload
|
||||
|
||||
described_class.perform!(account: account, inbox: inbox, user: user, contact: contact, conversation: conversation)
|
||||
|
||||
expect(conversation.reload.assignee_id).to eq(other_agent.id)
|
||||
end
|
||||
|
||||
it 'does not set conversation.identifier or write call state to additional_attributes' do
|
||||
call = described_class.perform!(
|
||||
account: account,
|
||||
|
||||
Reference in New Issue
Block a user