From 782ae133b712f4f2cf6252f9b02aec9b311edcf9 Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 20 Feb 2023 18:48:55 +1100 Subject: [PATCH] Part duplicate bug fix (#4370) * Make 'copy_category_parameters' part of actual serializer * Parameter copying is now handled by the API serializer * Make field not required * linting fixes * pre commit fix * Fix unit tests * Further fix for unit test * Unit tests for category parameter duplication --- InvenTree/part/api.py | 30 --------------- InvenTree/part/models.py | 31 --------------- InvenTree/part/serializers.py | 47 ++++++++++++++++++++++- InvenTree/part/test_api.py | 45 ++++++++++++++++++++++ InvenTree/part/test_param.py | 20 +--------- InvenTree/templates/js/translated/part.js | 3 -- 6 files changed, 92 insertions(+), 84 deletions(-) diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py index d42b60f1bb..233349e554 100644 --- a/InvenTree/part/api.py +++ b/InvenTree/part/api.py @@ -2,7 +2,6 @@ import functools -from django.db import transaction from django.db.models import Count, F, Q from django.http import JsonResponse from django.urls import include, path, re_path @@ -1204,35 +1203,6 @@ class PartList(APIDownloadMixin, ListCreateAPI): else: return Response(data) - @transaction.atomic - def create(self, request, *args, **kwargs): - """We wish to save the user who created this part! - - Note: Implementation copied from DRF class CreateModelMixin - """ - # TODO: Unit tests for this function! - - # Clean up input data - data = self.clean_data(request.data) - - serializer = self.get_serializer(data=data) - serializer.is_valid(raise_exception=True) - - part = serializer.save() - part.creation_user = self.request.user - - # Optionally copy templates from category or parent category - copy_templates = { - 'main': str2bool(data.get('copy_category_templates', False)), - 'parent': str2bool(data.get('copy_parent_templates', False)) - } - - part.save(**{'add_category_templates': copy_templates}) - - headers = self.get_success_headers(serializer.data) - - return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) - def get_queryset(self, *args, **kwargs): """Return an annotated queryset object""" queryset = super().get_queryset(*args, **kwargs) diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index 88976811f3..59e5d78213 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -443,9 +443,6 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel): If the part image has been updated, then check if the "old" (previous) image is still used by another part. If not, it is considered "orphaned" and will be deleted. """ - # Get category templates settings - - add_category_templates = kwargs.pop('add_category_templates', False) if self.pk: previous = Part.objects.get(pk=self.pk) @@ -469,34 +466,6 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel): 'variant_of': _('Invalid choice for parent part'), }) - if add_category_templates: - # Get part category - category = self.category - - if category is not None: - - template_list = [] - - parent_categories = category.get_ancestors(include_self=True) - - for category in parent_categories: - for template in category.get_parameter_templates(): - # Check that template wasn't already added - if template.parameter_template not in template_list: - - template_list.append(template.parameter_template) - - try: - PartParameter.create( - part=self, - template=template.parameter_template, - data=template.default_value, - save=True - ) - except IntegrityError: - # PartParameter already exists - pass - def __str__(self): """Return a string representation of the Part (for use in the admin interface)""" return f"{self.full_name} - {self.description}" diff --git a/InvenTree/part/serializers.py b/InvenTree/part/serializers.py index 21802fc50a..29d1fa8749 100644 --- a/InvenTree/part/serializers.py +++ b/InvenTree/part/serializers.py @@ -2,11 +2,12 @@ import imghdr import io +import logging from decimal import Decimal from django.core.files.base import ContentFile from django.core.validators import MinValueValidator -from django.db import models, transaction +from django.db import IntegrityError, models, transaction from django.db.models import ExpressionWrapper, F, FloatField, Q from django.db.models.functions import Coalesce from django.urls import reverse_lazy @@ -42,6 +43,8 @@ from .models import (BomItem, BomItemSubstitute, Part, PartAttachment, PartSellPriceBreak, PartStar, PartStocktake, PartStocktakeReport, PartTestTemplate) +logger = logging.getLogger("inventree") + class CategorySerializer(InvenTreeModelSerializer): """Serializer for PartCategory.""" @@ -454,6 +457,7 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer): 'duplicate', 'initial_stock', 'initial_supplier', + 'copy_category_parameters' ] read_only_fields = [ @@ -499,6 +503,7 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer): 'duplicate', 'initial_stock', 'initial_supplier', + 'copy_category_parameters' ] return fields @@ -613,6 +618,12 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer): write_only=True, required=False, ) + copy_category_parameters = serializers.BooleanField( + default=True, required=False, + label=_('Copy Category Parameters'), + help_text=_('Copy parameter templates from selected part category'), + ) + @transaction.atomic def create(self, validated_data): """Custom method for creating a new Part instance using this serializer""" @@ -620,9 +631,15 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer): duplicate = validated_data.pop('duplicate', None) initial_stock = validated_data.pop('initial_stock', None) initial_supplier = validated_data.pop('initial_supplier', None) + copy_category_parameters = validated_data.pop('copy_category_parameters', False) instance = super().create(validated_data) + # Save user information + if self.context['request']: + instance.creation_user = self.context['request'].user + instance.save() + # Copy data from original Part if duplicate: original = duplicate['part'] @@ -637,6 +654,34 @@ class PartSerializer(RemoteImageMixin, InvenTreeModelSerializer): if duplicate['copy_parameters']: instance.copy_parameters_from(original) + # Duplicate parameter data from part category (and parents) + if copy_category_parameters and instance.category is not None: + # Get flattened list of parent categories + categories = instance.category.get_ancestors(include_self=True) + + # All parameter templates within these categories + templates = PartCategoryParameterTemplate.objects.filter( + category__in=categories + ) + + for template in templates: + # First ensure that the part doesn't have that parameter + if PartParameter.objects.filter( + part=instance, + template=template.parameter_template + ).exists(): + continue + + try: + PartParameter.create( + part=instance, + template=template.parameter_template, + data=template.default_value, + save=True + ) + except IntegrityError: + logger.error(f"Could not create new PartParameter for part {instance}") + # Create initial stock entry if initial_stock: quantity = initial_stock['quantity'] diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py index 426fc37e05..76097faa8b 100644 --- a/InvenTree/part/test_api.py +++ b/InvenTree/part/test_api.py @@ -1363,6 +1363,51 @@ class PartCreationTests(PartAPITestBase): self.assertEqual(part.bom_items.count(), 4 if bom else 0) self.assertEqual(part.parameters.count(), 2 if params else 0) + def test_category_parameters(self): + """Test that category parameters are correctly applied""" + + cat = PartCategory.objects.get(pk=1) + + # Add some parameter template to the parent category + for pk in [1, 2, 3]: + PartCategoryParameterTemplate.objects.create( + parameter_template=PartParameterTemplate.objects.get(pk=pk), + category=cat, + default_value=f"Value {pk}" + ) + + self.assertEqual(cat.parameter_templates.count(), 3) + + # Creat a new Part, without copying category parameters + data = self.post( + reverse('api-part-list'), + { + 'category': 1, + 'name': 'Some new part', + 'description': 'A new part without parameters', + 'copy_category_parameters': False, + }, + expected_code=201, + ).data + + prt = Part.objects.get(pk=data['pk']) + self.assertEqual(prt.parameters.count(), 0) + + # Create a new part, this time copying category parameters + data = self.post( + reverse('api-part-list'), + { + 'category': 1, + 'name': 'Another new part', + 'description': 'A new part with parameters', + 'copy_category_parameters': True, + }, + expected_code=201, + ).data + + prt = Part.objects.get(pk=data['pk']) + self.assertEqual(prt.parameters.count(), 3) + class PartDetailTests(PartAPITestBase): """Test that we can create / edit / delete Part objects via the API.""" diff --git a/InvenTree/part/test_param.py b/InvenTree/part/test_param.py index 7a13bfa6ce..3c4c8e8644 100644 --- a/InvenTree/part/test_param.py +++ b/InvenTree/part/test_param.py @@ -3,7 +3,7 @@ import django.core.exceptions as django_exceptions from django.test import TestCase, TransactionTestCase -from .models import (Part, PartCategory, PartCategoryParameterTemplate, +from .models import (PartCategory, PartCategoryParameterTemplate, PartParameter, PartParameterTemplate) @@ -70,21 +70,3 @@ class TestCategoryTemplates(TransactionTestCase): n = PartCategoryParameterTemplate.objects.all().count() self.assertEqual(n, 3) - - # Get test part - part = Part.objects.get(pk=1) - - # Get part parameters count - n_param = part.get_parameters().count() - - add_category_templates = { - 'main': True, - 'parent': True, - } - - # Save it with category parameters - part.save(**{'add_category_templates': add_category_templates}) - - # Check new part parameters count - # Only 2 parameters should be added as one already existed with same template - self.assertEqual(n_param + 2, part.get_parameters().count()) diff --git a/InvenTree/templates/js/translated/part.js b/InvenTree/templates/js/translated/part.js index e4a92ae650..5f944fdc7c 100644 --- a/InvenTree/templates/js/translated/part.js +++ b/InvenTree/templates/js/translated/part.js @@ -209,9 +209,6 @@ function partFields(options={}) { delete fields['default_supplier']; fields.copy_category_parameters = { - type: 'boolean', - label: '{% trans "Copy Category Parameters" %}', - help_text: '{% trans "Copy parameter templates from selected part category" %}', value: global_settings.PART_CATEGORY_PARAMETERS, group: 'create', };