From 5b5a6d89c0cf7f3148a1439d6fcd847784a79b94 Mon Sep 17 00:00:00 2001
From: Jordan Brough <jordan-brough@users.noreply.github.com>
Date: Sat, 7 May 2022 06:27:16 -0600
Subject: [PATCH] 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              |  7 +++-
 ..._sid_unique_index_to_channel_twilio_sms.rb | 39 +++++++++++++++++++
 db/schema.rb                                  |  3 +-
 3 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 db/migrate/20220315204137_add_account_sid_unique_index_to_channel_twilio_sms.rb

diff --git a/app/models/channel/twilio_sms.rb b/app/models/channel/twilio_sms.rb
index 89e9cbdc8..5f4f26048 100644
--- a/app/models/channel/twilio_sms.rb
+++ b/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 }
 
diff --git a/db/migrate/20220315204137_add_account_sid_unique_index_to_channel_twilio_sms.rb b/db/migrate/20220315204137_add_account_sid_unique_index_to_channel_twilio_sms.rb
new file mode 100644
index 000000000..e9bb5e189
--- /dev/null
+++ b/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
diff --git a/db/schema.rb b/db/schema.rb
index cc619bc18..c998b839b 100644
--- a/db/schema.rb
+++ b/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|