diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index 47d2522afe..18ab79969a 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -4,10 +4,9 @@ module CheckoutHelper end def checkout_adjustments_for(order, opts = {}) - adjustments = order.adjustments.eligible exclude = opts[:exclude] || {} - adjustments = adjustments.to_a + order.shipment_adjustments.to_a + adjustments = order.all_adjustments.eligible.to_a # Remove empty tax adjustments and (optionally) shipping fees adjustments.reject! { |a| a.originator_type == 'Spree::TaxRate' && a.amount == 0 } @@ -26,17 +25,16 @@ module CheckoutHelper adjustments end - def display_checkout_admin_and_handling_adjustments_total_for(order) - adjustments = order.adjustments.eligible.where('originator_type = ? AND source_type != ? ', 'EnterpriseFee', 'Spree::LineItem') - Spree::Money.new adjustments.sum(:amount), currency: order.currency + def display_line_item_fees_total_for(order) + Spree::Money.new order.adjustments.enterprise_fee.sum(:amount), currency: order.currency end - def checkout_line_item_adjustments(order) - order.adjustments.eligible.where(source_type: "Spree::LineItem") + def checkout_line_item_fees(order) + order.line_item_adjustments.enterprise_fee end def checkout_subtotal(order) - order.item_total + checkout_line_item_adjustments(order).sum(:amount) + order.item_total + checkout_line_item_fees(order).sum(:amount) end def display_checkout_subtotal(order) diff --git a/app/jobs/subscription_placement_job.rb b/app/jobs/subscription_placement_job.rb index 4cd02392a4..c56df548d6 100644 --- a/app/jobs/subscription_placement_job.rb +++ b/app/jobs/subscription_placement_job.rb @@ -73,7 +73,7 @@ class SubscriptionPlacementJob < ActiveJob::Base end def handle_empty_order(order, changes) - order.reload.adjustments.destroy_all + order.reload.all_adjustments.destroy_all order.update! send_empty_email(order, changes) end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index b7a3eeb6e2..6406ad6868 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -41,7 +41,7 @@ class EnterpriseFee < ActiveRecord::Base } def self.clear_all_adjustments(order) - order.adjustments.enterprise_fee.destroy_all + order.all_adjustments.enterprise_fee.destroy_all end private diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index d224b154a8..df88535a2a 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -190,9 +190,9 @@ module Spree # so line_item.adjustments returns an empty array return 0 if quantity.zero? - line_item_adjustments = OrderAdjustmentsFetcher.new(order).line_item_adjustments(self) + fees = adjustments.enterprise_fee.sum(:amount) - (price + line_item_adjustments.to_a.sum(&:amount) / quantity).round(2) + (price + fees / quantity).round(2) end def single_display_amount_with_adjustments diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 3755604848..faac4a7089 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -650,7 +650,7 @@ module Spree end def enterprise_fee_tax - adjustments.reload.enterprise_fee.sum(:included_tax) + all_adjustments.reload.enterprise_fee.sum(:included_tax) end def total_tax diff --git a/app/services/order_adjustments_fetcher.rb b/app/services/order_adjustments_fetcher.rb index 9292fe8e91..ea553766c8 100644 --- a/app/services/order_adjustments_fetcher.rb +++ b/app/services/order_adjustments_fetcher.rb @@ -29,14 +29,6 @@ class OrderAdjustmentsFetcher sum_adjustments "shipping" end - def line_item_adjustments(line_item) - if adjustments_eager_loaded? - adjustments.select{ |adjustment| adjustment.source_id == line_item.id } - else - adjustments.where(source_id: line_item.id) - end - end - private attr_reader :order diff --git a/app/services/order_fees_handler.rb b/app/services/order_fees_handler.rb index 55c2416136..17247dcca6 100644 --- a/app/services/order_fees_handler.rb +++ b/app/services/order_fees_handler.rb @@ -19,6 +19,9 @@ class OrderFeesHandler create_line_item_fees! create_order_fees! + + order.updater.update_totals + order.updater.persist_totals end order.update! diff --git a/app/views/spree/orders/_adjustments.html.haml b/app/views/spree/orders/_adjustments.html.haml index 14fba90f72..915e1a3c39 100644 --- a/app/views/spree/orders/_adjustments.html.haml +++ b/app/views/spree/orders/_adjustments.html.haml @@ -3,7 +3,7 @@ %a{ href: "#" } = t :orders_fees -- checkout_line_item_adjustments(@order).each do |adjustment| +- checkout_line_item_fees(@order).each do |adjustment| %tr.cart_adjustment %td{colspan: "3"}= adjustment.label %td.text-right= adjustment.display_amount.to_html diff --git a/app/views/spree/orders/_form.html.haml b/app/views/spree/orders/_form.html.haml index b38327dd56..e7c83d1217 100644 --- a/app/views/spree/orders/_form.html.haml +++ b/app/views/spree/orders/_form.html.haml @@ -31,12 +31,12 @@ %td.text-right %span.order-total.item-total= display_checkout_subtotal(@order) %td - -if display_checkout_admin_and_handling_adjustments_total_for(@order) != Spree::Money.new(0 , currency: @order.currency) + -if display_line_item_fees_total_for(@order) != Spree::Money.new(0 , currency: @order.currency) %tr %td.text-right{colspan:"3"} = t :orders_form_admin %td.text-right - %span.order-total.distribution-total= display_checkout_admin_and_handling_adjustments_total_for(@order) + %span.order-total.distribution-total= display_line_item_fees_total_for(@order) %td - checkout_adjustments_for(@order, exclude: [:line_item, :admin_and_handling]).reject{ |a| a.amount == 0 }.reverse_each do |adjustment| diff --git a/db/migrate/20210228223114_migrate_line_item_fees.rb b/db/migrate/20210228223114_migrate_line_item_fees.rb new file mode 100644 index 0000000000..f923a7b7b5 --- /dev/null +++ b/db/migrate/20210228223114_migrate_line_item_fees.rb @@ -0,0 +1,22 @@ +class MigrateLineItemFees < ActiveRecord::Migration + class Spree::Adjustment < ActiveRecord::Base + belongs_to :originator, polymorphic: true + belongs_to :source, polymorphic: true + end + + def up + Spree::Adjustment. + where(originator_type: 'EnterpriseFee', source_type: 'Spree::LineItem'). + update_all( + "adjustable_id = source_id, adjustable_type = 'Spree::LineItem'" + ) + end + + def down + Spree::Adjustment. + where(originator_type: 'EnterpriseFee', source_type: 'Spree::LineItem'). + update_all( + "adjustable_id = order_id, adjustable_type = 'Spree::Order'" + ) + end +end diff --git a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb index eafbdbbad6..d23668edf1 100644 --- a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb +++ b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb @@ -52,7 +52,7 @@ module OrderManagement def for_orders chain_to_scope do - where(adjustable_type: ["Spree::Order", "Spree::Shipment"]) + where(adjustable_type: ["Spree::Order", "Spree::Shipment", "Spree::LineItem"]) end end diff --git a/lib/open_food_network/enterprise_fee_applicator.rb b/lib/open_food_network/enterprise_fee_applicator.rb index ab1c96eab1..76f6e78a53 100644 --- a/lib/open_food_network/enterprise_fee_applicator.rb +++ b/lib/open_food_network/enterprise_fee_applicator.rb @@ -1,7 +1,7 @@ module OpenFoodNetwork class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role) def create_line_item_adjustment(line_item) - create_adjustment(line_item_adjustment_label, line_item.order, line_item) + create_adjustment(line_item_adjustment_label, line_item, line_item) end def create_order_adjustment(order) @@ -11,23 +11,13 @@ module OpenFoodNetwork private def create_adjustment(label, target, calculable) - adjustment = create_enterprise_fee_adjustment(label, target, calculable) + adjustment = enterprise_fee.create_adjustment(label, target, calculable, true) AdjustmentMetadata.create! adjustment: adjustment, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role adjustment.set_absolute_included_tax! adjustment_tax(adjustment) end - def create_enterprise_fee_adjustment(label, target, calculable) - adjustment = enterprise_fee.create_adjustment(label, target, calculable, true) - - # This is necessary when source is a line_item - # probably because the association order.adjustments contains "inverse_of :source" - # which overrides the value (the line item) set in calculated_adjustment.create_adjustment - adjustment.source = calculable - adjustment - end - def line_item_adjustment_label "#{variant.product.name} - #{base_adjustment_label}" end diff --git a/spec/helpers/checkout_helper_spec.rb b/spec/helpers/checkout_helper_spec.rb index 966569459a..0283fabc23 100644 --- a/spec/helpers/checkout_helper_spec.rb +++ b/spec/helpers/checkout_helper_spec.rb @@ -36,7 +36,8 @@ describe CheckoutHelper, type: :helper do let(:order) { create(:order_with_totals_and_distribution) } let(:enterprise_fee) { create(:enterprise_fee, amount: 123) } let!(:fee_adjustment) { - create(:adjustment, originator: enterprise_fee, adjustable: order, source: order) + create(:adjustment, originator: enterprise_fee, adjustable: order, source: order, + order: order) } before do diff --git a/spec/jobs/subscription_placement_job_spec.rb b/spec/jobs/subscription_placement_job_spec.rb index 96e79012a7..db88322ffc 100644 --- a/spec/jobs/subscription_placement_job_spec.rb +++ b/spec/jobs/subscription_placement_job_spec.rb @@ -182,9 +182,9 @@ describe SubscriptionPlacementJob do allow(job).to receive(:unavailable_stock_lines_for) { order.line_items } end - it "does not place the order, clears, all adjustments, and sends an empty_order email" do + it "does not place the order, clears all adjustments, and sends an empty_order email" do expect{ job.send(:place_order, order) }.to_not change{ order.reload.completed_at }.from(nil) - expect(order.adjustments).to be_empty + expect(order.all_adjustments).to be_empty expect(order.total).to eq 0 expect(order.adjustment_total).to eq 0 expect(job).to_not have_received(:send_placement_email) diff --git a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb index 6ea5597ddc..8fee7bdad2 100644 --- a/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb +++ b/spec/lib/open_food_network/enterprise_fee_applicator_spec.rb @@ -16,7 +16,7 @@ module OpenFoodNetwork adjustment = Spree::Adjustment.last expect(adjustment.label).to eq('label') - expect(adjustment.adjustable).to eq(line_item.order) + expect(adjustment.adjustable).to eq(line_item) expect(adjustment.source).to eq(line_item) expect(adjustment.originator).to eq(enterprise_fee) expect(adjustment).to be_mandatory diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index b46a431041..b38666751a 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -274,7 +274,7 @@ module Spree let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator], variants: [variant]) } let(:line_item) { create(:line_item, variant: variant) } let(:order) { create(:order, line_items: [line_item], order_cycle: order_cycle, distributor: coordinator) } - let(:adjustment) { order.adjustments.reload.enterprise_fee.first } + let(:adjustment) { order.all_adjustments.reload.enterprise_fee.first } context "when enterprise fees have a fixed tax_category" do before do diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 0c0277f00a..e8db788b9f 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -418,9 +418,7 @@ module Spree li = LineItem.new allow(li).to receive(:price) { 55.55 } - allow(li).to receive_message_chain(:order, :adjustments, :loaded?) - allow(li).to receive_message_chain(:order, :adjustments, :select) - allow(li).to receive_message_chain(:order, :adjustments, :where, :to_a, :sum) { 11.11 } + allow(li).to receive_message_chain(:adjustments, :enterprise_fee, :sum) { 11.11 } allow(li).to receive(:quantity) { 2 } expect(li.price_with_adjustments).to eq(61.11) end @@ -431,9 +429,7 @@ module Spree li = LineItem.new allow(li).to receive(:price) { 55.55 } - allow(li).to receive_message_chain(:order, :adjustments, :loaded?) - allow(li).to receive_message_chain(:order, :adjustments, :select) - allow(li).to receive_message_chain(:order, :adjustments, :where, :to_a, :sum) { 11.11 } + allow(li).to receive_message_chain(:adjustments, :enterprise_fee, :sum) { 11.11 } allow(li).to receive(:quantity) { 2 } expect(li.amount_with_adjustments).to eq(122.22) end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index c1a225affd..a2c9ad50a7 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -490,6 +490,7 @@ describe Spree::Order do end describe "applying enterprise fees" do + subject { create(:order) } let(:fee_handler) { ::OrderFeesHandler.new(subject) } before do @@ -661,8 +662,14 @@ describe Spree::Order do let!(:order) { create(:order) } let(:enterprise_fee1) { create(:enterprise_fee) } let(:enterprise_fee2) { create(:enterprise_fee) } - let!(:adjustment1) { create(:adjustment, adjustable: order, originator: enterprise_fee1, label: "EF 1", amount: 123, included_tax: 10.00) } - let!(:adjustment2) { create(:adjustment, adjustable: order, originator: enterprise_fee2, label: "EF 2", amount: 123, included_tax: 2.00) } + let!(:adjustment1) { + create(:adjustment, adjustable: order, originator: enterprise_fee1, label: "EF 1", + amount: 123, included_tax: 10.00, order: order) + } + let!(:adjustment2) { + create(:adjustment, adjustable: order, originator: enterprise_fee2, label: "EF 2", + amount: 123, included_tax: 2.00, order: order) + } it "returns a sum of the tax included in all enterprise fees" do expect(order.reload.enterprise_fee_tax).to eq(12)