diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 38e8d94aa..1b3933d99 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -80,7 +80,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def toggle_status # FIXME: move this logic into a service object - if pending_to_open_by_bot? + if bot_handoff? @conversation.bot_handoff! elsif params[:status].present? set_conversation_status @@ -88,19 +88,15 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro else @status = @conversation.toggle_status end - assign_conversation if should_assign_conversation? + handle_human_open if @conversation.open? && Current.user.is_a?(User) end - def pending_to_open_by_bot? + def bot_handoff? return false unless Current.user.is_a?(AgentBot) @conversation.status == 'pending' && params[:status] == 'open' end - def should_assign_conversation? - @conversation.status == 'open' && Current.user.is_a?(User) && Current.user&.agent? - end - def toggle_priority @conversation.toggle_priority(params[:priority]) head :ok @@ -183,8 +179,9 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro @conversation.snoozed_until = parse_date_time(params[:snoozed_until].to_s) if params[:snoozed_until] end - def assign_conversation - @conversation.assignee = current_user + def handle_human_open + @conversation.assignee_agent_bot = nil + @conversation.assignee = Current.user if Current.user.agent? @conversation.save! end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 1e9c38c41..f629658d5 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -173,6 +173,7 @@ class Conversation < ApplicationRecord def bot_handoff! update(waiting_since: Time.current) if waiting_since.blank? + self.assignee_agent_bot = nil open! dispatcher_dispatch(CONVERSATION_BOT_HANDOFF) end @@ -291,13 +292,19 @@ class Conversation < ApplicationRecord return handle_campaign_status if campaign.present? - # TODO: make this an inbox config instead of assuming bot conversations should start as pending - self.status = :pending if inbox.active_bot? + set_active_bot_conversation if inbox.active_bot? end def handle_campaign_status - # If campaign has no sender (bot-initiated) and inbox has active bot, let bot handle it - self.status = :pending if campaign.sender_id.nil? && inbox.active_bot? + set_active_bot_conversation if campaign.sender_id.nil? && inbox.active_bot? + end + + def set_active_bot_conversation + # TODO: make this an inbox config instead of assuming bot conversations should start as pending + self.status = :pending + return unless inbox.agent_bot_inbox&.active? && assignee_id.blank? + + self.assignee_agent_bot = inbox.agent_bot end def notify_conversation_creation diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index b0eddd639..ea56f906a 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -565,15 +565,17 @@ RSpec.describe 'Conversations API', type: :request do expect(conversation.reload.assignee_id).to eq(agent.id) end - it 'disbale self assign if admin changes the conversation status to open' do - conversation.update!(status: 'pending') - conversation.update!(assignee_id: nil) + it 'does not self assign and clears the agent bot owner if admin changes the conversation status to open' do + conversation.update!(status: 'pending', assignee: nil, assignee_agent_bot: agent_bot) + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/toggle_status", headers: administrator.create_new_auth_token, as: :json + expect(response).to have_http_status(:success) expect(conversation.reload.status).to eq('open') expect(conversation.reload.assignee_id).not_to eq(administrator.id) + expect(conversation.reload.assignee_agent_bot).to be_nil end it 'toggles the conversation status to specific status when parameter is passed' do diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index c67aa0604..15e3ebc10 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -408,6 +408,14 @@ RSpec.describe Conversation do expect(conversation.reload.status).to eq('open') end + it 'clears agent bot ownership' do + conversation.update!(assignee_agent_bot: create(:agent_bot, account: conversation.account)) + + conversation.bot_handoff! + + expect(conversation.reload.assignee_agent_bot).to be_nil + 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)) @@ -719,6 +727,19 @@ RSpec.describe Conversation do expect(conversation.status).to eq('pending') end + it 'sets connected agent bot as the conversation owner' do + expect(conversation.assignee_agent_bot).to eq(bot_inbox.agent_bot) + expect(conversation.assignee).to be_nil + end + + it 'preserves explicit human assignee' do + agent = create(:user, account: bot_inbox.inbox.account) + conversation = create(:conversation, inbox: bot_inbox.inbox, assignee: agent) + + expect(conversation.assignee).to eq(agent) + expect(conversation.assignee_agent_bot).to be_nil + end + context 'with campaigns' do let(:user) { create(:user, account: bot_inbox.inbox.account) } @@ -726,12 +747,14 @@ RSpec.describe Conversation do campaign = create(:campaign, inbox: bot_inbox.inbox, account: bot_inbox.inbox.account, sender: user) conversation = create(:conversation, inbox: bot_inbox.inbox, campaign: campaign) expect(conversation.status).to eq('open') + expect(conversation.assignee_agent_bot).to be_nil end it 'returns conversation as pending if campaign has no sender (bot-initiated) and bot is active' do campaign = create(:campaign, inbox: bot_inbox.inbox, account: bot_inbox.inbox.account, sender: nil) conversation = create(:conversation, inbox: bot_inbox.inbox, campaign: campaign) expect(conversation.status).to eq('pending') + expect(conversation.assignee_agent_bot).to eq(bot_inbox.agent_bot) end end @@ -762,6 +785,10 @@ RSpec.describe Conversation do it 'returns conversation status as pending' do expect(conversation.status).to eq('pending') end + + it 'does not set agent bot ownership' do + expect(conversation.assignee_agent_bot).to be_nil + end end describe '#delete conversation' do