diff --git a/app/builders/contact_inbox_builder.rb b/app/builders/contact_inbox_builder.rb index 8fcd2b158..ffa45db2e 100644 --- a/app/builders/contact_inbox_builder.rb +++ b/app/builders/contact_inbox_builder.rb @@ -64,5 +64,40 @@ class ContactInboxBuilder inbox_id: @inbox.id, source_id: @source_id ) + rescue ActiveRecord::RecordNotUnique + Rails.logger.info("[ContactInboxBuilder] RecordNotUnique #{@source_id} #{@contact.id} #{@inbox.id}") + update_old_contact_inbox + retry + end + + def update_old_contact_inbox + # The race condition occurs when there’s a contact inbox with the + # same source ID but linked to a different contact. This can happen + # if the agent updates the contact’s email or phone number, or + # if the contact is merged with another. + # + # We update the old contact inbox source_id to a random value to + # avoid disrupting the current flow. However, the root cause of + # this issue is a flaw in the contact inbox model design. + # Contact inbox is essentially tracking a session and is not + # needed for non-live chat channels. + raise ActiveRecord::RecordNotUnique unless allowed_channels? + + contact_inbox = ::ContactInbox.find_by(inbox_id: @inbox.id, source_id: @source_id) + return if contact_inbox.blank? + + contact_inbox.update!(source_id: new_source_id) + end + + def new_source_id + if @inbox.whatsapp? || @inbox.sms? || @inbox.twilio? + "whatsapp:#{@source_id}#{rand(100)}" + else + "#{rand(10)}#{@source_id}" + end + end + + def allowed_channels? + @inbox.email? || @inbox.sms? || @inbox.twilio? || @inbox.whatsapp? end end diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 2c08172ff..508675858 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -98,6 +98,10 @@ class Inbox < ApplicationRecord update_account_cache end + def sms? + channel_type == 'Channel::Sms' + end + def facebook? channel_type == 'Channel::FacebookPage' end diff --git a/spec/builders/contact_inbox_builder_spec.rb b/spec/builders/contact_inbox_builder_spec.rb index 1dda58063..f0e3deb31 100644 --- a/spec/builders/contact_inbox_builder_spec.rb +++ b/spec/builders/contact_inbox_builder_spec.rb @@ -330,5 +330,42 @@ describe ContactInboxBuilder do expect(contact_inbox.source_id).not_to be_nil end end + + context 'when there is a race condition' do + let(:account) { create(:account) } + let(:contact) { create(:contact, account: account) } + let(:contact2) { create(:contact, account: account) } + let(:channel) { create(:channel_email, account: account) } + let(:channel_api) { create(:channel_api, account: account) } + let(:source_id) { 'source_123' } + + it 'handles RecordNotUnique error by updating source_id and retrying' do + existing_contact_inbox = create(:contact_inbox, contact: contact2, inbox: channel.inbox, source_id: source_id) + + described_class.new( + contact: contact, + inbox: channel.inbox, + source_id: source_id + ).perform + + expect(ContactInbox.last.source_id).to eq(source_id) + expect(ContactInbox.last.contact_id).to eq(contact.id) + expect(ContactInbox.last.inbox_id).to eq(channel.inbox.id) + expect(existing_contact_inbox.reload.source_id).to include(source_id) + expect(existing_contact_inbox.reload.source_id).not_to eq(source_id) + end + + it 'does not update source_id for channels other than email or phone number' do + create(:contact_inbox, contact: contact2, inbox: channel_api.inbox, source_id: source_id) + + expect do + described_class.new( + contact: contact, + inbox: channel_api.inbox, + source_id: source_id + ).perform + end.to raise_error(ActiveRecord::RecordNotUnique) + end + end end end