Skip to content

Commit

Permalink
Let Order#all_inventory_units_returned? always use fresh data
Browse files Browse the repository at this point in the history
This method is mainly used after inventory items are returned, and
that usually happens through customer_returns -> return_item ->
inventory_unit making the instances of InventoryUnit that are local to
the order stale. By using reload we avoid the bugs stemming from this
discrepancy.
  • Loading branch information
elia committed Jan 28, 2020
1 parent 5976dae commit 88b1220
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
7 changes: 6 additions & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,12 @@ def allow_cancel?
end

def all_inventory_units_returned?
inventory_units.all?(&:returned?)
# Inventory units are transitioned to the return state by ReturnItem
# across a different association path goes through CustomerReturn and
# ReturnItem instead of using Order#inventory_units. That makes any
# the cached association on order stale and require a *reload* before
# checking if they're returned.
inventory_units.reload.all?(&:returned?)
end

def contents
Expand Down
12 changes: 12 additions & 0 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,18 @@ def generate
expect(subject).to eq false
end
end

context "all inventory units are returned on the database (e.g. through another association)" do
it "is true" do
expect {
Spree::InventoryUnit
.where(id: order.inventory_unit_ids)
.update_all(state: 'returned')
}.to change {
order.all_inventory_units_returned?
}.from(false).to(true)
end
end
end

context "store credit" do
Expand Down

0 comments on commit 88b1220

Please sign in to comment.