diff --git a/app/controllers/api/v1/accounts/callbacks_controller.rb b/app/controllers/api/v1/accounts/callbacks_controller.rb index 90cdf2418..08c0ffe43 100644 --- a/app/controllers/api/v1/accounts/callbacks_controller.rb +++ b/app/controllers/api/v1/accounts/callbacks_controller.rb @@ -6,6 +6,7 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController page_access_token = params[:page_access_token] page_id = params[:page_id] inbox_name = params[:inbox_name] + ActiveRecord::Base.transaction do facebook_channel = Current.account.facebook_pages.create!( page_id: page_id, user_access_token: user_access_token, @@ -15,6 +16,8 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController set_instagram_id(page_access_token, facebook_channel) set_avatar(@facebook_inbox, page_id) end + rescue CustomExceptions::Inbox::LimitExceeded => e + render_error_response(e) rescue StandardError => e ChatwootExceptionTracker.new(e).capture_exception Rails.logger.error "Error in register_facebook_page: #{e.message}" diff --git a/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb b/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb index f3b14d49f..1691b5489 100644 --- a/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb +++ b/app/controllers/api/v1/accounts/channels/twilio_channels_controller.rb @@ -6,6 +6,8 @@ class Api::V1::Accounts::Channels::TwilioChannelsController < Api::V1::Accounts: def create process_create + rescue CustomExceptions::Inbox::LimitExceeded => e + render_error_response(e) rescue StandardError => e render_could_not_create_error(e.message) end diff --git a/app/controllers/api/v1/accounts/inboxes_controller.rb b/app/controllers/api/v1/accounts/inboxes_controller.rb index 757af9b62..9f56c3817 100644 --- a/app/controllers/api/v1/accounts/inboxes_controller.rb +++ b/app/controllers/api/v1/accounts/inboxes_controller.rb @@ -2,7 +2,6 @@ class Api::V1::Accounts::InboxesController < Api::V1::Accounts::BaseController include Api::V1::InboxesHelper before_action :fetch_inbox, except: [:index, :create] before_action :fetch_agent_bot, only: [:set_agent_bot] - before_action :validate_limit, only: [:create] # we are already handling the authorization in fetch inbox before_action :check_authorization, except: [:show] diff --git a/app/controllers/api/v1/accounts/whatsapp/authorizations_controller.rb b/app/controllers/api/v1/accounts/whatsapp/authorizations_controller.rb index d52f396fc..db94113d9 100644 --- a/app/controllers/api/v1/accounts/whatsapp/authorizations_controller.rb +++ b/app/controllers/api/v1/accounts/whatsapp/authorizations_controller.rb @@ -8,8 +8,10 @@ class Api::V1::Accounts::Whatsapp::AuthorizationsController < Api::V1::Accounts: validate_embedded_signup_params! channel = process_embedded_signup render_success_response(channel.inbox) - rescue StandardError => e + rescue CustomExceptions::Inbox::LimitExceeded => e render_error_response(e) + rescue StandardError => e + render_embedded_signup_error(e) end private @@ -55,7 +57,7 @@ class Api::V1::Accounts::Whatsapp::AuthorizationsController < Api::V1::Accounts: render json: response end - def render_error_response(error) + def render_embedded_signup_error(error) Rails.logger.error "[WHATSAPP AUTHORIZATION] Embedded signup error: #{error.message}" Rails.logger.error error.backtrace.join("\n") render json: { diff --git a/app/controllers/concerns/request_exception_handler.rb b/app/controllers/concerns/request_exception_handler.rb index 7f4e313b1..43d6edf1f 100644 --- a/app/controllers/concerns/request_exception_handler.rb +++ b/app/controllers/concerns/request_exception_handler.rb @@ -9,6 +9,7 @@ module RequestExceptionHandler included do rescue_from ActiveRecord::RecordInvalid, with: :render_record_invalid + rescue_from CustomExceptions::Inbox::LimitExceeded, with: :render_error_response end private @@ -40,8 +41,8 @@ module RequestExceptionHandler render json: { error: message }, status: :not_found end - def render_could_not_create_error(message) - render json: { error: sanitized_error_message(message) }, status: :unprocessable_entity + def render_could_not_create_error(error) + render json: { error: sanitized_error_message(error) }, status: :unprocessable_entity end def render_payment_required(message) diff --git a/app/controllers/instagram/callbacks_controller.rb b/app/controllers/instagram/callbacks_controller.rb index cd317363c..e9065119f 100644 --- a/app/controllers/instagram/callbacks_controller.rb +++ b/app/controllers/instagram/callbacks_controller.rb @@ -11,6 +11,8 @@ class Instagram::CallbacksController < ApplicationController end process_successful_authorization + rescue CustomExceptions::Inbox::LimitExceeded => e + handle_limit_error(e) rescue StandardError => e handle_error(e) end @@ -47,6 +49,14 @@ class Instagram::CallbacksController < ApplicationController redirect_to_error_page(error_info) end + def handle_limit_error(error) + redirect_to_error_page( + 'error_type' => error.class.name, + 'code' => Rack::Utils.status_code(error.http_status), + 'error_message' => error.message + ) + end + # Extract error details from the exception def extract_error_info(error) if error.is_a?(OAuth2::Error) diff --git a/app/controllers/tiktok/callbacks_controller.rb b/app/controllers/tiktok/callbacks_controller.rb index 20c0ee9c0..a39fec5ed 100644 --- a/app/controllers/tiktok/callbacks_controller.rb +++ b/app/controllers/tiktok/callbacks_controller.rb @@ -6,6 +6,8 @@ class Tiktok::CallbacksController < ApplicationController return handle_ungranted_scopes_error unless all_scopes_granted? process_successful_authorization + rescue CustomExceptions::Inbox::LimitExceeded => e + handle_limit_error(e) rescue StandardError => e handle_error(e) end @@ -36,6 +38,14 @@ class Tiktok::CallbacksController < ApplicationController redirect_to_error_page(error_type: error.class.name, code: 500, error_message: error.message) end + def handle_limit_error(error) + redirect_to_error_page( + error_type: error.class.name, + code: Rack::Utils.status_code(error.http_status), + error_message: error.message + ) + end + # Handles the case when a user denies permissions or cancels the authorization flow def handle_authorization_error redirect_to_error_page( diff --git a/app/helpers/api/v1/inboxes_helper.rb b/app/helpers/api/v1/inboxes_helper.rb index 8a10fa99c..6c64dd009 100644 --- a/app/helpers/api/v1/inboxes_helper.rb +++ b/app/helpers/api/v1/inboxes_helper.rb @@ -114,10 +114,4 @@ module Api::V1::InboxesHelper 'sms' => Current.account.sms_channels }[permitted_params[:channel][:type]] end - - def validate_limit - return unless Current.account.inboxes.count >= Current.account.usage_limits[:inboxes] - - render_payment_required('Account limit exceeded. Upgrade to a higher plan') - end end diff --git a/enterprise/app/models/enterprise/concerns/inbox.rb b/enterprise/app/models/enterprise/concerns/inbox.rb index bdcd0fd63..b327878b4 100644 --- a/enterprise/app/models/enterprise/concerns/inbox.rb +++ b/enterprise/app/models/enterprise/concerns/inbox.rb @@ -8,5 +8,11 @@ module Enterprise::Concerns::Inbox class_name: 'Captain::Assistant' has_many :inbox_capacity_limits, dependent: :destroy has_many :calls, dependent: :destroy_async + + before_create :ensure_create_permitted + end + + def ensure_create_permitted + raise CustomExceptions::Inbox::LimitExceeded.new({}) if account.inboxes.count >= account.usage_limits[:inboxes] end end diff --git a/lib/custom_exceptions/inbox/limit_exceeded.rb b/lib/custom_exceptions/inbox/limit_exceeded.rb new file mode 100644 index 000000000..9b5624929 --- /dev/null +++ b/lib/custom_exceptions/inbox/limit_exceeded.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CustomExceptions::Inbox::LimitExceeded < CustomExceptions::Base + def message + 'Account limit exceeded. Upgrade to a higher plan' + end + + def to_hash + { error: message } + end + + def http_status + :payment_required + end +end diff --git a/spec/enterprise/controllers/api/v1/accounts/callbacks_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/callbacks_controller_spec.rb new file mode 100644 index 000000000..33cc95fc1 --- /dev/null +++ b/spec/enterprise/controllers/api/v1/accounts/callbacks_controller_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Enterprise Callbacks API', type: :request do + describe 'POST /api/v1/accounts/{account.id}/callbacks/register_facebook_page' do + let(:account) { create(:account, limits: { inboxes: 1 }) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:params) do + { + user_access_token: 'user-token', + page_access_token: 'page-token', + page_id: '12345', + inbox_name: 'Facebook Inbox' + } + end + + before do + create(:inbox, account: account) + end + + it 'returns payment required before creating a Facebook channel when account inbox limit is reached' do + expect do + post "/api/v1/accounts/#{account.id}/callbacks/register_facebook_page", + headers: admin.create_new_auth_token, + params: params, + as: :json + end.not_to change(Channel::FacebookPage, :count) + + expect(response).to have_http_status(:payment_required) + expect(response.parsed_body['error']).to eq('Account limit exceeded. Upgrade to a higher plan') + end + end +end diff --git a/spec/enterprise/lib/captain/base_task_service_spec.rb b/spec/enterprise/lib/captain/base_task_service_spec.rb index fb970f726..9186874b7 100644 --- a/spec/enterprise/lib/captain/base_task_service_spec.rb +++ b/spec/enterprise/lib/captain/base_task_service_spec.rb @@ -5,6 +5,13 @@ RSpec.describe Captain::BaseTaskService, type: :model do let(:inbox) { create(:inbox, account: account) } let(:conversation) { create(:conversation, account: account, inbox: inbox) } let(:perform_result) { { message: 'Test response' } } + let(:exhausted_usage_limits) do + { + agents: ChatwootApp.max_limit, + inboxes: ChatwootApp.max_limit, + captain: { responses: { current_available: 0 } } + } + end # Create a concrete test service class with enterprise module prepended let(:test_service_class) do @@ -38,9 +45,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do context 'when usage limit is exceeded' do before do allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) - allow(account).to receive(:usage_limits).and_return({ - captain: { responses: { current_available: 0 } } - }) + allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits) end it 'returns usage limit exceeded error' do @@ -125,9 +130,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do context 'when the captain_responses quota is exhausted on Cloud' do before do allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) - allow(account).to receive(:usage_limits).and_return({ - captain: { responses: { current_available: 0 } } - }) + allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits) end it 'returns usage limit exceeded error for services that do not opt into BYOK' do @@ -162,9 +165,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do context 'when the captain_responses quota is exhausted on Cloud' do before do allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) - allow(account).to receive(:usage_limits).and_return({ - captain: { responses: { current_available: 0 } } - }) + allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits) end it 'bypasses the 429 gate and returns the underlying result' do @@ -249,9 +250,7 @@ RSpec.describe Captain::BaseTaskService, type: :model do context 'when the captain_responses quota is exhausted on Cloud' do before do allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) - allow(account).to receive(:usage_limits).and_return({ - captain: { responses: { current_available: 0 } } - }) + allow(account).to receive(:usage_limits).and_return(exhausted_usage_limits) end it 'bypasses the 429 gate and returns the underlying result' do diff --git a/spec/enterprise/lib/captain/conversation_completion_service_spec.rb b/spec/enterprise/lib/captain/conversation_completion_service_spec.rb index 8b059acc3..80b9ab1d8 100644 --- a/spec/enterprise/lib/captain/conversation_completion_service_spec.rb +++ b/spec/enterprise/lib/captain/conversation_completion_service_spec.rb @@ -166,9 +166,13 @@ RSpec.describe Captain::ConversationCompletionService do before do allow(ChatwootApp).to receive(:chatwoot_cloud?).and_return(true) - allow(account).to receive(:usage_limits).and_return({ - captain: { responses: { current_available: 0 } } - }) + allow(account).to receive(:usage_limits).and_return( + { + agents: ChatwootApp.max_limit, + inboxes: ChatwootApp.max_limit, + captain: { responses: { current_available: 0 } } + } + ) create(:message, conversation: conversation, message_type: :incoming, content: 'What are your hours?') create(:message, conversation: conversation, message_type: :outgoing, content: 'We are open 9-5 Monday to Friday.') allow(mock_chat).to receive(:ask).and_return(mock_response) diff --git a/spec/enterprise/models/inbox_spec.rb b/spec/enterprise/models/inbox_spec.rb index cfcbdd573..1dce2833b 100644 --- a/spec/enterprise/models/inbox_spec.rb +++ b/spec/enterprise/models/inbox_spec.rb @@ -134,6 +134,29 @@ RSpec.describe Inbox do end end + describe 'validations' do + describe 'account inbox limit' do + let(:account) { create(:account, limits: { inboxes: 1 }) } + + before do + create(:inbox, account: account) + end + + it 'prevents saving inboxes beyond the account limit' do + new_inbox = build(:inbox, account: account) + + expect { new_inbox.save! }.to raise_error(CustomExceptions::Inbox::LimitExceeded, 'Account limit exceeded. Upgrade to a higher plan') + end + + it 'does not block updates to existing inboxes when the account is at the limit' do + inbox = account.inboxes.first + inbox.name = 'Updated Inbox' + + expect(inbox).to be_valid + end + end + end + describe 'audit log' do context 'when inbox is created' do it 'has associated audit log created' do diff --git a/spec/enterprise/services/messages/audio_transcription_service_spec.rb b/spec/enterprise/services/messages/audio_transcription_service_spec.rb index 265ce6c33..4881e8cf1 100644 --- a/spec/enterprise/services/messages/audio_transcription_service_spec.rb +++ b/spec/enterprise/services/messages/audio_transcription_service_spec.rb @@ -12,7 +12,13 @@ RSpec.describe Messages::AudioTranscriptionService, type: :service do InstallationConfig.find_or_create_by!(name: 'CAPTAIN_OPEN_AI_MODEL') { |config| config.value = 'gpt-4o-mini' } # Mock usage limits for transcription to be available - allow(account).to receive(:usage_limits).and_return({ captain: { responses: { current_available: 100 } } }) + allow(account).to receive(:usage_limits).and_return( + { + agents: ChatwootApp.max_limit, + inboxes: ChatwootApp.max_limit, + captain: { responses: { current_available: 100 } } + } + ) end describe '#perform' do diff --git a/spec/lib/captain/base_task_service_spec.rb b/spec/lib/captain/base_task_service_spec.rb index b24a5c49c..2cb24ce04 100644 --- a/spec/lib/captain/base_task_service_spec.rb +++ b/spec/lib/captain/base_task_service_spec.rb @@ -385,7 +385,10 @@ RSpec.describe Captain::BaseTaskService do describe '#prompt_from_file' do it 'reads prompt from file' do - allow(Rails.root).to receive(:join).and_return(instance_double(Pathname, read: 'Test prompt content')) + service + prompt_path = instance_double(Pathname, read: 'Test prompt content') + allow(Rails.root).to receive(:join).with('lib/integrations/openai/openai_prompts', 'test.liquid').and_return(prompt_path) + expect(service.send(:prompt_from_file, 'test')).to eq('Test prompt content') end end diff --git a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb index 75abc8518..988fb0116 100644 --- a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb +++ b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb @@ -32,6 +32,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do before do account.enable_features('crm_integration') + allow(GlobalConfig).to receive(:get).and_return({}) allow(GlobalConfig).to receive(:get).with('BRAND_NAME').and_return({ 'BRAND_NAME' => 'TestBrand' }) end diff --git a/spec/services/whatsapp/channel_creation_service_spec.rb b/spec/services/whatsapp/channel_creation_service_spec.rb index 983af6c78..e7016f6a4 100644 --- a/spec/services/whatsapp/channel_creation_service_spec.rb +++ b/spec/services/whatsapp/channel_creation_service_spec.rb @@ -60,6 +60,17 @@ describe Whatsapp::ChannelCreationService do expect(inbox.name).to eq('Test Business WhatsApp') expect(inbox.account).to eq(account) end + + it 'does not leave an orphan channel when inbox creation fails' do + allow(Inbox).to receive(:create!).and_wrap_original do |method, *args| + method.call(*args) + raise ActiveRecord::RecordInvalid, Inbox.new + end + + expect do + expect { service.perform }.to raise_error(ActiveRecord::RecordInvalid) + end.not_to change(Channel::Whatsapp, :count) + end end context 'when channel already exists for the phone number' do