Skip to content

Commit

Permalink
Specify that domain should be nil if MAILGUN_SMTP_LOGIN not set
Browse files Browse the repository at this point in the history
  • Loading branch information
timcraft committed Nov 1, 2013
1 parent e02b1d3 commit cdcccd2
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 5 deletions.
7 changes: 3 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,9 @@ you can instead use your Mailgun public key to authenticate like this:
```ruby
require 'mailgunner'

mailgun = Mailgunner::Client.new({
api_key: 'pubkey-5ogiflzbnjrljiky49qxsiozqef5jxp7',
domain: nil
})
public_key = 'pubkey-5ogiflzbnjrljiky49qxsiozqef5jxp7'

mailgun = Mailgunner::Client.new(api_key: public_key)

response = mailgun.validate_address('john@gmail.com')
```
2 changes: 1 addition & 1 deletion lib/mailgunner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Client
attr_accessor :domain, :api_key, :http

def initialize(options = {})
@domain = options.fetch(:domain) { ENV.fetch('MAILGUN_SMTP_LOGIN').split('@').last }
@domain = options.fetch(:domain) { ENV['MAILGUN_SMTP_LOGIN'].to_s.split('@').last }

@api_key = options.fetch(:api_key) { ENV.fetch('MAILGUN_API_KEY') }

Expand Down
6 changes: 6 additions & 0 deletions spec/mailgunner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ def expect(request_class, arg)

Mailgunner::Client.new(api_key: @api_key).domain.must_equal(@domain)
end

it 'defaults to nil if the MAILGUN_SMTP_LOGIN environment variable does not exist' do

This comment has been minimized.

Copy link
@bergerjac

bergerjac Jan 19, 2015

why should domain allowed to be nil? I would suggest raising an error.

if it's only for email validation, then I'd be willing to refactor email validation to a new class and submit a pull request.

note: this 'feature' cost quite a bit of time trying to debug what was happening: Mailgunner gem was initialized beforeMAILGUN_SMTP_LOGIN was pulled in from dotenv gem

This comment has been minimized.

Copy link
@timcraft

timcraft Jan 21, 2015

Author Member

@bergerjac Not every resource is tied to a domain. IIRC the change was related to the email validation, but the domains, routes, and lists resources aren't tied to a domain either. I've considered refactoring the email validation to a new class but not sure exactly whether that would be an improvement overall.

Appreciate that would be an annoying one to debug. Open to suggestions, can you open an issue for this with some more details--you're presumably using it with Rails/ActionMailer?

ENV.delete('MAILGUN_SMTP_LOGIN')

Mailgunner::Client.new(api_key: @api_key).domain.must_be_nil
end
end

describe 'api_key method' do
Expand Down

0 comments on commit cdcccd2

Please sign in to comment.