From ea88a03b5ac455e9022ece2024c85fa254c954aa Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 16 May 2020 08:43:57 +1000 Subject: [PATCH] More serial number validation and unit testing - --- InvenTree/part/models.py | 82 ++++++++++++--- InvenTree/part/test_api.py | 2 +- InvenTree/part/test_category.py | 6 +- InvenTree/part/test_part.py | 2 + InvenTree/stock/fixtures/stock.yaml | 156 ++++++++++++++++++++++++++++ InvenTree/stock/models.py | 79 +++++++------- InvenTree/stock/tests.py | 84 ++++++++++++++- 7 files changed, 346 insertions(+), 65 deletions(-) diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index c2cfec8d9b..528d932cf0 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -266,6 +266,48 @@ class Part(MPTTModel): def __str__(self): return "{n} - {d}".format(n=self.full_name, d=self.description) + def check_if_serial_number_exists(self, sn): + """ + Check if a serial number exists for this Part. + + Note: Serial numbers must be unique across an entire Part "tree", + so here we filter by the entire tree. + """ + + parts = Part.objects.filter(tree_id=self.tree_id) + stock = StockModels.StockItem.objects.filter(part__in=parts, serial=sn) + + return stock.exists() + + def get_highest_serial_number(self): + """ + Return the highest serial number for this Part. + + Note: Serial numbers must be unique across an entire Part "tree", + so we filter by the entire tree. + """ + + parts = Part.objects.filter(tree_id=self.tree_id) + stock = StockModels.StockItem.objects.filter(part__in=parts).exclude(serial=None).order_by('-serial') + + if stock.count() > 0: + return stock.first().serial + + # No serial numbers found + return None + + def get_next_serial_number(self): + """ + Return the next-available serial number for this Part. + """ + + n = self.get_highest_serial_number() + + if n is None: + return 1 + else: + return n + 1 + @property def full_name(self): """ Format a 'full name' for this Part. @@ -642,32 +684,40 @@ class Part(MPTTModel): self.sales_order_allocation_count(), ]) - @property - def stock_entries(self): - """ Return all 'in stock' items. To be in stock: + def stock_entries(self, include_variants=True, in_stock=None): + """ Return all stock entries for this Part. - - build_order is None - - sales_order is None - - belongs_to is None + - If this is a template part, include variants underneath this. + + Note: To return all stock-entries for all part variants under this one, + we need to be creative with the filtering. """ - return self.stock_items.filter(StockModels.StockItem.IN_STOCK_FILTER) + if include_variants: + query = StockModels.StockItem.objects.filter(part__in=self.get_descendants(include_self=True)) + else: + query = self.stock_items + + if in_stock is True: + query = query.filter(StockModels.StockItem.IN_STOCK_FILTER) + elif in_stock is False: + query = query.exclude(StockModels.StockItem.IN_STOCK_FILTER) + + return query @property def total_stock(self): """ Return the total stock quantity for this part. - Part may be stored in multiple locations + + - Part may be stored in multiple locations + - If this part is a "template" (variants exist) then these are counted too """ - if self.is_template: - total = sum([variant.total_stock for variant in self.variants.all()]) - else: - total = self.stock_entries.filter(status__in=StockStatus.AVAILABLE_CODES).aggregate(total=Sum('quantity'))['total'] + entries = self.stock_entries(in_stock=True) - if total: - return total - else: - return Decimal(0) + query = entries.aggregate(t=Coalesce(Sum('quantity'), Decimal(0))) + + return query['t'] @property def has_bom(self): diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py index ca3c747b12..61e7f4ab32 100644 --- a/InvenTree/part/test_api.py +++ b/InvenTree/part/test_api.py @@ -85,7 +85,7 @@ class PartAPITest(APITestCase): data = {'cascade': True} response = self.client.get(url, data, format='json') self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.data), 8) + self.assertEqual(len(response.data), 13) def test_get_parts_by_cat(self): url = reverse('api-part-list') diff --git a/InvenTree/part/test_category.py b/InvenTree/part/test_category.py index abd5669016..99d4bce796 100644 --- a/InvenTree/part/test_category.py +++ b/InvenTree/part/test_category.py @@ -88,9 +88,9 @@ class CategoryTest(TestCase): self.assertEqual(self.electronics.partcount(), 3) - self.assertEqual(self.mechanical.partcount(), 4) - self.assertEqual(self.mechanical.partcount(active=True), 3) - self.assertEqual(self.mechanical.partcount(False), 2) + self.assertEqual(self.mechanical.partcount(), 8) + self.assertEqual(self.mechanical.partcount(active=True), 7) + self.assertEqual(self.mechanical.partcount(False), 6) self.assertEqual(self.electronics.item_count, self.electronics.partcount()) diff --git a/InvenTree/part/test_part.py b/InvenTree/part/test_part.py index 0ecb9b5997..622f0af547 100644 --- a/InvenTree/part/test_part.py +++ b/InvenTree/part/test_part.py @@ -63,6 +63,8 @@ class PartTest(TestCase): green = Part.objects.get(pk=10004) self.assertEqual(green.get_ancestors().count(), 2) self.assertEqual(green.get_root(), chair) + self.assertEqual(green.get_family().count(), 3) + self.assertEqual(Part.objects.filter(tree_id=chair.tree_id).count(), 5) def test_str(self): p = Part.objects.get(pk=100) diff --git a/InvenTree/stock/fixtures/stock.yaml b/InvenTree/stock/fixtures/stock.yaml index ebc207f29c..cac34fb01b 100644 --- a/InvenTree/stock/fixtures/stock.yaml +++ b/InvenTree/stock/fixtures/stock.yaml @@ -68,4 +68,160 @@ level: 0 tree_id: 0 lft: 0 + rght: 0 + +# Stock items for template / variant parts +- model: stock.stockitem + pk: 500 + fields: + part: 10001 + location: 7 + quantity: 5 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 501 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 1 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 501 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 1 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 502 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 2 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 503 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 3 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 504 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 4 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 505 + fields: + part: 10001 + location: 7 + quantity: 1 + serial: 5 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 510 + fields: + part: 10002 + location: 7 + quantity: 1 + serial: 10 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 511 + fields: + part: 10002 + location: 7 + quantity: 1 + serial: 11 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 512 + fields: + part: 10002 + location: 7 + quantity: 1 + serial: 12 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 520 + fields: + part: 10004 + location: 7 + quantity: 1 + serial: 20 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 521 + fields: + part: 10004 + location: 7 + quantity: 1 + serial: 21 + level: 0 + tree_id: 0 + lft: 0 + rght: 0 + +- model: stock.stockitem + pk: 522 + fields: + part: 10004 + location: 7 + quantity: 1 + serial: 22 + level: 0 + tree_id: 0 + lft: 0 rght: 0 \ No newline at end of file diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index ecfbf751c9..e7e4223f24 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -142,11 +142,31 @@ class StockItem(MPTTModel): ) def save(self, *args, **kwargs): + """ + Save this StockItem to the database. Performs a number of checks: + + - Unique serial number requirement + - Adds a transaction note when the item is first created. + """ + + # Query to look for duplicate serial numbers + parts = PartModels.Part.objects.filter(tree_id=self.part.tree_id) + stock = StockItem.objects.filter(part__in=parts, serial=self.serial) + if not self.pk: + # StockItem has not yet been saved add_note = True else: + # StockItem has already been saved add_note = False + stock = stock.exclude(pk=self.pk) + + if self.serial is not None: + # Check for presence of stock with same serial number + if stock.exists(): + raise ValidationError({"serial": _("StockItem with this serial number already exists")}) + user = kwargs.pop('user', None) add_note = add_note and kwargs.pop('note', True) @@ -172,37 +192,6 @@ class StockItem(MPTTModel): """ Return True if this StockItem is serialized """ return self.serial is not None and self.quantity == 1 - @classmethod - def check_serial_number(cls, part, serial_number): - """ Check if a new stock item can be created with the provided part_id - - Args: - part: The part to be checked - """ - - if not part.trackable: - return False - - # Return False if an invalid serial number is supplied - try: - serial_number = int(serial_number) - except ValueError: - return False - - items = StockItem.objects.filter(serial=serial_number) - - # Is this part a variant? If so, check S/N across all sibling variants - if part.variant_of is not None: - items = items.filter(part__variant_of=part.variant_of) - else: - items = items.filter(part=part) - - # An existing serial number exists - if items.exists(): - return False - - return True - def validate_unique(self, exclude=None): super(StockItem, self).validate_unique(exclude) @@ -210,18 +199,21 @@ class StockItem(MPTTModel): # ensure that the serial number is unique # across all variants of the same template part + print("validating...") + print(self.pk, self.serial) + try: if self.serial is not None: - # This is a variant part (check S/N across all sibling variants) - if self.part.variant_of is not None: - if StockItem.objects.filter(part__variant_of=self.part.variant_of, serial=self.serial).exclude(id=self.id).exists(): - raise ValidationError({ - 'serial': _('A stock item with this serial number already exists for template part {part}'.format(part=self.part.variant_of)) - }) - else: - if StockItem.objects.filter(part=self.part, serial=self.serial).exclude(id=self.id).exists(): - raise ValidationError({ - 'serial': _('A stock item with this serial number already exists') + + parts = PartModels.Part.objects.filter(tree_id=self.part.tree_id) + stock = StockItem.objects.filter( + part__in=parts, + serial=self.serial, + ).exclude(pk=self.pk) + + if stock.exists(): + raise ValidationError({ + 'serial': _('A stock item with this serial number already exists for this part'), }) except PartModels.Part.DoesNotExist: pass @@ -599,6 +591,9 @@ class StockItem(MPTTModel): if self.serialized: return + if not self.part.trackable: + raise ValidationError({"part": _("Part is not set as trackable")}) + # Quantity must be a valid integer value try: quantity = int(quantity) @@ -624,7 +619,7 @@ class StockItem(MPTTModel): existing = [] for serial in serials: - if not StockItem.check_serial_number(self.part, serial): + if self.part.check_if_serial_number_exists(serial): existing.append(serial) if len(existing) > 0: diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py index a866bdb880..307879629a 100644 --- a/InvenTree/stock/tests.py +++ b/InvenTree/stock/tests.py @@ -38,6 +38,10 @@ class StockTest(TestCase): self.user = User.objects.get(username='username') + # Ensure the MPTT objects are correctly rebuild + Part.objects.rebuild() + StockItem.objects.rebuild() + def test_loc_count(self): self.assertEqual(StockLocation.objects.count(), 7) @@ -91,13 +95,16 @@ class StockTest(TestCase): self.assertFalse(self.drawer2.has_items()) # Drawer 3 should have three stock items - self.assertEqual(self.drawer3.stock_items.count(), 3) - self.assertEqual(self.drawer3.item_count, 3) + self.assertEqual(self.drawer3.stock_items.count(), 15) + self.assertEqual(self.drawer3.item_count, 15) def test_stock_count(self): part = Part.objects.get(pk=1) + entries = part.stock_entries() - # There should be 5000 screws in stock + self.assertEqual(entries.count(), 2) + + # There should be 9000 screws in stock self.assertEqual(part.total_stock, 9000) # There should be 18 widgets in stock @@ -301,6 +308,7 @@ class StockTest(TestCase): item.delete_on_deplete = True item.save() + n = StockItem.objects.filter(part=25).count() self.assertEqual(item.quantity, 10) @@ -327,3 +335,73 @@ class StockTest(TestCase): # Serialize the remainder of the stock item.serializeStock(2, [99, 100], self.user) + + +class VariantTest(StockTest): + """ + Tests for calculation stock counts against templates / variants + """ + + def test_variant_stock(self): + # Check the 'Chair' variant + chair = Part.objects.get(pk=10000) + + # No stock items for the variant part itself + self.assertEqual(chair.stock_entries(include_variants=False).count(), 0) + + self.assertEqual(chair.stock_entries().count(), 12) + + green = Part.objects.get(pk=10003) + self.assertEqual(green.stock_entries(include_variants=False).count(), 0) + self.assertEqual(green.stock_entries().count(), 3) + + def test_serial_numbers(self): + # Test serial number functionality for variant / template parts + + chair = Part.objects.get(pk=10000) + + # Operations on the top-level object + self.assertTrue(chair.check_if_serial_number_exists(1)) + self.assertTrue(chair.check_if_serial_number_exists(2)) + self.assertTrue(chair.check_if_serial_number_exists(3)) + self.assertTrue(chair.check_if_serial_number_exists(4)) + self.assertTrue(chair.check_if_serial_number_exists(5)) + + self.assertTrue(chair.check_if_serial_number_exists(20)) + self.assertTrue(chair.check_if_serial_number_exists(21)) + self.assertTrue(chair.check_if_serial_number_exists(22)) + + self.assertFalse(chair.check_if_serial_number_exists(30)) + + self.assertEqual(chair.get_next_serial_number(), 23) + + # Same operations on a sub-item + variant = Part.objects.get(pk=10003) + self.assertEqual(variant.get_next_serial_number(), 23) + + # Create a new serial number + n = variant.get_highest_serial_number() + + item = StockItem( + part=variant, + quantity=1, + serial=n + ) + + # This should fail + with self.assertRaises(ValidationError): + item.save() + + # This should pass + item.serial = n + 1 + item.save() + + # Attempt to create the same serial number but for a variant (should fail!) + item.pk = None + item.part = Part.objects.get(pk=10004) + + with self.assertRaises(ValidationError): + item.save() + + item.serial += 1 + item.save()