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

Remove update_adjustments AR callbacks #1356

Merged
merged 2 commits into from
Aug 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@

* Removed "Clear cache" button from the admin [#1275](https://github.com/solidusio/solidus/pull/1275)

* Adjustments and totals are no longer updated when saving a Shipment or LineItem.

Previously adjustments and total columns were updated after saving a Shipment or LineItem.
This was unnecessary since it didn't update the order totals, and running
order.update! would recalculate the adjustments and totals again.

## Solidus 1.3.0 (unreleased)

* Order now requires a `store_id` in validations
Expand Down
12 changes: 0 additions & 12 deletions core/app/models/spree/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class LineItem < Spree::Base
after_create :update_tax_charge

after_save :update_inventory
after_save :update_adjustments

before_destroy :update_inventory
before_destroy :destroy_inventory_units
Expand Down Expand Up @@ -167,17 +166,6 @@ def destroy_inventory_units
inventory_units.destroy_all
end

def update_adjustments
if quantity_changed?
update_tax_charge # Called to ensure pre_tax_amount is updated.
recalculate_adjustments
end
end

def recalculate_adjustments
Spree::ItemAdjustments.new(self).update
end

def update_tax_charge
Spree::Tax::ItemAdjuster.new(self).adjust!
end
Expand Down
12 changes: 0 additions & 12 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ class Shipment < Spree::Base
has_many :state_changes, as: :stateful
has_many :cartons, -> { uniq }, through: :inventory_units

after_save :update_adjustments

before_validation :set_cost_zero_when_nil

before_destroy :ensure_can_destroy
Expand Down Expand Up @@ -404,20 +402,10 @@ def manifest_unstock(item)
stock_location.unstock item.variant, item.quantity, self
end

def recalculate_adjustments
Spree::ItemAdjustments.new(self).update
end

def set_cost_zero_when_nil
self.cost = 0 unless cost
end

def update_adjustments
if cost_changed? && state != 'shipped'
recalculate_adjustments
end
end

def ensure_can_destroy
unless pending?
errors.add(:state, :cannot_destroy, state: state)
Expand Down
19 changes: 0 additions & 19 deletions core/spec/models/spree/line_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,6 @@
end

context "#save" do
context "line item changes" do
before do
line_item.quantity = line_item.quantity + 1
end

it "triggers adjustment total recalculation" do
expect(line_item).to receive(:update_tax_charge) # Regression test for https://github.com/spree/spree/issues/4671
expect(line_item).to receive(:recalculate_adjustments)
line_item.save
end
end

context "line item does not change" do
it "does not trigger adjustment total recalculation" do
expect(line_item).not_to receive(:recalculate_adjustments)
line_item.save
end
end

context "target_shipment is provided" do
it "verifies inventory" do
line_item.target_shipment = Spree::Shipment.new
Expand Down
26 changes: 0 additions & 26 deletions core/spec/models/spree/shipment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -585,32 +585,6 @@
end
end

context "after_save" do
context "line item changes" do
before do
shipment.cost = shipment.cost + 10
end

it "triggers adjustment total recalculation" do
expect(shipment).to receive(:recalculate_adjustments)
shipment.save
end

it "does not trigger adjustment recalculation if shipment has shipped" do
shipment.state = 'shipped'
expect(shipment).not_to receive(:recalculate_adjustments)
shipment.save
end
end

context "line item does not change" do
it "does not trigger adjustment total recalculation" do
expect(shipment).not_to receive(:recalculate_adjustments)
shipment.save
end
end
end

context "currency" do
it "returns the order currency" do
expect(shipment.currency).to eq(order.currency)
Expand Down