fix: add digest methods

app/models/notification.rb

@@ -162,11 +162,21 @@ class Notification < ApplicationRecord
     # Should we do something about the case where user subscribed to both push and email ?
     # In future, we could probably add condition here to enqueue the job for 30 seconds later
     # when push enabled and then check in email job whether notification has been read already.
-    Notification::EmailNotificationJob.perform_later(self) if user_subscribed_to_notification?('email')
+    Notification::EmailNotificationJob.perform_later(self) if should_send_email? && user_subscribed_to_notification?('email')
 
     Notification::RemoveDuplicateNotificationJob.perform_later(self)
   end
 
+  def should_send_email?
+    case notification_type
+    when 'sla_missed_first_response', 'sla_missed_next_response', 'sla_missed_resolution'
+      # we send an email digest for this anyway so no need to send individual emails
+      false
+    else
+      true
+    end
+  end
+
   def user_subscribed_to_notification?(delivery_type)
     notification_setting = user.notification_settings.find_by(account_id: account.id)
     return false if notification_setting.blank?

app/views/mailers/agent_notifications/conversation_notifications_mailer/missed_sla_digest.liquid

@@ -0,0 +1,51 @@
+<p>Hello {{ user.available_name }},</p>
+<p>This is a digest of the missed SLAs in your conversations:</p>
+
+{% for conversation, missed_slas in missed_slas_by_conversation %}
+  <div style='margin-bottom: 20px;'>
+    <h3>Conversation: {{ conversation.display_id }}</h3>
+    <table style='width: 100%; border-collapse: collapse;'>
+      <thead>
+        <tr>
+          <th style='text-align: left; border-bottom: 1px solid #ccc; padding: 5px;'>
+            SLA Policy
+          </th>
+          <th style='text-align: left; border-bottom: 1px solid #ccc; padding: 5px;'>
+            Missed Event
+          </th>
+          <th style='text-align: left; border-bottom: 1px solid #ccc; padding: 5px;'>
+            Missed At
+          </th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for missed_sla in missed_slas %}
+          {% for sla_event in missed_sla.sla_events %}
+            <tr>
+              <td style='padding: 5px;'>
+                {{ missed_sla.applied_sla.sla_policy.name }}
+              </td>
+              <td style='padding: 5px;'>{{ sla_event.event }}</td>
+              <td style='padding: 5px;'>
+                {{ sla_event.created_at | date: '%b %d, %Y %I:%M %p' }}
+              </td>
+            </tr>
+          {% endfor %}
+        {% endfor %}
+      </tbody>
+    </table>
+    <p>
+      <a href='{{ conversation_url }}' target='_blank'>View Conversation</a>
+    </p>
+  </div>
+{% endfor %}
+
+<p>
+  Please take the necessary actions to address these missed SLAs and ensure
+  timely responses to the conversations.
+</p>
+
+<p>
+  Best regards,<br>
+  Your SLA Monitoring System
+</p>

enterprise/app/jobs/sla/process_account_applied_slas_job.rb

@@ -2,8 +2,39 @@ 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|
+    missed_slas = []
-      Sla::ProcessAppliedSlaJob.perform_later(applied_sla)
+
+    account.applied_slas.where(sla_status: %w[active active_with_misses]).includes(:conversation, :sla_policy).each do |applied_sla|
+      sla_events, updated_applied_sla = Sla::EvaluateAppliedSlaService.new(applied_sla: applied_sla).perform
+      if updated_applied_sla.status == :missed || updated_applied_sla.status == :active_with_misses
+        missed_slas << { sla_events: sla_events,
+                         applied_sla: updated_applied_sla }
+      end
+    end
+
+    user_sla_map = build_user_sla_map(missed_slas)
+    notify_users(user_sla_map)
+  end
+
+  def build_user_sla_map(missed_slas)
+    missed_slas.each_with_object({}) do |missed_sla, user_missed_slas_map|
+      applied_sla = missed_sla[:applied_sla]
+      sla_events = missed_sla[:sla_events]
+      conversation = applied_sla.conversation
+
+      users_to_notify = conversation.conversation_participants.map(&:user)
+      users_to_notify << conversation.assignee if conversation.assignee.present?
+
+      users_to_notify.uniq.each do |user|
+        user_missed_slas_map[user] ||= []
+        user_missed_slas_map[user] += [{ sla_events: sla_events, applied_sla: applied_sla }]
+      end
+    end
+  end
+
+  def notify_users(user_sla_map)
+    user_sla_map.each do |user, sla_data|
+      puts "Notifying user #{user.id} and #{admins.count} admins about #{sla_data.count} missed SLAs"
     end
   end
 end

enterprise/app/jobs/sla/process_applied_sla_job.rb

@@ -1,7 +0,0 @@
-class Sla::ProcessAppliedSlaJob < ApplicationJob
-  queue_as :medium
-
-  def perform(applied_sla)
-    Sla::EvaluateAppliedSlaService.new(applied_sla: applied_sla).perform
-  end
-end

enterprise/app/mailers/enterprise/agent_notifications/conversation_notifications_mailer.rb

@@ -1,38 +1,30 @@
 module Enterprise::AgentNotifications::ConversationNotificationsMailer
-  def sla_missed_first_response(conversation, agent, sla_policy)
+  def missed_slas_digest(user, missed_slas)
     return unless smtp_config_set_or_development?
 
-    @agent = agent
+    @user = user
-    @conversation = conversation
+    @missed_slas = missed_slas
-    @sla_policy = sla_policy
+    subject = 'Missed SLAs Digest'
-    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)
+    # Group missed SLAs by conversation using each_with_object
-    return unless smtp_config_set_or_development?
+    @missed_slas_by_conversation = @missed_slas.each_with_object({}) do |missed_sla, result|
+      applied_sla = missed_sla[:applied_sla]
+      sla_events = missed_sla[:sla_events]
+      conversation = applied_sla.conversation
 
-    @agent = agent
+      result[conversation] ||= []
-    @conversation = conversation
+      result[conversation] << {
-    @sla_policy = sla_policy
+        sla_events: sla_events,
-    @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id)
+        applied_sla: applied_sla
-    send_mail_with_liquid(to: @agent.email, subject: "Conversation [ID - #{@conversation.display_id}] missed SLA for next response") and return
+      }
-  end
+    end
 
-  def sla_missed_resolution(conversation, agent, sla_policy)
+    send_mail_with_liquid(to: @user.email, subject: subject) and return
-    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
 
   def liquid_droppables
     super.merge({
-                  sla_policy: @sla_policy
+                  missed_slas_by_conversation: @missed_slas_by_conversation
                 })
   end
 end

enterprise/app/services/sla/evaluate_applied_sla_service.rb

@@ -1,23 +1,32 @@
 class Sla::EvaluateAppliedSlaService
   pattr_initialize [:applied_sla!]
 
+  FRT_THRESHOLD = 'frt'.freeze
+  NRT_THRESHOLD = 'nrt'.freeze
+  RT_THRESHOLD = 'rt'.freeze
+
   def perform
-    check_sla_thresholds
+    sla_events = []
+
+    check_sla_thresholds(sla_events)
 
     # We will calculate again in the next iteration
-    return unless applied_sla.conversation.resolved?
+    return [sla_events, applied_sla] unless applied_sla.conversation.resolved?
 
     # after conversation is resolved, we will check if the SLA was hit or missed
     handle_hit_sla(applied_sla)
+
+    [sla_events, applied_sla]
   end
 
   private
 
-  def check_sla_thresholds
+  def check_sla_thresholds(sla_events)
     [:first_response_time_threshold, :next_response_time_threshold, :resolution_time_threshold].each do |threshold|
       next if applied_sla.sla_policy.send(threshold).blank?
 
-      send("check_#{threshold}", applied_sla, applied_sla.conversation, applied_sla.sla_policy)
+      event = send("check_#{threshold}", applied_sla, applied_sla.conversation, applied_sla.sla_policy)
+      sla_events << event if event.present?
     end
   end
 
@@ -30,7 +39,7 @@ class Sla::EvaluateAppliedSlaService
     return if first_reply_was_within_threshold?(conversation, threshold)
     return if still_within_threshold?(threshold)
 
-    handle_missed_sla(applied_sla, 'frt')
+    handle_missed_sla(applied_sla, FRT_THRESHOLD)
   end
 
   def first_reply_was_within_threshold?(conversation, threshold)
@@ -46,7 +55,7 @@ class Sla::EvaluateAppliedSlaService
     threshold = conversation.waiting_since.to_i + sla_policy.next_response_time_threshold.to_i
     return if still_within_threshold?(threshold)
 
-    handle_missed_sla(applied_sla, 'nrt')
+    handle_missed_sla(applied_sla, NRT_THRESHOLD)
   end
 
   def get_last_message_id(conversation)
@@ -64,11 +73,11 @@ class Sla::EvaluateAppliedSlaService
     threshold = conversation.created_at.to_i + sla_policy.resolution_time_threshold.to_i
     return if still_within_threshold?(threshold)
 
-    handle_missed_sla(applied_sla, 'rt')
+    handle_missed_sla(applied_sla, RT_THRESHOLD)
   end
 
   def handle_missed_sla(applied_sla, type, meta = {})
-    meta = { message_id: get_last_message_id(applied_sla.conversation) } if type == 'nrt'
+    meta = { message_id: get_last_message_id(applied_sla.conversation) } if type == NRT_THRESHOLD
     return if already_missed?(applied_sla, type, meta)
 
     create_sla_event(applied_sla, type, meta)

spec/enterprise/jobs/sla/process_account_applied_slas_job_spec.rb

@@ -14,15 +14,15 @@ RSpec.describe Sla::ProcessAccountAppliedSlasJob do
         .on_queue('medium')
     end
 
-    it 'calls the ProcessAppliedSlaJob for both active and active_with_misses' do
+    it 'calls the EvaluateAppliedSlaService 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::EvaluateAppliedSlaService).to receive(:perform).with(active_with_misses_applied_sla).and_call_original
-      expect(Sla::ProcessAppliedSlaJob).to receive(:perform_later).with(applied_sla).and_call_original
+      expect(Sla::EvaluateAppliedSlaService).to receive(:perform).with(applied_sla).and_call_original
       described_class.perform_now(account)
     end
 
-    it 'does not call the ProcessAppliedSlaJob for applied slas that are hit or miss' do
+    it 'does not call the EvaluateAppliedSlaService for applied slas that are hit or miss' do
-      expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(hit_applied_sla)
+      expect(Sla::EvaluateAppliedSlaService).not_to receive(:perform).with(hit_applied_sla)
-      expect(Sla::ProcessAppliedSlaJob).not_to receive(:perform_later).with(miss_applied_sla)
+      expect(Sla::EvaluateAppliedSlaService).not_to receive(:perform).with(miss_applied_sla)
       described_class.perform_now(account)
     end
   end

spec/enterprise/jobs/sla/process_applied_sla_job_spec.rb

@@ -1,18 +0,0 @@
-require 'rails_helper'
-
-RSpec.describe Sla::ProcessAppliedSlaJob do
-  context 'when perform is called' do
-    let(:account) { create(:account) }
-
-    it 'enqueues the job' do
-      expect { described_class.perform_later }.to have_enqueued_job(described_class)
-        .on_queue('medium')
-    end
-
-    it 'calls the EvaluateAppliedSlaService' do
-      applied_sla = create(:applied_sla)
-      expect(Sla::EvaluateAppliedSlaService).to receive(:new).with(applied_sla: applied_sla).and_call_original
-      described_class.perform_now(applied_sla)
-    end
-  end
-end