feat: Update location and country code when the contact create/update

* 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

app/actions/contact_identify_action.rb

@@ -9,7 +9,7 @@ class ContactIdentifyAction
   pattr_initialize [:contact!, :params!, { retain_original_contact_name: false, discard_invalid_attrs: false }]
 
   def perform
-    @attributes_to_update = [:identifier, :name, :email, :phone_number]
+    @attributes_to_update = [:identifier, :name, :email, :phone_number, :country_code, :location]
 
     ActiveRecord::Base.transaction do
       merge_if_existing_identified_contact
@@ -99,7 +99,8 @@ class ContactIdentifyAction
   def update_contact
     @contact.attributes = params.slice(*@attributes_to_update).reject do |_k, v|
       v.blank?
-    end.merge({ custom_attributes: custom_attributes, additional_attributes: additional_attributes })
+    end.merge({ custom_attributes: custom_attributes, additional_attributes: additional_attributes, location: contact_location,
+                country_code: contact_country_code })
     # blank identifier or email will throw unique index error
     # TODO: replace reject { |_k, v| v.blank? } with compact_blank when rails is upgraded
     @contact.discard_invalid_attrs if discard_invalid_attrs
@@ -117,6 +118,16 @@ class ContactIdentifyAction
     ).perform
   end
 
+  # TODO: Remove this once we standardize the location field across the app
+  def contact_location
+    params[:location] || params[:additional_attributes].fetch(:city, nil)
+  end
+
+  # TODO: Remove this once we standardize the location field across the app
+  def contact_country_code
+    params[:country_code] || params[:additional_attributes].fetch(:country, nil)
+  end
+
   def custom_attributes
     return @contact.custom_attributes if params[:custom_attributes].blank?
 

app/builders/contact_inbox_with_contact_builder.rb

@@ -49,14 +49,35 @@ class ContactInboxWithContactBuilder
   end
 
   def create_contact
-    account.contacts.create!(
-      name: contact_attributes[:name] || ::Haikunator.haikunate(1000),
+    contact_params = build_contact_params
+    account.contacts.create!(contact_params)
+  end
+
+  def build_contact_params
+    {
+      name: contact_name,
       phone_number: contact_attributes[:phone_number],
       email: contact_attributes[:email],
       identifier: contact_attributes[:identifier],
+      location: contact_location,
+      country_code: contact_country_code,
       additional_attributes: contact_attributes[:additional_attributes],
       custom_attributes: contact_attributes[:custom_attributes]
-    )
+    }
+  end
+
+  def contact_name
+    contact_attributes[:name] || ::Haikunator.haikunate(1000)
+  end
+
+  def contact_location
+    # TODO: Deprecate the additional_attributes['city'] in favor of country_code
+    contact_attributes[:location] || contact_attributes.dig(:additional_attributes, 'city')
+  end
+
+  def contact_country_code
+    # TODO: Deprecate the additional_attributes['country_code'] in favor of country_code
+    contact_attributes[:country_code] || contact_attributes.dig(:additional_attributes, 'country_code')
   end
 
   def find_contact

app/controllers/api/v1/accounts/contacts_controller.rb

@@ -89,6 +89,9 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
 
   def update
     @contact.assign_attributes(contact_update_params)
+    # TODO: Remove this once we standardize the location field across the app
+    @contact.country_code = params[:country_code] || params.dig(:additional_attributes, 'country_code')
+    @contact.location = params[:location] || params.dig(:additional_attributes, 'city')
     @contact.save!
     process_avatar_from_url
   end
@@ -148,7 +151,8 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController
   end
 
   def permitted_params
-    params.permit(:name, :identifier, :email, :phone_number, :avatar, :blocked, :avatar_url, additional_attributes: {}, custom_attributes: {})
+    params.permit(:name, :identifier, :email, :phone_number, :avatar, :location, :country_code, :blocked, :avatar_url, additional_attributes: {},
+                                                                                                                       custom_attributes: {})
   end
 
   def contact_custom_attributes

app/controllers/api/v1/widget/contacts_controller.rb

@@ -70,7 +70,18 @@ class Api::V1::Widget::ContactsController < Api::V1::Widget::BaseController
   end
 
   def permitted_params
-    params.permit(:website_token, :identifier, :identifier_hash, :email, :name, :avatar_url, :phone_number, custom_attributes: {},
-                                                                                                            additional_attributes: {})
+    params.permit(
+      :website_token,
+      :identifier,
+      :identifier_hash,
+      :email,
+      :name,
+      :avatar_url,
+      :location,
+      :country_code,
+      :phone_number,
+      custom_attributes: {},
+      additional_attributes: {}
+    )
   end
 end

app/controllers/public/api/v1/inboxes/contacts_controller.rb

@@ -43,6 +43,6 @@ class Public::Api::V1::Inboxes::ContactsController < Public::Api::V1::InboxesCon
   end
 
   def permitted_params
-    params.permit(:identifier, :identifier_hash, :email, :name, :avatar_url, :phone_number, custom_attributes: {})
+    params.permit(:identifier, :identifier_hash, :email, :name, :avatar_url, :location, :city, :phone_number, custom_attributes: {})
   end
 end

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": {

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",

app/jobs/contact_ip_lookup_job.rb

@@ -14,9 +14,12 @@ class ContactIpLookupJob < ApplicationJob
     return unless geocoder_result
 
     contact.additional_attributes ||= {}
+    # TODO: Deprecate these fields in favor of the new location field
     contact.additional_attributes['city'] = geocoder_result.city
     contact.additional_attributes['country'] = geocoder_result.country
     contact.additional_attributes['country_code'] = geocoder_result.country_code
+    contact.country_code = geocoder_result.country_code
+    contact.location = geocoder_result.city
     contact.save!
   end
 

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

app/services/data_import/contact_manager.rb

@@ -60,9 +60,15 @@ class DataImport::ContactManager
 
   def update_contact_attributes(params, contact)
     contact.name = params[:name] if params[:name].present?
+    update_additional_attributes(params, contact)
+    contact.country_code = params[:country_code] if params[:country_code].present?
+    contact.location = params[:location] if params[:location].present?
+    contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name, :phone_number)))
+  end
+
+  def update_additional_attributes(params, contact)
     contact.additional_attributes ||= {}
     contact.additional_attributes[:company] = params[:company] if params[:company].present?
     contact.additional_attributes[:city] = params[:city] if params[:city].present?
-    contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name, :phone_number)))
   end
 end

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:

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

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

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