Skip to content

Commit

Permalink
Correct behavior of Order#outstanding_balance
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stewart committed Oct 24, 2016
1 parent 856344b commit c8fe989
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 44 deletions.
8 changes: 2 additions & 6 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -353,14 +353,10 @@ def create_tax_charge!
end

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
-1 * payment_total
else
total - adjusted_payment_total
total - payment_total
end
end

Expand Down
71 changes: 33 additions & 38 deletions core/spec/models/spree/order/payment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,48 +154,43 @@ module Spree
it { is_expected.to be_truthy }
end

context "#outstanding_balance" do
it "should return positive amount when payment_total is less than total" do
order.payment_total = 20.20
order.total = 30.30
expect(order.outstanding_balance).to eq(10.10)
end
it "should return negative amount when payment_total is greater than total" do
order.total = 8.20
order.payment_total = 10.20
expect(order.outstanding_balance).to be_within(0.001).of(-2.00)
end

context "with reimburesements on the order" do
let(:amount) { 10.0 }
let(:reimbursement) { create(:reimbursement) }
let(:order) { reimbursement.order.reload }

before do
# Set the payment amount to actually be the order total of 110
reimbursement.order.payments.first.update_column :amount, amount
# Creates a refund of 110
create :refund, amount: amount,
payment: reimbursement.order.payments.first,
reimbursement: reimbursement
# Update the order totals so payment_total goes to 0 reflecting the refund..
order.update!
end
context "with reimburesements on the order" do
let(:amount_paid) { 20.0 }
let(:amount_refunded) { 10.0 }
let(:order) { create(:order_with_line_items) }
let(:reimbursement) { create(:reimbursement) }
let(:payment) do
create(:payment, amount: amount_paid, state: :completed, order: order)
end

before do
reimbursement.order = order

context "for canceled orders" do
before { order.update_attributes(state: 'canceled') }
create(:refund, {
amount: amount_refunded,
payment: payment,
reimbursement: reimbursement
})

it "it should be a negative amount incorporating reimbursements" do
expect(order.outstanding_balance).to eq(-10)
end
# Invoke OrderUpdater to update payment_total
order.update!
end

context "for canceled orders" do
before { order.update_attributes(state: 'canceled') }

it "it should be a negative amount incorporating reimbursements" do
# -1 * (Payment Total + Reimbursed)
# -1 * (20 - 10) = -10
expect(order.outstanding_balance).to eq(-10)
end
end

context "for non-canceled orders" do
it 'should incorporate refund reimbursements' do
# Order Total - (Payment Total + Reimbursed)
# 110 - (0 + 10) = 100
expect(order.outstanding_balance).to eq 100
end
context "for non-canceled orders" do
it 'should incorporate refund reimbursements' do
# Order Total - (Payment Total + Reimbursed)
# 110 - (20 - 10) = 100
expect(order.outstanding_balance).to eq 100
end
end
end
Expand Down

0 comments on commit c8fe989

Please sign in to comment.