From 5b9f997fa0c47d73115fea66ba21c682b7c7bded Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Fri, 11 Jul 2025 23:28:46 +0700 Subject: [PATCH 1/5] feat: Add the support for images in Captain (#11850) --- .../accounts/captain/assistants_controller.rb | 4 +- .../conversation/response_builder_job.rb | 44 +-- .../captain/llm/assistant_chat_service.rb | 13 +- .../open_ai_message_builder_service.rb | 60 ++++ .../hook_execution_service.rb | 29 +- .../captain/assistants_controller_spec.rb | 8 +- .../conversation/response_builder_job_spec.rb | 137 ++++++++ .../open_ai_message_builder_service_spec.rb | 310 ++++++++++++++++++ 8 files changed, 560 insertions(+), 45 deletions(-) create mode 100644 enterprise/app/services/captain/open_ai_message_builder_service.rb create mode 100644 spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb index e5a055836..ec8e8e653 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb @@ -25,8 +25,8 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Base def playground response = Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( - params[:message_content], - message_history + additional_message: params[:message_content], + message_history: message_history ) render json: response diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index f341a6e98..b207bd2a4 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -1,6 +1,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob MAX_MESSAGE_LENGTH = 10_000 - retry_on ActiveStorage::FileNotFoundError, attempts: 3 + retry_on ActiveStorage::FileNotFoundError, attempts: 3, wait: 2.seconds + retry_on Faraday::BadRequestError, attempts: 3, wait: 2.seconds def perform(conversation, assistant) @conversation = conversation @@ -13,7 +14,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob generate_and_process_response end rescue StandardError => e - raise e if e.is_a?(ActiveStorage::FileNotFoundError) + raise e if e.is_a?(ActiveStorage::FileNotFoundError) || e.is_a?(Faraday::BadRequestError) handle_error(e) ensure @@ -26,8 +27,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob def generate_and_process_response @response = Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( - @conversation.messages.incoming.last.content, - collect_previous_messages + message_history: collect_previous_messages ) return process_action('handoff') if handoff_requested? @@ -43,39 +43,19 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob .where(message_type: [:incoming, :outgoing]) .where(private: false) .map do |message| - { - content: message_content(message), - role: determine_role(message) - } - end - end - - def message_content(message) - return message.content if message.content.present? - return 'User has shared a message without content' unless message.attachments.any? - - audio_transcriptions = extract_audio_transcriptions(message.attachments) - return audio_transcriptions if audio_transcriptions.present? - - 'User has shared an attachment' - end - - def extract_audio_transcriptions(attachments) - audio_attachments = attachments.where(file_type: :audio) - return '' if audio_attachments.blank? - - transcriptions = '' - audio_attachments.each do |attachment| - result = Messages::AudioTranscriptionService.new(attachment).perform - transcriptions += result[:transcriptions] if result[:success] + { + content: prepare_multimodal_message_content(message), + role: determine_role(message) + } end - transcriptions end def determine_role(message) - return 'system' if message.content.blank? + message.message_type == 'incoming' ? 'user' : 'assistant' + end - message.message_type == 'incoming' ? 'user' : 'system' + def prepare_multimodal_message_content(message) + Captain::OpenAiMessageBuilderService.new(message: message).generate_content end def handoff_requested? diff --git a/enterprise/app/services/captain/llm/assistant_chat_service.rb b/enterprise/app/services/captain/llm/assistant_chat_service.rb index 569931d44..ca8fafaa0 100644 --- a/enterprise/app/services/captain/llm/assistant_chat_service.rb +++ b/enterprise/app/services/captain/llm/assistant_chat_service.rb @@ -12,9 +12,16 @@ class Captain::Llm::AssistantChatService < Llm::BaseOpenAiService register_tools end - def generate_response(input, previous_messages = [], role = 'user') - @messages += previous_messages - @messages << { role: role, content: input } if input.present? + # additional_message: A single message (String) from the user that should be appended to the chat. + # It can be an empty String or nil when you only want to supply historical messages. + # message_history: An Array of already formatted messages that provide the previous context. + # role: The role for the additional_message (defaults to `user`). + # + # NOTE: Parameters are provided as keyword arguments to improve clarity and avoid relying on + # positional ordering. + def generate_response(additional_message: nil, message_history: [], role: 'user') + @messages += message_history + @messages << { role: role, content: additional_message } if additional_message.present? request_chat_completion end diff --git a/enterprise/app/services/captain/open_ai_message_builder_service.rb b/enterprise/app/services/captain/open_ai_message_builder_service.rb new file mode 100644 index 000000000..43d2851c9 --- /dev/null +++ b/enterprise/app/services/captain/open_ai_message_builder_service.rb @@ -0,0 +1,60 @@ +class Captain::OpenAiMessageBuilderService + pattr_initialize [:message!] + + def generate_content + parts = [] + parts << text_part(@message.content) if @message.content.present? + parts.concat(attachment_parts(@message.attachments)) if @message.attachments.any? + + return 'Message without content' if parts.blank? + return parts.first[:text] if parts.one? && parts.first[:type] == 'text' + + parts + end + + private + + def text_part(text) + { type: 'text', text: text } + end + + def image_part(image_url) + { type: 'image_url', image_url: { url: image_url } } + end + + def attachment_parts(attachments) + image_attachments = attachments.where(file_type: :image) + image_content = image_parts(image_attachments) + + transcription = extract_audio_transcriptions(attachments) + transcription_part = text_part(transcription) if transcription.present? + + attachment_part = text_part('User has shared an attachment') if attachments.where.not(file_type: %i[image audio]).exists? + + [image_content, transcription_part, attachment_part].flatten.compact + end + + def image_parts(image_attachments) + image_attachments.each_with_object([]) do |attachment, parts| + url = get_attachment_url(attachment) + parts << image_part(url) if url.present? + end + end + + def get_attachment_url(attachment) + return attachment.download_url if attachment.download_url.present? + return attachment.external_url if attachment.external_url.present? + + attachment.file.attached? ? attachment.file_url : nil + end + + def extract_audio_transcriptions(attachments) + audio_attachments = attachments.where(file_type: :audio) + return '' if audio_attachments.blank? + + audio_attachments.map do |attachment| + result = Messages::AudioTranscriptionService.new(attachment).perform + result[:success] ? result[:transcriptions] : '' + end.join + end +end \ No newline at end of file diff --git a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb index 92ccba553..ac1c3a95d 100644 --- a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb +++ b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb @@ -1,13 +1,34 @@ module Enterprise::MessageTemplates::HookExecutionService + MAX_ATTACHMENT_WAIT_SECONDS = 4 + def trigger_templates super return unless should_process_captain_response? return perform_handoff unless inbox.captain_active? - Captain::Conversation::ResponseBuilderJob.perform_later( - conversation, - conversation.inbox.captain_assistant - ) + schedule_captain_response + end + + private + + def schedule_captain_response + job_args = [conversation, conversation.inbox.captain_assistant] + + if message.attachments.blank? + Captain::Conversation::ResponseBuilderJob.perform_later(*job_args) + else + wait_time = calculate_attachment_wait_time + Captain::Conversation::ResponseBuilderJob.set(wait: wait_time).perform_later(*job_args) + end + end + + def calculate_attachment_wait_time + attachment_count = message.attachments.size + base_wait = 1.second + + # Wait longer for more attachments or larger files + additional_wait = [attachment_count * 1, MAX_ATTACHMENT_WAIT_SECONDS].min.seconds + base_wait + additional_wait end def should_process_captain_response? diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index 1f6d83d80..80be6f30f 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb @@ -211,8 +211,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do expect(response).to have_http_status(:success) expect(chat_service).to have_received(:generate_response).with( - valid_params[:message_content], - valid_params[:message_history] + additional_message: valid_params[:message_content], + message_history: valid_params[:message_history] ) expect(json_response[:content]).to eq('Assistant response') end @@ -232,8 +232,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do expect(response).to have_http_status(:success) expect(chat_service).to have_received(:generate_response).with( - params_without_history[:message_content], - [] + additional_message: params_without_history[:message_content], + message_history: [] ) end end diff --git a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb index 1e4a6e824..c21205d52 100644 --- a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb @@ -30,5 +30,142 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do account.reload expect(account.usage_limits[:captain][:responses][:consumed]).to eq(1) end + + 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(:image_attachment) { message_with_image.attachments.create!(account: account, file_type: :image, external_url: 'https://example.com/error.jpg') } + + before do + image_attachment + end + + it 'includes image URL directly in the message content for OpenAI vision analysis' do + # Expect the generate_response to receive multimodal content with image URL + expect(mock_llm_chat_service).to receive(:generate_response) do |**kwargs| + history = kwargs[:message_history] + last_entry = history.last + expect(last_entry[:content]).to be_an(Array) + expect(last_entry[:content].any? { |part| part[:type] == 'text' && part[:text] == 'Can you help with this error?' }).to be true + expect(last_entry[:content].any? do |part| + part[:type] == 'image_url' && part[:image_url][:url] == 'https://example.com/error.jpg' + end).to be true + { 'response' => 'I can see the error in your image. It appears to be a database connection issue.' } + end + + described_class.perform_now(conversation, assistant) + end + end + end + + describe 'retry mechanisms for image processing' do + let(:conversation) { create(:conversation, inbox: inbox, account: account) } + let(:mock_llm_chat_service) { instance_double(Captain::Llm::AssistantChatService) } + let(:mock_message_builder) { instance_double(Captain::OpenAiMessageBuilderService) } + + before do + create(:message, conversation: conversation, content: 'Hello with image', message_type: :incoming) + allow(Captain::Llm::AssistantChatService).to receive(:new).and_return(mock_llm_chat_service) + allow(Captain::OpenAiMessageBuilderService).to receive(:new).with(message: anything).and_return(mock_message_builder) + allow(mock_message_builder).to receive(:generate_content).and_return('Hello with image') + allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'Test response' }) + end + + context 'when ActiveStorage::FileNotFoundError occurs' do + it 'handles file errors and triggers handoff' do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image file not found') + + # For retryable errors, the job should handle them and proceed with handoff + described_class.perform_now(conversation, assistant) + + # Verify handoff occurred due to repeated failures + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_message_builder).to receive(:generate_content) + .and_return('Image content processed successfully') + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.count).to eq(1) + expect(conversation.messages.outgoing.last.content).to eq('Test response') + end + end + + context 'when Faraday::BadRequestError occurs' do + it 'handles API errors and triggers handoff' do + allow(mock_llm_chat_service).to receive(:generate_response) + .and_raise(Faraday::BadRequestError, 'Bad request to image service') + + described_class.perform_now(conversation, assistant) + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_llm_chat_service).to receive(:generate_response) + .and_return({ 'response' => 'Response after retry' }) + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.last.content).to eq('Response after retry') + end + end + + context 'when image processing fails permanently' do + before do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image permanently unavailable') + end + + it 'triggers handoff after max retries' do + # Since perform_now re-raises retryable errors, simulate the final failure after retries + allow(mock_message_builder).to receive(:generate_content) + .and_raise(StandardError, 'Max retries exceeded') + + expect(ChatwootExceptionTracker).to receive(:new).and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + end + + context 'when non-retryable error occurs' do + let(:standard_error) { StandardError.new('Generic error') } + + before do + allow(mock_llm_chat_service).to receive(:generate_response).and_raise(standard_error) + end + + it 'handles error and triggers handoff' do + expect(ChatwootExceptionTracker).to receive(:new) + .with(standard_error, account: account) + .and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + + it 'ensures Current.executed_by is reset' do + expect(Current).to receive(:executed_by=).with(assistant) + expect(Current).to receive(:executed_by=).with(nil) + + described_class.perform_now(conversation, assistant) + end + end + end + + describe 'job configuration' do + it 'has retry_on configuration for retryable errors' do + expect(described_class).to respond_to(:retry_on) + end + + it 'defines MAX_MESSAGE_LENGTH constant' do + expect(described_class::MAX_MESSAGE_LENGTH).to eq(10_000) + end end end diff --git a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb new file mode 100644 index 000000000..76e91ae7a --- /dev/null +++ b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb @@ -0,0 +1,310 @@ +require 'rails_helper' + +RSpec.describe Captain::OpenAiMessageBuilderService do + subject(:service) { described_class.new(message: message) } + + let(:message) { create(:message, content: 'Hello world') } + + describe '#generate_content' do + context 'when message has only text content' do + it 'returns the text content directly' do + expect(service.generate_content).to eq('Hello world') + end + end + + context 'when message has no content and no attachments' do + let(:message) { create(:message, content: nil) } + + it 'returns default message' do + expect(service.generate_content).to eq('Message without content') + end + end + + context 'when message has text content and attachments' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'returns an array of content parts' do + result = service.generate_content + expect(result).to be_an(Array) + expect(result).to include({ type: 'text', text: 'Hello world' }) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + + context 'when message has only non-text attachments' do + let(:message) { create(:message, content: nil) } + + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'returns an array of content parts without text' do + result = service.generate_content + expect(result).to be_an(Array) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + expect(result).not_to include(hash_including(type: 'text', text: 'Hello world')) + end + end + end + + describe '#attachment_parts' do + let(:message) { create(:message, content: nil) } + let(:attachments) { message.attachments } + + context 'with image attachments' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'includes image parts' do + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + + context 'with audio attachments' do + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Audio transcription text' }) + ) + end + + it 'includes transcription text part' do + audio_attachment # trigger creation + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'text', text: 'Audio transcription text' }) + end + end + + context 'with other file types' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :file) + attachment.save! + end + + it 'includes generic attachment message' do + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'text', text: 'User has shared an attachment' }) + end + end + + context 'with mixed attachment types' do + let(:image_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + attachment + end + + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + let(:document_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :file) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Audio text' }) + ) + end + + it 'includes all relevant parts' do + image_attachment # trigger creation + audio_attachment # trigger creation + document_attachment # trigger creation + + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + expect(result).to include({ type: 'text', text: 'Audio text' }) + expect(result).to include({ type: 'text', text: 'User has shared an attachment' }) + end + end + end + + describe '#image_parts' do + let(:message) { create(:message, content: nil) } + + context 'with valid image attachments' do + let(:image1) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image1.jpg') + attachment.save! + attachment + end + + let(:image2) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image2.jpg') + attachment.save! + attachment + end + + it 'returns image parts for all valid images' do + image1 # trigger creation + image2 # trigger creation + + image_attachments = message.attachments.where(file_type: :image) + result = service.send(:image_parts, image_attachments) + + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image1.jpg' } }) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image2.jpg' } }) + end + end + + context 'with image attachments without URLs' do + let(:image_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: nil) + attachment.save! + attachment + end + + before do + allow(image_attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: false)) + end + + it 'skips images without valid URLs' do + image_attachment # trigger creation + + image_attachments = message.attachments.where(file_type: :image) + result = service.send(:image_parts, image_attachments) + + expect(result).to be_empty + end + end + end + + describe '#get_attachment_url' do + let(:attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image) + attachment.save! + attachment + end + + context 'when attachment has external_url' do + before { attachment.update(external_url: 'https://example.com/image.jpg') } + + it 'returns external_url' do + expect(service.send(:get_attachment_url, attachment)).to eq('https://example.com/image.jpg') + end + end + + context 'when attachment has attached file' do + before do + attachment.update(external_url: nil) + allow(attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: true)) + allow(attachment).to receive(:file_url).and_return('https://local.com/file.jpg') + allow(attachment).to receive(:download_url).and_return('') + end + + it 'returns file_url' do + expect(service.send(:get_attachment_url, attachment)).to eq('https://local.com/file.jpg') + end + end + + context 'when attachment has no URL or file' do + before do + attachment.update(external_url: nil) + allow(attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: false)) + end + + it 'returns nil' do + expect(service.send(:get_attachment_url, attachment)).to be_nil + end + end + end + + describe '#extract_audio_transcriptions' do + let(:message) { create(:message, content: nil) } + + context 'with no audio attachments' do + it 'returns empty string' do + result = service.send(:extract_audio_transcriptions, message.attachments) + expect(result).to eq('') + end + end + + context 'with successful audio transcriptions' do + let(:audio1) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + let(:audio2) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio1).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'First audio text. ' }) + ) + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio2).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Second audio text.' }) + ) + end + + it 'concatenates all successful transcriptions' do + audio1 # trigger creation + audio2 # trigger creation + + attachments = message.attachments + result = service.send(:extract_audio_transcriptions, attachments) + expect(result).to eq('First audio text. Second audio text.') + end + end + + context 'with failed audio transcriptions' do + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: false, transcriptions: nil }) + ) + end + + it 'returns empty string for failed transcriptions' do + audio_attachment # trigger creation + + attachments = message.attachments + result = service.send(:extract_audio_transcriptions, attachments) + expect(result).to eq('') + end + end + end + + describe 'private helper methods' do + describe '#text_part' do + it 'returns correct text part format' do + result = service.send(:text_part, 'Hello world') + expect(result).to eq({ type: 'text', text: 'Hello world' }) + end + end + + describe '#image_part' do + it 'returns correct image part format' do + result = service.send(:image_part, 'https://example.com/image.jpg') + expect(result).to eq({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + end +end From 1b02ebec68d650e86509e54d536c507411132e2b Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Sat, 12 Jul 2025 10:06:30 +0530 Subject: [PATCH 2/5] fix: Approved FAQ not disappearing from pending list after filtering (#11909) # Pull Request Template ## Description This PR fixes an issue where approved FAQs were not removed from the list when filtered by `pending` status. **Fix:** Implemented real-time client-side filtering using a `filteredResponses` computed property in `Index.vue`, updated all list and selection logic to use it, and also hide the card toggle dropdown button when the bulk action checkbox is selected. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### Loom video https://www.loom.com/share/216221a4910c44ebb1d49ab9e34c820c?sid=98c9c239-54eb-4bd9-b04d-aeccc55bfb3a ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Muhsin Keloth --- .../captain/assistant/ResponseCard.vue | 6 +- .../dashboard/captain/responses/Index.vue | 58 ++++++++++++------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/app/javascript/dashboard/components-next/captain/assistant/ResponseCard.vue b/app/javascript/dashboard/components-next/captain/assistant/ResponseCard.vue index 7879411c8..a924ca228 100644 --- a/app/javascript/dashboard/components-next/captain/assistant/ResponseCard.vue +++ b/app/javascript/dashboard/components-next/captain/assistant/ResponseCard.vue @@ -55,6 +55,10 @@ const props = defineProps({ type: Boolean, default: false, }, + showMenu: { + type: Boolean, + default: true, + }, }); const emit = defineEmits(['action', 'navigate', 'select', 'hover']); @@ -130,7 +134,7 @@ const handleDocumentableClick = () => { {{ question }} -
+
})) ); +const filteredResponses = computed(() => { + return selectedStatus.value === 'pending' + ? responses.value.filter(r => r.status === 'pending') + : responses.value; +}); + const selectedStatusLabel = computed(() => { const status = statusOptions.value.find( option => option.value === selectedStatus.value @@ -94,7 +100,9 @@ const handleEdit = () => { }; const handleAction = ({ action, id }) => { - selectedResponse.value = responses.value.find(response => id === response.id); + selectedResponse.value = filteredResponses.value.find( + response => id === response.id + ); nextTick(() => { if (action === 'delete') { handleDelete(); @@ -139,7 +147,7 @@ const hoveredCard = ref(null); const bulkSelectionState = computed(() => { const selectedCount = bulkSelectedIds.value.size; - const totalCount = responses.value?.length || 0; + const totalCount = filteredResponses.value?.length || 0; return { hasSelected: selectedCount > 0, @@ -152,13 +160,13 @@ const bulkCheckbox = computed({ get: () => bulkSelectionState.value.allSelected, set: value => { bulkSelectedIds.value = value - ? new Set(responses.value.map(r => r.id)) + ? new Set(filteredResponses.value.map(r => r.id)) : new Set(); }, }); const buildSelectedCountLabel = computed(() => { - const count = responses.value?.length || 0; + const count = filteredResponses.value?.length || 0; return bulkSelectionState.value.allSelected ? t('CAPTAIN.RESPONSES.UNSELECT_ALL', { count }) : t('CAPTAIN.RESPONSES.SELECT_ALL', { count }); @@ -174,6 +182,24 @@ const handleCardSelect = id => { bulkSelectedIds.value = selected; }; +const fetchResponseAfterBulkAction = () => { + const hasNoResponsesLeft = filteredResponses.value?.length === 0; + const currentPage = responseMeta.value?.page; + + if (hasNoResponsesLeft) { + // Page is now empty after bulk action. + // Fetch the previous page if not already on the first page. + const pageToFetch = currentPage > 1 ? currentPage - 1 : currentPage; + fetchResponses(pageToFetch); + } else { + // Page still has responses left, re-fetch the same page. + fetchResponses(currentPage); + } + + // Clear selection + bulkSelectedIds.value = new Set(); +}; + const handleBulkApprove = async () => { try { await store.dispatch( @@ -181,8 +207,7 @@ const handleBulkApprove = async () => { Array.from(bulkSelectedIds.value) ); - // Clear selection - bulkSelectedIds.value = new Set(); + fetchResponseAfterBulkAction(); useAlert(t('CAPTAIN.RESPONSES.BULK_APPROVE.SUCCESS_MESSAGE')); } catch (error) { useAlert( @@ -205,23 +230,13 @@ const onPageChange = page => { }; const onDeleteSuccess = () => { - if (responses.value?.length === 0 && responseMeta.value?.page > 1) { + if (filteredResponses.value?.length === 0 && responseMeta.value?.page > 1) { onPageChange(responseMeta.value.page - 1); } }; const onBulkDeleteSuccess = () => { - // Only fetch if no records left - if (responses.value?.length === 0) { - const page = - responseMeta.value?.page > 1 - ? responseMeta.value.page - 1 - : responseMeta.value.page; - fetchResponses(page); - } - - // Clear selection - bulkSelectedIds.value = new Set(); + fetchResponseAfterBulkAction(); }; const handleStatusFilterChange = ({ value }) => { @@ -249,8 +264,8 @@ onMounted(() => { :header-title="$t('CAPTAIN.RESPONSES.HEADER')" :button-label="$t('CAPTAIN.RESPONSES.ADD_NEW')" :is-fetching="isFetching" - :is-empty="!responses.length" - :show-pagination-footer="!isFetching && !!responses.length" + :is-empty="!filteredResponses.length" + :show-pagination-footer="!isFetching && !!filteredResponses.length" :feature-flag="FEATURE_FLAGS.CAPTAIN" @update:current-page="onPageChange" @click="handleCreate" @@ -368,7 +383,7 @@ onMounted(() => {
{ :updated-at="response.updated_at" :is-selected="bulkSelectedIds.has(response.id)" :selectable="hoveredCard === response.id || bulkSelectedIds.size > 0" + :show-menu="!bulkSelectedIds.has(response.id)" @action="handleAction" @navigate="handleNavigationAction" @select="handleCardSelect" From 18eb53acf4c8c232a30f2fb615275185f334a431 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Sat, 12 Jul 2025 17:02:10 +0530 Subject: [PATCH 3/5] feat: New Assistants Edit Page (#11920) # Pull Request Template ## Description Fixes https://linear.app/chatwoot/issue/CW-4602/assistants-edit-page ### Screenshots image image --------- Co-authored-by: Muhsin Keloth --- .../components-next/breadcrumb/Breadcrumb.vue | 29 ++-- .../captain/SettingsPageLayout.vue | 91 +++++++++++ .../settings/AssistantBasicSettingsForm.vue | 151 +++++++++++++++++ .../settings/AssistantControlItems.vue | 43 +++++ .../settings/AssistantSystemSettingsForm.vue | 125 ++++++++++++++ app/javascript/dashboard/featureFlags.js | 2 + .../i18n/locale/en/integrations.json | 31 ++++ .../dashboard/captain/assistants/Edit.vue | 17 +- .../captain/assistants/settings/Settings.vue | 154 ++++++++++++++++++ .../dashboard/captain/captain.routes.js | 1 + config/features.yml | 4 + 11 files changed, 634 insertions(+), 14 deletions(-) create mode 100644 app/javascript/dashboard/components-next/captain/SettingsPageLayout.vue create mode 100644 app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantBasicSettingsForm.vue create mode 100644 app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantControlItems.vue create mode 100644 app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantSystemSettingsForm.vue create mode 100644 app/javascript/dashboard/routes/dashboard/captain/assistants/settings/Settings.vue diff --git a/app/javascript/dashboard/components-next/breadcrumb/Breadcrumb.vue b/app/javascript/dashboard/components-next/breadcrumb/Breadcrumb.vue index 8ca325607..8a1b26e90 100644 --- a/app/javascript/dashboard/components-next/breadcrumb/Breadcrumb.vue +++ b/app/javascript/dashboard/components-next/breadcrumb/Breadcrumb.vue @@ -15,8 +15,8 @@ const emit = defineEmits(['click']); const { t } = useI18n(); -const onClick = event => { - emit('click', event); +const onClick = (item, index) => { + emit('click', item, index); }; @@ -24,22 +24,25 @@ const onClick = event => { diff --git a/app/javascript/dashboard/components-next/captain/SettingsPageLayout.vue b/app/javascript/dashboard/components-next/captain/SettingsPageLayout.vue new file mode 100644 index 000000000..4a48794b3 --- /dev/null +++ b/app/javascript/dashboard/components-next/captain/SettingsPageLayout.vue @@ -0,0 +1,91 @@ + + + diff --git a/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantBasicSettingsForm.vue b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantBasicSettingsForm.vue new file mode 100644 index 000000000..236791793 --- /dev/null +++ b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantBasicSettingsForm.vue @@ -0,0 +1,151 @@ + + + diff --git a/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantControlItems.vue b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantControlItems.vue new file mode 100644 index 000000000..2bbede899 --- /dev/null +++ b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantControlItems.vue @@ -0,0 +1,43 @@ + + + diff --git a/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantSystemSettingsForm.vue b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantSystemSettingsForm.vue new file mode 100644 index 000000000..b86bdacb4 --- /dev/null +++ b/app/javascript/dashboard/components-next/captain/pageComponents/assistant/settings/AssistantSystemSettingsForm.vue @@ -0,0 +1,125 @@ + + + diff --git a/app/javascript/dashboard/featureFlags.js b/app/javascript/dashboard/featureFlags.js index 1a6ada9fa..98234539b 100644 --- a/app/javascript/dashboard/featureFlags.js +++ b/app/javascript/dashboard/featureFlags.js @@ -36,6 +36,7 @@ export const FEATURE_FLAGS = { REPORT_V4: 'report_v4', CHANNEL_INSTAGRAM: 'channel_instagram', CONTACT_CHATWOOT_SUPPORT_TEAM: 'contact_chatwoot_support_team', + CAPTAIN_V2: 'captain_integration_v2', }; export const PREMIUM_FEATURES = [ @@ -44,4 +45,5 @@ export const PREMIUM_FEATURES = [ FEATURE_FLAGS.CUSTOM_ROLES, FEATURE_FLAGS.AUDIT_LOGS, FEATURE_FLAGS.HELP_CENTER, + FEATURE_FLAGS.CAPTAIN_V2, ]; diff --git a/app/javascript/dashboard/i18n/locale/en/integrations.json b/app/javascript/dashboard/i18n/locale/en/integrations.json index b3e091722..fc8cff644 100644 --- a/app/javascript/dashboard/i18n/locale/en/integrations.json +++ b/app/javascript/dashboard/i18n/locale/en/integrations.json @@ -478,6 +478,37 @@ "ERROR_MESSAGE": "There was an error updating the assistant, please try again.", "NOT_FOUND": "Could not find the assistant. Please try again." }, + "SETTINGS": { + "BREADCRUMB": { + "ASSISTANT": "Assistant" + }, + "BASIC_SETTINGS": { + "TITLE": "Basic settings", + "DESCRIPTION": "Customize what the assistant says when ending a conversation or transferring to a human." + }, + "SYSTEM_SETTINGS": { + "TITLE": "System settings", + "DESCRIPTION": "Customize what the assistant says when ending a conversation or transferring to a human." + }, + "CONTROL_ITEMS": { + "TITLE": "The Fun Stuff", + "DESCRIPTION": "Add more control to the assistant. (a bit more visual like a story : Query guardrail → scenarios → output) Nudges user to actually utilise these.", + "OPTIONS": { + "GUARDRAILS": { + "TITLE": "Guardrails", + "DESCRIPTION": "Keeps things on track—only the kinds of questions you want your assistant to answer, nothing off-limits or off-topic." + }, + "SCENARIOS": { + "TITLE": "Scenarios", + "DESCRIPTION": "Give your assistant some context—like “what to do when a user is stuck,” or “how to act during a refund request.”" + }, + "RESPONSE_GUIDELINES": { + "TITLE": "Response guidelines", + "DESCRIPTION": "The vibe and structure of your assistant’s replies—clear and friendly? Short and snappy? Detailed and formal?" + } + } + } + }, "OPTIONS": { "EDIT_ASSISTANT": "Edit Assistant", "DELETE_ASSISTANT": "Delete Assistant", diff --git a/app/javascript/dashboard/routes/dashboard/captain/assistants/Edit.vue b/app/javascript/dashboard/routes/dashboard/captain/assistants/Edit.vue index 8b64e2663..2b2506934 100644 --- a/app/javascript/dashboard/routes/dashboard/captain/assistants/Edit.vue +++ b/app/javascript/dashboard/routes/dashboard/captain/assistants/Edit.vue @@ -5,10 +5,13 @@ import { useStore } from 'dashboard/composables/store'; import { useMapGetter } from 'dashboard/composables/store'; import { useAlert } from 'dashboard/composables'; import { useI18n } from 'vue-i18n'; +import { FEATURE_FLAGS } from 'dashboard/featureFlags'; import PageLayout from 'dashboard/components-next/captain/PageLayout.vue'; import EditAssistantForm from '../../../../components-next/captain/pageComponents/assistant/EditAssistantForm.vue'; import AssistantPlayground from 'dashboard/components-next/captain/assistant/AssistantPlayground.vue'; +import AssistantSettings from 'dashboard/routes/dashboard/captain/assistants/settings/Settings.vue'; + const route = useRoute(); const store = useStore(); const { t } = useI18n(); @@ -19,6 +22,16 @@ const assistant = computed(() => store.getters['captainAssistants/getRecord'](Number(assistantId)) ); +const isFeatureEnabledonAccount = useMapGetter( + 'accounts/isFeatureEnabledonAccount' +); +const currentAccountId = useMapGetter('getCurrentAccountId'); + +const isCaptainV2Enabled = isFeatureEnabledonAccount.value( + currentAccountId.value, + FEATURE_FLAGS.CAPTAIN_V2 +); + const isAssistantAvailable = computed(() => !!assistant.value?.id); const handleSubmit = async updatedAssistant => { @@ -36,14 +49,16 @@ const handleSubmit = async updatedAssistant => { }; onMounted(() => { - if (!isAssistantAvailable.value) { + if (!isAssistantAvailable.value || !isCaptainV2Enabled) { store.dispatch('captainAssistants/show', assistantId); } });