From 521ce90e7923bdea65c59ae29ea18a9aad7efb73 Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Thu, 19 Mar 2026 19:20:11 -0300 Subject: [PATCH] fix(whatsapp): consolidate fragmented phone/LID contacts to prevent lost messages (#241) * fix(whatsapp): consolidate fragmented phone/LID contacts to prevent lost messages When a WhatsApp user had two separate contacts (one by phone, one by LID) with different contact_ids, the consolidation service did nothing. This caused update_contact_info to crash with a phone_number uniqueness violation, silently dropping incoming messages. Now properly merges the two contacts by treating the phone contact as canonical, moving conversations from the LID contact, and cleaning up duplicates. Co-Authored-By: Claude Opus 4.6 (1M context) * fix(whatsapp): address CodeRabbit review feedback - Destroy orphaned LID contact when it has no remaining contact_inboxes - Clarify spec comment to past-tense (no longer an active bug) Co-Authored-By: Claude Opus 4.6 (1M context) * fix(whatsapp): route legacy-source contact_inbox into merge path When a phone contact_inbox has a non-standard source_id (legacy format) and a separate LID contact_inbox exists for a different contact, the consolidation now merges them instead of early-returning. Co-Authored-By: Claude Opus 4.6 (1M context) * test: assert orphaned LID contact is destroyed in legacy-source spec Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- .../contact_inbox_consolidation_service.rb | 50 ++++++++++-- .../baileys_handlers/messages_upsert_spec.rb | 64 +++++++++++++-- ...ontact_inbox_consolidation_service_spec.rb | 77 ++++++++++++++++--- 3 files changed, 169 insertions(+), 22 deletions(-) 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