tansengming / stripe-rails

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

Respect plan ID case when constant not defined #226

Closed swebb closed 2 months ago

swebb commented 9 months ago

Prior to 1.6.2 the plan ID could have mixed case characters. From 1.6.2 onwards the plan ID is downcased which causes lookup misses. This PR restores the original behaviour when the constant name isn't defined.

The original change as in https://github.com/tansengming/stripe-rails/commit/ada910de738945ad7855d35b7b4e383234592a05

tansengming commented 9 months ago

For reference:

before this PR, configuring a plan with mixed case, e.g. Stripe.plan :mixed_CASE do ... would create

  1. Stripe::Plans[:mixed_case]
  2. Stripe::Plans::MIXED_CASE

With this PR, configuring a plan with mixed case, e.g. Stripe.plan :mixed_CASE do ... would create

  1. Stripe::Plans[:mixed_CASE]
  2. Stripe::Plans::MIXED_CASE
tansengming commented 9 months ago

Hi @swebb thanks for the PR!

I'm a little torn by this change. You're right, the change in https://github.com/tansengming/stripe-rails/commit/ada910de738945ad7855d35b7b4e383234592a05 broke the way plan IDs used to work with mixed case characters.

However, this change has been around for > 4 years. I'm worried that folks have started to depend on the way plan IDs have worked for the last 4 years.

On the other hand, I have a feeling most folks do not use mixed case ids. And I can bump the gem version up to make sure folks get a heads up.

Let me think about it for a few more days.

What do you think? I'd love it if other folks have comments too.