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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * test: assert orphaned LID contact is destroyed in legacy-source spec Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-19 19:20:11 -03:00 · 2026-03-19 19:20:11 -03:00 · 521ce90e79
commit 521ce90e79
parent 05d47931be
3 changed files with 169 additions and 22 deletions
--- 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
--- 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' }
--- 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