fix(whatsapp): resolve phone_number conflict when converting inbox between providers (#252)

* fix(whatsapp): resolve phone_number conflict when converting inbox between providers

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: move transfer_identifier_to inside transaction for atomic consolidation

* fix: reassign message sender before destroying merged contact

* fix: resolve identifier conflicts account-wide in adopt and consolidate paths

* fix: scope sender reassignment to moved conversations only

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

app/services/whatsapp/contact_inbox_consolidation_service.rb

@@ -67,11 +67,8 @@ class Whatsapp::ContactInboxConsolidationService
     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
+      moved_conversation_ids = lid_contact_inbox.conversations.pluck(:id)
-      lid_cleanup = {}
+      lid_contact.update!(phone_number: nil) if lid_contact.phone_number == "+#{@phone}"
-      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|
@@ -79,11 +76,10 @@ class Whatsapp::ContactInboxConsolidationService
       end
 
       lid_contact_inbox.destroy!
+      reassign_sender_and_destroy(lid_contact, phone_contact, conversation_ids: moved_conversation_ids)
 
-      # Clean up orphaned LID contact if it has no remaining contact_inboxes
+      # Resolve identifier conflicts account-wide, then update the canonical contact
-      lid_contact.destroy! if lid_contact.contact_inboxes.reload.empty?
+      transfer_identifier_to(phone_contact)
-
-      # 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
@@ -92,10 +88,10 @@ class Whatsapp::ContactInboxConsolidationService
   def migrate_phone_to_lid(phone_contact_inbox)
     existing_contact = phone_contact_inbox.contact
 
-    return if identifier_conflict?(existing_contact)
     return if phone_conflict?(existing_contact)
 
     ActiveRecord::Base.transaction do
+      transfer_identifier_to(existing_contact)
       phone_contact_inbox.update!(source_id: @lid)
       existing_contact.update!(identifier: @identifier, phone_number: "+#{@phone}")
     end
@@ -108,7 +104,14 @@ class Whatsapp::ContactInboxConsolidationService
     return unless existing_contact
 
     existing_contact_inbox = existing_contact.contact_inboxes.find_by(inbox_id: @inbox.id)
-    return unless existing_contact_inbox
+
+    unless existing_contact_inbox
+      # Contact exists by phone but has no contact_inbox in this inbox (e.g., after provider conversion).
+      # Must still resolve identifier conflicts so the builder finds the phone-based contact
+      # instead of a stale LID-only contact, which would cause "Phone number has already been taken".
+      adopt_or_resolve_lid_contact(existing_contact)
+      return
+    end
 
     # If a LID contact_inbox already exists, route into the merge logic instead of early-returning
     lid_contact_inbox = find_lid_contact_inbox
@@ -117,14 +120,59 @@ class Whatsapp::ContactInboxConsolidationService
 
       return consolidate_different_contacts(existing_contact_inbox, lid_contact_inbox)
     end
-    return if identifier_conflict?(existing_contact)
-
     ActiveRecord::Base.transaction do
+      transfer_identifier_to(existing_contact)
       existing_contact.update!(identifier: @identifier)
       existing_contact_inbox.update!(source_id: @lid)
     end
   end
 
+  # When the phone-based contact has no contact_inbox in this inbox, handle
+  # any conflicting LID contact that would otherwise intercept the builder lookup.
+  def adopt_or_resolve_lid_contact(phone_contact)
+    lid_contact_inbox = find_lid_contact_inbox
+
+    if lid_contact_inbox && lid_contact_inbox.contact_id != phone_contact.id
+      adopt_lid_contact_inbox(phone_contact, lid_contact_inbox)
+    else
+      transfer_identifier_to(phone_contact)
+    end
+  end
+
+  # Transfer a LID contact_inbox (and its conversations) from the LID contact to the phone contact.
+  def adopt_lid_contact_inbox(phone_contact, lid_ci)
+    lid_contact = lid_ci.contact
+
+    ActiveRecord::Base.transaction do
+      moved_conversation_ids = lid_ci.conversations.pluck(:id)
+      lid_contact.update!(phone_number: nil) if lid_contact.phone_number == "+#{@phone}"
+
+      lid_ci.conversations.find_each do |conversation|
+        conversation.update!(contact_id: phone_contact.id)
+      end
+      lid_ci.update!(contact_id: phone_contact.id)
+
+      # Resolve identifier conflicts account-wide, not just with lid_contact
+      transfer_identifier_to(phone_contact)
+      phone_contact.update!(identifier: @identifier)
+
+      reassign_sender_and_destroy(lid_contact, phone_contact, conversation_ids: moved_conversation_ids)
+    end
+  end
+
+  # Resolve identifier conflict by transferring the identifier to the phone-based contact.
+  def transfer_identifier_to(target_contact)
+    return if target_contact.identifier == @identifier
+
+    conflicting = @inbox.account.contacts.find_by(identifier: @identifier)
+    return unless conflicting && conflicting.id != target_contact.id
+
+    ActiveRecord::Base.transaction do
+      conflicting.update!(identifier: nil)
+      target_contact.update!(identifier: @identifier)
+    end
+  end
+
   def identifier_conflict?(existing_contact)
     conflicting = @inbox.account.contacts.find_by(identifier: @identifier)
     conflicting.present? && conflicting.id != existing_contact.id
@@ -134,4 +182,13 @@ class Whatsapp::ContactInboxConsolidationService
     conflicting = @inbox.account.contacts.find_by(phone_number: "+#{@phone}")
     conflicting.present? && conflicting.id != existing_contact.id
   end
+
+  # Reassign message sender references for moved conversations, then destroy source contact if orphaned.
+  # Scoped to conversation_ids to avoid touching messages in other inboxes the source contact may still own.
+  # Prevents dependent: :destroy_async on Contact#messages from deleting message history.
+  def reassign_sender_and_destroy(source_contact, target_contact, conversation_ids:)
+    Message.where(sender: source_contact, conversation_id: conversation_ids)
+           .update_all(sender_id: target_contact.id) # rubocop:disable Rails/SkipsModelValidations
+    source_contact.destroy! if source_contact.contact_inboxes.reload.empty?
+  end
 end

spec/services/whatsapp/baileys_handlers/messages_upsert_spec.rb

@@ -157,6 +157,39 @@ describe Whatsapp::BaileysHandlers::MessagesUpsert do
         end
       end
 
+      context 'when phone contact has contact_inbox with phone source_id and a separate LID contact exists (provider conversion)' do
+        it 'resolves the identifier conflict and uses the phone contact' do
+          phone_contact = create(:contact, account: inbox.account, phone_number: "+#{phone}", identifier: nil, name: 'Phone Contact')
+          phone_ci = create(:contact_inbox, inbox: inbox, contact: phone_contact, source_id: phone)
+          create(:contact, account: inbox.account, identifier: identifier, phone_number: nil, name: 'LID Contact')
+
+          raw_message = {
+            key: { id: 'msg_conv_123', remoteJid: "#{lid}@lid", remoteJidAlt: "#{phone}@s.whatsapp.net", fromMe: false,
+                   addressingMode: 'lid' },
+            pushName: 'Phone Contact',
+            messageTimestamp: timestamp,
+            message: { conversation: 'Hello after conversion' }
+          }
+          params = {
+            webhookVerifyToken: webhook_verify_token,
+            event: 'messages.upsert',
+            data: { type: 'notify', messages: [raw_message] }
+          }
+
+          expect do
+            Whatsapp::IncomingMessageBaileysService.new(inbox: inbox, params: params).perform
+          end.not_to raise_error
+
+          expect(phone_ci.reload.source_id).to eq(source_id)
+          expect(phone_contact.reload.identifier).to eq(identifier)
+          expect(phone_contact.phone_number).to eq("+#{phone}")
+
+          message = inbox.messages.find_by(source_id: 'msg_conv_123')
+          expect(message).to be_present
+          expect(message.sender).to eq(phone_contact)
+        end
+      end
+
       context 'when updating the same contact (no conflict)' do
         it 'successfully updates the contact' do
           contact = create(:contact, account: inbox.account, phone_number: "+#{phone}", identifier: nil)

spec/services/whatsapp/contact_inbox_consolidation_service_spec.rb

@@ -38,13 +38,57 @@ describe Whatsapp::ContactInboxConsolidationService do
     end
 
     context 'when no phone-based contact_inbox exists' do
-      it 'does nothing' do
+      it 'does nothing when no contacts exist' do
         service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
 
         expect { service.perform }.not_to change(ContactInbox, :count)
       end
     end
 
+    context 'when contact exists by phone but has no contact_inbox in this inbox' do
+      let!(:phone_contact) { create(:contact, account: inbox.account, phone_number: "+#{phone}") }
+
+      context 'when another contact has the target identifier (provider conversion scenario)' do
+        let!(:lid_contact) { create(:contact, account: inbox.account, identifier: identifier) }
+
+        it 'transfers identifier from the conflicting contact to the phone contact' do
+          service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
+          service.perform
+
+          expect(phone_contact.reload.identifier).to eq(identifier)
+          expect(lid_contact.reload.identifier).to be_nil
+        end
+      end
+
+      context 'when another contact has the identifier AND a lid contact_inbox in this inbox' do
+        let!(:lid_contact) { create(:contact, account: inbox.account, identifier: identifier) }
+        let!(:lid_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: lid) }
+
+        it 'adopts the lid contact_inbox, reassigns messages, and transfers it to the phone contact' do
+          lid_conversation = create(:conversation, inbox: inbox, contact: lid_contact, contact_inbox: lid_contact_inbox)
+          lid_message = create(:message, conversation: lid_conversation, sender: lid_contact, inbox: inbox, account: inbox.account)
+
+          service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
+          service.perform
+
+          expect(lid_contact_inbox.reload.contact_id).to eq(phone_contact.id)
+          expect(phone_contact.reload.identifier).to eq(identifier)
+          expect(lid_conversation.reload.contact_id).to eq(phone_contact.id)
+          expect(lid_message.reload.sender).to eq(phone_contact)
+          expect(Contact.exists?(lid_contact.id)).to be(false)
+        end
+      end
+
+      context 'when no identifier conflict exists' do
+        it 'does not change contacts' do
+          service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
+          service.perform
+
+          expect(phone_contact.reload.identifier).to be_nil
+        end
+      end
+    end
+
     context 'when only phone-based contact_inbox exists' do
       let!(:contact) { create(:contact, account: inbox.account, phone_number: "+#{phone}") }
       let!(:phone_contact_inbox) { create(:contact_inbox, inbox: inbox, contact: contact, source_id: phone) }
@@ -59,13 +103,15 @@ describe Whatsapp::ContactInboxConsolidationService do
       end
 
       context 'when there is an identifier conflict with a different contact' do
-        let!(:conflicting_contact) { create(:contact, account: inbox.account, identifier: identifier) } # rubocop:disable RSpec/LetSetup
+        let!(:conflicting_contact) { create(:contact, account: inbox.account, identifier: identifier) }
 
-        it 'does not migrate' do
+        it 'resolves the conflict and migrates' do
           service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
           service.perform
 
-          expect(phone_contact_inbox.reload.source_id).to eq(phone)
+          expect(phone_contact_inbox.reload.source_id).to eq(lid)
+          expect(contact.reload.identifier).to eq(identifier)
+          expect(conflicting_contact.reload.identifier).to be_nil
         end
       end
 
@@ -214,14 +260,15 @@ describe Whatsapp::ContactInboxConsolidationService do
       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
+        let!(:conflicting_contact) { create(:contact, account: inbox.account, identifier: identifier) }
 
-        it 'does not update to avoid identifier conflict' do
+        it 'resolves the conflict and updates' do
           service = described_class.new(inbox: inbox, phone: phone, lid: lid, identifier: identifier)
           service.perform
 
-          expect(old_contact_inbox.reload.source_id).to eq('999999999')
+          expect(old_contact_inbox.reload.source_id).to eq(lid)
-          expect(contact.reload.identifier).not_to eq(identifier)
+          expect(contact.reload.identifier).to eq(identifier)
+          expect(conflicting_contact.reload.identifier).to be_nil
         end
       end
     end

spec/services/whatsapp/zapi_handlers/received_callback_spec.rb

@@ -428,6 +428,122 @@ describe Whatsapp::ZapiHandlers::ReceivedCallback do
       end
     end
 
+    context 'when phone contact exists without contact_inbox and a separate LID contact exists (provider conversion)' do
+      let!(:phone_contact) do
+        create(:contact,
+               account: inbox.account,
+               identifier: 'old_baileys_lid@lid',
+               phone_number: '+5511987654321',
+               name: 'Phone Contact')
+      end
+      let!(:lid_contact) do
+        create(:contact,
+               account: inbox.account,
+               identifier: '123456789@lid',
+               phone_number: nil,
+               name: 'LID Contact')
+      end
+      let(:params) do
+        base_params.merge(
+          phone: '5511987654321',
+          chatLid: '123456789@lid'
+        )
+      end
+
+      it 'resolves the conflict and uses the phone contact' do
+        service.perform
+
+        expect(phone_contact.reload.identifier).to eq('123456789@lid')
+        message = Message.last
+        expect(message.sender).to eq(phone_contact)
+      end
+
+      context 'when the LID contact has a contact_inbox in this inbox' do
+        let!(:lid_contact_inbox) do
+          create(:contact_inbox, inbox: inbox, contact: lid_contact, source_id: '123456789')
+        end
+
+        it 'adopts the contact_inbox and uses the phone contact for the message' do
+          service.perform
+
+          expect(lid_contact_inbox.reload.contact_id).to eq(phone_contact.id)
+          expect(phone_contact.reload.identifier).to eq('123456789@lid')
+          message = Message.last
+          expect(message.sender).to eq(phone_contact)
+        end
+      end
+    end
+
+    context 'when phone contact has contact_inbox with phone as source_id and a separate LID contact exists' do
+      let!(:phone_contact) do
+        create(:contact,
+               account: inbox.account,
+               identifier: nil,
+               phone_number: '+5511987654321',
+               name: 'Phone Contact')
+      end
+      let!(:phone_contact_inbox) do
+        create(:contact_inbox, inbox: inbox, contact: phone_contact, source_id: '5511987654321')
+      end
+      let!(:lid_contact) do # rubocop:disable RSpec/LetSetup
+        create(:contact,
+               account: inbox.account,
+               identifier: '123456789@lid',
+               phone_number: nil,
+               name: 'LID Contact')
+      end
+      let(:params) do
+        base_params.merge(
+          phone: '5511987654321',
+          chatLid: '123456789@lid'
+        )
+      end
+
+      it 'resolves the identifier conflict, migrates the contact_inbox, and uses the phone contact' do
+        service.perform
+
+        expect(phone_contact_inbox.reload.source_id).to eq('123456789')
+        expect(phone_contact.reload.identifier).to eq('123456789@lid')
+        message = Message.last
+        expect(message.sender).to eq(phone_contact)
+      end
+    end
+
+    context 'when phone contact has contact_inbox with old source_id and a separate LID contact exists' do
+      let!(:phone_contact) do
+        create(:contact,
+               account: inbox.account,
+               identifier: 'old_lid@lid',
+               phone_number: '+5511987654321',
+               name: 'Phone Contact')
+      end
+      let!(:old_contact_inbox) do
+        create(:contact_inbox, inbox: inbox, contact: phone_contact, source_id: '999999999')
+      end
+      let!(:lid_contact) do # rubocop:disable RSpec/LetSetup
+        create(:contact,
+               account: inbox.account,
+               identifier: '123456789@lid',
+               phone_number: nil,
+               name: 'LID Contact')
+      end
+      let(:params) do
+        base_params.merge(
+          phone: '5511987654321',
+          chatLid: '123456789@lid'
+        )
+      end
+
+      it 'resolves the identifier conflict, updates the contact_inbox, and uses the phone contact' do
+        service.perform
+
+        expect(old_contact_inbox.reload.source_id).to eq('123456789')
+        expect(phone_contact.reload.identifier).to eq('123456789@lid')
+        message = Message.last
+        expect(message.sender).to eq(phone_contact)
+      end
+    end
+
     context 'when handling avatar' do
       let(:params) do
         base_params.merge(