fix: [CW-44] don't count private message as first reply (#6707)

* fix: don't count private message as first reply

* fix: update first_human_response_logic

* refactor: separate valid_first_reply method

* test: valid first reply

* feat: add check for automation rule

* test: update step that creates data

* fix: add boundary condition in case first_reply_created_at is not present

* test: fix report builder

* refactor: conditions

* test: remove second message condition

app/models/message.rb

@@ -173,6 +173,10 @@ class Message < ApplicationRecord
     true
   end
 
+  def valid_first_reply?
+    outgoing? && human_response? && not_created_by_automation? && !private?
+  end
+
   private
 
   def ensure_content_type
@@ -194,10 +198,29 @@ class Message < ApplicationRecord
     sender.update(last_activity_at: DateTime.now) if sender.is_a?(Contact)
   end
 
-  def first_human_response?
-    conversation.messages.outgoing
-                .where.not(sender_type: 'AgentBot')
-                .where("(additional_attributes->'campaign_id') is null").count == 1
+  def human_response?
+    # given the checks are already in place, we need not query
+    # the database again to check if the message is created by a human
+    # we can just see if the first_reply is recorded or not
+    # if it is record, we can just return false
+    return false if conversation.first_reply_created_at.present?
+
+    # if the sender is not a user, it's not a human response
+    return false unless sender.is_a?(User)
+
+    # if automation rule id is present, it's not a human response
+    # if campaign id is present, it's not a human response
+    # this check already happens in `not_created_by_automation` but added here for the sake of brevity
+    # also the purity of this method is intact, and can be relied on this solely
+    return false if content_attributes['automation_rule_id'].present? || additional_attributes['campaign_id'].present?
+
+    # adding this condition again to ensure if the first_reply_created_at is not present
+    return false if conversation.messages.outgoing
+                                .where.not(sender_type: 'AgentBot')
+                                .where.not(private: true)
+                                .where("(additional_attributes->'campaign_id') is null").count > 1
+
+    true
   end
 
   def not_created_by_automation?
@@ -206,7 +229,7 @@ class Message < ApplicationRecord
 
   def dispatch_create_events
     Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by)
-    if outgoing? && first_human_response? && not_created_by_automation?
+    if valid_first_reply?
       Rails.configuration.dispatcher.dispatch(FIRST_REPLY_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by)
     end
   end

spec/models/message_spec.rb

@@ -14,6 +14,59 @@ RSpec.describe Message, type: :model do
     it_behaves_like 'liqudable'
   end
 
+  describe 'Check if message is a valid first reply' do
+    it 'is valid if it is outgoing' do
+      outgoing_message = create(:message, message_type: :outgoing)
+      expect(outgoing_message.valid_first_reply?).to be true
+    end
+
+    it 'is invalid if it is not outgoing' do
+      incoming_message = create(:message, message_type: :incoming)
+      expect(incoming_message.valid_first_reply?).to be false
+
+      activity_message = create(:message, message_type: :activity)
+      expect(activity_message.valid_first_reply?).to be false
+
+      template_message = create(:message, message_type: :template)
+      expect(template_message.valid_first_reply?).to be false
+    end
+
+    it 'is invalid if it is outgoing but private' do
+      conversation = create(:conversation)
+
+      outgoing_message = create(:message, message_type: :outgoing, conversation: conversation, private: true)
+      expect(outgoing_message.valid_first_reply?).to be false
+
+      # next message should be a valid reply
+      next_message = create(:message, message_type: :outgoing, conversation: conversation)
+      expect(next_message.valid_first_reply?).to be true
+    end
+
+    it 'is invalid if it is not the first reply' do
+      conversation = create(:conversation)
+      first_message = create(:message, message_type: :outgoing, conversation: conversation)
+      expect(first_message.valid_first_reply?).to be true
+
+      second_message = create(:message, message_type: :outgoing, conversation: conversation)
+      expect(second_message.valid_first_reply?).to be false
+    end
+
+    it 'is invalid if it is sent as campaign' do
+      conversation = create(:conversation)
+      campaign_message = create(:message, message_type: :outgoing, conversation: conversation, additional_attributes: { campaign_id: 1 })
+      expect(campaign_message.valid_first_reply?).to be false
+
+      second_message = create(:message, message_type: :outgoing, conversation: conversation)
+      expect(second_message.valid_first_reply?).to be true
+    end
+
+    it 'is invalid if it is sent by automation' do
+      conversation = create(:conversation)
+      automation_message = create(:message, message_type: :outgoing, conversation: conversation, content_attributes: { automation_rule_id: 1 })
+      expect(automation_message.valid_first_reply?).to be false
+    end
+  end
+
   describe '#reopen_conversation' do
     let(:conversation) { create(:conversation) }
     let(:message) { build(:message, message_type: :incoming, conversation: conversation) }