Permalink
Browse files

Support `Mailer.deliver_foo(*args)` as a synonym for `Mailer.foo(*arg…

…s).deliver`.

This makes it easy to write e.g. `Mailer.expects(:deliver_foo)` when
testing code that calls the mailer.
  • Loading branch information...
1 parent aa8918a commit 7e0cf563639bc7508da381b1b8321c7a89be1aa8 @jonleighton jonleighton committed Sep 28, 2012
Showing with 15 additions and 0 deletions.
  1. +5 −0 actionmailer/CHANGELOG.md
  2. +3 −0 actionmailer/lib/action_mailer/base.rb
  3. +7 −0 actionmailer/test/base_test.rb
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* 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
+ the mailer. *Jon Leighton*
+
* Allow delivery method options to be set per mail instance *Aditya Sanghi*
If your smtp delivery settings are dynamic,
@@ -142,6 +142,7 @@ module ActionMailer
# for delivery later:
#
# Notifier.welcome(david).deliver # sends the email
+ # Notifier.deliver_welcome(david) # synonym for the former
@phlipper
phlipper Oct 28, 2012

This is an interesting addition, considering it was deprecated in ActionMailer 3.0 - see line 108 and this post among other resources.

I don't personally care for this addition and I'm interested to hear what other opinions are on this.

@jonleighton
jonleighton Oct 28, 2012 Member

Interesting. I wasn't aware of that. @josevalim wdyt about this? would you prefer it to be reverted?

@phlipper
phlipper Oct 28, 2012

@jonleighton what is your plan for this? It was added a month ago, but an ack of master shows the only place it is being used is the test that was added in this commit.

I personally agreed with the decision to remove the "magic" methods. If there was a reason to re-introduce them for some reason, one could argue that the full original API should come back.

What do you think?

@phlipper
phlipper Oct 30, 2012

@josevalim do you have any opinions on this?

@dhh
dhh Oct 30, 2012 Member

I'm -1 on this. It's imo easy enough to do Mailer.expects(:welcome) and then keep mocking from there. I don't like having two ways of doing the same thing purely to make it marginally easier to mock.

@jonleighton
jonleighton Oct 30, 2012 Member
Mailer.expects(:deliver_welcome)
Mailer.expects(:welcome).returns(mock(deliver: true))

I think the former tells the story of what's happening much more clearly than the latter.

@dhh you have also told me that "multiple ways of doing things" is the ruby way in the past (in an entirely different conversation). so not convinced by that argument.

However, I really don't care enough to argue about it, so please revert if you feel strongly.

@dhh
dhh via email Oct 30, 2012 Member
# mail = Notifier.welcome(david) # => a Mail::Message object
# mail.deliver # sends the email
#
@@ -487,6 +488,8 @@ def set_payload_for_mail(payload, mail) #:nodoc:
def method_missing(method_name, *args)
if action_methods.include?(method_name.to_s)
QueuedMessage.new(queue, self, method_name, *args)
+ elsif method_name.to_s =~ /^deliver_(.+)$/ && action_methods.include?($1)
+ public_send($1, *args).deliver
else
super
end
@@ -662,6 +662,13 @@ def welcome
assert_equal ["robert.pankowecki@gmail.com"], DefaultFromMailer.welcome.from
end
+ test "Mailer.deliver_welcome calls Mailer.welcome.deliver" do
+ BaseMailer.deliveries.clear
+ BaseMailer.deliver_welcome(subject: 'omg')
+ assert_equal 1, BaseMailer.deliveries.length
+ assert_equal 'omg', BaseMailer.deliveries.first.subject
+ end
+
protected
# Execute the block setting the given values and restoring old values after

0 comments on commit 7e0cf56

Please sign in to comment.