feat(whatsapp-call): support BSUID callers for inbound voice calls (#14743)
## Linear Ticket - https://linear.app/chatwoot/issue/CW-7276/bsuid-support-to-whatsapp-voice-calling ## Description Keeps WhatsApp voice calls in the same thread as the chat when a caller has adopted a **WhatsApp username** and hidden their phone number. This makes the inbound-call path BSUID-aware, reusing the same identifier the messaging pipeline keys on so calls land on the existing `ContactInbox`/conversation. ## Type of change - [ ] New feature (non-breaking change which adds functionality) ## How Has This Been Tested? - Locally via UI ## 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 --------- Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
Tanmay Deep Sharma
co-authored by
Muhsin Keloth
Muhsin Keloth
parent
67cab7171d
commit
0e376f4fe2
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user