From 2426c37a78ca0abaa9389e439c0ce876de45bb98 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 25 Feb 2016 17:40:51 +0100 Subject: [PATCH] Remove code that relates to Spree::ShippingRate#tax_rate Since now we have the persisted Spree::ShippingRate#taxes, we do not need to store one of the tax rates anymore on the shipping rate. The data migration contains three lines of code from tax_rate.rb so that the migration still runs in the future. --- core/app/models/spree/shipping_rate.rb | 10 +---- core/app/models/spree/stock/estimator.rb | 11 ----- ...2313_remove_tax_rate_from_shipping_rate.rb | 40 +++++++++++++++++++ core/spec/models/spree/shipping_rate_spec.rb | 19 --------- .../spec/models/spree/stock/estimator_spec.rb | 2 +- 5 files changed, 42 insertions(+), 40 deletions(-) create mode 100644 core/db/migrate/20160225152313_remove_tax_rate_from_shipping_rate.rb diff --git a/core/app/models/spree/shipping_rate.rb b/core/app/models/spree/shipping_rate.rb index 06ffb352134..c4431b09fde 100644 --- a/core/app/models/spree/shipping_rate.rb +++ b/core/app/models/spree/shipping_rate.rb @@ -2,7 +2,7 @@ module Spree class ShippingRate < Spree::Base belongs_to :shipment, class_name: 'Spree::Shipment' belongs_to :shipping_method, -> { with_deleted }, class_name: 'Spree::ShippingMethod', inverse_of: :shipping_rates - belongs_to :tax_rate, -> { with_deleted }, class_name: 'Spree::TaxRate' + has_many :taxes, class_name: "Spree::ShippingRateTax", foreign_key: "shipping_rate_id", @@ -18,10 +18,6 @@ class ShippingRate < Spree::Base extend DisplayMoney money_methods :amount - def calculate_tax_amount - tax_rate.compute_amount(self) - end - def display_price price = display_amount.to_s @@ -36,10 +32,6 @@ def display_price end alias_method :display_cost, :display_price - def display_tax_amount(tax_amount) - Spree::Money.new(tax_amount, currency: currency) - end - private def tax_label_separator diff --git a/core/app/models/spree/stock/estimator.rb b/core/app/models/spree/stock/estimator.rb index 70049a014d7..505e475f82b 100644 --- a/core/app/models/spree/stock/estimator.rb +++ b/core/app/models/spree/stock/estimator.rb @@ -33,22 +33,11 @@ def choose_default_shipping_rate(shipping_rates) def calculate_shipping_rates(package) shipping_methods(package).map do |shipping_method| cost = shipping_method.calculator.compute(package) - tax_category = shipping_method.tax_category - if tax_category - tax_rate = tax_category.tax_rates.detect do |rate| - # If the rate's zone matches the order's zone, a positive adjustment will be applied. - # If the rate is from the default tax zone, then a negative adjustment will be applied. - # See the tests in shipping_rate_spec.rb for an example of this.d - rate.zone == package.shipment.order.tax_zone || rate.zone.default_tax? - end - end - if cost rate = shipping_method.shipping_rates.new( cost: cost, shipment: package.shipment ) - rate.tax_rate = tax_rate if tax_rate Spree::Tax::ShippingRateTaxer.new.tax(rate) end end.compact diff --git a/core/db/migrate/20160225152313_remove_tax_rate_from_shipping_rate.rb b/core/db/migrate/20160225152313_remove_tax_rate_from_shipping_rate.rb new file mode 100644 index 00000000000..fc65fc45b64 --- /dev/null +++ b/core/db/migrate/20160225152313_remove_tax_rate_from_shipping_rate.rb @@ -0,0 +1,40 @@ +class RemoveTaxRateFromShippingRate < ActiveRecord::Migration + class Spree::ShippingRate < Spree::Base; end + class Spree::TaxRate < Spree::Base + has_one :calculator, class_name: "Spree::Calculator", as: :calculable, inverse_of: :calculable, dependent: :destroy, autosave: true + def compute_amount(item) + calculator.compute(item) + end + end + + def up + say_with_time "Pre-calculating taxes for existing shipping rates" do + Spree::ShippingRate.find_each do |shipping_rate| + tax_rate_id = shipping_rate.tax_rate_id + if tax_rate_id + tax_rate = Spree::TaxRate.unscoped.find_by(shipping_rate.tax_rate_id) + shipping_rate.taxes.create( + tax_rate: tax_rate, + amount: tax_rate.compute_amount(shipping_rate) + ) + end + end + end + + remove_column :spree_shipping_rates, :tax_rate_id + end + + def down + add_reference :spree_shipping_rates, :tax_rate, index: true, foreign_key: true + say_with_time "Attaching a tax rate to shipping rates" do + Spree::ShippingRate.find_each do |shipping_rate| + shipping_taxes = Spree::ShippingRateTax.where(shipping_rate_id: shipping_rate.id) + # We can only use one tax rate, so let's take the biggest. + selected_tax = shipping_taxes.sort_by(&:amount).last + if selected_tax + shipping_rate.update(tax_rate_id: tax_rate_id) + end + end + end + end +end diff --git a/core/spec/models/spree/shipping_rate_spec.rb b/core/spec/models/spree/shipping_rate_spec.rb index 11cff882506..52521198bca 100644 --- a/core/spec/models/spree/shipping_rate_spec.rb +++ b/core/spec/models/spree/shipping_rate_spec.rb @@ -170,25 +170,6 @@ end end - context "#tax_rate" do - let!(:tax_rate) { create(:tax_rate) } - - before do - shipping_rate.tax_rate = tax_rate - end - - it "can be retrieved" do - expect(shipping_rate.tax_rate.reload).to eq(tax_rate) - end - - it "can be retrieved even when deleted" do - tax_rate.update_column(:deleted_at, Time.current) - shipping_rate.save - shipping_rate.reload - expect(shipping_rate.tax_rate).to eq(tax_rate) - end - end - context "#shipping_method_code" do before do shipping_method.code = "THE_CODE" diff --git a/core/spec/models/spree/stock/estimator_spec.rb b/core/spec/models/spree/stock/estimator_spec.rb index 597201565a1..a82d1a093ad 100644 --- a/core/spec/models/spree/stock/estimator_spec.rb +++ b/core/spec/models/spree/stock/estimator_spec.rb @@ -148,7 +148,7 @@ module Stock it "links the shipping rate and the tax rate" do shipping_rates = subject.shipping_rates(package) - expect(shipping_rates.first.tax_rate).to eq(tax_rate) + expect(shipping_rates.first.taxes.first.tax_rate).to eq(tax_rate) end end