fix: bots included in time to response metrics (#6409)

* feat: ignore bots in avg_first_response_time

* feat: ignore bots in avg_first_response count

* feat: add bot handoff event

* feat: add handoff event listener and reporting event

* fix: ignore agent bot in first response

* refactor: calculate first_response with last handoff

* refactor: method defn order

* test: new reporting events

* feat: Revert "feat: ignore bots in avg_first_response count"

This reverts commit de1977c219a2e7a9180dd02272244fe3b3f7ce89.

* feat: Revert "feat: ignore bots in avg_first_response_time"

This reverts commit bb9171945d5e3b2f6015f4f96dd1b76b3efb6987.

* fix: business hour calculation for first_reply

* fix: event_start_time for first_response

* feat: add migration to recompute first_responses

* refactor: separate mute helpers for conversation

* refactor: rename migration

* refactor: migration script

* fix: migration typo

* fix: typo in query

* feat: update schema.rb

* Revert "feat: update schema.rb"

This reverts commit 353ef355f2d956dd219907bb66982dc90ca5d896.

* feat: update schema

* refactor: update events as a batch job

* fix: ignore the event if value is negative

* feat: don't create a new hand-off if it's already present

* refactor: break the action into smaller chunks

* refactor: update reporting listener spec

Handle the case to ensure extra bot handoffs are not created for a give conversation

* fix: import error

---------

Co-authored-by: Vishnu Narayanan <vishnu@chatwoot.com>

app/helpers/reporting_event_helper.rb

@@ -17,6 +17,15 @@ module ReportingEventHelper
     from_in_inbox_timezone.working_time_until(to_in_inbox_timezone)
   end
 
+  def last_non_human_activity(conversation)
+    # check if a handoff event already exists
+    handoff_event = ReportingEvent.where(conversation_id: conversation.id, name: 'conversation_bot_handoff').last
+
+    # if a handoff exists, last non human activity is when the handoff ended,
+    # otherwise it's when the conversation was created
+    handoff_event&.event_end_time || conversation.created_at
+  end
+
   private
 
   def configure_working_hours(working_hours)

app/jobs/migration/update_first_response_time_in_reporting_events_job.rb

@@ -0,0 +1,61 @@
+# Delete migration and spec after 2 consecutive releases.
+class Migration::UpdateFirstResponseTimeInReportingEventsJob < ApplicationJob
+  include ReportingEventHelper
+
+  queue_as :scheduled_jobs
+
+  def perform(account)
+    account.reporting_events.where(name: 'first_response', user_id: nil).each do |event|
+      conversation = event.conversation
+      next if conversation.nil?
+
+      update_event_data(event, conversation)
+    end
+  end
+
+  def update_event_data(event, conversation)
+    last_bot_reply = conversation.messages.where(sender_type: 'AgentBot').order(created_at: :asc).last
+    first_human_reply = conversation.messages.where(sender_type: 'User').order(created_at: :asc).first
+
+    # 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
+    #
+    # conversation       bot                         conversation
+    # start              handoff                     resolved
+    # |                  |                           |
+    # |____|___|_________|____|_______|_____|________|
+    #      bot reply     ^    ^  human reply
+    #                    |    |
+    #                    |    |
+    #       last_bot_reply    first_human_reply
+    #
+    #
+    # 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
+    update_event_details(event, last_bot_reply, first_human_reply, conversation.inbox)
+  end
+
+  def update_event_details(event, last_bot_reply, first_human_reply, inbox)
+    # rubocop:disable Rails/SkipsModelValidations
+    event.update_columns(event_start_time: last_bot_reply.created_at,
+                         event_end_time: first_human_reply.created_at,
+                         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)
+    # rubocop:enable Rails/SkipsModelValidations
+  end
+
+  def calculate_event_value(last_bot_reply, first_human_reply)
+    first_human_reply.created_at.to_i - last_bot_reply.created_at.to_i
+  end
+
+  def calculate_event_value_in_business_hours(inbox, last_bot_reply, first_human_reply)
+    business_hours(inbox, last_bot_reply.created_at, first_human_reply.created_at)
+  end
+end

app/listeners/reporting_event_listener.rb

@@ -1,5 +1,6 @@
 class ReportingEventListener < BaseListener
   include ReportingEventHelper
+
   def conversation_resolved(event)
     conversation = extract_conversation_and_account(event)[0]
     time_to_resolve = conversation.updated_at.to_i - conversation.created_at.to_i
@@ -22,18 +23,18 @@ class ReportingEventListener < BaseListener
   def first_reply_created(event)
     message = extract_message_and_account(event)[0]
     conversation = message.conversation
-    first_response_time = message.created_at.to_i - conversation.created_at.to_i
+    first_response_time = message.created_at.to_i - last_non_human_activity(conversation).to_i
 
     reporting_event = ReportingEvent.new(
       name: 'first_response',
       value: first_response_time,
-      value_in_business_hours: business_hours(conversation.inbox, conversation.created_at,
+      value_in_business_hours: business_hours(conversation.inbox, last_non_human_activity(conversation),
                                               message.created_at),
       account_id: conversation.account_id,
       inbox_id: conversation.inbox_id,
       user_id: conversation.assignee_id,
       conversation_id: conversation.id,
-      event_start_time: conversation.created_at,
+      event_start_time: last_non_human_activity(conversation),
       event_end_time: message.created_at
     )
 
@@ -41,4 +42,27 @@ class ReportingEventListener < BaseListener
 
     reporting_event.save!
   end
+
+  def conversation_bot_handoff(event)
+    conversation = extract_conversation_and_account(event)[0]
+
+    # check if a conversation_bot_handoff event exists for this conversation
+    bot_handoff_event = ReportingEvent.find_by(conversation_id: conversation.id, name: 'conversation_bot_handoff')
+    return if bot_handoff_event.present?
+
+    time_to_handoff = conversation.updated_at.to_i - conversation.created_at.to_i
+
+    reporting_event = ReportingEvent.new(
+      name: 'conversation_bot_handoff',
+      value: time_to_handoff,
+      value_in_business_hours: business_hours(conversation.inbox, conversation.created_at, conversation.updated_at),
+      account_id: conversation.account_id,
+      inbox_id: conversation.inbox_id,
+      user_id: conversation.assignee_id,
+      conversation_id: conversation.id,
+      event_start_time: conversation.created_at,
+      event_end_time: conversation.updated_at
+    )
+    reporting_event.save!
+  end
 end

app/models/concerns/conversation_mute_helpers.rb

@@ -0,0 +1,28 @@
+module ConversationMuteHelpers
+  extend ActiveSupport::Concern
+
+  def mute!
+    resolved!
+    Redis::Alfred.setex(mute_key, 1, mute_period)
+    create_muted_message
+  end
+
+  def unmute!
+    Redis::Alfred.delete(mute_key)
+    create_unmuted_message
+  end
+
+  def muted?
+    Redis::Alfred.get(mute_key).present?
+  end
+
+  private
+
+  def mute_key
+    format(Redis::RedisKeys::CONVERSATION_MUTE_KEY, id: id)
+  end
+
+  def mute_period
+    6.hours
+  end
+end

app/models/conversation.rb

@@ -48,6 +48,7 @@ class Conversation < ApplicationRecord
   include ActivityMessageHandler
   include UrlHelper
   include SortHandler
+  include ConversationMuteHelpers
 
   validates :account_id, presence: true
   validates :inbox_id, presence: true
@@ -142,19 +143,9 @@ class Conversation < ApplicationRecord
     save
   end
 
-  def mute!
+  def bot_handoff!
-    resolved!
+    open!
-    Redis::Alfred.setex(mute_key, 1, mute_period)
+    dispatcher_dispatch(CONVERSATION_BOT_HANDOFF)
-    create_muted_message
-  end
-
-  def unmute!
-    Redis::Alfred.delete(mute_key)
-    create_unmuted_message
-  end
-
-  def muted?
-    Redis::Alfred.get(mute_key).present?
   end
 
   def unread_messages
@@ -269,14 +260,6 @@ class Conversation < ApplicationRecord
     create_label_removed(user_name, previous_labels - current_labels)
   end
 
-  def mute_key
-    format(Redis::RedisKeys::CONVERSATION_MUTE_KEY, id: id)
-  end
-
-  def mute_period
-    6.hours
-  end
-
   def validate_referer_url
     return unless additional_attributes['referer']
 

app/models/message.rb

@@ -188,10 +188,15 @@ class Message < ApplicationRecord
     sender.update(last_activity_at: DateTime.now) if sender.is_a?(Contact)
   end
 
+  def first_human_response?
+    conversation.messages.outgoing
+                .where.not(sender_type: 'AgentBot')
+                .where("(additional_attributes->'campaign_id') is null").count == 1
+  end
+
   def dispatch_create_events
     Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by)
-
+    if outgoing? && first_human_response?
-    if outgoing? && conversation.messages.outgoing.where("(additional_attributes->'campaign_id') is null").count == 1
       Rails.configuration.dispatcher.dispatch(FIRST_REPLY_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by)
     end
   end

db/migrate/20230214025901_update_reporting_events_with_incorrect_first_responses.rb

@@ -0,0 +1,10 @@
+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
+  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_09_033203) do
+ActiveRecord::Schema.define(version: 2023_02_14_025901) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"

lib/events/types.rb

@@ -16,6 +16,7 @@ module Events::Types
   CONVERSATION_CREATED = 'conversation.created'
   CONVERSATION_UPDATED = 'conversation.updated'
   CONVERSATION_READ = 'conversation.read'
+  CONVERSATION_BOT_HANDOFF = 'conversation.bot_handoff'
   # FIXME: deprecate the opened and resolved events in future in favor of status changed event.
   CONVERSATION_OPENED = 'conversation.opened'
   CONVERSATION_RESOLVED = 'conversation.resolved'

lib/integrations/bot_processor_service.rb

@@ -55,7 +55,7 @@ class Integrations::BotProcessorService
   def process_action(message, action)
     case action
     when 'handoff'
-      message.conversation.open!
+      message.conversation.bot_handoff!
     when 'resolve'
       message.conversation.resolved!
     end

spec/listeners/reporting_event_listener_spec.rb

@@ -60,5 +60,56 @@ describe ReportingEventListener do
         expect(account.reporting_events.where(name: 'first_response')[0]['value_in_business_hours']).to be 144_000.0
       end
     end
+
+    # this ensures last_non_human_activity method accurately accounts for handoff events
+    context 'when last handoff event exists' do
+      let(:conversation_updated_at) { 20.seconds.from_now }
+      let(:human_message_created_at) { 62.seconds.from_now }
+      let(:new_conversation) { create(:conversation, account: account, inbox: inbox, assignee: user, updated_at: conversation_updated_at) }
+      let(:new_message) do
+        create(:message, message_type: 'outgoing', created_at: human_message_created_at, account: account, inbox: inbox,
+                         conversation: new_conversation)
+      end
+
+      it 'creates first_response event with handoff value' do
+        # this will create a handoff event
+        event = Events::Base.new('conversation.bot_handoff', conversation_updated_at, conversation: new_conversation)
+        listener.conversation_bot_handoff(event)
+
+        # create the first reply event
+        event = Events::Base.new('first.reply.created', human_message_created_at, message: new_message)
+        listener.first_reply_created(event)
+        expect(account.reporting_events.where(name: 'first_response')[0]['value']).to be 42.0
+      end
+    end
+  end
+
+  describe '#conversation_bot_handoff' do
+    it 'creates conversation_bot_handoff event only once' do
+      expect(account.reporting_events.where(name: 'conversation_bot_handoff').count).to be 0
+      event = Events::Base.new('conversation.bot_handoff', Time.zone.now, conversation: conversation)
+      listener.conversation_bot_handoff(event)
+      expect(account.reporting_events.where(name: 'conversation_bot_handoff').count).to be 1
+
+      # add extra handoff event for the same and ensure it's not created
+      event = Events::Base.new('conversation.bot_handoff', Time.zone.now, conversation: conversation)
+      listener.conversation_bot_handoff(event)
+      expect(account.reporting_events.where(name: 'conversation_bot_handoff').count).to be 1
+    end
+
+    context 'when business hours enabled for inbox' do
+      let(:created_at) { Time.zone.parse('March 20, 2022 00:00') }
+      let(:updated_at) { Time.zone.parse('March 26, 2022 23:59') }
+      let!(:new_inbox) { create(:inbox, working_hours_enabled: true, account: account) }
+      let!(:new_conversation) do
+        create(:conversation, created_at: created_at, updated_at: updated_at, account: account, inbox: new_inbox, assignee: user)
+      end
+
+      it 'creates conversation_bot_handoff event with business hour value' do
+        event = Events::Base.new('conversation.bot_handoff', Time.zone.now, conversation: new_conversation)
+        listener.conversation_bot_handoff(event)
+        expect(account.reporting_events.where(name: 'conversation_bot_handoff')[0]['value_in_business_hours']).to be 144_000.0
+      end
+    end
   end
 end