Support preauth and refunds on PaymentIntents #224
Conversation
bpcreech
force-pushed
the
feat/payment-intent-preauth
branch
2 times, most recently
from
9aad34f
to
3777c29
Compare
bpcreech
changed the title
|
Hello @bpcreech, Thanks for contributing upstream! I have to point out that this is a high-quality pull request with clean code, explanation and tests. The refactor of Do you see any points of attention we should look into before merging this? |
Before this commit, localstripe already supported Charges with pre-auths and refunds, and it supported PaymentIntents without pre-auths or refunds. Since PaymentIntents are either preferred or supported for most APIs now, let's support pre-auths on PaymentIntents! Since the PaymentIntent implementation in localstripe just proxies most stuff onto Charge objects, we can proxy pre-auths onto them too. Same for refunds. We are already using this at Via (ridewithvia.com) to test our Stripe integration. The tests are a big copy-paste-modify on the Charges tests, which already seemed pretty comprehensive... and we add tests for refunds on both Charges and PaymentIntents.
bpcreech
force-pushed
the
feat/payment-intent-preauth
branch
from
3777c29
to
bb05041
Compare
|
Thanks @adrienverge, and thanks very much for creating and open-sourcing localstripe! It let us retire a far less confidence-building mock we had for our Stripe integration. :) I updated the git commit message to match the PR. I don't know of thing to look into. We're using this with a pretty extensive test suite (717 total tests!) which runs the very same tests against the Stripe test API, so I have some confidence that it is an accurate simulation of the surface area we use. (That said, there's lots more to the API and surely areas we aren't simulating right yet.) The part where we (continue to) proxy to the Charges implementation seems like it might be regrettable some day but I don't understand enough of the nuance to know how an implementation specialized for PaymentIntents would be any different. |
|
OK 👍 Thanks a lot @bpcreech! Let's release it. |
localstripe already supports Charges with pre-auths and refunds, and it supports PaymentIntents without pre-auths or refunds.
Since PaymentIntents are either preferred or supported for most APIs now, let's support pre-auths on PaymentIntents! Since the PaymentIntent implementation in localstripe just proxies most stuff onto Charge objects, we can proxy pre-auths onto them too. Same for refunds.
We are already using this at Via (ridewithvia.com) to test our Stripe integration.
The tests are a big copy-paste-modify on the Charges tests, which seem pretty comprehensive... and we add tests for refunds on both Charges and PaymentIntents.