Skip to content

Commit

Permalink
Enhancement: ability to send out an email without summary #1048 (#1049)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sony-mathew committed Jul 15, 2020
1 parent a8ce9ae commit d5a6f5e
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 9 deletions.
26 changes: 23 additions & 3 deletions app/mailers/conversation_reply_mailer.rb
Expand Up @@ -20,15 +20,35 @@ def reply_with_summary(conversation, message_queued_time)
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
})
end

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
Expand All @@ -50,7 +70,7 @@ def from_email
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
Expand Down
@@ -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 %>
6 changes: 5 additions & 1 deletion app/workers/conversation_reply_email_worker.rb
Expand Up @@ -6,7 +6,11 @@ def perform(conversation_id, queued_time)
@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)
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/messages.rb
Expand Up @@ -2,7 +2,7 @@

FactoryBot.define do
factory :message do
content { 'Message' }
content { 'Incoming Message' }
status { 'sent' }
message_type { 'incoming' }
content_type { 'text' }
Expand Down
35 changes: 32 additions & 3 deletions spec/mailers/conversation_reply_mailer_spec.rb
Expand Up @@ -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 }
Expand All @@ -13,7 +13,7 @@
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) }
Expand All @@ -33,6 +33,35 @@
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) }
Expand All @@ -57,7 +86,7 @@
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 }
Expand Down
10 changes: 9 additions & 1 deletion spec/workers/conversation_reply_email_worker_spec.rb
Expand Up @@ -5,13 +5,15 @@
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
conversation.save!
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

Expand All @@ -28,10 +30,16 @@
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

0 comments on commit d5a6f5e

Please sign in to comment.