Skip to content

Commit

Permalink
Handle multiple files through a single attachment
Browse files Browse the repository at this point in the history
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
  • Loading branch information
carlosantoniodasilva committed Nov 4, 2022
1 parent 761bee3 commit c9a3f93
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 9 additions & 12 deletions lib/mail_form/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
10 changes: 8 additions & 2 deletions test/mail_form_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c9a3f93

Please sign in to comment.