From 17bf7bb5e331f30d3f1f433187257c8b3a3bb00f Mon Sep 17 00:00:00 2001 From: Oliver Date: Fri, 18 Oct 2024 13:45:35 +1100 Subject: [PATCH] [Plugin] Enhancements for serial number validation (#8311) * Error handling for plugin.increment_serial_number * Improve error handling for plugin validation functions * Add Part.get_next_serial_number method - Simplify call for incrementing * ValidationMixin: add "part" to increment_serial_number func * Pass part information through to serial number functions * Fix circular imports * Allow "get_latest_serial_number" to use plugin system * Better working example for plugin sample * Update SampleValidatorPlugin * Fix indent * Add code comment * Cleanup code logic * Revert previous commit * Update unit tests --- src/backend/InvenTree/InvenTree/helpers.py | 32 ++++++++++++----- src/backend/InvenTree/build/serializers.py | 3 +- src/backend/InvenTree/order/serializers.py | 7 ++-- src/backend/InvenTree/part/api.py | 13 +++---- src/backend/InvenTree/part/models.py | 36 ++++++++++++++++--- .../base/integration/ValidationMixin.py | 21 ++++++++++- src/backend/InvenTree/plugin/plugin.py | 6 ++-- .../samples/integration/validation_sample.py | 28 +++++++++++++++ src/backend/InvenTree/stock/api.py | 2 +- src/backend/InvenTree/stock/generators.py | 2 +- src/backend/InvenTree/stock/models.py | 5 +-- src/backend/InvenTree/stock/serializers.py | 6 +++- src/backend/InvenTree/stock/tests.py | 10 ++++-- 13 files changed, 135 insertions(+), 36 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/helpers.py b/src/backend/InvenTree/InvenTree/helpers.py index 0ba509aaef..aa890cf6ad 100644 --- a/src/backend/InvenTree/InvenTree/helpers.py +++ b/src/backend/InvenTree/InvenTree/helpers.py @@ -2,6 +2,7 @@ import datetime import hashlib +import inspect import io import logging import os @@ -432,7 +433,7 @@ def DownloadFile( return response -def increment_serial_number(serial): +def increment_serial_number(serial, part=None): """Given a serial number, (attempt to) generate the *next* serial number. Note: This method is exposed to custom plugins. @@ -443,6 +444,7 @@ def increment_serial_number(serial): Returns: incremented value, or None if incrementing could not be performed. """ + from InvenTree.exceptions import log_error from plugin.registry import registry # Ensure we start with a string value @@ -451,16 +453,30 @@ def increment_serial_number(serial): # First, let any plugins attempt to increment the serial number for plugin in registry.with_mixin('validation'): - result = plugin.increment_serial_number(serial) - if result is not None: - return str(result) + try: + if not hasattr(plugin, 'increment_serial_number'): + continue + + signature = inspect.signature(plugin.increment_serial_number) + + # Note: 2024-08-21 - The 'part' parameter has been added to the signature + if 'part' in signature.parameters: + result = plugin.increment_serial_number(serial, part=part) + else: + result = plugin.increment_serial_number(serial) + if result is not None: + return str(result) + except Exception: + log_error(f'{plugin.slug}.increment_serial_number') # If we get to here, no plugins were able to "increment" the provided serial value # Attempt to perform increment according to some basic rules return increment(serial) -def extract_serial_numbers(input_string, expected_quantity: int, starting_value=None): +def extract_serial_numbers( + input_string, expected_quantity: int, starting_value=None, part=None +): """Extract a list of serial numbers from a provided input string. The input string can be specified using the following concepts: @@ -480,7 +496,7 @@ def extract_serial_numbers(input_string, expected_quantity: int, starting_value= starting_value: Provide a starting value for the sequence (or None) """ if starting_value is None: - starting_value = increment_serial_number(None) + starting_value = increment_serial_number(None, part=part) try: expected_quantity = int(expected_quantity) @@ -497,12 +513,12 @@ def extract_serial_numbers(input_string, expected_quantity: int, starting_value= if len(input_string) == 0: raise ValidationError([_('Empty serial number string')]) - next_value = increment_serial_number(starting_value) + next_value = increment_serial_number(starting_value, part=part) # Substitute ~ character with latest value while '~' in input_string and next_value: input_string = input_string.replace('~', str(next_value), 1) - next_value = increment_serial_number(next_value) + next_value = increment_serial_number(next_value, part=part) # Split input string by whitespace or comma (,) characters groups = re.split(r'[\s,]+', input_string) diff --git a/src/backend/InvenTree/build/serializers.py b/src/backend/InvenTree/build/serializers.py index 2ce969537d..4a7d6e971b 100644 --- a/src/backend/InvenTree/build/serializers.py +++ b/src/backend/InvenTree/build/serializers.py @@ -396,7 +396,8 @@ class BuildOutputCreateSerializer(serializers.Serializer): self.serials = InvenTree.helpers.extract_serial_numbers( serial_numbers, quantity, - part.get_latest_serial_number() + part.get_latest_serial_number(), + part=part ) except DjangoValidationError as e: raise ValidationError({ diff --git a/src/backend/InvenTree/order/serializers.py b/src/backend/InvenTree/order/serializers.py index 6b0d927dae..2e9b0ae30b 100644 --- a/src/backend/InvenTree/order/serializers.py +++ b/src/backend/InvenTree/order/serializers.py @@ -835,7 +835,10 @@ class PurchaseOrderLineItemReceiveSerializer(serializers.Serializer): try: # Pass the serial numbers through to the parent serializer once validated data['serials'] = extract_serial_numbers( - serial_numbers, base_quantity, base_part.get_latest_serial_number() + serial_numbers, + base_quantity, + base_part.get_latest_serial_number(), + part=base_part, ) except DjangoValidationError as e: raise ValidationError({'serial_numbers': e.messages}) @@ -1602,7 +1605,7 @@ class SalesOrderSerialAllocationSerializer(serializers.Serializer): try: data['serials'] = extract_serial_numbers( - serial_numbers, quantity, part.get_latest_serial_number() + serial_numbers, quantity, part.get_latest_serial_number(), part=part ) except DjangoValidationError as e: raise ValidationError({'serial_numbers': e.messages}) diff --git a/src/backend/InvenTree/part/api.py b/src/backend/InvenTree/part/api.py index 5197911efd..f9e52c3f9e 100644 --- a/src/backend/InvenTree/part/api.py +++ b/src/backend/InvenTree/part/api.py @@ -29,7 +29,7 @@ from InvenTree.filters import ( InvenTreeDateFilter, InvenTreeSearchFilter, ) -from InvenTree.helpers import increment_serial_number, isNull, str2bool +from InvenTree.helpers import isNull, str2bool from InvenTree.mixins import ( CreateAPI, CustomRetrieveUpdateDestroyAPI, @@ -811,15 +811,10 @@ class PartSerialNumberDetail(RetrieveAPI): part = self.get_object() # Calculate the "latest" serial number - latest = part.get_latest_serial_number() + latest_serial = part.get_latest_serial_number() + next_serial = part.get_next_serial_number() - data = {'latest': latest} - - if latest is not None: - next_serial = increment_serial_number(latest) - - if next_serial != latest: - data['next'] = next_serial + data = {'latest': latest_serial, 'next': next_serial} return Response(data) diff --git a/src/backend/InvenTree/part/models.py b/src/backend/InvenTree/part/models.py index 6e20eba45e..096bfc9c1b 100644 --- a/src/backend/InvenTree/part/models.py +++ b/src/backend/InvenTree/part/models.py @@ -55,6 +55,7 @@ from common.icons import validate_icon from common.settings import get_global_setting from company.models import SupplierPart from InvenTree import helpers, validators +from InvenTree.exceptions import log_error from InvenTree.fields import InvenTreeURLField from InvenTree.helpers import decimal2money, decimal2string, normalize, str2bool from order import models as OrderModels @@ -659,6 +660,8 @@ class Part( except ValidationError as exc: if raise_error: raise ValidationError({'name': exc.message}) + except Exception: + log_error(f'{plugin.slug}.validate_part_name') def validate_ipn(self, raise_error=True): """Ensure that the IPN (internal part number) is valid for this Part". @@ -678,6 +681,8 @@ class Part( except ValidationError as exc: if raise_error: raise ValidationError({'IPN': exc.message}) + except Exception: + log_error(f'{plugin.slug}.validate_part_ipn') # If we get to here, none of the plugins have raised an error pattern = get_global_setting('PART_IPN_REGEX', '', create=False).strip() @@ -792,6 +797,8 @@ class Part( raise exc else: return False + except Exception: + log_error('part.validate_serial_number') """ If we are here, none of the loaded plugins (if any) threw an error or exited early @@ -868,7 +875,7 @@ class Part( return conflicts - def get_latest_serial_number(self): + def get_latest_serial_number(self, allow_plugins=True): """Find the 'latest' serial number for this Part. Here we attempt to find the "highest" serial number which exists for this Part. @@ -881,15 +888,26 @@ class Part( Returns: The latest serial number specified for this part, or None """ + from plugin.registry import registry + + if allow_plugins: + # Check with plugin system + # If any plugin returns a non-null result, that takes priority + for plugin in registry.with_mixin('validation'): + try: + result = plugin.get_latest_serial_number(self) + if result is not None: + return str(result) + except Exception: + log_error(f'{plugin.slug}.get_latest_serial_number') + + # No plugin returned a result, so we will run the default query stock = ( StockModels.StockItem.objects.all().exclude(serial=None).exclude(serial='') ) # Generate a query for any stock items for this part variant tree with non-empty serial numbers - if get_global_setting('SERIAL_NUMBER_GLOBALLY_UNIQUE', False): - # Serial numbers are unique across all parts - pass - else: + if not get_global_setting('SERIAL_NUMBER_GLOBALLY_UNIQUE', False): # Serial numbers are unique acros part trees stock = stock.filter(part__tree_id=self.tree_id) @@ -903,6 +921,12 @@ class Part( # Return the first serial value return stock[0].serial + def get_next_serial_number(self): + """Return the 'next' serial number in sequence.""" + sn = self.get_latest_serial_number() + + return InvenTree.helpers.increment_serial_number(sn, self) + @property def full_name(self) -> str: """Format a 'full name' for this Part based on the format PART_NAME_FORMAT defined in InvenTree settings.""" @@ -3928,6 +3952,8 @@ class PartParameter(InvenTree.models.InvenTreeMetadataModel): except ValidationError as exc: # Re-throw the ValidationError against the 'data' field raise ValidationError({'data': exc.message}) + except Exception: + log_error(f'{plugin.slug}.validate_part_parameter') def calculate_numeric_value(self): """Calculate a numeric value for the parameter data. diff --git a/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py b/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py index c77eda53af..6aee6c19ce 100644 --- a/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py +++ b/src/backend/InvenTree/plugin/base/integration/ValidationMixin.py @@ -191,7 +191,25 @@ class ValidationMixin: """ return None - def increment_serial_number(self, serial: str) -> str: + def get_latest_serial_number(self, part, **kwargs): + """Return the 'latest' serial number for a given Part instance. + + A plugin which implements this method can either return: + - A string which represents the "latest" serial number + - None (null value) if the latest value could not be determined + + Arguments: + part: The Part instance for which the latest serial number is being requested + + Returns: + The latest serial number (string), or None + """ + # Default implementation returns None + return None + + def increment_serial_number( + self, serial: str, part: part.models.Part = None, **kwargs + ) -> str: """Return the next sequential serial based on the provided value. A plugin which implements this method can either return: @@ -201,6 +219,7 @@ class ValidationMixin: Arguments: serial: Current serial value (string) + part: The Part instance for which this serial number is being incremented Returns: The next serial number in the sequence (string), or None diff --git a/src/backend/InvenTree/plugin/plugin.py b/src/backend/InvenTree/plugin/plugin.py index 4ead8f9302..7ed154cf76 100644 --- a/src/backend/InvenTree/plugin/plugin.py +++ b/src/backend/InvenTree/plugin/plugin.py @@ -14,7 +14,7 @@ from django.urls.base import reverse from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ -from InvenTree.helpers import pui_url +import InvenTree.helpers from plugin.helpers import get_git_log logger = logging.getLogger('inventree') @@ -380,9 +380,9 @@ class InvenTreePlugin(VersionMixin, MixinBase, MetaBase): return f'{reverse("settings")}#select-plugin-{self.slug}' config = self.plugin_config() if config: - return pui_url(f'/settings/admin/plugin/{config.pk}/') + return InvenTree.helpers.pui_url(f'/settings/admin/plugin/{config.pk}/') else: - return pui_url('/settings/admin/plugin/') + return InvenTree.helpers.pui_url('/settings/admin/plugin/') # region package info def _get_package_commit(self): diff --git a/src/backend/InvenTree/plugin/samples/integration/validation_sample.py b/src/backend/InvenTree/plugin/samples/integration/validation_sample.py index c5d7120001..903cc59b34 100644 --- a/src/backend/InvenTree/plugin/samples/integration/validation_sample.py +++ b/src/backend/InvenTree/plugin/samples/integration/validation_sample.py @@ -135,6 +135,34 @@ class SampleValidatorPlugin(SettingsMixin, ValidationMixin, InvenTreePlugin): if serial[0] != part.name[0]: self.raise_error('Serial number must start with same letter as part') + # Prevent serial numbers which are a multiple of 5 + try: + sn = int(serial) + if sn % 5 == 0: + self.raise_error('Serial number cannot be a multiple of 5') + except ValueError: + pass + + def increment_serial_number(self, serial: str, part=None, **kwargs): + """Increment a serial number. + + These examples are silly, but serve to demonstrate how the feature could be used + """ + try: + sn = int(serial) + sn += 1 + + # Skip any serial number which is a multiple of 5 + if sn % 5 == 0: + sn += 1 + + return str(sn) + except ValueError: + pass + + # Return "None" to defer to the next plugin or builtin functionality + return None + def validate_batch_code(self, batch_code: str, item): """Ensure that a particular batch code meets specification. diff --git a/src/backend/InvenTree/stock/api.py b/src/backend/InvenTree/stock/api.py index 47e8031767..9afa8ee2cf 100644 --- a/src/backend/InvenTree/stock/api.py +++ b/src/backend/InvenTree/stock/api.py @@ -997,7 +997,7 @@ class StockList(DataExportViewMixin, ListCreateDestroyAPIView): # If serial numbers are specified, check that they match! try: serials = extract_serial_numbers( - serial_numbers, quantity, part.get_latest_serial_number() + serial_numbers, quantity, part.get_latest_serial_number(), part=part ) # Determine if any of the specified serial numbers are invalid diff --git a/src/backend/InvenTree/stock/generators.py b/src/backend/InvenTree/stock/generators.py index 37accce6e7..f8487a0db6 100644 --- a/src/backend/InvenTree/stock/generators.py +++ b/src/backend/InvenTree/stock/generators.py @@ -99,7 +99,7 @@ def generate_serial_number(part=None, quantity=1, **kwargs) -> str: # Generate the required quantity of serial numbers # Note that this call gets passed through to the plugin system while quantity > 0: - sn = InvenTree.helpers.increment_serial_number(sn) + sn = InvenTree.helpers.increment_serial_number(sn, part=part) # Exit if an empty or duplicated serial is generated if not sn or sn in serials: diff --git a/src/backend/InvenTree/stock/models.py b/src/backend/InvenTree/stock/models.py index 116845fba4..e39f6511b4 100644 --- a/src/backend/InvenTree/stock/models.py +++ b/src/backend/InvenTree/stock/models.py @@ -1629,8 +1629,9 @@ class StockItem( existing = self.part.find_conflicting_serial_numbers(serials) if len(existing) > 0: - exists = ','.join([str(x) for x in existing]) - msg = _('Serial numbers already exist') + f': {exists}' + msg = _('The following serial numbers already exist or are invalid') + msg += ' : ' + msg += ','.join([str(x) for x in existing]) raise ValidationError({'serial_numbers': msg}) # Serialize this StockItem diff --git a/src/backend/InvenTree/stock/serializers.py b/src/backend/InvenTree/stock/serializers.py index c131d84acc..7d8f9e92ee 100644 --- a/src/backend/InvenTree/stock/serializers.py +++ b/src/backend/InvenTree/stock/serializers.py @@ -728,7 +728,10 @@ class SerializeStockItemSerializer(serializers.Serializer): try: serials = InvenTree.helpers.extract_serial_numbers( - serial_numbers, quantity, item.part.get_latest_serial_number() + serial_numbers, + quantity, + item.part.get_latest_serial_number(), + part=item.part, ) except DjangoValidationError as e: raise ValidationError({'serial_numbers': e.messages}) @@ -755,6 +758,7 @@ class SerializeStockItemSerializer(serializers.Serializer): data['serial_numbers'], data['quantity'], item.part.get_latest_serial_number(), + part=item.part, ) item.serializeStock( diff --git a/src/backend/InvenTree/stock/tests.py b/src/backend/InvenTree/stock/tests.py index ba8ca82fee..a7d065930e 100644 --- a/src/backend/InvenTree/stock/tests.py +++ b/src/backend/InvenTree/stock/tests.py @@ -1229,14 +1229,20 @@ class TestResultTest(StockTestBase): self.assertEqual(item2.test_results.count(), 4) # Test StockItem serialization - item2.serializeStock(1, [100], self.user) + # Note: This will create a new StockItem with a new serial number + + with self.assertRaises(ValidationError): + # Serial number #100 will be rejected by the sample plugin + item2.serializeStock(1, [100], self.user) + + item2.serializeStock(1, [101], self.user) # Add a test result to the parent *after* serialization item2.add_test_result(test_name='abcde') self.assertEqual(item2.test_results.count(), 5) - item3 = StockItem.objects.get(serial=100, part=item2.part) + item3 = StockItem.objects.get(serial=101, part=item2.part) self.assertEqual(item3.test_results.count(), 4)