diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index 998e9d964b..7cdcb879eb 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -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: diff --git a/src/backend/InvenTree/part/test_pricing.py b/src/backend/InvenTree/part/test_pricing.py index 476fbd93c4..8bdbfc31c4 100644 --- a/src/backend/InvenTree/part/test_pricing.py +++ b/src/backend/InvenTree/part/test_pricing.py @@ -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."""