diff --git a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder index 5fdd4ecee..538123ac1 100644 --- a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder +++ b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder @@ -58,6 +58,6 @@ json.last_non_activity_message conversation.messages.where(account_id: conversat json.last_activity_at conversation.last_activity_at.to_i json.priority conversation.priority json.waiting_since conversation.waiting_since.to_i.to_i -sla_applicable = !conversation.respond_to?(:sla_applicable?) || conversation.sla_applicable? +sla_applicable = conversation.account.feature_enabled?('sla') && (!conversation.respond_to?(:sla_applicable?) || conversation.sla_applicable?) json.sla_policy_id sla_applicable ? conversation.sla_policy_id : nil json.partial! 'enterprise/api/v1/conversations/partials/conversation', conversation: conversation if ChatwootApp.enterprise? diff --git a/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb b/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb index 1ca3c015e..83497a256 100644 --- a/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb @@ -4,6 +4,7 @@ class Api::V1::Accounts::AppliedSlasController < Api::V1::Accounts::EnterpriseAc RESULTS_PER_PAGE = 25 + before_action :ensure_sla_feature_enabled before_action :set_applied_slas, only: [:index, :metrics, :download] before_action :set_current_page, only: [:index] before_action :check_admin_authorization? @@ -30,6 +31,10 @@ class Api::V1::Accounts::AppliedSlasController < Api::V1::Accounts::EnterpriseAc private + def ensure_sla_feature_enabled + raise Pundit::NotAuthorizedError unless Current.account.feature_enabled?('sla') + end + def total_applied_slas @total_applied_slas ||= @applied_slas.count end diff --git a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb index b3b0f52d4..7789a1b59 100644 --- a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb @@ -1,4 +1,5 @@ class Api::V1::Accounts::SlaPoliciesController < Api::V1::Accounts::EnterpriseAccountsController + before_action :ensure_sla_feature_enabled before_action :fetch_sla, only: [:show, :update, :destroy] before_action :check_authorization @@ -29,4 +30,8 @@ class Api::V1::Accounts::SlaPoliciesController < Api::V1::Accounts::EnterpriseAc def fetch_sla @sla_policy = Current.account.sla_policies.find_by(id: params[:id]) end + + def ensure_sla_feature_enabled + raise Pundit::NotAuthorizedError unless Current.account.feature_enabled?('sla') + end end diff --git a/enterprise/app/controllers/enterprise/api/v1/accounts/conversations_controller.rb b/enterprise/app/controllers/enterprise/api/v1/accounts/conversations_controller.rb index f87c06658..c6ff54742 100644 --- a/enterprise/app/controllers/enterprise/api/v1/accounts/conversations_controller.rb +++ b/enterprise/app/controllers/enterprise/api/v1/accounts/conversations_controller.rb @@ -16,6 +16,9 @@ module Enterprise::Api::V1::Accounts::ConversationsController end def permitted_update_params + # SLA is a premium feature; only accept sla_policy_id assignment when it is enabled for the account. + return super unless Current.account.feature_enabled?('sla') + super.merge(params.permit(:sla_policy_id)) end diff --git a/enterprise/app/jobs/sla/process_account_applied_slas_job.rb b/enterprise/app/jobs/sla/process_account_applied_slas_job.rb index 4eb2d182b..20d29ed54 100644 --- a/enterprise/app/jobs/sla/process_account_applied_slas_job.rb +++ b/enterprise/app/jobs/sla/process_account_applied_slas_job.rb @@ -2,6 +2,9 @@ class Sla::ProcessAccountAppliedSlasJob < ApplicationJob queue_as :medium def perform(account) + # The scheduler filters on the feature, but this job can already be queued when a plan is downgraded. + return unless account.feature_enabled?('sla') + account.applied_slas.with_sla_applicable_conversation.where(sla_status: %w[active active_with_misses]).each do |applied_sla| Sla::ProcessAppliedSlaJob.perform_later(applied_sla) end diff --git a/enterprise/app/jobs/sla/process_applied_sla_job.rb b/enterprise/app/jobs/sla/process_applied_sla_job.rb index 10fc22c31..df06c0c5c 100644 --- a/enterprise/app/jobs/sla/process_applied_sla_job.rb +++ b/enterprise/app/jobs/sla/process_applied_sla_job.rb @@ -2,6 +2,9 @@ class Sla::ProcessAppliedSlaJob < ApplicationJob queue_as :medium def perform(applied_sla) + # This job can already be queued when a plan is downgraded, so re-check before evaluating. + return unless applied_sla.account.feature_enabled?('sla') + Sla::EvaluateAppliedSlaService.new(applied_sla: applied_sla).perform end end diff --git a/enterprise/app/jobs/sla/trigger_slas_for_accounts_job.rb b/enterprise/app/jobs/sla/trigger_slas_for_accounts_job.rb index a2a142430..6320578fb 100644 --- a/enterprise/app/jobs/sla/trigger_slas_for_accounts_job.rb +++ b/enterprise/app/jobs/sla/trigger_slas_for_accounts_job.rb @@ -2,7 +2,8 @@ class Sla::TriggerSlasForAccountsJob < ApplicationJob queue_as :scheduled_jobs def perform - Account.joins(:sla_policies).distinct.find_each do |account| + # SLA is a premium feature; skip accounts that have policies left over from a downgrade. + Account.feature_sla.joins(:sla_policies).distinct.find_each do |account| Rails.logger.info "Enqueuing ProcessAccountAppliedSlasJob for account #{account.id}" Sla::ProcessAccountAppliedSlasJob.perform_later(account) end diff --git a/enterprise/app/services/enterprise/action_service.rb b/enterprise/app/services/enterprise/action_service.rb index c841f5054..623e0a472 100644 --- a/enterprise/app/services/enterprise/action_service.rb +++ b/enterprise/app/services/enterprise/action_service.rb @@ -1,6 +1,8 @@ module Enterprise::ActionService def add_sla(sla_policy_id) return if sla_policy_id.blank? + # SLA is a premium feature; automation rules must not keep applying SLAs once it is disabled. + return unless @account.feature_enabled?('sla') sla_policy = @account.sla_policies.find_by(id: sla_policy_id.first) return if sla_policy.nil? diff --git a/spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb index e4b2bfe70..dbf98d1ab 100644 --- a/spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb @@ -12,6 +12,7 @@ RSpec.describe 'Applied SLAs API', type: :request do let(:sla_policy2) { create(:sla_policy, account: account) } before do + account.enable_features!('sla') AppliedSla.destroy_all end @@ -117,6 +118,18 @@ RSpec.describe 'Applied SLAs API', type: :request do expect(body).to include('hit_rate' => '50.0%') end end + + context 'when the sla feature is disabled' do + it 'returns unauthorized' do + account.disable_features!('sla') + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, sla_status: 'missed') + + get "/api/v1/accounts/#{account.id}/applied_slas/metrics", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:unauthorized) + end + end end describe 'GET /api/v1/accounts/{account.id}/applied_slas/download' do 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 dd1adf6e9..3a07a58c5 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -35,16 +35,18 @@ RSpec.describe 'Conversations API', type: :request do end it 'does not return SLA data for the conversation if the feature is disabled' do + account.enable_features!('sla') + sla_policy = create(:sla_policy, account: account) + conversation = create(:conversation, account: account, sla_policy: sla_policy) + create(:sla_event, conversation: conversation, applied_sla: conversation.applied_sla) account.disable_features!('sla') - conversation = create(:conversation, account: account) - create(:applied_sla, conversation: conversation) - create(:sla_event, conversation: conversation) get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: administrator.create_new_auth_token expect(response).to have_http_status(:ok) expect(response.parsed_body.keys).not_to include('applied_sla') expect(response.parsed_body.keys).not_to include('sla_events') + expect(response.parsed_body['sla_policy_id']).to be_nil end context 'when agent has team access' do diff --git a/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb index e1a4fa538..077b586b1 100644 --- a/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/sla_policies_controller_spec.rb @@ -6,6 +6,7 @@ RSpec.describe 'Enterprise SLA API', type: :request do let(:agent) { create(:user, account: account, role: :agent) } before do + account.enable_features!('sla') create(:sla_policy, account: account, name: 'SLA 1') end diff --git a/spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb index 4fe7f46e5..53ef8cb0a 100644 --- a/spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb @@ -13,6 +13,7 @@ RSpec.describe 'Enterprise Conversations API', type: :request do let(:agent) { create(:user, account: account, role: :agent) } before do + account.enable_features!('sla') create(:inbox_member, user: agent, inbox: conversation.inbox) end @@ -50,5 +51,27 @@ RSpec.describe 'Enterprise Conversations API', type: :request do .to eq('Sla policy cannot be assigned to conversations with blocked contacts') end end + + context 'when the sla feature is disabled' do + let(:agent) { create(:user, account: account, role: :agent) } + let(:existing_sla_policy) { create(:sla_policy, account: account) } + + before do + account.enable_features!('sla') + conversation.update!(sla_policy: existing_sla_policy) + account.disable_features!('sla') + create(:inbox_member, user: agent, inbox: conversation.inbox) + end + + it 'ignores the sla assignment and keeps the existing policy' do + patch "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(conversation.reload.sla_policy_id).to eq(existing_sla_policy.id) + end + end end end diff --git a/spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb b/spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb index e99a5087b..8d7eda4a7 100644 --- a/spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb +++ b/spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do let!(:blocked_contact_applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'active') } before do + account.enable_features!('sla') blocked_contact_applied_sla.conversation.contact.update!(blocked: true) end @@ -32,5 +33,12 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(miss_applied_sla) described_class.perform_now(account) end + + it 'does not call the ProcessAppliedSlaJob when the sla feature is disabled' do + account.disable_features!('sla') + + expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later) + described_class.perform_now(account) + end end end diff --git a/spec/enterprise/jobs/sla/process_applied_sla_job_spec.rb b/spec/enterprise/jobs/sla/process_applied_sla_job_spec.rb index 66c8da823..706e27ace 100644 --- a/spec/enterprise/jobs/sla/process_applied_sla_job_spec.rb +++ b/spec/enterprise/jobs/sla/process_applied_sla_job_spec.rb @@ -5,6 +5,8 @@ RSpec.describe Sla::ProcessAppliedSlaJob do let(:account) { create(:account) } let(:applied_sla) { create(:applied_sla, account: account) } + before { account.enable_features!('sla') } + it 'enqueues the job' do expect { described_class.perform_later(applied_sla) }.to have_enqueued_job(described_class) .with(applied_sla) @@ -13,7 +15,14 @@ RSpec.describe Sla::ProcessAppliedSlaJob do it 'calls the EvaluateAppliedSlaService' do expect(Sla::EvaluateAppliedSlaService).to receive(:new).with(applied_sla: applied_sla).and_call_original - described_class.perform_now(applied_sla) + described_class.perform_now(applied_sla.reload) + end + + it 'does not call the EvaluateAppliedSlaService when the sla feature is disabled' do + account.disable_features!('sla') + + expect(Sla::EvaluateAppliedSlaService).not_to receive(:new) + described_class.perform_now(applied_sla.reload) end end end diff --git a/spec/enterprise/jobs/sla/trigger_slas_for_accounts_job_spec.rb b/spec/enterprise/jobs/sla/trigger_slas_for_accounts_job_spec.rb index d02e543ba..d527582e1 100644 --- a/spec/enterprise/jobs/sla/trigger_slas_for_accounts_job_spec.rb +++ b/spec/enterprise/jobs/sla/trigger_slas_for_accounts_job_spec.rb @@ -3,8 +3,10 @@ RSpec.describe Sla::TriggerSlasForAccountsJob do context 'when perform is called' do let(:account_with_sla) { create(:account) } let(:account_without_sla) { create(:account) } + let(:downgraded_account) { create(:account) } before do + account_with_sla.enable_features!('sla') create(:sla_policy, account: account_with_sla) end @@ -22,5 +24,13 @@ RSpec.describe Sla::TriggerSlasForAccountsJob do expect(Sla::ProcessAccountAppliedSlasJob).not_to receive(:perform_later).with(account_without_sla) described_class.perform_now end + + it 'does not call the ProcessAccountAppliedSlasJob for accounts with the sla feature disabled' do + create(:sla_policy, account: downgraded_account) + downgraded_account.disable_features!('sla') + + expect(Sla::ProcessAccountAppliedSlasJob).not_to receive(:perform_later).with(downgraded_account) + described_class.perform_now + end end end diff --git a/spec/enterprise/services/enterprise/action_service_spec.rb b/spec/enterprise/services/enterprise/action_service_spec.rb index 9396dc15d..6fb165972 100644 --- a/spec/enterprise/services/enterprise/action_service_spec.rb +++ b/spec/enterprise/services/enterprise/action_service_spec.rb @@ -8,6 +8,8 @@ describe ActionService do let(:conversation) { create(:conversation, account: account) } let(:action_service) { described_class.new(conversation) } + before { account.enable_features!('sla') } + context 'when sla_policy_id is present' do it 'adds the sla policy to the conversation and create applied_sla entry' do action_service.add_sla([sla_policy.id])