Skip to content

Commit

Permalink
Prevent reading inline attachments after mail was called from raisi…
Browse files Browse the repository at this point in the history
…ng an inaccurate exception

Without this change, `attachments.inline['my_attachment'].present?`, for example,
would raise the exception `Can't add attachments after mail was called`.

I first brought this issue up at rails#16163 (comment).

Note that this commit addresses only one of the 2 problems I described in that comment.
The other problem is that using `attachments.inline['my_attachment']` for reading an
attachment is unnecessary--it's the same as `attachments['my_attachment']`--even before
`mail` is called.  We could add a warning about the unnecessary use of `inline` but I'm
saving that for a later PR since my comment has not received any feedback yet.
  • Loading branch information
betesh committed May 21, 2019
1 parent 973096b commit efeddb0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion actionmailer/lib/action_mailer/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def attachments
end

class LateAttachmentsProxy < SimpleDelegator
def inline; _raise_error end
def inline; self end
def []=(_name, _content); _raise_error end

private
Expand Down
11 changes: 11 additions & 0 deletions actionmailer/test/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,17 @@ def welcome
assert_match(/Can't add attachments after `mail` was called./, e.message)
end

test "accessing inline attachments after mail was called works" do
class LateInlineAttachmentMailer < ActionMailer::Base
def welcome
mail body: "yay", from: "[email protected]", to: "[email protected]"
attachments.inline["invoice.pdf"]
end
end

assert_nothing_raised { LateInlineAttachmentMailer.welcome.message }
end

test "adding inline attachments while rendering mail works" do
class LateInlineAttachmentMailer < ActionMailer::Base
def on_render
Expand Down

0 comments on commit efeddb0

Please sign in to comment.