feat: Add support for search_conversations in copilot (#11520)

Earlier, we were manually checking if a user was an agent and filtering
their conversations based on inboxes. This logic should have been part
of the conversation permissions service.

This PR moves the check to the right place and updates the logic
accordingly.

Other updates:
- Add support for search_conversations service for copilot.
- Use PermissionFilterService in contacts/conversations, conversations,
copilot search_conversations.

---------

Co-authored-by: Sojan <sojan@pepalo.com>
Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
Pranav
2025-05-20 19:22:17 -07:00
committed by GitHub
parent af650af489
commit a07f2a7c1b
9 changed files with 193 additions and 37 deletions

View File

@@ -12,10 +12,6 @@ class Api::V1::Accounts::Contacts::ConversationsController < Api::V1::Accounts::
Current.account Current.account
).perform ).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) @conversations = conversations.order(last_activity_at: :desc).limit(20)
end end
end end

View File

@@ -88,7 +88,10 @@ class ConversationFinder
def find_conversation_by_inbox def find_conversation_by_inbox
@conversations = current_account.conversations @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 end
def find_all_conversations def find_all_conversations

View File

@@ -28,16 +28,6 @@ class Conversations::FilterService < FilterService
:taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :messages, :contact_inbox :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::PermissionFilterService.new(
conversations, conversations,
@user, @user,

View File

@@ -8,9 +8,23 @@ class Conversations::PermissionFilterService
end end
def perform def perform
# The base implementation simply returns all conversations return conversations if user_role == 'administrator'
# Enterprise edition extends this with permission-based filtering
conversations 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
end end

View File

@@ -1,8 +1,9 @@
class Captain::Tools::BaseService class Captain::Tools::BaseService
attr_accessor :assistant attr_accessor :assistant
def initialize(assistant) def initialize(assistant, user: nil)
@assistant = assistant @assistant = assistant
@user = user
end end
def name def name

View File

@@ -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

View File

@@ -1,36 +1,37 @@
module Enterprise::Conversations::PermissionFilterService module Enterprise::Conversations::PermissionFilterService
def perform def perform
account_user = AccountUser.find_by(account_id: account.id, user_id: user.id) return filter_by_permissions(permissions) if user_has_custom_role?
permissions = account_user&.permissions || []
user_role = account_user&.role
# Skip filtering for administrators super
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)
end end
private 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) def filter_by_permissions(permissions)
# Permission-based filtering with hierarchy # Permission-based filtering with hierarchy
# conversation_manage > conversation_unassigned_manage > conversation_participating_manage # conversation_manage > conversation_unassigned_manage > conversation_participating_manage
if permissions.include?('conversation_manage') if permissions.include?('conversation_manage')
conversations accessible_conversations
elsif permissions.include?('conversation_unassigned_manage') elsif permissions.include?('conversation_unassigned_manage')
filter_unassigned_and_mine filter_unassigned_and_mine
elsif permissions.include?('conversation_participating_manage') elsif permissions.include?('conversation_participating_manage')
conversations.assigned_to(user) accessible_conversations.assigned_to(user)
else else
Conversation.none Conversation.none
end end
end end
def filter_unassigned_and_mine def filter_unassigned_and_mine
mine = conversations.assigned_to(user) mine = accessible_conversations.assigned_to(user)
unassigned = conversations.unassigned unassigned = accessible_conversations.unassigned
Conversation.from("(#{mine.to_sql} UNION #{unassigned.to_sql}) as conversations") Conversation.from("(#{mine.to_sql} UNION #{unassigned.to_sql}) as conversations")
.where(account_id: account.id) .where(account_id: account.id)

View File

@@ -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

View File

@@ -9,6 +9,8 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
let(:admin) { create(:user, account: account, role: :administrator) } let(:admin) { create(:user, account: account, role: :administrator) }
let(:agent) { create(:user, account: account, role: :agent) } let(:agent) { create(:user, account: account, role: :agent) }
let!(:inbox) { create(:inbox, account: account) } 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 # This inbox_member is used to establish the agent's access to the inbox
before { create(:inbox_member, user: agent, inbox: 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(assigned_conversation)
expect(result).to include(unassigned_conversation) expect(result).to include(unassigned_conversation)
expect(result).to include(another_assigned_conversation) expect(result).to include(another_assigned_conversation)
expect(result.count).to eq(3) expect(result.count).to eq(4)
end end
end end
context 'when user is a regular agent' do context 'when user is a regular agent' do
it 'returns all conversations in assigned inboxes' do it 'returns all conversations in assigned inboxes' do
inbox_ids = agent.inboxes.where(account_id: account.id).pluck(:id)
result = Conversations::PermissionFilterService.new( result = Conversations::PermissionFilterService.new(
account.conversations.where(inbox_id: inbox_ids), account.conversations,
agent, agent,
account account
).perform ).perform
@@ -42,6 +42,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
expect(result).to include(assigned_conversation) expect(result).to include(assigned_conversation)
expect(result).to include(unassigned_conversation) expect(result).to include(unassigned_conversation)
expect(result).to include(another_assigned_conversation) expect(result).to include(another_assigned_conversation)
expect(result).not_to include(another_inbox_conversation)
expect(result.count).to eq(3) expect(result.count).to eq(3)
end end
end end
@@ -52,7 +53,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Create a new isolated test environment # Create a new isolated test environment
test_account = create(:account) test_account = create(:account)
test_inbox = create(:inbox, account: test_account) test_inbox = create(:inbox, account: test_account)
test_inbox2 = create(:inbox, account: test_account)
# Create test agent # Create test agent
test_agent = create(:user, account: test_account, role: :agent) test_agent = create(:user, account: test_account, role: :agent)
create(:inbox_member, user: test_agent, inbox: test_inbox) 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) 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) 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_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 # Run the test
result = Conversations::PermissionFilterService.new( result = Conversations::PermissionFilterService.new(
@@ -79,6 +81,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
expect(result).to include(assigned_conversation) expect(result).to include(assigned_conversation)
expect(result).to include(unassigned_conversation) expect(result).to include(unassigned_conversation)
expect(result).to include(other_assigned_conversation) expect(result).to include(other_assigned_conversation)
expect(result).not_to include(other_inbox_conversation)
end end
end end
@@ -87,6 +90,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Create a new isolated test environment # Create a new isolated test environment
test_account = create(:account) test_account = create(:account)
test_inbox = create(:inbox, account: test_account) test_inbox = create(:inbox, account: test_account)
test_inbox2 = create(:inbox, account: test_account)
# Create test agent # Create test agent
test_agent = create(:user, account: test_account, role: :agent) test_agent = create(:user, account: test_account, role: :agent)
@@ -101,6 +105,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Create some conversations # Create some conversations
other_conversation = create(:conversation, account: test_account, inbox: test_inbox) other_conversation = create(:conversation, account: test_account, inbox: test_inbox)
assigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: test_agent) 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 # Run the test
result = Conversations::PermissionFilterService.new( result = Conversations::PermissionFilterService.new(
@@ -114,6 +119,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
expect(result.first.assignee).to eq(test_agent) expect(result.first.assignee).to eq(test_agent)
expect(result).to include(assigned_conversation) expect(result).to include(assigned_conversation)
expect(result).not_to include(other_conversation) expect(result).not_to include(other_conversation)
expect(result).not_to include(other_inbox_conversation)
end end
end end
@@ -122,6 +128,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Create a new isolated test environment # Create a new isolated test environment
test_account = create(:account) test_account = create(:account)
test_inbox = create(:inbox, account: test_account) test_inbox = create(:inbox, account: test_account)
test_inbox2 = create(:inbox, account: test_account)
# Create test agent # Create test agent
test_agent = create(:user, account: test_account, role: :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) 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) 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_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 # Run the test
result = Conversations::PermissionFilterService.new( result = Conversations::PermissionFilterService.new(
@@ -152,6 +160,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Should NOT include conversations assigned to others # Should NOT include conversations assigned to others
expect(result).not_to include(other_assigned_conversation) expect(result).not_to include(other_assigned_conversation)
expect(result).not_to include(other_inbox_conversation)
end end
end end
@@ -160,6 +169,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
# Create a new isolated test environment # Create a new isolated test environment
test_account = create(:account) test_account = create(:account)
test_inbox = create(:inbox, account: test_account) test_inbox = create(:inbox, account: test_account)
test_inbox2 = create(:inbox, account: test_account)
# Create test agent # Create test agent
test_agent = create(:user, account: test_account, role: :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) 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) 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_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 # Run the test
result = Conversations::PermissionFilterService.new( result = Conversations::PermissionFilterService.new(
@@ -191,6 +202,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do
expect(result).to include(unassigned_conversation) expect(result).to include(unassigned_conversation)
expect(result).to include(assigned_to_agent) expect(result).to include(assigned_to_agent)
expect(result).not_to include(other_assigned_conversation) expect(result).not_to include(other_assigned_conversation)
expect(result).not_to include(other_inbox_conversation)
end end
end end
end end