In generic_send_email, check if mail object responds to delivery method instead of checking inheritance from ActionMailer

[Plsr]

What changed

• generic_send_email now checks if the created mail object responds to the configured delivery method and attempts to send out the message if this is true. (Was checking for inheritance from ActionMailer::Base before)
• Added an empty manifest.js in the rails app in specs. Not having this would cause errors running bundle exec rake with sprockets 4 (happy to remove this if this is just a problem with my configuration)

Why?

In one of our projects, we are using the mandrill_mailer gem to send out mails with MailChimp. mandrill_mailer does not inherit from ActionMailer, but implements all its delivery methods to mimic its behavior. It took us quite a while to find out why out mails were not being sent by sorcery.

Since the user is required to configure a "Mailer" and pass it to sorcery and also can configure not to automatically send out messages, I would expect sorcery to send out messages if they are sendable, no matter the communication channel.

Feedback

From what I can see in the version history, this inheritance check has been there from the first commit and was just loosened up a little in 2012 (ab97a3f#diff-ae4a62ce1fc7636933767432515a82bdR187). Since the check was pretty explicit, I don't know what side effects this change might have and if I'm overlooking anything here.

Also I'm not sure if this might be a breaking change for some users, as it could trigger sending out messages twice, based on the configuration.

Looking forward to hear your thoughts on this!

[mladenilic]

I agree with this change.

it could trigger sending out messages twice, based on the configuration.

I'm not sure what do you mean by this?

P.S. build should now be fixed (5ac8d62)

[Plsr]

I was thinking of someone with a similar setup I described above that we are currently in.

Let's say SuperMailer was implementing deliver_later, but was not inheriting from ActionMailer. So, to make the automated mail delivery work in the current version of sorcery, people may do something like

class UserMailer < SuperMailer
  def activation_needed_email
    mail_construction_method(
      # Some mail setup
    ).deliver_later
  end
end

I'm not sure if we would be breaking something with the explicit deliver_later inside the activation_needed_email function that, with the changes introduced here, would also be invoked with deliver_later.
I feel like this should not be a problem, but I'm not 100% sure about it, that's why I brought it up.

[mladenilic]

@Plsr: Thanks for additional explanation, I see now what you mean.

I'd say that it would be enough simply to explain this in the change log, so users who were affected by this issue can now get rid of hacks.

[Plsr]

Any updates on this? We are encountering the same problem once again in another project.

[stephanpavlovic]

Is there anything more needed to merge this? I'm having the same issue....

[joshbuker]

Merging this into master, I think this would be a good opportunity to get @mladenilic to try their hands at the deployment process.

Hopefully we should have a new version out in the next couple days, barring that you should be able to point at the master branch to get the change in the short term.