From c9a3f934a0e7118b05ba093b245b04fcd99c7cbd Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Fri, 4 Nov 2022 11:54:32 -0300 Subject: [PATCH] Handle multiple files through a single attachment When setting up a form uploading multiple files through a single attachment, mail form was discarding all attachments and sending none, even though it could consider the form as delivered successfully. Now we will detect multiple files and add each of them individually. Note: the reason we are not checking for something like `respond_to?(:each)` and instead are doing a specific `is_a?(Array)` check, is because a single uploaded file from Rack (which we use in our tests) also responds to the `each` method (or rather, it delegates to the tempfile internal instance used for the upload). It appears that the Rails uploaded file does not do it, but for increased compatibility I decided to go with this implementation. The previous one worked because it tried both to add multiple files and handle a single file separately, so either one of them would work. This implementation makes an upfront decision about one or the other path, by performing the `is_a?` check. I don't love these types of checks, but this seemed the most straightforward way of doing it while keeping the code easier to follow. Closes #76 --- CHANGELOG.md | 1 + README.md | 2 ++ lib/mail_form/notifier.rb | 21 +++++++++------------ test/mail_form_test.rb | 10 ++++++++-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b38ec63..7601a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased * Add support for Rails 7.0 and Ruby 3.1 (no changes required) +* Add support for multiple files through a single attachment. [#76, #78] # 1.9.0 diff --git a/README.md b/README.md index bceb775..7e2537b 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,8 @@ Options: * `:attachment` - When given, expects a file to be sent and attaches it to the e-mail. Don't forget to set your form to multitype. + It also accepts multiple files through a single attachment attribute, + and will attach them individually to the e-mail. * `:captcha` - When true, validates the attributes must be blank. This is a simple way to avoid spam and the input should be hidden with CSS. diff --git a/lib/mail_form/notifier.rb b/lib/mail_form/notifier.rb index 426006f..9dd484d 100644 --- a/lib/mail_form/notifier.rb +++ b/lib/mail_form/notifier.rb @@ -12,8 +12,11 @@ def contact(resource) resource.class.mail_attachments.each do |attribute| value = resource.send(attribute) - handle_multiple_attachments value - add_attachment value + if value.is_a?(Array) + value.each { |attachment_file| add_attachment(attachment_file) } + else + add_attachment(value) + end end headers = resource.headers @@ -22,17 +25,11 @@ def contact(resource) mail(headers) end - private - def add_attachment(attch) - return unless attch.respond_to?(:read) - attachments[attch.original_filename] = attch.read - end + private - def handle_multiple_attachments(attchs) - return unless attchs.respond_to?('each') - attchs.each do |attch| - add_attachment attch - end + def add_attachment(attachment_file) + return unless attachment_file.respond_to?(:read) + attachments[attachment_file.original_filename] = attachment_file.read end end end diff --git a/test/mail_form_test.rb b/test/mail_form_test.rb index 4c43bf8..e4cbb1b 100644 --- a/test/mail_form_test.rb +++ b/test/mail_form_test.rb @@ -10,8 +10,8 @@ def setup @advanced = AdvancedForm.new(valid_attributes) @advanced.request = ActionController::TestRequest.create(Class.new) - test_file = Rack::Test::UploadedFile.new(File.join(File.dirname(__FILE__), 'test_file.txt')) - @with_file = FileForm.new(name: 'José', email: 'my.email@my.domain.com', message: "Cool", file: test_file) + @test_file = Rack::Test::UploadedFile.new(File.join(File.dirname(__FILE__), 'test_file.txt')) + @with_file = FileForm.new(name: 'José', email: 'my.email@my.domain.com', message: "Cool", file: @test_file) ActionMailer::Base.deliveries = [] end @@ -126,6 +126,12 @@ def test_form_with_file_does_not_output_attachment_as_attribute assert_no_match %r[File:], first_delivery.body.to_s end + def test_form_with_multiple_files_includes_attachments + @with_file.file = [@test_file, @test_file, @test_file] + @with_file.deliver + assert_equal 3, first_delivery.attachments.size + end + protected def first_delivery