From 4e28481f274d4a17b5b765444e30595ff2d77a4f Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Mon, 1 Apr 2024 23:30:07 +0530
Subject: [PATCH] feat: Conversation API to return applied_sla and sla_events
 (#9174)

* chore: Add sla_events to push_event_data

* chore: Return SLA details in the API

* chore: feature lock sla push event data

* Update _conversation.json.jbuilder

* chore: rubocop fixes
---
 app/controllers/application_controller.rb     |  1 +
 app/finders/conversation_finder.rb            |  9 +++--
 app/models/conversation.rb                    |  2 +-
 .../conversations/event_data_presenter.rb     |  1 +
 .../partials/_conversation.json.jbuilder      |  5 +++
 .../api/v1/models/_conversation.json.jbuilder |  2 ++
 .../enterprise_accounts_controller.rb         |  6 ----
 .../application_controller_concern.rb         | 12 +++++++
 .../finders/enterprise/conversation_finder.rb |  5 +++
 enterprise/app/models/applied_sla.rb          | 17 ++++++++++
 .../conversation.rb}                          |  5 +--
 enterprise/app/models/sla_event.rb            | 11 +++++-
 .../conversations/event_data_presenter.rb     | 12 +++++++
 .../api/v1/models/_applied_sla.json.jbuilder  | 11 ++++++
 .../partials/_conversation.json.jbuilder      | 10 ++++++
 .../accounts/conversations_controller_spec.rb | 34 +++++++++++++++++++
 spec/enterprise/models/applied_sla_spec.rb    | 21 ++++++++++++
 spec/enterprise/models/sla_event_spec.rb      | 15 ++++++++
 .../event_data_presenter_spec.rb              | 29 ++++++++++++++++
 .../event_data_presenter_spec.rb              |  3 +-
 20 files changed, 198 insertions(+), 13 deletions(-)
 create mode 100644 enterprise/app/controllers/enterprise/concerns/application_controller_concern.rb
 create mode 100644 enterprise/app/finders/enterprise/conversation_finder.rb
 rename enterprise/app/models/enterprise/{enterprise_conversation_concern.rb => concerns/conversation.rb} (88%)
 create mode 100644 enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb
 create mode 100644 enterprise/app/views/api/v1/models/_applied_sla.json.jbuilder
 create mode 100644 enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder
 create mode 100644 spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb
 create mode 100644 spec/enterprise/presenters/conversations/event_data_presenter_spec.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d2960a699..2f389049d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -25,3 +25,4 @@ class ApplicationController < ActionController::Base
     }
   end
 end
+ApplicationController.include_mod_with('Concerns::ApplicationControllerConcern')
diff --git a/app/finders/conversation_finder.rb b/app/finders/conversation_finder.rb
index f3c85be7a..44592c201 100644
--- a/app/finders/conversation_finder.rb
+++ b/app/finders/conversation_finder.rb
@@ -163,10 +163,14 @@ class ConversationFinder
     params[:page] || 1
   end
 
-  def conversations
-    @conversations = @conversations.includes(
+  def conversations_base_query
+    @conversations.includes(
       :taggings, :inbox, { assignee: { avatar_attachment: [:blob] } }, { contact: { avatar_attachment: [:blob] } }, :team, :contact_inbox
     )
+  end
+
+  def conversations
+    @conversations = conversations_base_query
 
     sort_by, sort_order = SORT_OPTIONS[params[:sort_by]] || SORT_OPTIONS['last_activity_at_desc']
     @conversations = @conversations.send(sort_by, sort_order)
@@ -178,3 +182,4 @@ class ConversationFinder
     end
   end
 end
+ConversationFinder.prepend_mod_with('ConversationFinder')
diff --git a/app/models/conversation.rb b/app/models/conversation.rb
index 513c993da..550c09df1 100644
--- a/app/models/conversation.rb
+++ b/app/models/conversation.rb
@@ -312,5 +312,5 @@ class Conversation < ApplicationRecord
   end
 end
 
-Conversation.include_mod_with('EnterpriseConversationConcern')
+Conversation.include_mod_with('Concerns::Conversation')
 Conversation.include_mod_with('SentimentAnalysisHelper')
diff --git a/app/presenters/conversations/event_data_presenter.rb b/app/presenters/conversations/event_data_presenter.rb
index f739554b3..67c7dc1dd 100644
--- a/app/presenters/conversations/event_data_presenter.rb
+++ b/app/presenters/conversations/event_data_presenter.rb
@@ -45,3 +45,4 @@ class Conversations::EventDataPresenter < SimpleDelegator
     }
   end
 end
+Conversations::EventDataPresenter.prepend_mod_with('Conversations::EventDataPresenter')
diff --git a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
index 2e90e073a..a9a580e26 100644
--- a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
+++ b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
@@ -1,3 +1,7 @@
+# TODO: Move this into models jbuilder
+# Currently the file there is used only for search endpoint.
+# Everywhere else we use conversation builder in partials folder
+
 json.meta do
   json.sender do
     json.partial! 'api/v1/models/contact', formats: [:json], resource: conversation.contact
@@ -48,3 +52,4 @@ json.last_activity_at conversation.last_activity_at.to_i
 json.priority conversation.priority
 json.waiting_since conversation.waiting_since.to_i.to_i
 json.sla_policy_id conversation.sla_policy_id
+json.partial! 'enterprise/api/v1/conversations/partials/conversation', conversation: conversation if ChatwootApp.enterprise?
diff --git a/app/views/api/v1/models/_conversation.json.jbuilder b/app/views/api/v1/models/_conversation.json.jbuilder
index 3fa54bcd9..68dfd2c40 100644
--- a/app/views/api/v1/models/_conversation.json.jbuilder
+++ b/app/views/api/v1/models/_conversation.json.jbuilder
@@ -1,3 +1,5 @@
+# This file is used to render conversation data search API response.
+
 json.id conversation.display_id
 json.uuid conversation.uuid
 json.created_at conversation.created_at.to_i
diff --git a/enterprise/app/controllers/api/v1/accounts/enterprise_accounts_controller.rb b/enterprise/app/controllers/api/v1/accounts/enterprise_accounts_controller.rb
index f3028721c..32182d798 100644
--- a/enterprise/app/controllers/api/v1/accounts/enterprise_accounts_controller.rb
+++ b/enterprise/app/controllers/api/v1/accounts/enterprise_accounts_controller.rb
@@ -1,8 +1,2 @@
 class Api::V1::Accounts::EnterpriseAccountsController < Api::V1::Accounts::BaseController
-  before_action :prepend_view_paths
-
-  # Prepend the view path to the enterprise/app/views won't be available by default
-  def prepend_view_paths
-    prepend_view_path 'enterprise/app/views/'
-  end
 end
diff --git a/enterprise/app/controllers/enterprise/concerns/application_controller_concern.rb b/enterprise/app/controllers/enterprise/concerns/application_controller_concern.rb
new file mode 100644
index 000000000..a44a0eceb
--- /dev/null
+++ b/enterprise/app/controllers/enterprise/concerns/application_controller_concern.rb
@@ -0,0 +1,12 @@
+module Enterprise::Concerns::ApplicationControllerConcern
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :prepend_view_paths
+  end
+
+  # Prepend the view path to the enterprise/app/views won't be available by default
+  def prepend_view_paths
+    prepend_view_path 'enterprise/app/views/'
+  end
+end
diff --git a/enterprise/app/finders/enterprise/conversation_finder.rb b/enterprise/app/finders/enterprise/conversation_finder.rb
new file mode 100644
index 000000000..24e6b493d
--- /dev/null
+++ b/enterprise/app/finders/enterprise/conversation_finder.rb
@@ -0,0 +1,5 @@
+module Enterprise::ConversationFinder
+  def conversations_base_query
+    current_account.feature_enabled?('sla') ? super.includes(:applied_sla, :sla_events) : super
+  end
+end
diff --git a/enterprise/app/models/applied_sla.rb b/enterprise/app/models/applied_sla.rb
index 111b78e84..8207b2cc0 100644
--- a/enterprise/app/models/applied_sla.rb
+++ b/enterprise/app/models/applied_sla.rb
@@ -40,6 +40,23 @@ class AppliedSla < ApplicationRecord
                                         end
                                       }
   scope :missed, -> { where(sla_status: :missed) }
+
+  def push_event_data
+    {
+      id: id,
+      sla_id: sla_policy_id,
+      sla_status: sla_status,
+      created_at: created_at.to_i,
+      updated_at: updated_at.to_i,
+      sla_description: sla_policy.description,
+      sla_name: sla_policy.name,
+      sla_first_response_time_threshold: sla_policy.first_response_time_threshold,
+      sla_next_response_time_threshold: sla_policy.next_response_time_threshold,
+      sla_only_during_business_hours: sla_policy.only_during_business_hours,
+      sla_resolution_time_threshold: sla_policy.resolution_time_threshold
+    }
+  end
+
   private
 
   def ensure_account_id
diff --git a/enterprise/app/models/enterprise/enterprise_conversation_concern.rb b/enterprise/app/models/enterprise/concerns/conversation.rb
similarity index 88%
rename from enterprise/app/models/enterprise/enterprise_conversation_concern.rb
rename to enterprise/app/models/enterprise/concerns/conversation.rb
index 721f069dc..f03d643f4 100644
--- a/enterprise/app/models/enterprise/enterprise_conversation_concern.rb
+++ b/enterprise/app/models/enterprise/concerns/conversation.rb
@@ -1,9 +1,10 @@
-module Enterprise::EnterpriseConversationConcern
+module Enterprise::Concerns::Conversation
   extend ActiveSupport::Concern
 
   included do
     belongs_to :sla_policy, optional: true
-    has_one :applied_sla, dependent: :destroy
+    has_one :applied_sla, dependent: :destroy_async
+    has_many :sla_events, dependent: :destroy_async
     before_validation :validate_sla_policy, if: -> { sla_policy_id_changed? }
     around_save :ensure_applied_sla_is_created, if: -> { sla_policy_id_changed? }
   end
diff --git a/enterprise/app/models/sla_event.rb b/enterprise/app/models/sla_event.rb
index 59068684e..1ba18f01a 100644
--- a/enterprise/app/models/sla_event.rb
+++ b/enterprise/app/models/sla_event.rb
@@ -31,9 +31,18 @@ class SlaEvent < ApplicationRecord
   enum event_type: { frt: 0, nrt: 1, rt: 2 }
 
   before_validation :ensure_applied_sla_id, :ensure_account_id, :ensure_inbox_id, :ensure_sla_policy_id
-
   after_create_commit :create_notifications
 
+  def push_event_data
+    {
+      id: id,
+      event_type: event_type,
+      meta: meta,
+      created_at: created_at.to_i,
+      updated_at: updated_at.to_i
+    }
+  end
+
   private
 
   def ensure_applied_sla_id
diff --git a/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb b/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb
new file mode 100644
index 000000000..686b05dc2
--- /dev/null
+++ b/enterprise/app/presenters/enterprise/conversations/event_data_presenter.rb
@@ -0,0 +1,12 @@
+module Enterprise::Conversations::EventDataPresenter
+  def push_data
+    if account.feature_enabled?('sla')
+      super.merge(
+        applied_sla: applied_sla&.push_event_data,
+        sla_events: sla_events.map(&:push_event_data)
+      )
+    else
+      super
+    end
+  end
+end
diff --git a/enterprise/app/views/api/v1/models/_applied_sla.json.jbuilder b/enterprise/app/views/api/v1/models/_applied_sla.json.jbuilder
new file mode 100644
index 000000000..e7f1c49fc
--- /dev/null
+++ b/enterprise/app/views/api/v1/models/_applied_sla.json.jbuilder
@@ -0,0 +1,11 @@
+json.id resource.id
+json.sla_id resource.sla_policy_id
+json.sla_status resource.sla_status
+json.created_at resource.created_at.to_i
+json.updated_at resource.updated_at.to_i
+json.sla_description resource.sla_policy.description
+json.sla_name resource.sla_policy.name
+json.sla_first_response_time_threshold resource.sla_policy.first_response_time_threshold
+json.sla_next_response_time_threshold resource.sla_policy.next_response_time_threshold
+json.sla_only_during_business_hours resource.sla_policy.only_during_business_hours
+json.sla_resolution_time_threshold resource.sla_policy.resolution_time_threshold
diff --git a/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder b/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder
new file mode 100644
index 000000000..5a390a68b
--- /dev/null
+++ b/enterprise/app/views/enterprise/api/v1/conversations/partials/_conversation.json.jbuilder
@@ -0,0 +1,10 @@
+if conversation.account.feature_enabled?('sla')
+  json.applied_sla do
+    json.partial! 'api/v1/models/applied_sla', formats: [:json], resource: conversation.applied_sla if conversation.applied_sla.present?
+  end
+  json.sla_events do
+    json.array! conversation.sla_events do |sla_event|
+      json.partial! 'api/v1/models/sla_event', formats: [:json], sla_event: sla_event
+    end
+  end
+end
diff --git a/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb
new file mode 100644
index 000000000..4d5269cde
--- /dev/null
+++ b/spec/enterprise/controllers/api/v1/accounts/conversations_controller_spec.rb
@@ -0,0 +1,34 @@
+require 'rails_helper'
+
+RSpec.describe 'Conversations API', type: :request do
+  let(:account) { create(:account) }
+  let(:administrator) { create(:user, account: account, role: :administrator) }
+
+  describe 'GET /api/v1/accounts/{account.id}/conversations/:id' do
+    it 'returns SLA data for the conversation if the feature is enabled' do
+      account.enable_features!('sla')
+      conversation = create(:conversation, account: account)
+      applied_sla = create(:applied_sla, conversation: conversation)
+      sla_event = create(:sla_event, conversation: conversation, applied_sla: applied_sla)
+
+      get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: administrator.create_new_auth_token
+
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body['applied_sla']['id']).to eq(applied_sla.id)
+      expect(response.parsed_body['sla_events'].first['id']).to eq(sla_event.id)
+    end
+
+    it 'does not return SLA data for the conversation if the feature is disabled' do
+      account.disable_features!('sla')
+      conversation = create(:conversation, account: account)
+      create(:applied_sla, conversation: conversation)
+      create(:sla_event, conversation: conversation)
+
+      get "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}", headers: administrator.create_new_auth_token
+
+      expect(response).to have_http_status(:ok)
+      expect(response.parsed_body.keys).not_to include('applied_sla')
+      expect(response.parsed_body.keys).not_to include('sla_events')
+    end
+  end
+end
diff --git a/spec/enterprise/models/applied_sla_spec.rb b/spec/enterprise/models/applied_sla_spec.rb
index af73395a6..a1433f6a2 100644
--- a/spec/enterprise/models/applied_sla_spec.rb
+++ b/spec/enterprise/models/applied_sla_spec.rb
@@ -7,6 +7,27 @@ RSpec.describe AppliedSla, type: :model do
     it { is_expected.to belong_to(:conversation) }
   end
 
+  describe 'push_event_data' do
+    it 'returns the correct hash' do
+      applied_sla = create(:applied_sla)
+      expect(applied_sla.push_event_data).to eq(
+        {
+          id: applied_sla.id,
+          sla_id: applied_sla.sla_policy_id,
+          sla_status: applied_sla.sla_status,
+          created_at: applied_sla.created_at.to_i,
+          updated_at: applied_sla.updated_at.to_i,
+          sla_description: applied_sla.sla_policy.description,
+          sla_name: applied_sla.sla_policy.name,
+          sla_first_response_time_threshold: applied_sla.sla_policy.first_response_time_threshold,
+          sla_next_response_time_threshold: applied_sla.sla_policy.next_response_time_threshold,
+          sla_only_during_business_hours: applied_sla.sla_policy.only_during_business_hours,
+          sla_resolution_time_threshold: applied_sla.sla_policy.resolution_time_threshold
+        }
+      )
+    end
+  end
+
   describe 'validates_factory' do
     it 'creates valid applied sla policy object' do
       applied_sla = create(:applied_sla)
diff --git a/spec/enterprise/models/sla_event_spec.rb b/spec/enterprise/models/sla_event_spec.rb
index 3c44d3961..8a839d626 100644
--- a/spec/enterprise/models/sla_event_spec.rb
+++ b/spec/enterprise/models/sla_event_spec.rb
@@ -9,6 +9,21 @@ RSpec.describe SlaEvent, type: :model do
     it { is_expected.to belong_to(:inbox) }
   end
 
+  describe 'push_event_data' do
+    it 'returns the correct hash' do
+      sla_event = create(:sla_event)
+      expect(sla_event.push_event_data).to eq(
+        {
+          id: sla_event.id,
+          event_type: 'frt',
+          meta: sla_event.meta,
+          created_at: sla_event.created_at.to_i,
+          updated_at: sla_event.updated_at.to_i
+        }
+      )
+    end
+  end
+
   describe 'validates_factory' do
     it 'creates valid sla event object' do
       sla_event = create(:sla_event)
diff --git a/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb b/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb
new file mode 100644
index 000000000..f9897c96d
--- /dev/null
+++ b/spec/enterprise/presenters/conversations/event_data_presenter_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Conversations::EventDataPresenter do
+  let!(:presenter) { described_class.new(conversation) }
+  let!(:conversation) { create(:conversation) }
+  let!(:applied_sla) { create(:applied_sla, conversation: conversation) }
+  let!(:sla_event) { create(:sla_event, conversation: conversation, applied_sla: applied_sla) }
+
+  describe '#push_data' do
+    it 'returns push event payload with applied sla & sla events if the feature is enabled' do
+      conversation.account.enable_features!('sla')
+
+      expect(presenter.push_data).to include(
+        {
+          applied_sla: applied_sla.push_event_data,
+          sla_events: [sla_event.push_event_data]
+        }
+      )
+    end
+
+    it 'returns push event payload without applied sla & sla events if the feature is disabled' do
+      conversation.account.disable_features!('sla')
+
+      expect(presenter.push_data).not_to include(:applied_sla, :sla_events)
+    end
+  end
+end
diff --git a/spec/presenters/conversations/event_data_presenter_spec.rb b/spec/presenters/conversations/event_data_presenter_spec.rb
index 557d2b95c..f6a400b96 100644
--- a/spec/presenters/conversations/event_data_presenter_spec.rb
+++ b/spec/presenters/conversations/event_data_presenter_spec.rb
@@ -38,7 +38,8 @@ RSpec.describe Conversations::EventDataPresenter do
     end
 
     it 'returns push event payload' do
-      expect(presenter.push_data).to eq(expected_data)
+      # the exceptions are the values that would be added in enterprise edition.
+      expect(presenter.push_data.except(:applied_sla, :sla_events)).to include(expected_data)
     end
   end
 end