From 7d3cd03d6c0c028107024e0c51b20f34d04d9298 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 7 Sep 2021 16:28:57 +1000 Subject: [PATCH 1/4] Add "scheduled_for_deletion" field to StockItem - If set to True, this StockItem will be deleted (soon) by the background worker - As deletion takes significant time, this prevents delete operations from blocking the UI --- InvenTree/stock/api.py | 3 +++ .../0066_stockitem_scheduled_for_deletion.py | 18 ++++++++++++++++++ InvenTree/stock/models.py | 9 ++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 InvenTree/stock/migrations/0066_stockitem_scheduled_for_deletion.py diff --git a/InvenTree/stock/api.py b/InvenTree/stock/api.py index 7ff5c55bbe..017457f600 100644 --- a/InvenTree/stock/api.py +++ b/InvenTree/stock/api.py @@ -653,6 +653,9 @@ class StockList(generics.ListCreateAPIView): queryset = StockItemSerializer.annotate_queryset(queryset) + # Do not expose StockItem objects which are scheduled for deletion + queryset = queryset.filter(scheduled_for_deletion=False) + return queryset def filter_queryset(self, queryset): diff --git a/InvenTree/stock/migrations/0066_stockitem_scheduled_for_deletion.py b/InvenTree/stock/migrations/0066_stockitem_scheduled_for_deletion.py new file mode 100644 index 0000000000..45922d2f9f --- /dev/null +++ b/InvenTree/stock/migrations/0066_stockitem_scheduled_for_deletion.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.4 on 2021-09-07 06:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('stock', '0065_auto_20210701_0509'), + ] + + operations = [ + migrations.AddField( + model_name='stockitem', + name='scheduled_for_deletion', + field=models.BooleanField(default=False, help_text='This StockItem will be deleted by the background worker', verbose_name='Scheduled for deletion'), + ), + ] diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index 11d5de9bf1..992008b10e 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -209,7 +209,8 @@ class StockItem(MPTTModel): belongs_to=None, customer=None, is_building=False, - status__in=StockStatus.AVAILABLE_CODES + status__in=StockStatus.AVAILABLE_CODES, + scheduled_for_deletion=False, ) # A query filter which can be used to filter StockItem objects which have expired @@ -588,6 +589,12 @@ class StockItem(MPTTModel): help_text=_('Select Owner'), related_name='stock_items') + scheduled_for_deletion = models.BooleanField( + default=False, + verbose_name=_('Scheduled for deletion'), + help_text=_('This StockItem will be deleted by the background worker'), + ) + def is_stale(self): """ Returns True if this Stock item is "stale". From 918106c225130ba85ad2fe766296efbf7c09f752 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 7 Sep 2021 16:45:58 +1000 Subject: [PATCH 2/4] Adds a background task to remove StockItem objects which are scheduled for deletion --- InvenTree/InvenTree/apps.py | 7 +++++++ InvenTree/stock/models.py | 11 ++++++++--- InvenTree/stock/tasks.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 InvenTree/stock/tasks.py diff --git a/InvenTree/InvenTree/apps.py b/InvenTree/InvenTree/apps.py index ee86b975dc..6456c5994f 100644 --- a/InvenTree/InvenTree/apps.py +++ b/InvenTree/InvenTree/apps.py @@ -63,6 +63,13 @@ class InvenTreeConfig(AppConfig): schedule_type=Schedule.DAILY, ) + # Delete "old" stock items + InvenTree.tasks.schedule_task( + 'stock.tasks.delete_old_stock_items', + schedule_type=Schedule.MINUTES, + minutes=30, + ) + def update_exchange_rates(self): """ Update exchange rates each time the server is started, *if*: diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index 992008b10e..b125fb55c9 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -216,6 +216,11 @@ class StockItem(MPTTModel): # A query filter which can be used to filter StockItem objects which have expired EXPIRED_FILTER = IN_STOCK_FILTER & ~Q(expiry_date=None) & Q(expiry_date__lt=datetime.now().date()) + def mark_for_deletion(self): + + self.scheduled_for_deletion = True + self.save() + def save(self, *args, **kwargs): """ Save this StockItem to the database. Performs a number of checks: @@ -1300,10 +1305,10 @@ class StockItem(MPTTModel): self.quantity = quantity - if quantity == 0 and self.delete_on_deplete and self.can_delete(): - # TODO - Do not actually "delete" stock at this point - instead give it a "DELETED" flag - self.delete() + if quantity == 0 and self.delete_on_deplete and self.can_delete(): + self.mark_for_deletion() + return False else: self.save() diff --git a/InvenTree/stock/tasks.py b/InvenTree/stock/tasks.py new file mode 100644 index 0000000000..b86e4e8ca9 --- /dev/null +++ b/InvenTree/stock/tasks.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +import logging + +from django.core.exceptions import AppRegistryNotReady + + +logger = logging.getLogger('inventree') + + +def delete_old_stock_items(): + """ + This function removes StockItem objects which have been marked for deletion. + + Bulk "delete" operations for database entries with foreign-key relationships + can be pretty expensive, and thus can "block" the UI for a period of time. + + Thus, instead of immediately deleting multiple StockItems, some UI actions + simply mark each StockItem as "scheduled for deletion". + + The background worker then manually deletes these at a later stage + """ + + try: + from stock.models import StockItem + except AppRegistryNotReady: + logger.info("Could not delete scheduled StockItems - AppRegistryNotReady") + return + + items = StockItem.objects.filter(scheduled_for_deletion=True) + + logger.info(f"Removing {items.count()} StockItem objects scheduled for deletion") + + items.delete() From 5ab4be7025426c1b110978295dfa8ab019bf565f Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 7 Sep 2021 17:36:53 +1000 Subject: [PATCH 3/4] Unit test fixes --- InvenTree/stock/models.py | 1 - InvenTree/stock/tasks.py | 6 +++--- InvenTree/stock/tests.py | 12 ++++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index b125fb55c9..3344dec0eb 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -1305,7 +1305,6 @@ class StockItem(MPTTModel): self.quantity = quantity - if quantity == 0 and self.delete_on_deplete and self.can_delete(): self.mark_for_deletion() diff --git a/InvenTree/stock/tasks.py b/InvenTree/stock/tasks.py index b86e4e8ca9..9fbd875384 100644 --- a/InvenTree/stock/tasks.py +++ b/InvenTree/stock/tasks.py @@ -30,6 +30,6 @@ def delete_old_stock_items(): items = StockItem.objects.filter(scheduled_for_deletion=True) - logger.info(f"Removing {items.count()} StockItem objects scheduled for deletion") - - items.delete() + if items.count() > 0: + logger.info(f"Removing {items.count()} StockItem objects scheduled for deletion") + items.delete() diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py index 205fb417a8..52f934544f 100644 --- a/InvenTree/stock/tests.py +++ b/InvenTree/stock/tests.py @@ -332,6 +332,8 @@ class StockTest(TestCase): w1 = StockItem.objects.get(pk=100) w2 = StockItem.objects.get(pk=101) + self.assertFalse(w2.scheduled_for_depletion) + # Take 25 units from w1 (there are only 10 in stock) w1.take_stock(30, None, notes='Took 30') @@ -342,6 +344,16 @@ class StockTest(TestCase): # Take 25 units from w2 (will be deleted) w2.take_stock(30, None, notes='Took 30') + # w2 should now be marked for future deletion + w2 = StockItem.objects.get(pk=101) + self.assertTrue(w2.scheduled_for_depletion) + + from stock.tasks import delete_old_stock_items + + # Now run the "background task" to delete these stock items + delete_old_stock_items() + + # This StockItem should now have been deleted with self.assertRaises(StockItem.DoesNotExist): w2 = StockItem.objects.get(pk=101) From ecc7bd2d5b1a1a604351b24b075475e1dc87b08c Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Tue, 7 Sep 2021 22:27:39 +1000 Subject: [PATCH 4/4] Unit test fixes --- InvenTree/build/test_build.py | 8 +++++++- InvenTree/stock/tests.py | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/InvenTree/build/test_build.py b/InvenTree/build/test_build.py index b572feb14b..30fe8c488b 100644 --- a/InvenTree/build/test_build.py +++ b/InvenTree/build/test_build.py @@ -8,8 +8,9 @@ from django.db.utils import IntegrityError from InvenTree import status_codes as status from build.models import Build, BuildItem, get_next_build_number -from stock.models import StockItem from part.models import Part, BomItem +from stock.models import StockItem +from stock.tasks import delete_old_stock_items class BuildTest(TestCase): @@ -352,6 +353,11 @@ class BuildTest(TestCase): # the original BuildItem objects should have been deleted! self.assertEqual(BuildItem.objects.count(), 0) + self.assertEqual(StockItem.objects.count(), 8) + + # Clean up old stock items + delete_old_stock_items() + # New stock items should have been created! self.assertEqual(StockItem.objects.count(), 7) diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py index 52f934544f..cb1bd406bb 100644 --- a/InvenTree/stock/tests.py +++ b/InvenTree/stock/tests.py @@ -332,7 +332,7 @@ class StockTest(TestCase): w1 = StockItem.objects.get(pk=100) w2 = StockItem.objects.get(pk=101) - self.assertFalse(w2.scheduled_for_depletion) + self.assertFalse(w2.scheduled_for_deletion) # Take 25 units from w1 (there are only 10 in stock) w1.take_stock(30, None, notes='Took 30') @@ -346,7 +346,7 @@ class StockTest(TestCase): # w2 should now be marked for future deletion w2 = StockItem.objects.get(pk=101) - self.assertTrue(w2.scheduled_for_depletion) + self.assertTrue(w2.scheduled_for_deletion) from stock.tasks import delete_old_stock_items