diff --git a/app/builders/messages/forwarded_message_builder.rb b/app/builders/messages/forwarded_message_builder.rb index cb45824ca..c1afc976e 100644 --- a/app/builders/messages/forwarded_message_builder.rb +++ b/app/builders/messages/forwarded_message_builder.rb @@ -42,9 +42,16 @@ class Messages::ForwardedMessageBuilder 'full' => full_content }) + # Copy over any attachment data if present + data['attachments'] = forwarded_message.attachments.map(&:serializable_hash) if forwarded_message.attachments.present? + data end + def forwarded_attachments + forwarded_message.attachments if forwarded_message&.attachments&.present? + end + private def valid_forwarded_data? diff --git a/app/builders/messages/message_builder.rb b/app/builders/messages/message_builder.rb index c0ef6803b..227e39691 100644 --- a/app/builders/messages/message_builder.rb +++ b/app/builders/messages/message_builder.rb @@ -16,6 +16,7 @@ class Messages::MessageBuilder process_forwarded_message if @forwarded_message_id.present? @message = @conversation.messages.build(message_params) process_attachments + process_forwarded_attachments if @forwarded_message_id.present? process_emails @message.save! @message @@ -26,6 +27,7 @@ class Messages::MessageBuilder def process_forwarded_message builder = Messages::ForwardedMessageBuilder.new(@forwarded_message_id) @forwarded_attributes = builder.perform + @forwarded_message_attachments = builder.forwarded_attachments # Update content to include forwarded message original_content = @params[:content_original] || @params[:content] @@ -38,6 +40,28 @@ class Messages::MessageBuilder @forwarded_attributes[:content_attributes][:email] = builder.forwarded_email_data(original_content) end + # Process attachments from the forwarded message + def process_forwarded_attachments + return if @forwarded_message_attachments.blank? + + @forwarded_message_attachments.each do |source_attachment| + # Create a new attachment for the current message + attachment = @message.attachments.build( + account_id: @message.account_id, + file_type: source_attachment.file_type + ) + + # Attach the file by directly copying it from the source attachment + next unless source_attachment.file.attached? + + attachment.file.attach( + io: StringIO.new(source_attachment.file.download), + filename: source_attachment.file.filename.to_s, + content_type: source_attachment.file.content_type + ) + end + end + # Extracts content attributes from the given params. # - Converts ActionController::Parameters to a regular hash if needed. # - Attempts to parse a JSON string if content is a string. diff --git a/app/javascript/dashboard/components-next/message/bubbles/Email/EmailMeta.vue b/app/javascript/dashboard/components-next/message/bubbles/Email/EmailMeta.vue index 0c0f04a7e..2f1d014b8 100644 --- a/app/javascript/dashboard/components-next/message/bubbles/Email/EmailMeta.vue +++ b/app/javascript/dashboard/components-next/message/bubbles/Email/EmailMeta.vue @@ -69,10 +69,7 @@ const showMeta = computed(() => { v-show="showMeta" :class="hasError ? 'text-n-ruby-11' : 'text-n-slate-11'" > -
+
{ data-bubble-name="email" >
- +
['sender@example.com'], + 'to' => ['recipient@example.com'], + 'subject' => 'Test Subject', + 'date' => '2025-04-29T14:29:07+05:30', + 'html_content' => { + 'full' => '
HTML content
', + 'quoted' => 'HTML content', + 'reply' => 'HTML content' + }, + 'text_content' => { + 'full' => 'Text content', + 'quoted' => 'Text content', + 'reply' => 'Text content' + } + } + end + # Create a message with email data let(:forwarded_message) do + create(:message, conversation: conversation, account: account, + content_attributes: { email: base_email_content }) + end + + # Create a message with multipart email data + let(:multipart_message) do create(:message, conversation: conversation, account: account, content_attributes: { - email: { - 'from' => ['sender@example.com'], - 'to' => ['recipient@example.com'], - 'subject' => 'Test Subject', - 'date' => '2025-04-29T14:29:07+05:30', - 'html_content' => { - 'full' => '
HTML content
', - 'quoted' => 'HTML content', - 'reply' => 'HTML content' - }, - 'text_content' => { - 'full' => 'Text content', - 'quoted' => 'Text content', - 'reply' => 'Text content' - } - } + email: base_email_content.merge( + 'html_content' => { 'full' => '
HTML formatted content
' }, + 'text_content' => { 'full' => 'Plain text content' } + ) }) end + # Create a message with email data and attachments + let(:message_with_attachments) do + message = create(:message, conversation: conversation, account: account, + content_attributes: { + email: base_email_content.merge( + 'html_content' => { 'full' => '
Message with attachments
' }, + 'text_content' => { 'full' => 'Message with attachments' } + ) + }) + + # Add attachments to the message + attachment1 = message.attachments.new(account_id: account.id, file_type: 'file') + attachment1.file.attach(io: StringIO.new('test file content'), filename: 'test.txt', content_type: 'text/plain') + attachment1.save! + + attachment2 = message.attachments.new(account_id: account.id, file_type: 'image') + attachment2.file.attach(io: StringIO.new('fake image content'), filename: 'test.jpg', content_type: 'image/jpeg') + attachment2.save! + + message + end + # Create a message with no email data let(:regular_message) { create(:message, conversation: conversation, account: account) } @@ -141,24 +176,6 @@ RSpec.describe Messages::ForwardedMessageBuilder do end context 'when handling multipart emails' do - let(:multipart_message) do - create(:message, conversation: conversation, account: account, - content_attributes: { - email: { - 'from' => ['sender@example.com'], - 'to' => ['recipient@example.com'], - 'subject' => 'Multipart Test', - 'date' => '2025-04-29T14:29:07+05:30', - 'html_content' => { - 'full' => '
HTML formatted content
' - }, - 'text_content' => { - 'full' => 'Plain text content' - } - } - }) - end - it 'preserves both HTML and text parts' do builder = described_class.new(multipart_message.id) result = builder.forwarded_email_data('New message') @@ -167,133 +184,155 @@ RSpec.describe Messages::ForwardedMessageBuilder do expect(result['text_content']['full']).to include('Plain text content') end end + + context 'when forwarding a message with attachments' do + it 'includes attachments in the forwarded email data' do + builder = described_class.new(message_with_attachments.id) + result = builder.forwarded_email_data('Original content') + + expect(result['attachments']).to be_present + expect(result['attachments'].length).to eq(2) + expect(result['attachments'].pluck('file_type')).to include('file', 'image') + end + end + + context 'when forwarding a message without attachments' do + it 'does not include attachments in the forwarded email data' do + builder = described_class.new(forwarded_message.id) + result = builder.forwarded_email_data('Original content') + + expect(result['attachments']).to be_nil + end + end + end + + describe '#forwarded_attachments' do + context 'with attachments' do + it 'provides access to the forwarded message attachments' do + builder = described_class.new(message_with_attachments.id) + attachments = builder.forwarded_attachments + + expect(attachments).to be_present + expect(attachments.length).to eq(2) + expect(attachments.map(&:file_type)).to include('file', 'image') + expect(attachments.first.file).to be_attached + end + end + + context 'without attachments' do + it 'returns nil when getting forwarded attachments' do + builder = described_class.new(forwarded_message.id) + attachments = builder.forwarded_attachments + + expect(attachments).to be_nil + end + end end describe '#convert_markdown_to_html' do + subject(:builder) { described_class.new(forwarded_message.id) } + it 'converts bold markdown to HTML' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:convert_markdown_to_html, '**Bold Text**') - expect(result).to eq('Bold Text') + expect(builder.send(:convert_markdown_to_html, '**Bold Text**')).to eq('Bold Text') end it 'converts italic markdown to HTML' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:convert_markdown_to_html, '*Italic Text*') - expect(result).to eq('Italic Text') + expect(builder.send(:convert_markdown_to_html, '*Italic Text*')).to eq('Italic Text') end it 'converts underscore italic markdown to HTML' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:convert_markdown_to_html, '_Italic Text_') - expect(result).to eq('Italic Text') + expect(builder.send(:convert_markdown_to_html, '_Italic Text_')).to eq('Italic Text') + end + + it 'handles multiple markdown elements' do + expect(builder.send(:convert_markdown_to_html, '**Bold** and _italic_')).to eq('Bold and italic') end it 'handles empty text' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:convert_markdown_to_html, '') - expect(result).to eq('') + expect(builder.send(:convert_markdown_to_html, '')).to eq('') end it 'handles nil text' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:convert_markdown_to_html, nil) - expect(result).to eq('') + expect(builder.send(:convert_markdown_to_html, nil)).to eq('') end end describe '#strip_markdown' do + subject(:builder) { described_class.new(forwarded_message.id) } + it 'strips bold markdown' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:strip_markdown, '**Bold Text**') - expect(result).to eq('Bold Text') + expect(builder.send(:strip_markdown, '**Bold Text**')).to eq('Bold Text') end it 'strips italic markdown' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:strip_markdown, '*Italic Text*') - expect(result).to eq('Italic Text') + expect(builder.send(:strip_markdown, '*Italic Text*')).to eq('Italic Text') end it 'strips underscore italic markdown' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:strip_markdown, '_Italic Text_') - expect(result).to eq('Italic Text') + expect(builder.send(:strip_markdown, '_Italic Text_')).to eq('Italic Text') + end + + it 'handles multiple markdown elements' do + expect(builder.send(:strip_markdown, '**Bold** and _italic_')).to eq('Bold and italic') end it 'handles empty text' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:strip_markdown, '') - expect(result).to eq('') + expect(builder.send(:strip_markdown, '')).to eq('') end it 'handles nil text' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:strip_markdown, nil) - expect(result).to eq('') + expect(builder.send(:strip_markdown, nil)).to eq('') end end describe '#extract_email' do + subject(:builder) { described_class.new(forwarded_message.id) } + it 'extracts email from format "Name "' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:extract_email, 'John Doe ') - expect(result).to eq('john@example.com') + expect(builder.send(:extract_email, 'John Doe ')).to eq('john@example.com') end it 'returns plain email as-is' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:extract_email, 'john@example.com') - expect(result).to eq('john@example.com') + expect(builder.send(:extract_email, 'john@example.com')).to eq('john@example.com') end it 'handles empty string' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:extract_email, '') - expect(result).to eq('') + expect(builder.send(:extract_email, '')).to eq('') end it 'handles nil' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:extract_email, nil) - expect(result).to eq('') + expect(builder.send(:extract_email, nil)).to eq('') end end describe '#parse_from_field' do + subject(:builder) { described_class.new(forwarded_message.id) } + it 'formats "Name " correctly' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:parse_from_field, 'John Doe ') - expect(result).to eq('John Doe ') + expect(builder.send(:parse_from_field, 'John Doe ')).to eq('John Doe ') end it 'handles plain email' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:parse_from_field, 'john@example.com') - expect(result).to eq('john@example.com') + expect(builder.send(:parse_from_field, 'john@example.com')).to eq('john@example.com') end it 'handles extra spaces' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:parse_from_field, 'John Doe < john@example.com >') - expect(result).to eq('John Doe ') + expect(builder.send(:parse_from_field, 'John Doe < john@example.com >')).to eq('John Doe ') end it 'handles empty string' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:parse_from_field, '') - expect(result).to eq('') + expect(builder.send(:parse_from_field, '')).to eq('') end it 'handles nil' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:parse_from_field, nil) - expect(result).to eq('') + expect(builder.send(:parse_from_field, nil)).to eq('') end end describe '#format_date_string' do + subject(:builder) { described_class.new(forwarded_message.id) } + it 'formats the date in a readable format' do - builder = described_class.new(forwarded_message.id) # NOTE: We don't test the exact formatted output since it uses DateTime.now # which would be different for each test run result = builder.send(:format_date_string, '2025-04-29T14:29:07+05:30') @@ -302,21 +341,15 @@ RSpec.describe Messages::ForwardedMessageBuilder do end it 'handles empty string' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:format_date_string, '') - expect(result).to eq('') + expect(builder.send(:format_date_string, '')).to eq('') end it 'handles nil' do - builder = described_class.new(forwarded_message.id) - result = builder.send(:format_date_string, nil) - expect(result).to eq('') + expect(builder.send(:format_date_string, nil)).to eq('') end it 'returns current date formatted regardless of input' do - builder = described_class.new(forwarded_message.id) result = builder.send(:format_date_string, 'Invalid Date') - expect(result).to match(/\w{3}, \w{3} \d+, \d{4} at \d+:\d+ [AP]M/) end end diff --git a/spec/builders/messages/message_builder_spec.rb b/spec/builders/messages/message_builder_spec.rb index 68e38f9c7..d3f98db45 100644 --- a/spec/builders/messages/message_builder_spec.rb +++ b/spec/builders/messages/message_builder_spec.rb @@ -183,162 +183,243 @@ describe Messages::MessageBuilder do end describe '#perform with forwarded messages' do - let(:email_channel) { create(:channel_email, account: account) } - let(:email_inbox) { email_channel.inbox } - let(:email_conversation) { create(:conversation, inbox: email_inbox, account: account) } + def create_email_channel_with_inbox + channel = create(:channel_email, account: account) + inbox = channel.inbox - # Create a message with email data to be forwarded - let(:forwarded_message) do - create(:message, conversation: email_conversation, account: account, - content_attributes: { - email: { - 'from' => ['sender@example.com'], - 'to' => ['recipient@example.com'], - 'subject' => 'Test Subject', - 'date' => '2025-04-29T14:29:07+05:30', - 'html_content' => { - 'full' => '
HTML content
', - 'quoted' => 'HTML content', - 'reply' => 'HTML content' - }, - 'text_content' => { - 'full' => 'Text content', - 'quoted' => 'Text content', - 'reply' => 'Text content' - } - } - }) + { + channel: channel, + inbox: inbox + } end - context 'when forwarding a message from a non-email inbox' do - let(:params) do - ActionController::Parameters.new({ - content: 'Forwarded message:', - content_attributes: { forwarded_message_id: forwarded_message.id } - }) - end - - it 'includes the forwarded message ID in content_attributes' do - message = described_class.new(user, conversation, params).perform - expect(message.content_attributes[:forwarded_message_id]).to eq(forwarded_message.id) - end - - it 'updates the content with the forwarded message text' do - message = described_class.new(user, conversation, params).perform - expect(message.content).to include('Forwarded message:') - expect(message.content).to include('---------- Forwarded message ---------') - end + def create_email_conversations(inbox) + { + source: create(:conversation, inbox: inbox, account: account), + target: create(:conversation, inbox: inbox, account: account) + } end - context 'when forwarding a message from an email inbox' do - let(:params) do - ActionController::Parameters.new({ - content: 'Forwarded message:', - content_attributes: { forwarded_message_id: forwarded_message.id } - }) - end + def standard_email_data + { + 'from' => ['sender@example.com'], + 'to' => ['recipient@example.com'], + 'subject' => 'Test Subject', + 'date' => '2025-04-29T14:29:07+05:30', + 'html_content' => { + 'full' => '
HTML content
', + 'quoted' => 'HTML content', + 'reply' => 'HTML content' + }, + 'text_content' => { + 'full' => 'Text content', + 'quoted' => 'Text content', + 'reply' => 'Text content' + } + } + end - let(:email_conversation_target) { create(:conversation, inbox: email_inbox, account: account) } + def setup_email_environment + channel_data = create_email_channel_with_inbox + conversations = create_email_conversations(channel_data[:inbox]) - it 'includes the forwarded email data in content_attributes' do - message = described_class.new(user, email_conversation_target, params).perform + { + email_inbox: channel_data[:inbox], + email_conversation: conversations[:source], + target_conversation: conversations[:target], + standard_email_data: standard_email_data + } + end + + context 'with different message types' do + let(:env) { setup_email_environment } + + it 'preserves email data when forwarding' do + # Create the original message to be forwarded + forwarded_message = create(:message, + conversation: env[:email_conversation], + account: account, + content_attributes: { email: env[:standard_email_data] }) + + # Setup params to forward the message + forward_params = ActionController::Parameters.new({ + content: 'Forwarded message:', + content_attributes: { forwarded_message_id: forwarded_message.id } + }) + + message = described_class.new(user, env[:target_conversation], forward_params).perform expect(message.content_attributes[:forwarded_message_id]).to eq(forwarded_message.id) expect(message.content_attributes[:email]).to be_present - expect(message.content_attributes[:email]['html_content']).to be_present - expect(message.content_attributes[:email]['text_content']).to be_present - end - - it 'preserves the HTML content in the forwarded email' do - message = described_class.new(user, email_conversation_target, params).perform - + expect(message.content).to include('Forwarded message:') + expect(message.content).to include('---------- Forwarded message ---------') html_content = message.content_attributes[:email]['html_content']['full'] - expect(html_content).to include('
HTML content
') - end - - it 'preserves the text content in the forwarded email' do - message = described_class.new(user, email_conversation_target, params).perform - text_content = message.content_attributes[:email]['text_content']['full'] + expect(html_content).to include('
HTML content
') expect(text_content).to include('Text content') end - end - context 'when forwarding a message with markdown content' do - let(:params) do - ActionController::Parameters.new({ - content: '**Bold text** and _italic text_', - content_attributes: { forwarded_message_id: forwarded_message.id } - }) - end + it 'handles markdown content in forwarded messages' do + # Create the original message to be forwarded + forwarded_message = create(:message, + conversation: env[:email_conversation], + account: account, + content_attributes: { email: env[:standard_email_data] }) - it 'converts markdown to HTML in the HTML content' do - message = described_class.new(user, email_conversation, params).perform + markdown_content = '**Bold text** and _italic text_' + forward_params = ActionController::Parameters.new({ + content: markdown_content, + content_attributes: { forwarded_message_id: forwarded_message.id } + }) + + message = described_class.new(user, env[:email_conversation], forward_params).perform html_content = message.content_attributes[:email]['html_content']['full'] + text_quoted = message.content_attributes[:email]['text_content']['quoted'] + full_text = message.content_attributes[:email]['text_content']['full'] + expect(html_content).to include('Bold text') expect(html_content).to include('italic text') + expect(text_quoted).to eq(markdown_content) + expect(full_text).to include(markdown_content) + expect(full_text).to include('---------- Forwarded message ---------') end - it 'preserves markdown in the text content' do - message = described_class.new(user, email_conversation, params).perform + it 'returns empty email data when forwarding a message with no email data' do + regular_message = create(:message, conversation: conversation, account: account) - text_content = message.content_attributes[:email]['text_content']['full'] - expect(text_content).to include('**Bold text**') - expect(text_content).to include('_italic text_') - end - end + forward_params = ActionController::Parameters.new({ + content: 'Forwarding a regular message:', + content_attributes: { forwarded_message_id: regular_message.id } + }) - context 'when forwarding a message with no email data' do - let(:regular_message) { create(:message, conversation: conversation, account: account) } - - let(:params) do - ActionController::Parameters.new({ - content: 'Forwarding a regular message:', - content_attributes: { forwarded_message_id: regular_message.id } - }) - end - - it 'includes the forwarded message ID and empty email data' do - message = described_class.new(user, email_conversation, params).perform + # Create the forwarded message + message = described_class.new(user, env[:email_conversation], forward_params).perform + # Verify empty email data expect(message.content_attributes[:forwarded_message_id]).to eq(regular_message.id) expect(message.content_attributes[:email]).to eq({}) end - end - context 'with multipart email content' do - let(:multipart_message) do - create(:message, conversation: email_conversation, account: account, - content_attributes: { - email: { - 'from' => ['sender@example.com'], - 'to' => ['recipient@example.com'], - 'subject' => 'Multipart Test', - 'date' => '2025-04-29T14:29:07+05:30', - 'html_content' => { - 'full' => '
HTML formatted content
' - }, - 'text_content' => { - 'full' => 'Plain text content' - } - } - }) - end + it 'preserves multipart content in forwarded messages' do + # Create a multipart email message + multipart_data = env[:standard_email_data].merge( + 'html_content' => { 'full' => '
HTML formatted content
' }, + 'text_content' => { 'full' => 'Plain text content' } + ) - let(:params) do - ActionController::Parameters.new({ - content: 'Forwarding multipart email:', - content_attributes: { forwarded_message_id: multipart_message.id } - }) - end + multipart_message = create(:message, + conversation: env[:email_conversation], + account: account, + content_attributes: { email: multipart_data }) - it 'preserves both HTML and text parts in the forwarded email' do - message = described_class.new(user, email_conversation, params).perform + forward_params = ActionController::Parameters.new({ + content: 'Forwarding multipart email:', + content_attributes: { forwarded_message_id: multipart_message.id } + }) + message = described_class.new(user, env[:email_conversation], forward_params).perform + + # Verify multipart content is preserved expect(message.content_attributes[:email]['html_content']['full']).to include('
HTML formatted content
') expect(message.content_attributes[:email]['text_content']['full']).to include('Plain text content') end end + + context 'with attachments' do + let(:env) { setup_email_environment } + + def create_base_message(conversation) + create(:message, + conversation: conversation, + account: account, + content_attributes: { + email: standard_email_data.merge( + 'html_content' => { 'full' => '
Message with attachments
' }, + 'text_content' => { 'full' => 'Message with attachments' } + ) + }) + end + + def add_text_attachment(message) + attachment = message.attachments.new(account_id: account.id, file_type: 'file') + attachment.file.attach( + io: StringIO.new('test file content'), + filename: 'test.txt', + content_type: 'text/plain' + ) + attachment.save! + end + + def add_image_attachment(message) + attachment = message.attachments.new(account_id: account.id, file_type: 'image') + attachment.file.attach( + io: StringIO.new('fake image content'), + filename: 'test.jpg', + content_type: 'image/jpeg' + ) + attachment.save! + end + + def create_message_with_attachments(conversation) + message = create_base_message(conversation) + add_text_attachment(message) + add_image_attachment(message) + message + end + + it 'copies attachments from the forwarded message' do + message_with_attachments = create_message_with_attachments(env[:email_conversation]) + + forward_params = ActionController::Parameters.new({ + content: 'Forwarding message with attachments:', + content_attributes: { forwarded_message_id: message_with_attachments.id } + }) + + message = described_class.new(user, env[:email_conversation], forward_params).perform + + # Verify attachments are copied + expect(message.attachments.count).to eq(2) + expect(message.attachments.map(&:file_type)).to include('file', 'image') + expect(message.attachments.first.file).to be_attached + + # Verify attachment data in content_attributes + expect(message.content_attributes[:email]['attachments']).to be_present + expect(message.content_attributes[:email]['attachments'].length).to eq(2) + end + end + + context 'with nested forwarding' do + let(:env) { setup_email_environment } + + it 'maintains proper forwarding chain data' do + # Create original message + parent_message = create(:message, + conversation: env[:email_conversation], + account: account, + content_attributes: { email: env[:standard_email_data] }) + + # Create first level forward + first_level_params = ActionController::Parameters.new({ + content: 'First forwarded message:', + content_attributes: { forwarded_message_id: parent_message.id } + }) + + first_forward = described_class.new(user, env[:email_conversation], first_level_params).perform + + # Create second level forward + second_level_params = ActionController::Parameters.new({ + content: 'Forwarding a forwarded message:', + content_attributes: { forwarded_message_id: first_forward.id } + }) + + message = described_class.new(user, env[:email_conversation], second_level_params).perform + + expect(message.content_attributes[:forwarded_message_id]).to be_present + expect(message.content).to include('Forwarding a forwarded message:') + expect(message.content).to include('---------- Forwarded message ---------') + expect(message.content).to include('First forwarded message:') + end + end end end