mirror of
https://github.com/inventree/InvenTree.git
synced 2026-09-09 22:30:17 +00:00
Refactor part pricing updates (#12787)
* Refactor part pricing updates - Handles cases where underlying part has been deleted * Add regression tests
This commit is contained in:
@@ -2637,6 +2637,20 @@ class PartPricing(common.models.MetaMixin):
|
||||
"""Return True if the cached pricing is valid."""
|
||||
return self.updated is not None
|
||||
|
||||
def get_part(self):
|
||||
"""Return the Part instance associated with this pricing object.
|
||||
|
||||
If the part does not exist (i.e. has been deleted), return None.
|
||||
"""
|
||||
from part.models import Part
|
||||
|
||||
try:
|
||||
part = self.part
|
||||
part.refresh_from_db()
|
||||
return part
|
||||
except (Part.DoesNotExist, IntegrityError):
|
||||
return None
|
||||
|
||||
def convert(self, money):
|
||||
"""Attempt to convert money value to default currency.
|
||||
|
||||
@@ -2695,11 +2709,9 @@ class PartPricing(common.models.MetaMixin):
|
||||
if InvenTree.ready.isRunningMigrations():
|
||||
return
|
||||
|
||||
if (
|
||||
not self.part
|
||||
or not self.part.pk
|
||||
or not Part.objects.filter(pk=self.part.pk).exists()
|
||||
):
|
||||
_part = self.get_part()
|
||||
|
||||
if not _part:
|
||||
logger.warning(
|
||||
'Referenced part instance does not exist - skipping pricing update.'
|
||||
)
|
||||
@@ -2710,31 +2722,30 @@ class PartPricing(common.models.MetaMixin):
|
||||
self.refresh_from_db()
|
||||
except (PartPricing.DoesNotExist, IntegrityError):
|
||||
# Error thrown if this PartPricing instance has already been removed
|
||||
logger.warning(
|
||||
"Error refreshing PartPricing instance for part '%s'", self.part
|
||||
)
|
||||
logger.warning("Error refreshing PartPricing instance for part '%s'", _part)
|
||||
return
|
||||
|
||||
# Ensure that the referenced part still exists in the database
|
||||
try:
|
||||
p = self.part
|
||||
if True: # refresh and p.pk:
|
||||
p.refresh_from_db()
|
||||
_part.refresh_from_db()
|
||||
except IntegrityError:
|
||||
logger.exception(
|
||||
"Could not update PartPricing as Part '%s' does not exist", self.part
|
||||
"Could not update PartPricing as Part '%s' does not exist", _part
|
||||
)
|
||||
return
|
||||
|
||||
if self.scheduled_for_update:
|
||||
# Ignore if the pricing is already scheduled to be updated
|
||||
logger.debug('Pricing for %s already scheduled for update - skipping', p)
|
||||
logger.debug(
|
||||
'Pricing for %s already scheduled for update - skipping', _part
|
||||
)
|
||||
return
|
||||
|
||||
if counter > self.MAX_PRICING_DEPTH:
|
||||
# Prevent infinite recursion / stack depth issues
|
||||
logger.debug(
|
||||
counter, f'Skipping pricing update for {p} - maximum depth exceeded'
|
||||
counter, f'Skipping pricing update for {_part} - maximum depth exceeded'
|
||||
)
|
||||
return
|
||||
|
||||
@@ -2744,7 +2755,7 @@ class PartPricing(common.models.MetaMixin):
|
||||
except IntegrityError:
|
||||
# An IntegrityError here likely indicates that the referenced part has already been deleted
|
||||
logger.exception(
|
||||
"Could not save PartPricing for part '%s' to the database", self.part
|
||||
"Could not save PartPricing for part '%s' to the database", _part
|
||||
)
|
||||
return
|
||||
|
||||
@@ -2828,30 +2839,39 @@ class PartPricing(common.models.MetaMixin):
|
||||
"""Schedule updates for any assemblies which use this part."""
|
||||
# If the linked Part is used in any assemblies, schedule a pricing update for those assemblies
|
||||
|
||||
used_in_parts = self.part.get_used_in()
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
used_in_parts = _part.get_used_in()
|
||||
|
||||
for p in used_in_parts:
|
||||
p.pricing.schedule_for_update(counter=counter + 1)
|
||||
|
||||
def update_templates(self, counter: int = 0):
|
||||
"""Schedule updates for any template parts above this part."""
|
||||
templates = self.part.get_ancestors(include_self=False)
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
templates = _part.get_ancestors(include_self=False)
|
||||
|
||||
for p in templates:
|
||||
p.pricing.schedule_for_update(counter + 1)
|
||||
p.pricing.schedule_for_update(counter=counter + 1)
|
||||
|
||||
def save(self, *args, **kwargs):
|
||||
"""Whenever pricing model is saved, automatically update overall prices."""
|
||||
# Update the currency which was used to perform the calculation
|
||||
self.currency = currency_code_default()
|
||||
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
try:
|
||||
self.update_overall_cost()
|
||||
super().save(*args, **kwargs)
|
||||
except Exception:
|
||||
log_error('PartPricing.save')
|
||||
logger.error(
|
||||
"Could not save PartPricing for part '%s' to the database", self.part
|
||||
"Could not save PartPricing for part '%s' to the database", _part
|
||||
)
|
||||
|
||||
def update_bom_cost(self, save=True):
|
||||
@@ -2864,7 +2884,10 @@ class PartPricing(common.models.MetaMixin):
|
||||
|
||||
Note: The cumulative costs are calculated based on the specified default currency
|
||||
"""
|
||||
if not self.part.assembly:
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
if not _part.assembly:
|
||||
# Not an assembly - no BOM pricing
|
||||
self.bom_cost_min = None
|
||||
self.bom_cost_max = None
|
||||
@@ -2883,7 +2906,7 @@ class PartPricing(common.models.MetaMixin):
|
||||
any_min_elements = False
|
||||
any_max_elements = False
|
||||
|
||||
for bom_item in self.part.get_bom_items():
|
||||
for bom_item in _part.get_bom_items():
|
||||
# Loop through each BOM item which is used to assemble this part
|
||||
|
||||
bom_item_min = None
|
||||
@@ -2939,11 +2962,14 @@ class PartPricing(common.models.MetaMixin):
|
||||
|
||||
Purchase history only takes into account "completed" purchase orders.
|
||||
"""
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
# Find all line items for completed orders which reference this part
|
||||
line_items = OrderModels.PurchaseOrderLineItem.objects.filter(
|
||||
order__status=PurchaseOrderStatus.COMPLETE.value,
|
||||
received__gt=0,
|
||||
part__part=self.part,
|
||||
part__part=_part,
|
||||
)
|
||||
|
||||
# Exclude line items which do not have an associated price
|
||||
@@ -3002,12 +3028,15 @@ class PartPricing(common.models.MetaMixin):
|
||||
|
||||
def update_internal_cost(self, save=True):
|
||||
"""Recalculate internal cost for the referenced Part instance."""
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
min_int_cost = None
|
||||
max_int_cost = None
|
||||
|
||||
if get_global_setting('PART_INTERNAL_PRICE', False):
|
||||
# Only calculate internal pricing if internal pricing is enabled
|
||||
for pb in self.part.internalpricebreaks.all():
|
||||
for pb in _part.internalpricebreaks.all():
|
||||
cost = self.convert(pb.price)
|
||||
|
||||
if cost is None:
|
||||
@@ -3032,12 +3061,15 @@ class PartPricing(common.models.MetaMixin):
|
||||
- The limits are simply the lower and upper bounds of available SupplierPriceBreaks
|
||||
- We do not take "quantity" into account here
|
||||
"""
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
min_sup_cost = None
|
||||
max_sup_cost = None
|
||||
|
||||
if self.part.purchaseable:
|
||||
if _part.purchaseable:
|
||||
# Iterate through each available SupplierPart instance
|
||||
for sp in self.part.supplier_parts.all():
|
||||
for sp in _part.supplier_parts.all():
|
||||
# Iterate through each available SupplierPriceBreak instance
|
||||
for pb in sp.pricebreaks.all():
|
||||
if pb.price is None:
|
||||
@@ -3066,13 +3098,16 @@ class PartPricing(common.models.MetaMixin):
|
||||
|
||||
Here we track the min/max costs of any variant parts.
|
||||
"""
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
variant_min = None
|
||||
variant_max = None
|
||||
|
||||
active_only = get_global_setting('PRICING_ACTIVE_VARIANTS', False)
|
||||
|
||||
if self.part.is_template:
|
||||
variants = self.part.get_descendants(include_self=False)
|
||||
if _part.is_template:
|
||||
variants = _part.get_descendants(include_self=False)
|
||||
|
||||
for v in variants:
|
||||
if active_only and not v.active:
|
||||
@@ -3167,11 +3202,14 @@ class PartPricing(common.models.MetaMixin):
|
||||
|
||||
def update_sale_cost(self, save=True):
|
||||
"""Recalculate sale cost data."""
|
||||
if not (_part := self.get_part()):
|
||||
return
|
||||
|
||||
# Iterate through the sell price breaks
|
||||
min_sell_price = None
|
||||
max_sell_price = None
|
||||
|
||||
for pb in self.part.salepricebreaks.all():
|
||||
for pb in _part.salepricebreaks.all():
|
||||
cost = self.convert(pb.price)
|
||||
|
||||
if cost is None:
|
||||
|
||||
@@ -534,6 +534,68 @@ class PartPricingTests(InvenTreeTestCase):
|
||||
p.schedule_pricing_update(create=False)
|
||||
self.assertFalse(part.models.PartPricing.objects.filter(part_id=p.pk).exists())
|
||||
|
||||
@override_settings(TESTING_PRICING=True)
|
||||
def test_get_part_deleted(self):
|
||||
"""Test that PartPricing.get_part() returns None if the linked Part has been deleted.
|
||||
|
||||
Regression test for a bug where a PartPricing instance which still held an
|
||||
in-memory reference to its linked Part (e.g. as passed into the background
|
||||
pricing update task) would raise Part.DoesNotExist when that Part had since
|
||||
been deleted from the database, instead of failing gracefully.
|
||||
"""
|
||||
p = part.models.Part.objects.create(
|
||||
name='Deletable Part', description='A part which will be deleted'
|
||||
)
|
||||
|
||||
# Accessing the 'pricing' property caches the (still in-memory) Part
|
||||
# instance against the 'part' relation of the new PartPricing object
|
||||
pricing = p.pricing
|
||||
self.assertIsNone(pricing.pk)
|
||||
self.assertEqual(pricing.get_part(), p)
|
||||
|
||||
# Remove the underlying part directly via a queryset delete
|
||||
# (this bypasses Part.delete(), and cascades to remove any PartPricing row)
|
||||
part.models.Part.objects.filter(pk=p.pk).delete()
|
||||
|
||||
# The 'pricing' object still holds a stale in-memory reference to the
|
||||
# now-deleted part - get_part() must detect this and return None
|
||||
self.assertIsNone(pricing.get_part())
|
||||
|
||||
@override_settings(TESTING_PRICING=True)
|
||||
def test_pricing_methods_with_deleted_part(self):
|
||||
"""Test that PartPricing update methods handle a deleted underlying Part gracefully.
|
||||
|
||||
Regression test: none of these methods should raise Part.DoesNotExist
|
||||
(or otherwise error) if the linked Part no longer exists in the database.
|
||||
"""
|
||||
p = part.models.Part.objects.create(
|
||||
name='Deletable Assembly',
|
||||
description='A part which will be deleted',
|
||||
assembly=True,
|
||||
)
|
||||
|
||||
# Do not save this yet - simulate a pricing update which is still in-flight
|
||||
# (e.g. queued as a background task) when the linked part is deleted
|
||||
pricing = p.pricing
|
||||
|
||||
part.models.Part.objects.filter(pk=p.pk).delete()
|
||||
|
||||
# None of the following should raise an exception
|
||||
pricing.update_bom_cost()
|
||||
pricing.update_purchase_cost()
|
||||
pricing.update_internal_cost()
|
||||
pricing.update_supplier_cost()
|
||||
pricing.update_variant_cost()
|
||||
pricing.update_sale_cost()
|
||||
pricing.update_assemblies()
|
||||
pricing.update_templates()
|
||||
pricing.schedule_for_update()
|
||||
pricing.save()
|
||||
pricing.update_pricing()
|
||||
|
||||
# As the part no longer exists, no PartPricing row should have been created
|
||||
self.assertFalse(part.models.PartPricing.objects.filter(part_id=p.pk).exists())
|
||||
|
||||
@override_settings(TESTING_PRICING=True)
|
||||
def test_multi_level_bom(self):
|
||||
"""Test that pricing for multi-level BOMs is calculated correctly."""
|
||||
|
||||
Reference in New Issue
Block a user