From 26ea87a6cb755caa2dfe1de359aded504aeff99a Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 28 Oct 2025 18:16:29 +0530 Subject: [PATCH] fix: Extend phone number normalization to Twilio WhatsApp (#12655) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Problem WhatsApp Cloud channels already handle Brazil/Argentina phone number format mismatches (PRs #12492, #11173), but Twilio WhatsApp channels were creating duplicate contacts when: - Template sent to new format: `whatsapp:+5541988887777` (13 digits) - User responds from old format: `whatsapp:+554188887777` (12 digits) ### Solution The solution extends the existing phone number normalization infrastructure to support both WhatsApp providers while handling their different payload formats: ### Provider Format Differences - **WhatsApp Cloud**: `wa_id: "919745786257"` (clean number) - **Twilio WhatsApp**: `From: "whatsapp:+919745786257"` (prefixed format) ### Test Coverage #### Brazil Phone Number Tests **Case 1: New Format (13 digits with "9")** - **Test 1**: No existing contact → Creates new contact with original format - **Test 2**: Contact exists in same format → Appends to existing conversation **Case 2: Old Format (12 digits without "9")** - **Test 3**: Contact exists in old format → Appends to existing conversation - **Test 4** *(Critical)*: Contact exists in new format, message in old format → Finds existing contact, prevents duplicate - **Test 5**: No contact exists → Creates new contact with incoming format #### Argentina Phone Number Tests **Case 3: With "9" after country code** - **Test 6**: No existing contact → Creates new contact - **Test 7**: Contact exists in normalized format → Uses existing contact **Case 4: Without "9" after country code** - **Test 8**: Contact exists in same format → Appends to existing - **Test 9**: No contact exists → Creates new contact Fixes https://linear.app/chatwoot/issue/CW-5565/inconsistencies-for-mobile-numbersargentina-brazil-and-mexico-numbers --- .../twilio/incoming_message_service.rb | 10 +- .../incoming_message_service_helpers.rb | 11 +- .../phone_number_normalization_service.rb | 51 +++- spec/factories/channel/twilio_sms.rb | 4 + .../twilio/incoming_message_service_spec.rb | 224 ++++++++++++++++++ 5 files changed, 278 insertions(+), 22 deletions(-) diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb index 7a74488e2..5d695ebb2 100644 --- a/app/services/twilio/incoming_message_service.rb +++ b/app/services/twilio/incoming_message_service.rb @@ -44,6 +44,12 @@ class Twilio::IncomingMessageService twilio_channel.sms? ? params[:From] : params[:From].gsub('whatsapp:', '') end + def normalized_phone_number + return phone_number unless twilio_channel.whatsapp? + + Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact_by_provider("whatsapp:#{phone_number}", :twilio) + end + def formatted_phone_number TelephoneNumber.parse(phone_number).international_number end @@ -53,8 +59,10 @@ class Twilio::IncomingMessageService end def set_contact + source_id = twilio_channel.whatsapp? ? normalized_phone_number : params[:From] + contact_inbox = ::ContactInboxWithContactBuilder.new( - source_id: params[:From], + source_id: source_id, inbox: inbox, contact_attributes: contact_attributes ).perform diff --git a/app/services/whatsapp/incoming_message_service_helpers.rb b/app/services/whatsapp/incoming_message_service_helpers.rb index 705babbba..46ad255aa 100644 --- a/app/services/whatsapp/incoming_message_service_helpers.rb +++ b/app/services/whatsapp/incoming_message_service_helpers.rb @@ -47,17 +47,8 @@ module Whatsapp::IncomingMessageServiceHelpers %w[reaction ephemeral unsupported request_welcome].include?(message_type) end - def argentina_phone_number?(phone_number) - phone_number.match(/^54/) - end - - def normalised_argentina_mobil_number(phone_number) - # Remove 9 before country code - phone_number.sub(/^549/, '54') - end - def processed_waid(waid) - Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact(waid) + Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact_by_provider(waid, :cloud) end def error_webhook_event?(message) diff --git a/app/services/whatsapp/phone_number_normalization_service.rb b/app/services/whatsapp/phone_number_normalization_service.rb index cd10db0d0..1e52d9b02 100644 --- a/app/services/whatsapp/phone_number_normalization_service.rb +++ b/app/services/whatsapp/phone_number_normalization_service.rb @@ -1,23 +1,32 @@ # Service to handle phone number normalization for WhatsApp messages # Currently supports Brazil and Argentina phone number format variations -# Designed to be extensible for additional countries in future PRs -# -# Usage: Whatsapp::PhoneNumberNormalizationService.new(inbox).normalize_and_find_contact(waid) +# Supports both WhatsApp Cloud API and Twilio WhatsApp providers class Whatsapp::PhoneNumberNormalizationService def initialize(inbox) @inbox = inbox end - # Main entry point for phone number normalization - # Returns the source_id of an existing contact if found, otherwise returns original waid - def normalize_and_find_contact(waid) - normalizer = find_normalizer_for_country(waid) - return waid unless normalizer + # @param raw_number [String] The phone number in provider-specific format + # - Cloud: "5541988887777" (clean number) + # - Twilio: "whatsapp:+5541988887777" (prefixed format) + # @param provider [Symbol] :cloud or :twilio + # @return [String] Normalized source_id in provider format or original if not found + def normalize_and_find_contact_by_provider(raw_number, provider) + # Extract clean number based on provider format + clean_number = extract_clean_number(raw_number, provider) - normalized_waid = normalizer.normalize(waid) - existing_contact_inbox = find_existing_contact_inbox(normalized_waid) + # Find appropriate normalizer for the country + normalizer = find_normalizer_for_country(clean_number) + return raw_number unless normalizer - existing_contact_inbox&.source_id || waid + # Normalize the clean number + normalized_clean_number = normalizer.normalize(clean_number) + + # Format for provider and check for existing contact + provider_format = format_for_provider(normalized_clean_number, provider) + existing_contact_inbox = find_existing_contact_inbox(provider_format) + + existing_contact_inbox&.source_id || raw_number end private @@ -33,6 +42,26 @@ class Whatsapp::PhoneNumberNormalizationService inbox.contact_inboxes.find_by(source_id: normalized_waid) end + # Extract clean number from provider-specific format + def extract_clean_number(raw_number, provider) + case provider + when :twilio + raw_number.gsub(/^whatsapp:\+/, '') # Remove prefix: "whatsapp:+5541988887777" → "5541988887777" + else + raw_number # Default fallback for unknown providers + end + end + + # Format normalized number for provider-specific storage + def format_for_provider(clean_number, provider) + case provider + when :twilio + "whatsapp:+#{clean_number}" # Add prefix: "5541988887777" → "whatsapp:+5541988887777" + else + clean_number # Default for :cloud and unknown providers: "5541988887777" + end + end + NORMALIZERS = [ Whatsapp::PhoneNormalizers::BrazilPhoneNormalizer, Whatsapp::PhoneNormalizers::ArgentinaPhoneNormalizer diff --git a/spec/factories/channel/twilio_sms.rb b/spec/factories/channel/twilio_sms.rb index 94f632efd..1963a4f28 100644 --- a/spec/factories/channel/twilio_sms.rb +++ b/spec/factories/channel/twilio_sms.rb @@ -13,5 +13,9 @@ FactoryBot.define do sequence(:phone_number) { |n| "+123456789#{n}1" } messaging_service_sid { nil } end + + trait :whatsapp do + medium { :whatsapp } + end end end diff --git a/spec/services/twilio/incoming_message_service_spec.rb b/spec/services/twilio/incoming_message_service_spec.rb index d32ef59bb..190a6c45a 100644 --- a/spec/services/twilio/incoming_message_service_spec.rb +++ b/spec/services/twilio/incoming_message_service_spec.rb @@ -402,6 +402,230 @@ describe Twilio::IncomingMessageService do existing_contact.reload expect(existing_contact.name).to eq('Alice Johnson') end + + describe 'When the incoming number is a Brazilian number in new format with 9 included' do + let!(:whatsapp_twilio_channel) do + create(:channel_twilio_sms, :whatsapp, account: account, account_sid: 'ACxxx', + inbox: create(:inbox, account: account, greeting_enabled: false)) + end + + it 'creates appropriate conversations, message and contacts if contact does not exist' do + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+5541988887777', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Brazil', + ProfileName: 'João Silva' + } + + described_class.new(params: params).perform + + expect(whatsapp_twilio_channel.inbox.conversations.count).not_to eq(0) + expect(whatsapp_twilio_channel.inbox.contacts.first.name).to eq('João Silva') + expect(whatsapp_twilio_channel.inbox.messages.first.content).to eq('Test message from Brazil') + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+5541988887777') + end + + it 'appends to existing contact if contact inbox exists' do + # Create existing contact with same format + normalized_contact = create(:contact, account: account, phone_number: '+5541988887777') + contact_inbox = create(:contact_inbox, source_id: 'whatsapp:+5541988887777', contact: normalized_contact, + inbox: whatsapp_twilio_channel.inbox) + last_conversation = create(:conversation, inbox: whatsapp_twilio_channel.inbox, contact_inbox: contact_inbox) + + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+5541988887777', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Another message from Brazil', + ProfileName: 'João Silva' + } + + described_class.new(params: params).perform + + # No new conversation should be created + expect(whatsapp_twilio_channel.inbox.conversations.count).to eq(1) + # Message appended to the last conversation + expect(last_conversation.messages.last.content).to eq('Another message from Brazil') + end + end + + describe 'When incoming number is a Brazilian number in old format without the 9 included' do + let!(:whatsapp_twilio_channel) do + create(:channel_twilio_sms, :whatsapp, account: account, account_sid: 'ACxxx', + inbox: create(:inbox, account: account, greeting_enabled: false)) + end + + it 'appends to existing contact when contact inbox exists in old format' do + # Create existing contact with old format (12 digits) + old_contact = create(:contact, account: account, phone_number: '+554188887777') + contact_inbox = create(:contact_inbox, source_id: 'whatsapp:+554188887777', contact: old_contact, inbox: whatsapp_twilio_channel.inbox) + last_conversation = create(:conversation, inbox: whatsapp_twilio_channel.inbox, contact_inbox: contact_inbox) + + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+554188887777', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Brazil old format', + ProfileName: 'Maria Silva' + } + + described_class.new(params: params).perform + + # No new conversation should be created + expect(whatsapp_twilio_channel.inbox.conversations.count).to eq(1) + # Message appended to the last conversation + expect(last_conversation.messages.last.content).to eq('Test message from Brazil old format') + end + + it 'appends to existing contact when contact inbox exists in new format' do + # Create existing contact with new format (13 digits) + normalized_contact = create(:contact, account: account, phone_number: '+5541988887777') + contact_inbox = create(:contact_inbox, source_id: 'whatsapp:+5541988887777', contact: normalized_contact, + inbox: whatsapp_twilio_channel.inbox) + last_conversation = create(:conversation, inbox: whatsapp_twilio_channel.inbox, contact_inbox: contact_inbox) + + # Incoming message with old format (12 digits) + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+554188887777', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Brazil', + ProfileName: 'João Silva' + } + + described_class.new(params: params).perform + + # Should find and use existing contact, not create duplicate + expect(whatsapp_twilio_channel.inbox.conversations.count).to eq(1) + # Message appended to the existing conversation + expect(last_conversation.messages.last.content).to eq('Test message from Brazil') + # Should use the existing contact's source_id (normalized format) + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+5541988887777') + end + + it 'creates contact inbox with incoming number when no existing contact' do + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+554188887777', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Brazil', + ProfileName: 'Carlos Silva' + } + + described_class.new(params: params).perform + + expect(whatsapp_twilio_channel.inbox.conversations.count).not_to eq(0) + expect(whatsapp_twilio_channel.inbox.contacts.first.name).to eq('Carlos Silva') + expect(whatsapp_twilio_channel.inbox.messages.first.content).to eq('Test message from Brazil') + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+554188887777') + end + end + + describe 'When the incoming number is an Argentine number with 9 after country code' do + let!(:whatsapp_twilio_channel) do + create(:channel_twilio_sms, :whatsapp, account: account, account_sid: 'ACxxx', + inbox: create(:inbox, account: account, greeting_enabled: false)) + end + + it 'creates appropriate conversations, message and contacts if contact does not exist' do + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+5491123456789', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Argentina', + ProfileName: 'Carlos Mendoza' + } + + described_class.new(params: params).perform + + expect(whatsapp_twilio_channel.inbox.conversations.count).not_to eq(0) + expect(whatsapp_twilio_channel.inbox.contacts.first.name).to eq('Carlos Mendoza') + expect(whatsapp_twilio_channel.inbox.messages.first.content).to eq('Test message from Argentina') + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+5491123456789') + end + + it 'appends to existing contact if contact inbox exists with normalized format' do + # Create existing contact with normalized format (without 9 after country code) + normalized_contact = create(:contact, account: account, phone_number: '+541123456789') + contact_inbox = create(:contact_inbox, source_id: 'whatsapp:+541123456789', contact: normalized_contact, + inbox: whatsapp_twilio_channel.inbox) + last_conversation = create(:conversation, inbox: whatsapp_twilio_channel.inbox, contact_inbox: contact_inbox) + + # Incoming message with 9 after country code + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+5491123456789', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Argentina', + ProfileName: 'Carlos Mendoza' + } + + described_class.new(params: params).perform + + # Should find and use existing contact, not create duplicate + expect(whatsapp_twilio_channel.inbox.conversations.count).to eq(1) + # Message appended to the existing conversation + expect(last_conversation.messages.last.content).to eq('Test message from Argentina') + # Should use the normalized source_id from existing contact + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+541123456789') + end + end + + describe 'When incoming number is an Argentine number without 9 after country code' do + let!(:whatsapp_twilio_channel) do + create(:channel_twilio_sms, :whatsapp, account: account, account_sid: 'ACxxx', + inbox: create(:inbox, account: account, greeting_enabled: false)) + end + + it 'appends to existing contact when contact inbox exists with same format' do + # Create existing contact with same format (without 9) + contact = create(:contact, account: account, phone_number: '+541123456789') + contact_inbox = create(:contact_inbox, source_id: 'whatsapp:+541123456789', contact: contact, inbox: whatsapp_twilio_channel.inbox) + last_conversation = create(:conversation, inbox: whatsapp_twilio_channel.inbox, contact_inbox: contact_inbox) + + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+541123456789', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Argentina', + ProfileName: 'Ana García' + } + + described_class.new(params: params).perform + + # No new conversation should be created + expect(whatsapp_twilio_channel.inbox.conversations.count).to eq(1) + # Message appended to the last conversation + expect(last_conversation.messages.last.content).to eq('Test message from Argentina') + end + + it 'creates contact inbox with incoming number when no existing contact' do + params = { + SmsSid: 'SMxx', + From: 'whatsapp:+541123456789', + AccountSid: 'ACxxx', + MessagingServiceSid: whatsapp_twilio_channel.messaging_service_sid, + Body: 'Test message from Argentina', + ProfileName: 'Diego López' + } + + described_class.new(params: params).perform + + expect(whatsapp_twilio_channel.inbox.conversations.count).not_to eq(0) + expect(whatsapp_twilio_channel.inbox.contacts.first.name).to eq('Diego López') + expect(whatsapp_twilio_channel.inbox.messages.first.content).to eq('Test message from Argentina') + expect(whatsapp_twilio_channel.inbox.contact_inboxes.first.source_id).to eq('whatsapp:+541123456789') + end + end end end end