Allow cancelling orders that have been fully refunded

[Sinetheta]

This fixes #1066 but I could use some feedback.

After an order is cancelled, all completed payments are also cancelled here. Each payment then tries to call cancel on it's payment method here, the implementation of which is left to the provider here.

I can't speak for the other gateways, but Braintree's cancel looks like this in solidus_gateway

def cancel(response_code)
  provider
  transaction = ::Braintree::Transaction.find(response_code)
  # "A transaction can be refunded if its status is settled or settling.
  # If the transaction has not yet begun settlement, it should be voided instead of refunded.
  if transaction.status == Braintree::Transaction::Status::SubmittedForSettlement
    provider.void(response_code)
  else
    provider.refund(response_code)
  end
end

The reason why it's sufficient to simply check whether a payment is "settled" or not, is because the Braintree provider docs say that

If you do not specify an amount to refund, the entire transaction amount will be refunded.

and

If you have already partially refunded a transaction, performing another refund without specifying the balance it will create a refund for the remaining non-refunded amount of the transaction.

but there's one more case, where a payment has already been fully refunded (through one or many refunds). In that case we get an error. This error is what causes the order cancellation to fail.

We could address this anywhere along the chain, but here are my thoughts.

1. Not in the Braintree ruby SDK, I can understand how a failure to create any refund could be considered an error even if a partial refund is not.
2. Not in solidus_gateway, because we don't currently pass the payment when calling cancel on the gateway and I'm not sure we should.

Any ideas how I might add a regression test for this? The behaviour seems specific to one (albeit an important) gateway.

[peterberkenbosch]

should == instead of >= be sufficient? If you refunded more then the amount paid there are some other issues going on IMHO.

[Sinetheta]

Fair enough. I don't like when we try to catch problems tangentially, but in that case we would raise an error during a cancel, which would probably be useful. I'm cool with swicthing to ==.

[peterberkenbosch]

This makes a lot of sense, curious why we do not have a payment#refunded state though.

[gmacdougall]

I agree that this makes sense. I can't think of anywhere else I would prefer to deal with this.

👍

[seantaylor]

Also ran into this issue on a store. 👍 for getting this in. Seems reasonable to me.

[Senjai]

👍 @jhawthorn any opinions?

[jarednorman]

@Sinetheta Thoughts on testing this?

[mamhoff]

I think we can save a DB lookup here by using the in-memory refunds:

refunds.map(&:amount).sum == amount

[gmacdougall]

👍

[cbrunsdon]

We decided to take this as is, it might have been an improvement to do the check during cancel! instead of doing it in the after_cancel, but we agree this is an improvement as is.