diff --git a/Gemfile b/Gemfile index 88f2fc75c..5b03b771b 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 35b00859c..b6a773967 100644 --- a/Gemfile.lock +++ b/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! diff --git a/app/models/account.rb b/app/models/account.rb index 36acd1e75..6c49f6263 100644 --- a/app/models/account.rb +++ b/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 diff --git a/app/models/conversation.rb b/app/models/conversation.rb index f6ba5c3cc..efd3aa963 100644 --- a/app/models/conversation.rb +++ b/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 -# updated_at :datetime not null +# created_at :datetime +# 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 - 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 + reload 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 diff --git a/db/migrate/20201125121240_create_triggers_accounts_insert_or_conversations_insert.rb b/db/migrate/20201125121240_create_triggers_accounts_insert_or_conversations_insert.rb new file mode 100644 index 000000000..5ea5d38a5 --- /dev/null +++ b/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 diff --git a/db/migrate/20201125123131_conv_dpid_seq_for_existing_accnts.rb b/db/migrate/20201125123131_conv_dpid_seq_for_existing_accnts.rb new file mode 100644 index 000000000..73acb8ae8 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index b0144fb09..e7c0eb71d 100644 --- a/db/schema.rb +++ b/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 "updated_at", null: false + t.datetime "created_at" + 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 diff --git a/spec/factories/conversations.rb b/spec/factories/conversations.rb index e3647cc64..f09fa6e11 100644 --- a/spec/factories/conversations.rb +++ b/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 } diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index df2a1bda7..d490a2441 100644 --- a/spec/models/conversation_spec.rb +++ b/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 diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 290bdef05..061c5af9a 100644 --- a/spec/models/message_spec.rb +++ b/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