Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement initial Stripe Checkout Server flow #6

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

diimpp
Copy link

@diimpp diimpp commented Sep 12, 2019

No description provided.

@diimpp diimpp mentioned this pull request Sep 12, 2019
Copy link
Owner

@lolmx lolmx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've a question about the status action and the template.
Also a feedback on the new gateway

*/
protected function populateConfig(ArrayObject $config)
{
if (false === class_exists(Stripe::class)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to check for PaymentIntent::class because this class has been added on 6.9 thus we force the minimal version.
Stripe::class is about to check if sdk is installed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stripe has const VERSION = '6.40.0'; so ultimately we should check against it. (Or at least I hope it appeared in earlier versions than 6.40).

StripeCheckoutServerGatewayFactory.php Outdated Show resolved Hide resolved
return;
}

if ($model['object'] === PaymentIntent::OBJECT_NAME && Constants::STATUS_REQUIRES_PAYMENT_METHOD == $model['status']) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if i'm not wrong Constants::STATUS_REQUIRES_PAYMENT_METHOD == $model['status'] when the authentication failed and customer needs to enter a new card, and Constants::STATUS_REQUIRES_ACTION == $model['status'] when it's okay but need authentication?

Copy link
Author

@diimpp diimpp Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'm kind of torn between requires_capture and requires_confirmation. It seem that requires_action is in-between state and authorized payment intent will have a requires_capture state.

If the selected payment method requires additional authentication steps, the PaymentIntent will transition to the requires_action status and suggest additional actions via next_action. If payment fails, the PaymentIntent will transition to the requires_payment_method status. If payment succeeds, the PaymentIntent will transition to the succeeded status (or requires_capture, if capture_method is set to manual).
https://stripe.com/docs/api/payment_intents/confirm

Constants.php Outdated Show resolved Hide resolved
], $config['payum.paths'] ?: []);
}

protected function parseStripeVersion(string $version): array
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lolmx I cannot think of more elegant solution right now. A solution to 6.9/7.0 difference can be to make this method public and static and check in all actions for current version.

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.

None yet

2 participants