mirror of
https://github.com/inventree/InvenTree
synced 2024-08-30 18:33:04 +00:00
Pricing bug fix (#4422)
* Control when a new PartPricing object can be created - Prevent this when calling from an on_delete signal - There is an edge case where deleting a part triggers a series of on_delete signals inside an atomic transaction - When the new PartPricing object is created, * Add unit testing: - Ensure PartPricing gets created when a new StockItem is added - Part.delete() works without error - PartPricing instances are deleted also * style fixes
This commit is contained in:
parent
0c5dc2865c
commit
b657fb4405
@ -23,7 +23,7 @@ def migrate_currencies(apps, schema_editor):
|
||||
for the SupplierPriceBreak model, to a new django-money compatible currency.
|
||||
"""
|
||||
|
||||
logger.info("Updating currency references for SupplierPriceBreak model...")
|
||||
logger.debug("Updating currency references for SupplierPriceBreak model...")
|
||||
|
||||
# A list of available currency codes
|
||||
currency_codes = CURRENCIES.keys()
|
||||
|
@ -722,7 +722,7 @@ def after_save_supplier_price(sender, instance, created, **kwargs):
|
||||
if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
|
||||
|
||||
if instance.part and instance.part.part:
|
||||
instance.part.part.schedule_pricing_update()
|
||||
instance.part.part.schedule_pricing_update(create=True)
|
||||
|
||||
|
||||
@receiver(post_delete, sender=SupplierPriceBreak, dispatch_uid='post_delete_supplier_price_break')
|
||||
@ -732,4 +732,4 @@ def after_delete_supplier_price(sender, instance, **kwargs):
|
||||
if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
|
||||
|
||||
if instance.part and instance.part.part:
|
||||
instance.part.part.schedule_pricing_update()
|
||||
instance.part.part.schedule_pricing_update(create=False)
|
||||
|
@ -389,7 +389,7 @@ class PurchaseOrder(Order):
|
||||
# Schedule pricing update for any referenced parts
|
||||
for line in self.lines.all():
|
||||
if line.part and line.part.part:
|
||||
line.part.part.schedule_pricing_update()
|
||||
line.part.part.schedule_pricing_update(create=True)
|
||||
|
||||
trigger_event('purchaseorder.completed', id=self.pk)
|
||||
|
||||
@ -782,7 +782,7 @@ class SalesOrder(Order):
|
||||
|
||||
# Schedule pricing update for any referenced parts
|
||||
for line in self.lines.all():
|
||||
line.part.schedule_pricing_update()
|
||||
line.part.schedule_pricing_update(create=True)
|
||||
|
||||
trigger_event('salesorder.completed', id=self.pk)
|
||||
|
||||
|
@ -24,7 +24,7 @@ def migrate_currencies(apps, schema_editor):
|
||||
for the SupplierPriceBreak model, to a new django-money compatible currency.
|
||||
"""
|
||||
|
||||
logger.info("Updating currency references for SupplierPriceBreak model...")
|
||||
logger.debug("Updating currency references for SupplierPriceBreak model...")
|
||||
|
||||
# A list of available currency codes
|
||||
currency_codes = CURRENCIES.keys()
|
||||
|
@ -1678,17 +1678,33 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
|
||||
|
||||
return pricing
|
||||
|
||||
def schedule_pricing_update(self):
|
||||
def schedule_pricing_update(self, create: bool = False):
|
||||
"""Helper function to schedule a pricing update.
|
||||
|
||||
Importantly, catches any errors which may occur during deletion of related objects,
|
||||
in particular due to post_delete signals.
|
||||
|
||||
Ref: https://github.com/inventree/InvenTree/pull/3986
|
||||
|
||||
Arguments:
|
||||
create: Whether or not a new PartPricing object should be created if it does not already exist
|
||||
"""
|
||||
|
||||
pricing = self.pricing
|
||||
pricing.schedule_for_update()
|
||||
try:
|
||||
self.refresh_from_db()
|
||||
except Part.DoesNotExist:
|
||||
return
|
||||
|
||||
try:
|
||||
pricing = self.pricing
|
||||
|
||||
if create or pricing.pk:
|
||||
pricing.schedule_for_update()
|
||||
except IntegrityError:
|
||||
# If this part instance has been deleted,
|
||||
# some post-delete or post-save signals may still be fired
|
||||
# which can cause issues down the track
|
||||
pass
|
||||
|
||||
def get_price_info(self, quantity=1, buy=True, bom=True, internal=False):
|
||||
"""Return a simplified pricing string for this part.
|
||||
@ -2261,6 +2277,10 @@ class PartPricing(common.models.MetaMixin):
|
||||
def schedule_for_update(self, counter: int = 0):
|
||||
"""Schedule this pricing to be updated"""
|
||||
|
||||
if not self.part or not self.part.pk or not Part.objects.filter(pk=self.part.pk).exists():
|
||||
logger.warning("Referenced part instance does not exist - skipping pricing update.")
|
||||
return
|
||||
|
||||
try:
|
||||
if self.pk:
|
||||
self.refresh_from_db()
|
||||
@ -3710,7 +3730,7 @@ def update_pricing_after_edit(sender, instance, created, **kwargs):
|
||||
|
||||
# Update part pricing *unless* we are importing data
|
||||
if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
|
||||
instance.part.schedule_pricing_update()
|
||||
instance.part.schedule_pricing_update(create=True)
|
||||
|
||||
|
||||
@receiver(post_delete, sender=BomItem, dispatch_uid='post_delete_bom_item')
|
||||
@ -3721,7 +3741,7 @@ def update_pricing_after_delete(sender, instance, **kwargs):
|
||||
|
||||
# Update part pricing *unless* we are importing data
|
||||
if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
|
||||
instance.part.schedule_pricing_update()
|
||||
instance.part.schedule_pricing_update(create=False)
|
||||
|
||||
|
||||
class BomItemSubstitute(models.Model):
|
||||
|
@ -448,3 +448,40 @@ class PartPricingTests(InvenTreeTestCase):
|
||||
from django_q.models import OrmQ
|
||||
|
||||
self.assertEqual(OrmQ.objects.count(), 101)
|
||||
|
||||
def test_delete_part_with_stock_items(self):
|
||||
"""Test deleting a part instance with stock items.
|
||||
|
||||
This is to test a specific edge condition which was discovered that caused an IntegrityError.
|
||||
Ref: https://github.com/inventree/InvenTree/issues/4419
|
||||
|
||||
Essentially a series of on_delete listeners caused a new PartPricing object to be created,
|
||||
but it pointed to a Part instance which was slated to be deleted inside an atomic transaction.
|
||||
"""
|
||||
|
||||
p = part.models.Part.objects.create(
|
||||
name="my part",
|
||||
description="my part description",
|
||||
active=False,
|
||||
)
|
||||
|
||||
# Create some stock items
|
||||
for _idx in range(3):
|
||||
stock.models.StockItem.objects.create(
|
||||
part=p,
|
||||
quantity=10,
|
||||
purchase_price=Money(10, 'USD')
|
||||
)
|
||||
|
||||
# Check that a PartPricing object exists
|
||||
self.assertTrue(part.models.PartPricing.objects.filter(part=p).exists())
|
||||
|
||||
# Delete the part
|
||||
p.delete()
|
||||
|
||||
# Check that the PartPricing object has been deleted
|
||||
self.assertFalse(part.models.PartPricing.objects.filter(part=p).exists())
|
||||
|
||||
# Try to update pricing (should fail gracefully as the Part has been deleted)
|
||||
p.schedule_pricing_update(create=False)
|
||||
self.assertFalse(part.models.PartPricing.objects.filter(part=p).exists())
|
||||
|
@ -2004,8 +2004,8 @@ def after_delete_stock_item(sender, instance: StockItem, **kwargs):
|
||||
InvenTree.tasks.offload_task(part_tasks.notify_low_stock_if_required, instance.part)
|
||||
|
||||
# Schedule an update on parent part pricing
|
||||
if InvenTree.ready.canAppAccessDatabase():
|
||||
instance.part.schedule_pricing_update()
|
||||
if InvenTree.ready.canAppAccessDatabase(allow_test=True):
|
||||
instance.part.schedule_pricing_update(create=False)
|
||||
|
||||
|
||||
@receiver(post_save, sender=StockItem, dispatch_uid='stock_item_post_save_log')
|
||||
@ -2017,8 +2017,8 @@ def after_save_stock_item(sender, instance: StockItem, created, **kwargs):
|
||||
# Run this check in the background
|
||||
InvenTree.tasks.offload_task(part_tasks.notify_low_stock_if_required, instance.part)
|
||||
|
||||
if InvenTree.ready.canAppAccessDatabase():
|
||||
instance.part.schedule_pricing_update()
|
||||
if InvenTree.ready.canAppAccessDatabase(allow_test=True):
|
||||
instance.part.schedule_pricing_update(create=True)
|
||||
|
||||
|
||||
class StockItemAttachment(InvenTreeAttachment):
|
||||
|
Loading…
Reference in New Issue
Block a user