diff --git a/app/javascript/dashboard/helper/voice.js b/app/javascript/dashboard/helper/voice.js index 0093f810c..16655b0cb 100644 --- a/app/javascript/dashboard/helper/voice.js +++ b/app/javascript/dashboard/helper/voice.js @@ -63,6 +63,7 @@ export function handleVoiceCallUpdated(commit, message, currentUserId) { commit(types.UPDATE_MESSAGE_CALL_STATUS, { conversationId, callStatus: status, + callSid, }); const isNewCall = diff --git a/app/javascript/dashboard/store/modules/conversations/index.js b/app/javascript/dashboard/store/modules/conversations/index.js index 509629001..9afa34ee5 100644 --- a/app/javascript/dashboard/store/modules/conversations/index.js +++ b/app/javascript/dashboard/store/modules/conversations/index.js @@ -307,19 +307,23 @@ export const mutations = { } }, - [types.UPDATE_MESSAGE_CALL_STATUS](_state, { conversationId, callStatus }) { + [types.UPDATE_MESSAGE_CALL_STATUS]( + _state, + { conversationId, callStatus, callSid } + ) { const chat = getConversationById(_state)(conversationId); if (!chat) return; - const lastCall = (chat.messages || []).findLast( - m => m.content_type === CONTENT_TYPES.VOICE_CALL + const message = (chat.messages || []).find( + m => + m.content_type === CONTENT_TYPES.VOICE_CALL && + m.content_attributes?.data?.call_sid === callSid ); + if (!message) return; - if (!lastCall) return; - - lastCall.content_attributes ??= {}; - lastCall.content_attributes.data = { - ...lastCall.content_attributes.data, + message.content_attributes ??= {}; + message.content_attributes.data = { + ...message.content_attributes.data, status: callStatus, }; }, diff --git a/app/javascript/dashboard/store/modules/conversations/specs/mutations.spec.js b/app/javascript/dashboard/store/modules/conversations/specs/mutations.spec.js index 6ac66353e..c535b7a72 100644 --- a/app/javascript/dashboard/store/modules/conversations/specs/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/conversations/specs/mutations.spec.js @@ -8,11 +8,12 @@ describe('#mutations', () => { mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { conversationId: 1, callStatus: 'ringing', + callSid: 'CA123', }); expect(state.allConversations).toEqual([]); }); - it('does nothing if no voice call message exists', () => { + it('does nothing if no matching voice call message exists', () => { const state = { allConversations: [ { id: 1, messages: [{ id: 1, content_type: 'text' }] }, @@ -21,6 +22,7 @@ describe('#mutations', () => { mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { conversationId: 1, callStatus: 'ringing', + callSid: 'CA123', }); expect(state.allConversations[0].messages[0]).toEqual({ id: 1, @@ -28,7 +30,7 @@ describe('#mutations', () => { }); }); - it('updates the last voice call message status', () => { + it('updates only the voice call message matching the given callSid', () => { const state = { allConversations: [ { @@ -37,12 +39,16 @@ describe('#mutations', () => { { id: 1, content_type: 'voice_call', - content_attributes: { data: { status: 'ringing' } }, + content_attributes: { + data: { call_sid: 'CA111', status: 'ringing' }, + }, }, { id: 2, content_type: 'voice_call', - content_attributes: { data: { status: 'ringing' } }, + content_attributes: { + data: { call_sid: 'CA222', status: 'ringing' }, + }, }, ], }, @@ -51,31 +57,14 @@ describe('#mutations', () => { mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { conversationId: 1, callStatus: 'in-progress', + callSid: 'CA111', }); expect( state.allConversations[0].messages[0].content_attributes.data.status - ).toBe('ringing'); + ).toBe('in-progress'); expect( state.allConversations[0].messages[1].content_attributes.data.status - ).toBe('in-progress'); - }); - - it('creates content_attributes.data if it does not exist', () => { - const state = { - allConversations: [ - { - id: 1, - messages: [{ id: 1, content_type: 'voice_call' }], - }, - ], - }; - mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { - conversationId: 1, - callStatus: 'completed', - }); - expect( - state.allConversations[0].messages[0].content_attributes.data.status - ).toBe('completed'); + ).toBe('ringing'); }); it('preserves existing data in content_attributes.data', () => { @@ -98,6 +87,7 @@ describe('#mutations', () => { mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { conversationId: 1, callStatus: 'in-progress', + callSid: 'CA123', }); expect( state.allConversations[0].messages[0].content_attributes.data @@ -114,6 +104,7 @@ describe('#mutations', () => { mutations[types.UPDATE_MESSAGE_CALL_STATUS](state, { conversationId: 1, callStatus: 'ringing', + callSid: 'CA123', }); expect(state.allConversations[0].messages).toEqual([]); }); diff --git a/enterprise/app/controllers/api/v1/accounts/conference_controller.rb b/enterprise/app/controllers/api/v1/accounts/conference_controller.rb index 6756ea33f..3361beac2 100644 --- a/enterprise/app/controllers/api/v1/accounts/conference_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/conference_controller.rb @@ -33,14 +33,10 @@ class Api::V1::Accounts::ConferenceController < Api::V1::Accounts::BaseControlle private def resolve_call! - inbox_calls = Call.where(inbox_id: @voice_inbox.id, provider: :twilio) + sid = params[:call_sid].presence + raise ActionController::ParameterMissing, :call_sid if sid.blank? - if params[:call_sid].present? - inbox_calls.find_by!(provider_call_id: params[:call_sid]) - else - conversation = fetch_conversation_by_display_id - inbox_calls.where(conversation_id: conversation.id).active.order(created_at: :desc).first! - end + Call.where(inbox_id: @voice_inbox.id, provider: :twilio).find_by!(provider_call_id: sid) end def set_voice_inbox_for_conference diff --git a/enterprise/app/controllers/twilio/voice_controller.rb b/enterprise/app/controllers/twilio/voice_controller.rb index 0f1892d38..72ea49a51 100644 --- a/enterprise/app/controllers/twilio/voice_controller.rb +++ b/enterprise/app/controllers/twilio/voice_controller.rb @@ -95,14 +95,10 @@ class Twilio::VoiceController < ApplicationController end def find_call_for_agent - if params[:call_sid].present? - Call.find_by!(provider: :twilio, provider_call_id: params[:call_sid]) - elsif params[:conversation_id].present? - conversation = current_account.conversations.find_by!(display_id: params[:conversation_id]) - Call.where(conversation_id: conversation.id).active.order(created_at: :desc).first! - else - Call.find_by!(provider: :twilio, provider_call_id: twilio_call_sid) - end + sid = params[:call_sid].presence + raise ArgumentError, 'call_sid is required for agent leg' if sid.blank? + + Call.find_by!(provider: :twilio, provider_call_id: sid) end def sync_outbound_leg(call_sid:, direction:) @@ -141,9 +137,10 @@ class Twilio::VoiceController < ApplicationController end def find_call_for_conference!(friendly_name:, call_sid:) + inbox_calls = Call.where(inbox_id: inbox.id, provider: :twilio) name = friendly_name.to_s - call = Call.twilio.by_conference_sid(name).first if name.present? - call || Call.find_by!(provider: :twilio, provider_call_id: call_sid) + call = inbox_calls.by_conference_sid(name).first if name.present? + call || inbox_calls.find_by!(provider_call_id: call_sid) end def set_inbox! diff --git a/enterprise/app/services/voice/conference/manager.rb b/enterprise/app/services/voice/conference/manager.rb index d86b70457..c6115137b 100644 --- a/enterprise/app/services/voice/conference/manager.rb +++ b/enterprise/app/services/voice/conference/manager.rb @@ -23,6 +23,9 @@ class Voice::Conference::Manager end def mark_ringing! + # Guard against delayed conference-start retries rolling a progressed call back to ringing. + return unless call.status == 'ringing' + status_manager.process_status_update('ringing') end diff --git a/enterprise/app/services/voice/status_update_service.rb b/enterprise/app/services/voice/status_update_service.rb index 7ff22f2c9..db854b2c3 100644 --- a/enterprise/app/services/voice/status_update_service.rb +++ b/enterprise/app/services/voice/status_update_service.rb @@ -19,7 +19,7 @@ class Voice::StatusUpdateService normalized_status = normalize_status(call_status) return if normalized_status.blank? - call = Call.find_by_provider_call_id(:twilio, call_sid) + call = Call.where(account_id: account.id).find_by(provider: :twilio, provider_call_id: call_sid) return unless call Voice::CallStatus::Manager.new(call: call).process_status_update( diff --git a/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb index ffb397464..c35689c84 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb @@ -91,13 +91,13 @@ RSpec.describe Api::V1::Accounts::ConferenceController, type: :request do expect(conference_service).to have_received(:mark_agent_joined) end - it 'falls back to the most recent active call on the conversation when call_sid is missing' do + it 'rejects the request when call_sid is missing' do post "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", headers: agent.create_new_auth_token, params: { conversation_id: conversation.display_id } - expect(response).to have_http_status(:ok) - expect(conference_service).to have_received(:ensure_conference_sid) + expect(response).to have_http_status(:unprocessable_content) + expect(conference_service).not_to have_received(:ensure_conference_sid) end it 'does not allow accessing calls from inboxes without access' do