From 344fbcea19f8848f0a40d8a341589f22de499e31 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 29 Jun 2024 00:21:35 +0000 Subject: [PATCH 1/5] Calculate weighted average price when merging stock items --- src/backend/InvenTree/stock/models.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index f587cdf71f6e..bb394fa2995f 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -20,6 +20,7 @@ from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from djmoney.contrib.exchange.models import convert_money from mptt.managers import TreeManager from mptt.models import MPTTModel, TreeForeignKey from taggit.managers import TaggableManager @@ -1706,6 +1707,9 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): parent_id = self.parent.pk if self.parent else None + # Keep track of the total cost of the items, to update the average cost + total_price = self.purchase_price * self.quantity + for other in other_items: # If the stock item cannot be merged, return if not self.can_merge(other, raise_error=raise_error, **kwargs): @@ -1716,9 +1720,19 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): tree_ids.add(other.tree_id) - for other in other_items: self.quantity += other.quantity + try: + # Update the total price (and try to account for differences in currency) + item_price = other.purchase_price * other.quantity + item_price = convert_money(item_price, self.purchase_price.currency) + except Exception: + # In the case where we cannot convert, use this item's price + item_price = self.purchase_price * other.quantity + + # Update total price + total_price += item_price + # Any "build order allocations" for the other item must be assigned to this one for allocation in other.allocations.all(): allocation.stock_item = self @@ -1744,7 +1758,12 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): deltas={'location': location.pk if location else None}, ) + # Update the location of the item self.location = location + + # Update the unit price + self.purchase_price = total_price / self.quantity + self.save() # Rebuild stock trees as required From 15be688cbf7d7bba3084f0e6235dcbec87d653d1 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 29 Jun 2024 07:02:52 +0000 Subject: [PATCH 2/5] refactor currency averaging - Only add samples which have an associated value --- src/backend/InvenTree/stock/models.py | 42 ++++++++++++++++++--------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index bb394fa2995f..913987f69a04 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -1707,8 +1707,11 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): parent_id = self.parent.pk if self.parent else None - # Keep track of the total cost of the items, to update the average cost - total_price = self.purchase_price * self.quantity + # Keep track of pricing data for the merged data + pricing_data = [] + + if self.purchase_price: + pricing_data.append([self.purchase_price, self.quantity]) for other in other_items: # If the stock item cannot be merged, return @@ -1722,16 +1725,9 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): self.quantity += other.quantity - try: - # Update the total price (and try to account for differences in currency) - item_price = other.purchase_price * other.quantity - item_price = convert_money(item_price, self.purchase_price.currency) - except Exception: - # In the case where we cannot convert, use this item's price - item_price = self.purchase_price * other.quantity - - # Update total price - total_price += item_price + if other.purchase_price: + # Only add pricing data if it is available + pricing_data.append([other.purchase_price, other.quantity]) # Any "build order allocations" for the other item must be assigned to this one for allocation in other.allocations.all(): @@ -1761,8 +1757,26 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): # Update the location of the item self.location = location - # Update the unit price - self.purchase_price = total_price / self.quantity + # Update the unit price - calculate weighted average of available pricing data + if len(pricing_data) > 0: + unit_price, quantity = pricing_data[0] + + # Use the first currency as the base currency + base_currency = unit_price.currency + + total_price = unit_price * quantity + + for price, qty in pricing_data[1:]: + # Attempt to convert the price to the base currency + try: + price = convert_money(price, base_currency) + total_price += price * qty + quantity += qty + except: + # Skip this entry, cannot convert to base currency + continue + + self.purchase_price = total_price / quantity self.save() From 56c061721594e10cb33a96aad4c4e99fc8897c51 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 29 Jun 2024 07:06:58 +0000 Subject: [PATCH 3/5] Revert to using two loops --- src/backend/InvenTree/stock/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index 913987f69a04..d419859d9604 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -1721,6 +1721,7 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): ) return + for other in other_items: tree_ids.add(other.tree_id) self.quantity += other.quantity From fce7a86ca44c877538fbcbc9b3cfdaeffa4300ce Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 29 Jun 2024 07:07:43 +0000 Subject: [PATCH 4/5] Check for div-by-zero --- src/backend/InvenTree/stock/models.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index d419859d9604..72ba1db44120 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -1777,7 +1777,8 @@ def merge_stock_items(self, other_items, raise_error=False, **kwargs): # Skip this entry, cannot convert to base currency continue - self.purchase_price = total_price / quantity + if quantity > 0: + self.purchase_price = total_price / quantity self.save() From dbd9f4eafde463dbdb5c48354b3f0044748d667a Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 29 Jun 2024 10:02:43 +0000 Subject: [PATCH 5/5] Add unit testing for purchase price averaging --- src/backend/InvenTree/stock/tests.py | 65 ++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/src/backend/InvenTree/stock/tests.py b/src/backend/InvenTree/stock/tests.py index fe7a2b0e9d97..99f6f51a4876 100644 --- a/src/backend/InvenTree/stock/tests.py +++ b/src/backend/InvenTree/stock/tests.py @@ -755,23 +755,14 @@ def test_location_tree(self): # First, we will create a stock location structure A = StockLocation.objects.create(name='A', description='Top level location') - B1 = StockLocation.objects.create(name='B1', parent=A) - B2 = StockLocation.objects.create(name='B2', parent=A) - B3 = StockLocation.objects.create(name='B3', parent=A) - C11 = StockLocation.objects.create(name='C11', parent=B1) - C12 = StockLocation.objects.create(name='C12', parent=B1) - C21 = StockLocation.objects.create(name='C21', parent=B2) - C22 = StockLocation.objects.create(name='C22', parent=B2) - C31 = StockLocation.objects.create(name='C31', parent=B3) - C32 = StockLocation.objects.create(name='C32', parent=B3) # Check that the tree_id is correct for each sublocation @@ -895,6 +886,62 @@ def test_metadata(self): self.assertEqual(len(p.metadata.keys()), 4) + def test_merge(self): + """Test merging of multiple stock items.""" + from djmoney.money import Money + + part = Part.objects.first() + part.stock_items.all().delete() + + # Test simple merge without any pricing information + s1 = StockItem.objects.create(part=part, quantity=10) + s2 = StockItem.objects.create(part=part, quantity=20) + s3 = StockItem.objects.create(part=part, quantity=30) + + self.assertEqual(part.stock_items.count(), 3) + s1.merge_stock_items([s2, s3]) + self.assertEqual(part.stock_items.count(), 1) + s1.refresh_from_db() + self.assertEqual(s1.quantity, 60) + self.assertIsNone(s1.purchase_price) + + part.stock_items.all().delete() + + # Create some stock items with pricing information + s1 = StockItem.objects.create(part=part, quantity=10, purchase_price=None) + s2 = StockItem.objects.create( + part=part, quantity=15, purchase_price=Money(10, 'USD') + ) + s3 = StockItem.objects.create(part=part, quantity=30) + + self.assertEqual(part.stock_items.count(), 3) + s1.merge_stock_items([s2, s3]) + self.assertEqual(part.stock_items.count(), 1) + s1.refresh_from_db() + self.assertEqual(s1.quantity, 55) + self.assertEqual(s1.purchase_price, Money(10, 'USD')) + + part.stock_items.all().delete() + + s1 = StockItem.objects.create( + part=part, quantity=10, purchase_price=Money(5, 'USD') + ) + s2 = StockItem.objects.create( + part=part, quantity=25, purchase_price=Money(10, 'USD') + ) + s3 = StockItem.objects.create( + part=part, quantity=5, purchase_price=Money(75, 'USD') + ) + + self.assertEqual(part.stock_items.count(), 3) + s1.merge_stock_items([s2, s3]) + self.assertEqual(part.stock_items.count(), 1) + s1.refresh_from_db() + self.assertEqual(s1.quantity, 40) + + # Final purchase price should be the weighted average + self.assertAlmostEqual(s1.purchase_price.amount, 16.875, places=3) + class StockBarcodeTest(StockTestBase): """Run barcode tests for the stock app."""