tansengming / stripe-rails

A Rails Engine for integrating with Stripe
MIT License
753 stars 123 forks source link

Added option to ignore missing API key and don't show any warning #197

Closed ndbroadbent closed 3 years ago

ndbroadbent commented 3 years ago

Hello! I have a few deployments and commands where a Stripe API key is not needed, so I wanted to add this option to silence the "No stripe.com API key was configured for environment ..." warning.

Would this be ok? Does ignore_missing_secret_key sound alright, or would you prefer something else? Thanks!

tansengming commented 3 years ago

Hi @ndbroadbent thanks for the PR. I think the name is fine. Can you finish it off when with 2 more tasks please?

Also I'm curious, how are those apps using the gem without the api_key?

ndbroadbent commented 3 years ago

Hi @tansengming, thanks for the review! I have added some tests and added a note to the readme.

Also I'm curious, how are those apps using the gem without the api_key?

So the main issue is that I have some cases where I'm not using the gem at all. Some of my customers are installing my application on their own servers as a private deployment without any Stripe integration. I also have some cases during my build script where I just need to run a task inside a Docker container, so instead of specifying a fake Stripe key, I would prefer to just silence the warning.

I could use gem 'stripe-rails', require: false in my Gemfile and manually require the gem when it's needed, but the huge problem with this approach is that I have a lot of other code throughout my application that does actually rely on the gem. e.g. Maybe I don't realize that some code depends on a constant loaded by stripe-rails or one of it's dependencies, and I accidentally call that code when stripe-rails isn't loaded. (This has happened to me a few times in the past for other dependencies, where I forgot that I actually needed them in production.)

So if I did this, then I would always have to run my test suite twice (once with ENV['STRIPE_SECRET_KEY'] set, and once without), just to make sure that nothing is crashing for both configurations. It's much easier to always keep the stripe-rails gem loaded and just dynamically change the behavior at runtime with some simple conditions. It's also much easier to write tests for these conditions.

tansengming commented 3 years ago

Looks good to me! Thanks for the PR @ndbroadbent !