solidusio / solidus

🛒 Solidus, the open-source eCommerce framework for industry trailblazers.
https://solidus.io
Other
4.98k stars 1.29k forks source link

Implement a payment capture events GUI #2032

Closed tvdeyen closed 2 years ago

tvdeyen commented 7 years ago

Following these discussions

we are missing an interface for capturing partial payments (Spree::PaymentCaptureEvent)

So, @moritzps, @pascalj and @rbjoern84 from @bitspire started a discussion on Slack about implementing one.

This is the original screenshot of the UI they proposing

solidus_multiple_captures_v2

TODO

tvdeyen commented 7 years ago

One thing I discussed with @pascalj IRL was to add a form for capturing partial payments to the payments detail page. Introducing a modal (we did not finally decided to have in our UI now) for this rarely used feature seems overkill for us.

Payment Detail page with capture events

payments - r987654321 - orders

We need to keep in mind that payments could be uncaptured and therefor do not have an Spree::PaymentCaptureEvent yet. But we still have the payments detail page, so we could still display the form and pre-fill the amount into the field

Payment Detail page without capture events

credit card pending - payments - r123456789 - orders 2017-06-20 09-23-57

The form should not be displayed if no money can be captured anymore (aka. the full amount of the payment is reached)

pascalj commented 7 years ago

After going through some commits and reading the comment at processing.rb I thought everything was in place for partial captures. Unfortunately the feature was removed shortly after it was introduced, rendering the comment outdated/wrong.

I cannot find a belonging PR, but I guess this stems from the difference between:

I'd gladly add a new method to the controller to get around that, but the transition to completed is done in the payment (Processing#capture!) itself. After a payment is completed there cannot be any more captures.

Adjusting the capture method even more with unknown side effects will probably be reverted again in the future, just as before. So I'd like to add a partial_capture! method which emits a capture event and leaves the payment in its current state until fully captured. Another option would be to split the payment as before. Any thoughts/preferences on that?

pascalj commented 7 years ago

I'd like to get some feedback on the general behaviour of capturing. For some code that allows partial capturing see #2039.

What option do you prefer?

  1. Add a new method inside of the payment processing a la partial_capture!(amount)?
  2. Don't change the amount any more (kind of what #2039 does) and change the specs in that regard. Could be a braking change, but it's definitely an edge case.
  3. "Force" payments to completed after their order was completed, no matter what their respective uncaptured_amount is.
  4. None of the above, but <thing>?

Any feedback is welcome!

tvdeyen commented 7 years ago

We will discuss this in the next core team meeting. Thanks for the write up

jhawthorn commented 7 years ago

Is there a popular gateway which supports multiple captures? Certainly they all support partial captures, but I don't know of any which support multiple captures (other than by making multiple charges against a stored credit card)

pascalj commented 7 years ago

Absolutely, it's not that common. Examples are Paypal and Amazon Pay which both work with only one authorization.

This amount cannot exceed the original amount that was authorized less any previously captured amount on this authorization

pascalj commented 7 years ago

Copying @cbrunsdon's response from #2039 so it's all in one place:

I definitely think the change is a good direction (multiple captures when payment gateways support it), but I'm worried that this is going to be a big change that everyone will get, even when multiple capture is not supported by payment gateways.

I do however think this is a very reasonable, and not too intrusive for an extension, would you consider supporting it in that way? If it lives there for a while and the kinks get worked out, I would be happy to re-examine it for core in a future release.

Yes, I can see why touching that part of Solidus could cause problems, especially for third party code. I'll try to ship it as an extension - this would potentially allow to use it with older Solidus versions, too. Without overriding anything it'll:

Thanks for all your feedback!