diff --git a/app/services/whatsapp/contact_inbox_consolidation_service.rb b/app/services/whatsapp/contact_inbox_consolidation_service.rb index 188430be6..ace73bc93 100644 --- a/app/services/whatsapp/contact_inbox_consolidation_service.rb +++ b/app/services/whatsapp/contact_inbox_consolidation_service.rb @@ -21,9 +21,13 @@ class Whatsapp::ContactInboxConsolidationService phone_contact_inbox = find_phone_contact_inbox lid_contact_inbox = find_lid_contact_inbox - if phone_contact_inbox && lid_contact_inbox && should_consolidate?(phone_contact_inbox, lid_contact_inbox) - consolidate_contact_inboxes(phone_contact_inbox, lid_contact_inbox) - elsif phone_contact_inbox && lid_contact_inbox.nil? + if phone_contact_inbox && lid_contact_inbox + if should_consolidate?(phone_contact_inbox, lid_contact_inbox) + consolidate_contact_inboxes(phone_contact_inbox, lid_contact_inbox) + else + consolidate_different_contacts(phone_contact_inbox, lid_contact_inbox) + end + elsif phone_contact_inbox migrate_phone_to_lid(phone_contact_inbox) elsif phone_contact_inbox.nil? # No phone-based contact_inbox exists, try to find contact by phone and update their contact_inbox @@ -55,6 +59,36 @@ class Whatsapp::ContactInboxConsolidationService end end + # Handles the case where phone and LID contact_inboxes belong to different contacts. + # The phone contact is treated as canonical (has real user data like name and phone_number). + # Merges by moving LID conversations to the phone contact and consolidating into a single contact_inbox. + def consolidate_different_contacts(phone_contact_inbox, lid_contact_inbox) + phone_contact = phone_contact_inbox.contact + lid_contact = lid_contact_inbox.contact + + ActiveRecord::Base.transaction do + # Clear identifier and phone from LID contact to avoid uniqueness conflicts when updating the canonical contact + lid_cleanup = {} + lid_cleanup[:identifier] = nil if lid_contact.identifier == @identifier + lid_cleanup[:phone_number] = nil if lid_contact.phone_number == "+#{@phone}" + lid_contact.update!(lid_cleanup) if lid_cleanup.present? + + # Move conversations from LID contact_inbox to the phone contact + lid_contact_inbox.conversations.find_each do |conversation| + conversation.update!(contact_inbox_id: phone_contact_inbox.id, contact_id: phone_contact.id) + end + + lid_contact_inbox.destroy! + + # Clean up orphaned LID contact if it has no remaining contact_inboxes + lid_contact.destroy! if lid_contact.contact_inboxes.reload.empty? + + # Update phone contact_inbox to use LID as source_id and set identifier on the canonical contact + phone_contact_inbox.update!(source_id: @lid) + phone_contact.update!(identifier: @identifier, phone_number: "+#{@phone}") + end + end + def migrate_phone_to_lid(phone_contact_inbox) existing_contact = phone_contact_inbox.contact @@ -75,8 +109,14 @@ class Whatsapp::ContactInboxConsolidationService existing_contact_inbox = existing_contact.contact_inboxes.find_by(inbox_id: @inbox.id) return unless existing_contact_inbox - # Don't update if we'd create a duplicate contact_inbox or identifier conflict - return if find_lid_contact_inbox + + # If a LID contact_inbox already exists, route into the merge logic instead of early-returning + lid_contact_inbox = find_lid_contact_inbox + if lid_contact_inbox + return if lid_contact_inbox.contact_id == existing_contact.id + + return consolidate_different_contacts(existing_contact_inbox, lid_contact_inbox) + end return if identifier_conflict?(existing_contact) ActiveRecord::Base.transaction do diff --git a/spec/services/whatsapp/baileys_handlers/messages_upsert_spec.rb b/spec/services/whatsapp/baileys_handlers/messages_upsert_spec.rb index 576ac17cb..a457ae543 100644 --- a/spec/services/whatsapp/baileys_handlers/messages_upsert_spec.rb +++ b/spec/services/whatsapp/baileys_handlers/messages_upsert_spec.rb @@ -55,7 +55,7 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do end context 'when identifier is already taken by a different contact (race condition)' do - it 'does not raise validation error and skips the update' do + it 'consolidates contacts and saves message on the canonical contact' do original_contact = create(:contact, account: inbox.account, phone_number: nil, identifier: nil, name: 'Original Contact') original_contact_inbox = create(:contact_inbox, inbox: inbox, contact: original_contact, source_id: phone) @@ -79,13 +79,15 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do Whatsapp::IncomingMessageBaileysService.new(inbox: inbox, params: params).perform end.not_to raise_error - expect(original_contact_inbox.reload.source_id).to eq(phone) - expect(original_contact.reload.identifier).to be_nil + # Consolidation merges into original_contact (phone contact_inbox's contact) + expect(original_contact_inbox.reload.source_id).to eq(source_id) + expect(original_contact.reload.identifier).to eq(identifier) + expect(original_contact.phone_number).to eq("+#{phone}") message = inbox.messages.last expect(message).to be_present - expect(message.sender).to eq(conflicting_contact) - expect(message.conversation.contact).to eq(conflicting_contact) + expect(message.sender).to eq(original_contact) + expect(message.conversation.contact).to eq(original_contact) end end @@ -121,9 +123,9 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do end context 'when LID contact_inbox already exists (race condition)' do - it 'does not raise unique constraint error and skips the update' do + it 'consolidates contacts and saves message on the canonical contact' do original_contact = create(:contact, account: inbox.account, phone_number: nil, identifier: nil) - create(:contact_inbox, inbox: inbox, contact: original_contact, source_id: phone) + original_contact_inbox = create(:contact_inbox, inbox: inbox, contact: original_contact, source_id: phone) lid_contact = create(:contact, account: inbox.account, phone_number: "+#{phone}", identifier: identifier) create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: source_id) @@ -144,9 +146,14 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do Whatsapp::IncomingMessageBaileysService.new(inbox: inbox, params: params).perform end.not_to raise_error + # Consolidation merges into original_contact (phone contact_inbox's contact) + expect(original_contact_inbox.reload.source_id).to eq(source_id) + expect(original_contact.reload.identifier).to eq(identifier) + expect(original_contact.phone_number).to eq("+#{phone}") + message = inbox.messages.last expect(message).to be_present - expect(message.sender).to eq(lid_contact) + expect(message.sender).to eq(original_contact) end end @@ -179,6 +186,47 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do end end + describe 'fragmented contacts: phone vs LID with different contact_ids' do + let(:phone) { '19543717011' } + let(:lid) { '116883390996710' } + let(:identifier) { "#{lid}@lid" } + + context 'when phone-based and LID-based contact_inboxes belong to different contacts' do + it 'still saves the incoming message without raising' do + # Setup: two separate contacts for the same WhatsApp user (fragmented identity) + phone_contact = create(:contact, account: inbox.account, phone_number: "+#{phone}", identifier: nil, name: 'Brigita Pinto') + create(:contact_inbox, inbox: inbox, contact: phone_contact, source_id: phone) + + lid_contact = create(:contact, account: inbox.account, phone_number: nil, identifier: identifier, name: lid) + create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: lid) + + raw_message = { + key: { id: 'msg_fragmented_001', remoteJid: "#{lid}@lid", remoteJidAlt: "#{phone}@s.whatsapp.net", fromMe: false, + addressingMode: 'lid' }, + pushName: 'Brigita Pinto', + messageTimestamp: timestamp, + message: { conversation: 'Queria agendar a reunião' } + } + params = { + webhookVerifyToken: webhook_verify_token, + event: 'messages.upsert', + data: { type: 'notify', messages: [raw_message] } + } + + # This scenario previously caused a uniqueness violation when update_contact_info + # tried to set phone_number on the LID contact. The consolidation service now handles this. + expect do + Whatsapp::IncomingMessageBaileysService.new(inbox: inbox, params: params).perform + end.not_to raise_error + + message = inbox.messages.find_by(source_id: 'msg_fragmented_001') + expect(message).to be_present + expect(message.content).to eq('Queria agendar a reunião') + expect(message.message_type).to eq('incoming') + end + end + end + describe 'ephemeral message handling' do let(:phone) { '5511912345678' } let(:lid) { '12345678' } diff --git a/spec/services/whatsapp/contact_inbox_consolidation_service_spec.rb b/spec/services/whatsapp/contact_inbox_consolidation_service_spec.rb index 78cbf2681..b7a5dbb12 100644 --- a/spec/services/whatsapp/contact_inbox_consolidation_service_spec.rb +++ b/spec/services/whatsapp/contact_inbox_consolidation_service_spec.rb @@ -115,18 +115,51 @@ describe Whatsapp::ContactInboxConsolidationService do end context 'when phone and lid contact_inboxes belong to different contacts' do - let!(:contact1) { create(:contact, account: inbox.account, phone_number: "+#{phone}") } - let!(:contact2) { create(:contact, account: inbox.account, identifier: identifier) } - let!(:phone_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: contact1, source_id: phone) } - let!(:lid_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: contact2, source_id: lid) } + let!(:phone_contact) { create(:contact, account: inbox.account, phone_number: "+#{phone}", name: 'Brigita Pinto') } + let!(:lid_contact) { create(:contact, account: inbox.account, identifier: identifier, name: lid) } + let!(:phone_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: phone_contact, source_id: phone) } + let!(:lid_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: lid) } + + it 'merges into the phone contact and consolidates contact_inboxes' do + lid_conversation = create(:conversation, inbox: inbox, contact: lid_contact, contact_inbox: lid_contact_inbox) - it 'does not consolidate different contacts' do service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier) service.perform - expect(ContactInbox.exists?(phone_contact_inbox.id)).to be(true) - expect(ContactInbox.exists?(lid_contact_inbox.id)).to be(true) - expect(phone_contact_inbox.reload.source_id).to eq(phone) + # LID contact_inbox is destroyed, phone contact_inbox gets LID source_id + expect(ContactInbox.exists?(lid_contact_inbox.id)).to be(false) + expect(phone_contact_inbox.reload.source_id).to eq(lid) + + # Phone contact becomes the canonical contact with LID identifier + expect(phone_contact.reload.identifier).to eq(identifier) + + # Orphaned LID contact is destroyed + expect(Contact.exists?(lid_contact.id)).to be(false) + + # Conversation is moved to the phone contact + expect(lid_conversation.reload.contact_id).to eq(phone_contact.id) + expect(lid_conversation.contact_inbox_id).to eq(phone_contact_inbox.id) + end + + it 'handles multiple conversations across both contact_inboxes' do + phone_conversation = create(:conversation, inbox: inbox, contact: phone_contact, contact_inbox: phone_contact_inbox) + lid_conversation1 = create(:conversation, inbox: inbox, contact: lid_contact, contact_inbox: lid_contact_inbox) + lid_conversation2 = create(:conversation, inbox: inbox, contact: lid_contact, contact_inbox: lid_contact_inbox) + + service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier) + service.perform + + # Phone conversation stays on phone contact + expect(phone_conversation.reload.contact_id).to eq(phone_contact.id) + expect(phone_conversation.contact_inbox_id).to eq(phone_contact_inbox.id) + + # LID conversations are moved to phone contact + expect(lid_conversation1.reload.contact_id).to eq(phone_contact.id) + expect(lid_conversation1.contact_inbox_id).to eq(phone_contact_inbox.id) + expect(lid_conversation2.reload.contact_id).to eq(phone_contact.id) + expect(lid_conversation2.contact_inbox_id).to eq(phone_contact_inbox.id) + + expect(inbox.contact_inboxes.where(contact: phone_contact).count).to eq(1) end end @@ -143,7 +176,7 @@ describe Whatsapp::ContactInboxConsolidationService do expect(contact.reload.identifier).to eq(identifier) end - context 'when a lid contact_inbox already exists' do + context 'when a lid contact_inbox already exists for the same contact' do let!(:lid_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: contact, source_id: lid) } # rubocop:disable RSpec/LetSetup it 'does not update to avoid duplicate' do @@ -154,6 +187,32 @@ describe Whatsapp::ContactInboxConsolidationService do end end + context 'when a lid contact_inbox exists for a different contact' do + let!(:lid_contact) { create(:contact, account: inbox.account, identifier: identifier, name: lid) } + let!(:lid_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: lid) } + + it 'consolidates by merging into the phone contact' do + lid_conversation = create(:conversation, inbox: inbox, contact: lid_contact, contact_inbox: lid_contact_inbox) + + service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier) + service.perform + + # LID contact_inbox destroyed, old contact_inbox updated to LID source_id + expect(ContactInbox.exists?(lid_contact_inbox.id)).to be(false) + expect(old_contact_inbox.reload.source_id).to eq(lid) + + # Phone contact becomes canonical + expect(contact.reload.identifier).to eq(identifier) + + # Orphaned LID contact is destroyed + expect(Contact.exists?(lid_contact.id)).to be(false) + + # Conversation moved to phone contact + expect(lid_conversation.reload.contact_id).to eq(contact.id) + expect(lid_conversation.contact_inbox_id).to eq(old_contact_inbox.id) + end + end + context 'when another contact already has the same identifier' do let!(:conflicting_contact) { create(:contact, account: inbox.account, identifier: identifier) } # rubocop:disable RSpec/LetSetup