From c078831a35ed908f54c6915188f56600e7140e2d Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Mon, 22 May 2023 00:52:48 +1000 Subject: [PATCH] Clean pack units when saving a SupplierPart - Convert to native part units - Handle empty units value - Add unit tests --- InvenTree/InvenTree/conversion.py | 2 +- InvenTree/company/models.py | 71 +++++++---- .../templates/company/supplier_part.html | 11 +- InvenTree/company/test_supplier_parts.py | 114 ++++++++++++++++++ .../migrations/0109_auto_20230517_1048.py | 4 + .../migrations/0111_auto_20230521_1350.py | 4 + 6 files changed, 182 insertions(+), 24 deletions(-) create mode 100644 InvenTree/company/test_supplier_parts.py diff --git a/InvenTree/InvenTree/conversion.py b/InvenTree/InvenTree/conversion.py index 6067a4a071..e5a918a4b0 100644 --- a/InvenTree/InvenTree/conversion.py +++ b/InvenTree/InvenTree/conversion.py @@ -43,7 +43,7 @@ def convert_physical_value(value: str, unit: str = None): unit: Optional unit to convert to, and validate against Raises: - ValidationError: If the value is invalid + ValidationError: If the value is invalid or cannot be converted to the specified unit Returns: The converted quantity, in the specified units diff --git a/InvenTree/company/models.py b/InvenTree/company/models.py index 7350195d0c..b204a7a9fa 100644 --- a/InvenTree/company/models.py +++ b/InvenTree/company/models.py @@ -2,6 +2,7 @@ import os from datetime import datetime +from decimal import Decimal from django.apps import apps from django.core.exceptions import ValidationError @@ -477,10 +478,35 @@ class SupplierPart(MetadataMixin, InvenTreeBarcodeMixin, common.models.MetaMixin """ super().clean() + self.pack_units = self.pack_units.strip() + + # An empty 'pack_units' value is equivalent to '1' + if self.pack_units == '': + self.pack_units = '1' + # Validate that the UOM is compatible with the base part if self.pack_units and self.part: try: - InvenTree.conversion.convert_physical_value(self.pack_units, self.part.units) + # Attempt conversion to specified unit + native_value = InvenTree.conversion.convert_physical_value( + self.pack_units, self.part.units + ) + + # If part units are not provided, value must be dimensionless + if not self.part.units and native_value.units not in ['', 'dimensionless']: + raise ValidationError({ + 'pack_units': _("Pack units must be compatible with the base part units") + }) + + # Native value must be greater than zero + if float(native_value.magnitude) <= 0: + raise ValidationError({ + 'pack_units': _("Pack units must be greater than zero") + }) + + # Update native pack units value + self.pack_units_native = Decimal(native_value.magnitude) + except ValidationError as e: raise ValidationError({ 'pack_units': e.messages @@ -521,21 +547,23 @@ class SupplierPart(MetadataMixin, InvenTreeBarcodeMixin, common.models.MetaMixin super().save(*args, **kwargs) - part = models.ForeignKey('part.Part', on_delete=models.CASCADE, - related_name='supplier_parts', - verbose_name=_('Base Part'), - limit_choices_to={ - 'purchaseable': True, - }, - help_text=_('Select part'), - ) + part = models.ForeignKey( + 'part.Part', on_delete=models.CASCADE, + related_name='supplier_parts', + verbose_name=_('Base Part'), + limit_choices_to={ + 'purchaseable': True, + }, + help_text=_('Select part'), + ) - supplier = models.ForeignKey(Company, on_delete=models.CASCADE, - related_name='supplied_parts', - limit_choices_to={'is_supplier': True}, - verbose_name=_('Supplier'), - help_text=_('Select supplier'), - ) + supplier = models.ForeignKey( + Company, on_delete=models.CASCADE, + related_name='supplied_parts', + limit_choices_to={'is_supplier': True}, + verbose_name=_('Supplier'), + help_text=_('Select supplier'), + ) SKU = models.CharField( max_length=100, @@ -543,12 +571,13 @@ class SupplierPart(MetadataMixin, InvenTreeBarcodeMixin, common.models.MetaMixin help_text=_('Supplier stock keeping unit') ) - manufacturer_part = models.ForeignKey(ManufacturerPart, on_delete=models.CASCADE, - blank=True, null=True, - related_name='supplier_parts', - verbose_name=_('Manufacturer Part'), - help_text=_('Select manufacturer part'), - ) + manufacturer_part = models.ForeignKey( + ManufacturerPart, on_delete=models.CASCADE, + blank=True, null=True, + related_name='supplier_parts', + verbose_name=_('Manufacturer Part'), + help_text=_('Select manufacturer part'), + ) link = InvenTreeURLField( blank=True, null=True, diff --git a/InvenTree/company/templates/company/supplier_part.html b/InvenTree/company/templates/company/supplier_part.html index 917b1ad2fe..86991d184f 100644 --- a/InvenTree/company/templates/company/supplier_part.html +++ b/InvenTree/company/templates/company/supplier_part.html @@ -165,8 +165,15 @@ src="{% static 'img/blank_image.png' %}" {% if part.pack_units %} - {% trans "Units" %} - {% part.pack_units %} {% include "part/part_units.html" with part=part.part %} + + {% trans "Units" %} + {% if part.part.units %} + + [ {% include "part/part_units.html" with part=part.part %}] + + {% endif %} + + {{ part.pack_units }} {% endif %} {% if part.note %} diff --git a/InvenTree/company/test_supplier_parts.py b/InvenTree/company/test_supplier_parts.py new file mode 100644 index 0000000000..31c715808a --- /dev/null +++ b/InvenTree/company/test_supplier_parts.py @@ -0,0 +1,114 @@ +"""Unit tests specific to the SupplierPart model""" + +from decimal import Decimal + +from django.core.exceptions import ValidationError + +from company.models import Company, SupplierPart +from InvenTree.unit_test import InvenTreeTestCase +from part.models import Part + + +class SupplierPartPackUnitsTests(InvenTreeTestCase): + """Unit tests for the SupplierPart pack_units field""" + + def test_pack_units_dimensionless(self): + """Test valid values for the 'pack_units' field""" + + # Create a part without units (dimensionless) + part = Part.objects.create(name='Test Part', description='Test part description', component=True) + + # Create a supplier (company) + company = Company.objects.create(name='Test Company', is_supplier=True) + + # Create a supplier part for this part + sp = SupplierPart.objects.create( + part=part, + supplier=company, + SKU='TEST-SKU' + ) + + # All these values are valid for a dimensionless part + pass_tests = { + '': 1, + '1': 1, + '1.01': 1.01, + '12.000001': 12.000001, + '99.99': 99.99, + } + + # All these values are invalid for a dimensionless part + fail_tests = [ + '1.2m', + '-1', + '0', + '0.0', + '100 feet', + '0 amps' + ] + + for test, expected in pass_tests.items(): + sp.pack_units = test + sp.full_clean() + self.assertEqual(sp.pack_units_native, expected) + + for test in fail_tests: + sp.pack_units = test + with self.assertRaises(ValidationError): + sp.full_clean() + + def test_pack_units(self): + """Test pack_units for a part with a specified dimension""" + + # Create a part with units 'm' + part = Part.objects.create(name='Test Part', description='Test part description', component=True, units='m') + + # Create a supplier (company) + company = Company.objects.create(name='Test Company', is_supplier=True) + + # Create a supplier part for this part + sp = SupplierPart.objects.create( + part=part, + supplier=company, + SKU='TEST-SKU' + ) + + # All these values are valid for a part with dimesion 'm' + pass_tests = { + '': 1, + '1': 1, + '1m': 1, + '1.01m': 1.01, + '1.01': 1.01, + '5 inches': 0.127, + '27 cm': 0.27, + '3km': 3000, + '14 feet': 4.2672, + '0.5 miles': 804.672, + } + + # All these values are invalid for a part with dimension 'm' + # Either the values are invalid, or the units are incomaptible + fail_tests = [ + '-1', + '-1m', + '0', + '0m', + '12 deg', + '57 amps', + '-12 oz', + '17 yaks', + ] + + for test, expected in pass_tests.items(): + sp.pack_units = test + sp.full_clean() + self.assertEqual( + round(Decimal(sp.pack_units_native), 10), + round(Decimal(str(expected)), 10) + ) + + for test in fail_tests: + sp.pack_units = test + with self.assertRaises(ValidationError): + sp.full_clean() diff --git a/InvenTree/part/migrations/0109_auto_20230517_1048.py b/InvenTree/part/migrations/0109_auto_20230517_1048.py index 9b7bfd8430..3067a37741 100644 --- a/InvenTree/part/migrations/0109_auto_20230517_1048.py +++ b/InvenTree/part/migrations/0109_auto_20230517_1048.py @@ -19,6 +19,10 @@ def update_template_units(apps, schema_editor): n_templates = PartParameterTemplate.objects.count() + if n_templates == 0: + # Escape early + return + ureg = InvenTree.conversion.get_unit_registry() n_converted = 0 diff --git a/InvenTree/part/migrations/0111_auto_20230521_1350.py b/InvenTree/part/migrations/0111_auto_20230521_1350.py index 41689200e2..10b1a66f69 100644 --- a/InvenTree/part/migrations/0111_auto_20230521_1350.py +++ b/InvenTree/part/migrations/0111_auto_20230521_1350.py @@ -20,6 +20,10 @@ def migrate_part_units(apps, schema_editor): parts = Part.objects.exclude(units=None).exclude(units='') n_parts = parts.count() + if n_parts == 0: + # Escape early + return + ureg = InvenTree.conversion.get_unit_registry() invalid_units = set()