Skip to content

Make environment name and intercept mail customizable#13

Merged
neerajsingh0101 merged 2 commits intobigbinary:masterfrom
monkbroc:master
Jan 29, 2015
Merged

Make environment name and intercept mail customizable#13
neerajsingh0101 merged 2 commits intobigbinary:masterfrom
monkbroc:master

Conversation

@monkbroc
Copy link
Copy Markdown
Contributor

I found the need to use something other than Rails.env.production? to decide whether to intercept mail. This is because my staging instance is very close to the production instance, down to the RAILS_ENV value set to production.

I'm following the recommendations from Heroku and using environment variables for the few things that should be different between the different instances.

This PR allows for passing the name of the environment and whether mail should be intercepted as options.

I also think it's useful to rename production? to intercept_mail? because I may want to send mail in my demo instance, even though it's not strictly a production environment.

Let me know you thoughts on these changes.

@neerajsingh0101
Copy link
Copy Markdown
Contributor

@monkbroc I like the idea. Here are my thoughts.

I also like the switch to intercept_mail? . It is much more logical name.

So currently proposed change looks like this

MailInterceptor::Interceptor.new({ 
                                   env_name: ENV["INSTANCE_NAME"],
                                   intercept_mail?: ENV["INTERCEPT_MAIL"] == '1',
                                   forward_emails_to: ['intercepted_emails@bigbinary.com',
                                   'qa@bigbinary.com'
 })

I think we can take it even one step further.

Whether to intercept email decision should solely be decided by key intercept_email?. I was thinking that one can pass an object and the gem will call intercept? on that class. In this way all the logic is encapsulated inside that class and the user has total control. The user can make use of whatever the user wants to make use of. And no need to pass env_name key.

In the above the case the proposed solution would look like this.

class MyInterceptor
def intercept?; ENV["INTERCEPT_MAIL"] == '1' ; end
end

MailInterceptor::Interceptor.new({ 
                                   intercept_mail?: MyInterceptor.new,
                                   forward_emails_to: ['intercepted_emails@bigbinary.com', 'qa@bigbinary.com'
 })

If nothing is passed in intercept_email? then default logic will kick in.

Let me know your thoughts.

@monkbroc
Copy link
Copy Markdown
Contributor Author

Great suggestions.

Additionally an object to pass the environment name and intercept? is good since both of those are external dependencies. Making the environment injectable removes the need for stubs in the test.

@neerajsingh0101
Copy link
Copy Markdown
Contributor

Great. So @monkbroc want to work on that. Or I can take care of it in next few days. Lemme know.

@monkbroc
Copy link
Copy Markdown
Contributor Author

I updated the PR with another commit that implements what I said
On Jan 28, 2015 9:31 PM, "Neeraj Singh" notifications@github.com wrote:

Great. So @monkbroc https://github.com/monkbroc want to work on that.
Or I can take care of it in next few days. Lemme know.


Reply to this email directly or view it on GitHub
#13 (comment)
.

neerajsingh0101 added a commit that referenced this pull request Jan 29, 2015
Make environment name and intercept mail customizable
@neerajsingh0101 neerajsingh0101 merged commit 5b60812 into bigbinary:master Jan 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants