From 41e269e873a565b545a4e88228534c2316a50899 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 28 Feb 2024 04:23:28 +0530 Subject: [PATCH] feat(ee): Add reporting events for bots (#9027) Added a new event conversation_bot_resolved and added a job to auto resolve the bot conversations if there was no activity for the last 1 hour. --- .github/workflows/run_response_bot_spec.yml | 1 + .../conversations_resolution_scheduler_job.rb | 1 + app/listeners/reporting_event_listener.rb | 14 +++++++ .../conversations_resolution_scheduler_job.rb | 10 +++++ ...ox_pending_conversations_resolution_job.rb | 19 ++++++++++ .../{ => response_bot}/response_bot_job.rb | 2 +- .../response_builder_job.rb | 2 +- .../response_document_content_job.rb | 2 +- enterprise/app/models/response_document.rb | 4 +- .../hook_execution_service.rb | 2 +- .../message_templates/response_bot_service.rb | 4 +- .../services/features/response_bot_service.rb | 5 +++ .../response_sources_controller_spec.rb | 2 +- .../v1/accounts/inboxes_controller_spec.rb | 2 +- ...ersations_resolution_scheduler_job_spec.rb | 29 ++++++++++++++ ...nding_conversations_resolution_job_spec.rb | 38 +++++++++++++++++++ .../response_bot_service_spec.rb | 23 ++++++++--- spec/factories/inbox_response_source.rb | 8 ++++ .../reporting_event_listener_spec.rb | 31 +++++++++++++++ spec/spec_helper.rb | 10 +++++ 20 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb create mode 100644 enterprise/app/jobs/response_bot/inbox_pending_conversations_resolution_job.rb rename enterprise/app/jobs/{ => response_bot}/response_bot_job.rb (75%) rename enterprise/app/jobs/{ => response_bot}/response_builder_job.rb (97%) rename enterprise/app/jobs/{ => response_bot}/response_document_content_job.rb (83%) create mode 100644 spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb create mode 100644 spec/enterprise/jobs/response_bot/inbox_pending_conversations_resolution_job_spec.rb create mode 100644 spec/factories/inbox_response_source.rb diff --git a/.github/workflows/run_response_bot_spec.yml b/.github/workflows/run_response_bot_spec.yml index 77b96c48a..c594ff404 100644 --- a/.github/workflows/run_response_bot_spec.yml +++ b/.github/workflows/run_response_bot_spec.yml @@ -73,6 +73,7 @@ jobs: spec/enterprise/controllers/api/v1/accounts/response_sources_controller_spec.rb \ spec/enterprise/services/enterprise/message_templates/response_bot_service_spec.rb \ spec/enterprise/controllers/enterprise/api/v1/accounts/inboxes_controller_spec.rb:47 \ + spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb \ --profile=10 \ --format documentation diff --git a/app/jobs/account/conversations_resolution_scheduler_job.rb b/app/jobs/account/conversations_resolution_scheduler_job.rb index 37848db49..ab9fc4029 100644 --- a/app/jobs/account/conversations_resolution_scheduler_job.rb +++ b/app/jobs/account/conversations_resolution_scheduler_job.rb @@ -7,3 +7,4 @@ class Account::ConversationsResolutionSchedulerJob < ApplicationJob end end end +Account::ConversationsResolutionSchedulerJob.prepend_mod_with('Account::ConversationsResolutionSchedulerJob') diff --git a/app/listeners/reporting_event_listener.rb b/app/listeners/reporting_event_listener.rb index 093452e92..7c625c12d 100644 --- a/app/listeners/reporting_event_listener.rb +++ b/app/listeners/reporting_event_listener.rb @@ -17,6 +17,8 @@ class ReportingEventListener < BaseListener event_start_time: conversation.created_at, event_end_time: conversation.updated_at ) + + create_bot_resolved_event(conversation, reporting_event) reporting_event.save! end @@ -83,4 +85,16 @@ class ReportingEventListener < BaseListener ) reporting_event.save! end + + private + + def create_bot_resolved_event(conversation, reporting_event) + return unless conversation.inbox.active_bot? + # We don't want to create a bot_resolved event if there is user interaction on the conversation + return if conversation.messages.exists?(message_type: :outgoing, sender_type: 'User') + + bot_resolved_event = reporting_event.dup + bot_resolved_event.name = 'conversation_bot_resolved' + bot_resolved_event.save! + end end diff --git a/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb b/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb new file mode 100644 index 000000000..4f80424f8 --- /dev/null +++ b/enterprise/app/jobs/enterprise/account/conversations_resolution_scheduler_job.rb @@ -0,0 +1,10 @@ +module Enterprise::Account::ConversationsResolutionSchedulerJob + def perform + super + Account.feature_response_bot.all.find_each(batch_size: 100) do |account| + account.inboxes.each do |inbox| + ResponseBot::InboxPendingConversationsResolutionJob.perform_later(inbox) if inbox.response_bot_enabled? + end + end + end +end diff --git a/enterprise/app/jobs/response_bot/inbox_pending_conversations_resolution_job.rb b/enterprise/app/jobs/response_bot/inbox_pending_conversations_resolution_job.rb new file mode 100644 index 000000000..f6333d446 --- /dev/null +++ b/enterprise/app/jobs/response_bot/inbox_pending_conversations_resolution_job.rb @@ -0,0 +1,19 @@ +class ResponseBot::InboxPendingConversationsResolutionJob < ApplicationJob + queue_as :low + + def perform(inbox) + # limiting the number of conversations to be resolved to avoid any performance issues + resolvable_conversations = inbox.conversations.pending.where('last_activity_at < ? ', Time.now.utc - 1.hour).limit(Limits::BULK_ACTIONS_LIMIT) + resolvable_conversations.each do |conversation| + conversation.messages.create!( + { + message_type: :outgoing, + account_id: conversation.account_id, + inbox_id: conversation.inbox_id, + content: 'Resolving the conversation as it has been inactive for a while. Please start a new conversation if you need further assistance.' + } + ) + conversation.resolved! + end + end +end diff --git a/enterprise/app/jobs/response_bot_job.rb b/enterprise/app/jobs/response_bot/response_bot_job.rb similarity index 75% rename from enterprise/app/jobs/response_bot_job.rb rename to enterprise/app/jobs/response_bot/response_bot_job.rb index 5ad703bab..c235d4803 100644 --- a/enterprise/app/jobs/response_bot_job.rb +++ b/enterprise/app/jobs/response_bot/response_bot_job.rb @@ -1,4 +1,4 @@ -class ResponseBotJob < ApplicationJob +class ResponseBot::ResponseBotJob < ApplicationJob queue_as :medium def perform(conversation) diff --git a/enterprise/app/jobs/response_builder_job.rb b/enterprise/app/jobs/response_bot/response_builder_job.rb similarity index 97% rename from enterprise/app/jobs/response_builder_job.rb rename to enterprise/app/jobs/response_bot/response_builder_job.rb index 536cdef5a..6a8cc07a2 100644 --- a/enterprise/app/jobs/response_builder_job.rb +++ b/enterprise/app/jobs/response_bot/response_builder_job.rb @@ -1,4 +1,4 @@ -class ResponseBuilderJob < ApplicationJob +class ResponseBot::ResponseBuilderJob < ApplicationJob queue_as :default def perform(response_document) diff --git a/enterprise/app/jobs/response_document_content_job.rb b/enterprise/app/jobs/response_bot/response_document_content_job.rb similarity index 83% rename from enterprise/app/jobs/response_document_content_job.rb rename to enterprise/app/jobs/response_bot/response_document_content_job.rb index bced3a12f..31c87e293 100644 --- a/enterprise/app/jobs/response_document_content_job.rb +++ b/enterprise/app/jobs/response_bot/response_document_content_job.rb @@ -1,5 +1,5 @@ # app/jobs/response_document_content_job.rb -class ResponseDocumentContentJob < ApplicationJob +class ResponseBot::ResponseDocumentContentJob < ApplicationJob queue_as :default def perform(response_document) diff --git a/enterprise/app/models/response_document.rb b/enterprise/app/models/response_document.rb index 91540e6ab..97e7ee949 100644 --- a/enterprise/app/models/response_document.rb +++ b/enterprise/app/models/response_document.rb @@ -35,12 +35,12 @@ class ResponseDocument < ApplicationRecord def ensure_content return unless content.nil? - ResponseDocumentContentJob.perform_later(self) + ResponseBot::ResponseDocumentContentJob.perform_later(self) end def handle_content_change return unless saved_change_to_content? && content.present? - ResponseBuilderJob.perform_later(self) + ResponseBot::ResponseBuilderJob.perform_later(self) end end diff --git a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb index 5eadd2416..47aa559f9 100644 --- a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb +++ b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb @@ -1,7 +1,7 @@ module Enterprise::MessageTemplates::HookExecutionService def trigger_templates super - ResponseBotJob.perform_later(conversation) if should_process_response_bot? + ResponseBot::ResponseBotJob.perform_later(conversation) if should_process_response_bot? end def should_process_response_bot? diff --git a/enterprise/app/services/enterprise/message_templates/response_bot_service.rb b/enterprise/app/services/enterprise/message_templates/response_bot_service.rb index 9df63f7e4..dca6f7279 100644 --- a/enterprise/app/services/enterprise/message_templates/response_bot_service.rb +++ b/enterprise/app/services/enterprise/message_templates/response_bot_service.rb @@ -80,8 +80,8 @@ class Enterprise::MessageTemplates::ResponseBotService case action when 'handoff' conversation.messages.create!('message_type': :outgoing, 'account_id': conversation.account_id, 'inbox_id': conversation.inbox_id, - 'content': 'passing to an agent') - conversation.update(status: :open) + 'content': 'Transferring to another agent for further assistance.') + conversation.bot_handoff! end end diff --git a/enterprise/app/services/features/response_bot_service.rb b/enterprise/app/services/features/response_bot_service.rb index da45cf48b..9162c085e 100644 --- a/enterprise/app/services/features/response_bot_service.rb +++ b/enterprise/app/services/features/response_bot_service.rb @@ -6,6 +6,11 @@ class Features::ResponseBotService create_tables end + def disable_in_installation + drop_tables + disable_vector_extension + end + def enable_vector_extension MIGRATION_VERSION.enable_extension 'vector' rescue ActiveRecord::StatementInvalid diff --git a/spec/enterprise/controllers/api/v1/accounts/response_sources_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/response_sources_controller_spec.rb index 44e0dced1..3d21a79a2 100644 --- a/spec/enterprise/controllers/api/v1/accounts/response_sources_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/response_sources_controller_spec.rb @@ -5,7 +5,7 @@ RSpec.describe 'Response Sources API', type: :request do let!(:admin) { create(:user, account: account, role: :administrator) } before do - skip('Skipping since vector is not enabled in this environment') unless Features::ResponseBotService.new.vector_extension_enabled? + skip_unless_response_bot_enabled_test_environment end describe 'POST /api/v1/accounts/{account.id}/response_sources/parse' do diff --git a/spec/enterprise/controllers/enterprise/api/v1/accounts/inboxes_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v1/accounts/inboxes_controller_spec.rb index 7783dc384..199ee7cd7 100644 --- a/spec/enterprise/controllers/enterprise/api/v1/accounts/inboxes_controller_spec.rb +++ b/spec/enterprise/controllers/enterprise/api/v1/accounts/inboxes_controller_spec.rb @@ -50,7 +50,7 @@ RSpec.describe 'Enterprise Inboxes API', type: :request do let(:administrator) { create(:user, account: account, role: :administrator) } before do - skip('Skipping since vector is not enabled in this environment') unless Features::ResponseBotService.new.vector_extension_enabled? + skip_unless_response_bot_enabled_test_environment end context 'when it is an unauthenticated user' do diff --git a/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb b/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb new file mode 100644 index 000000000..642752c5b --- /dev/null +++ b/spec/enterprise/jobs/enterprise/account/conversations_resolution_scheduler_job_spec.rb @@ -0,0 +1,29 @@ +require 'rails_helper' + +RSpec.describe Account::ConversationsResolutionSchedulerJob, type: :job do + let!(:account_with_bot) { create(:account) } + let!(:account_without_bot) { create(:account) } + let!(:inbox_with_bot) { create(:inbox, account: account_with_bot) } + let!(:inbox_without_bot) { create(:inbox, account: account_without_bot) } + let(:response_source) { create(:response_source, account: account_with_bot) } + + before do + skip_unless_response_bot_enabled_test_environment + account_with_bot.enable_features!(:response_bot) + create(:inbox_response_source, inbox: inbox_with_bot, response_source: response_source) + end + + describe '#perform' do + it 'enqueues resolution jobs only for inboxes with response bot enabled' do + expect do + described_class.perform_now + end.to have_enqueued_job(ResponseBot::InboxPendingConversationsResolutionJob).with(inbox_with_bot).and have_enqueued_job.exactly(:once) + end + + it 'does not enqueue resolution jobs for inboxes without response bot enabled' do + expect do + described_class.perform_now + end.not_to have_enqueued_job(ResponseBot::InboxPendingConversationsResolutionJob).with(inbox_without_bot) + end + end +end diff --git a/spec/enterprise/jobs/response_bot/inbox_pending_conversations_resolution_job_spec.rb b/spec/enterprise/jobs/response_bot/inbox_pending_conversations_resolution_job_spec.rb new file mode 100644 index 000000000..268d758cd --- /dev/null +++ b/spec/enterprise/jobs/response_bot/inbox_pending_conversations_resolution_job_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +RSpec.describe ResponseBot::InboxPendingConversationsResolutionJob, type: :job do + include ActiveJob::TestHelper + + let!(:inbox) { create(:inbox) } + let!(:resolvable_pending_conversation) { create(:conversation, inbox: inbox, last_activity_at: 2.hours.ago, status: :pending) } + let!(:recent_pending_conversation) { create(:conversation, inbox: inbox, last_activity_at: 10.minutes.ago, status: :pending) } + let!(:open_conversation) { create(:conversation, inbox: inbox, last_activity_at: 1.hour.ago, status: :open) } + + before do + stub_const('Limits::BULK_ACTIONS_LIMIT', 2) + end + + it 'queues the job' do + expect { described_class.perform_later(inbox) } + .to have_enqueued_job.on_queue('low') + end + + it 'resolves only the eligible pending conversations' do + perform_enqueued_jobs { described_class.perform_later(inbox) } + + expect(resolvable_pending_conversation.reload.status).to eq('resolved') + expect(recent_pending_conversation.reload.status).to eq('pending') + expect(open_conversation.reload.status).to eq('open') + end + + it 'creates an outgoing message for each resolved conversation' do + # resolution message + system message + expect { perform_enqueued_jobs { described_class.perform_later(inbox) } } + .to change { resolvable_pending_conversation.messages.reload.count }.by(2) + + resolved_conversation_messages = resolvable_pending_conversation.messages.map(&:content) + expect(resolved_conversation_messages).to include( + 'Resolving the conversation as it has been inactive for a while. Please start a new conversation if you need further assistance.' + ) + end +end diff --git a/spec/enterprise/services/enterprise/message_templates/response_bot_service_spec.rb b/spec/enterprise/services/enterprise/message_templates/response_bot_service_spec.rb index 42c62bc62..fc24a6379 100644 --- a/spec/enterprise/services/enterprise/message_templates/response_bot_service_spec.rb +++ b/spec/enterprise/services/enterprise/message_templates/response_bot_service_spec.rb @@ -8,9 +8,7 @@ RSpec.describe Enterprise::MessageTemplates::ResponseBotService, type: :service let(:response_object) { instance_double(Response, id: 1, question: 'Q1', answer: 'A1') } before do - # Uncomment if you want to run the spec in your local machine - # Features::ResponseBotService.new.enable_in_installation - skip('Skipping since vector is not enabled in this environment') unless Features::ResponseBotService.new.vector_extension_enabled? + skip_unless_response_bot_enabled_test_environment stub_request(:post, 'https://api.openai.com/v1/embeddings').to_return(status: 200, body: {}.to_json, headers: { Content_Type: 'application/json' }) create(:message, message_type: :incoming, conversation: conversation, content: 'Hi') @@ -33,6 +31,19 @@ RSpec.describe Enterprise::MessageTemplates::ResponseBotService, type: :service expect(last_message.content).to include(Response.first.question) expect(last_message.content).to include('**Sources**') end + + it 'hands off the conversation if the response is handoff' do + allow(chat_gpt_double).to receive(:generate_response).and_return({ 'response' => 'conversation_handoff' }) + expect(conversation).to receive(:bot_handoff!).and_call_original + + expect do + service.perform + end.to change { conversation.messages.where(message_type: :outgoing).count }.by(1) + + last_message = conversation.messages.last + expect(last_message.content).to eq('Transferring to another agent for further assistance.') + expect(conversation.status).to eq('open') + end end context 'when context_ids are not present' do @@ -67,12 +78,13 @@ RSpec.describe Enterprise::MessageTemplates::ResponseBotService, type: :service context 'when JSON::ParserError is raised' do it 'creates a handoff message' do allow(chat_gpt_double).to receive(:generate_response).and_raise(JSON::ParserError) + expect(conversation).to receive(:bot_handoff!).and_call_original expect do service.perform end.to change { conversation.messages.where(message_type: :outgoing).count }.by(1) - expect(conversation.messages.last.content).to eq('passing to an agent') + expect(conversation.messages.last.content).to eq('Transferring to another agent for further assistance.') expect(conversation.status).to eq('open') end end @@ -80,6 +92,7 @@ RSpec.describe Enterprise::MessageTemplates::ResponseBotService, type: :service context 'when StandardError is raised' do it 'captures the exception' do allow(chat_gpt_double).to receive(:generate_response).and_raise(StandardError) + expect(conversation).to receive(:bot_handoff!).and_call_original expect(ChatwootExceptionTracker).to receive(:new).and_call_original @@ -87,7 +100,7 @@ RSpec.describe Enterprise::MessageTemplates::ResponseBotService, type: :service service.perform end.to change { conversation.messages.where(message_type: :outgoing).count }.by(1) - expect(conversation.messages.last.content).to eq('passing to an agent') + expect(conversation.messages.last.content).to eq('Transferring to another agent for further assistance.') expect(conversation.status).to eq('open') end end diff --git a/spec/factories/inbox_response_source.rb b/spec/factories/inbox_response_source.rb new file mode 100644 index 000000000..d8b9d18c6 --- /dev/null +++ b/spec/factories/inbox_response_source.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :inbox_response_source do + inbox + response_source + end +end diff --git a/spec/listeners/reporting_event_listener_spec.rb b/spec/listeners/reporting_event_listener_spec.rb index 9577f87b2..86647eb87 100644 --- a/spec/listeners/reporting_event_listener_spec.rb +++ b/spec/listeners/reporting_event_listener_spec.rb @@ -32,6 +32,37 @@ describe ReportingEventListener do expect(account.reporting_events.where(name: 'conversation_resolved')[0]['value_in_business_hours']).to be 144_000.0 end end + + describe 'conversation_bot_resolved' do + # create an agent bot + let!(:agent_bot_inbox) { create(:inbox, account: account) } + let!(:agent_bot) { create(:agent_bot, account: account) } + let!(:bot_resolved_conversation) { create(:conversation, account: account, inbox: agent_bot_inbox, assignee: user) } + + before do + create(:agent_bot_inbox, agent_bot: agent_bot, inbox: agent_bot_inbox) + end + + it 'creates a conversation_bot_resolved event if resolved conversation does not have human interaction' do + event = Events::Base.new('conversation.resolved', Time.zone.now, conversation: bot_resolved_conversation) + listener.conversation_resolved(event) + expect(account.reporting_events.where(name: 'conversation_bot_resolved').count).to be 1 + end + + it 'does not create a conversation_bot_resolved event if resolved conversation inbox does not have active bot' do + bot_resolved_conversation.update(inbox: inbox) + event = Events::Base.new('conversation.resolved', Time.zone.now, conversation: bot_resolved_conversation) + listener.conversation_resolved(event) + expect(account.reporting_events.where(name: 'conversation_bot_resolved').count).to be 0 + end + + it 'does not create a conversation_bot_resolved event if resolved conversation has human interaction' do + create(:message, message_type: 'outgoing', account: account, inbox: agent_bot_inbox, conversation: bot_resolved_conversation) + event = Events::Base.new('conversation.resolved', Time.zone.now, conversation: bot_resolved_conversation) + listener.conversation_resolved(event) + expect(account.reporting_events.where(name: 'conversation_bot_resolved').count).to be 0 + end + end end describe '#reply_created' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3c275aeb4..3e18ec5df 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,4 +18,14 @@ RSpec.configure do |config| def with_modified_env(options, &) ClimateControl.modify(options, &) end + + def skip_unless_response_bot_enabled_test_environment + # Tests skipped using this method should be added to .github/workflows/run_response_bot_spec.yml + # Manage response bot tests in your local environment using the following commands: + # Enable response bot for tests + # RAILS_ENV=test bundle exec rails runner "Features::ResponseBotService.new.enable_in_installation" + # Disable response bot for tests + # RAILS_ENV=test bundle exec rails runner "Features::ResponseBotService.new.disable_in_installation" + skip('Skipping since vector is not enabled in this environment') unless Features::ResponseBotService.new.vector_extension_enabled? + end end