fix: bot handoff should set waiting time (#13417)
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
@@ -159,6 +159,7 @@ class Conversation < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
def bot_handoff!
|
def bot_handoff!
|
||||||
|
update(waiting_since: Time.current) if waiting_since.blank?
|
||||||
open!
|
open!
|
||||||
dispatcher_dispatch(CONVERSATION_BOT_HANDOFF)
|
dispatcher_dispatch(CONVERSATION_BOT_HANDOFF)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -42,10 +42,10 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob
|
|||||||
end
|
end
|
||||||
|
|
||||||
def process_response
|
def process_response
|
||||||
ActiveRecord::Base.transaction do
|
|
||||||
if handoff_requested?
|
if handoff_requested?
|
||||||
process_action('handoff')
|
process_action('handoff')
|
||||||
else
|
else
|
||||||
|
ActiveRecord::Base.transaction do
|
||||||
create_messages
|
create_messages
|
||||||
Rails.logger.info("[CAPTAIN][ResponseBuilderJob] Incrementing response usage for #{account.id}")
|
Rails.logger.info("[CAPTAIN][ResponseBuilderJob] Incrementing response usage for #{account.id}")
|
||||||
account.increment_response_usage
|
account.increment_response_usage
|
||||||
|
|||||||
@@ -92,6 +92,44 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Regression (PR #13417): wrapping create_handoff_message and bot_handoff! in the
|
||||||
|
# same transaction defers the message's after_create_commit until commit, at which
|
||||||
|
# point it clears waiting_since (bot_response). The handoff path must stay outside
|
||||||
|
# the transaction so the callback fires before bot_handoff! sets waiting_since.
|
||||||
|
context 'when handoff is requested' do
|
||||||
|
let(:conversation) { create(:conversation, inbox: inbox, account: account, status: :pending) }
|
||||||
|
let(:agent) { create(:user, account: account, role: :agent) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
allow(account).to receive(:feature_enabled?).and_return(false)
|
||||||
|
allow(account).to receive(:feature_enabled?).with('captain_integration_v2').and_return(false)
|
||||||
|
allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'conversation_handoff' })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'sets waiting_since to approximately the handoff time' do
|
||||||
|
freeze_time do
|
||||||
|
described_class.perform_now(conversation, assistant)
|
||||||
|
|
||||||
|
conversation.reload
|
||||||
|
expect(conversation.status).to eq('open')
|
||||||
|
expect(conversation.waiting_since).to be_within(1.second).of(Time.current)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'preserves waiting_since so a human reply consumes it for reply_time tracking' do
|
||||||
|
described_class.perform_now(conversation, assistant)
|
||||||
|
|
||||||
|
conversation.reload
|
||||||
|
expect(conversation.waiting_since).to be_present
|
||||||
|
|
||||||
|
# A human reply clears waiting_since (consumed by dispatch_create_events
|
||||||
|
# to emit FIRST_REPLY_CREATED or REPLY_CREATED for reply_time tracking).
|
||||||
|
create(:message, conversation: conversation, message_type: :outgoing,
|
||||||
|
sender: agent, account: account, inbox: inbox)
|
||||||
|
expect(conversation.reload.waiting_since).to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'when message contains an image' do
|
context 'when message contains an image' do
|
||||||
let(:message_with_image) { create(:message, conversation: conversation, message_type: :incoming, content: 'Can you help with this error?') }
|
let(:message_with_image) { create(:message, conversation: conversation, message_type: :incoming, content: 'Can you help with this error?') }
|
||||||
let(:image_attachment) { message_with_image.attachments.create!(account: account, file_type: :image, external_url: 'https://example.com/error.jpg') }
|
let(:image_attachment) { message_with_image.attachments.create!(account: account, file_type: :image, external_url: 'https://example.com/error.jpg') }
|
||||||
|
|||||||
@@ -313,6 +313,47 @@ RSpec.describe Conversation do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#bot_handoff!' do
|
||||||
|
let(:conversation) { create(:conversation, status: :pending) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
allow(Rails.configuration.dispatcher).to receive(:dispatch)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when waiting_since is blank' do
|
||||||
|
before { conversation.update(waiting_since: nil) }
|
||||||
|
|
||||||
|
it 'sets waiting_since to current time' do
|
||||||
|
freeze_time do
|
||||||
|
conversation.bot_handoff!
|
||||||
|
expect(conversation.reload.waiting_since).to eq(Time.current)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when waiting_since is already set' do
|
||||||
|
let(:original_time) { 1.hour.ago }
|
||||||
|
|
||||||
|
before { conversation.update(waiting_since: original_time) }
|
||||||
|
|
||||||
|
it 'preserves existing waiting_since' do
|
||||||
|
conversation.bot_handoff!
|
||||||
|
expect(conversation.reload.waiting_since).to be_within(1.second).of(original_time)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'changes status to open' do
|
||||||
|
conversation.bot_handoff!
|
||||||
|
expect(conversation.reload.status).to eq('open')
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'dispatches CONVERSATION_BOT_HANDOFF event' do
|
||||||
|
expect(Rails.configuration.dispatcher).to receive(:dispatch)
|
||||||
|
.with(described_class::CONVERSATION_BOT_HANDOFF, anything, hash_including(conversation: conversation))
|
||||||
|
conversation.bot_handoff!
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe '#toggle_priority' do
|
describe '#toggle_priority' do
|
||||||
it 'defaults priority to nil when created' do
|
it 'defaults priority to nil when created' do
|
||||||
conversation = create(:conversation, status: 'open')
|
conversation = create(:conversation, status: 'open')
|
||||||
|
|||||||
Reference in New Issue
Block a user