Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Tax extension point and merge tax updating code #1479

Merged
merged 5 commits into from
Oct 31, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Merge tax updating code and deprecate Order#create_tax_charge!
Previously we had two different sets of tax updating code.  One was
Order#create_tax_charge! and the other was the code in OrderUpdater.
The effect of the former encompasses the effect of the latter, so
functionally we can merge these.

This may result in increased load since we're now using the more
encompassing method which was more heavyweight.
  • Loading branch information
jordan-brough committed Oct 12, 2016
commit 7e0ba4277b014955480dd0ec47e4b8846af34477
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* Remove callback `Spree::LineItem.after_create :update_tax_charge`

Any code that creates `LineItem`s outside the context of OrderContents
should ensure that it calls `order.create_tax_charge!` after doing so.
should ensure that it calls `order.update!` after doing so.

* Mark `Spree::Tax::ItemAdjuster` as api-private

Expand Down
4 changes: 3 additions & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module Spree
# `checkout_allowed?` or `payment_required?`.
#
# * Implements an interface for mutating the order with methods like
# `create_tax_charge!` and `fulfill!`.
# `empty!` and `fulfill!`.
#
class Order < Spree::Base
ORDER_NUMBER_LENGTH = 9
Expand Down Expand Up @@ -348,9 +348,11 @@ def line_item_options_match(line_item, options)

# Creates new tax charges if there are any applicable rates. If prices already
# include taxes then price adjustments are created instead.
# @deprecated This now happens during #update!
def create_tax_charge!
Spree::Tax::OrderAdjuster.new(self).adjust!
end
deprecate create_tax_charge!: :update!, deprecator: Spree::Deprecation

def outstanding_balance
# If reimbursement has happened add it back to total to prevent balance_due payment state
Expand Down
2 changes: 0 additions & 2 deletions core/app/models/spree/order/checkout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def self.define_state_machine!

after_transition to: :complete, do: :add_payment_sources_to_wallet
before_transition to: :payment, do: :set_shipments_cost
before_transition to: :payment, do: :create_tax_charge!
before_transition to: :payment, do: :assign_default_credit_card

before_transition to: :confirm, do: :add_store_credit_payments
Expand All @@ -90,7 +89,6 @@ def self.define_state_machine!
before_transition from: :cart, do: :ensure_line_items_present

if states[:address]
before_transition from: :address, do: :create_tax_charge!
before_transition to: :address, do: :assign_default_addresses!
before_transition from: :address, do: :persist_user_address!
end
Expand Down
3 changes: 0 additions & 3 deletions core/app/models/spree/order_contents.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ def remove_line_item(line_item, options = {})

def update_cart(params)
if order.update_attributes(params)
order.create_tax_charge!

unless order.completed?
order.line_items = order.line_items.select { |li| li.quantity > 0 }
# Update totals, then check if the order is eligible for any cart promotions.
Expand Down Expand Up @@ -77,7 +75,6 @@ def after_add_or_remove(line_item, options = {})
shipment = options[:shipment]
shipment.present? ? shipment.update_amounts : order.ensure_updated_shipments
PromotionHandler::Cart.new(order, line_item).activate
order.create_tax_charge!
reload_totals
line_item
end
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/order_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ def update_order_promotions
end

def update_taxes
Spree::Tax::OrderAdjuster.new(order).adjust!

[*line_items, *shipments].each do |item|
tax_adjustments = item.adjustments.select(&:tax?)

tax_adjustments.each(&:update!)
# Tax adjustments come in not one but *two* exciting flavours:
# Included & additional

Expand Down
1 change: 0 additions & 1 deletion core/lib/spree/core/unreturned_item_charger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def charge_for_items
new_order.associate_user!(@original_order.user) if @original_order.user

add_exchange_variants_to_order
new_order.create_tax_charge!
set_shipment_for_new_order

new_order.update!
Expand Down
1 change: 0 additions & 1 deletion core/lib/spree/testing_support/factories/order_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
create(:line_item, attributes)
end
order.line_items.reload
order.create_tax_charge!

create(:shipment, order: order, cost: evaluator.shipment_cost, shipping_method: evaluator.shipping_method, stock_location: evaluator.stock_location)
order.shipments.reload
Expand Down
1 change: 0 additions & 1 deletion core/spec/lib/spree/core/unreturned_item_charger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@

it "applies tax" do
exchange_order = exchange_shipment.order
exchange_order.create_tax_charge!
exchange_order.update!
subject
expect(new_order.additional_tax_total).to be > 0
Expand Down
5 changes: 0 additions & 5 deletions core/spec/models/spree/order_contents_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@
}.to change { subject.order.total }
end

it "updates tax adjustments" do
expect(subject.order).to receive(:create_tax_charge!)
subject.update_cart params
end

context "submits item quantity 0" do
let(:params) do
{ line_items_attributes: {
Expand Down