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

Fix issue where OrderInventory creates superfluous InventoryUnits #1751

Merged
merged 6 commits into from
Apr 10, 2017

Conversation

jhawthorn
Copy link
Contributor

This fixes an issue where OrderInventory would miscount the number of existing inventory_units on a line item and create extra records.

InventoryUnits exist through the line_item.inventory_units, shipment.inventory_units, and order.inventory_units associations. Because of this, their associations can't be kept up to date in memory.

OrderInventory was counting inventory using inventory_units.size, which would return the in-memory count if the line_items.inventory_units association was loaded.

This issue was exposed by adding to the same line item multiple times without reloading the order. The line_item.inventory_units association would be become loaded?, because ActiveRecord's #size marks a record as loaded if the count returns 0 (TIL!). On the next pass it would see the wrong size because the association was out of date, and add two inventory units instead of one.

This avoids the issue by counting inventory using inventory_units.count, which doesn't consider the in-memory association.

@jhawthorn jhawthorn changed the title Fix creating superfluous InventoryUnits Fix issue where OrderInventory creates superfluous InventoryUnits Mar 1, 2017
Copy link

@luukveenis luukveenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🎉

@jhawthorn
Copy link
Contributor Author

jhawthorn commented Mar 1, 2017

Fixed a second issue while investigating this. Previously, when the inventory count reached 0, it would call shipment.destroy, which would leave the shipment in the in-memory order.shipments association.

This could later cause an order.update! to fail, as it would attempt to update a destroyed shipment.

@jhawthorn jhawthorn force-pushed the order_inventory_inventory_units branch 2 times, most recently from d910511 to e0fef9f Compare March 1, 2017 20:45
jhawthorn and others added 6 commits March 16, 2017 14:03
This fixes an issue where OrderInventory would miscount the number of
existing inventory_units on a line item and create extra records.

InventoryUnits exist through the line_item.inventory_units,
shipment.inventory_units, and order.inventory_units associations.
Because of this, their associations can't be kept up to date in memory.

OrderInventory was counting inventory using inventory_units.size, which
would return the in-memory count if the line_items.inventory_units
association was loaded.

This issue was exposed by adding to the same line item mutliple times
without reloading the order. The line_item.inventory_units association
would be become `loaded?`, because ActiveRecord's `#size` marks a record
as loaded if the count returns 0 (TIL!). On the next pass it would see
the wrong size because the association was out of date, and add two
inventory units instead of one.

This avoids the issue by counting inventory using inventory_units.count,
which doesn't consider the in-memory association.
@jhawthorn jhawthorn added this to the 2.2.0 milestone Apr 10, 2017
@jhawthorn jhawthorn merged commit 70a756c into solidusio:master Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants