mailer can be anonymous #6018

Merged
merged 1 commit into from May 14, 2012

Conversation

Projects
None yet
5 participants
Contributor

lest commented Apr 27, 2012

closes #5970

I understand the fix, I just wonder if problems won't appear with template lookup or related - including shoulda-matchers that's the use case of the issue. Thanks @lest :)

@josevalim josevalim commented on an outdated diff Apr 27, 2012

actionmailer/test/base_test.rb
@@ -610,6 +610,18 @@ def notify
assert_equal Set.new(["notify"]), FooMailer.action_methods
end
+ test "mailer can be anonymous" do
+ mailer = Class.new(ActionMailer::Base) do
+ def welcome
+ mail
+ end
+ end
+
+ assert_nil mailer.mailer_name
+
+ assert_equal "Welcome", mailer.welcome.subject
@josevalim

josevalim Apr 27, 2012

Member

Can we assert also on the message body? What happens in this case? Because it would be hard for this to render a template.

Owner

jeremy commented Apr 27, 2012

This works, but feels like it addresses the symptoms. Using to_s on the mailer name because it may be nil feels iffy...

Since this is a particular use case, perhaps we could default to an anonymous mailer name instead? At least we'll have a path to render the template / lookup the subject.

def mailer_name
  @mailer_name ||= anonymous? ? "anonymous" : name.underscore
end
Member

josevalim commented May 12, 2012

@carlosantoniodasilva I like it. But I don't think anyone is actually trying to render an anonymous mailer, they are doing it just for tests.

@josevalim agreed, I think it's mainly for this particular use case.

@lest could you update the pull request, testing for the message body as well as @josevalim said? Thanks!

Contributor

lest commented May 13, 2012

@carlosantoniodasilva I changed mailer_name as you proposed and added message body assertion.

@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this pull request May 14, 2012

@carlosantoniodasilva carlosantoniodasilva Merge pull request #6018 from lest/patch-1
mailer can be anonymous
647285b

@carlosantoniodasilva carlosantoniodasilva merged commit 647285b into rails:master May 14, 2012

@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this pull request May 14, 2012

@carlosantoniodasilva carlosantoniodasilva Merge pull request #6018 from lest/patch-1
mailer can be anonymous
2506d4a

@lest thanks!

Contributor

exviva commented on c17d200 May 14, 2012

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment