Stripe v2 (#906)
PaymentStripeV2 is ready for testing. Please keep in mind that the schema changes, which are already included in the change-sets, have to be applied to the database.
Hi @fhebel Thank you for the great work. While reviewing, there are some items I would like to understand more
What is the purpose of PaymentAttempt? To keep track the Stripe SessionId? I think we can use the existing Payment object instead (We should add an additional status call InProgress or Pending). In the StripeV2Controller --> CreateCheckoutSession, we should create the order (with status PendingPayment) and the payment object there. In the Payment table there is an column called GatewayTransactionId, we can keep the Stripe SessionId there.
General rule: We should create the order (with PendingPayment status) before the payment being charged. If the payment success we update the status of order, if it fail for any reasons then if we can catch the failure, we will update the order status, if not there is a background service to cancel the order with PendingPayment status after 5 minutes. Creating order first will make sure that the products, coupon,.. are in stock, and it prevent the situation that the payment is charged but we cannot create order.
Consider to use webhook also https://stripe.com/docs/webhooks/signatures
Hi @thiennn,
Thank you, that you found time to start the review!
In the StripeV2Controller --> CreateCheckoutSession, we should create the order (with status PendingPayment) and the payment object there. In the Payment table there is an column called GatewayTransactionId, we can keep the Stripe SessionId there.
The reason why I chose not to create the order in CreateCheckoutSession was, that if the user decides to click the back button (which hits CallbackStripePaymentFailure) during the ongoing payment on the stripe.com site (e.g. the user finds out that he doesn't have his token / cell phone for Strong Customer Authentication at hand), then the order is already created and the customer doesn't have the chance to choose another payment method.
Do you have an idea how we can create an order during CreateCheckoutSession avoiding the above mentioned problem?
What is the purpose of PaymentAttempt? To keep track the Stripe SessionId?
For my purposes I had the following requirements:
- Keep track of the Stripe SessionId and its respective environment (dev/test/qa/prod)
- Persist the requested amount to be able to check whether the payment status is partially paid or fully paid
- To have something like an audit log whenever the PaymentAttempt / Session was processed (see following example)
- A flag whether the payment has reached its final status
Example of a successful audit log (stored in AdditonalInformation of the PaymentAttempt)
[
{
"CreatedOn": "2020-07-24T15:49:06.907611+02:00",
"Component": "CreateCheckoutSession",
"Message": "Creating payment attempt for UserId=10 ..."
},
{
"CreatedOn": "2020-07-24T15:49:20.977107+02:00",
"Component": "CallbackStripePaymentSuccess",
"Message": "Received CallbackStripePaymentSuccess for sessionId=cs_test_rL3aX3zfIBNny6lqeehJ7tT56s4fdlKL2ZH8V1taU1tN8cgJQmvyF6VJ (paymentAttemptId=21) ..."
},
{
"CreatedOn": "2020-07-24T15:49:22.053715+02:00",
"Component": "CreateOrderForUser",
"Message": "Successfully created Order with Id=11."
}
]
or a failed attempt
[
{
"CreatedOn": "2020-07-24T15:59:39.979326+02:00",
"Component": "CreateCheckoutSession",
"Message": "Creating payment attempt for UserId=10 ..."
},
{
"CreatedOn": "2020-07-24T15:59:52.427711+02:00",
"Component": "CallbackStripePaymentFailure",
"Message": "Received CallbackStripePaymentFailure. Finalizing PaymentAttempt."
}
]
Would you allow me to move the necessary columns to the Payments_Payment table or do you have a better proposal?
Thanks in advance for all your efforts!
Best regards,
Florian
PS: Thanks a lot for the hint regarding the signed webhooks, I will definitely integrate them!
@thiennn this is take too long, any updates on this?
@fhebel is this PR functional with the latest bits?
Hi @hishamco,
I don’t think that this PR is still viable, because as far as I remember it contained an ef migration.
Because I left my home country I gave up my ecommerce business and for the moment I even don’t have a working dev environment available …
So I think we should close the PR.
Best regards,
Florian
Thanks @fhebel