fix(email): resolve reply recipients from the message being sent (#15194)
When an agent loops a new address into an ongoing email thread and then adds a private note, the reply currently goes out with no Cc, and with To falling back to the conversation contact. The newly added person never receives the mail. This change makes an outgoing email keep the To/Cc/Bcc that were entered on that reply, no matter what is added to the conversation afterwards. Closes #15193 ## How to reproduce 1. Open a conversation on an Email inbox. 2. Add a new address to **Cc** and send a reply. 3. Immediately add a private note to the same conversation. 4. Inspect the delivered mail: `Cc` is empty and `To` is the conversation contact instead of the addresses entered on the reply. ## What changed `ConversationReplyMailer#cc_bcc_emails` and `#to_emails_from_content_attributes` read the addresses from `@conversation.messages.outgoing.last` rather than from the message they were handed. Replies are delivered asynchronously (`SendReplyJob.perform_later`, and `wait: 2.seconds` when the message has attachments), so any outgoing message created in that window replaces the recipients of a mail that is already queued. A private note is the easiest way to hit it: private notes are outgoing messages, and on an Email inbox `Messages::MessageBuilder#process_emails` stores them with empty `to_emails` / `cc_emails` / `bcc_emails`. Both lookups now go through the existing `current_message` helper (`@message || @conversation.messages.outgoing.last`), which the `from` and `reply_to` builders already use. `email_reply` sets `@message`, so it resolves the recipients of the message being delivered; `reply_with_summary` and `reply_without_summary` leave `@message` nil and keep their current behaviour. Added a spec covering the private-note case in `spec/mailers/conversation_reply_mailer_spec.rb`. ## Note I could not run the Ruby test suite locally — there is no Ruby toolchain in the environment I worked in, so the new spec has not been executed on my side. The change and the spec were verified by reading the code paths (`Message#send_reply`, `Email::SendOnEmailService`, `Messages::MessageBuilder#process_emails`, `ConversationReplyMailerHelper#prepare_mail`). Deferring to CI for the actual run.
This commit is contained in:
@@ -174,7 +174,7 @@ class ConversationReplyMailer < ApplicationMailer
|
||||
end
|
||||
|
||||
def cc_bcc_emails
|
||||
content_attributes = @conversation.messages.outgoing.last&.content_attributes
|
||||
content_attributes = current_message&.content_attributes
|
||||
|
||||
return [] unless content_attributes
|
||||
return [] unless content_attributes[:cc_emails] || content_attributes[:bcc_emails]
|
||||
@@ -183,7 +183,7 @@ class ConversationReplyMailer < ApplicationMailer
|
||||
end
|
||||
|
||||
def to_emails_from_content_attributes
|
||||
content_attributes = @conversation.messages.outgoing.last&.content_attributes
|
||||
content_attributes = current_message&.content_attributes
|
||||
|
||||
return [] unless content_attributes
|
||||
return [] unless content_attributes[:to_emails]
|
||||
|
||||
@@ -342,6 +342,25 @@ RSpec.describe ConversationReplyMailer do
|
||||
expect(mail.message_id).to eq("conversation/#{conversation.uuid}/messages/#{message.id}@#{conversation.account.domain}")
|
||||
end
|
||||
|
||||
context 'when a newer outgoing message exists in the conversation' do
|
||||
let!(:message) do
|
||||
create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Looping in the vendor',
|
||||
content_attributes: { to_emails: ['customer@example.com'], cc_emails: ['vendor@example.com'],
|
||||
bcc_emails: ['audit@example.com'] })
|
||||
end
|
||||
|
||||
it 'sends to the recipients of the message being delivered' do
|
||||
# a private note added right after the reply carries empty recipient lists
|
||||
create(:message, conversation: conversation, account: account, message_type: 'outgoing', private: true,
|
||||
content: 'Vendor has been looped in',
|
||||
content_attributes: { to_emails: [], cc_emails: [], bcc_emails: [] })
|
||||
|
||||
expect(mail.to).to eq(message.content_attributes[:to_emails])
|
||||
expect(mail.cc).to eq(message.content_attributes[:cc_emails])
|
||||
expect(mail.bcc).to eq(message.content_attributes[:bcc_emails])
|
||||
end
|
||||
end
|
||||
|
||||
context 'when message is a CSAT survey' do
|
||||
let(:csat_message) do
|
||||
create(:message, conversation: conversation, account: account, message_type: 'template',
|
||||
|
||||
Reference in New Issue
Block a user