Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct behavior of Order#outstanding_balance #1557

Closed
wants to merge 1 commit into from

Conversation

stewart
Copy link
Contributor

@stewart stewart commented Oct 24, 2016

This commit addresses an issue in Spree::Order#outstanding_balance, in
circumstances where an Order has had refunds applied.

The existing behaviour cancels out the amount of refunded payments,
resulting in a negative outstanding balance and an order state of
'credit_due', when in fact the order balance is zero. This is caused by
the calculation of payment_total already factoring in refunds.

Some more discussion about this behaviour can be found in the following
issues:

#1107
#1536

With these changes, outstanding_balance is now defined as
total - payment_total. This corrects the behaviour around refunds.

Tests have been updated accordingly, with some help from
DanielePalombo's commit:

c851404

This commit addresses an issue in `Spree::Order#outstanding_balance`, in
circumstances where an Order has had refunds applied.

The existing behaviour cancels out the amount of refunded payments,
resulting in a negative outstanding balance and an order state of
'credit_due', when in fact the order balance is zero. This is caused by
the calculation of `payment_total` already factoring in refunds.

Some more discussion about this behaviour can be found in the following
issues:

solidusio#1107
solidusio#1536

With these changes, `outstanding_balance` is now defined as
`total - payment_total`. This corrects the behaviour around refunds.

Tests have been updated accordingly, with some help from
DanielePalombo's commit:

solidusio@c851404
Copy link
Member

@gmacdougall gmacdougall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed the behaviour in a sandbox. The code is looking good as well.

Thanks Andrew. 👍

@jhawthorn
Copy link
Contributor

This seems like a good change, but this reopens the issue which caused this in the first place, spree/spree#6229.

To summarize:

  1. From a completed, paid, shipped order
  2. Create an RMA
  3. Create a Customer Return for that RMA.
  4. Create a reimbursement

This leaves an order in the "balance due" state, which is wrong. We are not expecting more money from the customer since they have returned their item. Something needs to reflect this before this can be merged.

@jordan-brough
Copy link
Contributor

Please also see #1254

@stewart stewart closed this Dec 21, 2016
@stewart stewart deleted the fix/outstanding_balance branch December 21, 2016 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants