From aaf70cf1cfc3080a21dd79aad0635578aae92f81 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Mon, 11 Mar 2024 21:49:41 +0530 Subject: [PATCH] feat: add push notification when SLA missed (#9078) * feat: add push notification when SLA missed * chore: sent notification only for inbox members * feat: add conv particpants+admins to SLA notification list * chore: add spec to ensure notification is created * chore: refactor to multiple alerts for SLA conditions * chore: add sla_policy as secondary_actor in notification --- .../i18n/locale/en/generalSettings.json | 5 ++- .../dashboard/i18n/locale/en/inbox.json | 5 ++- app/models/notification.rb | 38 +++++++++++++------ config/locales/en.yml | 3 ++ enterprise/app/models/sla_policy.rb | 10 +++++ .../sla/evaluate_applied_sla_service.rb | 35 +++++++++++++++-- .../sla/evaluate_applied_sla_service_spec.rb | 37 +++++++++++++++++- 7 files changed, 114 insertions(+), 19 deletions(-) diff --git a/app/javascript/dashboard/i18n/locale/en/generalSettings.json b/app/javascript/dashboard/i18n/locale/en/generalSettings.json index 185d328a5..a252d776f 100644 --- a/app/javascript/dashboard/i18n/locale/en/generalSettings.json +++ b/app/javascript/dashboard/i18n/locale/en/generalSettings.json @@ -87,7 +87,10 @@ "conversation_assignment": "Conversation Assigned", "assigned_conversation_new_message": "New Message", "participating_conversation_new_message": "New Message", - "conversation_mention": "Mention" + "conversation_mention": "Mention", + "sla_missed_first_response": "SLA Missed", + "sla_missed_next_response": "SLA Missed", + "sla_missed_resolution": "SLA Missed" } }, "NETWORK": { diff --git a/app/javascript/dashboard/i18n/locale/en/inbox.json b/app/javascript/dashboard/i18n/locale/en/inbox.json index c9f67ed1c..137aac54b 100644 --- a/app/javascript/dashboard/i18n/locale/en/inbox.json +++ b/app/javascript/dashboard/i18n/locale/en/inbox.json @@ -22,7 +22,10 @@ "CONVERSATION_CREATION": "New conversation created", "CONVERSATION_ASSIGNMENT": "A conversation has been assigned to you", "ASSIGNED_CONVERSATION_NEW_MESSAGE": "New message in an assigned conversation", - "PARTICIPATING_CONVERSATION_NEW_MESSAGE": "New message in a conversation you are participating in" + "PARTICIPATING_CONVERSATION_NEW_MESSAGE": "New message in a conversation you are participating in", + "SLA_MISSED_FIRST_RESPONSE": "SLA target first response missed for conversation", + "SLA_MISSED_NEXT_RESPONSE": "SLA target next response missed for conversation", + "SLA_MISSED_RESOLUTION": "SLA target resolution missed for conversation" }, "MENU_ITEM": { "MARK_AS_READ": "Mark as read", diff --git a/app/models/notification.rb b/app/models/notification.rb index 25740b16a..c71bdbb62 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -39,7 +39,10 @@ class Notification < ApplicationRecord conversation_assignment: 2, assigned_conversation_new_message: 3, conversation_mention: 4, - participating_conversation_new_message: 5 + participating_conversation_new_message: 5, + sla_missed_first_response: 6, + sla_missed_next_response: 7, + sla_missed_resolution: 8 }.freeze enum notification_type: NOTIFICATION_TYPES @@ -86,21 +89,32 @@ class Notification < ApplicationRecord } end - # TODO: move to a data presenter + # rubocop:disable Metrics/MethodLength def push_message_title - case notification_type - when 'conversation_creation' - I18n.t('notifications.notification_title.conversation_creation', display_id: conversation.display_id, inbox_name: primary_actor.inbox.name) - when 'conversation_assignment' - I18n.t('notifications.notification_title.conversation_assignment', display_id: conversation.display_id) - when 'assigned_conversation_new_message', 'participating_conversation_new_message' - I18n.t('notifications.notification_title.assigned_conversation_new_message', display_id: conversation.display_id) - when 'conversation_mention' - I18n.t('notifications.notification_title.conversation_mention', display_id: conversation.display_id) + notification_title_map = { + 'conversation_creation' => 'notifications.notification_title.conversation_creation', + 'conversation_assignment' => 'notifications.notification_title.conversation_assignment', + 'assigned_conversation_new_message' => 'notifications.notification_title.assigned_conversation_new_message', + 'participating_conversation_new_message' => 'notifications.notification_title.assigned_conversation_new_message', + 'conversation_mention' => 'notifications.notification_title.conversation_mention', + 'sla_missed_first_response' => 'notifications.notification_title.sla_missed_first_response', + 'sla_missed_next_response' => 'notifications.notification_title.sla_missed_next_response', + 'sla_missed_resolution' => 'notifications.notification_title.sla_missed_resolution' + } + + i18n_key = notification_title_map[notification_type] + return '' unless i18n_key + + if notification_type == 'conversation_creation' + I18n.t(i18n_key, display_id: conversation.display_id, inbox_name: primary_actor.inbox.name) + elsif %w[conversation_assignment assigned_conversation_new_message participating_conversation_new_message + conversation_mention].include?(notification_type) + I18n.t(i18n_key, display_id: conversation.display_id) else - '' + I18n.t(i18n_key, display_id: primary_actor.display_id) end end + # rubocop:enable Metrics/MethodLength def push_message_body case notification_type diff --git a/config/locales/en.yml b/config/locales/en.yml index 075d27d1d..5a3816867 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -122,6 +122,9 @@ en: conversation_assignment: "A conversation (#%{display_id}) has been assigned to you" assigned_conversation_new_message: "A new message is created in conversation (#%{display_id})" conversation_mention: "You have been mentioned in conversation (#%{display_id})" + sla_missed_first_response: "SLA target first response missed for conversation (#%{display_id})" + sla_missed_next_response: "SLA target next response missed for conversation (#%{display_id})" + sla_missed_resolution: "SLA target resolution missed for conversation (#%{display_id})" attachment: "Attachment" no_content: "No content" conversations: diff --git a/enterprise/app/models/sla_policy.rb b/enterprise/app/models/sla_policy.rb index 328367aeb..647db2cfc 100644 --- a/enterprise/app/models/sla_policy.rb +++ b/enterprise/app/models/sla_policy.rb @@ -22,4 +22,14 @@ class SlaPolicy < ApplicationRecord validates :name, presence: true has_many :conversations, dependent: :nullify + + def push_event_data + { + id: id, + name: name, + frt: first_response_time_threshold, + nrt: next_response_time_threshold, + rt: resolution_time_threshold + } + 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 817a854c7..d6eb9839e 100644 --- a/enterprise/app/services/sla/evaluate_applied_sla_service.rb +++ b/enterprise/app/services/sla/evaluate_applied_sla_service.rb @@ -30,7 +30,7 @@ class Sla::EvaluateAppliedSlaService return if first_reply_was_within_threshold?(conversation, threshold) return if still_within_threshold?(threshold) - handle_missed_sla(applied_sla) + handle_missed_sla(applied_sla, 'frt') end def first_reply_was_within_threshold?(conversation, threshold) @@ -46,7 +46,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) + handle_missed_sla(applied_sla, 'nrt') end def check_resolution_time_threshold(applied_sla, conversation, sla_policy) @@ -55,13 +55,14 @@ 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) + handle_missed_sla(applied_sla, 'rt') end - def handle_missed_sla(applied_sla) + def handle_missed_sla(applied_sla, type) return unless applied_sla.active? applied_sla.update!(sla_status: 'missed') + generate_notifications_for_sla(applied_sla, type) Rails.logger.warn "SLA missed for conversation #{applied_sla.conversation.id} " \ "in account #{applied_sla.account_id} " \ "for sla_policy #{applied_sla.sla_policy.id}" @@ -75,4 +76,30 @@ class Sla::EvaluateAppliedSlaService "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 + 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 0f82715c0..17c09cb0e 100644 --- a/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb +++ b/spec/enterprise/services/sla/evaluate_applied_sla_service_spec.rb @@ -1,7 +1,11 @@ require 'rails_helper' RSpec.describe Sla::EvaluateAppliedSlaService do - let!(:conversation) { create(:conversation, created_at: 6.hours.ago) } + 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!(:conversation) { create(:conversation, created_at: 6.hours.ago, assignee: user_1, account: account) } let!(:sla_policy) do create(:sla_policy, account: conversation.account, first_response_time_threshold: nil, @@ -20,6 +24,16 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:warn).with("SLA 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) + # 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) end end @@ -35,6 +49,16 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:warn).with("SLA 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) + # 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) end end @@ -47,6 +71,15 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:warn).with("SLA 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) end end @@ -76,6 +109,7 @@ RSpec.describe Sla::EvaluateAppliedSlaService do expect(Rails.logger).to have_received(:warn).with("SLA 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) end end end @@ -99,6 +133,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(Notification.count).to eq(0) end end