readysteady / mailgunner Goto Github PK

Hey,

I'm trying to change the mailgun_settings on after_action callback but after looking at the code it seems that it does not respect the settings data and convention of DeliveryMethod class.

class UserMailer < ApplicationMailer
  before_action { @business, @user = params[:business], params[:user] }
 
  after_action :set_delivery_options
 
  private
    def set_delivery_options
      if @business && @business.has_mailgun_settings?
        mail.delivery_method.settings.merge!(@business.mailgun_settings)
      end
    end
end

Mailgun API specifies using multipart/form-data encoding for attachments. However, the only multi-part functionality in your public methods is in send_mime() which takes some kind of 'mail' object as the only parameter. Is this how you intend attachments to be sent, and how do you build this mail object? What class does it inherit from?

http://documentation.mailgun.com/api-sending.html#sending
“File attachment. You can post multiple attachment values. Important: You must use multipart/form-data encoding when sending attachments.”

Sadly sending mails using cc or bcc does not work for us. Can someone confirm this? Or do we do something wrong?

Would love to help you to investigate this and send a PR after we figured out what is wrong.

I get the RuntimeError: Invalid delivery method :mailgun error when using this gem on Heroku.

RubyGems.org doesn't report a license for your gem. This is because it is not specified in the gemspec of your last release.

via e.g.

  spec.license = 'MIT'
  # or
  spec.licenses = ['MIT', 'GPL-2']

Including a license in your gemspec is an easy way for rubygems.org and other tools to check how your gem is licensed. As you can imagine, scanning your repository for a LICENSE file or parsing the README, and then attempting to identify the license or licenses is much more difficult and more error prone. So, even for projects that already specify a license, including a license in your gemspec is a good practice. See, for example, how rubygems.org uses the gemspec to display the rails gem license.

There is even a License Finder gem to help companies/individuals ensure all gems they use meet their licensing needs. This tool depends on license information being available in the gemspec. This is an important enough issue that even Bundler now generates gems with a default 'MIT' license.

I hope you'll consider specifying a license in your gemspec. If not, please just close the issue with a nice message. In either case, I'll follow up. Thanks for your time!

Appendix:

If you need help choosing a license (sorry, I haven't checked your readme or looked for a license file), GitHub has created a license picker tool. Code without a license specified defaults to 'All rights reserved'-- denying others all rights to use of the code.
Here's a list of the license names I've found and their frequencies

p.s. In case you're wondering how I found you and why I made this issue, it's because I'm collecting stats on gems (I was originally looking for download data) and decided to collect license metadata,too, and make issues for gemspecs not specifying a license as a public service :). See the previous link or my blog post about this project for more information.

Changes I would like to see before v2.0.0, and working towards:

• allow :domain option to be nil
• eliminate faux development dependency
• add method for sending a Mail message in MIME format
• add ActionMailer integration
• remove deprecated json methods
• remove deprecated get_log/mailbox methods
• remove deprecated :json option
• remove Mailgunner::Response class and add exception classes
• switch to webmock
• add changelog

Feedback and other suggestions welcome.

Mailgun introduced a new api endpoint for sending from European regions. To keep api calls working with domains in the new region the api endpoint should be configurabel.

The new api endpoint is:
api.eu.mailgun.net

the main issue is that the response code (ref) is too generic. this error could be caught within Mailgunner, providing more information.

api calls requiring domain could be moved to a separate 'internal' class. all calls would be 'routed' through it and

if domain is nil
  # don't send the request
  raise an error # 'domain' variable not set; check your `ENV["MAILGUN_SMTP_LOGIN"]` variable or `Mailgunner::Client` initializer

The most recent version of mail (2.6.6) appears to have changed the semantics of Mail::CheckDeliveryParams in this commit: mikel/mail@37908c3

I believe the fix is to remove include Mail::CheckDeliveryParams from lib/mailgunner/delivery_method.rb and change check_delivery_params(mail) to Mail::CheckDeliveryParams.check(mail).