From 9df390233adc0a8e02c33677cfe2a1adf3d20187 Mon Sep 17 00:00:00 2001 From: irisfaraway Date: Tue, 4 Sep 2018 17:53:45 +0100 Subject: [PATCH] Deal with non-WorldPay payments when updating balance https://eaflood.atlassian.net/browse/WC-377 The `finance_details.update_balance` method previously just used payments with the correct `world_pay_payment_status`. However, payments by other means will never have this value. This commit changes the method to always count non-Worldpay payments when calculating the balance, while still only using WorldPay payments which have been authorised. --- .../waste_carriers_engine/finance_details.rb | 4 +++- spec/factories/payment.rb | 7 +++++++ .../finance_details_spec.rb | 21 ++++++++++++++----- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/app/models/waste_carriers_engine/finance_details.rb b/app/models/waste_carriers_engine/finance_details.rb index bbd20490b..f5f15a940 100644 --- a/app/models/waste_carriers_engine/finance_details.rb +++ b/app/models/waste_carriers_engine/finance_details.rb @@ -26,7 +26,9 @@ def self.new_finance_details(transient_registration, method, current_user) def update_balance order_balance = orders.sum { |item| item[:total_amount] } - payment_balance = payments.where(world_pay_payment_status: "AUTHORISED").sum { |item| item[:amount] } + # Select payments where the type is not WORLDPAY, or if it is, the status is AUTHORISED + payment_balance = payments.any_of({ :payment_type.ne => "WORLDPAY" }, + { world_pay_payment_status: "AUTHORISED" }).sum { |item| item[:amount] } self.balance = order_balance - payment_balance end end diff --git a/spec/factories/payment.rb b/spec/factories/payment.rb index 739601abd..05fe3f506 100644 --- a/spec/factories/payment.rb +++ b/spec/factories/payment.rb @@ -1,4 +1,11 @@ FactoryBot.define do factory :payment, class: WasteCarriersEngine::Payment do + trait :worldpay do + payment_type { "WORLDPAY" } + end + + trait :bank_transfer do + payment_type { "BANKTRANSFER" } + end end end diff --git a/spec/models/waste_carriers_engine/finance_details_spec.rb b/spec/models/waste_carriers_engine/finance_details_spec.rb index 42fb26ccd..cdcb32933 100644 --- a/spec/models/waste_carriers_engine/finance_details_spec.rb +++ b/spec/models/waste_carriers_engine/finance_details_spec.rb @@ -46,9 +46,9 @@ module WasteCarriersEngine expect(finance_details.balance).to eq(10_000) end - context "when there is also a payment" do + context "when there is also a WorldPay payment" do before do - finance_details.payments = [build(:payment, amount: 5_000, world_pay_payment_status: "AUTHORISED")] + finance_details.payments = [build(:payment, :worldpay, amount: 5_000, world_pay_payment_status: "AUTHORISED")] end it "should have the correct balance" do @@ -57,9 +57,9 @@ module WasteCarriersEngine end end - context "when the payment is not authorised" do + context "when the WorldPay payment is not authorised" do before do - finance_details.payments = [build(:payment, amount: 5_000, world_pay_payment_status: "REFUSED")] + finance_details.payments = [build(:payment, :worldpay, amount: 5_000, world_pay_payment_status: "REFUSED")] end it "should not include it when calculating the balance" do @@ -67,11 +67,22 @@ module WasteCarriersEngine expect(finance_details.balance).to eq(10_000) end end + + context "when the payment is non-WorldPay" do + before do + finance_details.payments = [build(:payment, :bank_transfer, amount: 5_000)] + end + + it "should have the correct balance" do + finance_details.update_balance + expect(finance_details.balance).to eq(5_000) + end + end end context "when there is a payment only" do before do - finance_details.payments = [build(:payment, amount: 5_000, world_pay_payment_status: "AUTHORISED")] + finance_details.payments = [build(:payment, :worldpay, amount: 5_000, world_pay_payment_status: "AUTHORISED")] end it "should have the correct balance" do