diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 81bee889d3d..a365693d0e0 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 diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb index 6c8e150e02c..e295d3cc6cb 100644 --- a/app/models/spree/order_contents.rb +++ b/app/models/spree/order_contents.rb @@ -38,6 +38,7 @@ def update_or_create(variant, attributes) line_item.price = variant.price order.line_items << line_item end + update_shipment order.reload line_item diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb index b31d685ea38..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 @@ -175,6 +237,22 @@ expect(line_item.max_quantity).to eq 3 expect(line_item.price).to eq variant.price end + + it "ensures shipments are updated" do + expect(order).to receive(:ensure_updated_shipments) + + 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 @@ -186,6 +264,22 @@ expect(line_item.reload.quantity).to eq 3 expect(line_item.max_quantity).to eq 4 end + + it "ensures shipments are updated" do + expect(order).to receive(:ensure_updated_shipments) + + 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 00a0cdb6214..e70458b7595 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -1211,76 +1211,73 @@ 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! + + item_num = order.line_items.sum(&:quantity) + expect(order.reload.adjustment_total).to eq(item_num * shipping_fee) + 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 +1293,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