diff --git a/app/models/message.rb b/app/models/message.rb index f45f2d8f9..b906f15b3 100644 --- a/app/models/message.rb +++ b/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 diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 4d7886f25..e3ee1959e 100644 --- a/spec/models/message_spec.rb +++ b/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) }