Permalink
Browse files

Merge pull request #8048 from senny/7761_dont_render_view_without_mai…

…l_call

Do not render views when mail() isn't called. (NullMail refactoring)
  • Loading branch information...
2 parents a698175 + b786f06 commit a273b6bd34fe5361b121e450f69d95277166c049 @rafaelfranca rafaelfranca committed Oct 28, 2012
View
5 actionmailer/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* Do not render views when mail() isn't called.
+ Fix #7761
+
+ *Yves Senn*
+
* Support `Mailer.deliver_foo(*args)` as a synonym for
`Mailer.foo(*args).deliver`. This makes it easy to write e.g.
`Mailer.expects(:deliver_foo)` when testing code that calls
View
14 actionmailer/lib/action_mailer/base.rb
@@ -510,7 +510,19 @@ def initialize(method_name=nil, *args)
def process(*args) #:nodoc:
lookup_context.skip_default_locale!
- super
+
+ generated_mail = super
+ unless generated_mail
+ @_message = NullMail.new
@spastorino
Ruby on Rails member
spastorino added a line comment Oct 28, 2012

what about @_message = NullMail.new unless super ?

@rafaelfranca
Ruby on Rails member
rafaelfranca added a line comment Oct 28, 2012

Seems good. But I think we changed the return value of process. I think it should be:

generated_mail = super

if generated_mail
  generated_mail
else
  @_message = NullMail.new
end

Or don't we care about the return value?

@spastorino
Ruby on Rails member
spastorino added a line comment Oct 29, 2012

super || @_message = NullMail.new

@spastorino
Ruby on Rails member
spastorino added a line comment Oct 29, 2012

Anyway your code is exactly the same as @_message = NullMail.new unless super or you meant something like ...

super.tap do |generated_mail|
  @_message = NullMail.new unless generated_mail
end
@rafaelfranca
Ruby on Rails member
rafaelfranca added a line comment Oct 29, 2012

Feel free to change bro 😉

@spastorino
Ruby on Rails member
spastorino added a line comment Nov 29, 2012

Done 544b5ee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+ end
+
+ class NullMail #:nodoc:
+ def body; '' end
+
+ def method_missing(*args)
+ nil
+ end
end
def mailer_name
View
6 actionmailer/test/base_test.rb
@@ -499,6 +499,12 @@ def teardown
end
end
+ test 'the view is not rendered when mail was never called' do
+ mail = BaseMailer.without_mail_call
+ assert_equal('', mail.body.to_s.strip)
+ mail.deliver
+ end
+
# Before and After hooks
class MyObserver
View
1 actionmailer/test/fixtures/base_mailer/without_mail_call.erb
@@ -0,0 +1 @@
+<% raise 'the template should not be rendered' %>
View
3 actionmailer/test/mailers/base_mailer.rb
@@ -115,4 +115,7 @@ def different_layout(layout_name='')
def email_with_translations
mail body: render("email_with_translations", formats: [:html])
end
+
+ def without_mail_call
+ end
end

0 comments on commit a273b6b

Please sign in to comment.