feat: sla-7 ensure applied_sla uniqueness (#8938)

* feat: refactor fetching sla in action service

* chore: modify spec

* chore: ensure applied_sla uniqueness

* chore: review fixes

* feat: add unique index on applied_sla

* chore: fix spec

* chore: add new specs to improve coverage

* chore: improve spec

* Update spec/enterprise/services/enterprise/action_service_spec.rb

---------

Co-authored-by: Sojan Jose <sojan@pepalo.com>

app/services/action_service.rb

@@ -3,6 +3,7 @@ class ActionService
 
   def initialize(conversation)
     @conversation = conversation.reload
+    @account = @conversation.account
   end
 
   def mute_conversation(_params)

db/migrate/20240216055809_add_unique_index_to_applied_slas.rb

@@ -0,0 +1,8 @@
+class AddUniqueIndexToAppliedSlas < ActiveRecord::Migration[7.0]
+  def change
+    add_index :applied_slas,
+              [:account_id, :sla_policy_id, :conversation_id],
+              unique: true,
+              name: 'index_applied_slas_on_account_sla_policy_conversation'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_02_15_065844) do
+ActiveRecord::Schema[7.0].define(version: 2024_02_16_055809) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_stat_statements"
   enable_extension "pg_trgm"
@@ -122,6 +122,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_02_15_065844) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.integer "sla_status", default: 0
+    t.index ["account_id", "sla_policy_id", "conversation_id"], name: "index_applied_slas_on_account_sla_policy_conversation", unique: true
     t.index ["account_id"], name: "index_applied_slas_on_account_id"
     t.index ["conversation_id"], name: "index_applied_slas_on_conversation_id"
     t.index ["sla_policy_id"], name: "index_applied_slas_on_sla_policy_id"

enterprise/app/models/applied_sla.rb

@@ -12,14 +12,17 @@
 #
 # Indexes
 #
-#  index_applied_slas_on_account_id       (account_id)
-#  index_applied_slas_on_conversation_id  (conversation_id)
-#  index_applied_slas_on_sla_policy_id    (sla_policy_id)
+#  index_applied_slas_on_account_id                       (account_id)
+#  index_applied_slas_on_account_sla_policy_conversation  (account_id,sla_policy_id,conversation_id) UNIQUE
+#  index_applied_slas_on_conversation_id                  (conversation_id)
+#  index_applied_slas_on_sla_policy_id                    (sla_policy_id)
 #
 class AppliedSla < ApplicationRecord
   belongs_to :account
   belongs_to :sla_policy
   belongs_to :conversation
 
+  validates :account_id, uniqueness: { scope: %i[sla_policy_id conversation_id] }
+
   enum sla_status: { active: 0, hit: 1, missed: 2 }
 end

enterprise/app/services/enterprise/action_service.rb

@@ -1,10 +1,18 @@
 module Enterprise::ActionService
-  def add_sla(sla_policy)
+  def add_sla(sla_policy_id)
+    return if sla_policy_id.blank?
+
+    sla_policy = @account.sla_policies.find_by(id: sla_policy_id.first)
+    return if sla_policy.nil?
+    return if @conversation.sla_policy.present?
+
+    Rails.logger.info "SLA:: Adding SLA #{sla_policy.id} to conversation: #{@conversation.id}"
     @conversation.update!(sla_policy_id: sla_policy.id)
     create_applied_sla(sla_policy)
   end
 
   def create_applied_sla(sla_policy)
+    Rails.logger.info "SLA:: Creating Applied SLA for conversation: #{@conversation.id}"
     AppliedSla.create!(
       account_id: @conversation.account_id,
       sla_policy_id: sla_policy.id,

spec/enterprise/services/enterprise/action_service_spec.rb

@@ -8,16 +8,42 @@ describe ActionService do
     let(:conversation) { create(:conversation, account: account) }
     let(:action_service) { described_class.new(conversation) }
 
-    it 'adds the sla policy to the conversation and create applied_sla entry' do
-      action_service.add_sla(sla_policy)
-      expect(conversation.reload.sla_policy_id).to eq(sla_policy.id)
+    context 'when sla_policy_id is present' do
+      it 'adds the sla policy to the conversation and create applied_sla entry' do
+        action_service.add_sla([sla_policy.id])
+        expect(conversation.reload.sla_policy_id).to eq(sla_policy.id)
 
-      # check if appliedsla table entry is created with matching attributes
-      applied_sla = AppliedSla.last
-      expect(applied_sla.account_id).to eq(account.id)
-      expect(applied_sla.sla_policy_id).to eq(sla_policy.id)
-      expect(applied_sla.conversation_id).to eq(conversation.id)
-      expect(applied_sla.sla_status).to eq('active')
+        # check if appliedsla table entry is created with matching attributes
+        applied_sla = AppliedSla.last
+        expect(applied_sla.account_id).to eq(account.id)
+        expect(applied_sla.sla_policy_id).to eq(sla_policy.id)
+        expect(applied_sla.conversation_id).to eq(conversation.id)
+        expect(applied_sla.sla_status).to eq('active')
+      end
+    end
+
+    context 'when sla_policy_id is not present' do
+      it 'does not add the sla policy to the conversation' do
+        action_service.add_sla(nil)
+        expect(conversation.reload.sla_policy_id).to be_nil
+      end
+    end
+
+    context 'when conversation already has a sla policy' do
+      it 'does not add the new sla policy to the conversation' do
+        existing_sla_policy = sla_policy
+        new_sla_policy = create(:sla_policy, account: account)
+        conversation.update!(sla_policy_id: existing_sla_policy.id)
+        action_service.add_sla([new_sla_policy.id])
+        expect(conversation.reload.sla_policy_id).to eq(existing_sla_policy.id)
+      end
+    end
+
+    context 'when sla_policy is not found' do
+      it 'does not add the sla policy to the conversation' do
+        action_service.add_sla([sla_policy.id + 1])
+        expect(conversation.reload.sla_policy_id).to be_nil
+      end
     end
   end
 end