From 07de9ae1f14e55f1ee7a4348c3e47235ac90e63b Mon Sep 17 00:00:00 2001 From: Muhsin <12408980+muhsin-k@users.noreply.github.com> Date: Fri, 17 Apr 2026 20:24:37 +0400 Subject: [PATCH] test(voice): update specs to match Call-model-backed voice flow - Add calls factory. - voice_controller_spec, conference_controller_spec, and the voice service specs now assert against Call records instead of conversation.identifier / additional_attributes. - Scope Call lookup in ConferenceController to the authorized inbox so call_sid alone can't cross-inbox escalate. --- .../api/v1/accounts/conference_controller.rb | 11 +- .../v1/accounts/conference_controller_spec.rb | 88 +++++++++---- .../twilio/voice_controller_spec.rb | 76 ++++++----- .../voice/inbound_call_builder_spec.rb | 124 +++++++++--------- .../voice/outbound_call_builder_spec.rb | 79 +++++------ .../twilio/conference_service_spec.rb | 40 +++--- .../voice/status_update_service_spec.rb | 58 ++++---- spec/factories/calls.rb | 12 ++ 8 files changed, 273 insertions(+), 215 deletions(-) create mode 100644 spec/factories/calls.rb diff --git a/enterprise/app/controllers/api/v1/accounts/conference_controller.rb b/enterprise/app/controllers/api/v1/accounts/conference_controller.rb index b20446413..6756ea33f 100644 --- a/enterprise/app/controllers/api/v1/accounts/conference_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/conference_controller.rb @@ -33,11 +33,14 @@ class Api::V1::Accounts::ConferenceController < Api::V1::Accounts::BaseControlle private def resolve_call! - call_sid = params[:call_sid] - return Call.find_by!(provider: :twilio, provider_call_id: call_sid) if call_sid.present? + inbox_calls = Call.where(inbox_id: @voice_inbox.id, provider: :twilio) - conversation = fetch_conversation_by_display_id - Call.where(conversation_id: conversation.id).active.order(created_at: :desc).first! + 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 end def set_voice_inbox_for_conference 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 f3019910a..ffb397464 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conference_controller_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Api::V1::Accounts::ConferenceController, type: :request do let(:account) { create(:account) } let(:voice_channel) { create(:channel_twilio_sms, :with_voice, account: account) } let(:voice_inbox) { voice_channel.inbox } - let(:conversation) { create(:conversation, account: account, inbox: voice_inbox, identifier: nil) } + let(:conversation) { create(:conversation, account: account, inbox: voice_inbox) } let(:admin) { create(:user, :administrator, account: account) } let(:agent) { create(:user, account: account, role: :agent) } @@ -66,41 +66,57 @@ RSpec.describe Api::V1::Accounts::ConferenceController, type: :request do end context 'when authenticated agent with inbox access' do - before { create(:inbox_member, inbox: voice_inbox, user: agent) } + before do + create(:inbox_member, inbox: voice_inbox, user: agent) + create( + :call, + account: account, + inbox: voice_inbox, + conversation: conversation, + contact: conversation.contact, + provider_call_id: 'CALL123' + ) + end - it 'creates conference and sets identifier' do + it 'resolves the Call by call_sid and invokes the conference service' do post "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", headers: agent.create_new_auth_token, params: { conversation_id: conversation.display_id, call_sid: 'CALL123' } expect(response).to have_http_status(:ok) body = response.parsed_body - expect(body['conference_sid']).to be_present - conversation.reload - expect(conversation.identifier).to eq('CALL123') + expect(body['conference_sid']).to eq('CF123') + expect(body['id']).to eq(conversation.display_id) expect(conference_service).to have_received(:ensure_conference_sid) expect(conference_service).to have_received(:mark_agent_joined) end - it 'does not allow accessing conversations from inboxes without access' do - other_inbox = create(:inbox, account: account) - other_conversation = create(:conversation, account: account, inbox: other_inbox, identifier: nil) - - post "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", - headers: agent.create_new_auth_token, - params: { conversation_id: other_conversation.display_id, call_sid: 'CALL123' } - - expect(response).to have_http_status(:not_found) - other_conversation.reload - expect(other_conversation.identifier).to be_nil - end - - it 'returns conflict when call_sid missing' do + it 'falls back to the most recent active call on the conversation 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(:unprocessable_content) + expect(response).to have_http_status(:ok) + expect(conference_service).to have_received(:ensure_conference_sid) + end + + it 'does not allow accessing calls from inboxes without access' do + other_inbox = create(:inbox, account: account) + other_conversation = create(:conversation, account: account, inbox: other_inbox) + create( + :call, + account: account, + inbox: other_inbox, + conversation: other_conversation, + contact: other_conversation.contact, + provider_call_id: 'OTHER123' + ) + + post "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", + headers: agent.create_new_auth_token, + params: { conversation_id: other_conversation.display_id, call_sid: 'OTHER123' } + + expect(response).to have_http_status(:not_found) end end end @@ -115,25 +131,43 @@ RSpec.describe Api::V1::Accounts::ConferenceController, type: :request do end context 'when authenticated agent with inbox access' do - before { create(:inbox_member, inbox: voice_inbox, user: agent) } + before do + create(:inbox_member, inbox: voice_inbox, user: agent) + create( + :call, + account: account, + inbox: voice_inbox, + conversation: conversation, + contact: conversation.contact, + provider_call_id: 'CALL123' + ) + end - it 'ends conference and returns success' do + it 'ends the conference for the resolved call' do delete "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", headers: agent.create_new_auth_token, - params: { conversation_id: conversation.display_id } + params: { conversation_id: conversation.display_id, call_sid: 'CALL123' } expect(response).to have_http_status(:ok) expect(response.parsed_body['id']).to eq(conversation.display_id) expect(conference_service).to have_received(:end_conference) end - it 'does not allow ending conferences for conversations from inboxes without access' do + it 'does not allow ending conferences for calls from inboxes without access' do other_inbox = create(:inbox, account: account) - other_conversation = create(:conversation, account: account, inbox: other_inbox, identifier: nil) + other_conversation = create(:conversation, account: account, inbox: other_inbox) + create( + :call, + account: account, + inbox: other_inbox, + conversation: other_conversation, + contact: other_conversation.contact, + provider_call_id: 'OTHER123' + ) delete "/api/v1/accounts/#{account.id}/inboxes/#{voice_inbox.id}/conference", headers: agent.create_new_auth_token, - params: { conversation_id: other_conversation.display_id } + params: { conversation_id: other_conversation.display_id, call_sid: 'OTHER123' } expect(response).to have_http_status(:not_found) end diff --git a/spec/enterprise/controllers/twilio/voice_controller_spec.rb b/spec/enterprise/controllers/twilio/voice_controller_spec.rb index c04f5ae41..ac3dda08f 100644 --- a/spec/enterprise/controllers/twilio/voice_controller_spec.rb +++ b/spec/enterprise/controllers/twilio/voice_controller_spec.rb @@ -19,15 +19,24 @@ RSpec.describe 'Twilio::VoiceController', type: :request do let(:to_number) { channel.phone_number } it 'invokes Voice::InboundCallBuilder for inbound calls and renders conference TwiML' do - instance_double(Voice::InboundCallBuilder) conversation = create(:conversation, account: account, inbox: inbox) + contact = conversation.contact + call = create( + :call, + account: account, + inbox: inbox, + conversation: conversation, + contact: contact, + provider_call_id: call_sid + ) + call.update!(conference_sid: Voice::Conference::Name.for(call)) expect(Voice::InboundCallBuilder).to receive(:perform!).with( account: account, inbox: inbox, from_number: from_number, call_sid: call_sid - ).and_return(conversation) + ).and_return(call) post "/twilio/voice/call/#{digits}", params: { 'CallSid' => call_sid, @@ -39,60 +48,63 @@ RSpec.describe 'Twilio::VoiceController', type: :request do expect(response).to have_http_status(:ok) expect(response.body).to include('') expect(response.body).to include('') + expect(response.body).to include(call.conference_sid) end - it 'syncs an existing outbound conversation when Twilio sends the PSTN leg' do - conversation = create(:conversation, account: account, inbox: inbox, identifier: call_sid) - sync_double = instance_double(Voice::CallSessionSyncService, perform: conversation) + it 'looks up the Call when Twilio sends the outbound-api PSTN leg' do + conversation = create(:conversation, account: account, inbox: inbox) + call = create( + :call, + account: account, + inbox: inbox, + conversation: conversation, + contact: conversation.contact, + direction: :outgoing, + provider_call_id: call_sid + ) + call.update!(conference_sid: Voice::Conference::Name.for(call)) + sync_double = instance_double(Voice::CallSessionSyncService, perform: call) expect(Voice::CallSessionSyncService).to receive(:new).with( - hash_including( - conversation: conversation, - call_sid: call_sid, - message_call_sid: conversation.identifier, - leg: { - from_number: from_number, - to_number: to_number, - direction: 'outbound' - } - ) + hash_including(call: call, parent_call_sid: nil) ).and_return(sync_double) post "/twilio/voice/call/#{digits}", params: { 'CallSid' => call_sid, - 'From' => from_number, - 'To' => to_number, + 'From' => to_number, + 'To' => from_number, 'Direction' => 'outbound-api' } expect(response).to have_http_status(:ok) - expect(response.body).to include('') + expect(response.body).to include(call.conference_sid) end it 'uses the parent call SID when syncing outbound-dial legs' do parent_sid = 'CA_parent' child_sid = 'CA_child' - conversation = create(:conversation, account: account, inbox: inbox, identifier: parent_sid) - sync_double = instance_double(Voice::CallSessionSyncService, perform: conversation) + conversation = create(:conversation, account: account, inbox: inbox) + call = create( + :call, + account: account, + inbox: inbox, + conversation: conversation, + contact: conversation.contact, + direction: :outgoing, + provider_call_id: parent_sid + ) + call.update!(conference_sid: Voice::Conference::Name.for(call)) + sync_double = instance_double(Voice::CallSessionSyncService, perform: call) expect(Voice::CallSessionSyncService).to receive(:new).with( - hash_including( - conversation: conversation, - call_sid: child_sid, - message_call_sid: parent_sid, - leg: { - from_number: from_number, - to_number: to_number, - direction: 'outbound' - } - ) + hash_including(call: call, parent_call_sid: parent_sid) ).and_return(sync_double) post "/twilio/voice/call/#{digits}", params: { 'CallSid' => child_sid, 'ParentCallSid' => parent_sid, - 'From' => from_number, - 'To' => to_number, + 'From' => to_number, + 'To' => from_number, 'Direction' => 'outbound-dial' } diff --git a/spec/enterprise/services/voice/inbound_call_builder_spec.rb b/spec/enterprise/services/voice/inbound_call_builder_spec.rb index a021c3a9c..e188373a4 100644 --- a/spec/enterprise/services/voice/inbound_call_builder_spec.rb +++ b/spec/enterprise/services/voice/inbound_call_builder_spec.rb @@ -7,7 +7,6 @@ RSpec.describe Voice::InboundCallBuilder do let(:channel) { create(:channel_twilio_sms, :with_voice, account: account, phone_number: '+15551239999') } let(:inbox) { channel.inbox } let(:from_number) { '+15550001111' } - let(:to_number) { channel.phone_number } let(:call_sid) { 'CA1234567890abcdef' } before do @@ -24,98 +23,95 @@ RSpec.describe Voice::InboundCallBuilder do ) end - context 'when no existing conversation matches call_sid' do - it 'creates a new inbound conversation with ringing status' do - conversation = nil - expect { conversation = perform_builder }.to change(account.conversations, :count).by(1) + context 'when no existing call matches call_sid' do + it 'creates a new conversation and Call with ringing status' do + call = nil + expect { call = perform_builder }.to change(account.conversations, :count).by(1).and change(Call, :count).by(1) - attrs = conversation.additional_attributes - expect(conversation.identifier).to eq(call_sid) - expect(attrs['call_direction']).to eq('inbound') - expect(attrs['call_status']).to eq('ringing') - expect(attrs['conference_sid']).to be_present - expect(attrs.dig('meta', 'initiated_at')).to be_present - expect(conversation.contact.phone_number).to eq(from_number) + aggregate_failures do + expect(call).to be_a(Call) + expect(call.provider_call_id).to eq(call_sid) + expect(call.provider).to eq('twilio') + expect(call.direction).to eq('incoming') + expect(call.status).to eq('ringing') + expect(call.conference_sid).to eq("conf_account_#{account.id}_call_#{call.id}") + expect(call.meta['initiated_at']).to be_present + expect(call.contact.phone_number).to eq(from_number) + end end - it 'creates a single voice_call message marked as incoming' do - conversation = perform_builder - voice_message = conversation.messages.voice_calls.last + it 'creates a voice_call message matched to the call and linked via message_id' do + call = perform_builder + voice_message = call.conversation.messages.voice_calls.last expect(voice_message).to be_present expect(voice_message.message_type).to eq('incoming') + expect(call.message_id).to eq(voice_message.id) + data = voice_message.content_attributes['data'] expect(data).to include( 'call_sid' => call_sid, 'status' => 'ringing', 'call_direction' => 'inbound', - 'conference_sid' => conversation.additional_attributes['conference_sid'], + 'conference_sid' => call.conference_sid, 'from_number' => from_number, 'to_number' => inbox.channel.phone_number ) - expect(data['meta']['created_at']).to be_present - expect(data['meta']['ringing_at']).to be_present end it 'sets the contact name to the phone number for new callers' do - conversation = perform_builder + call = perform_builder - expect(conversation.contact.name).to eq(from_number) + expect(call.contact.name).to eq(from_number) end - it 'ensures the conversation has a display_id before building the conference SID' do - allow(Voice::Conference::Name).to receive(:for).and_wrap_original do |original, conversation| - expect(conversation.display_id).to be_present - original.call(conversation) - end + it 'does not set conversation.identifier or write call state to additional_attributes' do + call = perform_builder + conversation = call.conversation - perform_builder + expect(conversation.identifier).to be_nil + expect(conversation.additional_attributes).not_to include('call_status', 'call_direction', 'conference_sid') end end - context 'when a conversation already exists for the call_sid' do - let(:contact) { create(:contact, account: account, phone_number: from_number) } + context 'when a Call already exists for the call_sid' do + let(:existing_call) do + conversation = create(:conversation, account: account, inbox: inbox) + create( + :call, + account: account, + inbox: inbox, + conversation: conversation, + contact: conversation.contact, + provider_call_id: call_sid + ) + end + + it 'returns the existing call without creating a duplicate' do + existing_call + expect { perform_builder }.not_to change(Call, :count) + expect(perform_builder).to eq(existing_call) + end + end + + context 'when the inbox has lock_to_single_conversation enabled' do + let!(:contact) { create(:contact, account: account, phone_number: from_number) } let!(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: inbox, source_id: from_number) } - let!(:existing_conversation) do - create( - :conversation, - account: account, - inbox: inbox, - contact: contact, - contact_inbox: contact_inbox, - identifier: call_sid, - additional_attributes: { 'call_direction' => 'outbound', 'conference_sid' => nil } - ) - end - let(:existing_message) do - create( - :message, - account: account, - inbox: inbox, - conversation: existing_conversation, - message_type: :incoming, - content_type: :voice_call, - sender: contact, - content_attributes: { 'data' => { 'call_sid' => call_sid, 'status' => 'queued' } } - ) + let!(:existing_open_conversation) do + create(:conversation, account: account, inbox: inbox, contact: contact, contact_inbox: contact_inbox, status: :open) end - it 'reuses the conversation without creating a duplicate' do - existing_message - expect { perform_builder }.not_to change(account.conversations, :count) - existing_conversation.reload - expect(existing_conversation.additional_attributes['call_direction']).to eq('inbound') - expect(existing_conversation.additional_attributes['call_status']).to eq('ringing') + before { inbox.update!(lock_to_single_conversation: true) } + + it 'reuses the most recent non-resolved conversation' do + call = nil + expect { call = perform_builder }.not_to change(account.conversations, :count) + expect(call.conversation).to eq(existing_open_conversation) end - it 'updates the existing voice call message instead of creating a new one' do - existing_message - expect { perform_builder }.not_to(change { existing_conversation.reload.messages.voice_calls.count }) - updated_message = existing_conversation.reload.messages.voice_calls.last - - data = updated_message.content_attributes['data'] - expect(data['status']).to eq('ringing') - expect(data['call_direction']).to eq('inbound') + it 'still creates a new Call and voice_call message on the reused conversation' do + expect { perform_builder }.to change(Call, :count).by(1) + .and change { existing_open_conversation.reload.messages.voice_calls.count }.by(1) end end end diff --git a/spec/enterprise/services/voice/outbound_call_builder_spec.rb b/spec/enterprise/services/voice/outbound_call_builder_spec.rb index 13d667925..49ed44474 100644 --- a/spec/enterprise/services/voice/outbound_call_builder_spec.rb +++ b/spec/enterprise/services/voice/outbound_call_builder_spec.rb @@ -15,45 +15,52 @@ RSpec.describe Voice::OutboundCallBuilder do .and_return(instance_double(Twilio::VoiceWebhookSetupService, perform: "AP#{SecureRandom.hex(8)}")) allow(inbox).to receive(:channel).and_return(channel) allow(channel).to receive(:initiate_call).and_return({ call_sid: call_sid }) - allow(Voice::Conference::Name).to receive(:for).and_call_original end describe '.perform!' do - it 'creates a conversation and voice call message' do - conversation_count = account.conversations.count - inbox_link_count = contact.contact_inboxes.where(inbox_id: inbox.id).count + it 'creates a conversation, Call, and voice_call message' do + call = nil + expect do + call = described_class.perform!( + account: account, + inbox: inbox, + user: user, + contact: contact + ) + end.to change(account.conversations, :count).by(1).and change(Call, :count).by(1) - result = described_class.perform!( + aggregate_failures do + expect(call).to be_a(Call) + expect(call.provider_call_id).to eq(call_sid) + expect(call.direction).to eq('outgoing') + expect(call.status).to eq('ringing') + expect(call.accepted_by_agent_id).to eq(user.id) + expect(call.conference_sid).to eq("conf_account_#{account.id}_call_#{call.id}") + + voice_message = call.conversation.messages.voice_calls.last + expect(call.message_id).to eq(voice_message.id) + expect(voice_message.message_type).to eq('outgoing') + + expect(voice_message.content_attributes['data']).to include( + 'call_sid' => call_sid, + 'call_direction' => 'outbound', + 'conference_sid' => call.conference_sid, + 'from_number' => channel.phone_number, + 'to_number' => contact.phone_number + ) + end + end + + it 'does not set conversation.identifier or write call state to additional_attributes' do + call = described_class.perform!( account: account, inbox: inbox, user: user, contact: contact ) - expect(account.conversations.count).to eq(conversation_count + 1) - expect(contact.contact_inboxes.where(inbox_id: inbox.id).count).to eq(inbox_link_count + 1) - - conversation = result[:conversation].reload - attrs = conversation.additional_attributes - - aggregate_failures do - expect(result[:call_sid]).to eq(call_sid) - expect(conversation.identifier).to eq(call_sid) - expect(attrs).to include('call_direction' => 'outbound', 'call_status' => 'ringing') - expect(attrs['agent_id']).to eq(user.id) - expect(attrs['conference_sid']).to be_present - - voice_message = conversation.messages.voice_calls.last - expect(voice_message.message_type).to eq('outgoing') - - message_data = voice_message.content_attributes['data'] - expect(message_data).to include( - 'call_sid' => call_sid, - 'conference_sid' => attrs['conference_sid'], - 'from_number' => channel.phone_number, - 'to_number' => contact.phone_number - ) - end + expect(call.conversation.identifier).to be_nil + expect(call.conversation.additional_attributes).not_to include('call_status', 'call_direction', 'agent_id', 'conference_sid') end it 'raises an error when contact is missing a phone number' do @@ -79,19 +86,5 @@ RSpec.describe Voice::OutboundCallBuilder do ) end.to raise_error(ArgumentError, 'Agent required') end - - it 'ensures the conversation has a display_id before building the conference SID' do - allow(Voice::Conference::Name).to receive(:for).and_wrap_original do |original, conversation| - expect(conversation.display_id).to be_present - original.call(conversation) - end - - described_class.perform!( - account: account, - inbox: inbox, - user: user, - contact: contact - ) - end end end diff --git a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb index 968d6558c..bf8ce9344 100644 --- a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb +++ b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb @@ -4,8 +4,17 @@ describe Voice::Provider::Twilio::ConferenceService do let(:account) { create(:account) } let(:channel) { create(:channel_twilio_sms, :with_voice, account: account) } let(:conversation) { create(:conversation, account: account, inbox: channel.inbox) } + let(:call) do + create( + :call, + account: account, + inbox: channel.inbox, + conversation: conversation, + contact: conversation.contact + ) + end let(:twilio_client) { instance_double(Twilio::REST::Client) } - let(:service) { described_class.new(conversation: conversation, twilio_client: twilio_client) } + let(:service) { described_class.new(call: call, twilio_client: twilio_client) } let(:webhook_service) { instance_double(Twilio::VoiceWebhookSetupService, perform: true) } before do @@ -13,42 +22,37 @@ describe Voice::Provider::Twilio::ConferenceService do end describe '#ensure_conference_sid' do - it 'returns existing sid if present' do - conversation.update!(additional_attributes: { 'conference_sid' => 'CF_EXISTING' }) + it 'returns existing sid if present on the Call' do + call.update!(conference_sid: 'CF_EXISTING') expect(service.ensure_conference_sid).to eq('CF_EXISTING') end it 'sets and returns generated sid when missing' do - allow(Voice::Conference::Name).to receive(:for).and_return('CF_GEN') - - sid = service.ensure_conference_sid - - expect(sid).to eq('CF_GEN') - expect(conversation.reload.additional_attributes['conference_sid']).to eq('CF_GEN') + expect(service.ensure_conference_sid).to eq("conf_account_#{account.id}_call_#{call.id}") + expect(call.reload.conference_sid).to eq("conf_account_#{account.id}_call_#{call.id}") end end describe '#mark_agent_joined' do - it 'stores agent join metadata' do + it 'sets accepted_by_agent on the Call' do agent = create(:user, account: account) service.mark_agent_joined(user: agent) - attrs = conversation.reload.additional_attributes - expect(attrs['agent_joined']).to be true - expect(attrs['joined_by']['id']).to eq(agent.id) + expect(call.reload.accepted_by_agent_id).to eq(agent.id) end end describe '#end_conference' do - it 'completes in-progress conferences' do + it 'completes in-progress conferences matching the call conference_sid' do + call.update!(conference_sid: 'CF123_FRIENDLY') conferences_proxy = instance_double(Twilio::REST::Api::V2010::AccountContext::ConferenceList) conf_instance = instance_double(Twilio::REST::Api::V2010::AccountContext::ConferenceInstance, sid: 'CF123') conf_context = instance_double(Twilio::REST::Api::V2010::AccountContext::ConferenceInstance) allow(twilio_client).to receive(:conferences).with(no_args).and_return(conferences_proxy) - allow(conferences_proxy).to receive(:list).and_return([conf_instance]) + allow(conferences_proxy).to receive(:list).with(friendly_name: 'CF123_FRIENDLY', status: 'in-progress').and_return([conf_instance]) allow(twilio_client).to receive(:conferences).with('CF123').and_return(conf_context) allow(conf_context).to receive(:update).with(status: 'completed') @@ -56,5 +60,11 @@ describe Voice::Provider::Twilio::ConferenceService do expect(conf_context).to have_received(:update).with(status: 'completed') end + + it 'no-ops when call has no conference_sid' do + allow(twilio_client).to receive(:conferences) + service.end_conference + expect(twilio_client).not_to have_received(:conferences) + end end end diff --git a/spec/enterprise/services/voice/status_update_service_spec.rb b/spec/enterprise/services/voice/status_update_service_spec.rb index 6f88f51c6..7a28e8d9f 100644 --- a/spec/enterprise/services/voice/status_update_service_spec.rb +++ b/spec/enterprise/services/voice/status_update_service_spec.rb @@ -4,75 +4,73 @@ require 'rails_helper' RSpec.describe Voice::StatusUpdateService do let(:account) { create(:account) } - let!(:contact) { create(:contact, account: account, phone_number: from_number) } - let(:contact_inbox) { ContactInbox.create!(contact: contact, inbox: inbox, source_id: from_number) } + let(:channel) { create(:channel_twilio_sms, :with_voice, account: account, phone_number: '+15551230002') } + let(:inbox) { channel.inbox } + let(:from_number) { '+15550002222' } + let(:call_sid) { 'CATESTSTATUS123' } + let(:contact) { create(:contact, account: account, phone_number: from_number) } + let(:contact_inbox) { create(:contact_inbox, contact: contact, inbox: inbox, source_id: from_number) } let(:conversation) do - Conversation.create!( - account_id: account.id, - inbox_id: inbox.id, - contact_id: contact.id, - contact_inbox_id: contact_inbox.id, - identifier: call_sid, - additional_attributes: { 'call_direction' => 'inbound', 'call_status' => 'ringing' } + create(:conversation, account: account, inbox: inbox, contact: contact, contact_inbox: contact_inbox) + end + let!(:call) do + create( + :call, + account: account, + inbox: inbox, + conversation: conversation, + contact: contact, + provider_call_id: call_sid ) end - let(:message) do - conversation.messages.create!( + let!(:message) do + msg = conversation.messages.create!( account_id: account.id, inbox_id: inbox.id, message_type: :incoming, sender: contact, content: 'Voice Call', content_type: 'voice_call', - content_attributes: { data: { call_sid: call_sid, status: 'ringing' } } + content_attributes: { 'data' => { 'call_sid' => call_sid, 'status' => 'ringing' } } ) + call.update!(message_id: msg.id) + msg end - let(:channel) { create(:channel_twilio_sms, :with_voice, account: account, phone_number: '+15551230002') } - let(:inbox) { channel.inbox } - let(:from_number) { '+15550002222' } - let(:call_sid) { 'CATESTSTATUS123' } before do allow(Twilio::VoiceWebhookSetupService).to receive(:new) .and_return(instance_double(Twilio::VoiceWebhookSetupService, perform: "AP#{SecureRandom.hex(16)}")) end - it 'updates conversation and last voice message with call status' do - # Ensure records are created after stub setup - conversation - message - + it 'updates the Call and the matching voice_call message with the normalized status' do described_class.new( account: account, call_sid: call_sid, call_status: 'completed' ).perform - conversation.reload + call.reload message.reload - expect(conversation.additional_attributes['call_status']).to eq('completed') + expect(call.status).to eq('completed') expect(message.content_attributes.dig('data', 'status')).to eq('completed') end - it 'normalizes busy to no-answer' do - conversation - message - + it 'normalizes busy to no_answer on the Call and no-answer on the message payload' do described_class.new( account: account, call_sid: call_sid, call_status: 'busy' ).perform - conversation.reload + call.reload message.reload - expect(conversation.additional_attributes['call_status']).to eq('no-answer') + expect(call.status).to eq('no_answer') expect(message.content_attributes.dig('data', 'status')).to eq('no-answer') end - it 'no-ops when conversation not found' do + it 'no-ops when no Call matches the provided call_sid' do expect do described_class.new(account: account, call_sid: 'UNKNOWN', call_status: 'busy').perform end.not_to raise_error diff --git a/spec/factories/calls.rb b/spec/factories/calls.rb new file mode 100644 index 000000000..d050449d9 --- /dev/null +++ b/spec/factories/calls.rb @@ -0,0 +1,12 @@ +FactoryBot.define do + factory :call do + association :account + association :inbox + association :conversation + association :contact + provider { :twilio } + direction { :incoming } + status { 'ringing' } + sequence(:provider_call_id) { |n| "CA#{SecureRandom.hex(15)}#{n}" } + end +end