Merge branch 'develop' into fix/conv-open-on-human-msg

app/models/message.rb

@@ -310,10 +310,11 @@ class Message < ApplicationRecord
   def execute_after_create_commit_callbacks
     # rails issue with order of active record callbacks being executed https://github.com/rails/rails/issues/20911
     reopen_conversation
+    open_conversation_on_human_response
     set_conversation_activity
     dispatch_create_events
     send_reply
-    execute_message_template_hooks
+    execute_message_template_hooks if incoming?
     update_contact_activity
   end
 
@@ -389,6 +390,14 @@ class Message < ApplicationRecord
     reopen_resolved_conversation if conversation.resolved?
   end
 
+  def open_conversation_on_human_response
+    return unless human_response?
+    return unless conversation.pending?
+    return if private?
+
+    conversation.open!
+  end
+
   def reopen_resolved_conversation
     # mark resolved bot conversation as pending to be reopened by bot processor service
     if conversation.inbox.active_bot?

app/services/message_templates/hook_execution_service.rb

@@ -49,6 +49,11 @@ class MessageTemplates::HookExecutionService
 
   # TODO: we should be able to reduce this logic once we have a toggle for email collect messages
   def should_send_email_collect?
+    # Don't send email collect if a human agent has already replied
+    # It breaks UX if an automated template shows up during a human conversation
+    # The agent can manually collect email if needed
+    return false if conversation.messages.outgoing.exists?(private: false, sender_type: 'User')
+
     !contact_has_email? && inbox.web_widget? && !email_collect_was_sent?
   end
 

spec/models/message_spec.rb

@@ -271,6 +271,64 @@ RSpec.describe Message do
     end
   end
 
+  describe '#open_conversation_on_human_response' do
+    let(:conversation) { create(:conversation, status: :pending) }
+    let(:agent) { create(:user, account: conversation.account) }
+
+    it 'opens pending conversation when human agent sends a message' do
+      message = create(:message, conversation: conversation, message_type: :outgoing, sender: agent)
+      expect(message.conversation.reload.open?).to be true
+    end
+
+    it 'does not change status when conversation is not pending' do
+      conversation.update!(status: :open)
+      message = create(:message, conversation: conversation, message_type: :outgoing, sender: agent)
+      expect(message.conversation.reload.open?).to be true
+    end
+
+    it 'does not change status for incoming messages' do
+      message = create(:message, conversation: conversation, message_type: :incoming)
+      expect(message.conversation.reload.pending?).to be true
+    end
+
+    it 'does not change status for bot messages' do
+      agent_bot = create(:agent_bot, account: conversation.account)
+      message = create(:message, conversation: conversation, message_type: :outgoing, sender: agent_bot)
+      expect(message.conversation.reload.pending?).to be true
+    end
+
+    it 'does not change status for automation rule messages' do
+      message = create(:message, conversation: conversation, message_type: :outgoing, sender: agent,
+                                 content_attributes: { 'automation_rule_id' => 123 })
+      expect(message.conversation.reload.pending?).to be true
+    end
+
+    it 'does not change status for private notes' do
+      message = create(:message, conversation: conversation, message_type: :outgoing, sender: agent, private: true)
+      expect(message.conversation.reload.pending?).to be true
+    end
+  end
+
+  describe '#execute_message_template_hooks' do
+    let(:conversation) { create(:conversation) }
+    let(:agent) { create(:user, account: conversation.account) }
+
+    it 'does not execute template hooks for human agent messages' do
+      expect(MessageTemplates::HookExecutionService).not_to receive(:new)
+      create(:message, conversation: conversation, message_type: :outgoing, sender: agent)
+    end
+
+    it 'does not execute template hooks for activity messages' do
+      expect(MessageTemplates::HookExecutionService).not_to receive(:new)
+      create(:message, conversation: conversation, message_type: :activity, content: 'Conversation was reopened')
+    end
+
+    it 'executes template hooks for incoming messages' do
+      expect(MessageTemplates::HookExecutionService).to receive(:new).and_call_original
+      create(:message, conversation: conversation, message_type: :incoming)
+    end
+  end
+
   describe '#waiting since' do
     let(:conversation) { create(:conversation) }
     let(:agent) { create(:user, account: conversation.account) }