mirror of
				https://github.com/inventree/InvenTree.git
				synced 2025-10-30 20:55:42 +00:00 
			
		
		
		
	Merge pull request #1705 from SchrodingersGat/part-validation
API Validation fixes
This commit is contained in:
		| @@ -6,12 +6,15 @@ Serializers used in various InvenTree apps | ||||
| # -*- coding: utf-8 -*- | ||||
| from __future__ import unicode_literals | ||||
|  | ||||
| from rest_framework import serializers | ||||
|  | ||||
| import os | ||||
|  | ||||
| from django.conf import settings | ||||
| from django.contrib.auth.models import User | ||||
| from django.core.exceptions import ValidationError as DjangoValidationError | ||||
|  | ||||
| from rest_framework import serializers | ||||
| from rest_framework.fields import empty | ||||
| from rest_framework.exceptions import ValidationError | ||||
|  | ||||
|  | ||||
| class UserSerializer(serializers.ModelSerializer): | ||||
| @@ -39,18 +42,34 @@ class InvenTreeModelSerializer(serializers.ModelSerializer): | ||||
|     but also ensures that the underlying model class data are checked on validation. | ||||
|     """ | ||||
|  | ||||
|     def validate(self, data): | ||||
|     def run_validation(self, data=empty): | ||||
|         """ Perform serializer validation. | ||||
|         In addition to running validators on the serializer fields, | ||||
|         this class ensures that the underlying model is also validated. | ||||
|         """ | ||||
|  | ||||
|         # Run any native validation checks first (may throw an ValidationError) | ||||
|         data = super(serializers.ModelSerializer, self).validate(data) | ||||
|         # Run any native validation checks first (may raise a ValidationError) | ||||
|         data = super().run_validation(data) | ||||
|  | ||||
|         # Now ensure the underlying model is correct | ||||
|         instance = self.Meta.model(**data) | ||||
|         instance.clean() | ||||
|  | ||||
|         if not hasattr(self, 'instance') or self.instance is None: | ||||
|             # No instance exists (we are creating a new one) | ||||
|             instance = self.Meta.model(**data) | ||||
|         else: | ||||
|             # Instance already exists (we are updating!) | ||||
|             instance = self.instance | ||||
|  | ||||
|             # Update instance fields | ||||
|             for attr, value in data.items(): | ||||
|                 setattr(instance, attr, value) | ||||
|  | ||||
|         # Run a 'full_clean' on the model. | ||||
|         # Note that by default, DRF does *not* perform full model validation! | ||||
|         try: | ||||
|             instance.full_clean() | ||||
|         except (ValidationError, DjangoValidationError) as exc: | ||||
|             raise ValidationError(detail=serializers.as_serializer_error(exc)) | ||||
|  | ||||
|         return data | ||||
|  | ||||
|   | ||||
| @@ -40,7 +40,8 @@ def assign_bom_items(apps, schema_editor): | ||||
|         except BomItem.DoesNotExist: | ||||
|             pass | ||||
|  | ||||
|     print(f"Assigned BomItem for {count_valid}/{count_total} entries") | ||||
|     if count_total > 0: | ||||
|         print(f"Assigned BomItem for {count_valid}/{count_total} entries") | ||||
|  | ||||
|  | ||||
| def unassign_bom_items(apps, schema_editor): | ||||
|   | ||||
| @@ -71,7 +71,8 @@ def migrate_currencies(apps, schema_editor): | ||||
|  | ||||
|         count += 1 | ||||
|  | ||||
|     print(f"Updated {count} SupplierPriceBreak rows") | ||||
|     if count > 0: | ||||
|         print(f"Updated {count} SupplierPriceBreak rows") | ||||
|  | ||||
| def reverse_currencies(apps, schema_editor): | ||||
|     """ | ||||
|   | ||||
| @@ -119,7 +119,9 @@ class ManufacturerTest(InvenTreeAPITestCase): | ||||
|         data = { | ||||
|             'MPN': 'MPN-TEST-123', | ||||
|         } | ||||
|          | ||||
|         response = self.client.patch(url, data, format='json') | ||||
|  | ||||
|         self.assertEqual(response.status_code, status.HTTP_200_OK) | ||||
|         self.assertEqual(response.data['MPN'], 'MPN-TEST-123') | ||||
|  | ||||
|   | ||||
| @@ -6,7 +6,7 @@ | ||||
|     name: 'M2x4 LPHS' | ||||
|     description: 'M2x4 low profile head screw' | ||||
|     category: 8 | ||||
|     link: www.acme.com/parts/m2x4lphs | ||||
|     link: http://www.acme.com/parts/m2x4lphs | ||||
|     tree_id: 0 | ||||
|     purchaseable: True | ||||
|     level: 0 | ||||
| @@ -56,6 +56,7 @@ | ||||
|   fields: | ||||
|     name: 'C_22N_0805' | ||||
|     description: '22nF capacitor in 0805 package' | ||||
|     purchaseable: true | ||||
|     category: 3 | ||||
|     tree_id: 0 | ||||
|     level: 0 | ||||
|   | ||||
| @@ -71,7 +71,8 @@ def migrate_currencies(apps, schema_editor): | ||||
|  | ||||
|         count += 1 | ||||
|  | ||||
|     print(f"Updated {count} SupplierPriceBreak rows") | ||||
|     if count > 0: | ||||
|         print(f"Updated {count} SupplierPriceBreak rows") | ||||
|  | ||||
| def reverse_currencies(apps, schema_editor): | ||||
|     """ | ||||
|   | ||||
							
								
								
									
										17
									
								
								InvenTree/part/migrations/0068_part_unique_part.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										17
									
								
								InvenTree/part/migrations/0068_part_unique_part.py
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,17 @@ | ||||
| # Generated by Django 3.2.4 on 2021-06-21 23:10 | ||||
|  | ||||
| from django.db import migrations, models | ||||
|  | ||||
|  | ||||
| class Migration(migrations.Migration): | ||||
|  | ||||
|     dependencies = [ | ||||
|         ('part', '0067_partinternalpricebreak'), | ||||
|     ] | ||||
|  | ||||
|     operations = [ | ||||
|         migrations.AddConstraint( | ||||
|             model_name='part', | ||||
|             constraint=models.UniqueConstraint(fields=('name', 'IPN', 'revision'), name='unique_part'), | ||||
|         ), | ||||
|     ] | ||||
| @@ -321,6 +321,9 @@ class Part(MPTTModel): | ||||
|         verbose_name = _("Part") | ||||
|         verbose_name_plural = _("Parts") | ||||
|         ordering = ['name', ] | ||||
|         constraints = [ | ||||
|             UniqueConstraint(fields=['name', 'IPN', 'revision'], name='unique_part') | ||||
|         ] | ||||
|  | ||||
|     class MPTTMeta: | ||||
|         # For legacy reasons the 'variant_of' field is used to indicate the MPTT parent | ||||
| @@ -379,7 +382,7 @@ class Part(MPTTModel): | ||||
|                     logger.info(f"Deleting unused image file '{previous.image}'") | ||||
|                     previous.image.delete(save=False) | ||||
|  | ||||
|         self.clean() | ||||
|         self.full_clean() | ||||
|  | ||||
|         super().save(*args, **kwargs) | ||||
|  | ||||
| @@ -642,23 +645,6 @@ class Part(MPTTModel): | ||||
|                     'IPN': _('Duplicate IPN not allowed in part settings'), | ||||
|                 }) | ||||
|  | ||||
|         # Part name uniqueness should be case insensitive | ||||
|         try: | ||||
|             parts = Part.objects.exclude(id=self.id).filter( | ||||
|                 name__iexact=self.name, | ||||
|                 IPN__iexact=self.IPN, | ||||
|                 revision__iexact=self.revision) | ||||
|  | ||||
|             if parts.exists(): | ||||
|                 msg = _("Part must be unique for name, IPN and revision") | ||||
|                 raise ValidationError({ | ||||
|                     "name": msg, | ||||
|                     "IPN": msg, | ||||
|                     "revision": msg, | ||||
|                 }) | ||||
|         except Part.DoesNotExist: | ||||
|             pass | ||||
|  | ||||
|     def clean(self): | ||||
|         """ | ||||
|         Perform cleaning operations for the Part model | ||||
| @@ -671,8 +657,6 @@ class Part(MPTTModel): | ||||
|  | ||||
|         super().clean() | ||||
|  | ||||
|         self.validate_unique() | ||||
|  | ||||
|         if self.trackable: | ||||
|             for part in self.get_used_in().all(): | ||||
|  | ||||
|   | ||||
| @@ -1,8 +1,10 @@ | ||||
| # -*- coding: utf-8 -*- | ||||
|  | ||||
| from rest_framework import status | ||||
|  | ||||
| from django.urls import reverse | ||||
|  | ||||
| from part.models import Part | ||||
| from part.models import Part, PartCategory | ||||
| from stock.models import StockItem | ||||
| from company.models import Company | ||||
|  | ||||
| @@ -230,6 +232,18 @@ class PartAPITest(InvenTreeAPITestCase): | ||||
|         response = self.client.get(url, data={'part': 10004}) | ||||
|         self.assertEqual(len(response.data), 7) | ||||
|  | ||||
|         # Try to post a new object (missing description) | ||||
|         response = self.client.post( | ||||
|             url, | ||||
|             data={ | ||||
|                 'part': 10000, | ||||
|                 'test_name': 'My very first test', | ||||
|                 'required': False, | ||||
|             } | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 400) | ||||
|  | ||||
|         # Try to post a new object (should succeed) | ||||
|         response = self.client.post( | ||||
|             url, | ||||
| @@ -237,6 +251,7 @@ class PartAPITest(InvenTreeAPITestCase): | ||||
|                 'part': 10000, | ||||
|                 'test_name': 'New Test', | ||||
|                 'required': True, | ||||
|                 'description': 'a test description' | ||||
|             }, | ||||
|             format='json', | ||||
|         ) | ||||
| @@ -248,7 +263,8 @@ class PartAPITest(InvenTreeAPITestCase): | ||||
|             url, | ||||
|             data={ | ||||
|                 'part': 10004, | ||||
|                 'test_name': "   newtest" | ||||
|                 'test_name': "   newtest", | ||||
|                 'description': 'dafsdf', | ||||
|             }, | ||||
|             format='json', | ||||
|         ) | ||||
| @@ -293,6 +309,171 @@ class PartAPITest(InvenTreeAPITestCase): | ||||
|             self.assertEqual(len(data['results']), n) | ||||
|  | ||||
|  | ||||
| class PartDetailTests(InvenTreeAPITestCase): | ||||
|     """ | ||||
|     Test that we can create / edit / delete Part objects via the API | ||||
|     """ | ||||
|  | ||||
|     fixtures = [ | ||||
|         'category', | ||||
|         'part', | ||||
|         'location', | ||||
|         'bom', | ||||
|         'test_templates', | ||||
|     ] | ||||
|  | ||||
|     roles = [ | ||||
|         'part.change', | ||||
|         'part.add', | ||||
|         'part.delete', | ||||
|         'part_category.change', | ||||
|         'part_category.add', | ||||
|     ] | ||||
|  | ||||
|     def setUp(self): | ||||
|         super().setUp() | ||||
|  | ||||
|     def test_part_operations(self): | ||||
|         n = Part.objects.count() | ||||
|  | ||||
|         # Create a part | ||||
|         response = self.client.post( | ||||
|             reverse('api-part-list'), | ||||
|             { | ||||
|                 'name': 'my test api part', | ||||
|                 'description': 'a part created with the API', | ||||
|                 'category': 1, | ||||
|             } | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 201) | ||||
|  | ||||
|         pk = response.data['pk'] | ||||
|  | ||||
|         # Check that a new part has been added | ||||
|         self.assertEqual(Part.objects.count(), n + 1) | ||||
|  | ||||
|         part = Part.objects.get(pk=pk) | ||||
|  | ||||
|         self.assertEqual(part.name, 'my test api part') | ||||
|  | ||||
|         # Edit the part | ||||
|         url = reverse('api-part-detail', kwargs={'pk': pk}) | ||||
|  | ||||
|         # Let's change the name of the part | ||||
|  | ||||
|         response = self.client.patch(url, { | ||||
|             'name': 'a new better name', | ||||
|         }) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 200) | ||||
|         self.assertEqual(response.data['pk'], pk) | ||||
|         self.assertEqual(response.data['name'], 'a new better name') | ||||
|  | ||||
|         part = Part.objects.get(pk=pk) | ||||
|  | ||||
|         # Name has been altered | ||||
|         self.assertEqual(part.name, 'a new better name') | ||||
|  | ||||
|         # Part count should not have changed | ||||
|         self.assertEqual(Part.objects.count(), n + 1) | ||||
|  | ||||
|         # Now, try to set the name to the *same* value | ||||
|         # 2021-06-22 this test is to check that the "duplicate part" checks don't do strange things | ||||
|         response = self.client.patch(url, { | ||||
|             'name': 'a new better name', | ||||
|         }) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 200) | ||||
|  | ||||
|         # Try to remove the part | ||||
|         response = self.client.delete(url) | ||||
|      | ||||
|         self.assertEqual(response.status_code, 204) | ||||
|  | ||||
|         # Part count should have reduced | ||||
|         self.assertEqual(Part.objects.count(), n) | ||||
|  | ||||
|     def test_duplicates(self): | ||||
|         """ | ||||
|         Check that trying to create 'duplicate' parts results in errors | ||||
|         """ | ||||
|  | ||||
|         # Create a part | ||||
|         response = self.client.post(reverse('api-part-list'), { | ||||
|             'name': 'part', | ||||
|             'description': 'description', | ||||
|             'IPN': 'IPN-123', | ||||
|             'category': 1, | ||||
|             'revision': 'A', | ||||
|         }) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 201) | ||||
|  | ||||
|         n = Part.objects.count() | ||||
|  | ||||
|         # Check that we cannot create a duplicate in a different category | ||||
|         response = self.client.post(reverse('api-part-list'), { | ||||
|             'name': 'part', | ||||
|             'description': 'description', | ||||
|             'IPN': 'IPN-123', | ||||
|             'category': 2, | ||||
|             'revision': 'A', | ||||
|         }) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 400) | ||||
|  | ||||
|         # Check that only 1 matching part exists | ||||
|         parts = Part.objects.filter( | ||||
|             name='part', | ||||
|             description='description', | ||||
|             IPN='IPN-123' | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(parts.count(), 1) | ||||
|  | ||||
|         # A new part should *not* have been created | ||||
|         self.assertEqual(Part.objects.count(), n) | ||||
|  | ||||
|         # But a different 'revision' *can* be created | ||||
|         response = self.client.post(reverse('api-part-list'), { | ||||
|             'name': 'part', | ||||
|             'description': 'description', | ||||
|             'IPN': 'IPN-123', | ||||
|             'category': 2, | ||||
|             'revision': 'B', | ||||
|         }) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 201) | ||||
|         self.assertEqual(Part.objects.count(), n + 1) | ||||
|  | ||||
|         # Now, check that we cannot *change* an existing part to conflict | ||||
|         pk = response.data['pk'] | ||||
|  | ||||
|         url = reverse('api-part-detail', kwargs={'pk': pk}) | ||||
|  | ||||
|         # Attempt to alter the revision code | ||||
|         response = self.client.patch( | ||||
|             url, | ||||
|             { | ||||
|                 'revision': 'A', | ||||
|             }, | ||||
|             format='json', | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 400) | ||||
|  | ||||
|         # But we *can* change it to a unique revision code | ||||
|         response = self.client.patch( | ||||
|             url, | ||||
|             { | ||||
|                 'revision': 'C', | ||||
|             } | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(response.status_code, 200) | ||||
|  | ||||
|  | ||||
| class PartAPIAggregationTest(InvenTreeAPITestCase): | ||||
|     """ | ||||
|     Tests to ensure that the various aggregation annotations are working correctly... | ||||
| @@ -319,6 +500,8 @@ class PartAPIAggregationTest(InvenTreeAPITestCase): | ||||
|         # Add a new part | ||||
|         self.part = Part.objects.create( | ||||
|             name='Banana', | ||||
|             description='This is a banana', | ||||
|             category=PartCategory.objects.get(pk=1), | ||||
|         ) | ||||
|  | ||||
|         # Create some stock items associated with the part | ||||
|   | ||||
| @@ -11,7 +11,7 @@ from django.core.exceptions import ValidationError | ||||
|  | ||||
| import os | ||||
|  | ||||
| from .models import Part, PartTestTemplate | ||||
| from .models import Part, PartCategory, PartTestTemplate | ||||
| from .models import rename_part_image, match_part_names | ||||
| from .templatetags import inventree_extras | ||||
|  | ||||
| @@ -78,6 +78,61 @@ class PartTest(TestCase): | ||||
|         p = Part.objects.get(pk=100) | ||||
|         self.assertEqual(str(p), "BOB | Bob | A2 - Can we build it?") | ||||
|  | ||||
|     def test_duplicate(self): | ||||
|         """ | ||||
|         Test that we cannot create a "duplicate" Part | ||||
|         """ | ||||
|  | ||||
|         n = Part.objects.count() | ||||
|  | ||||
|         cat = PartCategory.objects.get(pk=1) | ||||
|  | ||||
|         Part.objects.create( | ||||
|             category=cat, | ||||
|             name='part', | ||||
|             description='description', | ||||
|             IPN='IPN', | ||||
|             revision='A', | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(Part.objects.count(), n + 1) | ||||
|  | ||||
|         part = Part( | ||||
|             category=cat, | ||||
|             name='part', | ||||
|             description='description', | ||||
|             IPN='IPN', | ||||
|             revision='A', | ||||
|         ) | ||||
|  | ||||
|         with self.assertRaises(ValidationError): | ||||
|             part.validate_unique() | ||||
|  | ||||
|         try: | ||||
|             part.save() | ||||
|             self.assertTrue(False) | ||||
|         except: | ||||
|             pass | ||||
|  | ||||
|         self.assertEqual(Part.objects.count(), n + 1) | ||||
|  | ||||
|         # But we should be able to create a part with a different revision | ||||
|         part_2 = Part.objects.create( | ||||
|             category=cat, | ||||
|             name='part', | ||||
|             description='description', | ||||
|             IPN='IPN', | ||||
|             revision='B', | ||||
|         ) | ||||
|  | ||||
|         self.assertEqual(Part.objects.count(), n + 2) | ||||
|  | ||||
|         # Now, check that we cannot *change* part_2 to conflict | ||||
|         part_2.revision = 'A' | ||||
|  | ||||
|         with self.assertRaises(ValidationError): | ||||
|             part_2.validate_unique() | ||||
|  | ||||
|     def test_metadata(self): | ||||
|         self.assertEqual(self.r1.name, 'R_2K2_0805') | ||||
|         self.assertEqual(self.r1.get_absolute_url(), '/part/3/') | ||||
| @@ -277,21 +332,24 @@ class PartSettingsTest(TestCase): | ||||
|         """ | ||||
|  | ||||
|         # Create a part | ||||
|         Part.objects.create(name='Hello', description='A thing', IPN='IPN123') | ||||
|         Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='A') | ||||
|  | ||||
|         # Attempt to create a duplicate item (should fail) | ||||
|         with self.assertRaises(ValidationError): | ||||
|             Part.objects.create(name='Hello', description='A thing', IPN='IPN123') | ||||
|             part = Part(name='Hello', description='A thing', IPN='IPN123', revision='A') | ||||
|             part.validate_unique() | ||||
|  | ||||
|         # Attempt to create item with duplicate IPN (should be allowed by default) | ||||
|         Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='B') | ||||
|  | ||||
|         # And attempt again with the same values (should fail) | ||||
|         with self.assertRaises(ValidationError): | ||||
|             Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='B') | ||||
|             part = Part(name='Hello', description='A thing', IPN='IPN123', revision='B') | ||||
|             part.validate_unique() | ||||
|  | ||||
|         # Now update the settings so duplicate IPN values are *not* allowed | ||||
|         InvenTreeSetting.set_setting('PART_ALLOW_DUPLICATE_IPN', False, self.user) | ||||
|  | ||||
|         with self.assertRaises(ValidationError): | ||||
|             Part.objects.create(name='Hello', description='A thing', IPN='IPN123', revision='C') | ||||
|             part = Part(name='Hello', description='A thing', IPN='IPN123', revision='C') | ||||
|             part.full_clean() | ||||
|   | ||||
| @@ -199,7 +199,8 @@ def update_history(apps, schema_editor): | ||||
|                 update_count += 1 | ||||
|  | ||||
|  | ||||
|     print(f"\n==========================\nUpdated {update_count} StockItemHistory entries") | ||||
|     if update_count > 0: | ||||
|         print(f"\n==========================\nUpdated {update_count} StockItemHistory entries") | ||||
|  | ||||
|  | ||||
| def reverse_update(apps, schema_editor): | ||||
|   | ||||
| @@ -26,7 +26,8 @@ def extract_purchase_price(apps, schema_editor): | ||||
|     # Find all the StockItem objects without a purchase_price which point to a PurchaseOrder | ||||
|     items = StockItem.objects.filter(purchase_price=None).exclude(purchase_order=None) | ||||
|  | ||||
|     print(f"Found {items.count()} stock items with missing purchase price information") | ||||
|     if items.count() > 0: | ||||
|         print(f"Found {items.count()} stock items with missing purchase price information") | ||||
|  | ||||
|     update_count = 0 | ||||
|  | ||||
| @@ -56,7 +57,8 @@ def extract_purchase_price(apps, schema_editor): | ||||
|  | ||||
|                     break | ||||
|  | ||||
|     print(f"Updated pricing for {update_count} stock items") | ||||
|     if update_count > 0: | ||||
|         print(f"Updated pricing for {update_count} stock items") | ||||
|  | ||||
| def reverse_operation(apps, schema_editor): | ||||
|     """ | ||||
|   | ||||
		Reference in New Issue
	
	Block a user