Add support for the PaymentIntent resource #657
Conversation
remi-stripe
force-pushed
the
remi-add-payment-intent
branch
5 times, most recently
from
79dea47
to
c625218
Compare
| @@ -38,7 +38,7 @@ Metrics/MethodLength: | |||
| # Offense count: 1 | |||
| # Configuration parameters: CountComments. | |||
| Metrics/ModuleLength: | |||
| Max: 307 | |||
| Max: 350 | |||
There was a problem hiding this comment.
This was hardcoded to the current size which does not make sense I think so I bumped it a lot so that others don't get lost like I was
There was a problem hiding this comment.
You can regenerate the configuration with rubocop --auto-gen-config. Those metrics cops can be a bit annoying though, we should consider disabling some of them or setting real limits we want to enforce.
There was a problem hiding this comment.
Is it okay to do what I did to avoid this for every future pr adding a new resource?
There was a problem hiding this comment.
Yeah, it's fine for now. I'm guessing the largest module is Stripe::Util. We can look into fixing these metrics later.
| @@ -38,7 +38,7 @@ Metrics/MethodLength: | |||
| # Offense count: 1 | |||
| # Configuration parameters: CountComments. | |||
| Metrics/ModuleLength: | |||
| Max: 307 | |||
| Max: 350 | |||
There was a problem hiding this comment.
You can regenerate the configuration with rubocop --auto-gen-config. Those metrics cops can be a bit annoying though, we should consider disabling some of them or setting real limits we want to enforce.
test/stripe/payment_intent_test.rb
Outdated
| payment_intent = payment_intent.cancel | ||
| assert payment_intent.is_a?(Stripe::PaymentIntent) | ||
| end | ||
| end |
There was a problem hiding this comment.
Any specific reason you didn't write tests for #capture and #confirm?
There was a problem hiding this comment.
Mostly that rubocop was driving me nuts and I thought I'd fix it after and then forgot :)
test/stripe/payment_intent_test.rb
Outdated
|
|
||
| should "be saveable" do | ||
| stub_request(:get, "#{Stripe.api_base}/v1/payment_intents/pi_123") | ||
| .to_return(body: JSON.generate(id: "pi_123", object: "payment_intent", metadata: {})) |
There was a problem hiding this comment.
When testing an instance method, instead of stubbing the retrieve request then initializing the resource with .retrieve, I'd recommend just creating the resource directly with .new. This way only one request is sent which can clarify errors if assert_requested fails.
There was a problem hiding this comment.
I mostly mimicked what is in other tests though but will fix
remi-stripe
force-pushed
the
remi-add-payment-intent
branch
from
c625218
to
170dc98
Compare
|
@ob-stripe Fixed I think, PTAL |
test/stripe/payment_intent_test.rb
Outdated
|
|
||
| payment_intent = Stripe::PaymentIntent.construct_from(id: "pi_123", object: "payment_intent") | ||
| payment_intent = payment_intent.cancel | ||
| assert payment_intent.is_a?(Stripe::PaymentIntent) |
There was a problem hiding this comment.
You're missing the assert_requested in the 3 tests for the non-standard methods.
There was a problem hiding this comment.
Ah yes sorry. Fixed.
This feature is gated so the tests are stubbed for now
remi-stripe
force-pushed
the
remi-add-payment-intent
branch
from
170dc98
to
201f9c2
Compare
|
Released in 3.16.0. |
|
@ob-stripe Hello. Can you help me? I have "stripe (3.2.0)" at Gemfile.lock but if I call Stripe::PaymentIntent in rails console I get "NameError: uninitialized constant Stripe::PaymentIntent" |
This feature is gated so the tests are stubbed for now
r? @ob-stripe
cc @stripe/api-libraries
cc @michelle-stripe @jenan-stripe