chore: "Channel::TwilioSms" to be unique on account_sid+phone_number (#4188)

"Twilio::IncomingMessageService" searches for the correct "Channel::TwilioSms"
by account_sid+phone_number.  If these values are duplicated then which record it
finds is indeterminate and may alternate between queries.

Co-authored-by: Sojan Jose <sojan@pepalo.com>

app/models/channel/twilio_sms.rb

@@ -13,7 +13,8 @@
 #
 # Indexes
 #
-#  index_channel_twilio_sms_on_account_id_and_phone_number  (account_id,phone_number) UNIQUE
+#  index_channel_twilio_sms_on_account_sid_and_phone_number  (account_sid,phone_number) UNIQUE
+#  index_channel_twilio_sms_on_phone_number                  (phone_number) UNIQUE
 #
 
 class Channel::TwilioSms < ApplicationRecord
@@ -23,7 +24,9 @@ class Channel::TwilioSms < ApplicationRecord
 
   validates :account_sid, presence: true
   validates :auth_token, presence: true
-  validates :phone_number, uniqueness: { scope: :account_id }, presence: true
+  # NOTE: allowing nil for future when we suppor twilio messaging services
+  # https://github.com/chatwoot/chatwoot/pull/4242
+  validates :phone_number, uniqueness: true, allow_nil: true
 
   enum medium: { sms: 0, whatsapp: 1 }
 

db/migrate/20220315204137_add_account_sid_unique_index_to_channel_twilio_sms.rb

@@ -0,0 +1,39 @@
+class AddAccountSidUniqueIndexToChannelTwilioSms < ActiveRecord::Migration[6.1]
+  def change
+    clear_possible_duplicates
+
+    has_duplicates = Channel::TwilioSms.select(:phone_number).group(:phone_number).having('count(*) > 1').exists?
+
+    if has_duplicates
+      raise <<~ERR.squish
+        ERROR: You have duplicate values for phone_number in "channel_twilio_sms".
+        This causes Twilio account lookup to behave unpredictably. Please eliminate the duplicates
+        and then re-run this migration.
+      ERR
+    end
+
+    remove_index :channel_twilio_sms, [:account_id, :phone_number], unique: true
+    add_index :channel_twilio_sms, :phone_number, unique: true
+    # since look ups are done via account_sid + phone_number, we need to add a unique index
+    add_index :channel_twilio_sms, [:account_sid, :phone_number], unique: true
+  end
+
+  def clear_possible_duplicates
+    # based on the look up in saas it seems like only the first inbox is used in case of duplicates,
+    # so lets try to clear our inboxes with out conversations
+    duplicate_phone_numbers = Channel::TwilioSms.select(:phone_number).group(:phone_number).having('count(*) > 1').collect(&:phone_number)
+    duplicate_phone_numbers.each do |phone_number|
+      # we are skipping the first inbox that was created
+      Channel::TwilioSms.where(phone_number: phone_number).drop(1).each do |channel|
+        inbox = channel.inbox
+        # skip inboxes with conversations
+        next if inbox.conversations.count.positive?
+
+        inbox.destroy
+      end
+    end
+
+    # clear the accounts created with twilio sandbox whatsapp number
+    # Channel::TwilioSms.where(phone_number: 'whatsapp:+14155238886').each { |channel| channel.inbox.destroy }
+  end
+end

db/schema.rb

@@ -257,7 +257,8 @@ ActiveRecord::Schema.define(version: 2022_04_28_101325) do
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
     t.integer "medium", default: 0
-    t.index ["account_id", "phone_number"], name: "index_channel_twilio_sms_on_account_id_and_phone_number", unique: true
+    t.index ["account_sid", "phone_number"], name: "index_channel_twilio_sms_on_account_sid_and_phone_number", unique: true
+    t.index ["phone_number"], name: "index_channel_twilio_sms_on_phone_number", unique: true
   end
 
   create_table "channel_twitter_profiles", force: :cascade do |t|