diff --git a/enterprise/app/controllers/twilio/voice_controller.rb b/enterprise/app/controllers/twilio/voice_controller.rb index 66fef9583..52ec2637b 100644 --- a/enterprise/app/controllers/twilio/voice_controller.rb +++ b/enterprise/app/controllers/twilio/voice_controller.rb @@ -107,8 +107,8 @@ class Twilio::VoiceController < ApplicationController when 'inbound' Voice::InboundCallBuilder.perform!( inbox: inbox, - from_number: twilio_from, - call_sid: twilio_call_sid + call_sid: twilio_call_sid, + caller: { source_ids: [twilio_from], contact_attributes: { name: twilio_from, phone_number: twilio_from } } ) when 'outbound-api', 'outbound-dial' sync_outbound_leg(call_sid: twilio_call_sid, direction: twilio_direction) diff --git a/enterprise/app/services/voice/inbound_call_builder.rb b/enterprise/app/services/voice/inbound_call_builder.rb index eef70e76b..c928da43f 100644 --- a/enterprise/app/services/voice/inbound_call_builder.rb +++ b/enterprise/app/services/voice/inbound_call_builder.rb @@ -1,17 +1,19 @@ class Voice::InboundCallBuilder - attr_reader :inbox, :from_number, :call_sid, :provider, :extra_meta + attr_reader :inbox, :call_sid, :provider, :extra_meta, :source_ids, :contact_attributes - def self.perform!(inbox:, from_number:, call_sid:, provider: :twilio, extra_meta: {}) - new(inbox: inbox, from_number: from_number, call_sid: call_sid, - provider: provider, extra_meta: extra_meta).perform! + # `caller` carries the contact identity: { source_ids:, contact_attributes: }. Twilio passes + # its single +phone source_id; WhatsApp passes the message-path phone/user_id/parent_user_id set. + def self.perform!(inbox:, call_sid:, caller:, provider: :twilio, extra_meta: {}) + new(inbox: inbox, call_sid: call_sid, caller: caller, provider: provider, extra_meta: extra_meta).perform! end - def initialize(inbox:, from_number:, call_sid:, provider: :twilio, extra_meta: {}) + def initialize(inbox:, call_sid:, caller:, provider: :twilio, extra_meta: {}) @inbox = inbox - @from_number = from_number @call_sid = call_sid @provider = provider.to_sym @extra_meta = extra_meta || {} + @source_ids = Array(caller[:source_ids]).compact_blank + @contact_attributes = caller[:contact_attributes] || {} end def perform! @@ -43,46 +45,17 @@ class Voice::InboundCallBuilder .find_by(provider: provider, provider_call_id: call_sid) end - # Always look up by (inbox, source_id) first — that pair has a UNIQUE index, so - # creating with a colliding source_id under a different contact would raise - # RecordNotUnique. Reuse the existing ContactInbox (and its contact) when found. - # A concurrent message webhook for the same wa_id can win the (inbox_id, source_id) - # race; rescue and re-find so the call path doesn't drop the connect. + # Resolve the contact/ContactInbox the same way inbound messages do — match across every + # candidate source_id (phone + BSUID aliases) so a call reuses the existing thread, creating + # one keyed on the first (phone, else BSUID) only when none exists. Shared with messaging via + # ContactInboxSourceIdResolver, which also rescues the concurrent-webhook create race. def ensure_contact_inbox! - sid = source_id_for_provider - existing = inbox.contact_inboxes.find_by(source_id: sid) - return existing if existing - - ContactInbox.create!(contact: ensure_contact!, inbox: inbox, source_id: sid) - rescue ActiveRecord::RecordNotUnique - inbox.contact_inboxes.find_by!(source_id: sid) + ContactInboxSourceIdResolver.new( + inbox: inbox, source_ids: source_ids, contact_attributes: contact_attributes + ).perform end - def ensure_contact! - contact = account.contacts.find_or_create_by!(phone_number: from_number) do |record| - record.name = contact_name.presence || from_number - end - contact.update!(name: contact_name) if contact_name.present? && contact.name == from_number - contact - end - - # WhatsApp inbound calls carry the caller's profile name in extra_meta; Twilio - # calls don't, so contact naming falls back to the phone number. - def contact_name - extra_meta['contact_name'].presence - end - - # WhatsApp ContactInbox.source_id must be digits-only (the wa_id); Twilio accepts the +. - # Run BR/AR-style wa_id normalization (same path messaging uses) so an inbound call - # finds the existing ContactInbox instead of forking a new contact/conversation. - def source_id_for_provider - return from_number unless provider == :whatsapp - - digits = from_number.to_s.delete_prefix('+') - Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact_by_provider(digits, :cloud) - end - - # Mirror incoming-message routing: reuse the open conversation (or the last one when locked), else create new. + # Mirror Whatsapp::IncomingMessageBaseService#set_conversation: reuse this row's open conversation (or last when locked), else create. def resolve_conversation!(contact, contact_inbox) reusable = if inbox.lock_to_single_conversation contact_inbox.conversations.last diff --git a/enterprise/app/services/whatsapp/inbound_call_identity_builder.rb b/enterprise/app/services/whatsapp/inbound_call_identity_builder.rb new file mode 100644 index 000000000..590a1f923 --- /dev/null +++ b/enterprise/app/services/whatsapp/inbound_call_identity_builder.rb @@ -0,0 +1,48 @@ +class Whatsapp::InboundCallIdentityBuilder + pattr_initialize [:inbox!, :params!] + + # Build the message path's source_id set (phone wa_id -> user_id -> parent_user_id) plus + # contact attributes, so the resolver lands a call on the same ContactInbox a message would. + # BSUIDs ride in from_user_id/from_parent_user_id (or the contact's user_id/parent_user_id), + # never in `from` (the phone wa_id). + def perform(payload) + contact = caller_contact(payload) + phone = contact[:wa_id].presence || payload[:from].presence + source_ids = [ + phone_source_id(phone), + payload[:from_user_id].presence || contact[:user_id].presence, + payload[:from_parent_user_id].presence || contact[:parent_user_id].presence + ].compact_blank.uniq + { source_ids: source_ids, contact_attributes: contact_attributes(contact, phone, source_ids.first) } + end + + private + + # Normalize the wa_id the same way messaging does so a call matches its stored source_id. + def phone_source_id(phone) + return unless phone.to_s.match?(/\A\d{1,15}\z/) + + Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact_by_provider(phone.to_s, :cloud) + end + + def contact_attributes(contact, phone, source_identifier) + name = contact.dig(:profile, :name).presence || source_identifier + return { name: name } unless phone.to_s.match?(/\A\d{1,15}\z/) + + formatted = "+#{phone}" + { name: name == phone ? formatted : name, phone_number: formatted } + end + + # Match the contacts entry to THIS caller so batched payloads don't borrow another's identity. + def caller_contact(payload) + Array(params[:contacts]).map(&:with_indifferent_access).find do |c| + identifier_match?(c[:wa_id], payload[:from]) || + identifier_match?(c[:user_id], payload[:from_user_id]) || + identifier_match?(c[:parent_user_id], payload[:from_parent_user_id]) + end || {}.with_indifferent_access + end + + def identifier_match?(left, right) + left.present? && right.present? && left.to_s == right.to_s + end +end diff --git a/enterprise/app/services/whatsapp/incoming_call_service.rb b/enterprise/app/services/whatsapp/incoming_call_service.rb index 11b35d95a..ea08c8415 100644 --- a/enterprise/app/services/whatsapp/incoming_call_service.rb +++ b/enterprise/app/services/whatsapp/incoming_call_service.rb @@ -95,28 +95,21 @@ class Whatsapp::IncomingCallService # commit) already terminal, never `ringing` — agents aren't rung for a dead call. def build_inbound_call(payload, sdp_offer) ActiveRecord::Base.transaction do - call = Voice::InboundCallBuilder.perform!(inbox: inbox, from_number: "+#{payload[:from]}", call_sid: payload[:id], - provider: :whatsapp, extra_meta: inbound_extra_meta(payload, sdp_offer)) + identity = Whatsapp::InboundCallIdentityBuilder.new(inbox: inbox, params: params).perform(payload) + extra_meta = { 'sdp_offer' => sdp_offer, 'ice_servers' => Call.default_ice_servers } + call = Voice::InboundCallBuilder.perform!(inbox: inbox, call_sid: payload[:id], + provider: :whatsapp, extra_meta: extra_meta, caller: identity) + sync_caller_identifiers(call, identity) tombstone = consume_terminate_tombstone(payload[:id]) finalize_terminate(call, tombstone['duration'], tombstone['terminate_reason']) if tombstone call end end - def inbound_extra_meta(payload, sdp_offer) - extra_meta = { 'sdp_offer' => sdp_offer, 'ice_servers' => Call.default_ice_servers } - name = caller_profile_name(payload) - extra_meta['contact_name'] = name if name.present? - extra_meta - end - - # Match strictly on wa_id (== calls[].from): in a batched payload missing this - # call's contact entry, borrowing another caller's name would corrupt this - # contact, so fall back to the phone number (nil here) instead of contacts.first. - def caller_profile_name(payload) - contacts = Array(params[:contacts]).map(&:with_indifferent_access) - match = contacts.find { |c| c[:wa_id].to_s == payload[:from].to_s } - match&.dig(:profile, :name).presence + # Backfill every caller alias (the builder only stores the first) so a later event keyed on any one lands on this thread. + def sync_caller_identifiers(call, identity) + Whatsapp::IdentifierSyncService.new(contact_inbox: call.conversation.contact_inbox, contact: call.contact) + .perform(source_ids: identity[:source_ids], phone_number: identity.dig(:contact_attributes, :phone_number)) end # `connect` is the WebRTC tunnel-ready signal, not the pickup signal. Apply diff --git a/spec/enterprise/controllers/twilio/voice_controller_spec.rb b/spec/enterprise/controllers/twilio/voice_controller_spec.rb index 79fdd67a8..0f537af20 100644 --- a/spec/enterprise/controllers/twilio/voice_controller_spec.rb +++ b/spec/enterprise/controllers/twilio/voice_controller_spec.rb @@ -33,8 +33,8 @@ RSpec.describe 'Twilio::VoiceController', type: :request do expect(Voice::InboundCallBuilder).to receive(:perform!).with( inbox: inbox, - from_number: from_number, - call_sid: call_sid + call_sid: call_sid, + caller: { source_ids: [from_number], contact_attributes: { name: from_number, phone_number: from_number } } ).and_return(call) post "/twilio/voice/call/#{digits}", params: { diff --git a/spec/enterprise/services/voice/inbound_call_builder_spec.rb b/spec/enterprise/services/voice/inbound_call_builder_spec.rb index e36c9a1b7..c8c6b978e 100644 --- a/spec/enterprise/services/voice/inbound_call_builder_spec.rb +++ b/spec/enterprise/services/voice/inbound_call_builder_spec.rb @@ -17,8 +17,8 @@ RSpec.describe Voice::InboundCallBuilder do def perform_builder described_class.perform!( inbox: inbox, - from_number: from_number, - call_sid: call_sid + call_sid: call_sid, + caller: { source_ids: [from_number], contact_attributes: { name: from_number, phone_number: from_number } } ) end @@ -100,26 +100,29 @@ RSpec.describe Voice::InboundCallBuilder do end end - context 'when the WhatsApp wa_id needs Brazil normalization to match an existing ContactInbox' do + context 'when a WhatsApp call shares a BSUID with an existing ContactInbox' do let(:whatsapp_channel) do create(:channel_whatsapp, account: account, provider: 'whatsapp_cloud', provider_config: { 'phone_number_id' => '123', 'source' => 'embedded_signup', 'calling_enabled' => true }, validate_provider_config: false, sync_templates: false) end let(:whatsapp_inbox) { whatsapp_channel.inbox } - let!(:stored_contact) { create(:contact, account: account, phone_number: '+5541988887777') } + let!(:stored_contact) { create(:contact, account: account) } let!(:stored_contact_inbox) do - create(:contact_inbox, contact: stored_contact, inbox: whatsapp_inbox, source_id: '5541988887777') + create(:contact_inbox, contact: stored_contact, inbox: whatsapp_inbox, source_id: 'IN.2081978709342942') end before { account.enable_features!('channel_voice') } - it 'reuses the contact via normalized wa_id rather than forking a new ContactInbox' do + # Closes the gap: the contact was keyed by BSUID, but the call also carries a phone. + # Matching across every source_id reuses the contact instead of forking on the phone. + it 'reuses the contact by matching any source_id, not just the first' do call = described_class.perform!( inbox: whatsapp_inbox, - from_number: '+554188887777', - call_sid: 'wacall_br_1', - provider: :whatsapp + call_sid: 'wacall_bsuid_1', + provider: :whatsapp, + caller: { source_ids: ['5541988887777', 'IN.2081978709342942'], + contact_attributes: { name: 'Ada Lovelace', phone_number: '+5541988887777' } } ) expect(call.contact).to eq(stored_contact) diff --git a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb index a3c5246d2..cf031f1ca 100644 --- a/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/incoming_call_service_spec.rb @@ -74,6 +74,120 @@ describe Whatsapp::IncomingCallService do end end + describe 'inbound connect from a username (BSUID) caller' do + let(:sdp_offer) { "v=0\r\n...sdp..." } + let(:bsuid) { 'IN.2081978709342942' } + let!(:agent) { create(:user, account: account) } + + before { create(:inbox_member, inbox: inbox, user: agent) } + + it 'keys a phone caller by the phone (matching messaging) even when a BSUID is also present' do + allow(ActionCable.server).to receive(:broadcast) + + params = { + calls: [{ id: provider_call_id, from: from_number, from_user_id: bsuid, event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ wa_id: from_number, user_id: bsuid, profile: { name: 'Ada Lovelace' } }] + } + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1).and change(Conversation, :count).by(1) + + contact_inbox = Call.last.conversation.contact_inbox + expect(contact_inbox.source_id).to eq(from_number) + expect(contact_inbox.contact.name).to eq('Ada Lovelace') + end + + it 'keys a username-only caller by the BSUID when no phone `from` is present' do + allow(ActionCable.server).to receive(:broadcast) + + params = { + calls: [{ id: provider_call_id, from_user_id: bsuid, event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ user_id: bsuid, profile: { name: 'Ada Lovelace' } }] + } + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1) + + contact_inbox = Call.last.conversation.contact_inbox + expect(contact_inbox.source_id).to eq(bsuid) + expect(contact_inbox.contact.name).to eq('Ada Lovelace') + end + + it 'reuses the phone-keyed ContactInbox messaging created for a phone caller and backfills the BSUID alias' do + allow(ActionCable.server).to receive(:broadcast) + contact = create(:contact, account: account) + existing = create(:contact_inbox, inbox: inbox, contact: contact, source_id: from_number) + + params = { + calls: [{ id: provider_call_id, from: from_number, from_user_id: bsuid, event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ wa_id: from_number, user_id: bsuid }] + } + # The conversation reuses the existing phone thread; the BSUID alias is backfilled onto the same contact. + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1).and change(ContactInbox, :count).by(1) + + expect(Call.last.contact).to eq(contact) + expect(Call.last.conversation.contact_inbox).to eq(existing) + expect(inbox.contact_inboxes.find_by(source_id: bsuid).contact).to eq(contact) + end + + it 'reuses a phone ContactInbox via the same wa_id normalization messaging uses' do + allow(ActionCable.server).to receive(:broadcast) + contact = create(:contact, account: account, phone_number: '+5541988887777') + existing = create(:contact_inbox, inbox: inbox, contact: contact, source_id: '5541988887777') + + params = { + calls: [{ id: provider_call_id, from: '554188887777', event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ wa_id: '554188887777' }] + } + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1).and not_change(ContactInbox, :count) + + expect(Call.last.conversation.contact_inbox).to eq(existing) + end + + it 'reuses the BSUID-keyed ContactInbox messaging created for a username-only caller' do + allow(ActionCable.server).to receive(:broadcast) + contact = create(:contact, account: account) + existing = create(:contact_inbox, inbox: inbox, contact: contact, source_id: bsuid) + + params = { + calls: [{ id: provider_call_id, from_user_id: bsuid, event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ user_id: bsuid }] + } + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1).and not_change(ContactInbox, :count) + + expect(Call.last.contact).to eq(contact) + expect(Call.last.conversation.contact_inbox).to eq(existing) + end + + # The gap: messaging created the contact username-only (BSUID-keyed), and the call now + # also exposes a phone. Matching across every source_id reuses the BSUID thread instead + # of forking a new phone-keyed contact. + it 'reuses a BSUID-keyed ContactInbox even when the call also carries a phone and backfills the phone alias' do + allow(ActionCable.server).to receive(:broadcast) + contact = create(:contact, account: account) + existing = create(:contact_inbox, inbox: inbox, contact: contact, source_id: bsuid) + + params = { + calls: [{ id: provider_call_id, from: from_number, from_user_id: bsuid, event: 'connect', + session: { sdp: sdp_offer, sdp_type: 'offer' } }], + contacts: [{ wa_id: from_number, user_id: bsuid }] + } + # The conversation reuses the existing BSUID thread; the phone alias is backfilled onto the same contact. + expect { described_class.new(inbox: inbox, params: params).perform } + .to change(Call, :count).by(1).and change(ContactInbox, :count).by(1) + + expect(Call.last.contact).to eq(contact) + expect(Call.last.conversation.contact_inbox).to eq(existing) + expect(inbox.contact_inboxes.find_by(source_id: from_number).contact).to eq(contact) + end + end + describe 'outbound connect (existing call)' do let!(:call) do conversation = create(:conversation, account: account, inbox: inbox)