Skip to content

Commit

Permalink
Remove code that relates to Spree::ShippingRate#tax_rate
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mamhoff committed Apr 6, 2016
1 parent 28de8e5 commit 2426c37
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 40 deletions.
10 changes: 1 addition & 9 deletions core/app/models/spree/shipping_rate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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

Expand All @@ -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
Expand Down
11 changes: 0 additions & 11 deletions core/app/models/spree/stock/estimator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
19 changes: 0 additions & 19 deletions core/spec/models/spree/shipping_rate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/stock/estimator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 2426c37

Please sign in to comment.