From eabdfc81684b467c478bbf6ee238f5fa4ee8f7ec Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 22 Oct 2025 20:20:37 -0700 Subject: [PATCH 001/109] chore(sidekiq): log ActiveJob class and job_id on dequeue (#12704) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context Sidekiq logs only showed the Sidekiq wrapper class and JID, which wasn’t helpful when debugging ActiveJobs. ## Changes - Updated `ChatwootDequeuedLogger` to log the actual `ActiveJob class` and `job_id` instead of the generic Sidekiq wrapper and JID. > Example > ``` > Dequeued ActionMailer::MailDeliveryJob 123e4567-e89b-12d3-a456-426614174000 from default > ``` - Remove sidekiq worker and unify everything to `ActiveJob` --- app/jobs/conversation_reply_email_job.rb | 15 ++++++ .../send_email_notification_service.rb | 2 +- .../conversation_reply_email_worker.rb | 29 ------------ config/initializers/sidekiq.rb | 3 +- ...nding_conversations_resolution_job_spec.rb | 27 ++++++----- .../jobs/conversation_reply_email_job_spec.rb | 33 +++++++++++++ spec/models/message_spec.rb | 33 ++++--------- .../send_email_notification_service_spec.rb | 46 +++++++------------ .../conversation_reply_email_worker_spec.rb | 46 ------------------- 9 files changed, 92 insertions(+), 142 deletions(-) create mode 100644 app/jobs/conversation_reply_email_job.rb delete mode 100644 app/workers/conversation_reply_email_worker.rb create mode 100644 spec/jobs/conversation_reply_email_job_spec.rb delete mode 100644 spec/workers/conversation_reply_email_worker_spec.rb diff --git a/app/jobs/conversation_reply_email_job.rb b/app/jobs/conversation_reply_email_job.rb new file mode 100644 index 000000000..5d186bf29 --- /dev/null +++ b/app/jobs/conversation_reply_email_job.rb @@ -0,0 +1,15 @@ +class ConversationReplyEmailJob < ApplicationJob + queue_as :mailers + + def perform(conversation_id, last_queued_id) + conversation = Conversation.find(conversation_id) + + if conversation.messages.incoming&.last&.content_type == 'incoming_email' + ConversationReplyMailer.with(account: conversation.account).reply_without_summary(conversation, last_queued_id).deliver_later + else + ConversationReplyMailer.with(account: conversation.account).reply_with_summary(conversation, last_queued_id).deliver_later + end + + Redis::Alfred.delete(format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: conversation.id)) + end +end diff --git a/app/services/messages/send_email_notification_service.rb b/app/services/messages/send_email_notification_service.rb index 87ba1a797..25a77b0d5 100644 --- a/app/services/messages/send_email_notification_service.rb +++ b/app/services/messages/send_email_notification_service.rb @@ -12,7 +12,7 @@ class Messages::SendEmailNotificationService # the worker never manages to clean up. return unless Redis::Alfred.set(conversation_mail_key, message.id, nx: true, ex: 1.hour.to_i) - ConversationReplyEmailWorker.perform_in(2.minutes, conversation.id, message.id) + ConversationReplyEmailJob.set(wait: 2.minutes).perform_later(conversation.id, message.id) end private diff --git a/app/workers/conversation_reply_email_worker.rb b/app/workers/conversation_reply_email_worker.rb deleted file mode 100644 index 0eddecaf7..000000000 --- a/app/workers/conversation_reply_email_worker.rb +++ /dev/null @@ -1,29 +0,0 @@ -# TODO: lets move this to active job, since thats what we use over all -class ConversationReplyEmailWorker - include Sidekiq::Worker - sidekiq_options queue: :mailers - - def perform(conversation_id, last_queued_id) - @conversation = Conversation.find(conversation_id) - - # send the email - if @conversation.messages.incoming&.last&.content_type == 'incoming_email' - ConversationReplyMailer.with(account: @conversation.account).reply_without_summary(@conversation, last_queued_id).deliver_later - else - ConversationReplyMailer.with(account: @conversation.account).reply_with_summary(@conversation, last_queued_id).deliver_later - end - - # delete the redis set from the first new message on the conversation - Redis::Alfred.delete(conversation_mail_key) - end - - private - - def email_inbox? - @conversation.inbox&.inbox_type == 'Email' - end - - def conversation_mail_key - format(::Redis::Alfred::CONVERSATION_MAILER_KEY, conversation_id: @conversation.id) - end -end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 1ada67cc1..04d605c2e 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -9,7 +9,8 @@ end # Logs whenever a job is pulled off Redis for execution. class ChatwootDequeuedLogger def call(_worker, job, queue) - Sidekiq.logger.info("Dequeued #{job['class']} #{job['jid']} from #{queue}") + payload = job['args'].first + Sidekiq.logger.info("Dequeued #{job['wrapped']} #{payload['job_id']} from #{queue}") yield end end diff --git a/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb b/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb index 40f1ea294..1a8a5a342 100644 --- a/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb +++ b/spec/enterprise/jobs/captain/inbox_pending_conversations_resolution_job_spec.rb @@ -1,8 +1,6 @@ require 'rails_helper' RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do - include ActiveJob::TestHelper - let!(:inbox) { create(:inbox) } let!(:resolvable_pending_conversation) { create(:conversation, inbox: inbox, last_activity_at: 2.hours.ago, status: :pending) } @@ -14,6 +12,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do before do create(:captain_inbox, inbox: inbox, captain_assistant: captain_assistant) stub_const('Limits::BULK_ACTIONS_LIMIT', 2) + inbox.reload end it 'queues the job' do @@ -22,7 +21,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do end it 'resolves only the eligible pending conversations' do - perform_enqueued_jobs { described_class.perform_later(inbox) } + described_class.perform_now(inbox) expect(resolvable_pending_conversation.reload.status).to eq('resolved') expect(recent_pending_conversation.reload.status).to eq('pending') @@ -34,7 +33,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do captain_assistant.update!(config: { 'resolution_message' => custom_message }) expect do - perform_enqueued_jobs { described_class.perform_later(inbox) } + described_class.perform_now(inbox) end.to change { resolvable_pending_conversation.messages.outgoing.reload.count }.by(1) outgoing_message = resolvable_pending_conversation.messages.outgoing.last @@ -44,7 +43,7 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do it 'creates an outgoing message with default auto resolution message if not configured' do captain_assistant.update!(config: {}) - perform_enqueued_jobs { described_class.perform_later(inbox) } + described_class.perform_now(inbox) outgoing_message = resolvable_pending_conversation.messages.outgoing.last expect(outgoing_message.content).to eq( I18n.t('conversations.activity.auto_resolution_message') @@ -52,11 +51,17 @@ RSpec.describe Captain::InboxPendingConversationsResolutionJob, type: :job do end it 'adds the correct activity message after resolution by Captain' do - perform_enqueued_jobs { described_class.perform_later(inbox) } - activity_message = resolvable_pending_conversation.messages.activity.last - expect(activity_message).not_to be_nil - expect(activity_message.content).to eq( - I18n.t('conversations.activity.captain.resolved', user_name: captain_assistant.name) - ) + described_class.perform_now(inbox) + expected_content = I18n.t('conversations.activity.captain.resolved', user_name: captain_assistant.name) + expect(Conversations::ActivityMessageJob) + .to have_been_enqueued.with( + resolvable_pending_conversation, + { + account_id: resolvable_pending_conversation.account_id, + inbox_id: resolvable_pending_conversation.inbox_id, + message_type: :activity, + content: expected_content + } + ) end end diff --git a/spec/jobs/conversation_reply_email_job_spec.rb b/spec/jobs/conversation_reply_email_job_spec.rb new file mode 100644 index 000000000..32758c48e --- /dev/null +++ b/spec/jobs/conversation_reply_email_job_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +RSpec.describe ConversationReplyEmailJob, type: :job do + let(:conversation) { create(:conversation) } + let(:mailer) { double } + let(:mailer_action) { double } + + before do + allow(Conversation).to receive(:find).and_return(conversation) + allow(ConversationReplyMailer).to receive(:with).and_return(mailer) + allow(mailer).to receive(:reply_with_summary).and_return(mailer_action) + allow(mailer).to receive(:reply_without_summary).and_return(mailer_action) + allow(mailer_action).to receive(:deliver_later).and_return(true) + end + + it 'enqueues on mailers queue' do + ActiveJob::Base.queue_adapter = :test + expect do + described_class.perform_later(conversation.id, 123) + end.to have_enqueued_job(described_class).on_queue('mailers') + end + + it 'calls reply_with_summary when last incoming message was not email' do + described_class.perform_now(conversation.id, 123) + expect(mailer).to have_received(:reply_with_summary) + end + + it 'calls reply_without_summary when last incoming message was email' do + create(:message, conversation: conversation, message_type: :incoming, content_type: 'incoming_email') + described_class.perform_now(conversation.id, 123) + expect(mailer).to have_received(:reply_without_summary) + end +end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 49a1d1e51..9cc43d0fd 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -327,14 +327,11 @@ RSpec.describe Message do message.conversation.contact.update!(email: 'test@example.com') message.message_type = 'outgoing' - # Perform jobs inline to test full integration - perform_enqueued_jobs do - message.save! + ActiveJob::Base.queue_adapter = :test + allow(Redis::Alfred).to receive(:set).and_return(true) + perform_enqueued_jobs(only: SendReplyJob) do + expect { message.save! }.to have_enqueued_job(ConversationReplyEmailJob).with(message.conversation.id, kind_of(Integer)).on_queue('mailers') end - - # Verify the email worker is eventually scheduled through the service - jobs_for_conversation_count = ConversationReplyEmailWorker.jobs.count { |job| job['args'].first == message.conversation.id } - expect(jobs_for_conversation_count).to eq(1) end it 'does not schedule email for website channel if continuity is disabled' do @@ -345,15 +342,8 @@ RSpec.describe Message do message.conversation.contact.update!(email: 'test@example.com') message.message_type = 'outgoing' - initial_job_count = ConversationReplyEmailWorker.jobs.count { |job| job['args'].first == message.conversation.id } - - perform_enqueued_jobs do - message.save! - end - - # No new jobs should be scheduled for this conversation - jobs_for_conversation_count = ConversationReplyEmailWorker.jobs.count { |job| job['args'].first == message.conversation.id } - expect(jobs_for_conversation_count).to eq(initial_job_count) + ActiveJob::Base.queue_adapter = :test + expect { message.save! }.not_to have_enqueued_job(ConversationReplyEmailJob) end it 'does not schedule email for private notes' do @@ -363,15 +353,8 @@ RSpec.describe Message do message.private = true message.message_type = 'outgoing' - initial_job_count = ConversationReplyEmailWorker.jobs.count { |job| job['args'].first == message.conversation.id } - - perform_enqueued_jobs do - message.save! - end - - # No new jobs should be scheduled for this conversation - jobs_for_conversation_count = ConversationReplyEmailWorker.jobs.count { |job| job['args'].first == message.conversation.id } - expect(jobs_for_conversation_count).to eq(initial_job_count) + ActiveJob::Base.queue_adapter = :test + expect { message.save! }.not_to have_enqueued_job(ConversationReplyEmailJob) end it 'calls SendReplyJob for all channels' do diff --git a/spec/services/messages/send_email_notification_service_spec.rb b/spec/services/messages/send_email_notification_service_spec.rb index cda728f21..7c0970fe1 100644 --- a/spec/services/messages/send_email_notification_service_spec.rb +++ b/spec/services/messages/send_email_notification_service_spec.rb @@ -14,17 +14,11 @@ describe Messages::SendEmailNotificationService do before do conversation.contact.update!(email: 'test@example.com') allow(Redis::Alfred).to receive(:set).and_return(true) - allow(ConversationReplyEmailWorker).to receive(:perform_in) + ActiveJob::Base.queue_adapter = :test end - it 'schedules ConversationReplyEmailWorker' do - service.perform - - expect(ConversationReplyEmailWorker).to have_received(:perform_in).with( - 2.minutes, - conversation.id, - message.id - ) + it 'enqueues ConversationReplyEmailJob' do + expect { service.perform }.to have_enqueued_job(ConversationReplyEmailJob).with(conversation.id, message.id).on_queue('mailers') end it 'atomically sets redis key to prevent duplicate emails' do @@ -40,10 +34,8 @@ describe Messages::SendEmailNotificationService do allow(Redis::Alfred).to receive(:set).and_return(false) end - it 'does not schedule worker' do - service.perform - - expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) + it 'does not enqueue job' do + expect { service.perform }.not_to have_enqueued_job(ConversationReplyEmailJob) end it 'attempts atomic set once' do @@ -62,7 +54,7 @@ describe Messages::SendEmailNotificationService do conversation.contact.update!(email: 'test@example.com') end - it 'prevents duplicate workers under race conditions' do + it 'prevents duplicate jobs under race conditions' do # Create 5 threads that simultaneously try to enqueue workers for the same conversation threads = Array.new(5) do Thread.new do @@ -73,24 +65,24 @@ describe Messages::SendEmailNotificationService do threads.each(&:join) - # Only ONE worker should be scheduled despite 5 concurrent attempts - jobs_for_conversation = ConversationReplyEmailWorker.jobs.select { |job| job['args'].first == conversation.id } + # Only ONE job should be scheduled despite 5 concurrent attempts + jobs_for_conversation = ActiveJob::Base.queue_adapter.enqueued_jobs.select do |job| + job[:job] == ConversationReplyEmailJob && job[:args].first == conversation.id + end expect(jobs_for_conversation.size).to eq(1) end end context 'when email notification should not be sent' do before do - allow(ConversationReplyEmailWorker).to receive(:perform_in) + ActiveJob::Base.queue_adapter = :test end context 'when message is not email notifiable' do let(:message) { create(:message, conversation: conversation, message_type: 'incoming') } - it 'does not schedule worker' do - service.perform - - expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) + it 'does not enqueue job' do + expect { service.perform }.not_to have_enqueued_job(ConversationReplyEmailJob) end end @@ -102,10 +94,8 @@ describe Messages::SendEmailNotificationService do conversation.contact.update!(email: nil) end - it 'does not schedule worker' do - service.perform - - expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) + it 'does not enqueue job' do + expect { service.perform }.not_to have_enqueued_job(ConversationReplyEmailJob) end end @@ -117,10 +107,8 @@ describe Messages::SendEmailNotificationService do conversation.contact.update!(email: 'test@example.com') end - it 'does not schedule worker' do - service.perform - - expect(ConversationReplyEmailWorker).not_to have_received(:perform_in) + it 'does not enqueue job' do + expect { service.perform }.not_to have_enqueued_job(ConversationReplyEmailJob) end end end diff --git a/spec/workers/conversation_reply_email_worker_spec.rb b/spec/workers/conversation_reply_email_worker_spec.rb deleted file mode 100644 index 7b28eedde..000000000 --- a/spec/workers/conversation_reply_email_worker_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'rails_helper' - -Sidekiq::Testing.fake! -RSpec.describe ConversationReplyEmailWorker, type: :worker do - let(:conversation) { build(:conversation, display_id: nil) } - let(:message) { build(:message, conversation: conversation, content_type: 'incoming_email', inbox: conversation.inbox) } - let(:mailer) { double } - let(:mailer_action) { double } - - describe 'testing ConversationSummaryEmailWorker' do - before do - conversation.save! - allow(Conversation).to receive(:find).and_return(conversation) - allow(ConversationReplyMailer).to receive(:with).and_return(mailer) - allow(ConversationReplyMailer).to receive(:with).and_return(mailer) - allow(mailer).to receive(:reply_with_summary).and_return(mailer_action) - allow(mailer).to receive(:reply_without_summary).and_return(mailer_action) - allow(mailer_action).to receive(:deliver_later).and_return(true) - end - - it 'worker jobs are enqueued in the mailers queue' do - described_class.perform_async - expect(described_class.queue).to eq(:mailers) - end - - it 'goes into the jobs array for testing environment' do - expect do - described_class.perform_async - end.to change(described_class.jobs, :size).by(1) - described_class.new.perform(1, message.id) - end - - context 'with actions performed by the worker' do - it 'calls ConversationSummaryMailer#reply_with_summary when last incoming message was not email' do - described_class.new.perform(1, message.id) - expect(mailer).to have_received(:reply_with_summary) - end - - it 'calls ConversationSummaryMailer#reply_without_summary when last incoming message was from email' do - message.save! - described_class.new.perform(1, message.id) - expect(mailer).to have_received(:reply_without_summary) - end - end - end -end From 9898ccee9eb5fb887b88d6a76063fb687f28940a Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 22 Oct 2025 20:23:37 -0700 Subject: [PATCH 002/109] chore: Enforce custom role permissions on conversation access (#12583) ## Summary - ensure conversation lookup uses the permission filter before fetching records - add request specs covering custom role access to unassigned conversations ## Testing - bundle exec rspec spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb ------ https://chatgpt.com/codex/tasks/task_e_68de1f62b9b883268a54882e608a8bb8 --- .../accounts/conversations/base_controller.rb | 2 +- .../v1/accounts/conversations_controller.rb | 2 +- .../accounts/integrations/dyte_controller.rb | 2 +- .../concerns/access_token_auth_helper.rb | 1 + app/policies/conversation_policy.rb | 40 ++++++++++- .../enterprise/conversation_policy.rb | 42 +++++++++++ spec/controllers/api/base_controller_spec.rb | 23 ++++++ .../accounts/conversations_controller_spec.rb | 71 +++++++++++++++++++ .../policies/conversation_policy_spec.rb | 65 +++++++++++++++++ spec/policies/conversation_policy_spec.rb | 45 +++++++++++- 10 files changed, 286 insertions(+), 7 deletions(-) create mode 100644 enterprise/app/policies/enterprise/conversation_policy.rb create mode 100644 spec/enterprise/policies/conversation_policy_spec.rb diff --git a/app/controllers/api/v1/accounts/conversations/base_controller.rb b/app/controllers/api/v1/accounts/conversations/base_controller.rb index 500c7772f..223530e27 100644 --- a/app/controllers/api/v1/accounts/conversations/base_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/base_controller.rb @@ -5,6 +5,6 @@ class Api::V1::Accounts::Conversations::BaseController < Api::V1::Accounts::Base def conversation @conversation ||= Current.account.conversations.find_by!(display_id: params[:conversation_id]) - authorize @conversation.inbox, :show? + authorize @conversation, :show? end end diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index e27869d82..4301eaa4a 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -160,7 +160,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro def conversation @conversation ||= Current.account.conversations.find_by!(display_id: params[:id]) - authorize @conversation.inbox, :show? + authorize @conversation, :show? end def inbox diff --git a/app/controllers/api/v1/accounts/integrations/dyte_controller.rb b/app/controllers/api/v1/accounts/integrations/dyte_controller.rb index c5f795d34..845caab5e 100644 --- a/app/controllers/api/v1/accounts/integrations/dyte_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/dyte_controller.rb @@ -22,7 +22,7 @@ class Api::V1::Accounts::Integrations::DyteController < Api::V1::Accounts::BaseC private def authorize_request - authorize @conversation.inbox, :show? + authorize @conversation, :show? end def render_response(response) diff --git a/app/controllers/concerns/access_token_auth_helper.rb b/app/controllers/concerns/access_token_auth_helper.rb index 9b0f9021f..338b290da 100644 --- a/app/controllers/concerns/access_token_auth_helper.rb +++ b/app/controllers/concerns/access_token_auth_helper.rb @@ -14,6 +14,7 @@ module AccessTokenAuthHelper ensure_access_token render_unauthorized('Invalid Access Token') && return if @access_token.blank? + # NOTE: This ensures that current_user is set and available for the rest of the controller actions @resource = @access_token.owner Current.user = @resource if allowed_current_user_type?(@resource) end diff --git a/app/policies/conversation_policy.rb b/app/policies/conversation_policy.rb index 931e17435..d23f29e54 100644 --- a/app/policies/conversation_policy.rb +++ b/app/policies/conversation_policy.rb @@ -4,6 +4,44 @@ class ConversationPolicy < ApplicationPolicy end def destroy? - @account_user&.administrator? + administrator? + end + + def show? + administrator? || agent_bot? || agent_can_view_conversation? + end + + private + + def agent_can_view_conversation? + inbox_access? || team_access? + end + + def administrator? + account_user&.administrator? + end + + def agent_bot? + user.is_a?(AgentBot) + end + + def inbox_access? + user.inboxes.where(account_id: account&.id).exists?(id: record.inbox_id) + end + + def team_access? + return false if record.team_id.blank? + + user.teams.where(account_id: account&.id).exists?(id: record.team_id) + end + + def assigned_to_user? + record.assignee_id == user.id + end + + def participant? + record.conversation_participants.exists?(user_id: user.id) end end + +ConversationPolicy.prepend_mod_with('ConversationPolicy') diff --git a/enterprise/app/policies/enterprise/conversation_policy.rb b/enterprise/app/policies/enterprise/conversation_policy.rb new file mode 100644 index 000000000..d956db2d7 --- /dev/null +++ b/enterprise/app/policies/enterprise/conversation_policy.rb @@ -0,0 +1,42 @@ +module Enterprise::ConversationPolicy + def show? + return false unless super + return true unless custom_role_permissions? + + permissions = custom_role_permissions + return true if manage_all_conversations?(permissions) + return true if permits_unassigned_manage?(permissions) + + permits_participating?(permissions) + end + + private + + def manage_all_conversations?(permissions) + permissions.include?('conversation_manage') + end + + def permits_unassigned_manage?(permissions) + return false unless permissions.include?('conversation_unassigned_manage') + + unassigned_conversation? || assigned_to_user? + end + + def permits_participating?(permissions) + return false unless permissions.include?('conversation_participating_manage') + + assigned_to_user? || participant? + end + + def unassigned_conversation? + record.assignee_id.nil? + end + + def custom_role_permissions? + account_user&.custom_role_id.present? + end + + def custom_role_permissions + account_user&.custom_role&.permissions || [] + end +end diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index ac2306d14..69e4f5cae 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -5,6 +5,29 @@ RSpec.describe 'API Base', type: :request do let!(:user) { create(:user, account: account) } describe 'request with api_access_token for user' do + context 'when accessing an account scoped resource' do + let!(:admin) { create(:user, :administrator, account: account) } + let!(:conversation) { create(:conversation, account: account) } + + it 'sets Current attributes for the request and then returns the response' do + # expect Current.account_user is set to the admin's account_user + allow(Current).to receive(:user=).and_call_original + allow(Current).to receive(:account=).and_call_original + allow(Current).to receive(:account_user=).and_call_original + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", + headers: { api_access_token: admin.access_token.token }, + as: :json + + expect(Current).to have_received(:user=).with(admin).at_least(:once) + expect(Current).to have_received(:account=).with(account).at_least(:once) + expect(Current).to have_received(:account_user=).with(admin.account_users.first).at_least(:once) + + expect(response).to have_http_status(:success) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + end + context 'when it is an invalid api_access_token' do it 'returns unauthorized' do get '/api/v1/profile', diff --git a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb index 4d5269cde..bbb3f1eb3 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -30,5 +30,76 @@ RSpec.describe 'Conversations API', type: :request do expect(response.parsed_body.keys).not_to include('applied_sla') expect(response.parsed_body.keys).not_to include('sla_events') end + + context 'when agent has team access' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:team) { create(:team, account: account) } + let(:conversation) { create(:conversation, account: account, team: team) } + + before do + create(:team_member, team: team, user: agent) + end + + it 'allows accessing the conversation via team membership' do + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + end + + context 'when agent has a custom role' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:conversation) { create(:conversation, account: account) } + + before do + create(:inbox_member, user: agent, inbox: conversation.inbox) + end + + it 'returns unauthorized for unassigned conversation without permission' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) + account.account_users.find_by(user_id: agent.id).update!(custom_role: custom_role) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:unauthorized) + end + + it 'returns the conversation when permission allows managing unassigned conversations, including when assigned to agent' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_unassigned_manage']) + account_user = account.account_users.find_by(user_id: agent.id) + account_user.update!(custom_role: custom_role) + conversation.update!(assignee: agent) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + + it 'returns the conversation when permission allows managing assigned conversations' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) + account_user = account.account_users.find_by(user_id: agent.id) + account_user.update!(custom_role: custom_role) + conversation.update!(assignee: agent) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + + it 'returns the conversation when permission allows managing participating conversations' do + custom_role = create(:custom_role, account: account, permissions: ['conversation_participating_manage']) + account_user = account.account_users.find_by(user_id: agent.id) + account_user.update!(custom_role: custom_role) + create(:conversation_participant, conversation: conversation, account: account, user: agent) + + get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: agent.create_new_auth_token + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['id']).to eq(conversation.display_id) + end + end end end diff --git a/spec/enterprise/policies/conversation_policy_spec.rb b/spec/enterprise/policies/conversation_policy_spec.rb new file mode 100644 index 000000000..e48a84852 --- /dev/null +++ b/spec/enterprise/policies/conversation_policy_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +RSpec.describe ConversationPolicy, type: :policy do + subject { described_class } + + let(:account) { create(:account) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:inbox) { create(:inbox, account: account) } + let(:agent_account_user) { agent.account_users.find_by(account: account) } + let(:context) { { user: agent, account: account, account_user: agent_account_user } } + + before do + create(:inbox_member, user: agent, inbox: inbox) + end + + permissions :show? do + context 'when role grants conversation_unassigned_manage' do + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_unassigned_manage']) } + + before do + agent_account_user.update!(role: :agent, custom_role: custom_role) + end + + it 'allows access to conversations assigned to the agent' do + conversation = create(:conversation, account: account, inbox: inbox, assignee: agent) + + expect(subject).to permit(context, conversation) + end + + it 'denies access to conversations assigned to someone else' do + other_agent = create(:user, account: account, role: :agent) + conversation = create(:conversation, account: account, inbox: inbox, assignee: other_agent) + + expect(subject).not_to permit(context, conversation) + end + end + + context 'when role grants conversation_participating_manage' do + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_participating_manage']) } + + before do + agent_account_user.update!(role: :agent, custom_role: custom_role) + end + + it 'allows access to conversations assigned to the agent' do + conversation = create(:conversation, account: account, inbox: inbox, assignee: agent) + + expect(subject).to permit(context, conversation) + end + + it 'allows access to conversations where the agent is a participant' do + conversation = create(:conversation, account: account, inbox: inbox, assignee: nil) + create(:conversation_participant, conversation: conversation, account: account, user: agent) + + expect(subject).to permit(context, conversation) + end + + it 'denies access to unrelated conversations' do + conversation = create(:conversation, account: account, inbox: inbox, assignee: nil) + + expect(subject).not_to permit(context, conversation) + end + end + end +end diff --git a/spec/policies/conversation_policy_spec.rb b/spec/policies/conversation_policy_spec.rb index ecc3134fc..d75a6bc3a 100644 --- a/spec/policies/conversation_policy_spec.rb +++ b/spec/policies/conversation_policy_spec.rb @@ -4,11 +4,12 @@ RSpec.describe ConversationPolicy, type: :policy do subject { described_class } let(:account) { create(:account) } - let(:conversation) { create(:conversation, account: account) } let(:administrator) { create(:user, account: account, role: :administrator) } let(:agent) { create(:user, account: account, role: :agent) } - let(:administrator_context) { { user: administrator, account: account, account_user: administrator.account_users.first } } - let(:agent_context) { { user: agent, account: account, account_user: agent.account_users.first } } + let(:administrator_context) { { user: administrator, account: account, account_user: administrator.account_users.find_by(account: account) } } + let(:agent_context) { { user: agent, account: account, account_user: agent.account_users.find_by(account: account) } } + + let(:conversation) { create(:conversation, account: account) } permissions :destroy? do context 'when user is an administrator' do @@ -31,4 +32,42 @@ RSpec.describe ConversationPolicy, type: :policy do end end end + + permissions :show? do + context 'when user is an administrator' do + it 'allows access' do + expect(subject).to permit(administrator_context, conversation) + end + end + + context 'when agent has inbox access' do + let(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + + before { create(:inbox_member, user: agent, inbox: inbox) } + + it 'allows access' do + expect(subject).to permit(agent_context, conversation) + end + end + + context 'when agent has team access' do + let(:team) { create(:team, account: account) } + let(:conversation) { create(:conversation, :with_team, account: account, team: team) } + + before { create(:team_member, team: team, user: agent) } + + it 'allows access' do + expect(subject).to permit(agent_context, conversation) + end + end + + context 'when agent lacks inbox and team access' do + let(:conversation) { create(:conversation, account: account) } + + it 'denies access' do + expect(subject).not_to permit(agent_context, conversation) + end + end + end end From 74fc9c0321936d3cf95fb0528236ceee6a078425 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 23 Oct 2025 13:51:07 +0530 Subject: [PATCH 003/109] fix: parameterize agent name (#12709) --- enterprise/app/models/captain/assistant.rb | 2 +- enterprise/app/models/captain/scenario.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/app/models/captain/assistant.rb b/enterprise/app/models/captain/assistant.rb index 771360659..4f039d57a 100644 --- a/enterprise/app/models/captain/assistant.rb +++ b/enterprise/app/models/captain/assistant.rb @@ -88,7 +88,7 @@ class Captain::Assistant < ApplicationRecord private def agent_name - name + name.parameterize(separator: '_') end def agent_tools diff --git a/enterprise/app/models/captain/scenario.rb b/enterprise/app/models/captain/scenario.rb index d876a7127..c43468804 100644 --- a/enterprise/app/models/captain/scenario.rb +++ b/enterprise/app/models/captain/scenario.rb @@ -56,7 +56,7 @@ class Captain::Scenario < ApplicationRecord private def agent_name - "#{title} Agent".titleize + "#{title} Agent".parameterize(separator: '_') end def agent_tools From 5f97a2fd7bf2058697c1714987b43b798906d398 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Fri, 24 Oct 2025 23:01:30 +0530 Subject: [PATCH 004/109] chore: Remove channel icons from the create inbox page (#12727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request Template ## Description This PR removes the frame containing all channel icons from the “Create Inbox” page. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### Screenshots **Before** image **After** image ## 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 --- .../settings/inbox/InboxChannels.vue | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/InboxChannels.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/InboxChannels.vue index 68dba3cda..f69a50064 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/inbox/InboxChannels.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/InboxChannels.vue @@ -6,7 +6,6 @@ import { useMapGetter } from 'dashboard/composables/store'; import { useBranding } from 'shared/composables/useBranding'; import PageHeader from '../SettingsSubPageHeader.vue'; -import Icon from 'next/icon/Icon.vue'; const { t } = useI18n(); const route = useRoute(); @@ -14,22 +13,6 @@ const { replaceInstallationName } = useBranding(); const globalConfig = useMapGetter('globalConfig/get'); -const ALL_CHANNEL_ICONS = [ - 'i-woot-line', - 'i-woot-facebook', - 'i-woot-whatsapp', - 'i-woot-instagram', - 'i-woot-messenger', - 'i-woot-website', - 'i-woot-mail', - 'i-woot-sms', - 'i-woot-telegram', - 'i-woot-api', - 'i-woot-twilio', - 'i-woot-gmail', - 'i-woot-outlook', -]; - const createFlowSteps = computed(() => { const steps = ['CHANNEL', 'INBOX', 'AGENT', 'FINISH']; @@ -78,18 +61,6 @@ const items = computed(() => { diff --git a/app/javascript/dashboard/components-next/sidebar/SidebarChangelogCard.vue b/app/javascript/dashboard/components-next/sidebar/SidebarChangelogCard.vue new file mode 100644 index 000000000..8da24678d --- /dev/null +++ b/app/javascript/dashboard/components-next/sidebar/SidebarChangelogCard.vue @@ -0,0 +1,110 @@ + + + diff --git a/app/javascript/dashboard/i18n/locale/en/integrations.json b/app/javascript/dashboard/i18n/locale/en/integrations.json index c65d2d040..e0c25e39d 100644 --- a/app/javascript/dashboard/i18n/locale/en/integrations.json +++ b/app/javascript/dashboard/i18n/locale/en/integrations.json @@ -861,6 +861,7 @@ }, "RESPONSES": { "HEADER": "FAQs", + "PENDING_FAQS": "Pending FAQs", "ADD_NEW": "Create new FAQ", "DOCUMENTABLE": { "CONVERSATION": "Conversation #{id}" @@ -900,6 +901,10 @@ "APPROVED": "Approved", "ALL": "All" }, + "PENDING_BANNER": { + "TITLE": "Captain has found some FAQs your customers were looking for.", + "ACTION": "Click here to review" + }, "FORM_DESCRIPTION": "Add a question and its corresponding answer to the knowledge base and select the assistant it should be associated with.", "CREATE": { "TITLE": "Add an FAQ", @@ -931,9 +936,9 @@ "APPROVE_SUCCESS_MESSAGE": "The FAQ was marked as approved" }, "OPTIONS": { - "APPROVE": "Mark as approved", - "EDIT_RESPONSE": "Edit FAQ", - "DELETE_RESPONSE": "Delete FAQ" + "APPROVE": "Approve", + "EDIT_RESPONSE": "Edit", + "DELETE_RESPONSE": "Delete" }, "EMPTY_STATE": { "TITLE": "No FAQs Found", diff --git a/app/javascript/dashboard/routes/dashboard/captain/captain.routes.js b/app/javascript/dashboard/routes/dashboard/captain/captain.routes.js index 9d5609ab7..12c5c53f3 100644 --- a/app/javascript/dashboard/routes/dashboard/captain/captain.routes.js +++ b/app/javascript/dashboard/routes/dashboard/captain/captain.routes.js @@ -10,6 +10,7 @@ import AssistantGuidelinesIndex from './assistants/guidelines/Index.vue'; import AssistantScenariosIndex from './assistants/scenarios/Index.vue'; import DocumentsIndex from './documents/Index.vue'; import ResponsesIndex from './responses/Index.vue'; +import ResponsesPendingIndex from './responses/Pending.vue'; import CustomToolsIndex from './tools/Index.vue'; export const routes = [ @@ -125,6 +126,19 @@ export const routes = [ ], }, }, + { + path: frontendURL('accounts/:accountId/captain/responses/pending'), + component: ResponsesPendingIndex, + name: 'captain_responses_pending', + meta: { + permissions: ['administrator', 'agent'], + featureFlag: FEATURE_FLAGS.CAPTAIN, + installationTypes: [ + INSTALLATION_TYPES.CLOUD, + INSTALLATION_TYPES.ENTERPRISE, + ], + }, + }, { path: frontendURL('accounts/:accountId/captain/tools'), component: CustomToolsIndex, diff --git a/app/javascript/dashboard/routes/dashboard/captain/responses/Index.vue b/app/javascript/dashboard/routes/dashboard/captain/responses/Index.vue index a20b96e45..4ed117806 100644 --- a/app/javascript/dashboard/routes/dashboard/captain/responses/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/captain/responses/Index.vue @@ -1,17 +1,15 @@ @@ -276,8 +212,8 @@ onMounted(() => { :header-title="$t('CAPTAIN.RESPONSES.HEADER')" :button-label="$t('CAPTAIN.RESPONSES.ADD_NEW')" :is-fetching="isFetching" - :is-empty="!filteredResponses.length" - :show-pagination-footer="!isFetching && !!filteredResponses.length" + :is-empty="!responses.length" + :show-pagination-footer="!isFetching && !!responses.length" :feature-flag="FEATURE_FLAGS.CAPTAIN" @update:current-page="onPageChange" @click="handleCreate" @@ -315,25 +251,7 @@ onMounted(() => { v-if="!bulkSelectionState.hasSelected" class="flex gap-3 justify-between w-full items-center" > -
- -