From 6475a6a59329a45d87c4216031fd14aa0ebfbab7 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 29 Jul 2025 14:24:14 +0400 Subject: [PATCH] feat: add references header to reply emails (#11719) Co-authored-by: Muhsin Keloth Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/mailers/conversation_reply_mailer.rb | 6 + .../conversation_reply_mailer_helper.rb | 25 ++- app/mailers/references_header_builder.rb | 101 +++++++++++ app/presenters/mail_presenter.rb | 7 + spec/fixtures/files/mail_with_references.eml | 17 ++ spec/mailboxes/reply_mailbox_spec.rb | 2 +- spec/mailboxes/support_mailbox_spec.rb | 25 ++- .../mailers/conversation_reply_mailer_spec.rb | 93 ++++++++++ .../mailers/references_header_builder_spec.rb | 164 ++++++++++++++++++ spec/presenters/mail_presenter_spec.rb | 26 +++ 10 files changed, 449 insertions(+), 17 deletions(-) create mode 100644 app/mailers/references_header_builder.rb create mode 100644 spec/fixtures/files/mail_with_references.eml create mode 100644 spec/mailers/references_header_builder_spec.rb diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index ba46a5fed..360b227cb 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -4,6 +4,7 @@ class ConversationReplyMailer < ApplicationMailer attr_reader :large_attachments include ConversationReplyMailerHelper + include ReferencesHeaderBuilder default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot ') layout :choose_layout @@ -160,6 +161,7 @@ class ConversationReplyMailer < ApplicationMailer end def conversation_reply_email_id + # Find the last incoming message's message_id to reply to content_attributes = @conversation.messages.incoming.last&.content_attributes if content_attributes && content_attributes['email'] && content_attributes['email']['message_id'] @@ -169,6 +171,10 @@ class ConversationReplyMailer < ApplicationMailer nil end + def references_header + build_references_header(@conversation, in_reply_to_email) + end + def cc_bcc_emails content_attributes = @conversation.messages.outgoing.last&.content_attributes diff --git a/app/mailers/conversation_reply_mailer_helper.rb b/app/mailers/conversation_reply_mailer_helper.rb index d34369832..55f7fe12b 100644 --- a/app/mailers/conversation_reply_mailer_helper.rb +++ b/app/mailers/conversation_reply_mailer_helper.rb @@ -6,15 +6,15 @@ module ConversationReplyMailerHelper reply_to: email_reply_to, subject: mail_subject, message_id: custom_message_id, - in_reply_to: in_reply_to_email + in_reply_to: in_reply_to_email, + references: references_header } if cc_bcc_enabled @options[:cc] = cc_bcc_emails[0] @options[:bcc] = cc_bcc_emails[1] end - ms_smtp_settings - google_smtp_settings + oauth_smtp_settings set_delivery_method # Email type detection logic: @@ -57,22 +57,17 @@ module ConversationReplyMailerHelper private - def google_smtp_settings - return unless @inbox.email? && @channel.imap_enabled && @inbox.channel.google? - - smtp_settings = base_smtp_settings('smtp.gmail.com') + def oauth_smtp_settings + return unless @inbox.email? && @channel.imap_enabled + return unless oauth_provider_domain @options[:delivery_method] = :smtp - @options[:delivery_method_options] = smtp_settings + @options[:delivery_method_options] = base_smtp_settings(oauth_provider_domain) end - def ms_smtp_settings - return unless @inbox.email? && @channel.imap_enabled && @inbox.channel.microsoft? - - smtp_settings = base_smtp_settings('smtp.office365.com') - - @options[:delivery_method] = :smtp - @options[:delivery_method_options] = smtp_settings + def oauth_provider_domain + return 'smtp.gmail.com' if @inbox.channel.google? + return 'smtp.office365.com' if @inbox.channel.microsoft? end def base_smtp_settings(domain) diff --git a/app/mailers/references_header_builder.rb b/app/mailers/references_header_builder.rb new file mode 100644 index 000000000..a48d7c9c1 --- /dev/null +++ b/app/mailers/references_header_builder.rb @@ -0,0 +1,101 @@ +# Builds RFC 5322 compliant References headers for email threading +# +# This module provides functionality to construct proper References headers +# that maintain email conversation threading according to RFC 5322 standards. +module ReferencesHeaderBuilder + # Builds a complete References header for an email reply + # + # According to RFC 5322, the References header should contain: + # - References from the message being replied to (if available) + # - The In-Reply-To message ID as the final element + # - Proper line folding if the header exceeds 998 characters + # + # If the message being replied to has no stored References, we use a minimal + # approach with only the In-Reply-To message ID rather than rebuilding. + # + # @param conversation [Conversation] The conversation containing the message thread + # @param in_reply_to_message_id [String] The message ID being replied to + # @return [String] A properly formatted and folded References header value + def build_references_header(conversation, in_reply_to_message_id) + references = get_references_from_replied_message(conversation, in_reply_to_message_id) + references << in_reply_to_message_id + + references = references.compact.uniq + fold_references_header(references) + rescue StandardError => e + Rails.logger.error("Error building references header for ##{conversation.id}: #{e.message}") + ChatwootExceptionTracker.new(e, account: conversation.account).capture_exception + '' + end + + private + + # Gets References header from the message being replied to + # + # Finds the message by its source_id matching the in_reply_to_message_id + # and extracts its stored References header. If no References are found, + # we return an empty array (minimal approach - no rebuilding). + # + # @param conversation [Conversation] The conversation containing the message thread + # @param in_reply_to_message_id [String] The message ID being replied to + # @return [Array] Array of properly formatted message IDs with angle brackets + def get_references_from_replied_message(conversation, in_reply_to_message_id) + return [] if in_reply_to_message_id.blank? + + replied_to_message = find_replied_to_message(conversation, in_reply_to_message_id) + return [] unless replied_to_message + + extract_references_from_message(replied_to_message) + end + + # Finds the message being replied to based on its source_id + # + # @param conversation [Conversation] The conversation containing the message thread + # @param in_reply_to_message_id [String] The message ID to search for + # @return [Message, nil] The message being replied to + def find_replied_to_message(conversation, in_reply_to_message_id) + return nil if in_reply_to_message_id.blank? + + # Remove angle brackets if present for comparison + normalized_id = in_reply_to_message_id.gsub(/[<>]/, '') + + # Use database query to find the message efficiently + # Search for exact match or with angle brackets + conversation.messages + .where.not(source_id: nil) + .where('source_id = ? OR source_id = ? OR source_id = ?', + normalized_id, + "<#{normalized_id}>", + in_reply_to_message_id) + .first + end + + # Extracts References header from a message's content_attributes + # + # @param message [Message] The message to extract References from + # @return [Array] Array of properly formatted message IDs with angle brackets + def extract_references_from_message(message) + return [] unless message.content_attributes&.dig('email', 'references') + + references = message.content_attributes['email']['references'] + Array.wrap(references).map do |ref| + ref.start_with?('<') ? ref : "<#{ref}>" + end + end + + # Folds References header to comply with RFC 5322 line folding requirements + # + # RFC 5322 requires that continuation lines in folded headers start with + # whitespace (space or tab). This method joins message IDs with CRLF + space, + # ensuring the first line has no leading space and all continuation lines + # start with a space as required by the RFC. + # + # @param references_array [Array] Array of message IDs to be folded + # @return [String] A properly folded header value with CRLF line endings + def fold_references_header(references_array) + return '' if references_array.empty? + return references_array.first if references_array.size == 1 + + references_array.join("\r\n ") + end +end diff --git a/app/presenters/mail_presenter.rb b/app/presenters/mail_presenter.rb index 1c951cbe3..890e97a78 100644 --- a/app/presenters/mail_presenter.rb +++ b/app/presenters/mail_presenter.rb @@ -100,6 +100,7 @@ class MailPresenter < SimpleDelegator message_id: message_id, multipart: multipart?, number_of_attachments: number_of_attachments, + references: references, subject: subject, text_content: text_content, to: to @@ -115,6 +116,12 @@ class MailPresenter < SimpleDelegator @mail.in_reply_to.is_a?(Array) ? @mail.in_reply_to.first : @mail.in_reply_to end + def references + return [] if @mail.references.blank? + + Array.wrap(@mail.references) + end + def from # changing to downcase to avoid case mismatch while finding contact (@mail.reply_to.presence || @mail.from).map(&:downcase) diff --git a/spec/fixtures/files/mail_with_references.eml b/spec/fixtures/files/mail_with_references.eml new file mode 100644 index 000000000..4fca32726 --- /dev/null +++ b/spec/fixtures/files/mail_with_references.eml @@ -0,0 +1,17 @@ +From: Sony Mathew +To: care@example.com +Mime-Version: 1.0 (Apple Message framework v1244.3) +Content-Type: multipart/alternative; boundary="Apple-Mail=_33A037C7-4BB3-4772-AE52-FCF2D7535F74" +Subject: Discussion: Let's debate these attachments +Date: Tue, 20 Apr 2020 04:20:20 -0400 +In-Reply-To: <4e6e35f5a38b4_479f13bb90078178@small-app-01.mail> +References: <4e6e35f5a38b4_479f13bb90078178@small-app-01.mail> +Message-Id: <0CB459E0-0336-41DA-BC88-E6E28C697DDBF@chatwoot.com> +X-Mailer: Apple Mail (2.1244.3) + +--Apple-Mail=_33A037C7-4BB3-4772-AE52-FCF2D7535F74 +Content-Transfer-Encoding: quoted-printable +Content-Type: text/plain; + charset=utf-8 + +Email with references header \ No newline at end of file diff --git a/spec/mailboxes/reply_mailbox_spec.rb b/spec/mailboxes/reply_mailbox_spec.rb index a9a802bb9..2320e3e07 100644 --- a/spec/mailboxes/reply_mailbox_spec.rb +++ b/spec/mailboxes/reply_mailbox_spec.rb @@ -12,7 +12,7 @@ RSpec.describe ReplyMailbox do let(:conversation) { create(:conversation, assignee: agent, inbox: create(:inbox, account: account, greeting_enabled: false), account: account) } let(:described_subject) { described_class.receive reply_mail } let(:serialized_attributes) do - %w[bcc cc content_type date from html_content in_reply_to message_id multipart number_of_attachments subject text_content to] + %w[bcc cc content_type date from html_content in_reply_to message_id multipart number_of_attachments references subject text_content to] end context 'with reply uuid present' do diff --git a/spec/mailboxes/support_mailbox_spec.rb b/spec/mailboxes/support_mailbox_spec.rb index f9e9aa2ff..f75e3ef80 100644 --- a/spec/mailboxes/support_mailbox_spec.rb +++ b/spec/mailboxes/support_mailbox_spec.rb @@ -55,7 +55,7 @@ RSpec.describe SupportMailbox do let(:support_in_reply_to_mail) { create_inbound_email_from_fixture('support_in_reply_to.eml') } let(:described_subject) { described_class.receive support_mail } let(:serialized_attributes) do - %w[bcc cc content_type date from html_content in_reply_to message_id multipart number_of_attachments subject + %w[bcc cc content_type date from html_content in_reply_to message_id multipart number_of_attachments references subject text_content to] end let(:conversation) { Conversation.where(inbox_id: channel_email.inbox).last } @@ -111,6 +111,29 @@ RSpec.describe SupportMailbox do end end + describe 'email with references header' do + let(:mail_with_references) { create_inbound_email_from_fixture('mail_with_references.eml') } + let(:described_subject) { described_class.receive mail_with_references } + + before do + # reuse the existing channel_email that's already set to 'care@example.com' + described_subject + end + + it 'includes references in the message content_attributes' do + message = conversation.messages.last + email_attributes = message.content_attributes['email'] + + expect(email_attributes['references']).to be_present + expect(email_attributes['references']).to eq(['4e6e35f5a38b4_479f13bb90078178@small-app-01.mail', 'test-reference-id']) + end + + it 'includes references in serialized email attributes' do + message = conversation.messages.last + expect(message.content_attributes['email'].keys).to include('references') + end + end + describe 'Sender without name' do let(:support_mail_without_sender_name) { create_inbound_email_from_fixture('support_without_sender_name.eml') } let(:described_subject) { described_class.receive support_mail_without_sender_name } diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb index 2a0d6c8b0..ecd97333e 100644 --- a/spec/mailers/conversation_reply_mailer_spec.rb +++ b/spec/mailers/conversation_reply_mailer_spec.rb @@ -137,6 +137,99 @@ RSpec.describe ConversationReplyMailer do end end + context 'with references header' do + let(:conversation) { create(:conversation, assignee: agent, inbox: email_channel.inbox, account: account).reload } + let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } + let(:mail) { described_class.email_reply(message).deliver_now } + + context 'when starting a new conversation' do + let(:first_outgoing_message) do + create(:message, + conversation: conversation, + account: account, + message_type: 'outgoing', + content: 'First outgoing message') + end + let(:mail) { described_class.email_reply(first_outgoing_message).deliver_now } + + it 'has only the conversation reference' do + # When starting a conversation, references will have the default conversation ID + # Extract domain from the actual references header to handle dynamic domain selection + actual_domain = mail.references.split('@').last + expected_reference = "account/#{account.id}/conversation/#{conversation.uuid}@#{actual_domain}" + expect(mail.references).to eq(expected_reference) + end + end + + context 'when replying to a message with no references' do + let(:incoming_message) do + create(:message, + conversation: conversation, + account: account, + message_type: 'incoming', + source_id: '', + content: 'Incoming message', + content_attributes: { + 'email' => { + 'message_id' => 'incoming-123@example.com' + } + }) + end + let(:reply_message) do + create(:message, + conversation: conversation, + account: account, + message_type: 'outgoing', + content: 'Reply to incoming') + end + let(:mail) { described_class.email_reply(reply_message).deliver_now } + + before do + incoming_message + end + + it 'includes only the in_reply_to id in references' do + # References should only have the incoming message ID when no prior references exist + expect(mail.references).to eq('incoming-123@example.com') + end + end + + context 'when replying to a message that has references' do + let(:incoming_message_with_refs) do + create(:message, + conversation: conversation, + account: account, + message_type: 'incoming', + source_id: '', + content: 'Incoming with references', + content_attributes: { + 'email' => { + 'message_id' => 'incoming-456@example.com', + 'references' => ['', ''] + } + }) + end + let(:reply_message) do + create(:message, + conversation: conversation, + account: account, + message_type: 'outgoing', + content: 'Reply to message with refs') + end + let(:mail) { described_class.email_reply(reply_message).deliver_now } + + before do + incoming_message_with_refs + end + + it 'includes existing references plus the in_reply_to id' do + # Rails returns references as an array when multiple values are present + expected_references = ['ref-1@example.com', 'ref-2@example.com', 'incoming-456@example.com'] + expect(mail.references).to eq(expected_references) + end + end + end + context 'with email reply' do let(:conversation) { create(:conversation, assignee: agent, inbox: email_channel.inbox, account: account).reload } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } diff --git a/spec/mailers/references_header_builder_spec.rb b/spec/mailers/references_header_builder_spec.rb new file mode 100644 index 000000000..b10d63dba --- /dev/null +++ b/spec/mailers/references_header_builder_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ReferencesHeaderBuilder do + include described_class + + let(:account) { create(:account) } + let(:email_channel) { create(:channel_email, account: account) } + let(:inbox) { create(:inbox, channel: email_channel, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + + describe '#build_references_header' do + let(:in_reply_to_message_id) { '' } + + context 'when no message is found with the in_reply_to_message_id' do + it 'returns only the in_reply_to message ID' do + result = build_references_header(conversation, in_reply_to_message_id) + expect(result).to eq('') + end + end + + context 'when a message is found with matching source_id' do + context 'with stored References' do + let(:original_message) do + create(:message, conversation: conversation, account: account, + source_id: '', + content_attributes: { + 'email' => { + 'references' => ['', ''] + } + }) + end + + before do + original_message + end + + it 'includes stored References plus in_reply_to' do + result = build_references_header(conversation, in_reply_to_message_id) + expect(result).to eq("\r\n \r\n ") + end + + it 'removes duplicates while preserving order' do + # If in_reply_to is already in the References, it should appear only once at the end + original_message.content_attributes['email']['references'] = ['', ''] + original_message.save! + + result = build_references_header(conversation, in_reply_to_message_id) + message_ids = result.split("\r\n ").map(&:strip) + expect(message_ids).to eq(['', '']) + end + end + + context 'without stored References' do + let(:original_message) do + create(:message, conversation: conversation, account: account, + source_id: 'reply-to-123@example.com', # without angle brackets + content_attributes: { 'email' => {} }) + end + + before do + original_message + end + + it 'returns only the in_reply_to message ID (no rebuild)' do + result = build_references_header(conversation, in_reply_to_message_id) + expect(result).to eq('') + end + end + end + + context 'with folding multiple References' do + let(:original_message) do + create(:message, conversation: conversation, account: account, + source_id: '', + content_attributes: { + 'email' => { + 'references' => ['', '', ''] + } + }) + end + + before do + original_message + end + + it 'folds the header with CRLF between message IDs' do + result = build_references_header(conversation, in_reply_to_message_id) + + expect(result).to include("\r\n") + lines = result.split("\r\n") + + # First line has no leading space, continuation lines do + expect(lines.first).not_to start_with(' ') + expect(lines[1..]).to all(start_with(' ')) + end + end + + context 'with source_id in different formats' do + it 'finds message with source_id without angle brackets' do + create(:message, conversation: conversation, account: account, + source_id: 'test-123@example.com', + content_attributes: { + 'email' => { + 'references' => [''] + } + }) + + result = build_references_header(conversation, '') + expect(result).to eq("\r\n ") + end + + it 'finds message with source_id with angle brackets' do + create(:message, conversation: conversation, account: account, + source_id: '', + content_attributes: { + 'email' => { + 'references' => [''] + } + }) + + result = build_references_header(conversation, 'test-456@example.com') + expect(result).to eq("\r\n test-456@example.com") + end + end + end + + describe '#fold_references_header' do + it 'returns single message ID without folding' do + single_array = [''] + result = fold_references_header(single_array) + + expect(result).to eq('') + expect(result).not_to include("\r\n") + end + + it 'folds multiple message IDs with CRLF + space' do + multiple_array = ['', '', ''] + result = fold_references_header(multiple_array) + + expect(result).to eq("\r\n \r\n ") + end + + it 'ensures RFC 5322 compliance with continuation line spacing' do + multiple_array = ['', ''] + result = fold_references_header(multiple_array) + lines = result.split("\r\n") + + # First line has no leading space (not a continuation line) + expect(lines.first).to eq('') + expect(lines.first).not_to start_with(' ') + + # Second line starts with space (continuation line) + expect(lines[1]).to eq(' ') + expect(lines[1]).to start_with(' ') + end + + it 'handles empty array' do + result = fold_references_header([]) + expect(result).to eq('') + end + end +end diff --git a/spec/presenters/mail_presenter_spec.rb b/spec/presenters/mail_presenter_spec.rb index d5b313ce9..af6768e41 100644 --- a/spec/presenters/mail_presenter_spec.rb +++ b/spec/presenters/mail_presenter_spec.rb @@ -46,6 +46,7 @@ RSpec.describe MailPresenter do :message_id, :multipart, :number_of_attachments, + :references, :subject, :text_content, :to @@ -100,6 +101,31 @@ RSpec.describe MailPresenter do end end + describe '#references' do + let(:references_mail) { create_inbound_email_from_fixture('references.eml').mail } + let(:mail_presenter_with_references) { described_class.new(references_mail) } + + context 'when mail has references' do + it 'returns an array of reference IDs' do + expect(mail_presenter_with_references.references).to eq(['4e6e35f5a38b4_479f13bb90078178@small-app-01.mail', 'test-reference-id']) + end + end + + context 'when mail has no references' do + it 'returns an empty array' do + mail_presenter = described_class.new(mail_without_in_reply_to) + expect(mail_presenter.references).to eq([]) + end + end + + context 'when references are included in serialized_data' do + it 'includes references in the serialized data' do + data = mail_presenter_with_references.serialized_data + expect(data[:references]).to eq(['4e6e35f5a38b4_479f13bb90078178@small-app-01.mail', 'test-reference-id']) + end + end + end + describe 'auto_reply?' do let(:auto_reply_mail) { create_inbound_email_from_fixture('auto_reply.eml').mail } let(:auto_reply_with_auto_submitted_mail) { create_inbound_email_from_fixture('auto_reply_with_auto_submitted.eml').mail }