diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 742e61c02..91fbd7a49 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/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 - "" + "" end def in_reply_to_email diff --git a/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb b/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb new file mode 100644 index 000000000..8d7d73978 --- /dev/null +++ b/app/views/mailers/conversation_reply_mailer/reply_without_summary.html.erb @@ -0,0 +1,12 @@ +<% @messages.each do |message| %> +

+ <% if message.content %> + <%= message.content %> + <% end %> + <% if message.attachments %> + <% message.attachments.each do |attachment| %> + attachment [click here to view] + <% end %> + <% end %> +

+<% end %> diff --git a/app/workers/conversation_reply_email_worker.rb b/app/workers/conversation_reply_email_worker.rb index e63d8fafb..da6e18fe0 100644 --- a/app/workers/conversation_reply_email_worker.rb +++ b/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) diff --git a/spec/factories/messages.rb b/spec/factories/messages.rb index 236095a0a..264e6cae9 100644 --- a/spec/factories/messages.rb +++ b/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' } diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 61c3c8f98..2fb6f9f62 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/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 } diff --git a/spec/workers/conversation_reply_email_worker_spec.rb b/spec/workers/conversation_reply_email_worker_spec.rb index 0ebcb58d3..11be19b3f 100644 --- a/spec/workers/conversation_reply_email_worker_spec.rb +++ b/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