Hello there,
+ +The automation rule {{meta['rule_name']}} has been disabled becuase it has invalid conditions.
+This typically happens when you delete any custom attributes which are still being used in automation rules.
+ ++Click here to update the conditions. +
diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_first_response.liquid b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_first_response.liquid new file mode 100644 index 000000000..d7988ad5f --- /dev/null +++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_first_response.liquid @@ -0,0 +1,10 @@ +Hi {{user.available_name}},
+ ++ Conversation #{{conversation.display_id}} in {{ inbox.name }} + has missed the SLA for first response under policy {{ sla_policy.name }}. +
+ ++Please address immediately. +
diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_next_response.liquid b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_next_response.liquid new file mode 100644 index 000000000..d7bf8d445 --- /dev/null +++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_next_response.liquid @@ -0,0 +1,10 @@ +Hi {{user.available_name}},
+ ++ Conversation #{{conversation.display_id}} in {{ inbox.name }} + has missed the SLA for next response under policy {{ sla_policy.name }}.. +
+ ++Please address immediately. +
diff --git a/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_resolution.liquid b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_resolution.liquid new file mode 100644 index 000000000..efd24913e --- /dev/null +++ b/app/views/mailers/agent_notifications/conversation_notifications_mailer/sla_missed_resolution.liquid @@ -0,0 +1,10 @@ +Hi {{user.available_name}},
+ ++ Conversation #{{conversation.display_id}} in {{ inbox.name }} + has missed the SLA for resolution time under policy {{ sla_policy.name }}. +
+ + diff --git a/enterprise/app/drops/sla_policy_drop.rb b/enterprise/app/drops/sla_policy_drop.rb new file mode 100644 index 000000000..ea9fbe34d --- /dev/null +++ b/enterprise/app/drops/sla_policy_drop.rb @@ -0,0 +1,9 @@ +class SlaPolicyDrop < BaseDrop + def name + @obj.try(:name) + end + + def description + @obj.try(:description) + end +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 153749267..d8786565c 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: 'active').each do |applied_sla| + account.applied_slas.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/mailers/enterprise/agent_notifications/conversation_notifications_mailer.rb b/enterprise/app/mailers/enterprise/agent_notifications/conversation_notifications_mailer.rb new file mode 100644 index 000000000..df71beb10 --- /dev/null +++ b/enterprise/app/mailers/enterprise/agent_notifications/conversation_notifications_mailer.rb @@ -0,0 +1,32 @@ +module Enterprise::AgentNotifications::ConversationNotificationsMailer + def sla_missed_first_response(conversation, agent, sla_policy) + return unless smtp_config_set_or_development? + + @agent = agent + @conversation = conversation + @sla_policy = sla_policy + subject = "Conversation [ID - #{@conversation.display_id}] missed SLA for first response" + @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) + send_mail_with_liquid(to: @agent.email, subject: subject) and return + end + + def sla_missed_next_response(conversation, agent, sla_policy) + return unless smtp_config_set_or_development? + + @agent = agent + @conversation = conversation + @sla_policy = sla_policy + @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) + send_mail_with_liquid(to: @agent.email, subject: "Conversation [ID - #{@conversation.display_id}] missed SLA for next response") and return + end + + def sla_missed_resolution(conversation, agent, sla_policy) + return unless smtp_config_set_or_development? + + @agent = agent + @conversation = conversation + @sla_policy = sla_policy + @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) + send_mail_with_liquid(to: @agent.email, subject: "Conversation [ID - #{@conversation.display_id}] missed SLA for resolution time") and return + end +end diff --git a/enterprise/app/models/applied_sla.rb b/enterprise/app/models/applied_sla.rb index 4558d5d23..111b78e84 100644 --- a/enterprise/app/models/applied_sla.rb +++ b/enterprise/app/models/applied_sla.rb @@ -27,7 +27,7 @@ class AppliedSla < ApplicationRecord validates :account_id, uniqueness: { scope: %i[sla_policy_id conversation_id] } before_validation :ensure_account_id - enum sla_status: { active: 0, hit: 1, missed: 2 } + enum sla_status: { active: 0, hit: 1, missed: 2, active_with_misses: 3 } scope :filter_by_date_range, ->(range) { where(created_at: range) if range.present? } scope :filter_by_inbox_id, ->(inbox_id) { where(inbox_id: inbox_id) if inbox_id.present? } diff --git a/enterprise/app/models/enterprise/application_record.rb b/enterprise/app/models/enterprise/application_record.rb new file mode 100644 index 000000000..a05f60767 --- /dev/null +++ b/enterprise/app/models/enterprise/application_record.rb @@ -0,0 +1,5 @@ +module Enterprise::ApplicationRecord + def droppables + super + %w[SlaPolicy] + end +end diff --git a/enterprise/app/models/sla_event.rb b/enterprise/app/models/sla_event.rb index 28f709346..59068684e 100644 --- a/enterprise/app/models/sla_event.rb +++ b/enterprise/app/models/sla_event.rb @@ -32,6 +32,8 @@ class SlaEvent < ApplicationRecord before_validation :ensure_applied_sla_id, :ensure_account_id, :ensure_inbox_id, :ensure_sla_policy_id + after_create_commit :create_notifications + private def ensure_applied_sla_id @@ -49,4 +51,28 @@ class SlaEvent < ApplicationRecord def ensure_sla_policy_id self.sla_policy_id ||= applied_sla&.sla_policy_id end + + def create_notifications + notify_users = conversation.conversation_participants.map(&:user) + # Add all admins from the account to notify list + notify_users += account.administrators + # Ensure conversation assignee is notified + notify_users += [conversation.assignee] if conversation.assignee.present? + + notification_type = { + 'frt' => 'sla_missed_first_response', + 'nrt' => 'sla_missed_next_response', + 'rt' => 'sla_missed_resolution' + }[event_type] + + notify_users.uniq.each do |user| + NotificationBuilder.new( + notification_type: notification_type, + user: user, + account: account, + primary_actor: conversation, + secondary_actor: sla_policy + ).perform + end + end end diff --git a/enterprise/app/services/sla/evaluate_applied_sla_service.rb b/enterprise/app/services/sla/evaluate_applied_sla_service.rb index d6eb9839e..4cc953899 100644 --- a/enterprise/app/services/sla/evaluate_applied_sla_service.rb +++ b/enterprise/app/services/sla/evaluate_applied_sla_service.rb @@ -7,8 +7,8 @@ class Sla::EvaluateAppliedSlaService # We will calculate again in the next iteration return unless applied_sla.conversation.resolved? - # No SLA missed, so marking as hit as conversation is resolved - handle_hit_sla(applied_sla) if applied_sla.active? + # after conversation is resolved, we will check if the SLA was hit or missed + handle_hit_sla(applied_sla) end private @@ -49,6 +49,14 @@ class Sla::EvaluateAppliedSlaService handle_missed_sla(applied_sla, 'nrt') end + def get_last_message_id(conversation) + conversation.messages.where(message_type: :incoming).last&.id + end + + def already_missed?(applied_sla, type, meta = {}) + SlaEvent.exists?(applied_sla: applied_sla, event_type: type, meta: meta) + end + def check_resolution_time_threshold(applied_sla, conversation, sla_policy) return if conversation.resolved? @@ -58,48 +66,41 @@ class Sla::EvaluateAppliedSlaService handle_missed_sla(applied_sla, 'rt') end - def handle_missed_sla(applied_sla, type) - return unless applied_sla.active? + def handle_missed_sla(applied_sla, type, meta = {}) + meta = { message_id: get_last_message_id(applied_sla.conversation) } if type == 'nrt' + return if already_missed?(applied_sla, type, meta) - applied_sla.update!(sla_status: 'missed') - generate_notifications_for_sla(applied_sla, type) - Rails.logger.warn "SLA missed for conversation #{applied_sla.conversation.id} " \ + create_sla_event(applied_sla, type, meta) + Rails.logger.warn "SLA #{type} missed for conversation #{applied_sla.conversation.id} " \ "in account #{applied_sla.account_id} " \ "for sla_policy #{applied_sla.sla_policy.id}" + + applied_sla.update!(sla_status: 'active_with_misses') if applied_sla.sla_status != 'active_with_misses' end def handle_hit_sla(applied_sla) - return unless applied_sla.active? - - applied_sla.update!(sla_status: 'hit') - Rails.logger.info "SLA hit for conversation #{applied_sla.conversation.id} " \ - "in account #{applied_sla.account_id} " \ - "for sla_policy #{applied_sla.sla_policy.id}" - end - - def generate_notifications_for_sla(applied_sla, type) - notify_users = applied_sla.conversation.conversation_participants.map(&:user) - # add all admins from the account to notify list - notify_users += applied_sla.account.administrators - # ensure conversation assignee is notified - notify_users += [applied_sla.conversation.assignee] if applied_sla.conversation.assignee.present? - - notification_type = if type == 'frt' - 'sla_missed_first_response' - elsif type == 'nrt' - 'sla_missed_next_response' - else - 'sla_missed_resolution' - end - - notify_users.uniq.each do |user| - NotificationBuilder.new( - notification_type: notification_type, - user: user, - account: applied_sla.account, - primary_actor: applied_sla.conversation, - secondary_actor: applied_sla.sla_policy - ).perform + if applied_sla.active? + applied_sla.update!(sla_status: 'hit') + Rails.logger.info "SLA hit for conversation #{applied_sla.conversation.id} " \ + "in account #{applied_sla.account_id} " \ + "for sla_policy #{applied_sla.sla_policy.id}" + else + applied_sla.update!(sla_status: 'missed') + Rails.logger.info "SLA missed for conversation #{applied_sla.conversation.id} " \ + "in account #{applied_sla.account_id} " \ + "for sla_policy #{applied_sla.sla_policy.id}" end end + + def create_sla_event(applied_sla, event_type, meta = {}) + SlaEvent.create!( + applied_sla: applied_sla, + conversation: applied_sla.conversation, + event_type: event_type, + meta: meta, + account: applied_sla.account, + inbox: applied_sla.conversation.inbox, + sla_policy: applied_sla.sla_policy + ) + end end diff --git a/spec/enterprise/drops/sla_policy_drop_spec.rb b/spec/enterprise/drops/sla_policy_drop_spec.rb new file mode 100644 index 000000000..c1be13c70 --- /dev/null +++ b/spec/enterprise/drops/sla_policy_drop_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe SlaPolicyDrop do + subject(:sla_policy_drop) { described_class.new(sla_policy) } + + let!(:sla_policy) { create(:sla_policy) } + + it 'returns name' do + expect(sla_policy_drop.name).to eq sla_policy.name + end + + it 'returns description' do + expect(sla_policy_drop.description).to eq sla_policy.description + 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 beae967db..5d628f71e 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 @@ -7,18 +7,20 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do let!(:applied_sla) { create(:applied_sla, account: account, sla_policy: sla_policy, sla_status: 'active') } 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') } it 'enqueues the job' do expect { described_class.perform_later }.to have_enqueued_job(described_class) .on_queue('medium') end - it 'calls the ProcessAppliedSlaJob' 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 described_class.perform_now(account) end - it 'does not call the ProcessAppliedSlaJob for not active applied slas' do + it 'does not call the ProcessAppliedSlaJob for applied slas that are hit or miss' do expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(hit_applied_sla) expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(miss_applied_sla) described_class.perform_now(account) diff --git a/spec/enterprise/mailers/enterprise/agent_notifications/conversation_notifications_mailer_spec.rb b/spec/enterprise/mailers/enterprise/agent_notifications/conversation_notifications_mailer_spec.rb new file mode 100644 index 000000000..e5e2b14da --- /dev/null +++ b/spec/enterprise/mailers/enterprise/agent_notifications/conversation_notifications_mailer_spec.rb @@ -0,0 +1,54 @@ +require 'rails_helper' + +# rails helper is using infer filetype to detect rspec type +# so we need to include type: :mailer to make this test work in enterprise namespace +RSpec.describe AgentNotifications::ConversationNotificationsMailer, type: :mailer do + let(:class_instance) { described_class.new } + let!(:account) { create(:account) } + let(:agent) { create(:user, email: 'agent1@example.com', account: account) } + let(:conversation) { create(:conversation, assignee: agent, account: account) } + + before do + allow(described_class).to receive(:new).and_return(class_instance) + allow(class_instance).to receive(:smtp_config_set_or_development?).and_return(true) + end + + describe 'sla_missed_first_response' do + let(:sla_policy) { create(:sla_policy, account: account) } + let(:mail) { described_class.with(account: account).sla_missed_first_response(conversation, agent, sla_policy).deliver_now } + + it 'renders the subject' do + expect(mail.subject).to eq("Conversation [ID - #{conversation.display_id}] missed SLA for first response") + end + + it 'renders the receiver email' do + expect(mail.to).to eq([agent.email]) + end + end + + describe 'sla_missed_next_response' do + let(:sla_policy) { create(:sla_policy, account: account) } + let(:mail) { described_class.with(account: account).sla_missed_next_response(conversation, agent, sla_policy).deliver_now } + + it 'renders the subject' do + expect(mail.subject).to eq("Conversation [ID - #{conversation.display_id}] missed SLA for next response") + end + + it 'renders the receiver email' do + expect(mail.to).to eq([agent.email]) + end + end + + describe 'sla_missed_resolution' do + let(:sla_policy) { create(:sla_policy, account: account) } + let(:mail) { described_class.with(account: account).sla_missed_resolution(conversation, agent, sla_policy).deliver_now } + + it 'renders the subject' do + expect(mail.subject).to eq("Conversation [ID - #{conversation.display_id}] missed SLA for resolution time") + end + + it 'renders the receiver email' do + expect(mail.to).to eq([agent.email]) + end + end +end diff --git a/spec/enterprise/models/sla_event_spec.rb b/spec/enterprise/models/sla_event_spec.rb index 862b22e6c..3c44d3961 100644 --- a/spec/enterprise/models/sla_event_spec.rb +++ b/spec/enterprise/models/sla_event_spec.rb @@ -25,4 +25,37 @@ RSpec.describe SlaEvent, type: :model do expect(sla_event.sla_policy_id).to eq sla_event.applied_sla.sla_policy_id end end + + describe 'create notifications' do + # create account, user and inbox + let!(:account) { create(:account) } + let!(:assignee) { create(:user, account: account) } + let!(:participant) { create(:user, account: account) } + let!(:admin) { create(:user, account: account, role: :administrator) } + let!(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, inbox: inbox, assignee: assignee, account: account) } + let(:sla_policy) { create(:sla_policy, account: conversation.account) } + let(:sla_event) { create(:sla_event, event_type: 'frt', conversation: conversation, sla_policy: sla_policy) } + + before do + # to ensure notifications are not sent to other users + create(:user, account: account) + create(:inbox_member, inbox: inbox, user: participant) + create(:conversation_participant, conversation: conversation, user: participant) + end + + it 'creates notifications for conversation participants, admins, and assignee' do + sla_event + + expect(Notification.count).to eq(3) + # check if notification type is sla_missed_first_response + expect(Notification.where(notification_type: 'sla_missed_first_response').count).to eq(3) + # Check if notification is created for the assignee + expect(Notification.where(user_id: assignee.id).count).to eq(1) + # Check if notification is created for the account admin + expect(Notification.where(user_id: admin.id).count).to eq(1) + # Check if notification is created for participant + expect(Notification.where(user_id: participant.id).count).to eq(1) + end + end end 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 12cb59d35..f6cd657be 100644 --- a/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb +++ b/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb @@ -3,8 +3,6 @@ require 'rails_helper' RSpec.describe Sla::EvaluateAppliedSlaService do let!(:account) { create(:account) } let!(:user_1) { create(:user, account: account) } - let!(:user_2) { create(:user, account: account) } - let!(:admin) { create(:user, account: account, role: :administrator) } let!(:sla_policy) do create(:sla_policy, @@ -28,19 +26,17 @@ RSpec.describe Sla::EvaluateAppliedSlaService do it 'updates the SLA status to missed and logs a warning' do allow(Rails.logger).to receive(:warn) described_class.new(applied_sla: applied_sla).perform - expect(Rails.logger).to have_received(:warn).with("SLA missed for conversation #{conversation.id} in account " \ + expect(Rails.logger).to have_received(:warn).with("SLA frt missed for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") - expect(applied_sla.reload.sla_status).to eq('missed') + expect(applied_sla.reload.sla_status).to eq('active_with_misses') + end - expect(Notification.count).to eq(2) - # check if notification type is sla_missed_first_response - expect(Notification.where(notification_type: 'sla_missed_first_response').count).to eq(2) - # Check if notification is created for the assignee - expect(Notification.where(user_id: user_1.id).count).to eq(1) - # Check if notification is created for the account admin - expect(Notification.where(user_id: admin.id).count).to eq(1) - # Check if no notification is created for other user - expect(Notification.where(user_id: user_2.id).count).to eq(0) + it 'creates SlaEvent only for frt miss' do + described_class.new(applied_sla: applied_sla).perform + + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'frt').count).to eq(1) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'nrt').count).to eq(0) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'rt').count).to eq(0) end end @@ -53,19 +49,17 @@ RSpec.describe Sla::EvaluateAppliedSlaService do it 'updates the SLA status to missed and logs a warning' do allow(Rails.logger).to receive(:warn) described_class.new(applied_sla: applied_sla).perform - expect(Rails.logger).to have_received(:warn).with("SLA missed for conversation #{conversation.id} in account " \ + expect(Rails.logger).to have_received(:warn).with("SLA nrt missed for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") - expect(applied_sla.reload.sla_status).to eq('missed') + expect(applied_sla.reload.sla_status).to eq('active_with_misses') + end - expect(Notification.count).to eq(2) - # check if notification type is sla_missed_first_response - expect(Notification.where(notification_type: 'sla_missed_next_response').count).to eq(2) - # Check if notification is created for the assignee - expect(Notification.where(user_id: user_1.id).count).to eq(1) - # Check if notification is created for the account admin - expect(Notification.where(user_id: admin.id).count).to eq(1) - # Check if no notification is created for other user - expect(Notification.where(user_id: user_2.id).count).to eq(0) + it 'creates SlaEvent only for nrt miss' do + described_class.new(applied_sla: applied_sla).perform + + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'frt').count).to eq(0) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'nrt').count).to eq(1) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'rt').count).to eq(0) end end @@ -75,18 +69,18 @@ RSpec.describe Sla::EvaluateAppliedSlaService do it 'updates the SLA status to missed and logs a warning' do allow(Rails.logger).to receive(:warn) described_class.new(applied_sla: applied_sla).perform - expect(Rails.logger).to have_received(:warn).with("SLA missed for conversation #{conversation.id} in account " \ + expect(Rails.logger).to have_received(:warn).with("SLA rt missed for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") - expect(applied_sla.reload.sla_status).to eq('missed') - expect(Notification.count).to eq(2) - expect(Notification.where(notification_type: 'sla_missed_resolution').count).to eq(2) - # Check if notification is created for the assignee - expect(Notification.where(user_id: user_1.id).count).to eq(1) - # Check if notification is created for the account admin - expect(Notification.where(user_id: admin.id).count).to eq(1) - # Check if no notification is created for other user - expect(Notification.where(user_id: user_2.id).count).to eq(0) + expect(applied_sla.reload.sla_status).to eq('active_with_misses') + end + + it 'creates SlaEvent only for rt miss' do + described_class.new(applied_sla: applied_sla).perform + + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'frt').count).to eq(0) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'nrt').count).to eq(0) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'rt').count).to eq(1) end end @@ -110,13 +104,14 @@ RSpec.describe Sla::EvaluateAppliedSlaService do conversation.update(first_reply_created_at: 5.hours.ago, waiting_since: 5.hours.ago) end - it 'updates the SLA status to missed and logs a warning' do + it 'updates the SLA status to missed and logs multiple warnings' do allow(Rails.logger).to receive(:warn) described_class.new(applied_sla: applied_sla).perform - expect(Rails.logger).to have_received(:warn).with("SLA missed for conversation #{conversation.id} in account " \ + expect(Rails.logger).to have_received(:warn).with("SLA rt missed for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}").exactly(1).time - expect(applied_sla.reload.sla_status).to eq('missed') - expect(Notification.count).to eq(2) + expect(Rails.logger).to have_received(:warn).with("SLA nrt missed for conversation #{conversation.id} in account " \ + "#{applied_sla.account_id} for sla_policy #{sla_policy.id}").exactly(1).time + expect(applied_sla.reload.sla_status).to eq('active_with_misses') end end end @@ -140,6 +135,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:info).with("SLA hit for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") expect(applied_sla.reload.sla_status).to eq('hit') + expect(SlaEvent.count).to eq(0) expect(Notification.count).to eq(0) end end @@ -162,6 +158,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:info).with("SLA hit for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") expect(applied_sla.reload.sla_status).to eq('hit') + expect(SlaEvent.count).to eq(0) end end @@ -177,7 +174,52 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:info).with("SLA hit for conversation #{conversation.id} in account " \ "#{applied_sla.account_id} for sla_policy #{sla_policy.id}") expect(applied_sla.reload.sla_status).to eq('hit') + expect(SlaEvent.count).to eq(0) end end end + + describe 'SLA evaluation with frt hit, multiple nrt misses and rt miss' do + before do + # Setup SLA Policy thresholds + sla_policy.update( + first_response_time_threshold: 2.hours, # Hit frt + next_response_time_threshold: 1.hour, # Miss nrt multiple times + resolution_time_threshold: 4.hours # Miss rt + ) + + # Simulate conversation timeline + # Hit frt + # incoming message from customer + create(:message, conversation: conversation, created_at: 6.hours.ago, message_type: :incoming) + # outgoing message from agent within frt + create(:message, conversation: conversation, created_at: 5.hours.ago, message_type: :outgoing) + + # Miss nrt first time + create(:message, conversation: conversation, created_at: 4.hours.ago, message_type: :incoming) + described_class.new(applied_sla: applied_sla).perform + + # Miss nrt second time + create(:message, conversation: conversation, created_at: 3.hours.ago, message_type: :incoming) + described_class.new(applied_sla: applied_sla).perform + + # Conversation is resolved missing rt + conversation.update(status: 'resolved') + + # this will not create a new notification for rt miss as conversation is resolved + # but we would have already created an rt miss notification during previous evaluation + described_class.new(applied_sla: applied_sla).perform + end + + it 'updates the SLA status to missed' do + # the status would be missed as the conversation is resolved + expect(applied_sla.reload.sla_status).to eq('missed') + end + + it 'creates necessary sla events' do + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'frt').count).to eq(0) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'nrt').count).to eq(2) + expect(SlaEvent.where(applied_sla: applied_sla, event_type: 'rt').count).to eq(1) + end + end end diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index a20d3d71d..53ebfa0c7 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -1,6 +1,11 @@ require 'rails_helper' +require Rails.root.join 'spec/models/concerns/reauthorizable_shared.rb' RSpec.describe AutomationRule do + describe 'concerns' do + it_behaves_like 'reauthorizable' + end + describe 'associations' do let(:account) { create(:account) } let(:params) do @@ -56,4 +61,35 @@ RSpec.describe AutomationRule do expect(rule.errors.messages[:conditions]).to eq(['Automation conditions should have query operator.']) end end + + describe 'reauthorizable' do + context 'when prompt_reauthorization!' do + it 'marks the rule inactive' do + rule = create(:automation_rule) + expect(rule.active).to be true + rule.prompt_reauthorization! + expect(rule.active).to be false + end + end + + context 'when reauthorization_required?' do + it 'unsets the error count if conditions are updated' do + rule = create(:automation_rule) + rule.prompt_reauthorization! + expect(rule.reauthorization_required?).to be true + + rule.update!(conditions: [{ attribute_key: 'browser_language', filter_operator: 'equal_to', values: ['en'], query_operator: 'AND' }]) + expect(rule.reauthorization_required?).to be false + end + + it 'will not unset the error count if conditions are not updated' do + rule = create(:automation_rule) + rule.prompt_reauthorization! + expect(rule.reauthorization_required?).to be true + + rule.update!(name: 'Updated name') + expect(rule.reauthorization_required?).to be true + end + end + end end diff --git a/spec/models/concerns/reauthorizable_shared.rb b/spec/models/concerns/reauthorizable_shared.rb index 0bfa112c4..9efe232e8 100644 --- a/spec/models/concerns/reauthorizable_shared.rb +++ b/spec/models/concerns/reauthorizable_shared.rb @@ -25,10 +25,19 @@ shared_examples_for 'reauthorizable' do it 'prompt_reauthorization!' do obj = FactoryBot.create(model.to_s.underscore.tr('/', '_').to_sym) + mailer = double + mailer_method = double + allow(AdministratorNotifications::ChannelNotificationsMailer).to receive(:with).and_return(mailer) + # allow mailer to receive any methods and return mailer + allow(mailer).to receive(:method_missing).and_return(mailer_method) + allow(mailer_method).to receive(:deliver_later) + expect(obj.reauthorization_required?).to be false obj.prompt_reauthorization! expect(obj.reauthorization_required?).to be true + expect(AdministratorNotifications::ChannelNotificationsMailer).to have_received(:with).with(account: obj.account) + expect(mailer_method).to have_received(:deliver_later) end it 'reauthorized!' do