diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index b118df107..139be8d2b 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -79,8 +79,12 @@ class ConversationReplyMailer < ApplicationMailer @conversation.messages.chat.where.not(message_type: :incoming)&.last end - def assignee_name - @assignee_name ||= @agent&.available_name || 'Notifications' + def sender_name + @sender_name ||= current_message&.sender&.name || @agent&.available_name || 'Notifications' + end + + def current_message + @message || @messages.last end def mail_subject @@ -97,7 +101,7 @@ class ConversationReplyMailer < ApplicationMailer def reply_email if should_use_conversation_email_address? - I18n.t('conversations.reply.email.header.reply_with_name', assignee_name: assignee_name, inbox_name: @inbox.name, + I18n.t('conversations.reply.email.header.reply_with_name', assignee_name: sender_name, inbox_name: @inbox.name, reply_email: "#{@conversation.uuid}@#{@account.inbound_email_domain}") else @inbox.email_address || @agent&.email @@ -106,21 +110,17 @@ class ConversationReplyMailer < ApplicationMailer def from_email_with_name if should_use_conversation_email_address? - I18n.t('conversations.reply.email.header.from_with_name', assignee_name: assignee_name, inbox_name: @inbox.name, + I18n.t('conversations.reply.email.header.from_with_name', assignee_name: sender_name, inbox_name: @inbox.name, from_email: parse_email(@account.support_email)) else - I18n.t('conversations.reply.email.header.from_with_name', assignee_name: assignee_name, inbox_name: @inbox.name, + I18n.t('conversations.reply.email.header.from_with_name', assignee_name: sender_name, inbox_name: @inbox.name, from_email: parse_email(inbox_from_email_address)) end end def channel_email_with_name - if @conversation.assignee.present? - I18n.t('conversations.reply.channel_email.header.reply_with_name', assignee_name: assignee_name, inbox_name: @inbox.name, - from_email: @channel.email) - else - I18n.t('conversations.reply.channel_email.header.reply_with_inbox_name', inbox_name: @inbox.name, from_email: @channel.email) - end + I18n.t('conversations.reply.channel_email.header.reply_with_name', assignee_name: sender_name, inbox_name: @inbox.name, + from_email: @channel.email) end def parse_email(email_string) diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index c10dfe2d5..8e65e9396 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -86,7 +86,7 @@ RSpec.describe ConversationReplyMailer do let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now } it 'has correct name' do - expect(mail[:from].display_names).to eq(['Notifications from Inbox']) + expect(mail[:from].display_names).to eq(["#{message.sender.name} from Inbox"]) end end @@ -169,15 +169,29 @@ RSpec.describe ConversationReplyMailer do expect(mail.delivery_method.settings[:port]).to eq 587 end - it 'renders assignee name in the from address' do + it 'renders sender name in the from address' do + mail = described_class.email_reply(message) + expect(mail['from'].value).to eq "#{message.sender.name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + end + + it 'renders sender name even when assignee is not present' do + conversation.update(assignee_id: nil) + mail = described_class.email_reply(message) + expect(mail['from'].value).to eq "#{message.sender.name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + end + + it 'renders assignee name in the from address when sender_name not available' do + message.update(sender_id: nil) mail = described_class.email_reply(message) expect(mail['from'].value).to eq "#{conversation.assignee.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" end - it 'renders inbox name in the from address' do - conversation.update(assignee: nil) + it 'renders inbox name as sender and assignee not present' do + message.update(sender_id: nil) + conversation.update(assignee_id: nil) + mail = described_class.email_reply(message) - expect(mail['from'].value).to eq "#{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" + expect(mail['from'].value).to eq "Notifications from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" end end @@ -260,13 +274,13 @@ RSpec.describe ConversationReplyMailer do it 'sets reply to email to be based on the domain' do reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}" - reply_to = "#{agent.available_name} from #{conversation.inbox.name} <#{reply_to_email}>" + reply_to = "#{message.sender.name} from #{conversation.inbox.name} <#{reply_to_email}>" expect(mail['REPLY-TO'].value).to eq(reply_to) expect(mail.reply_to).to eq([reply_to_email]) end it 'sets the from email to be the support email' do - expect(mail['FROM'].value).to eq("#{agent.available_name} from Inbox <#{conversation.account.support_email}>") + expect(mail['FROM'].value).to eq("#{conversation.messages.last.sender.name} from Inbox <#{conversation.account.support_email}>") expect(mail.from).to eq([conversation.account.support_email]) end