diff --git a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb index fda19b8c2..20d66fb4d 100644 --- a/app/controllers/api/v1/accounts/contacts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/contacts/conversations_controller.rb @@ -12,10 +12,6 @@ class Api::V1::Accounts::Contacts::ConversationsController < Api::V1::Accounts:: Current.account ).perform - # Only allow conversations from inboxes the user has access to - inbox_ids = Current.user.assigned_inboxes.pluck(:id) - conversations = conversations.where(inbox_id: inbox_ids) - @conversations = conversations.order(last_activity_at: :desc).limit(20) end end diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb index 1694199ba..d43ed31e7 100644 --- a/app/finders/conversation_finder.rb +++ b/app/finders/conversation_finder.rb @@ -88,7 +88,10 @@ class ConversationFinder def find_conversation_by_inbox @conversations = current_account.conversations - @conversations = @conversations.where(inbox_id: @inbox_ids) unless params[:inbox_id].blank? && @is_admin + + return unless params[:inbox_id] + + @conversations = @conversations.where(inbox_id: @inbox_ids) end def find_all_conversations diff --git a/app/services/conversations/filter_service.rb b/app/services/conversations/filter_service.rb index 21fe3c716..db2892c31 100644 --- a/app/services/conversations/filter_service.rb +++ b/app/services/conversations/filter_service.rb @@ -28,16 +28,6 @@ class Conversations::FilterService < FilterService :taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :messages, :contact_inbox ) - account_user = @account.account_users.find_by(user_id: @user.id) - is_administrator = account_user&.role == 'administrator' - - # Ensure we only include conversations from inboxes the user has access to - unless is_administrator - inbox_ids = @user.inboxes.where(account_id: @account.id).pluck(:id) - conversations = conversations.where(inbox_id: inbox_ids) - end - - # Apply permission-based filtering Conversations::PermissionFilterService.new( conversations, @user, diff --git a/app/services/conversations/permission_filter_service.rb b/app/services/conversations/permission_filter_service.rb index a8561dbdf..31dd011d7 100644 --- a/app/services/conversations/permission_filter_service.rb +++ b/app/services/conversations/permission_filter_service.rb @@ -8,9 +8,23 @@ class Conversations::PermissionFilterService end def perform - # The base implementation simply returns all conversations - # Enterprise edition extends this with permission-based filtering - conversations + return conversations if user_role == 'administrator' + + accessible_conversations + end + + private + + def accessible_conversations + conversations.where(inbox: user.inboxes.where(account_id: account.id)) + end + + def account_user + AccountUser.find_by(account_id: account.id, user_id: user.id) + end + + def user_role + account_user&.role end end diff --git a/enterprise/app/services/captain/tools/base_service.rb b/enterprise/app/services/captain/tools/base_service.rb index 7fadc806b..9143726b7 100644 --- a/enterprise/app/services/captain/tools/base_service.rb +++ b/enterprise/app/services/captain/tools/base_service.rb @@ -1,8 +1,9 @@ class Captain::Tools::BaseService attr_accessor :assistant - def initialize(assistant) + def initialize(assistant, user: nil) @assistant = assistant + @user = user end def name diff --git a/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb b/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb new file mode 100644 index 000000000..6b6b063ea --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb @@ -0,0 +1,72 @@ +class Captain::Tools::Copilot::SearchConversationsService < Captain::Tools::BaseService + def name + 'search_conversations' + end + + def description + 'Search conversations based on parameters' + end + + def parameters + { + type: 'object', + properties: properties, + required: [] + } + end + + def execute(arguments) + status = arguments['status'] + contact_id = arguments['contact_id'] + priority = arguments['priority'] + + conversations = get_conversations(status, contact_id, priority) + + return 'No conversations found' unless conversations.exists? + + total_count = conversations.count + conversations = conversations.limit(100) + + <<~RESPONSE + #{total_count > 100 ? "Found #{total_count} conversations (showing first 100)" : "Total number of conversations: #{total_count}"} + #{conversations.map { |conversation| conversation.to_llm_text(include_contact_details: true) }.join("\n---\n")} + RESPONSE + end + + private + + def get_conversations(status, contact_id, priority) + conversations = permissible_conversations + conversations = conversations.where(contact_id: contact_id) if contact_id.present? + conversations = conversations.where(status: status) if status.present? + conversations = conversations.where(priority: priority) if priority.present? + conversations + end + + def permissible_conversations + Conversations::PermissionFilterService.new( + @assistant.account.conversations, + @user, + @assistant.account + ).perform + end + + def properties + { + contact_id: { + type: 'number', + description: 'Filter conversations by contact ID' + }, + status: { + type: 'string', + enum: %w[open resolved pending snoozed], + description: 'Filter conversations by status' + }, + priority: { + type: 'string', + enum: %w[low medium high urgent], + description: 'Filter conversations by priority' + } + } + end +end diff --git a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb index 5da65ffa6..f55265a90 100644 --- a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb +++ b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb @@ -1,36 +1,37 @@ module Enterprise::Conversations::PermissionFilterService def perform - account_user = AccountUser.find_by(account_id: account.id, user_id: user.id) - permissions = account_user&.permissions || [] - user_role = account_user&.role + return filter_by_permissions(permissions) if user_has_custom_role? - # Skip filtering for administrators - return conversations if user_role == 'administrator' - # Skip filtering for regular agents (without custom roles/permissions) - return conversations if user_role == 'agent' && account_user&.custom_role_id.nil? - - filter_by_permissions(permissions) + super end private + def user_has_custom_role? + user_role == 'agent' && account_user&.custom_role_id.present? + end + + def permissions + account_user&.permissions || [] + end + def filter_by_permissions(permissions) # Permission-based filtering with hierarchy # conversation_manage > conversation_unassigned_manage > conversation_participating_manage if permissions.include?('conversation_manage') - conversations + accessible_conversations elsif permissions.include?('conversation_unassigned_manage') filter_unassigned_and_mine elsif permissions.include?('conversation_participating_manage') - conversations.assigned_to(user) + accessible_conversations.assigned_to(user) else Conversation.none end end def filter_unassigned_and_mine - mine = conversations.assigned_to(user) - unassigned = conversations.unassigned + mine = accessible_conversations.assigned_to(user) + unassigned = accessible_conversations.unassigned Conversation.from("(#{mine.to_sql} UNION #{unassigned.to_sql}) as conversations") .where(account_id: account.id) diff --git a/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb new file mode 100644 index 000000000..e60ad6f28 --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb @@ -0,0 +1,67 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::SearchConversationsService do + let(:account) { create(:account) } + let(:user) { create(:user, role: 'administrator', account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('search_conversations') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Search conversations based on parameters') + end + end + + describe '#parameters' do + it 'returns the correct parameter schema' do + params = service.parameters + expect(params[:type]).to eq('object') + expect(params[:properties]).to include(:contact_id, :status, :priority) + end + end + + describe '#execute' do + let(:contact) { create(:contact, account: account) } + let!(:open_conversation) { create(:conversation, account: account, contact: contact, status: 'open', priority: 'high') } + let!(:resolved_conversation) { create(:conversation, account: account, status: 'resolved', priority: 'low') } + + it 'returns all conversations when no filters are applied' do + result = service.execute({}) + expect(result).to include('Total number of conversations: 2') + expect(result).to include(open_conversation.to_llm_text(include_contact_details: true)) + expect(result).to include(resolved_conversation.to_llm_text(include_contact_details: true)) + end + + it 'filters conversations by status' do + result = service.execute({ 'status' => 'open' }) + expect(result).to include('Total number of conversations: 1') + expect(result).to include(open_conversation.to_llm_text(include_contact_details: true)) + expect(result).not_to include(resolved_conversation.to_llm_text(include_contact_details: true)) + end + + it 'filters conversations by contact_id' do + result = service.execute({ 'contact_id' => contact.id }) + expect(result).to include('Total number of conversations: 1') + expect(result).to include(open_conversation.to_llm_text(include_contact_details: true)) + expect(result).not_to include(resolved_conversation.to_llm_text(include_contact_details: true)) + end + + it 'filters conversations by priority' do + result = service.execute({ 'priority' => 'high' }) + expect(result).to include('Total number of conversations: 1') + expect(result).to include(open_conversation.to_llm_text(include_contact_details: true)) + expect(result).not_to include(resolved_conversation.to_llm_text(include_contact_details: true)) + end + + it 'returns appropriate message when no conversations are found' do + result = service.execute({ 'status' => 'snoozed' }) + expect(result).to eq('No conversations found') + end + end +end 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 b26832faf..0cfec97eb 100644 --- a/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb +++ b/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb @@ -9,6 +9,8 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do let(:admin) { create(:user, account: account, role: :administrator) } let(:agent) { create(:user, account: account, role: :agent) } let!(:inbox) { create(:inbox, account: account) } + let!(:inbox2) { create(:inbox, account: account) } + let!(:another_inbox_conversation) { create(:conversation, account: account, inbox: inbox2) } # This inbox_member is used to establish the agent's access to the inbox before { create(:inbox_member, user: agent, inbox: inbox) } @@ -25,16 +27,14 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result).to include(assigned_conversation) expect(result).to include(unassigned_conversation) expect(result).to include(another_assigned_conversation) - expect(result.count).to eq(3) + expect(result.count).to eq(4) end end context 'when user is a regular agent' do it 'returns all conversations in assigned inboxes' do - inbox_ids = agent.inboxes.where(account_id: account.id).pluck(:id) - result = Conversations::PermissionFilterService.new( - account.conversations.where(inbox_id: inbox_ids), + account.conversations, agent, account ).perform @@ -42,6 +42,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result).to include(assigned_conversation) expect(result).to include(unassigned_conversation) expect(result).to include(another_assigned_conversation) + expect(result).not_to include(another_inbox_conversation) expect(result.count).to eq(3) end end @@ -52,7 +53,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create a new isolated test environment test_account = create(:account) test_inbox = create(:inbox, account: test_account) - + test_inbox2 = create(:inbox, account: test_account) # Create test agent test_agent = create(:user, account: test_account, role: :agent) create(:inbox_member, user: test_agent, inbox: test_inbox) @@ -66,6 +67,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do 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) 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) # Run the test result = Conversations::PermissionFilterService.new( @@ -79,6 +81,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result).to include(assigned_conversation) expect(result).to include(unassigned_conversation) expect(result).to include(other_assigned_conversation) + expect(result).not_to include(other_inbox_conversation) end end @@ -87,6 +90,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create a new isolated test environment test_account = create(:account) test_inbox = create(:inbox, account: test_account) + test_inbox2 = create(:inbox, account: test_account) # Create test agent test_agent = create(:user, account: test_account, role: :agent) @@ -101,6 +105,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create some conversations other_conversation = create(:conversation, account: test_account, inbox: test_inbox) assigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: test_agent) + other_inbox_conversation = create(:conversation, account: test_account, inbox: test_inbox2, assignee: nil) # Run the test result = Conversations::PermissionFilterService.new( @@ -114,6 +119,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result.first.assignee).to eq(test_agent) expect(result).to include(assigned_conversation) expect(result).not_to include(other_conversation) + expect(result).not_to include(other_inbox_conversation) end end @@ -122,6 +128,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create a new isolated test environment test_account = create(:account) test_inbox = create(:inbox, account: test_account) + test_inbox2 = create(:inbox, account: test_account) # Create test agent test_agent = create(:user, account: test_account, role: :agent) @@ -137,6 +144,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do 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) 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) # Run the test result = Conversations::PermissionFilterService.new( @@ -152,6 +160,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Should NOT include conversations assigned to others expect(result).not_to include(other_assigned_conversation) + expect(result).not_to include(other_inbox_conversation) end end @@ -160,6 +169,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create a new isolated test environment test_account = create(:account) test_inbox = create(:inbox, account: test_account) + test_inbox2 = create(:inbox, account: test_account) # Create test agent test_agent = create(:user, account: test_account, role: :agent) @@ -176,6 +186,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do assigned_to_agent = create(:conversation, account: test_account, inbox: test_inbox, assignee: test_agent) unassigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: nil) 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) # Run the test result = Conversations::PermissionFilterService.new( @@ -191,6 +202,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do expect(result).to include(unassigned_conversation) expect(result).to include(assigned_to_agent) expect(result).not_to include(other_assigned_conversation) + expect(result).not_to include(other_inbox_conversation) end end end