feat: Move email attachments from links to file attachments (#11304)
Add ability to send files as attachments instead of links Fixes: https://github.com/chatwoot/chatwoot/issues/1074 ## Changes - `emaily_reply` : We will attach the small attachments as attachments and large ones as links - `reply_with_summary`, `conversation_transcript`, `reply_with_out_summary` : We will change the attachment format to the following instead of the previous `View the attachment here` ``` Attachments: file_name file_name2 ``` --------- ref: https://github.com/chatwoot/chatwoot/pull/10318/files -> for fixing : https://github.com/chatwoot/chatwoot/pull/9655#issuecomment-2183962550 --------- Co-authored-by: Marco Marinho <marcomarinho12@gmail.com> Co-authored-by: Pranav <pranavrajs@gmail.com>
This commit is contained in:
@@ -1,4 +1,8 @@
|
|||||||
class ConversationReplyMailer < ApplicationMailer
|
class ConversationReplyMailer < ApplicationMailer
|
||||||
|
# We needs to expose large attachments to the view as links
|
||||||
|
# Small attachments are linked as mail attachments directly
|
||||||
|
attr_reader :large_attachments
|
||||||
|
|
||||||
include ConversationReplyMailerHelper
|
include ConversationReplyMailerHelper
|
||||||
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')
|
default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')
|
||||||
layout :choose_layout
|
layout :choose_layout
|
||||||
|
|||||||
@@ -17,11 +17,44 @@ module ConversationReplyMailerHelper
|
|||||||
google_smtp_settings
|
google_smtp_settings
|
||||||
set_delivery_method
|
set_delivery_method
|
||||||
|
|
||||||
Rails.logger.info("Email sent from #{email_from} to #{to_emails} with subject #{mail_subject}")
|
# Email type detection logic:
|
||||||
|
# - email_reply: Sets @message with a single message
|
||||||
|
# - Other actions: Set @messages with a collection of messages
|
||||||
|
#
|
||||||
|
# So this check implicitly determines we're handling an email_reply
|
||||||
|
# and not one of the other email types (summary, transcript, etc.)
|
||||||
|
process_attachments_as_files_for_email_reply if @message&.attachments.present?
|
||||||
mail(@options)
|
mail(@options)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def process_attachments_as_files_for_email_reply
|
||||||
|
# Attachment processing for direct email replies (when replying to a single message)
|
||||||
|
#
|
||||||
|
# How attachments are handled:
|
||||||
|
# 1. Total file size (<20MB): Added directly to the email as proper attachments
|
||||||
|
# 2. Total file size (>20MB): Added to @large_attachments to be displayed as links in the email
|
||||||
|
|
||||||
|
@options[:attachments] = []
|
||||||
|
@large_attachments = []
|
||||||
|
current_total_size = 0
|
||||||
|
|
||||||
|
@message.attachments.each do |attachment|
|
||||||
|
raw_data = attachment.file.download
|
||||||
|
attachment_name = attachment.file.filename.to_s
|
||||||
|
file_size = raw_data.bytesize
|
||||||
|
|
||||||
|
# Attach files directly until we hit 20MB total
|
||||||
|
# After reaching 20MB, send remaining files as links
|
||||||
|
if current_total_size + file_size <= 20.megabytes
|
||||||
|
mail.attachments[attachment_name] = raw_data
|
||||||
|
@options[:attachments] << { name: attachment_name }
|
||||||
|
current_total_size += file_size
|
||||||
|
else
|
||||||
|
@large_attachments << attachment
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
def google_smtp_settings
|
def google_smtp_settings
|
||||||
|
|||||||
@@ -32,9 +32,10 @@
|
|||||||
<% if message.content %>
|
<% if message.content %>
|
||||||
<%= ChatwootMarkdownRenderer.new(message.content).render_message %>
|
<%= ChatwootMarkdownRenderer.new(message.content).render_message %>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% if message.attachments %>
|
<% if message.attachments.present? %>
|
||||||
|
<p>Attachments:</p>
|
||||||
<% message.attachments.each do |attachment| %>
|
<% message.attachments.each do |attachment| %>
|
||||||
Attachment [<a href="<%= attachment.file_url %>" _target="blank">Click here to view</a>]
|
<p><a href="<%= attachment.file_url %>" target="_blank"><%= attachment.file.filename.to_s %></a></p>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% end %>
|
<% end %>
|
||||||
<p style="font-size: 90%; font-size: 90%;color: #899096;margin-top: -8px; margin-bottom: 0px;">
|
<p style="font-size: 90%; font-size: 90%;color: #899096;margin-top: -8px; margin-bottom: 0px;">
|
||||||
|
|||||||
@@ -1,8 +1,9 @@
|
|||||||
<% if @message.content %>
|
<% if @message.content %>
|
||||||
<%= ChatwootMarkdownRenderer.new(@message.content).render_message %>
|
<%= ChatwootMarkdownRenderer.new(@message.content).render_message %>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% if @message.attachments %>
|
<% if @large_attachments.present? %>
|
||||||
<% @message.attachments.each do |attachment| %>
|
<p>Attachments:</p>
|
||||||
attachment [<a href="<%= attachment.file_url %>" _target="blank">click here to view</a>]
|
<% @large_attachments.each do |attachment| %>
|
||||||
|
<p><a href="<%= attachment.file_url %>" target="_blank"><%= attachment.file.filename.to_s %></a></p>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|||||||
@@ -20,9 +20,10 @@
|
|||||||
<% if message.content.present? %>
|
<% if message.content.present? %>
|
||||||
<hr style="border: 0; border-bottom: 1px solid #AEC3D5;"/>
|
<hr style="border: 0; border-bottom: 1px solid #AEC3D5;"/>
|
||||||
<% end %>
|
<% end %>
|
||||||
This message contains <%= message.attachments.count > 1 ? 'attachments' : 'an attachment' %>.
|
Attachments:
|
||||||
<% message.attachments.each do |attachment| %>
|
<% message.attachments.each_with_index do |attachment, index| %>
|
||||||
<br />- View the attachment <a href="<%= attachment.file_url %>" _target="blank">here</a>.
|
<% if index > 0 %><br /><% end %>
|
||||||
|
<a href="<%= attachment.file_url %>" target="_blank"><%= attachment.file.filename.to_s %></a>
|
||||||
<% end %>
|
<% end %>
|
||||||
</p>
|
</p>
|
||||||
<% end %>
|
<% end %>
|
||||||
|
|||||||
@@ -3,9 +3,10 @@
|
|||||||
<% if message.content %>
|
<% if message.content %>
|
||||||
<%= ChatwootMarkdownRenderer.new(message.content).render_message %>
|
<%= ChatwootMarkdownRenderer.new(message.content).render_message %>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% if message.attachments %>
|
<% if message.attachments.present? %>
|
||||||
|
<p>Attachments:</p>
|
||||||
<% message.attachments.each do |attachment| %>
|
<% message.attachments.each do |attachment| %>
|
||||||
attachment [<a href="<%= attachment.file_url %>" _target="blank">click here to view</a>]
|
<p><a href="<%= attachment.file_url %>" target="_blank"><%= attachment.file.filename.to_s %></a></p>
|
||||||
<% end %>
|
<% end %>
|
||||||
<% end %>
|
<% end %>
|
||||||
</p>
|
</p>
|
||||||
|
|||||||
BIN
spec/assets/large_file.pdf
Normal file
BIN
spec/assets/large_file.pdf
Normal file
Binary file not shown.
@@ -153,6 +153,74 @@ RSpec.describe ConversationReplyMailer do
|
|||||||
it 'updates the source_id' do
|
it 'updates the source_id' do
|
||||||
expect(mail.message_id).to eq message.source_id
|
expect(mail.message_id).to eq message.source_id
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'with email attachments' do
|
||||||
|
it 'includes small attachments as email attachments' do
|
||||||
|
message_with_attachment = create(:message, conversation: conversation, account: account, message_type: 'outgoing',
|
||||||
|
content: 'Message with small attachment')
|
||||||
|
attachment = message_with_attachment.attachments.new(account_id: account.id, file_type: :file)
|
||||||
|
attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
|
||||||
|
attachment.save!
|
||||||
|
|
||||||
|
mail = described_class.email_reply(message_with_attachment).deliver_now
|
||||||
|
|
||||||
|
# Should be attached to the email
|
||||||
|
expect(mail.attachments.map(&:filename).map(&:to_s)).to include('avatar.png')
|
||||||
|
# Should not be in large_attachments
|
||||||
|
expect(mail.body.encoded).not_to include('Attachments:')
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders large attachments as links in the email body' do
|
||||||
|
message_with_large_attachment = create(:message, conversation: conversation, account: account, message_type: 'outgoing',
|
||||||
|
content: 'Message with large attachment')
|
||||||
|
attachment = message_with_large_attachment.attachments.new(account_id: account.id, file_type: :file)
|
||||||
|
attachment.file.attach(io: Rails.root.join('spec/assets/large_file.pdf').open, filename: 'large_file.pdf', content_type: 'application/pdf')
|
||||||
|
attachment.save!
|
||||||
|
|
||||||
|
mail = described_class.email_reply(message_with_large_attachment).deliver_now
|
||||||
|
|
||||||
|
# Should NOT be attached to the email
|
||||||
|
expect(mail.attachments.map(&:filename).map(&:to_s)).not_to include('large_file.pdf')
|
||||||
|
# Should be rendered as a link in the body
|
||||||
|
expect(mail.body.encoded).to include('Attachments:')
|
||||||
|
expect(mail.body.encoded).to include('large_file.pdf')
|
||||||
|
# Should render a link with large_file.pdf as the link text
|
||||||
|
expect(mail.body.encoded).to match(%r{<a [^>]*>large_file\.pdf</a>})
|
||||||
|
# Small file should not be rendered as a link in the body
|
||||||
|
expect(mail.body.encoded).not_to match(%r{<a [^>]*>avatar\.png</a>})
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'handles both small and large attachments correctly' do
|
||||||
|
message_with_mixed_attachments = create(:message, conversation: conversation, account: account, message_type: 'outgoing',
|
||||||
|
content: 'Message with mixed attachments')
|
||||||
|
|
||||||
|
# Small attachment
|
||||||
|
small_attachment = message_with_mixed_attachments.attachments.new(account_id: account.id, file_type: :file)
|
||||||
|
small_attachment.file.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png')
|
||||||
|
small_attachment.save!
|
||||||
|
|
||||||
|
# Large attachment
|
||||||
|
large_attachment = message_with_mixed_attachments.attachments.new(account_id: account.id, file_type: :file)
|
||||||
|
large_attachment.file.attach(io: Rails.root.join('spec/assets/large_file.pdf').open, filename: 'large_file.pdf',
|
||||||
|
content_type: 'application/pdf')
|
||||||
|
large_attachment.save!
|
||||||
|
|
||||||
|
mail = described_class.email_reply(message_with_mixed_attachments).deliver_now
|
||||||
|
|
||||||
|
# Small file should be attached
|
||||||
|
expect(mail.attachments.map(&:filename).map(&:to_s)).to include('avatar.png')
|
||||||
|
# Large file should NOT be attached
|
||||||
|
expect(mail.attachments.map(&:filename).map(&:to_s)).not_to include('large_file.pdf')
|
||||||
|
|
||||||
|
# Large file should be rendered as a link in the body
|
||||||
|
expect(mail.body.encoded).to include('Attachments:')
|
||||||
|
expect(mail.body.encoded).to include('large_file.pdf')
|
||||||
|
# Should render a link with large_file.pdf as the link text
|
||||||
|
expect(mail.body.encoded).to match(%r{<a [^>]*>large_file\.pdf</a>})
|
||||||
|
# Small file should not be rendered as a link in the body
|
||||||
|
expect(mail.body.encoded).not_to match(%r{<a [^>]*>avatar\.png</a>})
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when smtp enabled for email channel' do
|
context 'when smtp enabled for email channel' do
|
||||||
|
|||||||
Reference in New Issue
Block a user