From c71ae35685e563389480f526ef71018c0cf3530f Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 20 Jan 2025 16:07:32 +1100 Subject: [PATCH] Fix payment_total calculation For payment that complete during the checkout (Paypal, Stripe) the amount was recorded twice against `order.payment_total`. This is because the `payment_total` gets updated in an afer_save Payment callback when a payment is completed, and then once more when we process payment in `Spree::Order#process_each_payment`. This is an existing issue on master, but it was hidden by the `update_shipping_fees!` callback, it trigerred an update of the order's total, which then updated `order.payment_total` with the correct value. Now that we removed the callback, the bug showed up. Note, I updated the stripe specs for consistency even though they are currently disabled. --- app/models/spree/order.rb | 4 ---- app/views/spree/orders/_totals_footer.html.haml | 2 +- spec/models/spree/order_spec.rb | 9 --------- spec/system/consumer/shopping/checkout_paypal_spec.rb | 1 + spec/system/consumer/shopping/checkout_stripe_spec.rb | 2 ++ 5 files changed, 4 insertions(+), 14 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index a365693d0e..17ffe51700 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -673,10 +673,6 @@ module Spree break if payment_total >= total yield payment - - if payment.completed? - self.payment_total += payment.amount - end end end diff --git a/app/views/spree/orders/_totals_footer.html.haml b/app/views/spree/orders/_totals_footer.html.haml index 7319869eb9..223524e676 100644 --- a/app/views/spree/orders/_totals_footer.html.haml +++ b/app/views/spree/orders/_totals_footer.html.haml @@ -29,7 +29,7 @@ %td.text-right{colspan: "3"} %strong = t :order_amount_paid - %td.text-right.total + %td.text-right.total{id: "amount-paid"} %strong = order.display_payment_total.to_html - if order.outstanding_balance.positive? diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e70458b759..f10c1f4f80 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -294,15 +294,6 @@ RSpec.describe Spree::Order do expect(payment).to receive(:process!) expect(order.process_payments!).to be_truthy end - - it "stores the payment total on the order" do - allow(payment).to receive(:process!) - allow(payment).to receive(:completed?).and_return(true) - - order.process_payments! - - expect(order.payment_total).to eq(payment.amount) - end end context "when a payment raises a GatewayError" do diff --git a/spec/system/consumer/shopping/checkout_paypal_spec.rb b/spec/system/consumer/shopping/checkout_paypal_spec.rb index 98947d099c..93f2fdbabe 100644 --- a/spec/system/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/system/consumer/shopping/checkout_paypal_spec.rb @@ -69,6 +69,7 @@ RSpec.describe "Check out with Paypal" do click_on "Complete order" expect(page).to have_content "Your order has been processed successfully" + expect(page.find("#amount-paid").text).to have_content "$19.99" expect(order.reload.state).to eq "complete" expect(order.payments.count).to eq 1 diff --git a/spec/system/consumer/shopping/checkout_stripe_spec.rb b/spec/system/consumer/shopping/checkout_stripe_spec.rb index 7758f582db..f1974fd5af 100644 --- a/spec/system/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/system/consumer/shopping/checkout_stripe_spec.rb @@ -71,6 +71,8 @@ RSpec.describe "Check out with Stripe" do checkout_with_stripe expect(page).to have_content "Confirmed" + expect(page.find("#amount-paid").text).to have_content "$19.99" + expect(order.reload.completed?).to eq true expect(order.payments.first.state).to eq "completed" end