Incorrect payment_state when using refunds

[fredericboivin]

Issue

outstanding_balance from order.rb

def outstanding_balance
   # If reimbursement has happened add it back to total to prevent balance_due payment state
   # See: https://github.com/spree/spree/issues/6229
   adjusted_payment_total = payment_total + refund_total

   if state == 'canceled'
     -1 * adjusted_payment_total
   else
     total - adjusted_payment_total
   end
end

It's used by the order_updater to determine the payment state of an order :

outstanding_balance	state
outstanding_balance > 0	balance_due
outstanding_balance < 0	credit_owed
outstanding_balance = 0	paid

History

Current behavior comes from this spree issue

Basically, if you refund something on a completed order, instead of being paid, it was balance_due. So this fix this, outstanding_balance takes into account the refunds

Scenario

1. User place an order for a total of 25$

2. Admin captures the payment

3. For any reason (error in the order, need to use another payment method or credit card), you have the refund the payment

You know have an order with a "paid" payment_state, but without having any valid, completed payment at all

Outstanding balance will do the following calculation : 25 [total] - ( 0 [payment_total] + 25 [refund_total])

So at this point, the order will forever be in a "wrong state" because you can't add new payments (since outstanding balance is 0). You will be able to process and ship it, even though it's not paid for.
If you add an item or edit it in any way, you will be only be able to add and capture payments for the newly added items.

If we take the above order and add a 10$ mug, we will be able to add a 10$ payment, and if we capture it, we'll get:

35 [total] - ( 10 [payment_total] + 25 [refund_total]), so we're still short of 25$

The issue is compounded with certain gateways like Moneris where you can't void or cancel a payment, and thus you need to refund+add a new one if you need to change it.

Problem

The behavior was added to Solidus with the following commit with the following commit 84eddb501822f2602ed7542def20278db0714223 and is visible from 1.1 onwards

outstanding_balance has a different behavior depending on state and from my perspective, it feels like a stretch to include refunds in the calculation (too much responsability, it should do one thing : gives the balance which logically is total - payment ) considering it's breaking one side of the order process to fix another.

It looks like it's not causing much problems for now since it wasn't reported, I can see how it's heavily dependent on individual store workflow, and there might be some use cases that I'm not considering.

From my side it's a major pain point since we do a lot a phone orders for older, less tech savy clients and Moneris force us to do a lot of refunds.

If the surrounding issue is only that completed orders are showing as balance_due in the admin panel after refunds, there's probably a better way to handle it by correcting the total so it's still paid (one-time promotions to apply free shipping, using manual adjustments to account for defects or fees or discretionary discounts, proper use of rma/returns/reimbursements)

So before tackling this, I'd like someone else perspective. Comments and suggestions are very welcome.

The less disruptive refactoring I'd see is adding an additionnal payment_state (refunded?) to more explicitly handle the state of a finished order with arbitrary refunds.

[fredericboivin]

Related to issue #1254