Enhancement: ability to send out an email without summary #1048 (#1049)

Right now as part of conversation continuity, we are using the
ConversationReplyMailer which sends a summary of messages including
the incoming messages when an agent replies. Ideally, we want
to send only the reply of that agent and not a summary when
Conversation continuity is enabled. Added the functionality
to send the reply email without summary. Added required unit
tests to cover the changes.

ref: #1048

app/mailers/conversation_reply_mailer.rb

@@ -20,7 +20,27 @@ class ConversationReplyMailer < ApplicationMailer
            to: @contact&.email,
            from: from_email,
            reply_to: reply_email,
-           subject: mail_subject(@messages.last),
+           subject: mail_subject,
+           message_id: custom_message_id,
+           in_reply_to: in_reply_to_email
+         })
+  end
+
+  def reply_without_summary(conversation, message_queued_time)
+    return unless smtp_config_set_or_development?
+
+    @conversation = conversation
+    @account = @conversation.account
+    @contact = @conversation.contact
+    @agent = @conversation.assignee
+
+    @messages = @conversation.messages.outgoing.where('created_at >= ?', message_queued_time)
+
+    mail({
+           to: @contact&.email,
+           from: from_email,
+           reply_to: reply_email,
+           subject: mail_subject,
            message_id: custom_message_id,
            in_reply_to: in_reply_to_email
          })
@@ -28,7 +48,7 @@ class ConversationReplyMailer < ApplicationMailer
 
   private
 
-  def mail_subject(_last_message, _trim_length = 50)
+  def mail_subject
     subject_line = I18n.t('conversations.reply.email_subject')
     "[##{@conversation.display_id}] #{subject_line}"
   end
@@ -50,7 +70,7 @@ class ConversationReplyMailer < ApplicationMailer
   end
 
   def custom_message_id
-    "<conversation/#{@conversation.uuid}/messages/#{@messages.last.id}@#{current_domain}>"
+    "<conversation/#{@conversation.uuid}/messages/#{@messages&.last&.id}@#{current_domain}>"
   end
 
   def in_reply_to_email

app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb

@@ -0,0 +1,12 @@
+<% @messages.each do |message| %>
+  <p>
+    <% if message.content %>
+      <%= message.content %>
+    <% end %>
+    <% if message.attachments %>
+      <% message.attachments.each do |attachment| %>
+        attachment [<a href="<%= attachment.file_url %>" _target="blank">click here to view</a>]
+      <% end %>
+    <% end %>
+  </p>
+<% end %>

app/workers/conversation_reply_email_worker.rb

@@ -6,7 +6,11 @@ class ConversationReplyEmailWorker
     @conversation = Conversation.find(conversation_id)
 
     # send the email
-    ConversationReplyMailer.reply_with_summary(@conversation, queued_time).deliver_later
+    if @conversation.messages.incoming&.last&.content_type == 'incoming_email'
+      ConversationReplyMailer.reply_without_summary(@conversation, queued_time).deliver_later
+    else
+      ConversationReplyMailer.reply_with_summary(@conversation, queued_time).deliver_later
+    end
 
     # delete the redis set from the first new message on the conversation
     Redis::Alfred.delete(conversation_mail_key)

spec/factories/messages.rb

@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :message do
-    content { 'Message' }
+    content { 'Incoming Message' }
     status { 'sent' }
     message_type { 'incoming' }
     content_type { 'text' }

spec/mailers/conversation_reply_mailer_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 
 RSpec.describe ConversationReplyMailer, type: :mailer do
-  describe 'reply_with_summary' do
+  describe 'reply' do
     let!(:account) { create(:account) }
     let!(:agent) { create(:user, email: 'agent1@example.com', account: account) }
     let(:class_instance) { described_class.new }
@@ -13,7 +13,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       allow(class_instance).to receive(:smtp_config_set_or_development?).and_return(true)
     end
 
-    context 'with all mails' do
+    context 'with summary' do
       let(:conversation) { create(:conversation, assignee: agent) }
       let(:message) { create(:message, conversation: conversation) }
       let(:private_message) { create(:message, content: 'This is a private message', conversation: conversation) }
@@ -33,6 +33,35 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       end
     end
 
+    context 'without summary' do
+      let(:conversation) { create(:conversation, assignee: agent) }
+      let(:message_1) { create(:message, conversation: conversation) }
+      let(:message_2) { build(:message, conversation: conversation, message_type: 'outgoing', content: 'Outgoing Message') }
+      let(:private_message) { create(:message, content: 'This is a private message', conversation: conversation) }
+      let(:mail) { described_class.reply_without_summary(message_1.conversation, Time.zone.now - 1.minute).deliver_now }
+
+      before do
+        message_2.save
+      end
+
+      it 'renders the subject' do
+        expect(mail.subject).to eq("[##{message_2.conversation.display_id}] New messages on this conversation")
+      end
+
+      it 'not have private notes' do
+        # make the message private
+        private_message.private = true
+        private_message.save
+
+        expect(mail.body.decoded).not_to include(private_message.content)
+      end
+
+      it 'onlies have the messages sent by the agent' do
+        expect(mail.body.decoded).not_to include(message_1.content)
+        expect(mail.body.decoded).to include(message_2.content)
+      end
+    end
+
     context 'when custom domain and email is not enabled' do
       let(:inbox) { create(:inbox, account: account) }
       let(:inbox_member) { create(:inbox_member, user: agent, inbox: inbox) }
@@ -57,7 +86,7 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
       end
     end
 
-    context 'when the cutsom domain emails are enabled' do
+    context 'when the custom domain emails are enabled' do
       let(:conversation) { create(:conversation, assignee: agent) }
       let(:message) { create(:message, conversation: conversation) }
       let(:mail) { described_class.reply_with_summary(message.conversation, Time.zone.now).deliver_now }

spec/workers/conversation_reply_email_worker_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe ConversationReplyEmailWorker, type: :worker do
   let(:perform_at) { (Time.zone.today + 6.hours).to_datetime }
   let(:scheduled_job) { described_class.perform_at(perform_at, 1, Time.zone.now) }
   let(:conversation) { build(:conversation, display_id: nil) }
+  let(:message) { build(:message, conversation: conversation, content_type: 'incoming_email', inbox: conversation.inbox) }
 
   describe 'testing ConversationSummaryEmailWorker' do
     before do
@@ -12,6 +13,7 @@ RSpec.describe ConversationReplyEmailWorker, type: :worker do
       allow(Conversation).to receive(:find).and_return(conversation)
       mailer = double
       allow(ConversationReplyMailer).to receive(:reply_with_summary).and_return(mailer)
+      allow(ConversationReplyMailer).to receive(:reply_without_summary).and_return(mailer)
       allow(mailer).to receive(:deliver_later).and_return(true)
     end
 
@@ -28,10 +30,16 @@ RSpec.describe ConversationReplyEmailWorker, type: :worker do
     end
 
     context 'with actions performed by the worker' do
-      it 'calls ConversationSummaryMailer' do
+      it 'calls ConversationSummaryMailer#reply_with_summary when last incoming message was not email' do
         described_class.new.perform(1, Time.zone.now)
         expect(ConversationReplyMailer).to have_received(:reply_with_summary)
       end
+
+      it 'calls ConversationSummaryMailer#reply_without_summary when last incoming message was from email' do
+        message.save
+        described_class.new.perform(1, Time.zone.now)
+        expect(ConversationReplyMailer).to have_received(:reply_without_summary)
+      end
     end
   end
 end