fix: SLA handling for blocked contacts (#14861)

# 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 <muhsinkeramam@gmail.com>
This commit is contained in:
Sony Mathew
2026-07-02 14:16:29 +05:30
committed by GitHub
co-authored by Sivin Varghese Muhsin Keloth
parent a3c7f3b204
commit 7bf76057c2
25 changed files with 279 additions and 23 deletions
@@ -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
);
});
@@ -126,6 +126,7 @@ const onCardClick = e => {
v-show="!showMessagePreviewWithoutMeta"
ref="cardMessagePreviewWithMetaRef"
:conversation="conversation"
:contact="contact"
:account-labels="accountLabels"
/>
</div>
@@ -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({
@@ -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;
@@ -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 {
@@ -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);
});
});
@@ -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?
@@ -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
@@ -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
+1
View File
@@ -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
@@ -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
@@ -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
@@ -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)
@@ -2,6 +2,8 @@ class Sla::EvaluateAppliedSlaService
pattr_initialize [:applied_sla!]
def perform
return unless conversation.sla_applicable?
check_frt
check_nrt
check_rt
@@ -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
+3 -1
View File
@@ -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?
@@ -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')
@@ -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)
@@ -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
@@ -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
@@ -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)
@@ -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
@@ -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')
@@ -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
@@ -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) }