RFC: Add handler for voiding pending payments

[peterberkenbosch]

What happens

When there is a confirmation step in the order flow, i.e. the payment is not the last step in the flow, there can be a situation where there are multiple pending payments for the order. This is blocking users to checkout with payment providers who only accept a single authorized payment (Affirm for example). Also, with this there is a possibility to have multiple pending payments on a creditcard. This will not block the user to finish the checkout, but does have a negative impact.

The problem

When working with payment providers (PSP) who need a redirect, for example Mollie, Affirm, Paypal etc. We currently create a Payment instance, redirect to the PSP and handle the response with a callback url. This usally is checking the payment state and amount with the PSP Api and redirect the user to the order complete or order confirm page.

When the payment is the last step of the order process, this has minimum impact. The payment is done and the order is marked as completed. There are edge cases known that also create multiple payments. Fixing that is not the main goal with the RFC, but the assumption is that this will has a positive impact on those edge cases too.

When the order has a confirmation step and the user changes the order and checks out again, we create another payment and redirection. This can cause problems with creditcards and Affirm (and alike) payments. The authorization of a credit card for example is usually a hold on the card. When checking out multiple times with multiple pending / authorized payments on a card can cause problems with the user's card balance.

related input: solidusio/solidus_affirm#58 (comment)

Solution

Introduce a (configurable) handler class that is responsible for handling canceling pending payments in the checkout process.

Working on a PR to have some code around this, want to have this RFC out here as well to have a discussion around this.

[aldesantis]

Would this be similar to the behavior in Spree::OrderCapturing? Or do you want to implement a per-gateway handler class?

[peterberkenbosch]

Similar indeed, not for capturing though. Only for making sure there are no "outdated" or potential duplicate payments. It's a bit tricky, but will figure it out :)

[rainerdema]

I think I had a similar but inherent multiple payment issues through Affirm and Spree::StoreCredit which can be useful to better understand the problem.
Consider the invalidate_old_payments method called with the after_create callback:

solidus/core/app/models/spree/payment.rb

Lines 38 to 39 in 9f99aa6

	# invalidate previously entered payments
	after_create :invalidate_old_payments

solidus/core/app/models/spree/payment.rb

Lines 202 to 208 in 9f99aa6

	def invalidate_old_payments
	if !store_credit? && !['invalid', 'failed'].include?(state)
	order.payments.select { |payment|
	payment.state == 'checkout' && !payment.store_credit? && payment.id != id
	}.each(&:invalidate!)
	end
	end

This method takes care of invalidating all previous payments in the checkout state made on the order, leaving out all store credits payments which are managed separately and are, by default, the only case handled which it's possible to have multiple payment methods at the same order in Solidus.
With Affirm, for example, it's possible to similarly invalidate all Affirm payments in a pending state on the order, after the new generic valid payment is created.

Specifically, with the store credits, after making the payment with Affirm (for example) and going to the confirm status:

solidus/core/app/models/spree/order/checkout.rb

Line 86 in e3be38d

before_transition to: :confirm, do: :add_store_credit_payments

This condition in the add_store_credit_payments method generates an exception since there is an Affirm payment in pending state, which with the new store credit payment, exceeds the order total amount:

solidus/core/app/models/spree/order.rb

Lines 686 to 688 in 9f99aa6

	if payments.where(state: %w(checkout pending completed)).sum(:amount) != total
	errors.add(:base, I18n.t('spree.store_credit.errors.unable_to_fund')) && (return false)
	end