diff --git a/.rubocop.yml b/.rubocop.yml index e30a71ee9..ea688792b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,7 +23,7 @@ Metrics/MethodLength: - 'enterprise/lib/captain/agent.rb' RSpec/ExampleLength: - Max: 25 + Max: 50 Style/Documentation: Enabled: false @@ -336,4 +336,4 @@ FactoryBot/RedundantFactoryOption: Enabled: false FactoryBot/FactoryAssociationWithStrategy: - Enabled: false \ No newline at end of file + Enabled: false 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/db/migrate/20251022152158_add_index_to_conversations_identifier.rb b/db/migrate/20251022152158_add_index_to_conversations_identifier.rb new file mode 100644 index 000000000..e7d02b53d --- /dev/null +++ b/db/migrate/20251022152158_add_index_to_conversations_identifier.rb @@ -0,0 +1,6 @@ +class AddIndexToConversationsIdentifier < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + def change + add_index :conversations, [:identifier, :account_id], name: 'index_conversations_on_identifier_and_account_id', algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 922c8f444..a04096c98 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -676,6 +676,7 @@ ActiveRecord::Schema[7.1].define(version: 2025_10_25_033349) do t.index ["contact_id"], name: "index_conversations_on_contact_id" t.index ["contact_inbox_id"], name: "index_conversations_on_contact_inbox_id" t.index ["first_reply_created_at"], name: "index_conversations_on_first_reply_created_at" + t.index ["identifier", "account_id"], name: "index_conversations_on_identifier_and_account_id" t.index ["inbox_id"], name: "index_conversations_on_inbox_id" t.index ["priority"], name: "index_conversations_on_priority" t.index ["status", "account_id"], name: "index_conversations_on_status_and_account_id" diff --git a/spec/controllers/api/v2/accounts/reports_controller_spec.rb b/spec/controllers/api/v2/accounts/reports_controller_spec.rb index 4dbbcf406..2d505822f 100644 --- a/spec/controllers/api/v2/accounts/reports_controller_spec.rb +++ b/spec/controllers/api/v2/accounts/reports_controller_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Api::V2::Accounts::ReportsController, type: :request do context 'when authenticated and authorized' do before do # Create conversations across 24 hours at different times - base_time = Time.utc(2024, 1, 15, 0, 0) # Start at midnight UTC + base_time = Time.utc(2024, 1, 14, 23, 0) # Start at 23:00 to span 2 days # Create conversations every 4 hours across 24 hours 6.times do |i| @@ -23,36 +23,38 @@ RSpec.describe Api::V2::Accounts::ReportsController, type: :request do end it 'timezone_offset affects data grouping and timestamps correctly' do - Time.use_zone('UTC') do - base_time = Time.utc(2024, 1, 15, 0, 0) - base_params = { - metric: 'conversations_count', - type: 'account', - since: (base_time - 1.day).to_i.to_s, - until: (base_time + 2.days).to_i.to_s, - group_by: 'day' - } + travel_to Time.utc(2024, 1, 15, 12, 0) do + Time.use_zone('UTC') do + base_time = Time.utc(2024, 1, 14, 23, 0) # Start at 23:00 to span 2 days + base_params = { + metric: 'conversations_count', + type: 'account', + since: (base_time - 1.day).to_i.to_s, + until: (base_time + 2.days).to_i.to_s, + group_by: 'day' + } - responses = [0, -8, 9].map do |offset| - get "/api/v2/accounts/#{account.id}/reports", - params: base_params.merge(timezone_offset: offset), - headers: admin.create_new_auth_token, as: :json - response.parsed_body + responses = [0, -8, 9].map do |offset| + get "/api/v2/accounts/#{account.id}/reports", + params: base_params.merge(timezone_offset: offset), + headers: admin.create_new_auth_token, as: :json + response.parsed_body + end + + data_entries = responses.map { |r| r.select { |e| e['value'] > 0 } } + totals = responses.map { |r| r.sum { |e| e['value'] } } + timestamps = responses.map { |r| r.map { |e| e['timestamp'] } } + + # Data conservation and redistribution + expect(totals.uniq).to eq([6]) + expect(data_entries[0].map { |e| e['value'] }).to eq([1, 5]) + expect(data_entries[1].map { |e| e['value'] }).to eq([3, 3]) + expect(data_entries[2].map { |e| e['value'] }).to eq([4, 2]) + + # Timestamp differences + expect(timestamps.uniq.size).to eq(3) + timestamps[0].zip(timestamps[1]).each { |utc, pst| expect(utc - pst).to eq(-28_800) } end - - data_entries = responses.map { |r| r.select { |e| e['value'] > 0 } } - totals = responses.map { |r| r.sum { |e| e['value'] } } - timestamps = responses.map { |r| r.map { |e| e['timestamp'] } } - - # Data conservation and redistribution - expect(totals.uniq).to eq([6]) - expect(data_entries[0].map { |e| e['value'] }).to eq([1, 5]) - expect(data_entries[1].map { |e| e['value'] }).to eq([3, 3]) - expect(data_entries[2].map { |e| e['value'] }).to eq([4, 2]) - - # Timestamp differences - expect(timestamps.uniq.size).to eq(3) - timestamps[0].zip(timestamps[1]).each { |utc, pst| expect(utc - pst).to eq(-28_800) } end end 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