diff --git a/CHANGELOG.md b/CHANGELOG.md index 1413f510f79..18fbf2ef477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Solidus 2.3.0 (master, unreleased) +- Order#outstanding_balance now uses reimbursements instead of refunds to calculate the amount that should be paid on an order. [#2002](https://github.com/solidusio/solidus/pull/2002) (many contributors :heart:) + - Renamed `Spree::Gateway` payment method into `Spree::PaymentMethod::CreditCard` [\#2001](https://github/com/solidusio/solidus/pull/2001) ([tvdeyen](https://github.com/tvdeyen)) Run `rake solidus:migrations:rename_gateways:up` to migrate your data. diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 5a1f890b55f..90378a61310 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -364,15 +364,18 @@ def create_tax_charge! end deprecate create_tax_charge!: :update!, deprecator: Spree::Deprecation + def reimbursement_total + reimbursements.sum(:total) + 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 - reimbursement_total - payment_total end end diff --git a/core/lib/spree/testing_support/factories/payment_factory.rb b/core/lib/spree/testing_support/factories/payment_factory.rb index c88f78e193c..40aea9806a9 100644 --- a/core/lib/spree/testing_support/factories/payment_factory.rb +++ b/core/lib/spree/testing_support/factories/payment_factory.rb @@ -11,6 +11,10 @@ state 'checkout' response_code '12345' + trait :completed do + state 'completed' + end + trait :failing do response_code '00000' association(:source, :failing, { factory: :credit_card }) diff --git a/core/spec/models/spree/order/outstanding_balance_integration_spec.rb b/core/spec/models/spree/order/outstanding_balance_integration_spec.rb new file mode 100644 index 00000000000..1dce444b839 --- /dev/null +++ b/core/spec/models/spree/order/outstanding_balance_integration_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +# This method in particular has been difficult to get right. +# Many things will affect this amount +# +# See also: +# https://github.com/solidusio/solidus/issues/1254 +# https://github.com/spree/spree/issues/6229 +# https://github.com/solidusio/solidus/issues/1107 +# https://github.com/solidusio/solidus/pull/1557 +# https://github.com/solidusio/solidus/pull/1536 + +RSpec.describe "Outstanding balance integration tests" do + let!(:order) { create(:order_with_line_items, line_items_count: 2, line_items_price: 3, shipment_cost: 13) } + before { order.update_attributes!(state: 'complete', completed_at: Time.now) } + + subject do + order.reload + order.update! + order.outstanding_balance + end + + context 'when the order is unpaid' do + it { should eq order.total } + it { should eq 19 } + + context 'when the order is cancelled' do + before { order.cancel! } + it { should eq 0 } + end + end + + context 'when the order is fully paid' do + let!(:payment) { create(:payment, :completed, order: order, amount: order.total) } + it { should eq 0 } + + context 'and there is a full refund' do + let!(:refund) { create(:refund, payment: payment, amount: payment.amount) } + it { should eq 19 } + end + + context 'when the order is cancelled' do + before { order.update_attributes!(state: "canceled") } + it { should eq(-19) } + + context 'and the payment is voided' do + before { payment.update_attributes!(state: "void") } + it { should eq 0 } + end + + context 'and there is a full refund' do + let!(:refund) { create(:refund, payment: payment, amount: payment.amount) } + it { should eq 0 } + end + + context 'and there is a partial refund' do + let!(:refund) { create(:refund, payment: payment, amount: 6) } + it { should eq(-13) } + end + end + + context 'with a cancelled item' do + let(:cancelations) { Spree::OrderCancellations.new(order) } + let(:cancelled_item) { order.line_items.first } + + before do + # Required to refund + Spree::RefundReason.create!(name: Spree::RefundReason::RETURN_PROCESSING_REASON, mutable: false) + + cancelations.cancel_unit(cancelled_item.inventory_units.first) + cancelations.reimburse_units(cancelled_item.inventory_units) + + order.reload + end + + it 'discounts the cancelled item amount' do + expect(order.refund_total).to eq(3) + expect(order.reimbursement_total).to eq(3) + expect(order.payment_total).to eq(16) + expect(order.outstanding_balance).to eq(0) + + expect(order.total).to eq(19) + end + end + end + + context 'when the order is partly paid' do + let!(:payment) { create(:payment, :completed, order: order, amount: 10) } + it { should eq 9 } + + context 'and there is a full refund' do + let!(:refund) { create(:refund, payment: payment, amount: payment.amount) } + it { should eq 19 } + end + + context 'when the order is cancelled' do + before { order.update_attributes!(state: "canceled") } + it { should eq(-10) } + + context 'and the payment is voided' do + before { payment.update_attributes!(state: "void") } + it { should eq 0 } + end + + context 'and there is a full refund' do + let!(:refund) { create(:refund, payment: payment, amount: payment.amount) } + it { should eq 0 } + end + + context 'and there is a partial refund' do + let!(:refund) { create(:refund, payment: payment, amount: 6) } + it { should eq(-4) } + end + end + end +end diff --git a/core/spec/models/spree/order/payment_spec.rb b/core/spec/models/spree/order/payment_spec.rb index 29fe21c66d4..2c1daa9b64d 100644 --- a/core/spec/models/spree/order/payment_spec.rb +++ b/core/spec/models/spree/order/payment_spec.rb @@ -182,8 +182,13 @@ module Spree context "for canceled orders" do before { order.update_attributes(state: 'canceled') } - it "it should be a negative amount incorporating reimbursements" do - expect(order.outstanding_balance).to eq(-10) + it "it should be zero" do + expect(order.total).to eq(110) + expect(order.payments.sum(:amount)).to eq(10) + expect(order.refund_total).to eq(10) + expect(order.reimbursement_total).to eq(10) + expect(order.payment_total).to eq(0) + expect(order.outstanding_balance).to eq(0) end end diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 52959074e2a..08ea4e207bc 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -206,6 +206,21 @@ end end + context '#outstanding_balance' do + let(:order) { create(:order_ready_to_ship, line_items_count: 3) } + let(:payment) { order.payments.first } + + it "should handle refunds properly" do + order.cancellations.short_ship([order.inventory_units.first]) + expect(order.outstanding_balance).to be_negative + expect(order.payment_state).to eq('credit_owed') + create(:refund, amount: order.outstanding_balance.abs, payment: payment, transaction_id: nil) + order.reload + expect(order.outstanding_balance).to eq(0) + expect(order.payment_state).to eq('paid') + end + end + context "#display_outstanding_balance" do it "returns the value as a spree money" do allow(order).to receive(:outstanding_balance) { 10.55 }