chore: Automate conversation display_id generation with db triggers (#1412)

Automate conversation display_id generation with db triggers

Co-authored-by: Saurabh Mehta <saurabh1.mehta@airtel.com>
Co-authored-by: Sojan Jose <sojan@pepalo.com>

Gemfile

@@ -96,6 +96,9 @@ gem 'geocoder'
 # to parse maxmind db
 gem 'maxminddb'
 
+# to create db triggers
+gem 'hairtrigger'
+
 group :development do
   gem 'annotate'
   gem 'bullet'

Gemfile.lock

@@ -237,6 +237,10 @@ GEM
     groupdate (5.1.0)
       activesupport (>= 5)
     haikunator (1.1.0)
+    hairtrigger (0.2.23)
+      activerecord (>= 5.0, < 7)
+      ruby2ruby (~> 2.4)
+      ruby_parser (~> 3.10)
     hana (1.3.6)
     hashdiff (1.0.1)
     hashie (4.1.0)
@@ -427,13 +431,19 @@ GEM
       parser (>= 2.7.1.4)
     rubocop-performance (1.7.1)
       rubocop (>= 0.82.0)
-    rubocop-rails (2.7.1)
+    rubocop-rails (2.8.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
       rubocop (>= 0.87.0)
     rubocop-rspec (1.43.2)
       rubocop (~> 0.87)
     ruby-progressbar (1.10.1)
+    ruby2ruby (2.4.4)
+      ruby_parser (~> 3.1)
+      sexp_processor (~> 4.6)
+    ruby_parser (3.15.0)
+      rubocop (>= 0.87.0)
+      sexp_processor (~> 4.9)
     safe_yaml (1.0.5)
     sass (3.7.4)
       sass-listen (~> 4.0.0)
@@ -459,6 +469,7 @@ GEM
     semantic_range (2.3.0)
     sentry-raven (3.0.3)
       faraday (>= 1.0)
+    sexp_processor (4.15.1)
     shoulda-matchers (4.4.1)
       activesupport (>= 4.2.0)
     sidekiq (6.1.1)
@@ -511,7 +522,7 @@ GEM
       nokogiri (>= 1.6, < 2.0)
     twitty (0.1.1)
       oauth
-    tzinfo (1.2.7)
+    tzinfo (1.2.8)
       thread_safe (~> 0.1)
     tzinfo-data (1.2020.1)
       tzinfo (>= 1.0.0)
@@ -554,7 +565,7 @@ GEM
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
     wisper (2.0.0)
-    zeitwerk (2.4.0)
+    zeitwerk (2.4.1)
 
 PLATFORMS
   ruby
@@ -589,6 +600,7 @@ DEPENDENCIES
   google-cloud-storage
   groupdate
   haikunator
+  hairtrigger
   hashie
   jbuilder
   json_refs!

app/models/account.rb

@@ -91,4 +91,8 @@ class Account < ApplicationRecord
   def notify_creation
     Rails.configuration.dispatcher.dispatch(ACCOUNT_CREATED, Time.zone.now, account: self)
   end
+
+  trigger.after(:insert).for_each(:row) do
+    "execute format('create sequence IF NOT EXISTS conv_dpid_seq_%s', NEW.id);"
+  end
 end

app/models/conversation.rb

@@ -7,12 +7,12 @@
 #  agent_last_seen_at    :datetime
 #  contact_last_seen_at  :datetime
 #  identifier            :string
-#  last_activity_at      :datetime         not null
+#  last_activity_at      :datetime
 #  locked                :boolean          default(FALSE)
 #  status                :integer          default("open"), not null
 #  uuid                  :uuid             not null
-#  created_at            :datetime         not null
+#  created_at            :datetime
-#  updated_at            :datetime         not null
+#  updated_at            :datetime
 #  account_id            :integer          not null
 #  assignee_id           :integer
 #  contact_id            :bigint
@@ -52,12 +52,13 @@ class Conversation < ApplicationRecord
   has_many :messages, dependent: :destroy, autosave: true
 
   before_create :set_bot_conversation
-  before_create :set_display_id, unless: :display_id?
+
   # wanted to change this to after_update commit. But it ended up creating a loop
   # reinvestigate in future and identity the implications
   after_update :notify_status_change, :create_activity
   after_create_commit :notify_conversation_creation, :queue_conversation_auto_resolution_job
   after_save :run_round_robin
+  after_commit :set_display_id, unless: :display_id?
 
   delegate :auto_resolve_duration, to: :account
 
@@ -154,10 +155,7 @@ class Conversation < ApplicationRecord
   end
 
   def set_display_id
-    self.display_id = loop do
+    reload
-      next_display_id = account.conversations.maximum('display_id').to_i + 1
-      break next_display_id unless account.conversations.exists?(display_id: next_display_id)
-    end
   end
 
   def create_activity
@@ -289,4 +287,9 @@ class Conversation < ApplicationRecord
   def mute_period
     6.hours
   end
+
+  # creating db triggers
+  trigger.before(:insert).for_each(:row) do
+    "NEW.display_id := nextval('conv_dpid_seq_' || NEW.account_id);"
+  end
 end

db/migrate/20201125121240_create_triggers_accounts_insert_or_conversations_insert.rb

@@ -0,0 +1,27 @@
+# This migration was auto-generated via `rake db:generate_trigger_migration'.
+# While you can edit this file, any changes you make to the definitions here
+# will be undone by the next auto-generated trigger migration.
+
+class CreateTriggersAccountsInsertOrConversationsInsert < ActiveRecord::Migration[6.0]
+  def up
+    create_trigger('accounts_after_insert_row_tr', generated: true, compatibility: 1)
+      .on('accounts')
+      .after(:insert)
+      .for_each(:row) do
+      "execute format('create sequence IF NOT EXISTS conv_dpid_seq_%s', NEW.id);"
+    end
+
+    create_trigger('conversations_before_insert_row_tr', generated: true, compatibility: 1)
+      .on('conversations')
+      .before(:insert)
+      .for_each(:row) do
+      "NEW.display_id := nextval('conv_dpid_seq_' || NEW.account_id);"
+    end
+  end
+
+  def down
+    drop_trigger('accounts_after_insert_row_tr', 'accounts', generated: true)
+
+    drop_trigger('conversations_before_insert_row_tr', 'conversations', generated: true)
+  end
+end

db/migrate/20201125123131_conv_dpid_seq_for_existing_accnts.rb

@@ -0,0 +1,21 @@
+class ConvDpidSeqForExistingAccnts < ActiveRecord::Migration[6.0]
+  def up
+    ::Account.find_in_batches do |accounts_batch|
+      Rails.logger.info "migrated till #{accounts_batch.first.id}\n"
+      accounts_batch.each do |account|
+        display_id = Conversation.where(account_id: account.id).maximum('display_id')
+        display_id ||= 0 # for accounts with out conversations
+        ActiveRecord::Base.connection.exec_query("create sequence IF NOT EXISTS conv_dpid_seq_#{account.id} START #{display_id + 1}")
+      end
+    end
+  end
+
+  def down
+    ::Account.find_in_batches do |accounts_batch|
+      Rails.logger.info "migrated till #{accounts_batch.first.id}\n"
+      accounts_batch.each do |account|
+        ActiveRecord::Base.connection.exec_query("drop sequence IF EXISTS conv_dpid_seq_#{account.id}")
+      end
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_10_27_135006) do
+ActiveRecord::Schema.define(version: 2020_11_25_123131) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
@@ -219,8 +219,8 @@ ActiveRecord::Schema.define(version: 2020_10_27_135006) do
     t.integer "inbox_id", null: false
     t.integer "status", default: 0, null: false
     t.integer "assignee_id"
-    t.datetime "created_at", null: false
+    t.datetime "created_at"
-    t.datetime "updated_at", null: false
+    t.datetime "updated_at"
     t.bigint "contact_id"
     t.integer "display_id", null: false
     t.datetime "contact_last_seen_at"
@@ -230,7 +230,7 @@ ActiveRecord::Schema.define(version: 2020_10_27_135006) do
     t.bigint "contact_inbox_id"
     t.uuid "uuid", default: -> { "gen_random_uuid()" }, null: false
     t.string "identifier"
-    t.datetime "last_activity_at", default: -> { "CURRENT_TIMESTAMP" }, null: false
+    t.datetime "last_activity_at", default: -> { "CURRENT_TIMESTAMP" }
     t.index ["account_id", "display_id"], name: "index_conversations_on_account_id_and_display_id", unique: true
     t.index ["account_id"], name: "index_conversations_on_account_id"
     t.index ["contact_inbox_id"], name: "index_conversations_on_contact_inbox_id"
@@ -536,4 +536,18 @@ ActiveRecord::Schema.define(version: 2020_10_27_135006) do
   add_foreign_key "contact_inboxes", "contacts"
   add_foreign_key "contact_inboxes", "inboxes"
   add_foreign_key "conversations", "contact_inboxes"
+  create_trigger("accounts_after_insert_row_tr", :generated => true, :compatibility => 1).
+      on("accounts").
+      after(:insert).
+      for_each(:row) do
+    "execute format('create sequence IF NOT EXISTS conv_dpid_seq_%s', NEW.id);"
+  end
+
+  create_trigger("conversations_before_insert_row_tr", :generated => true, :compatibility => 1).
+      on("conversations").
+      before(:insert).
+      for_each(:row) do
+    "NEW.display_id := nextval('conv_dpid_seq_' || NEW.account_id);"
+  end
+
 end

spec/factories/conversations.rb

@@ -3,7 +3,6 @@
 FactoryBot.define do
   factory :conversation do
     status { 'open' }
-    display_id { rand(10_000_000) }
     agent_last_seen_at { Time.current }
     locked { false }
     identifier { SecureRandom.hex }

spec/models/conversation_spec.rb

@@ -109,8 +109,8 @@ RSpec.describe Conversation, type: :model do
     end
 
     it 'adds a message for system auto resolution if marked resolved by system' do
-      conversation2 = create(:conversation, status: 'open', account: account, assignee: old_assignee)
       account.update(auto_resolve_duration: 40)
+      conversation2 = create(:conversation, status: 'open', account: account, assignee: old_assignee)
       Current.user = nil
       conversation2.update(status: :resolved)
       system_resolved_message = "Conversation was marked resolved by system due to #{account.auto_resolve_duration} days of inactivity"
@@ -124,7 +124,7 @@ RSpec.describe Conversation, type: :model do
 
     it 'does trigger AutoResolutionJob if conversation reopened and account has auto resolve duration' do
       account.update(auto_resolve_duration: 40)
-      expect { conversation.update(status: :open) }
+      expect { conversation.reload.update(status: :open) }
         .to have_enqueued_job(AutoResolveConversationsJob).with(conversation.id)
     end
   end

spec/models/message_spec.rb

@@ -13,6 +13,7 @@ RSpec.describe Message, type: :model do
     let(:message) { build(:message, account: create(:account)) }
 
     it 'updates conversation last_activity_at when created' do
+      message.save!
       expect(message.created_at).to eq message.conversation.last_activity_at
     end