Merge pull request #2856 from luisramos0/2-0-adjust-clean-up

[Spree Upgrade] Running Transpec and Rubocop in "new" models/spree/adjustment_spec

.rubocop_todo.yml

@@ -241,7 +241,6 @@ Layout/EmptyLines:
     - 'spec/models/enterprise_spec.rb'
     - 'spec/models/model_set_spec.rb'
     - 'spec/models/product_distribution_spec.rb'
-    - 'spec/models/spree/adjustment_spec.rb'
     - 'spec/models/spree/line_item_spec.rb'
     - 'spec/models/spree/order_populator_spec.rb'
     - 'spec/models/spree/order_spec.rb'
@@ -394,7 +393,6 @@ Layout/ExtraSpacing:
     - 'spec/models/enterprise_fee_spec.rb'
     - 'spec/models/enterprise_spec.rb'
     - 'spec/models/order_cycle_spec.rb'
-    - 'spec/models/spree/adjustment_spec.rb'
     - 'spec/models/spree/gateway/stripe_connect_spec.rb'
     - 'spec/models/spree/order_spec.rb'
     - 'spec/models/variant_override_spec.rb'
@@ -1150,7 +1148,6 @@ Lint/Void:
     - 'spec/models/enterprise_spec.rb'
     - 'spec/models/exchange_spec.rb'
     - 'spec/models/order_cycle_spec.rb'
-    - 'spec/models/spree/adjustment_spec.rb'
     - 'spec/models/spree/line_item_spec.rb'
     - 'spec/models/spree/order_populator_spec.rb'
     - 'spec/models/spree/order_spec.rb'

app/models/spree/tax_rate_decorator.rb

@@ -8,26 +8,24 @@ module Spree
       alias_method_chain :match, :sales_tax_registration
     end
 
-
     def adjust_with_included_tax(order)
       adjust_without_included_tax(order)
 
       order.adjustments(:reload)
       order.line_items(:reload)
-      (order.adjustments.tax + order.price_adjustments).each do |a|
-        a.set_absolute_included_tax! a.amount
+      # TaxRate adjustments (order.adjustments.tax) and price adjustments (tax included on line items) consist of 100% tax
+      (order.adjustments.tax + order.price_adjustments).each do |adjustment|
+        adjustment.set_absolute_included_tax! adjustment.amount
       end
     end
     alias_method_chain :adjust, :included_tax
 
-
     # Manually apply a TaxRate to a particular amount. TaxRates normally compute against
     # LineItems or Orders, so we mock out a line item here to fit the interface
     # that our calculator (usually DefaultTax) expects.
     def compute_tax(amount)
-      product = OpenStruct.new tax_category: tax_category
       line_item = LineItem.new quantity: 1
-      line_item.define_singleton_method(:product) { product }
+      line_item.tax_category = tax_category
       line_item.define_singleton_method(:price) { amount }
 
       # Tax on adjustments (represented by the included_tax field) is always inclusive of
@@ -41,7 +39,6 @@ module Spree
       end
     end
 
-
     private
 
     def with_tax_included_in_price

config/initializers/spree.rb

@@ -7,7 +7,6 @@
 # config.setting_name = 'new value'
 
 require 'spree/product_filters'
-require 'spree/core/calculated_adjustments_decorator'
 
 require "#{Rails.root}/app/models/spree/payment_method_decorator"
 require "#{Rails.root}/app/models/spree/gateway_decorator"

lib/open_food_network/enterprise_fee_applicator.rb

@@ -1,24 +1,33 @@
 module OpenFoodNetwork
   class EnterpriseFeeApplicator < Struct.new(:enterprise_fee, :variant, :role)
     def create_line_item_adjustment(line_item)
-      a = enterprise_fee.create_adjustment(line_item_adjustment_label, line_item.order, line_item, true)
-
-      AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role
-
-      a.set_absolute_included_tax! adjustment_tax(line_item, a)
+      create_adjustment(line_item_adjustment_label, line_item.order, line_item)
     end
 
     def create_order_adjustment(order)
-      a = enterprise_fee.create_adjustment(order_adjustment_label, order, order, true)
-
-      AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role
-
-      a.set_absolute_included_tax! adjustment_tax(order, a)
+      create_adjustment(order_adjustment_label, order, order)
     end
 
-
     private
 
+    def create_adjustment(label, target, calculable)
+      adjustment = create_enterprise_fee_adjustment(label, target, calculable)
+
+      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
@@ -31,7 +40,7 @@ module OpenFoodNetwork
       I18n.t(:enterprise_fee_by, type: enterprise_fee.fee_type, role: role, enterprise_name: enterprise_fee.enterprise.name)
     end
 
-    def adjustment_tax(adjustable, adjustment)
+    def adjustment_tax(adjustment)
       tax_rates = adjustment.tax_rates
 
       tax_rates.select(&:included_in_price).sum do |rate|

lib/spree/core/calculated_adjustments_decorator.rb

@@ -1,16 +0,0 @@
-module Spree
-  module Core
-    module CalculatedAdjustments
-      class << self
-        def included_with_explicit_class_name(klass)
-          included_without_explicit_class_name(klass)
-
-          klass.class_eval do
-            has_one :calculator, as: :calculable, dependent: :destroy, class_name: 'Spree::Calculator'
-          end
-        end
-        alias_method_chain :included, :explicit_class_name
-      end
-    end
-  end
-end

spec/factories.rb

@@ -374,6 +374,7 @@ FactoryBot.define do
         shipping_method { create :shipping_method }
       end
       after(:create) do |shipment, evaluator|
+        shipment.shipping_rates.destroy_all
         shipment.add_shipping_method(evaluator.shipping_method, true)
       end
     end
@@ -385,7 +386,7 @@ FactoryBot.define do
 
       after(:create) do |shipment, evaluator|
         shipping_method = create(:shipping_method_with, :shipping_fee, shipping_fee: evaluator.shipping_fee)
-        shipment.shipping_rates.destroy_all # remove existing shipping_rates from shipment
+        shipment.shipping_rates.destroy_all
         shipment.add_shipping_method(shipping_method, true)
       end
     end

spec/models/spree/adjustment_spec.rb

@@ -4,7 +4,7 @@ module Spree
   describe Adjustment do
     it "has metadata" do
       adjustment = create(:adjustment, metadata: create(:adjustment_metadata))
-      adjustment.metadata.should be
+      expect(adjustment.metadata).to be
     end
 
     describe "querying included tax" do
@@ -13,23 +13,23 @@ module Spree
 
       describe "finding adjustments with and without tax included" do
         it "finds adjustments with tax" do
-          Adjustment.with_tax.should     include adjustment_with_tax
-          Adjustment.with_tax.should_not include adjustment_without_tax
+          expect(Adjustment.with_tax).to     include adjustment_with_tax
+          expect(Adjustment.with_tax).not_to include adjustment_without_tax
         end
 
         it "finds adjustments without tax" do
-          Adjustment.without_tax.should     include adjustment_without_tax
-          Adjustment.without_tax.should_not include adjustment_with_tax
+          expect(Adjustment.without_tax).to     include adjustment_without_tax
+          expect(Adjustment.without_tax).not_to include adjustment_with_tax
         end
       end
 
       describe "checking if an adjustment includes tax" do
         it "returns true when it has > 0 tax" do
-          adjustment_with_tax.should have_tax
+          expect(adjustment_with_tax).to have_tax
         end
 
         it "returns false when it has 0 tax" do
-          adjustment_without_tax.should_not have_tax
+          expect(adjustment_without_tax).not_to have_tax
         end
       end
     end
@@ -48,8 +48,8 @@ module Spree
         end
 
         it "has 100% tax included" do
-          adjustment.amount.should be > 0
-          adjustment.included_tax.should == adjustment.amount
+          expect(adjustment.amount).to be > 0
+          expect(adjustment.included_tax).to eq(adjustment.amount)
         end
 
         it "does not crash when order data has been updated previously" do
@@ -61,70 +61,76 @@ module Spree
       describe "Shipment adjustments" do
         let(:shipping_method) { create(:shipping_method_with, :flat_rate) }
         let(:shipment)        { create(:shipment_with, :shipping_method, shipping_method: shipping_method) }
-        let!(:order)          { create(:order, distributor: hub, shipments: [shipment]) }
+        let(:order)           { create(:order, distributor: hub) }
         let(:hub)             { create(:distributor_enterprise, charges_sales_tax: true) }
-        let!(:line_item)      { create(:line_item, order: order) }
+        let(:line_item)       { create(:line_item, order: order) }
         let(:adjustment)      { order.adjustments(:reload).shipping.first }
 
-        it "has a shipping charge of $50" do
-          adjustment.amount.should == 50
+        describe "the shipping charge" do
+          it "is the adjustment amount" do
+            order.shipments = [shipment]
+
+            expect(adjustment.amount).to eq(50)
+          end
         end
 
         describe "when tax on shipping is disabled" do
+          before { Config.shipment_inc_vat = false }
           it "records 0% tax on shipment adjustments" do
-            Config.shipment_inc_vat = false
             Config.shipping_tax_rate = 0
+            order.shipments = [shipment]
 
-            adjustment.included_tax.should == 0
+            expect(adjustment.included_tax).to eq(0)
           end
 
           it "records 0% tax on shipments when a rate is set but shipment_inc_vat is false" do
-            Config.shipment_inc_vat = false
             Config.shipping_tax_rate = 0.25
+            order.shipments = [shipment]
 
-            adjustment.included_tax.should == 0
+            expect(adjustment.included_tax).to eq(0)
           end
         end
 
         describe "when tax on shipping is enabled" do
-          before do
-            Config.shipment_inc_vat = true
-            Config.shipping_tax_rate = 0.25
-          end
+          before { Config.shipment_inc_vat = true }
 
           it "takes the shipment adjustment tax included from the system setting" do
+            Config.shipping_tax_rate = 0.25
+            order.shipments = [shipment]
+
             # Finding the tax included in an amount that's already inclusive of tax:
             # total - ( total / (1 + rate) )
             # 50    - ( 50    / (1 + 0.25) )
             # = 10
-            adjustment.included_tax.should == 10.00
+            expect(adjustment.included_tax).to eq(10.00)
           end
 
           it "records 0% tax on shipments when shipping_tax_rate is not set" do
-            Config.shipment_inc_vat = true
             Config.shipping_tax_rate = nil
+            order.shipments = [shipment]
 
-            adjustment.included_tax.should == 0
+            expect(adjustment.included_tax).to eq(0)
           end
 
           it "records 0% tax on shipments when the distributor does not charge sales tax" do
             order.distributor.update_attributes! charges_sales_tax: false
+            order.shipments = [shipment]
 
-            adjustment.included_tax.should == 0
+            expect(adjustment.included_tax).to eq(0)
           end
         end
       end
 
       describe "EnterpriseFee adjustments" do
-        let!(:zone)                { create(:zone_with_member) }
-        let(:fee_tax_rate)             { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) }
-        let(:fee_tax_category)         { create(:tax_category, tax_rates: [fee_tax_rate]) }
+        let(:zone)             { create(:zone_with_member) }
+        let(:fee_tax_rate)     { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.1) }
+        let(:fee_tax_category) { create(:tax_category, tax_rates: [fee_tax_rate]) }
 
         let(:coordinator) { create(:distributor_enterprise, charges_sales_tax: true) }
         let(:variant)     { create(:variant, product: create(:product, tax_category: nil)) }
         let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, coordinator_fees: [enterprise_fee], distributors: [coordinator], variants: [variant]) }
-        let!(:order)      { create(:order, order_cycle: order_cycle, distributor: coordinator) }
-        let!(:line_item)  { create(:line_item, order: order, variant: 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 }
 
         context "when enterprise fees have a fixed tax_category" do
@@ -140,7 +146,7 @@ module Spree
                 # The fee is $50, tax is 10%, and the fee is inclusive of tax
                 # Therefore, the included tax should be 0.1/1.1 * 50 = $4.55
 
-                adjustment.included_tax.should == 4.55
+                expect(adjustment.included_tax).to eq(4.55)
               end
             end
 
@@ -152,8 +158,8 @@ module Spree
               end
 
               it "records the tax on TaxRate adjustment on the order" do
-                adjustment.included_tax.should == 0
-                order.adjustments.tax.first.amount.should == 5.0
+                expect(adjustment.included_tax).to eq(0)
+                expect(order.adjustments.tax.first.amount).to eq(5.0)
               end
             end
 
@@ -165,44 +171,44 @@ module Spree
               end
 
               it "records no tax as charged" do
-                adjustment.included_tax.should == 0
+                expect(adjustment.included_tax).to eq(0)
               end
             end
           end
 
-
           context "when enterprise fees are taxed per-item" do
             let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, tax_category: fee_tax_category, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) }
 
             describe "when the tax rate includes the tax in the price" do
               it "records the tax on the enterprise fee adjustments" do
-                adjustment.included_tax.should == 4.55
+                expect(adjustment.included_tax).to eq(4.55)
               end
             end
 
             describe "when the tax rate does not include the tax in the price" do
               before do
                 fee_tax_rate.update_attribute :included_in_price, false
-                order.reload.create_tax_charge!  # Updating line_item or order has the same effect
+                order.reload.create_tax_charge! # Updating line_item or order has the same effect
                 order.update_distribution_charge!
               end
 
               it "records the tax on TaxRate adjustment on the order" do
-                adjustment.included_tax.should == 0
-                order.adjustments.tax.first.amount.should == 5.0
+                expect(adjustment.included_tax).to eq(0)
+                expect(order.adjustments.tax.first.amount).to eq(5.0)
               end
             end
           end
         end
 
-        context "when enterprise fees inherit their tax_category product they are applied to" do
+        context "when enterprise fees inherit their tax_category from the product they are applied to" do
           let(:product_tax_rate)             { create(:tax_rate, included_in_price: true, calculator: Calculator::DefaultTax.new, zone: zone, amount: 0.2) }
           let(:product_tax_category)         { create(:tax_category, tax_rates: [product_tax_rate]) }
 
           before do
             variant.product.update_attribute(:tax_category_id, product_tax_category.id)
-            order.reload.create_tax_charge! # Updating line_item or order has the same effect
-            order.reload.update_distribution_charge!
+
+            order.create_tax_charge! # Updating line_item or order has the same effect
+            order.update_distribution_charge!
           end
 
           context "when enterprise fees are taxed per-order" do
@@ -213,8 +219,8 @@ module Spree
                 # EnterpriseFee tax category is nil and inheritance only applies to per item fees
                 # so tax on the enterprise_fee adjustment will be 0
                 # Tax on line item is: 0.2/1.2 x $10 = $1.67
-                adjustment.included_tax.should == 0.0
-                line_item.adjustments.first.included_tax.should == 1.67
+                expect(adjustment.included_tax).to eq(0.0)
+                expect(line_item.adjustments.first.included_tax).to eq(1.67)
               end
             end
 
@@ -229,13 +235,12 @@ module Spree
                 # EnterpriseFee tax category is nil and inheritance only applies to per item fees
                 # so total tax on the order is only that which applies to the line_item itself
                 # ie. $10 x 0.2 = $2.0
-                adjustment.included_tax.should == 0
-                order.adjustments.tax.first.amount.should == 2.0
+                expect(adjustment.included_tax).to eq(0)
+                expect(order.adjustments.tax.first.amount).to eq(2.0)
               end
             end
           end
 
-
           context "when enterprise fees are taxed per-item" do
             let(:enterprise_fee) { create(:enterprise_fee, enterprise: coordinator, inherits_tax_category: true, calculator: Calculator::PerItem.new(preferred_amount: 50.0)) }
 
@@ -244,15 +249,15 @@ module Spree
                 # Applying product tax rate of 0.2 to enterprise fee of $50
                 # gives tax on fee of 0.2/1.2 x $50 = $8.33
                 # Tax on line item is: 0.2/1.2 x $10 = $1.67
-                adjustment.included_tax.should == 8.33
-                line_item.adjustments.first.included_tax.should == 1.67
+                expect(adjustment.included_tax).to eq(8.33)
+                expect(line_item.adjustments.first.included_tax).to eq(1.67)
               end
             end
 
             describe "when the tax rate does not include the tax in the price" do
               before do
                 product_tax_rate.update_attribute :included_in_price, false
-                order.reload.create_tax_charge!  # Updating line_item or order has the same effect
+                order.reload.create_tax_charge! # Updating line_item or order has the same effect
                 order.update_distribution_charge!
               end
 
@@ -260,8 +265,8 @@ module Spree
                 # EnterpriseFee inherits tax_category from product so total tax on
                 # the order is that which applies to the line item itself, plus the
                 # same rate applied to the fee of $50. ie. ($10 + $50) x 0.2 = $12.0
-                adjustment.included_tax.should == 0
-                order.adjustments.tax.first.amount.should == 12.0
+                expect(adjustment.included_tax).to eq(0)
+                expect(order.adjustments.tax.first.amount).to eq(12.0)
               end
             end
           end
@@ -273,7 +278,7 @@ module Spree
 
         it "sets it, rounding to two decimal places" do
           adjustment.set_included_tax! 0.25
-          adjustment.included_tax.should == 10.00
+          expect(adjustment.included_tax).to eq(10.00)
         end
       end
 
@@ -284,11 +289,11 @@ module Spree
         let!(:other_tax_rate)         { create(:tax_rate, calculator: Spree::Calculator::DefaultTax.new, amount: 0.3) }
 
         it "returns [] if there is no included tax" do
-          adjustment_without_tax.find_closest_tax_rates_from_included_tax.should == []
+          expect(adjustment_without_tax.find_closest_tax_rates_from_included_tax).to eq([])
         end
 
         it "returns the most accurate tax rate" do
-          adjustment_with_tax.find_closest_tax_rates_from_included_tax.should == [tax_rate]
+          expect(adjustment_with_tax.find_closest_tax_rates_from_included_tax).to eq([tax_rate])
         end
       end
     end