Skip to content

Commit

Permalink
FEATURE: Add attachments to outgoing emails
Browse files Browse the repository at this point in the history
This feature is off by default and can can be configured with the `email_total_attachment_size_limit_kb` site setting.

Co-authored-by: Maja Komel <maja.komel@gmail.com>
  • Loading branch information
gschlager and majakomel committed Jul 25, 2019
1 parent 0e1d615 commit 7e0eeed
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 0 deletions.
1 change: 1 addition & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1812,6 +1812,7 @@ en:
enable_forwarded_emails: "[BETA] Allow users to create a topic by forwarding an email in."
always_show_trimmed_content: "Always show trimmed part of incoming emails. WARNING: might reveal email addresses."
private_email: "Don't include content from posts or topics in email title or email body. NOTE: also disables digest emails."
email_total_attachment_size_limit_kb: "Max total size of files attached to outgoing emails. Set to 0 to disable sending of attachments."

manual_polling_enabled: "Push emails using the API for email replies."
pop3_polling_enabled: "Poll via POP3 for email replies."
Expand Down
3 changes: 3 additions & 0 deletions config/site_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1021,6 +1021,9 @@ email:
enable_forwarded_emails: false
always_show_trimmed_content: false
private_email: false
email_total_attachment_size_limit_kb:
default: 0
max: 51200

files:
max_image_size_kb:
Expand Down
34 changes: 34 additions & 0 deletions lib/email/sender.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ def send
# guards against deleted posts
return skip(SkippedEmailLog.reason_types[:sender_post_deleted]) unless post

add_attachments(post)

topic = post.topic
first_post = topic.ordered_posts.first

Expand Down Expand Up @@ -239,6 +241,38 @@ def self.host_for(base_url)

private

def add_attachments(post)
max_email_size = SiteSetting.email_total_attachment_size_limit_kb.kilobytes
return if max_email_size == 0

email_size = 0
post.uploads.each do |upload|
next if FileHelper.is_supported_image?(upload.original_filename)
next if email_size + upload.filesize > max_email_size

begin
path = if upload.local?
Discourse.store.path_for(upload)
else
Discourse.store.download(upload).path
end

@message.attachments[upload.original_filename] = File.read(path)
email_size += File.size(path)
rescue => e
Discourse.warn_exception(
e,
message: "Failed to attach file to email",
env: {
post_id: post.id,
upload_id: upload.id,
filename: upload.original_filename
}
)
end
end
end

def header_value(name)
header = @message.header[name]
return nil unless header
Expand Down
63 changes: 63 additions & 0 deletions spec/components/email/sender_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,69 @@
end
end

context "with attachments" do
fab!(:small_pdf) do
SiteSetting.authorized_extensions = 'pdf'
UploadCreator.new(file_from_fixtures("small.pdf", "pdf"), "small.pdf")
.create_for(Discourse.system_user.id)
end
fab!(:large_pdf) do
SiteSetting.authorized_extensions = 'pdf'
UploadCreator.new(file_from_fixtures("large.pdf", "pdf"), "large.pdf")
.create_for(Discourse.system_user.id)
end
fab!(:csv_file) do
SiteSetting.authorized_extensions = 'csv'
UploadCreator.new(file_from_fixtures("words.csv", "csv"), "words.csv")
.create_for(Discourse.system_user.id)
end
fab!(:image) do
SiteSetting.authorized_extensions = 'png'
UploadCreator.new(file_from_fixtures("logo.png", "images"), "logo.png")
.create_for(Discourse.system_user.id)
end
fab!(:post) { Fabricate(:post) }
fab!(:reply) do
raw = <<~RAW
Hello world!
#{DiscourseMarkdown.attachment_markdown(small_pdf)}
#{DiscourseMarkdown.attachment_markdown(large_pdf)}
#{DiscourseMarkdown.image_markdown(image)}
#{DiscourseMarkdown.attachment_markdown(csv_file)}
RAW
reply = Fabricate(:post, raw: raw, topic: post.topic, user: Fabricate(:user))
reply.link_post_uploads
reply
end
fab!(:notification) { Fabricate(:posted_notification, user: post.user, post: reply) }
let(:message) do
UserNotifications.user_posted(
post.user,
post: reply,
notification_type: notification.notification_type,
notification_data_hash: notification.data_hash
)
end

it "adds only non-image uploads as attachments to the email" do
SiteSetting.email_total_attachment_size_limit_kb = 10_000
Email::Sender.new(message, :valid_type).send

expect(message.attachments.length).to eq(3)
expect(message.attachments.map(&:filename))
.to contain_exactly(*[small_pdf, large_pdf, csv_file].map(&:original_filename))
end

it "respects the size limit and attaches only files that fit into the max email size" do
SiteSetting.email_total_attachment_size_limit_kb = 40
Email::Sender.new(message, :valid_type).send

expect(message.attachments.length).to eq(2)
expect(message.attachments.map(&:filename))
.to contain_exactly(*[small_pdf, csv_file].map(&:original_filename))
end
end

context 'with a deleted post' do

it 'should skip sending the email' do
Expand Down
Binary file added spec/fixtures/pdf/large.pdf
Binary file not shown.

1 comment on commit 7e0eeed

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/mailing-list-replacement-email-attachments/17710/27

Please sign in to comment.