From ecb7a44f070ef0f22b49441ccc28be2736e467e8 Mon Sep 17 00:00:00 2001 From: Vaibhav Dewangan Date: Fri, 31 Jul 2026 06:23:11 +0530 Subject: [PATCH] fix(email): resolve reply recipients from the message being sent (#15194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/mailers/conversation_reply_mailer.rb | 4 ++-- .../mailers/conversation_reply_mailer_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 22b176421..f936dc98d 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -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] diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 576ca882a..6df5e1560 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -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',