From 7bf76057c2f2dac08234d7344850fb2022fa009a Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Thu, 2 Jul 2026 14:16:29 +0530 Subject: [PATCH] fix: SLA handling for blocked contacts (#14861) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request Template ## Description Blocked contacts are now excluded from SLA assignment, processing, reports, and conversation SLA UI while they remain blocked. Existing SLA records are preserved, and SLA behavior resumes if the contact is unblocked. Fixes https://linear.app/chatwoot/issue/CW-7435/sla-should-not-trigger-for-blocked-contacts ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - `bundle exec rspec spec/enterprise/models/conversation_spec.rb spec/enterprise/models/applied_sla_spec.rb spec/enterprise/services/enterprise/action_service_spec.rb spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb spec/enterprise/presenters/conversations/event_data_presenter_spec.rb` — 78 examples, 0 failures - `bundle exec rubocop enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb enterprise/app/jobs/sla/process_account_applied_slas_job.rb enterprise/app/models/applied_sla.rb enterprise/app/models/enterprise/concerns/conversation.rb enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb enterprise/app/services/enterprise/action_service.rb enterprise/app/services/sla/evaluate_applied_sla_service.rb lib/tasks/apply_sla.rake spec/enterprise/controllers/api/v1/accounts/applied_slas_controller_spec.rb spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb spec/enterprise/controllers/enterprise/api/v1/accounts/conversations_controller_spec.rb spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb spec/enterprise/models/applied_sla_spec.rb spec/enterprise/models/conversation_spec.rb spec/enterprise/presenters/conversations/event_data_presenter_spec.rb spec/enterprise/services/enterprise/action_service_spec.rb spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb` — no offenses - `pnpm exec vitest --no-watch --no-cache --no-coverage app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js` — 2 tests passed - `pnpm exec eslint app/javascript/dashboard/components-next/Conversation/ConversationCard/CardMessagePreviewWithMeta.vue app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCardExpanded.vue app/javascript/dashboard/components/widgets/conversation/ConversationCard.vue app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js` — passed with existing raw-text warnings in `ConversationHeader.vue` - `git diff --cached --check` — clean ## 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 - [x] 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: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Co-authored-by: Muhsin Keloth --- .../CardMessagePreviewWithMeta.vue | 8 ++- .../ConversationCard/ConversationCard.vue | 1 + .../ConversationCardExpanded.vue | 4 +- .../widgets/conversation/ConversationCard.vue | 4 +- .../conversation/ConversationHeader.vue | 4 +- .../specs/ConversationCard.spec.js | 60 +++++++++++++++++++ .../partials/_conversation.json.jbuilder | 3 +- .../v1/accounts/applied_slas_controller.rb | 2 +- .../sla/process_account_applied_slas_job.rb | 2 +- enterprise/app/models/applied_sla.rb | 1 + .../enterprise/concerns/conversation.rb | 11 ++++ .../conversations/event_data_presenter.rb | 18 +++--- .../app/services/enterprise/action_service.rb | 1 + .../sla/evaluate_applied_sla_service.rb | 2 + .../partials/_conversation.json.jbuilder | 17 ++++-- lib/tasks/apply_sla.rake | 4 +- .../accounts/applied_slas_controller_spec.rb | 45 ++++++++++++++ .../accounts/conversations_controller_spec.rb | 15 +++++ .../accounts/conversations_controller_spec.rb | 13 ++++ .../process_account_applied_slas_job_spec.rb | 6 ++ spec/enterprise/models/applied_sla_spec.rb | 14 +++++ spec/enterprise/models/conversation_spec.rb | 24 ++++++++ .../event_data_presenter_spec.rb | 13 ++++ .../enterprise/action_service_spec.rb | 7 +++ .../sla/evaluate_applied_sla_service_spec.rb | 23 +++++++ 25 files changed, 279 insertions(+), 23 deletions(-) create mode 100644 app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js diff --git a/app/javascript/dashboard/components-next/Conversation/ConversationCard/CardMessagePreviewWithMeta.vue b/app/javascript/dashboard/components-next/Conversation/ConversationCard/CardMessagePreviewWithMeta.vue index df2b22b7e..3486816d5 100644 --- a/app/javascript/dashboard/components-next/Conversation/ConversationCard/CardMessagePreviewWithMeta.vue +++ b/app/javascript/dashboard/components-next/Conversation/ConversationCard/CardMessagePreviewWithMeta.vue @@ -16,6 +16,10 @@ const props = defineProps({ type: Array, required: true, }, + contact: { + type: Object, + required: true, + }, }); const { t } = useI18n(); @@ -49,7 +53,9 @@ const unreadMessagesCount = computed(() => { const hasSlaThreshold = computed(() => { return ( - slaCardLabelRef.value?.hasSlaThreshold && props.conversation?.slaPolicyId + !props.contact?.blocked && + slaCardLabelRef.value?.hasSlaThreshold && + props.conversation?.appliedSla?.id ); }); diff --git a/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCard.vue b/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCard.vue index f9a2507a1..be3ddf280 100644 --- a/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCard.vue +++ b/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCard.vue @@ -126,6 +126,7 @@ const onCardClick = e => { v-show="!showMessagePreviewWithoutMeta" ref="cardMessagePreviewWithMetaRef" :conversation="conversation" + :contact="contact" :account-labels="accountLabels" /> diff --git a/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCardExpanded.vue b/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCardExpanded.vue index d0f8f0211..0119b7168 100644 --- a/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCardExpanded.vue +++ b/app/javascript/dashboard/components-next/Conversation/ConversationCard/ConversationCardExpanded.vue @@ -51,7 +51,9 @@ const unreadCount = computed(() => props.chat.unread_count); const slaCardLabel = useTemplateRef('slaCardLabel'); const hasSlaPolicyId = computed( - () => props.chat?.sla_policy_id || slaCardLabel.value?.hasSlaThreshold + () => + !props.currentContact?.blocked && + (props.chat?.applied_sla?.id || slaCardLabel.value?.hasSlaThreshold) ); const selectedModel = computed({ diff --git a/app/javascript/dashboard/components/widgets/conversation/ConversationCard.vue b/app/javascript/dashboard/components/widgets/conversation/ConversationCard.vue index 95ef53491..a5dec806d 100644 --- a/app/javascript/dashboard/components/widgets/conversation/ConversationCard.vue +++ b/app/javascript/dashboard/components/widgets/conversation/ConversationCard.vue @@ -57,7 +57,9 @@ const showMetaSection = computed(() => { ); }); -const hasSlaPolicyId = computed(() => props.chat?.sla_policy_id); +const hasSlaPolicyId = computed( + () => props.chat?.applied_sla?.id && !props.currentContact?.blocked +); const showLabelsSection = computed(() => { return props.chat.labels?.length > 0 || hasSlaPolicyId.value; diff --git a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue index ccd1a8aae..b1e3bc603 100644 --- a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue +++ b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue @@ -93,7 +93,9 @@ const hasMultipleInboxes = computed( () => store.getters['inboxes/getInboxes'].length > 1 ); -const hasSlaPolicyId = computed(() => props.chat?.sla_policy_id); +const hasSlaPolicyId = computed( + () => props.chat?.applied_sla?.id && !currentContact.value?.blocked +); const copyConversationId = async () => { try { diff --git a/app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js b/app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js new file mode 100644 index 000000000..a5f45fc56 --- /dev/null +++ b/app/javascript/dashboard/components/widgets/conversation/specs/ConversationCard.spec.js @@ -0,0 +1,60 @@ +import { shallowMount } from '@vue/test-utils'; +import ConversationCard from '../ConversationCard.vue'; + +const defaultChat = { + id: 1, + labels: [], + messages: [], + priority: null, + unread_count: 0, + timestamp: 1700000000, + created_at: 1700000000, +}; + +const mountComponent = (chat, currentContact = {}) => + shallowMount(ConversationCard, { + props: { + chat: { ...defaultChat, ...chat }, + currentContact: { + name: 'Jane Doe', + thumbnail: '', + availability_status: 'offline', + ...currentContact, + }, + inbox: { id: 1 }, + }, + global: { + stubs: { + 'fluent-icon': true, + }, + }, + }); + +describe('ConversationCard', () => { + it('does not reserve the labels row when only a persisted SLA policy id is present', () => { + const wrapper = mountComponent({ sla_policy_id: 1, applied_sla: null }); + + expect(wrapper.findComponent({ name: 'CardLabels' }).exists()).toBe(false); + }); + + it('shows the labels row when an active applied SLA is present', () => { + const wrapper = mountComponent({ + sla_policy_id: 1, + applied_sla: { id: 1 }, + }); + + expect(wrapper.findComponent({ name: 'CardLabels' }).exists()).toBe(true); + }); + + it('does not reserve the labels row when the contact is blocked', () => { + const wrapper = mountComponent( + { + sla_policy_id: 1, + applied_sla: { id: 1 }, + }, + { blocked: true } + ); + + expect(wrapper.findComponent({ name: 'CardLabels' }).exists()).toBe(false); + }); +}); diff --git a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder index 4cb13f543..5fdd4ecee 100644 --- a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder +++ b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder @@ -58,5 +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 -json.sla_policy_id conversation.sla_policy_id +sla_applicable = !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 e195686a3..1ca3c015e 100644 --- a/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/applied_slas_controller.rb @@ -47,7 +47,7 @@ class Api::V1::Accounts::AppliedSlasController < Api::V1::Accounts::EnterpriseAc end def set_applied_slas - initial_query = Current.account.applied_slas.includes(:conversation) + initial_query = Current.account.applied_slas.with_sla_applicable_conversation.includes(:conversation) @applied_slas = apply_filters(initial_query) 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 d8786565c..4eb2d182b 100644 --- a/enterprise/app/jobs/sla/process_account_applied_slas_job.rb +++ b/enterprise/app/jobs/sla/process_account_applied_slas_job.rb @@ -2,7 +2,7 @@ class Sla::ProcessAccountAppliedSlasJob < ApplicationJob queue_as :medium def perform(account) - account.applied_slas.where(sla_status: %w[active active_with_misses]).each do |applied_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 end diff --git a/enterprise/app/models/applied_sla.rb b/enterprise/app/models/applied_sla.rb index 092daf256..cab812b36 100644 --- a/enterprise/app/models/applied_sla.rb +++ b/enterprise/app/models/applied_sla.rb @@ -40,6 +40,7 @@ class AppliedSla < ApplicationRecord joins(:conversation).where(conversations: { assignee_id: assigned_agent_id }) if assigned_agent_id.present? } scope :missed, -> { where(sla_status: %i[missed active_with_misses]) } + scope :with_sla_applicable_conversation, -> { where(conversation_id: Conversation.with_sla_applicable_contact.select(:id)) } after_update_commit :push_conversation_event diff --git a/enterprise/app/models/enterprise/concerns/conversation.rb b/enterprise/app/models/enterprise/concerns/conversation.rb index 0f7595e0d..a075704d1 100644 --- a/enterprise/app/models/enterprise/concerns/conversation.rb +++ b/enterprise/app/models/enterprise/concerns/conversation.rb @@ -7,10 +7,16 @@ module Enterprise::Concerns::Conversation has_many :sla_events, dependent: :destroy_async has_many :calls, dependent: :destroy_async has_many :captain_responses, class_name: 'Captain::AssistantResponse', dependent: :nullify, as: :documentable + scope :with_sla_applicable_contact, -> { left_joins(:contact).where(contacts: { blocked: [false, nil] }) } + before_validation :validate_sla_policy, if: -> { sla_policy_id_changed? } around_save :ensure_applied_sla_is_created, if: -> { sla_policy_id_changed? } end + def sla_applicable? + !contact&.blocked? + end + private def validate_sla_policy @@ -20,6 +26,11 @@ module Enterprise::Concerns::Conversation return end + unless sla_applicable? + errors.add(:sla_policy, 'cannot be assigned to conversations with blocked contacts') + return + end + if changes[:sla_policy_id].first.present? errors.add(:sla_policy, 'conversation already has a different sla') return diff --git a/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb b/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb index 9ec0a1875..142ce4dc2 100644 --- a/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb +++ b/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb @@ -1,13 +1,13 @@ module Enterprise::Conversations::EventDataPresenter def push_data - if account.feature_enabled?('sla') - super.merge( - applied_sla: applied_sla&.push_event_data, - sla_events: sla_events.map(&:push_event_data), - sla_policy_id: sla_policy_id - ) - else - super - end + return super unless account.feature_enabled?('sla') + + sla_applicable = sla_applicable? + + super.merge( + applied_sla: sla_applicable ? applied_sla&.push_event_data : nil, + sla_events: sla_applicable ? sla_events.map(&:push_event_data) : [], + sla_policy_id: sla_applicable ? sla_policy_id : nil + ) end end diff --git a/enterprise/app/services/enterprise/action_service.rb b/enterprise/app/services/enterprise/action_service.rb index f0c3bbf9f..c841f5054 100644 --- a/enterprise/app/services/enterprise/action_service.rb +++ b/enterprise/app/services/enterprise/action_service.rb @@ -5,6 +5,7 @@ module Enterprise::ActionService sla_policy = @account.sla_policies.find_by(id: sla_policy_id.first) return if sla_policy.nil? return if @conversation.sla_policy.present? + return unless @conversation.sla_applicable? Rails.logger.info "SLA:: Adding SLA #{sla_policy.id} to conversation: #{@conversation.id}" @conversation.update!(sla_policy_id: sla_policy.id) diff --git a/enterprise/app/services/sla/evaluate_applied_sla_service.rb b/enterprise/app/services/sla/evaluate_applied_sla_service.rb index 49d01cf93..2adf3ad9f 100644 --- a/enterprise/app/services/sla/evaluate_applied_sla_service.rb +++ b/enterprise/app/services/sla/evaluate_applied_sla_service.rb @@ -2,6 +2,8 @@ class Sla::EvaluateAppliedSlaService pattr_initialize [:applied_sla!] def perform + return unless conversation.sla_applicable? + check_frt check_nrt check_rt diff --git a/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder b/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder index 5a390a68b..741a4d58b 100644 --- a/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder +++ b/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder @@ -1,10 +1,15 @@ if conversation.account.feature_enabled?('sla') - json.applied_sla do - json.partial! 'api/v1/models/applied_sla', formats: [:json], resource: conversation.applied_sla if conversation.applied_sla.present? - end - json.sla_events do - json.array! conversation.sla_events do |sla_event| - json.partial! 'api/v1/models/sla_event', formats: [:json], sla_event: sla_event + if conversation.sla_applicable? + json.applied_sla do + json.partial! 'api/v1/models/applied_sla', formats: [:json], resource: conversation.applied_sla if conversation.applied_sla.present? end + json.sla_events do + json.array! conversation.sla_events do |sla_event| + json.partial! 'api/v1/models/sla_event', formats: [:json], sla_event: sla_event + end + end + else + json.applied_sla nil + json.sla_events [] end end diff --git a/lib/tasks/apply_sla.rake b/lib/tasks/apply_sla.rake index 70adf8cf3..162a372bb 100644 --- a/lib/tasks/apply_sla.rake +++ b/lib/tasks/apply_sla.rake @@ -62,7 +62,9 @@ namespace :sla do exit(1) end - conversations = account.conversations.where(sla_policy_id: nil).order(id: :desc).limit(batch_size) + conversations = account.conversations.where(sla_policy_id: nil) + conversations = conversations.with_sla_applicable_contact if conversations.respond_to?(:with_sla_applicable_contact) + conversations = conversations.order(id: :desc).limit(batch_size) total_count = conversations.count if total_count.zero? 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 299b8ac7a..4187adfea 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 @@ -37,6 +37,21 @@ RSpec.describe 'Applied SLAs API', type: :request do expect(body).to include('hit_rate' => '0.0%') end + it 'excludes conversations with blocked contacts from metrics' do + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, sla_status: 'missed') + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation2, sla_status: 'missed') + conversation2.contact.update!(blocked: true) + + get "/api/v1/accounts/#{account.id}/applied_slas/metrics", + headers: administrator.create_new_auth_token + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + + expect(body).to include('total_applied_slas' => 1) + expect(body).to include('number_of_sla_misses' => 1) + expect(body).to include('hit_rate' => '0.0%') + end + it 'filters sla metrics based on a date range' do create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, created_at: 10.days.ago) create(:applied_sla, sla_policy: sla_policy1, conversation: conversation2, created_at: 3.days.ago) @@ -131,6 +146,21 @@ RSpec.describe 'Applied SLAs API', type: :request do expect(csv_data.size).to eq(3) expect(csv_data[1][0].to_i).to eq(conversation1.display_id) end + + it 'excludes conversations with blocked contacts from the CSV file' do + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, sla_status: 'missed') + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation2, sla_status: 'missed') + conversation2.contact.update!(blocked: true) + + get "/api/v1/accounts/#{account.id}/applied_slas/download", + headers: administrator.create_new_auth_token + + expect(response).to have_http_status(:success) + csv_data = CSV.parse(response.body) + csv_data.reject! { |row| row.all?(&:nil?) } + expect(csv_data.size).to eq(2) + expect(csv_data[1][0].to_i).to eq(conversation1.display_id) + end end end @@ -156,6 +186,21 @@ RSpec.describe 'Applied SLAs API', type: :request do expect(body['meta']).to include('count' => 1) end + it 'excludes conversations with blocked contacts' do + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, sla_status: 'missed') + create(:applied_sla, sla_policy: sla_policy1, conversation: conversation2, sla_status: 'missed') + conversation2.contact.update!(blocked: true) + + get "/api/v1/accounts/#{account.id}/applied_slas", + headers: administrator.create_new_auth_token + expect(response).to have_http_status(:success) + body = JSON.parse(response.body) + + expect(body['payload'].size).to eq(1) + expect(body['payload'].first['conversation']['id']).to eq(conversation1.display_id) + expect(body['meta']).to include('count' => 1) + end + it 'filters applied slas based on a date range' do create(:applied_sla, sla_policy: sla_policy1, conversation: conversation1, created_at: 10.days.ago, sla_status: 'missed') create(:applied_sla, sla_policy: sla_policy1, conversation: conversation2, created_at: 3.days.ago, sla_status: 'missed') 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 472dc959b..7d053eccf 100644 --- a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -18,6 +18,21 @@ RSpec.describe 'Conversations API', type: :request do expect(response.parsed_body['sla_events'].first['id']).to eq(sla_event.id) end + it 'returns cleared SLA data when the contact is blocked' do + account.enable_features!('sla') + conversation = create(:conversation, account: account) + applied_sla = create(:applied_sla, conversation: conversation) + create(:sla_event, conversation: conversation, applied_sla: applied_sla) + conversation.contact.update!(blocked: true) + + 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['sla_policy_id']).to be_nil + expect(response.parsed_body['applied_sla']).to be_nil + expect(response.parsed_body['sla_events']).to eq([]) + end + it 'does not return SLA data for the conversation if the feature is disabled' do account.disable_features!('sla') conversation = create(:conversation, account: account) 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 fb028b76f..4fe7f46e5 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 @@ -36,6 +36,19 @@ RSpec.describe 'Enterprise Conversations API', type: :request do expect(response).to have_http_status(:unprocessable_entity) expect(JSON.parse(response.body, symbolize_names: true)[:message]).to eq('Sla policy conversation already has a different sla') end + + it 'throws error if conversation contact is blocked' do + conversation.contact.update!(blocked: true) + + 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(:unprocessable_entity) + expect(JSON.parse(response.body, symbolize_names: true)[:message]) + .to eq('Sla policy cannot be assigned to conversations with blocked contacts') + 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 abddfca23..e99a5087b 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 @@ -8,6 +8,11 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do let!(:hit_applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'hit') } let!(:miss_applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'missed') } let!(:active_with_misses_applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'active_with_misses') } + let!(:blocked_contact_applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'active') } + + before do + blocked_contact_applied_sla.conversation.contact.update!(blocked: true) + end it 'enqueues the job' do expect { described_class.perform_later(account) }.to have_enqueued_job(described_class) @@ -18,6 +23,7 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do it 'calls the ProcessAppliedSlaJob for both active and active_with_misses' do expect(Sla::ProcessAppliedSlaJob).to receive(:perform_later).with(active_with_misses_applied_sla).and_call_original expect(Sla::ProcessAppliedSlaJob).to receive(:perform_later).with(applied_sla).and_call_original + expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(blocked_contact_applied_sla) described_class.perform_now(account) end diff --git a/spec/enterprise/models/applied_sla_spec.rb b/spec/enterprise/models/applied_sla_spec.rb index 343294b75..df685444c 100644 --- a/spec/enterprise/models/applied_sla_spec.rb +++ b/spec/enterprise/models/applied_sla_spec.rb @@ -70,6 +70,20 @@ RSpec.describe AppliedSla, type: :model do end end + describe '.with_sla_applicable_conversation' do + it 'excludes blocked contacts and keeps conversations with missing contacts' do + applied_sla = create(:applied_sla) + blocked_applied_sla = create(:applied_sla) + missing_contact_applied_sla = create(:applied_sla) + + blocked_applied_sla.conversation.contact.update!(blocked: true) + missing_contact_applied_sla.conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + + expect(described_class.with_sla_applicable_conversation).to include(applied_sla, missing_contact_applied_sla) + expect(described_class.with_sla_applicable_conversation).not_to include(blocked_applied_sla) + end + end + describe '#frt_due_at' do it 'returns nil when first_response_time_threshold is blank' do applied_sla = create(:applied_sla) diff --git a/spec/enterprise/models/conversation_spec.rb b/spec/enterprise/models/conversation_spec.rb index f7116eb54..7138c468c 100644 --- a/spec/enterprise/models/conversation_spec.rb +++ b/spec/enterprise/models/conversation_spec.rb @@ -59,6 +59,30 @@ RSpec.describe Conversation, type: :model do conversation.save! expect(conversation.applied_sla.sla_policy_id).to eq(sla_policy.id) end + + it 'throws error if contact is blocked' do + conversation.contact.update!(blocked: true) + conversation.sla_policy = sla_policy + + expect(conversation.valid?).to be false + expect(conversation.errors[:sla_policy]).to eq(['cannot be assigned to conversations with blocked contacts']) + end + + it 'allows assigning sla after contact is unblocked' do + conversation.contact.update!(blocked: true) + conversation.contact.update!(blocked: false) + conversation.sla_policy = sla_policy + + conversation.save! + + expect(conversation.applied_sla.sla_policy_id).to eq(sla_policy.id) + end + + it 'keeps existing behavior when contact is missing' do + conversation.update_columns(contact_id: nil, contact_inbox_id: nil) # rubocop:disable Rails/SkipsModelValidations + + expect(conversation.reload.sla_applicable?).to be true + end end context 'when conversation already has a different sla' do diff --git a/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb b/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb index f9897c96d..d87a43363 100644 --- a/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb +++ b/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb @@ -20,6 +20,19 @@ RSpec.describe Conversations::EventDataPresenter do ) end + it 'returns push event payload without active sla data when contact is blocked' do + conversation.account.enable_features!('sla') + conversation.contact.update!(blocked: true) + + expect(presenter.push_data).to include( + { + applied_sla: nil, + sla_events: [], + sla_policy_id: nil + } + ) + end + it 'returns push event payload without applied sla & sla events if the feature is disabled' do conversation.account.disable_features!('sla') diff --git a/spec/enterprise/services/enterprise/action_service_spec.rb b/spec/enterprise/services/enterprise/action_service_spec.rb index a77a039dd..9396dc15d 100644 --- a/spec/enterprise/services/enterprise/action_service_spec.rb +++ b/spec/enterprise/services/enterprise/action_service_spec.rb @@ -20,6 +20,13 @@ describe ActionService do expect(applied_sla.conversation_id).to eq(conversation.id) expect(applied_sla.sla_status).to eq('active') end + + it 'does not add the sla policy when contact is blocked' do + conversation.contact.update!(blocked: true) + + expect { action_service.add_sla([sla_policy.id]) }.not_to change(AppliedSla, :count) + expect(conversation.reload.sla_policy_id).to be_nil + end end context 'when sla_policy_id is not present' do diff --git a/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb b/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb index 40e361e00..6f672e694 100644 --- a/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb +++ b/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb @@ -19,6 +19,29 @@ RSpec.describe Sla::EvaluateAppliedSlaService do end let!(:applied_sla) { conversation.applied_sla } + describe '#perform - blocked contacts' do + before do + applied_sla.sla_policy.update(first_response_time_threshold: 1.hour, resolution_time_threshold: 1.hour) + conversation.contact.update!(blocked: true) + end + + it 'does not create SLA events or update SLA status' do + described_class.new(applied_sla: applied_sla).perform + + expect(SlaEvent.where(applied_sla: applied_sla)).not_to exist + expect(applied_sla.reload.sla_status).to eq('active') + end + + it 'does not mark resolved conversations as hit or missed' do + conversation.resolved! + + described_class.new(applied_sla: applied_sla).perform + + expect(SlaEvent.where(applied_sla: applied_sla)).not_to exist + expect(applied_sla.reload.sla_status).to eq('active') + end + end + describe '#perform - SLA misses' do context 'when first response SLA is missed' do before { applied_sla.sla_policy.update(first_response_time_threshold: 1.hour) }