From 5693f44f5e49760616cf141c87874b5c5af72ed4 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 1 Oct 2018 14:47:56 +0100 Subject: [PATCH 1/5] Fix shipping adjustment basic test in adjustment_spec by removing extra shipping_method from test shipment --- spec/factories.rb | 1 + spec/models/spree/adjustment_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index 2476a91be7..d062abfd16 100644 --- a/spec/factories.rb +++ b/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 # remove existing shipping_rates from shipment shipment.add_shipping_method(evaluator.shipping_method, true) end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 114c7e7cd3..c4d8f3f786 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -61,11 +61,13 @@ 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(:adjustment) { order.adjustments(:reload).shipping.first } + before { order.shipments = [shipment] } + it "has a shipping charge of $50" do adjustment.amount.should == 50 end From bd31348b944971ee2d2793836134ae6487330fe0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 2 Oct 2018 11:57:58 +0100 Subject: [PATCH 2/5] Improve docs and readability on tax_rate_decorator --- app/models/spree/tax_rate_decorator.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/models/spree/tax_rate_decorator.rb b/app/models/spree/tax_rate_decorator.rb index fd111084ce..8c8998735e 100644 --- a/app/models/spree/tax_rate_decorator.rb +++ b/app/models/spree/tax_rate_decorator.rb @@ -8,19 +8,18 @@ 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. @@ -41,7 +40,6 @@ module Spree end end - private def with_tax_included_in_price From a5522b90f6dc39d4f380b37ae6b5d71004c17688 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 2 Oct 2018 12:01:09 +0100 Subject: [PATCH 3/5] Fix Shipment adjustments specs on adjustment_spec by setting Config values before test objects are created --- spec/models/spree/adjustment_spec.rb | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index c4d8f3f786..7ddcfd0d62 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -61,40 +61,43 @@ 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) } + 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 } - before { order.shipments = [shipment] } + describe "the shipping charge" do + it "is the adjustment amount" do + order.shipments = [shipment] - it "has a shipping charge of $50" do - adjustment.amount.should == 50 + adjustment.amount.should == 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 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 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) ) @@ -103,14 +106,15 @@ module Spree 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 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 end From 4380ff7bd702f4a801870a2750cb480a83d1b3e3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 2 Oct 2018 13:28:50 +0100 Subject: [PATCH 4/5] Drop calculated_adjustments_decorator. This file was introduced in ofn's a0b740f52df995f9da8d60df99fcab363e0486e8. The change is already in spree 2.0.4, see here https://github.com/spree/spree/commit/2c82aab566c7e867f63ec8b1c7e151cefb51c69b#diff-00aa4190da81ca29804a406252f1d0f4 --- config/initializers/spree.rb | 1 - .../core/calculated_adjustments_decorator.rb | 16 ---------------- 2 files changed, 17 deletions(-) delete mode 100644 lib/spree/core/calculated_adjustments_decorator.rb diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 83e854a08f..5c60a92778 100644 --- a/config/initializers/spree.rb +++ b/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" diff --git a/lib/spree/core/calculated_adjustments_decorator.rb b/lib/spree/core/calculated_adjustments_decorator.rb deleted file mode 100644 index 592bf59854..0000000000 --- a/lib/spree/core/calculated_adjustments_decorator.rb +++ /dev/null @@ -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 From f133524a9c8c8e959c75aa7242d7f9fd346f5b6a Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 9 Oct 2018 17:03:36 +0100 Subject: [PATCH 5/5] Remove useless comment from spec factories --- spec/factories.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index d062abfd16..3db6cf46e1 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -374,7 +374,7 @@ FactoryBot.define do shipping_method { create :shipping_method } end after(:create) do |shipment, evaluator| - shipment.shipping_rates.destroy_all # remove existing shipping_rates from shipment + shipment.shipping_rates.destroy_all shipment.add_shipping_method(evaluator.shipping_method, true) end end @@ -386,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