fix: migration script to run on all reporting events (#6590)

* fix: migration script to run on all reporting events

* fix: don't update user_id if it is already present

* refactor: create a new migration

* feat: update schema

* feat: ignore events with bot handoff

* feat: prefetch conversations with handoff events

* Revert "feat: update schema"

This reverts commit 25ed2856e62655f5f1db14fd0cffad3a69d0b1fb.

* feat: update schema

* refactor: separate method get_conversations_with_bot_handoffs

* refactor: cognitive complexity

* refactor: early return if last_bot_reply is blank

* feat: add async_database_migration queue

* feat: update queue priority

app/jobs/migration/update_first_response_time_in_reporting_events_job.rb

@@ -2,20 +2,30 @@
 class Migration::UpdateFirstResponseTimeInReportingEventsJob < ApplicationJob
   include ReportingEventHelper
 
-  queue_as :scheduled_jobs
+  queue_as :async_database_migration
 
   def perform(account)
-    account.reporting_events.where(name: 'first_response', user_id: nil).each do |event|
+    get_conversations_with_bot_handoffs(account)
+    account.reporting_events.where(name: 'first_response').each do |event|
       conversation = event.conversation
-      next if conversation.nil?
+
+      # if the conversation has a bot handoff event, we don't need to update the response_time
+      next if conversation.nil? || @conversations_with_handoffs.include?(conversation.id)
 
       update_event_data(event, conversation)
     end
   end
 
+  def get_conversations_with_bot_handoffs(account)
+    @conversations_with_handoffs = account.reporting_events.where(name: 'conversation_bot_handoff').pluck(:conversation_id)
+  end
+
   def update_event_data(event, conversation)
     last_bot_reply = conversation.messages.where(sender_type: 'AgentBot').order(created_at: :asc).last
+    return if last_bot_reply.blank?
+
     first_human_reply = conversation.messages.where(sender_type: 'User').order(created_at: :asc).first
+    return if first_human_reply.blank?
 
     # accomodate for campaign if required
     # new_value = difference between the first_human_reply and the first_bot_reply if it exists or first_human_reply and created at
@@ -32,8 +42,6 @@ class Migration::UpdateFirstResponseTimeInReportingEventsJob < ApplicationJob
     #
     # bot handoff happens at the last_bot_reply created time
     # the response time is the time between last bot reply created and the first human reply created
-
-    return if last_bot_reply.blank? || first_human_reply.blank?
     return if last_bot_reply.created_at.to_i >= first_human_reply.created_at.to_i
 
     # this means a bot replied existed, so we need to update the event_start_time
@@ -47,7 +55,7 @@ class Migration::UpdateFirstResponseTimeInReportingEventsJob < ApplicationJob
                          value: calculate_event_value(last_bot_reply, first_human_reply),
                          value_in_business_hours: calculate_event_value_in_business_hours(inbox, last_bot_reply,
                                                                                           first_human_reply),
-                         user_id: first_human_reply.sender_id)
+                         user_id: event.user_id || first_human_reply.sender_id)
     # rubocop:enable Rails/SkipsModelValidations
   end
 

config/sidekiq.yml

@@ -12,19 +12,20 @@
 # even put in dynamic logic, like a host-specific queue.
 # http://www.mikeperham.com/2013/11/13/advanced-sidekiq-host-specific-queues/
 :queues:
-  - [low, 1]
-  - [scheduled_jobs, 1]
-  - [webhooks, 1]
-  - [bots, 1]
-  - [active_storage_analysis, 1]
-  - [action_mailbox_incineration, 1]
-  - [active_storage_purge, 1]
-  - [integrations, 2]
-  - [default, 2]
-  - [mailers, 2]
-  - [medium, 3]
-  - [events, 3]
-  - [action_mailbox_routing, 3]
+  - [async_database_migration, 1]
+  - [low, 2]
+  - [scheduled_jobs, 2]
+  - [webhooks, 2]
+  - [bots, 2]
+  - [active_storage_analysis, 2]
+  - [action_mailbox_incineration, 2]
+  - [active_storage_purge, 2]
+  - [integrations, 3]
+  - [default, 3]
+  - [mailers, 3]
+  - [medium, 4]
+  - [events, 4]
+  - [action_mailbox_routing, 4]
   - [high, 5]
   - [critical, 10]
 

db/migrate/20230214025901_update_reporting_events_with_incorrect_first_responses.rb

@@ -1,10 +1,5 @@
 class UpdateReportingEventsWithIncorrectFirstResponses < ActiveRecord::Migration[6.1]
   def change
-    ::Account.find_in_batches do |account_batch|
-      Rails.logger.info "Updated reporting events till #{account_batch.first.id}\n"
-      account_batch.each do |account|
-        Migration::UpdateFirstResponseTimeInReportingEventsJob.perform_later(account)
-      end
-    end
+    Rails.logger.info "Skipping this migration, it's replaced by a new one"
   end
 end

db/migrate/20230302054408_update_reporting_events_first_response_time.rb

@@ -0,0 +1,10 @@
+class UpdateReportingEventsFirstResponseTime < ActiveRecord::Migration[6.1]
+  def change
+    ::Account.find_in_batches do |account_batch|
+      Rails.logger.info "Updated reporting events till #{account_batch.first.id}\n"
+      account_batch.each do |account|
+        Migration::UpdateFirstResponseTimeInReportingEventsJob.perform_later(account)
+      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: 2023_02_24_124632) do
+ActiveRecord::Schema.define(version: 2023_03_02_054408) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"