Refactor part pricing updates (#12787) (#12791)

* Refactor part pricing updates

- Handles cases where underlying part has been deleted

* Add regression tests

(cherry picked from commit fa44925b21)

Co-authored-by: Oliver <oliver.henry.walters@gmail.com>

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 (
+        _part = self.get_part()
-            not self.part
+
-            or not self.part.pk
+        if not _part:
-            or not Part.objects.filter(pk=self.part.pk).exists()
-        ):
             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(
+            logger.warning("Error refreshing PartPricing instance for part '%s'", _part)
-                "Error refreshing PartPricing instance for part '%s'", self.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:
+        if _part.is_template:
-            variants = self.part.get_descendants(include_self=False)
+            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:

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."""