diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb index 871c55591..2149bd9e4 100644 --- a/app/finders/conversation_finder.rb +++ b/app/finders/conversation_finder.rb @@ -190,7 +190,7 @@ class ConversationFinder counts = @conversations.unscope(:order).pick( Arel.sql("COUNT(*) FILTER (WHERE assignee_id = #{current_user.id})"), - Arel.sql('COUNT(*) FILTER (WHERE assignee_id IS NULL)'), + Arel.sql('COUNT(*) FILTER (WHERE assignee_id IS NULL AND assignee_agent_bot_id IS NULL)'), Arel.sql('COUNT(*)') ) counts || [0, 0, 0] diff --git a/app/helpers/filters/filter_helper.rb b/app/helpers/filters/filter_helper.rb index d32c9468a..9f76cee98 100644 --- a/app/helpers/filters/filter_helper.rb +++ b/app/helpers/filters/filter_helper.rb @@ -89,9 +89,22 @@ module Filters::FilterHelper end def default_filter(query_hash, filter_operator_value) + if query_hash[:attribute_key] == 'assignee_id' && query_hash[:filter_operator].in?(%w[is_present is_not_present]) + return assignee_presence_filter(filter_config[:table_name], query_hash) + end + "#{filter_config[:table_name]}.#{query_hash[:attribute_key]} #{filter_operator_value} #{query_hash[:query_operator]}" end + # Assignee ownership can live in either column until it is standardized as a polymorphic association. + def assignee_presence_filter(table_name, query_hash) + if query_hash[:filter_operator] == 'is_present' + return "(#{table_name}.assignee_id IS NOT NULL OR #{table_name}.assignee_agent_bot_id IS NOT NULL) #{query_hash[:query_operator]}" + end + + "(#{table_name}.assignee_id IS NULL AND #{table_name}.assignee_agent_bot_id IS NULL) #{query_hash[:query_operator]}" + end + def text_search_on_display_id?(query_hash) query_hash[:attribute_key] == 'display_id' && %w[contains does_not_contain].include?(query_hash[:filter_operator]) end diff --git a/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js b/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js index 4603da83b..5355f0a7b 100644 --- a/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js +++ b/app/javascript/dashboard/store/modules/conversations/helpers/filterHelpers.js @@ -248,6 +248,24 @@ const matchesCondition = (conversationValue, filter) => { } }; +const matchesConversationCondition = (conversation, filter) => { + const isHumanAssigneeFilter = + filter.attribute_key === 'assignee_id' && + ['equal_to', 'not_equal_to'].includes(filter.filter_operator); + + if ( + isHumanAssigneeFilter && + conversation.meta?.assignee_type === 'AgentBot' + ) { + return false; + } + + return matchesCondition( + getValueFromConversation(conversation, filter.attribute_key), + filter + ); +}; + /** * Converts an array of evaluated filters into a JSON Logic rule * that respects SQL-like operator precedence (AND before OR) @@ -351,8 +369,7 @@ const buildJsonLogicRule = evaluatedFilters => { */ const evaluateFilters = (conversation, filters) => { return filters.map((filter, index) => { - const value = getValueFromConversation(conversation, filter.attribute_key); - const result = matchesCondition(value, filter); + const result = matchesConversationCondition(conversation, filter); // This part determines the logical operator that connects this filter to the next one: // - If this is not the last filter (index < filters.length - 1), use the filter's query_operator @@ -379,12 +396,7 @@ export const matchesFilters = (conversation, filters) => { // Handle single filter case if (filters.length === 1) { - const value = getValueFromConversation( - conversation, - filters[0].attribute_key - ); - - return matchesCondition(value, filters[0]); + return matchesConversationCondition(conversation, filters[0]); } // Evaluate all conditions and prepare for jsonLogic diff --git a/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js b/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js index 7b20b94e1..1662c9a81 100644 --- a/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js +++ b/app/javascript/dashboard/store/modules/conversations/helpers/specs/filterHelpers.spec.js @@ -192,6 +192,51 @@ describe('filterHelpers', () => { expect(matchesFilters(conversation, filters)).toBe(true); }); + it('should match an AgentBot-owned conversation when assignee is present', () => { + const conversation = { + meta: { assignee: { id: 1 }, assignee_type: 'AgentBot' }, + }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'is_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(true); + }); + + it('should not match an AgentBot-owned conversation to a human assignee id', () => { + const conversation = { + meta: { assignee: { id: 1 }, assignee_type: 'AgentBot' }, + }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'equal_to', + values: { id: 1, name: 'John Doe' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + + it('should not match an AgentBot-owned conversation to a human assignee not-equal filter', () => { + const conversation = { + meta: { assignee: { id: 1 }, assignee_type: 'AgentBot' }, + }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'not_equal_to', + values: { id: 1, name: 'John Doe' }, + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + it('should not match conversation with equal_to operator when assignee is null', () => { const conversation = { meta: { assignee: null } }; const filters = [ @@ -231,6 +276,21 @@ describe('filterHelpers', () => { expect(matchesFilters(conversation, filters)).toBe(true); }); + it('should not match an AgentBot-owned conversation when assignee is not present', () => { + const conversation = { + meta: { assignee: { id: 1 }, assignee_type: 'AgentBot' }, + }; + const filters = [ + { + attribute_key: 'assignee_id', + filter_operator: 'is_not_present', + values: [], + query_operator: 'and', + }, + ]; + expect(matchesFilters(conversation, filters)).toBe(false); + }); + it('should not match conversation with is_present operator when assignee is null', () => { const conversation = { meta: { assignee: null } }; const filters = [ diff --git a/app/models/concerns/assignment_handler.rb b/app/models/concerns/assignment_handler.rb index a9f529f65..1a8d49d07 100644 --- a/app/models/concerns/assignment_handler.rb +++ b/app/models/concerns/assignment_handler.rb @@ -11,6 +11,7 @@ module AssignmentHandler def ensure_assignee_is_from_team return unless team_id_changed? + return if assignee_agent_bot_id.present? validate_current_assignee_team self.assignee ||= find_assignee_from_team @@ -29,7 +30,7 @@ module AssignmentHandler def notify_assignment_change { - ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? }, + ASSIGNEE_CHANGED => -> { saved_change_to_assignee_id? || saved_change_to_assignee_agent_bot_id? }, TEAM_CHANGED => -> { saved_change_to_team_id? } }.each do |event, condition| condition.call && dispatcher_dispatch(event, previous_changes) diff --git a/app/models/concerns/auto_assignment_handler.rb b/app/models/concerns/auto_assignment_handler.rb index 1110cbd27..787524790 100644 --- a/app/models/concerns/auto_assignment_handler.rb +++ b/app/models/concerns/auto_assignment_handler.rb @@ -42,6 +42,7 @@ module AutoAssignmentHandler # Assignment V2: Resolved/snoozed conversations still have an assignee, so bypass the # assignee-blank check below. The AssignmentJob needs to run to rebalance assignments. return true if conversation_status_changed_to_resolved_or_snoozed? + return false if assignee_agent_bot_id.present? # run only if assignee is blank or doesn't have access to inbox assignee.blank? || inbox.members.exclude?(assignee) diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 38fb0063f..8ff548c32 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -85,8 +85,8 @@ class Conversation < ApplicationRecord enum status: { open: 0, resolved: 1, pending: 2, snoozed: 3 } enum priority: { low: 0, medium: 1, high: 2, urgent: 3 } - scope :unassigned, -> { where(assignee_id: nil) } - scope :assigned, -> { where.not(assignee_id: nil) } + scope :unassigned, -> { where(assignee_id: nil, assignee_agent_bot_id: nil) } + scope :assigned, -> { where.not(assignee_id: nil).or(where.not(assignee_agent_bot_id: nil)) } scope :assigned_to, ->(agent) { where(assignee_id: agent.id) } scope :sort_on_unread, lambda { |_direction| order(unread_messages_count_arel.desc).sort_on_last_activity_at('desc') diff --git a/app/services/auto_assignment/agent_assignment_service.rb b/app/services/auto_assignment/agent_assignment_service.rb index 3653449f1..0273df73b 100644 --- a/app/services/auto_assignment/agent_assignment_service.rb +++ b/app/services/auto_assignment/agent_assignment_service.rb @@ -27,6 +27,8 @@ class AutoAssignment::AgentAssignmentService private def reassignment_still_needed?(locked_conversation) + return false if locked_conversation.assignee_agent_bot_id.present? + locked_conversation.assignee.blank? || locked_conversation.inbox.members.exclude?(locked_conversation.assignee) end diff --git a/app/services/auto_assignment/assignment_service.rb b/app/services/auto_assignment/assignment_service.rb index b29f0ca02..1df9497af 100644 --- a/app/services/auto_assignment/assignment_service.rb +++ b/app/services/auto_assignment/assignment_service.rb @@ -111,7 +111,7 @@ class AutoAssignment::AssignmentService Conversation.transaction do locked = inbox.conversations - .where(id: conversation.id, assignee_id: nil) + .where(id: conversation.id).unassigned .lock('FOR UPDATE SKIP LOCKED') .first next false unless locked diff --git a/app/services/automation_rules/conditions_filter_service.rb b/app/services/automation_rules/conditions_filter_service.rb index 862faceac..88b802531 100644 --- a/app/services/automation_rules/conditions_filter_service.rb +++ b/app/services/automation_rules/conditions_filter_service.rb @@ -145,6 +145,11 @@ class AutomationRules::ConditionsFilterService < FilterService def conversation_query_string(table_name, current_filter, query_hash, current_index) attribute_key = query_hash['attribute_key'] query_operator = query_hash['query_operator'] + + if attribute_key == 'assignee_id' && query_hash['filter_operator'].in?(%w[is_present is_not_present]) + return assignee_presence_filter(table_name, query_hash) + end + filter_operator_value = filter_operation(query_hash, current_index) case current_filter['attribute_type'] diff --git a/app/services/conversations/unread_counts/builder.rb b/app/services/conversations/unread_counts/builder.rb index 54e466dbd..16582a015 100644 --- a/app/services/conversations/unread_counts/builder.rb +++ b/app/services/conversations/unread_counts/builder.rb @@ -28,6 +28,7 @@ class Conversations::UnreadCounts::Builder def write_memberships(assignment:) unread_conversations.in_batches(of: BATCH_SIZE) do |relation| + relation = relation.where(assignee_agent_bot_id: nil) if assignment columns = %i[id inbox_id assignee_id cached_label_list team_id] memberships = relation.pluck(*columns).map do |id, inbox_id, assignee_id, cached_label_list, team_id| { diff --git a/app/services/conversations/unread_counts/refresher.rb b/app/services/conversations/unread_counts/refresher.rb index 9e49ecd28..34f29b2a6 100644 --- a/app/services/conversations/unread_counts/refresher.rb +++ b/app/services/conversations/unread_counts/refresher.rb @@ -78,15 +78,17 @@ class Conversations::UnreadCounts::Refresher end def refresh_assignment_membership + conversation_id = conversation.id store.remove_assignment_membership( account_id: account.id, inbox_ids: affected_inbox_ids, label_ids: affected_label_ids, assignee_ids: affected_assignee_ids, team_ids: affected_team_ids, - conversation_id: conversation.id + conversation_id: conversation_id ) return unless unread? + return if conversation.assignee_agent_bot_id store.add_assignment_membership( account_id: account.id, @@ -94,7 +96,7 @@ class Conversations::UnreadCounts::Refresher label_ids: current_label_ids, assignee_id: conversation.assignee_id, team_id: conversation.team_id, - conversation_id: conversation.id + conversation_id: conversation_id ) end diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb index b41a2787a..4331f90c2 100644 --- a/app/services/filter_service.rb +++ b/app/services/filter_service.rb @@ -117,7 +117,7 @@ class FilterService def set_count_for_all_conversations counts = @conversations.except(:includes, :order).pick( Arel.sql(ActiveRecord::Base.sanitize_sql_array(['COUNT(*) FILTER (WHERE assignee_id = ?)', @user.id])), - Arel.sql('COUNT(*) FILTER (WHERE assignee_id IS NULL)'), + Arel.sql('COUNT(*) FILTER (WHERE assignee_id IS NULL AND assignee_agent_bot_id IS NULL)'), Arel.sql('COUNT(*)') ) # pick short-circuits to nil on a none relation (e.g. permission scope with no access) diff --git a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb index 521599e32..b8d9add16 100644 --- a/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/whatsapp_calls_controller.rb @@ -130,7 +130,7 @@ class Api::V1::Accounts::WhatsappCallsController < Api::V1::Accounts::BaseContro def create_outbound_call # A reused thread unassigned at click time is claimed for the caller (wins over auto-assignment); a # fresh thread (@conversation nil until the dial succeeds) is created already assigned to the caller. - claim_for_caller = @conversation.present? && @conversation.assignee_id.nil? + claim_for_caller = @conversation.present? && @conversation.assigned_entity.nil? result = provider_service.initiate_call(@contact.phone_number.delete('+'), params[:sdp_offer]) provider_call_id = result.dig('calls', 0, 'id') || result['call_id'] diff --git a/enterprise/app/policies/enterprise/conversation_policy.rb b/enterprise/app/policies/enterprise/conversation_policy.rb index d956db2d7..b2da80a3d 100644 --- a/enterprise/app/policies/enterprise/conversation_policy.rb +++ b/enterprise/app/policies/enterprise/conversation_policy.rb @@ -29,7 +29,7 @@ module Enterprise::ConversationPolicy end def unassigned_conversation? - record.assignee_id.nil? + record.assignee_id.nil? && record.assignee_agent_bot_id.nil? end def custom_role_permissions? diff --git a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb index 118ae3d14..bff6fcfbf 100644 --- a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb +++ b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb @@ -39,6 +39,7 @@ module Enterprise::Conversations::PermissionFilterService end def filter_unassigned_and_mine - accessible_conversations.where(assignee_id: [nil, user.id]) + conversations = accessible_conversations + conversations.unassigned.or(conversations.assigned_to(user)) end end diff --git a/enterprise/app/services/voice/conference/manager.rb b/enterprise/app/services/voice/conference/manager.rb index 8be2b656e..914d069be 100644 --- a/enterprise/app/services/voice/conference/manager.rb +++ b/enterprise/app/services/voice/conference/manager.rb @@ -52,7 +52,7 @@ class Voice::Conference::Manager def auto_assign_conversation!(user_id) conversation = call.conversation - return if conversation.assignee_id.present? + return if conversation.assigned_entity.present? Conversations::AssignmentService.new(conversation: conversation, assignee_id: user_id).perform end diff --git a/enterprise/app/services/voice/outbound_call_builder.rb b/enterprise/app/services/voice/outbound_call_builder.rb index c58407a3f..e8a1ccbcd 100644 --- a/enterprise/app/services/voice/outbound_call_builder.rb +++ b/enterprise/app/services/voice/outbound_call_builder.rb @@ -19,7 +19,7 @@ class Voice::OutboundCallBuilder # Claim for the caller if a reused conversation is unassigned at trigger time; wins over auto-assignment. # New conversations set the assignee at creation instead (see create_conversation!). - claim_for_caller = @existing_conversation && @existing_conversation.assignee_id.nil? + claim_for_caller = @existing_conversation && @existing_conversation.assigned_entity.nil? ActiveRecord::Base.transaction do contact_inbox = ensure_contact_inbox! diff --git a/enterprise/app/services/voice/provider/twilio/conference_service.rb b/enterprise/app/services/voice/provider/twilio/conference_service.rb index 0ee368d00..ad8a6d1e5 100644 --- a/enterprise/app/services/voice/provider/twilio/conference_service.rb +++ b/enterprise/app/services/voice/provider/twilio/conference_service.rb @@ -44,7 +44,7 @@ class Voice::Provider::Twilio::ConferenceService # (e.g., lock_to_single_conversation) shouldn't be stomped on pickup. def assign_conversation!(user) conversation = call.conversation - return if conversation.assignee_id.present? + return if conversation.assigned_entity.present? Conversations::AssignmentService.new(conversation: conversation, assignee_id: user.id).perform end diff --git a/enterprise/app/services/whatsapp/call_service.rb b/enterprise/app/services/whatsapp/call_service.rb index bd0dd6ae5..08178ffa5 100644 --- a/enterprise/app/services/whatsapp/call_service.rb +++ b/enterprise/app/services/whatsapp/call_service.rb @@ -69,7 +69,7 @@ class Whatsapp::CallService def claim_conversation_and_set_call_status conversation = call.conversation attrs = { additional_attributes: (conversation.additional_attributes || {}).merge('call_status' => call.display_status) } - attrs[:assignee] = agent if conversation.assignee_id.blank? + attrs[:assignee] = agent if conversation.assigned_entity.nil? conversation.update!(attrs) end diff --git a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb index a66249d5a..94c5b86d0 100644 --- a/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/whatsapp_calls_controller_spec.rb @@ -138,6 +138,19 @@ RSpec.describe 'WhatsApp Calls API', type: :request do expect(initiate_conversation.reload.assignee_id).to eq(other_agent.id) end + it 'keeps the AgentBot owner when the conversation is already assigned' do + agent_bot = create(:agent_bot, account: account) + initiate_conversation.update!(assignee_agent_bot: agent_bot) + allow(provider_service).to receive(:initiate_call).and_return({ 'calls' => [{ 'id' => 'wacid_outbound' }] }) + + post "/api/v1/accounts/#{account.id}/whatsapp_calls/initiate", + params: { conversation_id: initiate_conversation.display_id, sdp_offer: 'sdp_offer' }, + headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(initiate_conversation.reload.assigned_entity).to eq(agent_bot) + end + it 'sends a permission request and records the wamid when Meta returns NoCallPermission' do allow(provider_service).to receive(:initiate_call).and_raise(Voice::CallErrors::NoCallPermission) allow(provider_service).to receive(:send_call_permission_request).and_return({ 'messages' => [{ 'id' => 'wamid.req_xyz' }] }) diff --git a/spec/enterprise/policies/conversation_policy_spec.rb b/spec/enterprise/policies/conversation_policy_spec.rb index e48a84852..5875578d6 100644 --- a/spec/enterprise/policies/conversation_policy_spec.rb +++ b/spec/enterprise/policies/conversation_policy_spec.rb @@ -33,6 +33,12 @@ RSpec.describe ConversationPolicy, type: :policy do expect(subject).not_to permit(context, conversation) end + + it 'denies access to conversations assigned to an agent bot' do + conversation = create(:conversation, account: account, inbox: inbox, assignee_agent_bot: create(:agent_bot, account: account)) + + expect(subject).not_to permit(context, conversation) + end end context 'when role grants conversation_participating_manage' do diff --git a/spec/enterprise/services/conversations/unread_counts/counter_spec.rb b/spec/enterprise/services/conversations/unread_counts/counter_spec.rb index e8f850e78..b91a69c1b 100644 --- a/spec/enterprise/services/conversations/unread_counts/counter_spec.rb +++ b/spec/enterprise/services/conversations/unread_counts/counter_spec.rb @@ -38,6 +38,8 @@ RSpec.describe Conversations::UnreadCounts::Counter do create_unread_conversation(account: account, inbox: inbox, labels: [label.title], assignee: agent, team: team) create_unread_conversation(account: account, inbox: inbox, labels: [label.title], team: team) create_unread_conversation(account: account, inbox: inbox, labels: [label.title], assignee: other_agent, team: team) + agent_bot_conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title], team: team) + agent_bot_conversation.update!(assignee_agent_bot: create(:agent_bot, account: account)) result = described_class.new(account: account, user: agent).perform diff --git a/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb b/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb index 08d900b6e..13a015618 100644 --- a/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb +++ b/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb @@ -145,6 +145,8 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create some conversations assigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: test_agent) unassigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: nil) + agent_bot_conversation = create(:conversation, account: test_account, inbox: test_inbox, + assignee_agent_bot: create(:agent_bot, account: test_account)) other_assigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: create(:user, account: test_account)) other_inbox_conversation = create(:conversation, account: test_account, inbox: test_inbox2, assignee: nil) @@ -161,6 +163,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result).to include(assigned_conversation) # Should NOT include conversations assigned to others + expect(result).not_to include(agent_bot_conversation) expect(result).not_to include(other_assigned_conversation) expect(result).not_to include(other_inbox_conversation) end diff --git a/spec/enterprise/services/voice/outbound_call_builder_spec.rb b/spec/enterprise/services/voice/outbound_call_builder_spec.rb index 0dc565eaf..fc52de3b6 100644 --- a/spec/enterprise/services/voice/outbound_call_builder_spec.rb +++ b/spec/enterprise/services/voice/outbound_call_builder_spec.rb @@ -92,6 +92,15 @@ RSpec.describe Voice::OutboundCallBuilder do expect(conversation.reload.assignee_id).to eq(other_agent.id) end + it 'keeps the AgentBot owner when a reused conversation is already assigned' do + agent_bot = create(:agent_bot, account: account) + conversation = create(:conversation, account: account, inbox: inbox, contact: contact, assignee_agent_bot: agent_bot).reload + + described_class.perform!(account: account, inbox: inbox, user: user, contact: contact, conversation: conversation) + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + end + it 'does not set conversation.identifier or write call state to additional_attributes' do call = described_class.perform!( account: account, diff --git a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb index 519519eed..686b96386 100644 --- a/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb +++ b/spec/enterprise/services/voice/provider/twilio/conference_service_spec.rb @@ -43,6 +43,16 @@ describe Voice::Provider::Twilio::ConferenceService do expect(call.reload.accepted_by_agent_id).to eq(agent.id) end + + it 'keeps an existing AgentBot conversation owner' do + agent = create(:user, account: account) + agent_bot = create(:agent_bot, account: account) + conversation.update!(assignee_agent_bot: agent_bot) + + service.mark_agent_joined(user: agent) + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + end end describe '#end_conference' do diff --git a/spec/enterprise/services/whatsapp/call_service_spec.rb b/spec/enterprise/services/whatsapp/call_service_spec.rb index 4620ca588..09e5e8bd7 100644 --- a/spec/enterprise/services/whatsapp/call_service_spec.rb +++ b/spec/enterprise/services/whatsapp/call_service_spec.rb @@ -50,6 +50,15 @@ describe Whatsapp::CallService do expect(conversation.reload.assignee_id).to eq(agent.id) end + it 'keeps the AgentBot owner when accepting the call' do + agent_bot = create(:agent_bot, account: account) + conversation.update!(assignee_agent_bot: agent_bot) + + described_class.new(call: call, agent: agent, sdp_answer: sdp_answer).accept + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + end + it 'raises AlreadyAccepted when another agent has already accepted the call' do call.update!(status: 'in_progress') diff --git a/spec/finders/conversation_finder_spec.rb b/spec/finders/conversation_finder_spec.rb index c502336c0..1a41eb898 100644 --- a/spec/finders/conversation_finder_spec.rb +++ b/spec/finders/conversation_finder_spec.rb @@ -80,10 +80,14 @@ describe ConversationFinder do context 'with assignee_type unassigned' do let(:params) { { assignee_type: 'unassigned' } } + let!(:agent_bot_conversation) do + create(:conversation, account: account, inbox: inbox, assignee_agent_bot: create(:agent_bot, account: account)) + end it 'filter conversations by assignee type unassigned' do result = conversation_finder.perform expect(result[:conversations].length).to be 1 + expect(result[:conversations]).not_to include(agent_bot_conversation) end end @@ -159,19 +163,23 @@ describe ConversationFinder do context 'with assignee_type assigned' do let(:params) { { assignee_type: 'assigned' } } + let!(:agent_bot_conversation) do + create(:conversation, account: account, inbox: inbox, assignee_agent_bot: create(:agent_bot, account: account)) + end it 'filter conversations by assignee type assigned' do result = conversation_finder.perform - expect(result[:conversations].length).to be 3 + expect(result[:conversations].length).to be 4 + expect(result[:conversations]).to include(agent_bot_conversation) end it 'returns the correct meta' do result = conversation_finder.perform expect(result[:count]).to eq({ mine_count: 2, - assigned_count: 3, + assigned_count: 4, unassigned_count: 1, - all_count: 4 + all_count: 5 }) end end diff --git a/spec/models/concerns/assignment_handler_shared.rb b/spec/models/concerns/assignment_handler_shared.rb index 8447b8146..3b6c0485e 100644 --- a/spec/models/concerns/assignment_handler_shared.rb +++ b/spec/models/concerns/assignment_handler_shared.rb @@ -50,6 +50,17 @@ shared_examples_for 'assignment_handler' do content: "Assigned to #{conversation.assignee.name} via #{team.name} by #{agent.name}" })) end + it 'keeps AgentBot ownership when assigning an auto-assigning team' do + team.update!(allow_auto_assign: true) + agent_bot = create(:agent_bot, account: conversation.account) + conversation.update!(assignee: nil, assignee_agent_bot: agent_bot) + + conversation.update!(team: team) + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + expect(conversation.assignee).to be_nil + end + it 'wont change assignee if he is already a team member' do team.update!(allow_auto_assign: true) assignee = create(:user, account: conversation.account, role: :agent) diff --git a/spec/models/concerns/auto_assignment_handler_shared.rb b/spec/models/concerns/auto_assignment_handler_shared.rb index 90c9c9d20..1e381d3c2 100644 --- a/spec/models/concerns/auto_assignment_handler_shared.rb +++ b/spec/models/concerns/auto_assignment_handler_shared.rb @@ -45,6 +45,15 @@ shared_examples_for 'auto_assignment_handler' do expect(conversation.reload.assignee).to be_nil end + it 'keeps AgentBot ownership when the conversation opens' do + agent_bot = create(:agent_bot, account: account) + conversation = create(:conversation, account: account, inbox: inbox, status: 'pending', assignee_agent_bot: agent_bot) + + conversation.update!(status: 'open') + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + end + it 'gets triggered on update only when status changes to open' do conversation.status = 'resolved' conversation.save! diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 29e00c204..c2f8b5034 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -194,6 +194,17 @@ RSpec.describe Conversation do changed_attributes: changed_attributes, performed_by: nil) end + it 'dispatches an assignee changed event when an agent bot is assigned' do + conversation = create(:conversation, status: 'open', account: account) + agent_bot = create(:agent_bot, account: account) + + conversation.update!(assignee_agent_bot: agent_bot) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch) + .with(described_class::ASSIGNEE_CHANGED, kind_of(Time), conversation: conversation, notifiable_assignee_change: false, + changed_attributes: conversation.previous_changes, performed_by: nil) + end + it 'will not run conversation_updated event for empty updates' do conversation.save! expect(Rails.configuration.dispatcher).not_to have_received(:dispatch) diff --git a/spec/services/auto_assignment/agent_assignment_service_spec.rb b/spec/services/auto_assignment/agent_assignment_service_spec.rb index 16337dc04..5312ee04e 100644 --- a/spec/services/auto_assignment/agent_assignment_service_spec.rb +++ b/spec/services/auto_assignment/agent_assignment_service_spec.rb @@ -26,6 +26,15 @@ RSpec.describe AutoAssignment::AgentAssignmentService do described_class.new(conversation: conversation, allowed_agent_ids: inbox_members.map(&:user_id).map(&:to_s)).perform expect(conversation.reload.assignee).not_to be_nil end + + it 'keeps an existing AgentBot owner' do + agent_bot = create(:agent_bot, account: account) + conversation.update!(assignee_agent_bot: agent_bot) + + described_class.new(conversation: conversation, allowed_agent_ids: inbox_members.map(&:user_id).map(&:to_s)).perform + + expect(conversation.reload.assigned_entity).to eq(agent_bot) + end end describe '#find_assignee' do diff --git a/spec/services/auto_assignment/assignment_service_spec.rb b/spec/services/auto_assignment/assignment_service_spec.rb index 44bbf89b1..35863e0ec 100644 --- a/spec/services/auto_assignment/assignment_service_spec.rb +++ b/spec/services/auto_assignment/assignment_service_spec.rb @@ -107,6 +107,18 @@ RSpec.describe AutoAssignment::AssignmentService do expect(unassigned_conversation.reload.assignee).to eq(agent) end + it 'does not reassign conversations owned by an agent bot' do + agent_bot = create(:agent_bot, account: account) + agent_bot_conversation = create(:conversation, inbox: inbox, status: 'open', assignee_agent_bot: agent_bot) + allow(service).to receive(:unassigned_conversations).and_return([agent_bot_conversation]) + + assigned_count = service.perform_bulk_assignment(limit: 1) + + expect(assigned_count).to eq(0) + expect(agent_bot_conversation.reload.assignee_agent_bot).to eq(agent_bot) + expect(agent_bot_conversation.assignee).to be_nil + end + it 'dispatches assignee changed event' do conversation # ensure it exists conversation.update!(assignee_id: nil) diff --git a/spec/services/automation_rules/conditions_filter_service_spec.rb b/spec/services/automation_rules/conditions_filter_service_spec.rb index c4ff81275..f272d448a 100644 --- a/spec/services/automation_rules/conditions_filter_service_spec.rb +++ b/spec/services/automation_rules/conditions_filter_service_spec.rb @@ -63,6 +63,26 @@ RSpec.describe AutomationRules::ConditionsFilterService do end end + context 'when conditions check assignee presence' do + let(:agent_bot) { create(:agent_bot, account: account) } + + before do + conversation.update!(assignee_agent_bot: agent_bot) + end + + it 'treats AgentBot ownership as present' do + rule.update!(conditions: [{ 'values': [], 'attribute_key': 'assignee_id', 'query_operator': nil, 'filter_operator': 'is_present' }]) + + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(true) + end + + it 'does not treat AgentBot ownership as absent' do + rule.update!(conditions: [{ 'values': [], 'attribute_key': 'assignee_id', 'query_operator': nil, 'filter_operator': 'is_not_present' }]) + + expect(described_class.new(rule, conversation, { changed_attributes: {} }).perform).to be(false) + end + end + context 'when conditions based on messages attributes' do context 'when filter_operator is equal_to' do before do diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index d307e1a28..5dc1a35f4 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -356,6 +356,47 @@ describe Conversations::FilterService do expect(result[:conversations].pluck(:campaign_id).sort).to eq [campaign_2.id, campaign_1.id].sort end + it 'treats AgentBot-owned conversations as having an assignee' do + account.conversations.destroy_all + agent_bot = create(:agent_bot, account: account) + bot_owned_conversation = create(:conversation, account: account, inbox: inbox, assignee_agent_bot: agent_bot) + human_owned_conversation = create(:conversation, account: account, inbox: inbox, assignee: user_1) + create(:conversation, account: account, inbox: inbox) + + params[:payload] = [{ + attribute_key: 'assignee_id', + filter_operator: 'is_present', + values: [], + query_operator: nil, + custom_attribute_type: '' + }.with_indifferent_access] + + result = filter_service.new(params, user_1, account).perform + + expect(result[:conversations].pluck(:id)).to contain_exactly(bot_owned_conversation.id, human_owned_conversation.id) + expect(result[:count]).to include(assigned_count: 2, unassigned_count: 0, all_count: 2) + end + + it 'excludes AgentBot-owned conversations from assignee is not present' do + account.conversations.destroy_all + agent_bot = create(:agent_bot, account: account) + create(:conversation, account: account, inbox: inbox, assignee_agent_bot: agent_bot) + unassigned_conversation = create(:conversation, account: account, inbox: inbox) + + params[:payload] = [{ + attribute_key: 'assignee_id', + filter_operator: 'is_not_present', + values: [], + query_operator: nil, + custom_attribute_type: '' + }.with_indifferent_access] + + result = filter_service.new(params, user_1, account).perform + + expect(result[:conversations].pluck(:id)).to contain_exactly(unassigned_conversation.id) + expect(result[:count]).to include(assigned_count: 0, unassigned_count: 1, all_count: 1) + end + it 'handles invalid query conditions' do params[:payload] = [ { @@ -751,6 +792,20 @@ describe Conversations::FilterService do ) end + it 'counts conversations owned by an agent bot as assigned' do + create(:conversation, account: account, inbox: inbox, assignee_agent_bot: create(:agent_bot, account: account)) + params[:payload] = payload + + result = filter_service.new(params, user_1, account).perform + + expect(result[:count]).to eq( + mine_count: 3, + assigned_count: 5, + unassigned_count: 1, + all_count: 6 + ) + end + it 'returns zero counts when the permission scope resolves to no conversations' do params[:payload] = payload permission_filter = instance_double(Conversations::PermissionFilterService, perform: Conversation.none) diff --git a/spec/services/conversations/unread_counts/builder_spec.rb b/spec/services/conversations/unread_counts/builder_spec.rb index 4b3aec230..23fb69a6e 100644 --- a/spec/services/conversations/unread_counts/builder_spec.rb +++ b/spec/services/conversations/unread_counts/builder_spec.rb @@ -54,6 +54,8 @@ RSpec.describe Conversations::UnreadCounts::Builder do team: team ) unassigned_conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title], team: team) + agent_bot_conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title], team: team) + agent_bot_conversation.update!(assignee_agent_bot: create(:agent_bot, account: account)) described_class.new(account).build_assignment! diff --git a/spec/services/conversations/unread_counts/refresher_spec.rb b/spec/services/conversations/unread_counts/refresher_spec.rb index 5f361e7aa..92e397376 100644 --- a/spec/services/conversations/unread_counts/refresher_spec.rb +++ b/spec/services/conversations/unread_counts/refresher_spec.rb @@ -156,6 +156,19 @@ RSpec.describe Conversations::UnreadCounts::Refresher do ) end + it 'removes assignment-aware unassigned membership when an agent bot is assigned' do + conversation = create_unread_conversation(account: account, inbox: inbox) + Conversations::UnreadCounts::Builder.new(account).build_assignment! + agent_bot = create(:agent_bot, account: account) + + conversation.update!(assignee_agent_bot: agent_bot) + result = described_class.new(conversation.reload, changed_attributes: { assignee_agent_bot_id: [nil, agent_bot.id] }).perform + + key = store.inbox_unassigned_key(account.id, inbox.id) + expect(result).to be(true) + expect(store.counts_for_keys([key])).to eq(key => 0) + end + it 'moves assignment-aware team membership when team changes' do create(:team_member, user: assignee, team: new_team) conversation = create_unread_conversation(account: account, inbox: inbox, assignee: assignee, team: team)