diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index c296556c716..66bfdb3db33 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -106,7 +106,6 @@ def states before_validation :clone_billing_address, if: :use_billing? before_validation :ensure_customer - before_save :update_shipping_fees!, if: :complete? before_save :update_payment_fees!, if: :complete? before_create :link_by_email @@ -672,10 +671,6 @@ def process_each_payment 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 7319869eb9f..223524e6763 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/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index d8d464fcb05..3353dcb4471 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -83,13 +83,9 @@ module Admin let!(:variant) { create(:variant, price: 10.0) } before do - allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true) - order.contents.add(variant) - order.payments << create(:payment, order:, amount: order.total) - order.reload + order.payments << create(:payment, :completed, order:, amount: order.total) - order.process_payments! order.update_attribute(:state, 'canceled') end diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index 2e3af6e9536..a3527849ba6 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -15,13 +15,13 @@ module Spree create(:credit_card) } let(:payment1) { - create(:payment, amount: 50, payment_method:, source:, response_code: "12345") + create(:payment, order:, amount: 50, payment_method:, source:, response_code: "12345") } let(:payment2) { - create(:payment, amount: 50, payment_method:, source:, response_code: "12345") + create(:payment, order:, amount: 50, payment_method:, source:, response_code: "12345") } let(:payment3) { - create(:payment, amount: 50, payment_method:, source:, response_code: "12345") + create(:payment, order:, amount: 50, payment_method:, source:, response_code: "12345") } let(:failed_payment) { create(:payment, amount: 50, state: 'failed', payment_method:, source:, diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb index d3a55fcf8b8..19788fbeb81 100644 --- a/spec/models/spree/order_contents_spec.rb +++ b/spec/models/spree/order_contents_spec.rb @@ -38,6 +38,27 @@ expect(order.item_total.to_f).to eq 19.99 expect(order.total.to_f).to eq 19.99 end + + context "with a completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.add(variant, 1) + end + end + + context "when passing a shipment" do + let!(:order) { create(:order_with_line_items) } + + it "updates shipping fees" do + shipment = order.shipments.first + expect(order).to receive(:update_shipping_fees!) + + subject.add(variant, 1, shipment) + end + end end context "#remove" do @@ -86,6 +107,27 @@ expect(order.item_total.to_f).to eq 19.99 expect(order.total.to_f).to eq 19.99 end + + context "with a completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.remove(order.line_items.first.variant, 1) + end + end + + context "when passing a shipment" do + let!(:order) { create(:order_with_line_items) } + + it "updates shipping fees" do + shipment = order.reload.shipments.first + expect(order).to receive(:update_shipping_fees!) + + subject.remove(order.line_items.first.variant, 1, shipment) + end + end end context "#update_cart" do @@ -126,6 +168,16 @@ expect(subject.order).to receive(:ensure_updated_shipments) subject.update_cart params end + + context "with a completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.update_cart params + end + end end describe "#update_item" do @@ -163,6 +215,16 @@ subject.update_item(line_item, { quantity: 3 }) end + + context "with a completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.update_item(order.line_items.first, { quantity: 3 }) + end + end end describe "#update_or_create" do @@ -181,6 +243,16 @@ subject.update_or_create(variant, { quantity: 2, max_quantity: 3 }) end + + context "with completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.update_or_create(variant, { quantity: 2, max_quantity: 3 }) + end + end end describe "updating" do @@ -198,6 +270,16 @@ subject.update_or_create(variant, { quantity: 3, max_quantity: 4 }) end + + context "with completed order" do + let!(:order) { create(:completed_order_with_totals) } + + it "updates shipping fees" do + expect(order).to receive(:update_shipping_fees!) + + subject.update_or_create(variant, { quantity: 3, max_quantity: 4 }) + end + end end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 5abc19da12d..72326e8dfe8 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -280,40 +280,31 @@ end end - context "#process_payments!" do + describe "#process_payments!" do let(:payment) { build(:payment) } before { allow(order).to receive_messages pending_payments: [payment], total: 10 } - it "should return false if no pending_payments available" do + it "returns false if no pending_payments available" do allow(order).to receive_messages pending_payments: [] expect(order.process_payments!).to be_falsy end context "when the processing is sucessful" do - it "should process the payments" do + it "processes the payments" 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 before { expect(payment).to receive(:process!).and_raise(Spree::Core::GatewayError) } - it "should return true when configured to allow checkout on gateway failures" do + it "returns true when configured to allow checkout on gateway failures" do Spree::Config.set allow_checkout_on_gateway_error: true expect(order.process_payments!).to be_truthy end - it "should return false when not configured to allow checkout on gateway failures" do + it "returns false when not configured to allow checkout on gateway failures" do Spree::Config.set allow_checkout_on_gateway_error: false expect(order.process_payments!).to be_falsy end @@ -1211,76 +1202,72 @@ end end - describe "a completed order with shipping and transaction fees" do + describe "#update_shipping_fees!" do + let(:distributor) { create(:distributor_enterprise) } + let(:order) { + create(:completed_order_with_fees, distributor:, shipping_fee:, payment_fee: 0) + } + let(:shipping_fee) { 5 } + + it "does nothing if shipment is shipped" do + # An order need to be paid before we can ship a shipment + create(:payment, amount: order.total, order:, state: "completed") + + shipment = order.shipments.first + shipment.ship + + expect(shipment).not_to receive(:save) + + order.update_shipping_fees! + end + + it "saves the each shipment" do + order.shipments << create(:shipment, order:) + order.shipments.each do |shipment| + expect(shipment).to receive(:save) + end + + order.update_shipping_fees! + end + + context "with shipment including a shipping fee" do + it "updates shipping fee" do + # Manually udate line item quantity, in a normal scenario we would use + # order.contents method, which takes care of updating shipments + order.line_items.first.update(quantity: 2) + + order.update_shipping_fees! + + expect(order.reload.adjustment_total).to eq(15) # 3 items * 5 + end + end + end + + describe "a completed order with transaction fees" do let(:distributor) { create(:distributor_enterprise_with_tax) } - let(:zone) { create(:zone_with_member) } - let(:shipping_tax_rate) { create(:tax_rate, amount: 0.25, included_in_price: true, zone:) } - let(:shipping_tax_category) { create(:tax_category, tax_rates: [shipping_tax_rate]) } let(:order) { - create(:completed_order_with_fees, distributor:, shipping_fee:, - payment_fee:, - shipping_tax_category:) + create(:completed_order_with_fees, distributor:, shipping_fee: 0, payment_fee:) } - let(:shipping_fee) { 3 } let(:payment_fee) { 5 } let(:item_num) { order.line_items.length } - let(:expected_fees) { item_num * (shipping_fee + payment_fee) } + let(:expected_fees) { item_num * payment_fee } before do order.reload order.create_tax_charge! # Sanity check the fees - expect(order.all_adjustments.length).to eq 3 - expect(order.shipment_adjustments.length).to eq 2 + expect(order.all_adjustments.length).to eq 2 expect(item_num).to eq 2 expect(order.adjustment_total).to eq expected_fees - expect(order.shipment.adjustments.tax.inclusive.sum(:amount)).to eq 1.2 - expect(order.shipment.included_tax_total).to eq 1.2 end context "removing line_items" do - it "updates shipping and transaction fees" do + it "updates transaction fees" do order.line_items.first.update_attribute(:quantity, 0) order.save - expect(order.adjustment_total).to eq expected_fees - shipping_fee - payment_fee - expect(order.shipment.adjustments.tax.inclusive.sum(:amount)).to eq 0.6 - expect(order.shipment.included_tax_total).to eq 0.6 - end - - context "when finalized fee adjustments exist on the order" do - before do - order.all_adjustments.each(&:finalize!) - order.reload - end - - it "does not attempt to update such adjustments" do - order.update(line_items_attributes: [{ id: order.line_items.first.id, quantity: 0 }]) - - # Check if fees got updated - order.reload - - expect(order.adjustment_total).to eq expected_fees - expect(order.shipment.adjustments.tax.inclusive.sum(:amount)).to eq 1.2 - expect(order.shipment.included_tax_total).to eq 1.2 - end - end - end - - context "changing the shipping method to one without fees" do - let(:shipping_method) { - create(:shipping_method, calculator: Calculator::FlatRate.new(preferred_amount: 0)) - } - - it "updates shipping fees" do - order.shipments = [create(:shipment_with, :shipping_method, - shipping_method:)] - order.save - - expect(order.adjustment_total).to eq expected_fees - (item_num * shipping_fee) - expect(order.shipment.adjustments.tax.inclusive.sum(:amount)).to eq 0 - expect(order.shipment.included_tax_total).to eq 0 + expect(order.adjustment_total).to eq expected_fees - payment_fee end end @@ -1296,6 +1283,7 @@ # Check if fees got updated order.reload + expect(order.adjustment_total).to eq expected_fees - (item_num * payment_fee) end end diff --git a/spec/system/admin/orders_spec.rb b/spec/system/admin/orders_spec.rb index fd306cebbc7..e672166a6c7 100644 --- a/spec/system/admin/orders_spec.rb +++ b/spec/system/admin/orders_spec.rb @@ -452,14 +452,11 @@ context "orders with different order totals" do before do - Spree::LineItem.where(order_id: order2.id).first.update!(quantity: 5) - Spree::LineItem.where(order_id: order3.id).first.update!(quantity: 4) - Spree::LineItem.where(order_id: order4.id).first.update!(quantity: 3) - Spree::LineItem.where(order_id: order5.id).first.update!(quantity: 2) - order2.save - order3.save - order4.save - order5.save + order2.contents.update_item(Spree::LineItem.find_by(order_id: order2.id), { quantity: 5 }) + order3.contents.update_item(Spree::LineItem.find_by(order_id: order3.id), { quantity: 4 }) + order4.contents.update_item(Spree::LineItem.find_by(order_id: order4.id), { quantity: 3 }) + order5.contents.update_item(Spree::LineItem.find_by(order_id: order5.id), { quantity: 2 }) + login_as_admin visit spree.admin_orders_path end diff --git a/spec/system/consumer/shopping/checkout_paypal_spec.rb b/spec/system/consumer/shopping/checkout_paypal_spec.rb index 98947d099cf..93f2fdbabe0 100644 --- a/spec/system/consumer/shopping/checkout_paypal_spec.rb +++ b/spec/system/consumer/shopping/checkout_paypal_spec.rb @@ -69,6 +69,7 @@ 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 7758f582db2..f1974fd5af9 100644 --- a/spec/system/consumer/shopping/checkout_stripe_spec.rb +++ b/spec/system/consumer/shopping/checkout_stripe_spec.rb @@ -71,6 +71,8 @@ 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